Add exec_restorecon to hiera calls

[FStelzer]

Also add type definitions for hiera values and replace hiera_hash with the new lookup variant (fixing #238)

[vinzent]

Hi Fabian

Thanks for your contribution. If you move the lookup_options to hiera data itself this can be merged IMHO:

[vinzent]

this does not work IMHO. lookup() will never be used because automatic parameter lookup will see selinux::boolean defined and then the default is not applicable. The defaults for the lookup need to be defined in hiera data itself. see https://puppet.com/docs/puppet/5.0/hiera_merging.html#configuring-merge-behavior-in-hiera-data

[FStelzer]

Hmm. i think you're right.
But i'm not sure what a good solution would be.

Would lookup options defined in a module hiera be actually used if hiera finds the key in global/environment hiera? I would be surprised if it did.

Normally i would simply remove the merge options and leave that up to the user. But that might break backwards compatibility in this case.

[FStelzer]

ok, forget what i wrote ;)
i've looked up the documentation and lookup_options is a special case for which puppet actually does a merge to cover this use-case we have here explicitly.

i'll add it to hiera

[ekohl]

Would it make sense (and work) to just make this (and the other ones below it) Hash $boolean = {} and still use hiera?

[FStelzer]

Shouldn't be a problem and i prefer to have simple datatypes too. But it would break for users explicitly passing "undef" to the class (to override hiera values from higher up) which would no longer be valid (no idea if anyone actually does this).

[ekohl]

That's a valid point

[vinzent]

does passing undef work to not trigger automatic parameter lookup? I think there was a discussion about that some time ago.

But I prefer undef over an empty hash as it shows that nothing needs to be passed and Optional describes this.

[vinzent]

i've run the acceptance tests . they don't use hiera data but at least it doesn't break passing things by param. 👍

[FStelzer]

yeah, the hiera data stuff is always hard to test properly... (especially if it involves merge strategies)
i'll try to do a more complex test with hiera later today and let you know

[FStelzer]

ok. so hiera also works as expected. tested the hash merge as well

If i'd like to change the default values of the exec_restorecon define how would i implement that?
For example in our case i wanna always have refreshonly => false and have an onlyif with a restorecon -n to basically make sure the contexts are correct on every puppet run for certain directories.

As exec_restorecon is a define and the params get hash merged i can't set defaults via hiera. And the onlyif default is even trickier as it would include the the path as well...
Would you accept a pull request doing something like the following:

• Add explicit hiera lookups to the define on sth like 'selinux::exec_restorecon::defaults' to change things like the refreshonly value.
• Add a new parameter like 'Boolean $onlyif_contexts_change = lookup("selinux::exec_restorecon::defaults::onlyif_contexts_change")' that would automatically create an onlyif condition using restorecon -n

But i'm open for other ideas.

[vinzent]

ok. so hiera also works as expected. tested the hash merge as well

If i'd like to change the default values of the exec_restorecon define
how would i implement that?

You would define a resource default:
https://puppet.com/docs/puppet/5.3/lang_defaults.html

Selinux::Exec_restorecon {
refreshonly => false
}

somewhere on top or node scope. ("$env/manifest/site.pp")

[FStelzer]

Ah right. Didn't think about that. Thanks!

Still the onlyif_contexts_change or 'exec_on_changes' would be a nice addition. Otherwise you'll have a puppet exec show up in the logs on every run no matter if it had to change contexts or not.