-
Notifications
You must be signed in to change notification settings - Fork 130
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
Fixes #24012 - Add PuppetCA providers settings #433
Conversation
manifests/init.pp
Outdated
@@ -328,7 +334,10 @@ | |||
Stdlib::Absolutepath $puppetdir = $::foreman_proxy::params::puppetdir, | |||
String $puppetca_cmd = $::foreman_proxy::params::puppetca_cmd, | |||
String $puppet_group = $::foreman_proxy::params::puppet_group, | |||
Enum['puppetca_hostname_whitelisting', 'puppetca_token_whitelisting'] $puppetca_provider = $::foreman_proxy::params::puppetca_provider, |
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.
For other providers we've decided against an enum because we never know which plugins could be added. While unlikely, I would like users to be able to add their own providers. It would also be consistent with other $*_provider
parameters.
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 will also need a switch to stay compatible with current releases.
5688dc5
to
270fb4f
Compare
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.
Code looks good. This depends on theforeman/smart-proxy#592 and we also need a changing in packaging to handle the /var/lib/foreman-proxy/tokens.yaml
.
270fb4f
to
3149d56
Compare
Updated this to contain the new settings we added in theforeman/smart-proxy#592 |
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.
Looks good to me.
@ekohl: I think this is ready to merge? Anything to add? Then we can merge the smart-proxy changes. |
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.
Pending other merges
The smart-proxy PR has just been merged. |
merged, danke @juliantodt! |
As we are adding new providers to the PuppetCa SmartProxy module, we need to add its settings-file to the puppet module and add the settings as params to have them configurable.
See SmartProxy-Refactor and new TokenWhitelistingProvider (to be pull-requested after refactor gets merged).
Waiting for both of the SmartProxy-PRs to get merged before removing WIP-status.