-
Notifications
You must be signed in to change notification settings - Fork 39
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
Use data in modules to determine values #424
base: master
Are you sure you want to change the base?
Conversation
@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. |
String $org, | ||
String $org_unit, | ||
String $expiration, | ||
Stdlib::Absolutepath $ca_key_password_file, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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 |
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. |
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 |
Testing this out with an installer PR. Not ready at all.