Skip to content
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

Fixes #38297 - Use client certificates for Candlepin events #11327

Merged
merged 1 commit into from
Mar 20, 2025

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Feb 28, 2025

What are the changes introduced in this pull request?

This relies upon first getting in theforeman/puppet-certs#490

The Candlepin events use the Foreman client certificates but the default CA since Candlepin runs using localhost certificates generated by the default CA. This means that it can't use the /etc/foreman/proxy_ca.pem certificate in it's current form as it represents the server CA. In the dependent PR, this would move to using a bundle CA combining the default and server CA into the single file allowing it to be used.

In production, the candlepin_events get configured in katello.yaml as:

  :candlepin_events:
    :ssl_cert_file: /etc/foreman/client_cert.pem
    :ssl_key_file: /etc/foreman/client_key.pem
    :ssl_ca_file: /etc/pki/katello/certs/katello-default-ca.crt

If this change goes forward, we would remove this section entirely, and rely upon Foreman core to handle configuration of certificates and reduce the configuration surface area of Katello. It would then follow that we can drop this parameter as well:

  :candlepin:
    :ca_cert_file: /etc/pki/katello/certs/katello-default-ca.crt

Which the code is already prepared to handle (https://github.com/Katello/katello/blob/master/app/services/cert/certs.rb#L31-L33).

What are the testing steps for this pull request?

  1. The Packit PR from this change needs to be installed.
  2. Install fresh or update install with custom certificates (this can be helpful).
  3. The installer PR is needed (Bundle default and server CA certificate and use for Foreman client CA theforeman/puppet-certs#490)

This can either be installed via Forklift, or wait for it to land in the installer before testing this.

@ehelms
Copy link
Member Author

ehelms commented Mar 6, 2025

Now that theforeman/puppet-certs#490 has been merged, the CA certificate will contain the default and server certificates. Candlepin communication can then use this instead of the CA certificate deployed for Apache's usage and rely on the Foreman settings.

@ehelms ehelms force-pushed the use-client-certs-candlepin-events branch 2 times, most recently from f48796f to 86424ec Compare March 7, 2025 17:33
@ehelms
Copy link
Member Author

ehelms commented Mar 7, 2025

As an additional note, this would reduce the needed configuration for Katello to:

:katello:
  :rest_client_timeout: <%= @rest_client_timeout %>

  :candlepin:
    :oauth_key: "<%= @candlepin_oauth_key %>"
    :oauth_secret: "<%= @candlepin_oauth_secret %>"

@ekohl Do you think we could re-use the Foreman oauth key and secret? Or should we keep that separate?

@parthaa Do you think rest_client_timeout could be moved to an application Setting rather than an installer SETTING?

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

@ekohl Do you think we could re-use the Foreman oauth key and secret? Or should we keep that separate?

I think we should keep that separate. Those are server side credentials and these are client side credentials.

@ehelms ehelms force-pushed the use-client-certs-candlepin-events branch from 86424ec to 1d5fc41 Compare March 9, 2025 13:52
@ehelms ehelms force-pushed the use-client-certs-candlepin-events branch from 1d5fc41 to f9bd175 Compare March 10, 2025 15:38
@ehelms ehelms force-pushed the use-client-certs-candlepin-events branch from f9bd175 to 867987b Compare March 17, 2025 20:29
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Generally 👍 on this, but I can't see why the tests fail. I have a feeling that it's more common in Katello that the tests just exit with rake aborted!. Do we not set up a reporter or something?

@ehelms
Copy link
Member Author

ehelms commented Mar 18, 2025

@jeremylenz Mind taking a look? Are the test failures expected?

@ekohl
Copy link
Member

ekohl commented Mar 18, 2025

@ehelms probably good to create a Redmine issue already

@sjha4
Copy link
Member

sjha4 commented Mar 18, 2025

Commit message is invalid but the other failures are unrelated. The React tests are failing on PF5 upgrades and the ruby failure is from a flaky test.
katello/test/actions/katello/content_view_version/republish_repositories_test.rb#L23 - Failure: test_0001_plans with default values**

@ehelms ehelms force-pushed the use-client-certs-candlepin-events branch from 867987b to a369d6f Compare March 18, 2025 11:05
@ehelms ehelms changed the title Use client certificates for Candlepin events Fixes #38297 - Use client certificates for Candlepin events Mar 18, 2025
@ehelms
Copy link
Member Author

ehelms commented Mar 18, 2025

Thanks @sjha4. Added a Redmine.

Copy link
Member

@sjha4 sjha4 left a comment

Choose a reason for hiding this comment

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

Ack..👍🏼
Unrelated React tests are going to stay broken but feel free to merge this.

@ianballou ianballou merged commit 1a5b7dc into Katello:master Mar 20, 2025
17 of 19 checks passed
@ianballou
Copy link
Member

Merged since the test failures are unrelated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants