-
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
Capsule related certs settings #8
Conversation
# | ||
# $certs_tar:: path to tar file with certs to generate | ||
# | ||
# $katello_user:: Katello username used for creating repo with certs. |
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.
Just to clarify my understanding, this is a username coming from the application itself?
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.
Yes, this are the credentials needed to creating the certificates repositories in Katello and uploading the cert packages
Generate certs and configure certificates for capsule related stuff (smart-proxy, pulp node etc).
unless => "${foreman_config_cmd} --dry-run", | ||
user => $::foreman::user, | ||
require => Class['foreman::service'] | ||
} |
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 looks like we should add an explicit include ::foreman
to ensure that the Foreman class is loaded by the time it reaches this manifest.
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.
include ::foreman
will not help you here, as it needs to be not just included, but also initialized: the include
doesn't make $::foreman::user
to be set. Actually, it would cause more troubles, as it would use the defaults, instead the real params for foreman
module, that would work for base cases, but start failing if you actually would change same foreman params. That's the reason why there is order
setting for kafo, to define the right order of initializing of the classes
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.
Fair enough. My only worry with this being required by one class but ordering ensured by kafo is that it will be easy but not obvious that there is a required ordering. And further, ensuring that that isn't broken.
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.
The only other solution I can think of is having one entry-point class, that would make sure the ordering is right (something the capsule module does), but it has downsides of having to maintain the list of parameters twice: one in the entry class and one in the sub-classes. Therefore, letting kafo to deal with that is better from maintainability POV (not saying it's ideal)
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.
Sounds reasonable given the alternative - ACK
Generate certs and configure certificates for capsule related stuff
(smart-proxy, pulp node etc).