(IAC-1186) Add new $use_servername_for_filenames parameter #2086
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sanfrancrisko
force-pushed
the
IAC-1186/main/sanitize_log_filename
branch
from
f3e0f18 to
8d26539
Compare
Codecov Report
@@ Coverage Diff @@
## main #2086 +/- ##
=======================================
Coverage ? 57.40%
=======================================
Files ? 12
Lines ? 216
Branches ? 0
=======================================
Hits ? 124
Misses ? 92
Partials ? 0 Continue to review full report at Codecov.
|
sanfrancrisko
force-pushed
the
IAC-1186/main/sanitize_log_filename
branch
from
8d26539 to
4aaf37f
Compare
This was referenced Nov 9, 2020
sanfrancrisko
force-pushed
the
IAC-1186/main/sanitize_log_filename
branch
from
4aaf37f to
ed85578
Compare
sheenaajay
reviewed
Nov 10, 2020
Contributor
|
@sanfrancrisko The changes looks good. Thank you. |
Prior to this commit, the `$filename` variable, which is used to construct the filename of Apache's various config and log files, obtained it's default value from the `$name` parameter. As puppetlabs#2064 highlights, it is possible for `$name` to contain spaces in it and this can cause cumbersome log file names, albeit POSIX compliant. Also related is puppetlabs#2068, which changes the `$filename` variable source from `$name` to `$servername`. This arguably seems more appropriate, especially given that `$servername` defaults to `$name` if undefined. This commit attempts to create a satisfactory solution to both puppetlabs#2064 and puppetlabs#2068 by introducing the `$use_servername_for_filenames` param. When set to `true`, a sanitized `$servername` parameter value will be used to construct `$filename`. When undefined or set to `$false`, it will retain the existing behaviour and use the `$name` parameter value. This will default to `false` until the next major release (v6.0.0), after which it will default to `true`. Then, in the subsequent major release (v7.0.0) it will be deprecated altogether and the default behaviour will be to use the sanitized value of `$servername` for the `$filename` var. Warning messages are output to the console to alert users of this change in behaviour.
sanfrancrisko
force-pushed
the
IAC-1186/main/sanitize_log_filename
branch
from
ed85578 to
7e233fe
Compare
sheenaajay
approved these changes
Nov 12, 2020
ekohl
reviewed
Jul 13, 2021
| # value of the $servername parameter. | ||
| # When set to false (default), the existing behaviour of using the $name parameter | ||
| # will remain. | ||
|
|
Collaborator
There was a problem hiding this comment.
This broke puppet-strings: #2165.
Yet another reason why I hate this change and would still be strongly in favor of reverting it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prior to this commit, the
$filenamevariable, which is used toconstruct the filename of Apache's various config and log files,
obtained it's default value from the
$nameparameter.As #2064 highlights, it is possible for
$nameto contain spacesin it and this can cause cumbersome log file names, albeit POSIX
compliant.
Also related is #2068, which changes the
$filenamevariable sourcefrom
$nameto$servername. This arguably seems more appropriate,especially given that
$servernamedefaults to$nameif undefined.This commit attempts to create a satisfactory solution to both #2064
and #2068 by introducing the
$use_servername_for_filenamesparam.When set to
true, a sanitized$servernameparameter value will beused to construct
$filename.When undefined or set to
$false, it will retain the existingbehaviour and use the
$nameparameter value.This will default to
falseuntil the next major release (v6.0.0),after which it will default to
true. Then, in the subsequent majorrelease (v7.0.0) it will be deprecated altogether and the default
behaviour will be to use the sanitized value of
$servernamefor the$filenamevar.Warning messages are output to the console to alert users of this
change in behaviour.
Closes: #2064 , #2068