Add client_authentication parameter to plugin::pulp #682
Conversation
ehelms
force-pushed
the
cert-common-name-auth
branch
from
6d94b39
to
e4f23a5
Compare
ehelms
changed the title
manifests/plugin/pulp.pp
Outdated
| class foreman_proxy::plugin::pulp ( | ||
| Foreman_proxy::ListenOn $listen_on = 'https', | ||
| Boolean $pulpcore_enabled = true, | ||
| Boolean $pulpcore_mirror = false, | ||
| Stdlib::HTTPUrl $pulpcore_api_url = $foreman_proxy::plugin::pulp::params::pulpcore_api_url, | ||
| Stdlib::HTTPUrl $pulpcore_content_url = $foreman_proxy::plugin::pulp::params::pulpcore_content_url, | ||
| Optional[String] $version = undef, | ||
| Boolean $client_authentication = ['client_certificate'], |
There was a problem hiding this comment.
| Boolean $client_authentication = ['client_certificate'], | |
| Array[Enum['password', 'client_certificate', 'client_certificate_admin_only']] $client_authentication = ['client_certificate'], |
There was a problem hiding this comment.
As a general question, do we want to be coupling in these situations? Whereby an update to smart_proxy_pulp adding some new type would then force an update to this code.
There was a problem hiding this comment.
I also wondered that. The benefit of having a type here is that if you make a typo, it's caught during catalog compilation rather than smart proxy startup. On the other hand, we're not that likely to have user input so I'd also be fine with Array[String[1], 1] (the last 1 ensures there's at least one value, which would also be a good addition with the enum).
The client_authentication parameter controls the available types of client authentication supported by the Pulp installation.
ehelms
force-pushed
the
cert-common-name-auth
branch
from
e4f23a5
to
11bb9e0
Compare
|
Additionally, need some thoughts on how this gets released. I have a major version bump planned to drop Puppet 5 that could include this (#683) and (#680) or this could be a minor version inclusion. I tend towards just including in the major given we are aiming to drop Puppet 5 across our modules for the coming Foreman release and that makes things easiest. |
|
I think that's ok. I'm always happy to keep the git log clean and bumping minor and only then bump major is just noise IMHO. I don't see changes that need to be released to a stable branch for now (perhaps smart_proxy_acd support). |
|
The Debian/Ubuntu tests failing are due to theforeman/smart_proxy_dynflow#86 so wont pass until that is released. However, for this change, given Pulp is only deployed on EL7 I dont think the failing tests apply so I am leaning to merge this and continue forward with these changes in the other modules. |
Depends on the outcome of theforeman/smart_proxy_pulp#28