Use data in modules to determine values

[ekohl]

Testing this out with an installer PR. Not ready at all.

[ekohl]

@ehelms together with this I get:

$ bundle exec ./bin/foreman-certs --help 
Usage:
    foreman-certs [OPTIONS]

Options:

= Generic:
    -i, --interactive                  Run in interactive mode
    -n, --noop                         Run puppet in noop mode? (default: false)
    -s, --skip-checks-i-know-better    Skip all system checks (default: false)
    -v, --[no-]verbose                 Display log on STDOUT instead of progressbar (default: false)
    -l, --verbose-log-level LEVEL      Log level for log based terminal output.
                                       The available levels are ERROR, WARN, NOTICE, INFO, DEBUG. See --full-help for definitions. (default: "notice")
    -S, --scenario SCENARIO            Use installation scenario
    --list-scenarios                   List available installation scenarios
    -h, --help                         print help
    --full-help                        print complete help
    --[no-]enable-certs                Enable 'certs' puppet module (default: true)
    --[no-]enable-certs-apache         Enable 'certs_apache' puppet module (default: true)


= Module certs:
    --certs-cname                      The alternative names of the host the generated certificates
                                       should be for (current: [])
    --certs-node-fqdn                  The fqdn of the host the generated certificates
                                       should be for (current: "guus.kohlvanwijngaarden.nl")
    --certs-server-ca-cert             Path to the CA that issued the ssl certificates for https
                                       if not specified, the default CA will be used (current: UNDEF)
    --certs-server-cert                Path to the ssl certificate for https
                                       if not specified, the default CA will generate one (current: UNDEF)
    --certs-server-cert-req            Path to the ssl certificate request for https
                                       if not specified, the default CA will generate one (current: UNDEF)
    --certs-server-key                 Path to the ssl key for https
                                       if not specified, the default CA will generate one (current: UNDEF)
    --certs-tar-file                   Use a tarball with certificates rather than generate
                                       new ones. This can be used on another node which is
                                       not the CA. (current: UNDEF)


= Module certs_apache:
    --certs-apache-cname               The alternative names of the host the generated certificates
                                       should be for (current: [])
    --certs-apache-hostname            The fqdn of the host the generated certificates
                                       should be for (current: "guus.kohlvanwijngaarden.nl")
    --certs-apache-server-cert         Path to the ssl certificate for https
                                       if not specified, the default CA will generate one (current: "")
    --certs-apache-server-cert-req     Path to the ssl certificate request for https
                                       if not specified, the default CA will generate one (current: "")
    --certs-apache-server-key          Path to the ssl key for https
                                       if not specified, the default CA will generate one (current: "")

Only commonly used options have been displayed.
Use --full-help to view the complete list.

As you can see, it converts aliases that are undef to empty strings. I recall this being a Hiera problem with alias. However, it does correctly determine values for the other items.

[ehelms]

This is defined inside certs/init.pp and I think should be dropped here? or do the same pick strategy down below. With that change your proposal here did allow me to run my command with success.

[ehelms]

Are there any reasons not to go this route with changing the module in this way? This is a bit different than our other modules, but does enable some more interesting abilities to use these classes.

[ekohl]

Are there any reasons not to go this route with changing the module in this way? This is a bit different than our other modules, but does enable some more interesting abilities to use these classes.

Way WAY back in the day that was the intention. You can see https://github.com/theforeman/puppet-tftp was converted by Dominic: theforeman/puppet-tftp@e2003d4

There are some downsides, like puppet-strings output being less useful since it doesn't show the default value. I aimed to mitigate that with puppetlabs/puppet-strings#273 but never found the time to wrap it up.

We can note that puppet-strings issues don't affect us since we already use rdoc due to parameter grouping. You can still argue that parameters directly in the file are better, but this is no worse than looking it up in params.pp. Except if there are multiple OS-specific overrides. Then you need multiple files open to track the data.

[ehelms]

How much of this is Puppet and how much of this is how Kafo is making assumptions and doing look ups? For example, would it be better to teach Kafo about classes as the base rather than modules? I'm not sure if that fixes things but if native Puppet works just fine, I'm curious what special things we do in Kafo are actually breaking this.

[ekohl]

Kafo needs to extract data to determine the values. If puppet-strings can determine the value, it will be faster because it can use a (JSON) cache. Otherwise it needs to evaluate the class in noop mode. That will increase the runtime of the whole installer. On the other hand, this isn't slower than the code we need now for $class::params::*. Now if everything is Hiera, we can perhaps use puppet hiera lookup (or some equivalent) to bypass all of that, but we don't have the code today.