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.

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

Added keep_secret parameter feature #152

Merged
merged 3 commits into from Feb 2, 2016

Conversation

stepanstipl
Copy link
Contributor

This is followup on pull request #92 (sorry for long delay with addressing the issues).

Added feature keep_secret to redact how are information outputted to the logs.

Typical use case is to use this when managing sensitive values like passwords, which we do not want in the logs. Using this new parameter keep_secret will prevent this.

Accepted values:

false - default, normal behavior
true - value will be replaced with "[redacted sensitive information]"
md5 - value will be replaced with it's md5 hash

@tphoney
Copy link
Contributor

tphoney commented Jul 20, 2015

This needs to be rebased, is this any different to https://docs.puppetlabs.com/references/latest/type.html#file-attribute-show_diff ?

Thanks for putting in the effort and time.

@stepanstipl stepanstipl force-pushed the feature-keep_secret2 branch 2 times, most recently from e89b516 to e030676 Compare July 21, 2015 10:31
@stepanstipl
Copy link
Contributor Author

Hi, yep I have rebased it, it's been a while. I would say it's very similar to show_diff (didn't know about that one). Just the logic here is reverse and in addition allows you to set 'md5' and will output md5 hashes instead of actual values.

Would you prefer to rename this to "show_diff" and align the behaviour with the show_diff from file type? I think it would make sense.

@tphoney
Copy link
Contributor

tphoney commented Oct 29, 2015

Apologies for getting back to this so late. renaming this to "show_diff" makes total sense

@tphoney
Copy link
Contributor

tphoney commented Oct 29, 2015

@stepanstipl thanks for your patience

@bmjen
Copy link
Contributor

bmjen commented Nov 20, 2015

@stepanstipl Thanks for the contribution, have you had a chance to take a look at the comments made by @tphoney and @asasfu ?

@stepanstipl stepanstipl force-pushed the feature-keep_secret2 branch 3 times, most recently from f25f226 to 31e309d Compare November 22, 2015 21:56
@stepanstipl
Copy link
Contributor Author

@bmjen, thanks for a poke, it was kind of waiting on my laptop to be pushed.

So, I've made following changes:

  • renamed to show_diff
  • realigned logic to be same as show_diff from file type
  • with regards to @asasfu I've put in conversion function to handle boolean values entered as strings or symbols
  • updated docs, unit and beaker tests

One note here - as I've tried to follow behaviour of show_diff on file type, it'll change default output. By default global show_diff option (puppet_conf) is false, and because we respect this, values will be redacted. I assume for typical use case this would affect only log files, as if you run puppet with --test it implies --show_diff.

stepanstipl referenced this pull request in stepanstipl/puppet-inifile Nov 22, 2015
def munge_boolean_md5(value)
case value
when true, :true, 'true', :yes, 'yes'
true
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks pretty good, take a look here though https://github.com/puppetlabs/puppet/blob/master/lib/puppet/type/macauthorization.rb#L20-L29 you'll notice it should return :true or :false not true or false as the insync? seems to have an issue if it's not a symbol(I don't know why, just tested it and found it to be the correct method)

stepanstipl added a commit to stepanstipl/puppet-inifile that referenced this pull request Nov 23, 2015
@stepanstipl
Copy link
Contributor Author

@asasfu thanks for comments, updated

@jonnytdevops
Copy link
Contributor

@stepanstipl It would be great if you could fix the test failures before this can be merged.

Thanks!

type file

show_diff => true (default) prints diff to logs
show_diff => md5 prints only md5 hashes instead of actual values
show_diff => false redacts any information about values

Global show_diff config takes priotiry over this one.
@stepanstipl
Copy link
Contributor Author

@jonnytpuppet I have rebased it onto the current master, the tests seems to be passing now

@tphoney
Copy link
Contributor

tphoney commented Feb 2, 2016

Thanks for the work @stepanstipl :)

tphoney added a commit that referenced this pull request Feb 2, 2016
@tphoney tphoney merged commit 1635b84 into puppetlabs:master Feb 2, 2016
rdo-ci-service added a commit to rdo-puppet-modules/puppetlabs-inifile that referenced this pull request Feb 3, 2016
…9cf8a0a749df8639c183661100c4247e5f31' into target-original-master-1635b843779f68dd63b59ce8457621628776c9bc

* commit '1635b843779f68dd63b59ce8457621628776c9bc':
  Convert boolean to :true & :false PR puppetlabs#152
  Acceptance tests for show_diff
  Implemented show_diff parameter, with similar behaviour as show_diff on type file

* commit 'e50e9cf8a0a749df8639c183661100c4247e5f31':
  dummy change
cegeka-jenkins pushed a commit to cegeka/puppet-inifile that referenced this pull request Oct 23, 2017
cegeka-jenkins pushed a commit to cegeka/puppet-inifile that referenced this pull request Oct 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants