Skip to content

Conversation

@ehelms
Copy link
Member

@ehelms ehelms commented Mar 5, 2025

The removal of parameters in puppet-certs that were not used internally were used externally -- this updates to use current canonical parameters.

ssl => true,
ssl_proxyengine => true,
ssl_proxy_ca_cert => $certs::ca_cert,
ssl_proxy_ca_cert => $certs::katello_default_ca_cert,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this the certificate we expect on the remote server (so Foreman)? https://httpd.apache.org/docs/current/mod/mod_ssl.html#sslproxycacertificatefile states:

This directive sets the all-in-one file where you can assemble the Certificates of Certification Authorities (CA) whose remote servers you deal with. These are used for Remote Server Authentication. Such a file is simply the concatenation of the various PEM-encoded Certificate files, in order of preference. This can be used alternatively and/or additionally to SSLProxyCACertificatePath.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at theforeman/puppet-certs@9585baa:

$ca_cert = "${pki_dir}/certs/${default_ca_name}.crt"
$katello_default_ca_cert = "${pki_dir}/certs/${default_ca_name}.crt"

so this is an equivalent change

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this change,to fix tests I think we keep it equivalent. As for your point, this change dates back a long, long time to #54.

The configuration is not setting SSLProxyVerify explicitly, where as we do set SSLClientVerify (granted to optional) so I am guessing this setting doesn't actually matter or do anything at the moment.

@ehelms ehelms force-pushed the use-correct-apache-certs branch 2 times, most recently from 0e585ab to 6ab0dd4 Compare March 6, 2025 13:13
@ehelms
Copy link
Member Author

ehelms commented Mar 6, 2025

Updated.

@ekohl ekohl merged commit 13b8c13 into theforeman:master Mar 6, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants