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

OSASINFRA-3657: Add support for storing OpenStack CA bundles #780

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stephenfin
Copy link

If a CA bundle is required to talk to your OpenStack then obviously all services that talk to the cloud need to have both credentials and said bundle. Currently, these users can get their credentials via cloud credential operator, but they need to source their CA bundle from elsewhere (typically by extracting it from the cloud controller manager's configuration). This makes configuration of services more complicated than necessary.

Begin the resolution of the issue by allowing users (i.e. the Installer) to store the CA bundle in their root secret and dole this out to anyone who asks for it via a CredentialsRequest. Follow-up changes will be needed in places like the Installer and csi-operator to start setting/consuming this.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Nov 8, 2024

@stephenfin: This pull request references OSASINFRA-3657 which is a valid jira issue.

In response to this:

If a CA bundle is required to talk to your OpenStack then obviously all services that talk to the cloud need to have both credentials and said bundle. Currently, these users can get their credentials via cloud credential operator, but they need to source their CA bundle from elsewhere (typically by extracting it from the cloud controller manager's configuration). This makes configuration of services more complicated than necessary.

Begin the resolution of the issue by allowing users (i.e. the Installer) to store the CA bundle in their root secret and dole this out to anyone who asks for it via a CredentialsRequest. Follow-up changes will be needed in places like the Installer and csi-operator to start setting/consuming this.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 8, 2024
@openshift-ci openshift-ci bot requested a review from EmilienM November 8, 2024 18:09
@stephenfin
Copy link
Author

/cc @mandre

@openshift-ci openshift-ci bot requested review from mandre, 2uasimojo and dlom November 8, 2024 18:09
Copy link

codecov bot commented Nov 8, 2024

Codecov Report

Attention: Patch coverage is 46.66667% with 8 lines in your changes missing coverage. Please review.

Project coverage is 46.99%. Comparing base (db5f253) to head (d4d1748).

Files with missing lines Patch % Lines
pkg/openstack/actuator.go 0.00% 5 Missing ⚠️
pkg/openstack/utils.go 62.50% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #780      +/-   ##
==========================================
- Coverage   47.01%   46.99%   -0.03%     
==========================================
  Files          97       97              
  Lines       11874    11877       +3     
==========================================
- Hits         5583     5582       -1     
- Misses       5676     5679       +3     
- Partials      615      616       +1     
Files with missing lines Coverage Δ
...g/operator/secretannotator/openstack/reconciler.go 55.11% <100.00%> (ø)
pkg/openstack/utils.go 53.84% <62.50%> (-12.83%) ⬇️
pkg/openstack/actuator.go 0.00% <0.00%> (ø)

stephenfin added a commit to shiftstack/installer that referenced this pull request Nov 11, 2024
If a CA bundle is required to talk to your OpenStack then obviously all
services that talk to the cloud need to have both credentials and said
bundle. Currently, these users can get their credentials via cloud
credential operator, but they need to source their CA bundle from
elsewhere (typically by extracting it from the cloud controller
manager's configuration). This makes configuration of services more
complicated than necessary.

Continue the resolution of the issue by storing the CA bundle, if any,
in the root secret on OpenStack. When coupled with the changes
introduced in openshift/cloud-credential-operator#780 [1], this allows
us to dole out the bundle to anyone who asks for it via a
'CredentialsRequest'.

[1] openshift/cloud-credential-operator#780

Signed-off-by: Stephen Finucane <[email protected]>
@stephenfin
Copy link
Author

/hold

Will wait for 4.19 for this.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 18, 2024
If a CA cert is required to talk to your OpenStack then obviously all
services that talk to the cloud need to have both credentials and said
cert. Currently, these users can get their credentials via cloud
credential operator, but they need to source their CA cert from
elsewhere (typically by extracting it from the cloud controller
manager's configuration). This makes configuration of services more
complicated than necessary.

Begin the resolution of the issue by allowing users (i.e. the Installer)
to store the CA cert in their root secret and dole this out to anyone
who asks for it via a CredentialsRequest. Follow-up changes will be
needed in places like the Installer and csi-operator to start
setting/consuming this.

Note that the term 'cacert' is important, since this is the terminology
used for clouds.yaml files and is what various OpenStack-based tooling
like cluster-api-provider-openstack (CAPO) or
openstack-resource-controller (ORC) use. Using a different name would
necessitate a controller copying the generated secret and changing the
key name.

Signed-off-by: Stephen Finucane <[email protected]>
@EmilienM
Copy link
Member

/lgtm
thanks!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 14, 2025
Copy link
Contributor

openshift-ci bot commented Feb 14, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: EmilienM, stephenfin
Once this PR has been reviewed and has the lgtm label, please assign dlom for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

stephenfin added a commit to shiftstack/installer that referenced this pull request Feb 14, 2025
If a CA cert is required to talk to your OpenStack then obviously all
services that talk to the cloud need to have both credentials and said
cert. Currently, these users can get their credentials via cloud
credential operator, but they need to source their CA cert from
elsewhere (typically by extracting it from the cloud controller
manager's configuration). This makes configuration of services more
complicated than necessary.

Continue the resolution of the issue by storing the CA cert, if any,
in the root secret on OpenStack. When coupled with the changes
introduced in openshift/cloud-credential-operator#780 [1], this allows
us to dole out the cert to anyone who asks for it via a
'CredentialsRequest'.

[1] openshift/cloud-credential-operator#780

Signed-off-by: Stephen Finucane <[email protected]>
@stephenfin
Copy link
Author

/hold cancel

@stephenfin stephenfin closed this Feb 14, 2025
@stephenfin stephenfin reopened this Feb 14, 2025
@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 14, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Feb 14, 2025

@stephenfin: This pull request references OSASINFRA-3657 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "4.19." or "openshift-4.19.", but it targets "4.18.0" instead.

In response to this:

If a CA bundle is required to talk to your OpenStack then obviously all services that talk to the cloud need to have both credentials and said bundle. Currently, these users can get their credentials via cloud credential operator, but they need to source their CA bundle from elsewhere (typically by extracting it from the cloud controller manager's configuration). This makes configuration of services more complicated than necessary.

Begin the resolution of the issue by allowing users (i.e. the Installer) to store the CA bundle in their root secret and dole this out to anyone who asks for it via a CredentialsRequest. Follow-up changes will be needed in places like the Installer and csi-operator to start setting/consuming this.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

Copy link
Contributor

openshift-ci bot commented Feb 14, 2025

@stephenfin: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/okd-scos-e2e-aws-ovn d4d1748 link false /test okd-scos-e2e-aws-ovn
ci/prow/security d4d1748 link true /test security

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@EmilienM
Copy link
Member

/test e2e-hypershift

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants