Fixes #30368 - Make Candlepin CA file optional #8832
Conversation
|
Issues: #30368 |
|
Related, and based on some discussion with @jlsherrill , today Pulp communication relies on the the CA being trusted by the system (the whole use of trusted_ca module in the installer) and we should switch to doing this explicitly -- |
ekohl
force-pushed
the
candlepin-use-ca-file
branch
from
6d1bd33
to
8ef3711
Compare
|
I took a stab at this. In doing so, I found 2 settings that were removed but still had references to them. It then also unifies the certificate handling in the certs service. There are a few more changes, documented in the commit message. Please read it and let me hear your thoughts. |
| @@ -24,10 +24,14 @@ def self.ssl_client_key_filename | |||
| Setting['pulp_client_key'] | |||
| end | |||
|
|
|||
| def self.backend_ca_cert_file(backend) | |||
| SETTINGS.dig(:katello, backend, :ca_cert_file) || Setting[:ssl_ca_file] | |||
There was a problem hiding this comment.
I honestly cannot think of a scenario where we would have or want a different CA cert for Candlepin or Pulp given our architecture. @jlsherrill can you think of any reason to not just set the CA cert used to Setting[:ssl_ca_file] and reduce the potential configuration options?
There was a problem hiding this comment.
I didn't know how breaking we would be if we dropped it immediately and if that's acceptable.
There was a problem hiding this comment.
unless there is some quirk related with a 3rd party CA i can't think of this should be fine. I would imagine all sorts of things would not work properly if you started using different CAs for differnet subsystems.
There was a problem hiding this comment.
There is one case. Pulp 3 can use a different CA than the Smart Proxy. For example, one signed by a publicly trusted CA.
|
[test katello] |
ekohl
force-pushed
the
candlepin-use-ca-file
branch
from
8ef3711
to
85f4f40
Compare
|
One test failure seems relevant |
ekohl
force-pushed
the
candlepin-use-ca-file
branch
from
85f4f40
to
d483104
Compare
|
Rebased to resolve merge conflict and to get recent tests. I've also split of unrelated work to #9268 since it doesn't impact this PR. |
|
Haven't tested yet but the test failure definitely seems related |
|
Given stabilization is coming soon and my lack of time, I'm afraid we'll have to push this to the next Katello release. |
|
@ekohl is this PR still valid ? Rebase if so |
theforeman-bot
added
Waiting on contributor
and removed
Needs testing
Not yet reviewed
labels
|
@ekohl, this pull request is currently not mergeable. Please rebase against the master branch and push again. If you have a remote called 'upstream' that points to this repository, you can do this by running: This message was auto-generated by Foreman's prprocessor |
This change makes the Candlepin CA file optional by falling back to the Foreman CA file. The result is a reduced configuration in most deployments. For Pulp the certificate is now read in the same way as Candlepin. Previously it partly relied on the CA being in the global allowed CA. This may be an issue in some cases, but in the default deployment it isn't. Following the general SSL config makes the configuration more predictable for users. It can also be easier in a containerized setup or on a system where the admin is not allowed to modify the system CA certificates. The example config is now also consistent with reality. The verify_ssl option is dropped from the ping model. This isn't respected elsewhere and it's misleading to have a valid ping only to have it fail at runtime.
ekohl
force-pushed
the
candlepin-use-ca-file
branch
from
d483104
to
48289b6
Compare
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
I've tried to rebase it, but I'm a bit unclear about the current Pulp 2 vs Pulp 3 status. I still see traces of Runcible, but isn't that Pulp 2-only? I now think this unifies everything. It does mean the system store isn't really used anymore (unless you don't configure anything I think), but that's probably a good thing. Perhaps @ehelms and @evgeni could also think about what kind of behavior we would prefer to have from a platform point of view. |
This change makes the Candlepin CA file optional by falling back to the Foreman CA file. The result is a reduced configuration in most deployments.
For Pulp the certificate is now read in the same way as Candlepin. Previously it partly relied on the CA being in the global allowed CA. This may be an issue in some cases, but in the default deployment it isn't. Following the general SSL config makes the configuration more predictable for users. It can also be easier in a containerized setup or on a system where the admin is not allowed to modify the system CA certificates. The example config is now also consistent with reality.
The verify_ssl option is dropped from the ping model. This isn't respected elsewhere and it's misleading to have a valid ping only to have it fail at runtime.