-
Notifications
You must be signed in to change notification settings - Fork 295
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
pass our CA cert when configuring Pulp3 Client #11190
Conversation
This reminds me I started #8832 a long time ago and I should revisit that. |
bleh, this doesn't work as it should, whyyy |
because @evgeni restarted foreman after changing the code, but not the dynflow worker, and the code talking to Pulp obviously runs there… |
@@ -4,8 +4,12 @@ def self.ueber_cert(organization) | |||
organization.debug_cert | |||
end | |||
|
|||
def self.ca_cert_filename | |||
Setting[:ssl_ca_file] |
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.
but… is that the right file?
it's /etc/foreman/proxy_ca.pem
, which is the one we use to talk to proxies.
but here, were talking to Pulpcore via Apache, and Apache will be using the server-ca?
@ehelms halp
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.
I think so, but I still prefer #8832 which defaults to that file while still allowing to override it if needed. It also works it into the ping controller.
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.
it's
/etc/foreman/proxy_ca.pem
, which is the one we use to talk to proxies.
but here, were talking to Pulpcore via Apache, and Apache will be using the server-ca?
And that should be correct and fine: see https://github.com/theforeman/foreman-installer/pull/965/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R165
Fixed by #8832, thanks Ewoud! |
What are the changes introduced in this pull request?
Considerations taken when implementing this change?
What are the testing steps for this pull request?