STOR-2954: feat: have CVO inject the centralized TLS configuration into the operator's config#703
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@ingvagabund: This pull request references STOR-2954 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 epic to target the "5.0.0" version, but no target version was set. 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: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughThis PR adds a ConfigMap containing a GenericOperatorConfig and updates standard, HyperShift, and IBM Cloud managed cluster-storage-operator deployments to mount that ConfigMap and pass its config file path via --config and --terminate-on-files. A HyperShift profile patch appends guest-kubeconfig entries and adds image environment variables. ChangesOperator Configuration Introduction
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Actionable comments posted: 0 |
|
/retest-required |
ingvagabund
force-pushed
the
cvo-injected-tls-configuration
branch
from
0883e0b to
be400cd
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
profile-patches/hypershift/10_deployment.yaml-patch (1)
37-75: ⚡ Quick winPrefer appending env vars with
/-instead of fixed indices in JSON PatchIn
profile-patches/hypershift/10_deployment.yaml-patchthe env vars are added at/spec/template/spec/containers/0/env/33…/env/40. JSON Patchaddwith/array/<index>inserts at a fixed position and shifts the rest, so upstream changes to the env array ordering/length can make this patch fragile or mis-target. Switching these ops to/env/-makes them append safely (and matches the existing/volumeMounts/-style in this patch).♻️ Suggested patch shape
- path: /spec/template/spec/containers/0/env/33 + path: /spec/template/spec/containers/0/env/- ... - path: /spec/template/spec/containers/0/env/34 + path: /spec/template/spec/containers/0/env/- ... - path: /spec/template/spec/containers/0/env/35 + path: /spec/template/spec/containers/0/env/- ... - path: /spec/template/spec/containers/0/env/36 + path: /spec/template/spec/containers/0/env/- ... - path: /spec/template/spec/containers/0/env/37 + path: /spec/template/spec/containers/0/env/- ... - path: /spec/template/spec/containers/0/env/38 + path: /spec/template/spec/containers/0/env/- ... - path: /spec/template/spec/containers/0/env/39 + path: /spec/template/spec/containers/0/env/- ... - path: /spec/template/spec/containers/0/env/40 + path: /spec/template/spec/containers/0/env/-🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@profile-patches/hypershift/10_deployment.yaml-patch` around lines 37 - 75, The JSON Patch uses fixed env indices (/spec/template/spec/containers/0/env/33 … /env/40) which makes the patch brittle; update each add operation to append instead by changing the path to /spec/template/spec/containers/0/env/- for all ENV entries (HYPERSHIFT_IMAGE, AWS_EBS_DRIVER_CONTROL_PLANE_IMAGE, AZURE_DISK_DRIVER_CONTROL_PLANE_IMAGE, LIVENESS_PROBE_CONTROL_PLANE_IMAGE, AZURE_FILE_DRIVER_CONTROL_PLANE_IMAGE, OPENSTACK_CINDER_DRIVER_CONTROL_PLANE_IMAGE, MANILA_DRIVER_CONTROL_PLANE_IMAGE, KUBE_RBAC_PROXY_CONTROL_PLANE_IMAGE) so they are appended safely (matching the existing /volumeMounts/- style).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@profile-patches/hypershift/10_deployment.yaml-patch`:
- Around line 37-75: The JSON Patch uses fixed env indices
(/spec/template/spec/containers/0/env/33 … /env/40) which makes the patch
brittle; update each add operation to append instead by changing the path to
/spec/template/spec/containers/0/env/- for all ENV entries (HYPERSHIFT_IMAGE,
AWS_EBS_DRIVER_CONTROL_PLANE_IMAGE, AZURE_DISK_DRIVER_CONTROL_PLANE_IMAGE,
LIVENESS_PROBE_CONTROL_PLANE_IMAGE, AZURE_FILE_DRIVER_CONTROL_PLANE_IMAGE,
OPENSTACK_CINDER_DRIVER_CONTROL_PLANE_IMAGE, MANILA_DRIVER_CONTROL_PLANE_IMAGE,
KUBE_RBAC_PROXY_CONTROL_PLANE_IMAGE) so they are appended safely (matching the
existing /volumeMounts/- style).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7adc3ab9-2d1d-4cb8-9a9a-fb9077369df7
📒 Files selected for processing (5)
manifests/04_configmap.yamlmanifests/10_deployment-hypershift.yamlmanifests/10_deployment-ibm-cloud-managed.yamlmanifests/10_deployment.yamlprofile-patches/hypershift/10_deployment.yaml-patch
🚧 Files skipped from review as they are similar to previous changes (4)
- manifests/10_deployment.yaml
- manifests/10_deployment-ibm-cloud-managed.yaml
- manifests/04_configmap.yaml
- manifests/10_deployment-hypershift.yaml
{
"apiVersion": "v1",
"data": {
"config.yaml": "apiVersion: operator.openshift.io/v1alpha1\nkind: GenericOperatorConfig\nservingInfo:\n cipherSuites:\n - TLS_AES_128_GCM_SHA256\n - TLS_AES_256_GCM_SHA384\n - TLS_CHACHA20_POLY1305_SHA256\n - TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256\n - TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384\n - TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256\n - TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256\n - TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384\n - TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256\n minTLSVersion: VersionTLS12\n"
},
"kind": "ConfigMap",
"metadata": {
"annotations": {
"capability.openshift.io/name": "Storage",
"config.openshift.io/inject-tls": "true",
"include.release.openshift.io/hypershift": "true",
"include.release.openshift.io/ibm-cloud-managed": "true",
"include.release.openshift.io/self-managed-high-availability": "true",
"include.release.openshift.io/single-node-developer": "true"
},
"creationTimestamp": "2026-05-29T11:54:09Z",
"name": "cluster-storage-operator-config",
"namespace": "openshift-cluster-storage-operator",
"ownerReferences": [
{
"apiVersion": "config.openshift.io/v1",
"controller": true,
"kind": "ClusterVersion",
"name": "version",
"uid": "6079762a-b2b2-4d0b-a321-c9a558950840"
}
],
"resourceVersion": "2022",
"uid": "4b10a857-116e-491d-986a-4835e6857dcf"
}
},TLS injected |
|
/retest |
|
/lgtm |
| namespace: openshift-cluster-storage-operator | ||
| name: cluster-storage-operator-config | ||
| annotations: | ||
| include.release.openshift.io/hypershift: "true" |
There was a problem hiding this comment.
This will cause CVO creating the config map in guest cluster, where nobody reads it. I'd remove this annotation.
| - --config=/var/run/configmaps/config/config.yaml | ||
| - --terminate-on-files=/var/run/configmaps/config/config.yaml |
There was a problem hiding this comment.
Who creates the ConfigMap in a hosted control plane? I think it will need something like openshift/hypershift#8011 to create it there.
We can merge this PR + then someone must copy 10_deployment-hypershift.yaml to hypershift here and add ConfigMap creation in the same hypershift PR to make sure nothing breaks.
…ator's config Also, have the operator restart whenever the config changes.
ingvagabund
force-pushed
the
cvo-injected-tls-configuration
branch
from
be400cd to
58bf9ab
Compare
|
/test e2e-gcp-csi |
|
@ingvagabund: all tests passed! 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dfajmon, ingvagabund 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
5 participants
Also, have the operator restart whenever the config changes.
wip-docs: openshift/enhancements#2020
Summary by CodeRabbit