Rename selinux parameter

[alexjfisher]

selinux is a core fact that returns a boolean.
https://docs.puppet.com/facter/latest/core_facts.html#selinux

If also set as a parameter in Foreman, it becomes the output of the ENC
and overrides the true/false returned by facter breaking puppet modules
including theforeman/foreman that uses a str2bool($::selinux)
expression.

https://github.com/theforeman/puppet-foreman/blob/5.2.0/manifests/install.pp#L23

selinux-mode is hopefully a good choice for the rename. It doesn't
conflict with any other core fact such as
https://docs.puppet.com/facter/latest/core_facts.html#selinuxconfigmode
or
https://docs.puppet.com/facter/latest/core_facts.html#selinuxcurrentmode

[domcleal]

I think we should probably try to keep some compatibility with the old parameter name, but I don't know if there's a good (i.e. visible) way to deprecate the old name for users. It might just be worth doing:

@host.params['selinux-mode'] || @host.params['selinux'] || 'enforcing'

for now. Then perhaps only document selinux-mode?

[ekohl]

It's rather annoying we can't use the facts hash yet. https://docs.puppet.com/puppet/latest/reference/lang_facts_and_builtin_vars.html#the-factsfactname-hash states that prior to 4.0 it wasn't available by default. I also don't know if the ENC parameters are actually facts or just global variables that overwrite facts.

[alexjfisher]

@domcleal I might actually go the other way. Have the template resolution fail dramatically if selinux is set. There are modules other than theforeman/foreman that (quite reasonably) assume $::selinux will be a boolean... Overriding facts from foreman may be a valid use case sometimes, but overriding a boolean with a string, is probably never going to be.

Are there any other options worth considering?

• Filter the selinux parameter out of the ENC output?
• Display a big warning on the main dashboard if it's set as a global parameter?

[domcleal]

Failing with an explanatory message if selinux is set is probably OK too.

I think it's fine if you'd like to file a feature request against Foreman for some better handling of clashes (I'd tend towards a warning), but we will still need to merge an update to the template.

[alexjfisher]

What's the correct way to fail?
If I call fail('selinux parameter has been renamed to selinux-mode') I get

here was an error rendering the Kickstart default template: Safemode doesn't allow to access 'fail' on #<Safemode::ScopeObject>

[domcleal]

I don't know if that's possible with safemode, I can't think of another way that's permitted, sorry! It'd need a change to Foreman or safemode to support it.

[alexjfisher]

Ok. I've gone with the backwards compatible option instead. If a change to Foreman would be needed to fail in a non-hacky way [1], then it's probably better to just to open the clash handling feature ticket instead.

[1] - This fails dramatically with an almost useful error message >:)

if @host.params.has_key?('selinux')
  selinux_has_been_renamed_to_selinux_mode
end

[domcleal]

Thanks @alexjfisher!

I also don't know if the ENC parameters are actually facts or just global variables that overwrite facts.

They're variables that overwrite facts, IIRC.

[alexjfisher]

Great. I've opened http://projects.theforeman.org/issues/15505 to track any enhancement that could be made to Foreman.