diff --git a/manifests/config.pp b/manifests/config.pp index 3755689f5..7faae2577 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -24,6 +24,7 @@ $global_mode = $::nginx::params::global_mode, $log_dir = $::nginx::params::log_dir, $http_access_log = $::nginx::params::http_access_log, + $http_format_log = undef, $nginx_error_log = $::nginx::params::nginx_error_log, $nginx_error_log_severity = 'error', $pid = $::nginx::params::pid, @@ -198,9 +199,15 @@ } } - validate_string($nginx_error_log) + if !(is_string($http_access_log) or is_array($http_access_log)) { + fail('$http_access_log must be either a string or array') + } + + if !(is_string($nginx_error_log) or is_array($nginx_error_log)) { + fail('$nginx_error_log must be either a string or array') + } + validate_re($nginx_error_log_severity,['debug','info','notice','warn','error','crit','alert','emerg'],'$nginx_error_log_severity must be debug, info, notice, warn, error, crit, alert or emerg') - validate_string($http_access_log) validate_string($proxy_headers_hash_bucket_size) validate_bool($super_user) ### END VALIDATIONS ### diff --git a/manifests/resource/vhost.pp b/manifests/resource/vhost.pp index 0a324240f..0f1f4d82d 100644 --- a/manifests/resource/vhost.pp +++ b/manifests/resource/vhost.pp @@ -143,10 +143,15 @@ # [*rewrite_to_https*] - Adds a server directive and rewrite rule to # rewrite to ssl # [*include_files*] - Adds include files to vhost -# [*access_log*] - Where to write access log. May add additional -# options like log format to the end. +# [*access_log*] - Where to write access log (log format can be +# set with $format_log). This can be either a string or an array; in the +# latter case, multiple lines will be created. Additionally, unlike the +# earlier behavior, setting it to 'absent' in the vhost context will remove +# this directive entirely from the vhost stanza, rather than setting a +# default. Can also be disabled for this vhost with the string 'off'. # [*error_log*] - Where to write error log. May add additional -# options like error level to the end. +# options like error level to the end. May set to 'absent', in which case +# it will be omitted in this vhost stanza (and default to nginx.conf setting) # [*passenger_cgi_param*] - Allows one to define additional CGI environment # variables to pass to the backend application # [*passenger_set_header*] - Allows one to set headers to pass to the @@ -292,7 +297,7 @@ $mode = $::nginx::config::global_mode, $maintenance = false, $maintenance_value = 'return 503', - $error_pages = {}, + $error_pages = undef, $locations = {} ) { @@ -408,6 +413,12 @@ fail('$proxy_cache_valid must be a string or an array or false.') } } + if ($access_log != undef) and !(is_array($access_log) or is_string($access_log)) { + fail('$access_log must be a string or array.') + } + if ($error_log != undef) and !(is_array($error_log) or is_string($error_log)) { + fail('$error_log must be a string or array.') + } if ($proxy_method != undef) { validate_string($proxy_method) } @@ -508,12 +519,6 @@ if ($include_files != undef) { validate_array($include_files) } - if ($access_log != undef) { - validate_string($access_log) - } - if ($error_log != undef) { - validate_string($error_log) - } if ($passenger_cgi_param != undef) { validate_hash($passenger_cgi_param) } @@ -587,20 +592,6 @@ } } - # This was a lot to add up in parameter list so add it down here - # Also opted to add more logic here and keep template cleaner which - # unfortunately means resorting to the $varname_real thing - $access_log_real = $access_log ? { - 'off' => 'off', - undef => "${::nginx::config::log_dir}/${name_sanitized}.access.log ${format_log}", - default => "${access_log} ${format_log}", - } - - $error_log_real = $error_log ? { - undef => "${::nginx::config::log_dir}/${name_sanitized}.error.log", - default => $error_log, - } - concat { $config_file: owner => $owner, group => $group, @@ -697,20 +688,6 @@ if ($ssl == true) { # Access and error logs are named differently in ssl template - # This was a lot to add up in parameter list so add it down here - # Also opted to add more logic here and keep template cleaner which - # unfortunately means resorting to the $varname_real thing - $ssl_access_log_real = $access_log ? { - 'off' => 'off', - undef => "${::nginx::config::log_dir}/ssl-${name_sanitized}.access.log ${format_log}", - default => "${access_log} ${format_log}", - } - - $ssl_error_log_real = $error_log ? { - undef => "${::nginx::config::log_dir}/ssl-${name_sanitized}.error.log", - default => $error_log, - } - concat::fragment { "${name_sanitized}-ssl-header": target => $config_file, content => template('nginx/vhost/vhost_ssl_header.erb'), diff --git a/spec/classes/config_spec.rb b/spec/classes/config_spec.rb index 905eb6062..1c09c7392 100644 --- a/spec/classes/config_spec.rb +++ b/spec/classes/config_spec.rb @@ -125,6 +125,15 @@ value: '/path/to/error.log', match: 'error_log /path/to/error.log error;' }, + { + title: 'should set multiple error_logs', + attr: 'nginx_error_log', + value: ['/path/to/error.log', 'syslog:server=localhost'], + match: [ + 'error_log /path/to/error.log error;', + 'error_log syslog:server=localhost error;' + ] + }, { title: 'should set error_log severity level', attr: 'nginx_error_log_severity', @@ -215,6 +224,21 @@ value: '/path/to/access.log', match: ' access_log /path/to/access.log;' }, + { + title: 'should set multiple access_logs', + attr: 'http_access_log', + value: ['/path/to/access.log', 'syslog:server=localhost'], + match: [ + ' access_log /path/to/access.log;', + ' access_log syslog:server=localhost;' + ] + }, + { + title: 'should set custom log format', + attr: 'http_format_log', + value: 'mycustomformat', + match: ' access_log /var/log/nginx/access.log mycustomformat;' + }, { title: 'should set sendfile', attr: 'sendfile', diff --git a/spec/defines/resource_vhost_spec.rb b/spec/defines/resource_vhost_spec.rb index e792c9a0e..cbe564aab 100644 --- a/spec/defines/resource_vhost_spec.rb +++ b/spec/defines/resource_vhost_spec.rb @@ -242,6 +242,15 @@ value: '/path/to/access.log', match: ' access_log /path/to/access.log combined;' }, + { + title: 'should set multiple access_log directives', + attr: 'access_log', + value: ['/path/to/log/1', 'syslog:server=localhost'], + match: [ + ' access_log /path/to/log/1 combined;', + ' access_log syslog:server=localhost combined;' + ] + }, { title: 'should set access_log off', attr: 'access_log', @@ -260,12 +269,33 @@ value: 'custom', match: ' access_log /var/log/nginx/www.rspec.example.com.access.log custom;' }, + { + title: 'should not include access_log in vhost when set to absent', + attr: 'access_log', + value: 'absent', + notmatch: 'access_log' + }, { title: 'should set error_log', attr: 'error_log', value: '/path/to/error.log', match: ' error_log /path/to/error.log;' }, + { + title: 'should allow multiple error_log directives', + attr: 'error_log', + value: ['/path/to/error.log', 'syslog:server=localhost'], + match: [ + ' error_log /path/to/error.log;', + ' error_log syslog:server=localhost;' + ] + }, + { + title: 'should not include error_log in vhost when set to absent', + attr: 'error_log', + value: 'absent', + notmatch: 'error_log' + }, { title: 'should set error_pages', attr: 'error_pages', @@ -566,12 +596,27 @@ value: '/path/to/access.log', match: ' access_log /path/to/access.log combined;' }, + { + title: 'should set multiple access_log directives', + attr: 'access_log', + value: ['/path/to/log/1', 'syslog:server=localhost'], + match: [ + ' access_log /path/to/log/1 combined;', + ' access_log syslog:server=localhost combined;' + ] + }, { title: 'should set access_log off', attr: 'access_log', value: 'off', match: ' access_log off;' }, + { + title: 'should not include access_log in vhost when set to absent', + attr: 'access_log', + value: 'absent', + notmatch: 'access_log' + }, { title: 'should set access_log to syslog', attr: 'access_log', @@ -590,6 +635,21 @@ value: '/path/to/error.log', match: ' error_log /path/to/error.log;' }, + { + title: 'should allow multiple error_log directives', + attr: 'error_log', + value: ['/path/to/error.log', 'syslog:server=localhost'], + match: [ + ' error_log /path/to/error.log;', + ' error_log syslog:server=localhost;' + ] + }, + { + title: 'should not include error_log in vhost when set to absent', + attr: 'error_log', + value: 'absent', + notmatch: 'error_log' + }, { title: 'should set error_pages', attr: 'error_pages', diff --git a/templates/conf.d/nginx.conf.erb b/templates/conf.d/nginx.conf.erb index f7340ee1c..b4d6e6b5d 100644 --- a/templates/conf.d/nginx.conf.erb +++ b/templates/conf.d/nginx.conf.erb @@ -10,7 +10,13 @@ worker_rlimit_nofile <%= @worker_rlimit_nofile %>; <% if @pid -%> pid <%= @pid %>; <% end -%> +<% if @nginx_error_log.is_a?(Array) -%> + <%- @nginx_error_log.each do |log_item| -%> +error_log <%= log_item %> <%= @nginx_error_log_severity %>; + <%- end -%> +<% else -%> error_log <%= @nginx_error_log %> <%= @nginx_error_log_severity %>; +<% end -%> <% if @nginx_cfg_prepend -%> <%- field_width = @nginx_cfg_prepend.inject(0) { |l,(k,v)| k.size > l ? k.size : l } -%> @@ -52,7 +58,13 @@ http { <% end -%> <% end -%> - access_log <%= @http_access_log %>; +<% if @http_access_log.is_a?(Array) -%> + <%- @http_access_log.each do |log_item| -%> + access_log <%= log_item %><% if @http_format_log %> <%= @http_format_log%><% end %>; + <%- end -%> +<% else -%> + access_log <%= @http_access_log %><% if @http_format_log %> <%= @http_format_log%><% end %>; +<% end -%> <% if @sendfile == 'on' -%> sendfile on; diff --git a/templates/vhost/vhost_header.erb b/templates/vhost/vhost_header.erb index 82ca37916..65c32fd86 100644 --- a/templates/vhost/vhost_header.erb +++ b/templates/vhost/vhost_header.erb @@ -141,6 +141,7 @@ server { <% end -%> <% if @index_files.count > 0 -%> index <% Array(@index_files).each do |i| %> <%= i %><% end %>; + <% end -%> <% if defined? @log_by_lua -%> log_by_lua '<%= @log_by_lua %>'; @@ -148,10 +149,30 @@ server { <% if defined? @log_by_lua_file -%> log_by_lua_file "<%= @log_by_lua_file %>"; <% end -%> - - access_log <%= @access_log_real %>; - error_log <%= @error_log_real %>; +<% if @access_log.is_a?(Array) -%> + <%- @access_log.each do |log_item| -%> + access_log <%= log_item %> <%= @format_log %>; + <%- end -%> +<% elsif @access_log == 'absent' -%> +<% elsif @access_log == 'off' -%> + access_log off; +<% elsif not @access_log -%> + access_log <%= scope['::nginx::config::log_dir'] %>/<%= @name_sanitized %>.access.log <%= @format_log %>; +<% else -%> + access_log <%= @access_log %> <%= @format_log %>; +<% end -%> +<% if @error_log.is_a?(Array) -%> + <%- @error_log.each do |log_item| -%> + error_log <%= log_item %>; + <%- end -%> +<% elsif @error_log == 'absent' -%> +<% elsif not @error_log -%> + error_log <%= scope['::nginx::config::log_dir'] %>/<%= @name_sanitized %>.error.log; +<% else -%> + error_log <%= @error_log %>; +<% end -%> <% if @error_pages -%> + <%- @error_pages.keys.sort.each do |key| -%> error_page <%= key %> <%= @error_pages[key] %>; <%- end -%> diff --git a/templates/vhost/vhost_ssl_header.erb b/templates/vhost/vhost_ssl_header.erb index 6613b28d6..382af3280 100644 --- a/templates/vhost/vhost_ssl_header.erb +++ b/templates/vhost/vhost_ssl_header.erb @@ -79,11 +79,32 @@ server { <% end -%> <% if @index_files.count > 0 -%> index <% Array(@index_files).each do |i| %> <%= i %><% end %>; -<% end -%> - access_log <%= @ssl_access_log_real %>; - error_log <%= @ssl_error_log_real %>; +<% end -%> +<% if @access_log.is_a?(Array) -%> + <%- @access_log.each do |log_item| -%> + access_log <%= log_item %> <%= @format_log %>; + <%- end -%> +<% elsif @access_log == 'absent' -%> +<% elsif @access_log == 'off' -%> + access_log off; +<% elsif not @access_log -%> + access_log <%= scope['::nginx::config::log_dir'] %>/ssl-<%= @name_sanitized %>.access.log <%= @format_log %>; +<% else -%> + access_log <%= @access_log %> <%= @format_log %>; +<% end -%> +<% if @error_log.is_a?(Array) -%> + <%- @error_log.each do |log_item| -%> + error_log <%= log_item %>; + <%- end -%> +<% elsif @error_log == 'absent' -%> +<% elsif not @error_log -%> + error_log <%= scope['::nginx::config::log_dir'] %>/ssl-<%= @name_sanitized %>.error.log; +<% else -%> + error_log <%= @error_log %>; +<% end -%> <% if @error_pages -%> + <%- @error_pages.keys.sort.each do |key| -%> error_page <%= key %> <%= @error_pages[key] %>; <%- end -%>