-
Notifications
You must be signed in to change notification settings - Fork 270
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
Pass potentially Sensitive params as Sensitive #1018
base: master
Are you sure you want to change the base?
Conversation
Oh, this is an interesting one:
Because it converts it to |
After changing the data type this doesn't appear to work?
|
120efb3
to
447bf08
Compare
In 699f944 the parameters started to accept Sensitive but it didn't default to Sensitive. They also weren't converting data coming from Hiera. This adds a data-in-modules setup and sets lookup_options for those. This means Kafo (which heavily relies on Hiera) will pass sensitive values. The commit also missed settings.yml.erb and database.yml.erb which now also unwraps if needed. Ideally this would be converted to EPP instead but this fixes it in the short term. However, hammer_root.yml.epp somehow doesn't properly render the sensitive data. That's why it's unwrapper for now, just to make it work. A test case is added to prevent future regressions. It also changes the data type to accept Sensitive[Undef] which is needed if Hiera unconditionally converts the value to Sensitive. Fixes: 699f944
@cocker-cc this isn't pretty, but I couldn't get it to work otherwise. Please have a look. |
Hiera itself will
Hiera will not return This means IMHO: If the EPP receives |
That does not reflect my findings in our installer. That is because our installer copies all answers and persists them. This results in I also couldn't get https://puppet.com/docs/puppet/6/securing-sensitive-data.html#epp-templates to work. Using
|
My Thoughts on this – without having had a deep Insight into the Code: Here as an Example-Commit I see:
Furthermore:
Eventually
The other Variables are accordingly. |
For now I want to revert it: #1019. This week we're planning to do module releases for Foreman 3.2 and it needs to be stable. Reverting it now is the quickest path. It can be added in the release candidate phase. Once it's reverted, I'll update this PR to make a complete implementation. |
What about a function that converts |
In 699f944 the parameters started to accept Sensitive but it didn't default to Sensitive. They also weren't converting data coming from Hiera. This adds a data-in-modules setup and sets lookup_options for those. This means Kafo (which heavily relies on Hiera) will pass sensitive values.
Currently a draft so I can verify it end to end with the installer.