OCPBUGS-115269: openstack-manila: Consume CA cert from CCO secret - #617
OCPBUGS-115269: openstack-manila: Consume CA cert from CCO secret#617stephenfin wants to merge 5 commits into
Conversation
Commit 6dd0da4 removed the last user. Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Since 4.19, Cloud Credential Operator (CCO) will provision the CA file as part of a cloud credential secret alongside the clouds.yaml, while Cluster Storage Operator (CSO) will mount this secret at '/etc/openstack/ca.crt' [1]. This is true for both standalone [2] and hypershift deployments [3]. Update the code to consume from the new location, allowing us to drop the CSO fallback. While here, we also replace use of the deprecated `ioutil.ReadFile` function in favour of its suggested replacement, `os.ReadFile` [2]. We also replace use of `os.IsNotExist` in favour of its suggested replacement, `errors.Is(err, fs.ErrNotExist)` [3]. [1] github.com/openshift/cluster-storage-operator/pull/557 [2] https://github.com/openshift/cluster-storage-operator/blob/25672c2a/assets/csidriveroperators/openstack-manila/standalone/generated/openshift-cluster-csi-drivers_apps_v1_deployment_manila-csi-driver-operator.yaml#L68-L74 [3] https://github.com/openshift/cluster-storage-operator/blob/25672c2a/assets/csidriveroperators/openstack-manila/hypershift/mgmt/generated/apps_v1_deployment_manila-csi-driver-operator.yaml#L106-L111 [2] https://pkg.go.dev/io/ioutil#ReadFile [3] https://pkg.go.dev/os#IsNotExist Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Make the purpose of this patch more obvious. We also fix some indentation. Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This is going to be superseded in a coming change. Rename it in preparation. Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
cloud-credential-operator and hypershift-operator now support deploying the CA cert to the credentials secrets they generate, which means we can start consuming them from there rather than from configuration. In this change, we modify the controller to start (optionally) consuming the CA cert from the secret. We leave a fallback in place since we still need to update cloud-credentials-operator to have clouds.yaml point to the new location. We can remove this in the future once the CCO change has merged. Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
jira/valid-reference
|
@stephenfin: This pull request references Jira Issue OCPBUGS-115269, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: stephenfin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@stephenfin: This pull request references Jira Issue OCPBUGS-115269, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe OpenStack Manila controller and DaemonSet now use projected Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change is merge-ready after normal checks and review; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (3 skipped: 3 unsupported.) Full details: Stable And Deterministic Test NamesExplanation PASS: The pull request changes 11 asset and production Go files, with no test files changed. The added-line diff contains no Ginkgo declarations such as It(), Describe(), Context(), or When(), and no changed file contains those test-title APIs. Therefore, the pull request introduces no unstable or overly-specific test name. Full details: Test Structure And QualityExplanation PASS: The pull request changes 11 asset and production Go files, with no Full details: Microshift Test CompatibilityExplanation PASS: The pull request adds or modifies no Ginkgo e2e tests. The full diff from origin/main...HEAD contains only YAML asset patches/generated manifests and Go implementation files. No changed file contains It(), Describe(), Context(), or When(), so the MicroShift test-compatibility check is not applicable. Full details: Single Node Openshift (Sno) Test CompatibilityExplanation PASS: The pull request adds no Ginkgo e2e tests. The complete diff from origin/main changes only OpenStack Manila asset YAML files and three Go implementation/configuration files. No changed path is a test file, and no added line contains Ginkgo declarations such as It(), Describe(), Context(), or When(). Therefore the SNO test-compatibility check is not applicable. Full details: Topology-Aware Scheduling CompatibilityExplanation PASS. The pull request changes credential and CA-certificate volumes, certificate handling, and the HyperShift config-map patch. It does not add or change scheduling constraints. The diff contains no changed affinity, topology-spread, replica, node-selector, node-affinity, toleration, taint, or PDB fields. Existing controller manifests retain preferred anti-affinity and Full details: Ote Binary Stdout ContractExplanation PASS: The pull request does not introduce stdout writes in process-level code. Its Go changes only update an asset patch reference, an error check, and constants. The diff adds no fmt.Print*, log/klog output, os.Stdout, Ginkgo setup, main(), init(), TestMain(), or suite setup code. The repository also contains no OTE or Ginkgo suite code. Existing klog calls are unchanged and are not causal to this pull request. Full details: Ipv6 And Disconnected Network Test CompatibilityExplanation PASS: The pull request adds or modifies only OpenStack Manila assets and Go implementation files. The diff adds no Ginkgo tests or e2e test files, and added-line searches found no IPv4 assumptions or external connectivity calls. The check is therefore not applicable. Full details: No-Weak-CryptoExplanation PASS: The PR diff introduces no MD5, SHA1, DES, 3DES, RC4, Blowfish, or ECB usage. It adds no custom cryptography or secret/token comparisons. The crypto-related Go change only replaces Full details: Container-PrivilegesExplanation PASS. The PR adds no Full details: No-Sensitive-Data-In-LogsExplanation PASS: The PR adds no logging or print statements. The Go changes only update certificate error matching, the certificate path, and the HyperShift asset patch. The YAML changes mount ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@stephenfin: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
2 participants
Update both the operator itself and the assets for the CSI driver to reflect changes made to cluster storage operator (CSO) in openshift/cluster-storage-operator#557 and cloud credential operator (CCO) in openshift/cloud-credential-operator#850.
This was originally targeted in #359 but #373 has broken hypershift support, so this is now targeted as a bug.
Depends on openshift/hypershift#9467