puppet-nginx
puppet-nginx copied to clipboard
Add support for log_format escape parameter
Pull Request (PR) description
The escape parameter was added to nginx 1.11.8. This optional parameter allow 3 values: 'default', 'json' and 'none'. Setting this parameter is currently tricky do to the way the configuration snippet is generated (note the unbalanced quotes in the middle of the log string):
class { 'nginx':
# ...
log_format => {
json_combined => "escape=json' '{...}",
# ^ ^
# | `-- start quote of the log string
# `---- end quote of the escape format
}
}
Adjust the data type of the log_format parameter to match the "legacy" way of only passing a Hash consisting of a name (String) matching a format string (String), but also accept a Tuple for the escape parameter (Enum) followed by the format string, allowing a less cluttered Puppet manifest:
class { 'nginx':
# ...
log_format => {
json_combined => [
'escape=json',
'{...}',
],
},
}
This Pull Request (PR) fixes the following issues
n/a
nginx is a class
that may have no external impact to Forge modules.
This module is declared in 9 of 578 indexed public Puppetfiles.
These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.
Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.
Maybe it would be better to use a Variant[String,Struct[...]] to accommodate possible future parameters?
class { 'nginx':
# ...
log_format => {
json_combined => {
escape => 'json',
format => '{...}',
},
},
}
Opinions from people using this module?
This seems a little complex, suggest adding an example to the README.
This seems a little complex, suggest adding an example to the README.
The data type is also lacking documentation. Maybe we can add examples at this level, I'll try in the next few days when I continue working on this at $WORK.
In the meantime, to you have an opinion regarding a Tuple (['escape=json', '{...}']) vs. a Struct ({ escape => 'json', format => '{...}' }) for the new way or providing a log format? I was first thinking about a Tuple because it allows a (little) less cumbersome template, but the extra validation when using a Struct would be good to have. Beyond this log_format example, I was thinking about access_log which is currently implemented using multiple parameters and where passing multiple arguments could be error-prone. I suspect there are many similar patterns in the module.
Superseded by #1513. I completely forgot I already opened a PR for it.