Skip to content

Commit

Permalink
Merge pull request voxpupuli#888 from wyardley/feature_log_behavior
Browse files Browse the repository at this point in the history 
Allow multiple access / error logs in main config and vhosts, allow suppressing log directive in certain contexts, other logging changes
  • Loading branch information
@wyardley
wyardley authored Oct 6, 2016
2 parents 2fabeef + f64c5c1 commit 7271eb0
Show file tree
Hide file tree
Showing 7 changed files with 169 additions and 47 deletions.
11 changes: 9 additions & 2 deletions manifests/config.pp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 ###
Expand Down
53 changes: 15 additions & 38 deletions manifests/resource/vhost.pp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -292,7 +297,7 @@
$mode = $::nginx::config::global_mode,
$maintenance = false,
$maintenance_value = 'return 503',
$error_pages = {},
$error_pages = undef,
$locations = {}
) {

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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'),
Expand Down
24 changes: 24 additions & 0 deletions spec/classes/config_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
60 changes: 60 additions & 0 deletions spec/defines/resource_vhost_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand Down
14 changes: 13 additions & 1 deletion templates/conf.d/nginx.conf.erb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 } -%>
Expand Down Expand Up @@ -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;
Expand Down
27 changes: 24 additions & 3 deletions templates/vhost/vhost_header.erb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,17 +141,38 @@ 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 %>';
<% end -%>
<% 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 -%>
Expand Down
27 changes: 24 additions & 3 deletions templates/vhost/vhost_ssl_header.erb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 -%>
Expand Down

0 comments on commit 7271eb0

Please sign in to comment.