-
Notifications
You must be signed in to change notification settings - Fork 406
OSASINFRA-3732: openstack: Sync CA cert to new key #5702
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
Conversation
@stephenfin: This pull request references OSASINFRA-3732 which is a valid jira issue. In response to 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. |
Skipping CI for Draft Pull Request. |
6d17824
to
cc51ddd
Compare
// cluster-storage-operator now uses the certs from 'cacert', meaning | ||
// this is no longer necessary. It is only kept here temporarily to | ||
// ease upgrades. Remove in 4.20+ | ||
secret.Data[CABundleKey] = caCertData |
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.
Confirmed there are no references to this on a running HCP cluster:
❯ oc get -A pods -o yaml | grep --group-separator=$'\n---\n' -e '\bca-bundle.pem\b' -B 5 -A 2
defaultMode: 420
secretName: openstack-cinder-csi-driver-node-metrics-serving-cert
- configMap:
defaultMode: 420
items:
- key: ca-bundle.pem
path: ca-bundle.pem
name: cloud-conf
optional: true
---
secretName: serving-cert
- configMap:
defaultMode: 420
items:
- key: ca-bundle.crt
path: tls-ca-bundle.pem
name: trusted-ca
name: trusted-ca
---
name: service-ca
- configMap:
defaultMode: 420
items:
- key: ca-bundle.crt
path: tls-ca-bundle.pem
name: trusted-ca-bundle
name: trusted-ca-bundle
---
name: metrics-client-ca
- configMap:
defaultMode: 420
items:
- key: ca-bundle.crt
path: tls-ca-bundle.pem
name: alertmanager-trusted-ca-bundle
name: alertmanager-trusted-ca-bundle
---
name: prometheus-k8s-db
- configMap:
defaultMode: 420
items:
- key: ca-bundle.crt
path: tls-ca-bundle.pem
name: prometheus-trusted-ca-bundle
name: prometheus-trusted-ca-bundle
---
name: metrics-client-ca
- configMap:
defaultMode: 420
items:
- key: ca-bundle.crt
path: tls-ca-bundle.pem
name: telemeter-trusted-ca-bundle-56c9b9fa8d9gs
optional: true
// csi-operator) consume configuration from this secret: cinder sources it | ||
// from the config map, and manila does its own special thing. Remove in | ||
// 4.20+ | ||
secret.Data[CloudConfigKey] = []byte(config) |
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.
Ditto:
❯ oc get -A pods -o yaml | grep --group-separator=$'\n---\n' -e '\bcloud.conf\b' -B 5 -A 2
- --v=2
env:
- name: CSI_ENDPOINT
value: unix://csi/csi.sock
- name: CLOUD_CONFIG
value: /etc/kubernetes/config/cloud.conf
image: registry.build11.ci.openshift.org/ci-ln-6gzd92b/stable@sha256:9ec419f3de22d194cabf4f24f160c5f5e75485e8fc58dca4178869e357463629
imagePullPolicy: IfNotPresent
---
- configMap:
defaultMode: 420
items:
- key: ca-bundle.pem
path: ca-bundle.pem
name: cloud-conf
optional: true
name: cacert
- configMap:
defaultMode: 420
items:
- key: cloud.conf
path: cloud.conf
name: cloud-conf
name: config-cinderplugin
- name: secret-cinderplugin
cloud-credential-operator now supports syncing CA certs from the root credential secret to the generated credentials secrets. If necessary, CCO expects the CA cert to be provided in the `cacert` key and will place it in the same location in the generated secrets. Start doing the same in control-plane-operator, which allows us to significantly simplify the assets used in cluster-storage-operator and csi-operator. Note that we are intentionally *not* changing how CA certs are managed for cluster-cloud-controller-manager-operator. There's a good reason for this, and a note is left inline to that effect. Signed-off-by: Stephen Finucane <[email protected]>
We'll actually resolve this separately to avoid conflating things in one PR. We also fix a type and group two similar secrets that we are creating. Signed-off-by: Stephen Finucane <[email protected]>
cc51ddd
to
89f8be1
Compare
/retest |
/lgtm |
I'll re-add a /hold so we can wait for the CSI PRs to land: |
/hold |
/unhold We don't need the CSI changes to land to start testing this, as it's effectively a just no-op without them. In fact, it'd probably be better have the Hypershift change land first so that we can easily test the CSI changes. |
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: csrwng, stephenfin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/test Red Hat Konflux / hypershift-operator-main-on-pull-request |
@stephenfin: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In response to 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 kubernetes-sigs/prow repository. |
/ok-to-test |
/retest |
/override "Red Hat Konflux / hypershift-operator-main-enterprise-contract / hypershift-operator-main" |
@csrwng: Overrode contexts on behalf of csrwng: Red Hat Konflux / hypershift-operator-main-enterprise-contract / hypershift-operator-main In response to 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 kubernetes-sigs/prow repository. |
@stephenfin: all tests passed! 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. |
/override "Red Hat Konflux / hypershift-operator-main-enterprise-contract / pr group" |
/override "Red Hat Konflux / hypershift-operator-main-enterprise-contract / hypershift-operator-main" |
@csrwng: Overrode contexts on behalf of csrwng: Red Hat Konflux / hypershift-operator-main-enterprise-contract / pr group In response to 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 kubernetes-sigs/prow repository. |
@csrwng: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. In response to 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 kubernetes-sigs/prow repository. |
What this PR does / why we need it:
cloud-credential-operator now supports syncing CA certs from the root credential secret to the generated credentials secrets. If necessary, CCO expects the CA cert to be provided in the
cacert
key and will place it in the same location in the generated secrets. Start doing the same in control-plane-operator, which allows us to significantly simplify the assets used in cluster-storage-operator and csi-operator.Note that we are intentionally not changing how CA certs are managed for cluster-cloud-controller-manager-operator. There's a good reason for this, and a note is left inline to that effect.
/hold
Which issue(s) this PR fixes (optional, use
fixes #<issue_number>(, fixes #<issue_number>, ...)
format, where issue_number might be a GitHub issue, or a Jira story:Per $subject.
Checklist