Accept Sensitive for Secrets

[cocker-cc]

No description provided.

[cocker-cc]

Related: theforeman/puppet-candlepin#210

[ekohl]

I wonder if this would need to require a sufficiently recent version of the candlepin module in metadata.json. It certainly depends on it so I won't merge this until the other PR has been.

[ekohl]

Could you also add a unit test to ensure it works end to end?

I'm currently looking into a failure to parse Variant[ which I don't understand (https://community.theforeman.org/t/foreman-installer-develop-package-release-1015-failed/26627) so I'm first going to investigate that. It could be unrelated but then you at least know where I'm at.

[ekohl]

The good news is that that failure is unrelated but the bad news is that it looks like something else broke our CI.

[cocker-cc]

The good news is that that failure is unrelated but the bad news is that it looks like something else broke our CI.

     #   wrong number of arguments (given 5, expected 1)
     #   ./vendor/bundle/ruby/2.7.0/gems/psych-4.0.3/lib/psych.rb:323:in `safe_load'

Try pinning psych to Version 3.

[ekohl]

I'd like to see where puppetlabs/puppet#8849 goes first.

[ekohl]

I found with puppet-foreman testing that this wouldn't work well with our installer. It writes out all the answers so you end up with Sensitive[Undef], which isn't accepted here.

So what you end with in the installer answers file is:

katello:
  candlepin_db_password:

I think this translates to Hiera as:

katello::candlepin_db_password:

So that ends up sending undef via Hiera. If you add a conversion to sensitive in Hiera using lookup_options, this ends up as Sensitive[Undef].

Suggested change

	Optional[Variant[Sensitive[String], String]] $candlepin_db_password = undef,
	Variant[Sensitive[Optional[String]], Optional[String]] $db_password = undef,

This would then also need to be reflected in all other types.

I merged theforeman/kafo#345, will release it shortly. I'm also considering a solution where we add the lookup option and use this option:

Suggested change

	Optional[Variant[Sensitive[String], String]] $candlepin_db_password = undef,
	Sensitive[Optional[String]] $candlepin_db_password = sensitive(undef),

That would be a lot more straight forward. It may be breaking to users who pass in arguments directly from Puppet, but we can look at that.

[cocker-cc]

I have no Knowledge, how the Installer-Answer-File works, but Hiera does not translate

katello:
  candlepin_db_password:

to katello::candlepin_db_password. Instead what Hiera does here, is: Lookup a Hash named katello, find the Hash named katello, which turns out to have a Key named candlepin_db_password.

Isn't it possible to state an empty String "" instead of "nothing"

katello:
  candlepin_db_password: ""

in the Installer-Answer-File?

[ekohl]

Not really. What the installer does is that it extracts the default value from Puppet and stores that. Thinking more about it, other code would probably also fail, like if $candlepin_db_password {} is "false" with undef but perhaps Sensitive[Undef] may not.

I wonder if this is a more fundamental problem in Puppet and the lookup option may provide to only convert it if it isn't undef. Perhaps we need a conversion function that essentially is to_optional_sensitive(Optional[Any]) -> Optional[Sensitive[Any]]) (compared to sensitive(Any) -> Sensitive[Any]). Does that make sense?

[cocker-cc]

Yes, this totally makes Sense.
I created an Issue in the Hiera-Board: https://tickets.puppetlabs.com/browse/HI-628

[ekohl]

Thanks! In theforeman/puppet-foreman#1018 we had a less structured discussion but I think I now understand the problem properly and I'm an issue there is helpful moving it forward. Thanks for distilling it down to the minimal problem statement.