Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow multiple access / error logs in main config and vhosts, other logging changes #888

Merged
merged 1 commit into from
Oct 6, 2016
Merged

Allow multiple access / error logs in main config and vhosts, other logging changes #888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions manifests/config.pp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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