Added support for keep_secret parameter #92
Conversation
|
This looks solid to me. |
|
|
||
| Parameter `keep_secret` allows to prevent outputing actual values to the log. Possible values are: | ||
| * `false` - default, normal behavior | ||
| * `true` - value will be replaced with `[sorry, this is secret]` |
There was a problem hiding this comment.
Can we reword this to make it clear that we're redacting stuff from the logs? Otherwise this is mergable.
There was a problem hiding this comment.
Sure, what formulation did you have in mind?
There was a problem hiding this comment.
Just something along the lines of:
###Passwords and other sensitive information
In order to support sensitive information we have the following parameters available to control how entries within Puppet logfiles look.
####keep_secret
This parameter allows you to prevent outputting actual values contained within the catalog to the logfile. Possible values are:
false: This allows all values to be passed to logfiles. (default)true: The values in the logfiles will be replaced with[redacted sensitive information].
(something like that, then with the corresponding change to sorry this is secret to match the wording!)
There was a problem hiding this comment.
Cheers, I've updated this in accordance with suggestions.
|
This is looking great! The only thing we're missing is unit and acceptance tests. Any chance you could take a look at those? They should be relatively easy in this case (I hope!) as you can crib from similar nearby tests. |
|
Hi Ashley, I've added unit tests for keep_secret, can you have a look if those are what's expected? I'll also try to have a look at the acceptance tests. Cheers |
|
@stepanstipl the unit tests look very good, we're very much looking forward for your acceptance tests |
|
Ok I've added acceptance tests. Not sure why unit tests fail suddenly, they run fine on my machine and seems to be unrelated to changes I've made. |
|
Seems to be issue with new version of rspec-puppet - rodjek/rspec-puppet#200, I've pinned down rspec dependency to |
| @@ -10,6 +10,7 @@ group :development, :test do | |||
| gem 'puppet-lint', :require => false | |||
| gem 'serverspec', :require => false | |||
| gem 'pry', :require => false | |||
| gem 'rspec','<3.0.0', :require => false | |||
There was a problem hiding this comment.
sooner or later we'll have to be rspec compatible.
There was a problem hiding this comment.
Yeah, I agree and I can remove that, but in that case it won't pass Travis CI. Btw. those are not tests that I've added, it's just that rspec recently updated :).
|
Waiting for CLA signature by @stepanstipl @stepanstipl - We require a Contributor License Agreement (CLA) for people who contribute to Puppet, but we have an easy click-through license with instructions, which is available at https://cla.puppetlabs.com/ Note: if your contribution is trivial and you think it may be exempt from the CLA, please post a short reply to this comment with details. http://docs.puppetlabs.com/community/trivial_patch_exemption.html |
|
Hi, I've just signed the CLA |
|
CLA signed by all contributors. |
|
cool. @stepanstipl can you rebase your and squash them down to a few logical commits? |
|
@stepanstipl thanks for the contribution! I'm going to close this for now though, since we haven't heard from you in a while and it needs to be rebased, have conflicts resolved, and be squashed. If this is work you'd still like to see added to the module please address the required work and submit a new PR. |
Typical use case is to use this module for managing sensitive values like passwords, which we do not want in the logs. Using this new parameter
keep_secretwill prevent this.Accepted values:
false- default, normal behaviortrue- value will be replaced with "[sorry, this is secret]"md5- value will be replaced with it's md5 hash