Skip to content

Conversation

waipeng
Copy link
Contributor

@waipeng waipeng commented Jul 18, 2019

Overview

This pull request introduces hiding of Sensitive data output.

Puppet now has type Sensitive, which hides the content from logs, etc. Hide it similarly in output of octocatalog-diff, by using a trivial hash so that any diff will still be show.

See also: https://puppet.com/docs/puppet/5.3/lang_data_sensitive.html

Checklist

  • Make sure that all of the tests pass, and fix any that don't. Just run rake in your checkout directory, or review the CI job triggered whenever you push to a pull request.
  • Make sure that there is 100% test coverage by running rake coverage:spec or ignoring untestable sections of code with # :nocov comments. If you need help getting to 100% coverage please ask; however, don't just submit code with no tests.
  • If you have added a new command line option, we would greatly appreciate a corresponding integration test that exercises it from start to finish. This is optional but recommended.
  • If you have added any new gem dependencies, make sure those gems are licensed under the MIT or Apache 2.0 license. We cannot add any dependencies on gems licensed under GPL.
  • If you have added any new gem dependencies, make sure you've checked in a copy of the .gem file into the vendor/cache directory.

Puppet now has type Sensitive, which hides the content from logs, etc.
Hide it similarly in output of octocatalog-diff, by using a trivial hash
so that any diff will still be show.
@alexjfisher
Copy link
Contributor

This works well with Sensitive[String] parameters, but not so much with Sensitive[Hash]...

TypeError: no implicit conversion of Hash into String
  /home/fishera/control-repo/vendor/bundle/ruby/2.5.0/bundler/gems/octocatalog-diff-957a8175d326/lib/octocatalog-diff/catalog-diff/differ.rb:470:in `digest'

@ahayworth
Copy link
Contributor

@waipeng Thank you for this! We'll merge it into the 1.6.0 release. I did have to make a change on the release branch to get tests to pass - take a look here. I think that's a benign change, just a copying/pasting error most likely - but please do let me know if it seems wrong.

@waipeng
Copy link
Contributor Author

waipeng commented Oct 30, 2019

Oh yes I forgot to update the test after this change. Thanks for maintaining this software, it is very useful to us!

@ahayworth ahayworth merged commit e9d3744 into github:master Oct 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants