Fixes #34684 - install pulp-cli #252
Conversation
evgeni
force-pushed
the
pulp-cli
branch
9 times, most recently
from
d74f7a1
to
403950c
Compare
|
EL7 currently fails, as there is no |
| it { is_expected.to compile.with_all_deps } | ||
| it { is_expected.to contain_package('pulp-cli') } |
There was a problem hiding this comment.
I guess I should add some asserts on the config content too?
evgeni
force-pushed
the
pulp-cli
branch
8 times, most recently
from
7ceb72b
to
bb27c00
Compare
| # can be set to specific version number, 'latest', 'present' etc. | ||
| # | ||
| class pulpcore::cli ( | ||
| Optional[Stdlib::HTTPUrl] $pulpcore_url = undef, |
There was a problem hiding this comment.
should this default to https://${fqdn}?
There was a problem hiding this comment.
It's not exposed to users so I think this is fine.
There was a problem hiding this comment.
This looks good. I do wonder how we're going to present this to users. One thing to consider is to automatically inherit from Foreman. So one solution is how foreman::cli does it: https://github.com/theforeman/puppet-foreman/blob/9e483426c511e7a7a2648c2eaba18f50ac7df8f9/manifests/cli.pp#L39-L50=
That's probably the easiest if we take Kafo into consideration. It should be noted that this approach is compile order dependent (sadly it does exist in Puppet and here you can notice it). That is why the order is defined in the installer config (https://github.com/theforeman/foreman-installer/blob/8d7fb8ef47ffaf1023ffe8ade3db581ed5b1b655/config/foreman.yaml#L34-L37=).
Modern tooling prefers the subjectAltName extension and the CN on a certificate is considered deprecated. Some tools even complain about this. However, the -addext command is unavailable on EL7 because openssl is too old.
That is a solid question, cause we view this as a bit of advanced functionality when it's deployed in a Katello scenario. I, for example, would be hesistent to make it easy for a user to configure the dry-run (https://github.com/theforeman/puppet-pulpcore/pull/252/files#diff-751b60988fac3d7c7c119b3a70b6f70d6ef0c3facfaf6b9c0c0d44fc564cacb1R25) parameter as this can have severe implications to the system as a whole opened up. |
|
The Katello integration of Pulpcore happens via Now, the integration, I think, should be one of these modules including class { 'pulpcore::cli':
pulpcore_url => "https://${servername}",
dry_run => true,
cert => …,
key => …,
}Edit: so like this: theforeman/puppet-foreman_proxy_content#405 |
| # | ||
| # === Parameters: | ||
| # | ||
| # $pulpcore_url:: URL on which Pulpcore runs |
There was a problem hiding this comment.
in other places (foreman_proxy, foreman_proxy_content) we call this pulpcore_api_url.
pulp-cli itself calls it base_url.
🚲 shed?
There was a problem hiding this comment.
Given it's not exposed to users, I don't care that much. Let's start with this.
|
Okay, why does |
| ), | ||
| } | ||
|
|
||
| if $manage_root_config and (($username and $password) or ($cert and $key)) { |
There was a problem hiding this comment.
Should we have some validates above that throw a clean error if for either combo one is supplied and not the other?
There was a problem hiding this comment.
So in pulp-cli the validation is a bit more involved: https://github.com/pulp/pulp-cli/blob/aa50bc63e949e7f413bce6bc10d323d119e61b09/pulpcore/cli/common/openapi.py#L49-L62
I think for this module, it would be sufficient to check:
- (username OR password) AND (cert OR key) → error
- password AND !username → error
- key AND !cert → error
Everything else is imho valid (like setting username via config, but using CLI --password for the password, or providing a cert/key combo in the cert parameter).
|
@ehelms any further comments, or are you happy with it as it is too? |
No description provided.