From a96e57e0117074d111844d2cecdc86ac64b487c7 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 12 Mar 2025 12:48:15 +0000 Subject: [PATCH 1/9] openstack-cinder: Better name for cloud credentials This at least tells you what the secret is for. Signed-off-by: Stephen Finucane --- .../generated/hypershift/controller.yaml | 10 +++---- .../generated/hypershift/node.yaml | 24 +++++++-------- .../generated/standalone/controller.yaml | 10 +++---- .../generated/standalone/node.yaml | 24 +++++++-------- .../patches/controller_add_driver.yaml | 20 +++++-------- .../patches/node_add_driver.yaml | 29 +++++++++---------- 6 files changed, 55 insertions(+), 62 deletions(-) diff --git a/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml b/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml index 5469d033a..0242480ea 100644 --- a/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml @@ -50,7 +50,7 @@ spec: template: metadata: annotations: - cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: cacert,config-cinderplugin,secret-cinderplugin,socket-dir + cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: cacert,config-cinderplugin,cloud-credentials,socket-dir openshift.io/required-scc: restricted-v2 target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}' labels: @@ -138,16 +138,16 @@ spec: readOnlyRootFilesystem: true terminationMessagePolicy: FallbackToLogsOnError volumeMounts: + - mountPath: /csi + name: socket-dir - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config name: cacert - mountPath: /etc/kubernetes/config name: config-cinderplugin readOnly: true - mountPath: /etc/kubernetes/secret - name: secret-cinderplugin + name: cloud-credentials readOnly: true - - mountPath: /csi - name: socket-dir - args: - --secure-listen-address=0.0.0.0:9202 - --upstream=http://127.0.0.1:8202/ @@ -422,7 +422,7 @@ spec: - name: metrics-serving-cert secret: secretName: openstack-cinder-csi-driver-controller-metrics-serving-cert - - name: secret-cinderplugin + - name: cloud-credentials secret: items: - key: clouds.yaml diff --git a/assets/overlays/openstack-cinder/generated/hypershift/node.yaml b/assets/overlays/openstack-cinder/generated/hypershift/node.yaml index e3809aeb7..5c1bf344e 100644 --- a/assets/overlays/openstack-cinder/generated/hypershift/node.yaml +++ b/assets/overlays/openstack-cinder/generated/hypershift/node.yaml @@ -98,7 +98,7 @@ spec: name: config-cinderplugin readOnly: true - mountPath: /etc/kubernetes/secret - name: secret-cinderplugin + name: cloud-credentials readOnly: true - args: - --csi-address=/csi/csi.sock @@ -196,25 +196,25 @@ spec: - name: metrics-serving-cert secret: secretName: openstack-cinder-csi-driver-node-metrics-serving-cert - - configMap: + - name: cloud-credentials + secret: items: - - key: ca-bundle.pem - path: ca-bundle.pem - name: cloud-conf - optional: true - name: cacert + - key: clouds.yaml + path: clouds.yaml + secretName: openstack-cloud-credentials - configMap: items: - key: cloud.conf path: cloud.conf name: cloud-conf name: config-cinderplugin - - name: secret-cinderplugin - secret: + - configMap: items: - - key: clouds.yaml - path: clouds.yaml - secretName: openstack-cloud-credentials + - key: ca-bundle.pem + path: ca-bundle.pem + name: cloud-conf + optional: true + name: cacert updateStrategy: rollingUpdate: maxUnavailable: 10% diff --git a/assets/overlays/openstack-cinder/generated/standalone/controller.yaml b/assets/overlays/openstack-cinder/generated/standalone/controller.yaml index 4b5d0b812..cd2017bca 100644 --- a/assets/overlays/openstack-cinder/generated/standalone/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/standalone/controller.yaml @@ -41,7 +41,7 @@ spec: template: metadata: annotations: - cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: cacert,config-cinderplugin,secret-cinderplugin,socket-dir + cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: cacert,config-cinderplugin,cloud-credentials,socket-dir openshift.io/required-scc: restricted-v2 target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}' labels: @@ -104,16 +104,16 @@ spec: readOnlyRootFilesystem: true terminationMessagePolicy: FallbackToLogsOnError volumeMounts: + - mountPath: /csi + name: socket-dir - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config name: cacert - mountPath: /etc/kubernetes/config name: config-cinderplugin readOnly: true - mountPath: /etc/kubernetes/secret - name: secret-cinderplugin + name: cloud-credentials readOnly: true - - mountPath: /csi - name: socket-dir - args: - --secure-listen-address=0.0.0.0:9202 - --upstream=http://127.0.0.1:8202/ @@ -361,7 +361,7 @@ spec: - name: metrics-serving-cert secret: secretName: openstack-cinder-csi-driver-controller-metrics-serving-cert - - name: secret-cinderplugin + - name: cloud-credentials secret: items: - key: clouds.yaml diff --git a/assets/overlays/openstack-cinder/generated/standalone/node.yaml b/assets/overlays/openstack-cinder/generated/standalone/node.yaml index e3809aeb7..5c1bf344e 100644 --- a/assets/overlays/openstack-cinder/generated/standalone/node.yaml +++ b/assets/overlays/openstack-cinder/generated/standalone/node.yaml @@ -98,7 +98,7 @@ spec: name: config-cinderplugin readOnly: true - mountPath: /etc/kubernetes/secret - name: secret-cinderplugin + name: cloud-credentials readOnly: true - args: - --csi-address=/csi/csi.sock @@ -196,25 +196,25 @@ spec: - name: metrics-serving-cert secret: secretName: openstack-cinder-csi-driver-node-metrics-serving-cert - - configMap: + - name: cloud-credentials + secret: items: - - key: ca-bundle.pem - path: ca-bundle.pem - name: cloud-conf - optional: true - name: cacert + - key: clouds.yaml + path: clouds.yaml + secretName: openstack-cloud-credentials - configMap: items: - key: cloud.conf path: cloud.conf name: cloud-conf name: config-cinderplugin - - name: secret-cinderplugin - secret: + - configMap: items: - - key: clouds.yaml - path: clouds.yaml - secretName: openstack-cloud-credentials + - key: ca-bundle.pem + path: ca-bundle.pem + name: cloud-conf + optional: true + name: cacert updateStrategy: rollingUpdate: maxUnavailable: 10% diff --git a/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml b/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml index a0bdcaa90..72c732974 100644 --- a/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml +++ b/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml @@ -8,7 +8,7 @@ spec: template: metadata: annotations: - cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: "cacert,config-cinderplugin,secret-cinderplugin,socket-dir" + cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: "cacert,config-cinderplugin,cloud-credentials,socket-dir" openshift.io/required-scc: restricted-v2 labels: openshift.storage.network-policy.all-egress: allow @@ -71,23 +71,24 @@ spec: periodSeconds: 30 failureThreshold: 5 volumeMounts: + - name: socket-dir + mountPath: /csi + # credentials and configuration - name: cacert mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config - name: config-cinderplugin mountPath: /etc/kubernetes/config readOnly: true - - name: secret-cinderplugin + - name: cloud-credentials mountPath: /etc/kubernetes/secret readOnly: true - - name: socket-dir - mountPath: /csi resources: requests: memory: 50Mi cpu: 10m terminationMessagePolicy: FallbackToLogsOnError volumes: - - name: secret-cinderplugin + - name: cloud-credentials secret: secretName: openstack-cloud-credentials items: @@ -100,14 +101,9 @@ spec: - key: cloud.conf path: cloud.conf - name: cacert - # If present, extract ca-bundle.pem to - # /etc/kubernetes/static-pod-resources/configmaps/cloud-config - # Let the pod start when the ConfigMap does not exist or the certificate - # is not preset there. The certificate file will be created once the - # ConfigMap is created / the certificate is added to it. configMap: name: cloud-conf items: - - key: ca-bundle.pem - path: ca-bundle.pem + - key: ca-bundle.pem + path: ca-bundle.pem optional: true diff --git a/assets/overlays/openstack-cinder/patches/node_add_driver.yaml b/assets/overlays/openstack-cinder/patches/node_add_driver.yaml index 87ccadfc1..45683ceb5 100644 --- a/assets/overlays/openstack-cinder/patches/node_add_driver.yaml +++ b/assets/overlays/openstack-cinder/patches/node_add_driver.yaml @@ -50,12 +50,13 @@ spec: mountPath: /etc/selinux - name: sys-fs mountPath: /sys/fs + # credentials and configuration - name: cacert mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config - name: config-cinderplugin mountPath: /etc/kubernetes/config readOnly: true - - name: secret-cinderplugin + - name: cloud-credentials mountPath: /etc/kubernetes/secret readOnly: true ports: @@ -77,26 +78,22 @@ spec: cpu: 10m terminationMessagePolicy: FallbackToLogsOnError volumes: - - name: cacert - # Extract ca-bundle.pem to /etc/kubernetes/static-pod-resources/configmaps/cloud-config if present. - # Let the pod start when the ConfigMap does not exist or the certificate - # is not preset there. The certificate file will be created once the - # ConfigMap is created / the certificate is added to it. - configMap: - name: cloud-conf + - name: cloud-credentials + secret: + secretName: openstack-cloud-credentials items: - - key: ca-bundle.pem - path: ca-bundle.pem - optional: true + - key: clouds.yaml + path: clouds.yaml - name: config-cinderplugin configMap: name: cloud-conf items: - key: cloud.conf path: cloud.conf - - name: secret-cinderplugin - secret: - secretName: openstack-cloud-credentials + - name: cacert + configMap: + name: cloud-conf items: - - key: clouds.yaml - path: clouds.yaml + - key: ca-bundle.pem + path: ca-bundle.pem + optional: true From b61849003c146ce1ee5f3cf2b49f65d77f5b80b1 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 12 Mar 2025 12:59:42 +0000 Subject: [PATCH 2/9] openstack-cinder: Make CA cert mount read-only Signed-off-by: Stephen Finucane --- .../openstack-cinder/generated/hypershift/controller.yaml | 1 + assets/overlays/openstack-cinder/generated/hypershift/node.yaml | 1 + .../openstack-cinder/generated/standalone/controller.yaml | 1 + assets/overlays/openstack-cinder/generated/standalone/node.yaml | 1 + .../overlays/openstack-cinder/patches/controller_add_driver.yaml | 1 + assets/overlays/openstack-cinder/patches/node_add_driver.yaml | 1 + 6 files changed, 6 insertions(+) diff --git a/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml b/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml index 0242480ea..c8703b4ff 100644 --- a/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml @@ -142,6 +142,7 @@ spec: name: socket-dir - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config name: cacert + readOnly: true - mountPath: /etc/kubernetes/config name: config-cinderplugin readOnly: true diff --git a/assets/overlays/openstack-cinder/generated/hypershift/node.yaml b/assets/overlays/openstack-cinder/generated/hypershift/node.yaml index 5c1bf344e..2db00a876 100644 --- a/assets/overlays/openstack-cinder/generated/hypershift/node.yaml +++ b/assets/overlays/openstack-cinder/generated/hypershift/node.yaml @@ -94,6 +94,7 @@ spec: name: sys-fs - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config name: cacert + readOnly: true - mountPath: /etc/kubernetes/config name: config-cinderplugin readOnly: true diff --git a/assets/overlays/openstack-cinder/generated/standalone/controller.yaml b/assets/overlays/openstack-cinder/generated/standalone/controller.yaml index cd2017bca..790b3f625 100644 --- a/assets/overlays/openstack-cinder/generated/standalone/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/standalone/controller.yaml @@ -108,6 +108,7 @@ spec: name: socket-dir - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config name: cacert + readOnly: true - mountPath: /etc/kubernetes/config name: config-cinderplugin readOnly: true diff --git a/assets/overlays/openstack-cinder/generated/standalone/node.yaml b/assets/overlays/openstack-cinder/generated/standalone/node.yaml index 5c1bf344e..2db00a876 100644 --- a/assets/overlays/openstack-cinder/generated/standalone/node.yaml +++ b/assets/overlays/openstack-cinder/generated/standalone/node.yaml @@ -94,6 +94,7 @@ spec: name: sys-fs - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config name: cacert + readOnly: true - mountPath: /etc/kubernetes/config name: config-cinderplugin readOnly: true diff --git a/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml b/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml index 72c732974..df2987c28 100644 --- a/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml +++ b/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml @@ -76,6 +76,7 @@ spec: # credentials and configuration - name: cacert mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config + readOnly: true - name: config-cinderplugin mountPath: /etc/kubernetes/config readOnly: true diff --git a/assets/overlays/openstack-cinder/patches/node_add_driver.yaml b/assets/overlays/openstack-cinder/patches/node_add_driver.yaml index 45683ceb5..f1590e036 100644 --- a/assets/overlays/openstack-cinder/patches/node_add_driver.yaml +++ b/assets/overlays/openstack-cinder/patches/node_add_driver.yaml @@ -53,6 +53,7 @@ spec: # credentials and configuration - name: cacert mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config + readOnly: true - name: config-cinderplugin mountPath: /etc/kubernetes/config readOnly: true From 7ca449f90665434fbcadedf551666de0b5c94fa3 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 13 Mar 2025 10:37:42 +0000 Subject: [PATCH 3/9] openstack-cinder: Change mount for cloud credentials Put it in a more usual place. Signed-off-by: Stephen Finucane --- .../generated/hypershift/controller.yaml | 2 +- .../openstack-cinder/generated/hypershift/node.yaml | 2 +- .../generated/standalone/controller.yaml | 2 +- .../openstack-cinder/generated/standalone/node.yaml | 2 +- .../patches/controller_add_driver.yaml | 2 +- .../openstack-cinder/patches/node_add_driver.yaml | 2 +- pkg/openstack-cinder/config/configsync.go | 2 +- pkg/openstack-cinder/config/configsync_test.go | 10 +++++----- 8 files changed, 12 insertions(+), 12 deletions(-) diff --git a/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml b/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml index c8703b4ff..165e58dc8 100644 --- a/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml @@ -146,7 +146,7 @@ spec: - mountPath: /etc/kubernetes/config name: config-cinderplugin readOnly: true - - mountPath: /etc/kubernetes/secret + - mountPath: /etc/openstack name: cloud-credentials readOnly: true - args: diff --git a/assets/overlays/openstack-cinder/generated/hypershift/node.yaml b/assets/overlays/openstack-cinder/generated/hypershift/node.yaml index 2db00a876..b3d681883 100644 --- a/assets/overlays/openstack-cinder/generated/hypershift/node.yaml +++ b/assets/overlays/openstack-cinder/generated/hypershift/node.yaml @@ -98,7 +98,7 @@ spec: - mountPath: /etc/kubernetes/config name: config-cinderplugin readOnly: true - - mountPath: /etc/kubernetes/secret + - mountPath: /etc/openstack name: cloud-credentials readOnly: true - args: diff --git a/assets/overlays/openstack-cinder/generated/standalone/controller.yaml b/assets/overlays/openstack-cinder/generated/standalone/controller.yaml index 790b3f625..445256447 100644 --- a/assets/overlays/openstack-cinder/generated/standalone/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/standalone/controller.yaml @@ -112,7 +112,7 @@ spec: - mountPath: /etc/kubernetes/config name: config-cinderplugin readOnly: true - - mountPath: /etc/kubernetes/secret + - mountPath: /etc/openstack name: cloud-credentials readOnly: true - args: diff --git a/assets/overlays/openstack-cinder/generated/standalone/node.yaml b/assets/overlays/openstack-cinder/generated/standalone/node.yaml index 2db00a876..b3d681883 100644 --- a/assets/overlays/openstack-cinder/generated/standalone/node.yaml +++ b/assets/overlays/openstack-cinder/generated/standalone/node.yaml @@ -98,7 +98,7 @@ spec: - mountPath: /etc/kubernetes/config name: config-cinderplugin readOnly: true - - mountPath: /etc/kubernetes/secret + - mountPath: /etc/openstack name: cloud-credentials readOnly: true - args: diff --git a/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml b/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml index df2987c28..e948dc333 100644 --- a/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml +++ b/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml @@ -81,7 +81,7 @@ spec: mountPath: /etc/kubernetes/config readOnly: true - name: cloud-credentials - mountPath: /etc/kubernetes/secret + mountPath: /etc/openstack readOnly: true resources: requests: diff --git a/assets/overlays/openstack-cinder/patches/node_add_driver.yaml b/assets/overlays/openstack-cinder/patches/node_add_driver.yaml index f1590e036..3c26246b3 100644 --- a/assets/overlays/openstack-cinder/patches/node_add_driver.yaml +++ b/assets/overlays/openstack-cinder/patches/node_add_driver.yaml @@ -58,7 +58,7 @@ spec: mountPath: /etc/kubernetes/config readOnly: true - name: cloud-credentials - mountPath: /etc/kubernetes/secret + mountPath: /etc/openstack readOnly: true ports: - name: healthz diff --git a/pkg/openstack-cinder/config/configsync.go b/pkg/openstack-cinder/config/configsync.go index 50f2b4e74..bac78ac8b 100644 --- a/pkg/openstack-cinder/config/configsync.go +++ b/pkg/openstack-cinder/config/configsync.go @@ -295,7 +295,7 @@ func generateConfig( } for _, o := range []struct{ k, v string }{ {"use-clouds", "true"}, - {"clouds-file", "/etc/kubernetes/secret/clouds.yaml"}, + {"clouds-file", "/etc/openstack/clouds.yaml"}, {"cloud", "openstack"}, } { _, err = global.NewKey(o.k, o.v) diff --git a/pkg/openstack-cinder/config/configsync_test.go b/pkg/openstack-cinder/config/configsync_test.go index 54dea154f..29e977619 100644 --- a/pkg/openstack-cinder/config/configsync_test.go +++ b/pkg/openstack-cinder/config/configsync_test.go @@ -25,14 +25,14 @@ func TestGenerateConfigMap(t *testing.T) { source: nil, expected: `[Global] use-clouds = true -clouds-file = /etc/kubernetes/secret/clouds.yaml +clouds-file = /etc/openstack/clouds.yaml cloud = openstack`, }, { name: "Empty config", source: []byte(""), expected: `[Global] use-clouds = true -clouds-file = /etc/kubernetes/secret/clouds.yaml +clouds-file = /etc/openstack/clouds.yaml cloud = openstack`, }, { name: "Minimal config", @@ -43,7 +43,7 @@ ignore-volume-az = True [Global] use-clouds = true -clouds-file = /etc/kubernetes/secret/clouds.yaml +clouds-file = /etc/openstack/clouds.yaml cloud = openstack`, }, { name: "With CA cert", @@ -51,7 +51,7 @@ cloud = openstack`, caCert: ptr.To("not-so-secret CA data goes here"), expected: `[Global] use-clouds = true -clouds-file = /etc/kubernetes/secret/clouds.yaml +clouds-file = /etc/openstack/clouds.yaml cloud = openstack ca-file = /etc/kubernetes/static-pod-resources/configmaps/cloud-config/ca-bundle.pem`, }, { @@ -61,7 +61,7 @@ ca-file = /etc/kubernetes/static-pod-resources/configmaps/cloud-config/ca-bu trust-device-path = /dev/sdb1`), expected: `[Global] use-clouds = true -clouds-file = /etc/kubernetes/secret/clouds.yaml +clouds-file = /etc/openstack/clouds.yaml cloud = openstack`, }, } From b41c54a5d46a7f795be98e1b089b8b999c7d7737 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 13 Mar 2025 10:58:42 +0000 Subject: [PATCH 4/9] openstack-cinder: Rename cacert mount This is going to be superseded in a coming change. Rename it in preparation. Signed-off-by: Stephen Finucane --- .../generated/hypershift/controller.yaml | 10 +++++----- .../openstack-cinder/generated/hypershift/node.yaml | 8 ++++---- .../generated/standalone/controller.yaml | 10 +++++----- .../openstack-cinder/generated/standalone/node.yaml | 8 ++++---- .../patches/controller_add_driver.yaml | 10 +++++----- .../openstack-cinder/patches/node_add_driver.yaml | 8 ++++---- 6 files changed, 27 insertions(+), 27 deletions(-) diff --git a/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml b/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml index 165e58dc8..3223788fa 100644 --- a/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml @@ -50,7 +50,7 @@ spec: template: metadata: annotations: - cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: cacert,config-cinderplugin,cloud-credentials,socket-dir + cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: config-cinderplugin,cloud-credentials,legacy-cacert,socket-dir openshift.io/required-scc: restricted-v2 target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}' labels: @@ -140,15 +140,15 @@ spec: volumeMounts: - mountPath: /csi name: socket-dir - - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config - name: cacert - readOnly: true - mountPath: /etc/kubernetes/config name: config-cinderplugin readOnly: true - mountPath: /etc/openstack name: cloud-credentials readOnly: true + - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config + name: legacy-cacert + readOnly: true - args: - --secure-listen-address=0.0.0.0:9202 - --upstream=http://127.0.0.1:8202/ @@ -441,7 +441,7 @@ spec: path: ca-bundle.pem name: cloud-conf optional: true - name: cacert + name: legacy-cacert - name: hosted-kubeconfig secret: defaultMode: 420 diff --git a/assets/overlays/openstack-cinder/generated/hypershift/node.yaml b/assets/overlays/openstack-cinder/generated/hypershift/node.yaml index b3d681883..e76842584 100644 --- a/assets/overlays/openstack-cinder/generated/hypershift/node.yaml +++ b/assets/overlays/openstack-cinder/generated/hypershift/node.yaml @@ -92,15 +92,15 @@ spec: name: etc-selinux - mountPath: /sys/fs name: sys-fs - - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config - name: cacert - readOnly: true - mountPath: /etc/kubernetes/config name: config-cinderplugin readOnly: true - mountPath: /etc/openstack name: cloud-credentials readOnly: true + - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config + name: legacy-cacert + readOnly: true - args: - --csi-address=/csi/csi.sock - --http-endpoint=127.0.0.1:10300 @@ -215,7 +215,7 @@ spec: path: ca-bundle.pem name: cloud-conf optional: true - name: cacert + name: legacy-cacert updateStrategy: rollingUpdate: maxUnavailable: 10% diff --git a/assets/overlays/openstack-cinder/generated/standalone/controller.yaml b/assets/overlays/openstack-cinder/generated/standalone/controller.yaml index 445256447..a3c498b04 100644 --- a/assets/overlays/openstack-cinder/generated/standalone/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/standalone/controller.yaml @@ -41,7 +41,7 @@ spec: template: metadata: annotations: - cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: cacert,config-cinderplugin,cloud-credentials,socket-dir + cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: config-cinderplugin,cloud-credentials,legacy-cacert,socket-dir openshift.io/required-scc: restricted-v2 target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}' labels: @@ -106,15 +106,15 @@ spec: volumeMounts: - mountPath: /csi name: socket-dir - - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config - name: cacert - readOnly: true - mountPath: /etc/kubernetes/config name: config-cinderplugin readOnly: true - mountPath: /etc/openstack name: cloud-credentials readOnly: true + - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config + name: legacy-cacert + readOnly: true - args: - --secure-listen-address=0.0.0.0:9202 - --upstream=http://127.0.0.1:8202/ @@ -380,4 +380,4 @@ spec: path: ca-bundle.pem name: cloud-conf optional: true - name: cacert + name: legacy-cacert diff --git a/assets/overlays/openstack-cinder/generated/standalone/node.yaml b/assets/overlays/openstack-cinder/generated/standalone/node.yaml index b3d681883..e76842584 100644 --- a/assets/overlays/openstack-cinder/generated/standalone/node.yaml +++ b/assets/overlays/openstack-cinder/generated/standalone/node.yaml @@ -92,15 +92,15 @@ spec: name: etc-selinux - mountPath: /sys/fs name: sys-fs - - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config - name: cacert - readOnly: true - mountPath: /etc/kubernetes/config name: config-cinderplugin readOnly: true - mountPath: /etc/openstack name: cloud-credentials readOnly: true + - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config + name: legacy-cacert + readOnly: true - args: - --csi-address=/csi/csi.sock - --http-endpoint=127.0.0.1:10300 @@ -215,7 +215,7 @@ spec: path: ca-bundle.pem name: cloud-conf optional: true - name: cacert + name: legacy-cacert updateStrategy: rollingUpdate: maxUnavailable: 10% diff --git a/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml b/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml index e948dc333..95a921eea 100644 --- a/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml +++ b/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml @@ -8,7 +8,7 @@ spec: template: metadata: annotations: - cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: "cacert,config-cinderplugin,cloud-credentials,socket-dir" + cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: "config-cinderplugin,cloud-credentials,legacy-cacert,socket-dir" openshift.io/required-scc: restricted-v2 labels: openshift.storage.network-policy.all-egress: allow @@ -74,15 +74,15 @@ spec: - name: socket-dir mountPath: /csi # credentials and configuration - - name: cacert - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config - readOnly: true - name: config-cinderplugin mountPath: /etc/kubernetes/config readOnly: true - name: cloud-credentials mountPath: /etc/openstack readOnly: true + - name: legacy-cacert + mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config + readOnly: true resources: requests: memory: 50Mi @@ -101,7 +101,7 @@ spec: items: - key: cloud.conf path: cloud.conf - - name: cacert + - name: legacy-cacert configMap: name: cloud-conf items: diff --git a/assets/overlays/openstack-cinder/patches/node_add_driver.yaml b/assets/overlays/openstack-cinder/patches/node_add_driver.yaml index 3c26246b3..367b5bc9f 100644 --- a/assets/overlays/openstack-cinder/patches/node_add_driver.yaml +++ b/assets/overlays/openstack-cinder/patches/node_add_driver.yaml @@ -51,15 +51,15 @@ spec: - name: sys-fs mountPath: /sys/fs # credentials and configuration - - name: cacert - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config - readOnly: true - name: config-cinderplugin mountPath: /etc/kubernetes/config readOnly: true - name: cloud-credentials mountPath: /etc/openstack readOnly: true + - name: legacy-cacert + mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config + readOnly: true ports: - name: healthz containerPort: 10300 @@ -91,7 +91,7 @@ spec: items: - key: cloud.conf path: cloud.conf - - name: cacert + - name: legacy-cacert configMap: name: cloud-conf items: From 9fb1579e70a9ce9efe2db8e49017f341478b1267 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 13 Mar 2025 10:59:48 +0000 Subject: [PATCH 5/9] openstack-cinder: Consume CA cert from credentials secret (assets) In this change, we modify the assets to start (optionally) mounting the CA cert from the secret in the containers. We leave a fallback in place for the old config map source to allow time for the cloud-credential-operator to update things in an upgrade scenario. This fallback can be removed in 4.20, as noted by the copious TODOs. Signed-off-by: Stephen Finucane --- .../generated/hypershift/controller.yaml | 18 ++++++++++++----- .../generated/hypershift/node.yaml | 18 ++++++++++++----- .../generated/standalone/controller.yaml | 18 ++++++++++++----- .../generated/standalone/node.yaml | 18 ++++++++++++----- .../patches/controller_add_driver.yaml | 20 ++++++++++++++----- .../patches/node_add_driver.yaml | 20 ++++++++++++++----- 6 files changed, 82 insertions(+), 30 deletions(-) diff --git a/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml b/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml index 3223788fa..e6b08c3f3 100644 --- a/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml @@ -424,11 +424,19 @@ spec: secret: secretName: openstack-cinder-csi-driver-controller-metrics-serving-cert - name: cloud-credentials - secret: - items: - - key: clouds.yaml - path: clouds.yaml - secretName: openstack-cloud-credentials + projected: + sources: + - secret: + items: + - key: cacert + path: ca.crt + name: openstack-cloud-credentials + optional: true + - secret: + items: + - key: clouds.yaml + path: clouds.yaml + name: openstack-cloud-credentials - configMap: items: - key: cloud.conf diff --git a/assets/overlays/openstack-cinder/generated/hypershift/node.yaml b/assets/overlays/openstack-cinder/generated/hypershift/node.yaml index e76842584..6ba6b1ff6 100644 --- a/assets/overlays/openstack-cinder/generated/hypershift/node.yaml +++ b/assets/overlays/openstack-cinder/generated/hypershift/node.yaml @@ -198,11 +198,19 @@ spec: secret: secretName: openstack-cinder-csi-driver-node-metrics-serving-cert - name: cloud-credentials - secret: - items: - - key: clouds.yaml - path: clouds.yaml - secretName: openstack-cloud-credentials + projected: + sources: + - secret: + items: + - key: cacert + path: ca.crt + name: openstack-cloud-credentials + optional: true + - secret: + items: + - key: clouds.yaml + path: clouds.yaml + name: openstack-cloud-credentials - configMap: items: - key: cloud.conf diff --git a/assets/overlays/openstack-cinder/generated/standalone/controller.yaml b/assets/overlays/openstack-cinder/generated/standalone/controller.yaml index a3c498b04..8165c73ce 100644 --- a/assets/overlays/openstack-cinder/generated/standalone/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/standalone/controller.yaml @@ -363,11 +363,19 @@ spec: secret: secretName: openstack-cinder-csi-driver-controller-metrics-serving-cert - name: cloud-credentials - secret: - items: - - key: clouds.yaml - path: clouds.yaml - secretName: openstack-cloud-credentials + projected: + sources: + - secret: + items: + - key: cacert + path: ca.crt + name: openstack-cloud-credentials + optional: true + - secret: + items: + - key: clouds.yaml + path: clouds.yaml + name: openstack-cloud-credentials - configMap: items: - key: cloud.conf diff --git a/assets/overlays/openstack-cinder/generated/standalone/node.yaml b/assets/overlays/openstack-cinder/generated/standalone/node.yaml index e76842584..6ba6b1ff6 100644 --- a/assets/overlays/openstack-cinder/generated/standalone/node.yaml +++ b/assets/overlays/openstack-cinder/generated/standalone/node.yaml @@ -198,11 +198,19 @@ spec: secret: secretName: openstack-cinder-csi-driver-node-metrics-serving-cert - name: cloud-credentials - secret: - items: - - key: clouds.yaml - path: clouds.yaml - secretName: openstack-cloud-credentials + projected: + sources: + - secret: + items: + - key: cacert + path: ca.crt + name: openstack-cloud-credentials + optional: true + - secret: + items: + - key: clouds.yaml + path: clouds.yaml + name: openstack-cloud-credentials - configMap: items: - key: cloud.conf diff --git a/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml b/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml index 95a921eea..eee418eef 100644 --- a/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml +++ b/assets/overlays/openstack-cinder/patches/controller_add_driver.yaml @@ -80,6 +80,7 @@ spec: - name: cloud-credentials mountPath: /etc/openstack readOnly: true + # TODO(stephenfin): Remove in 4.22 - name: legacy-cacert mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config readOnly: true @@ -90,17 +91,26 @@ spec: terminationMessagePolicy: FallbackToLogsOnError volumes: - name: cloud-credentials - secret: - secretName: openstack-cloud-credentials - items: - - key: clouds.yaml - path: clouds.yaml + projected: + sources: + - secret: + name: openstack-cloud-credentials + items: + - key: cacert + path: ca.crt + optional: true + - secret: + name: openstack-cloud-credentials + items: + - key: clouds.yaml + path: clouds.yaml - name: config-cinderplugin configMap: name: cloud-conf items: - key: cloud.conf path: cloud.conf + # TODO(stephenfin): Remove in 4.22 - name: legacy-cacert configMap: name: cloud-conf diff --git a/assets/overlays/openstack-cinder/patches/node_add_driver.yaml b/assets/overlays/openstack-cinder/patches/node_add_driver.yaml index 367b5bc9f..a69546223 100644 --- a/assets/overlays/openstack-cinder/patches/node_add_driver.yaml +++ b/assets/overlays/openstack-cinder/patches/node_add_driver.yaml @@ -57,6 +57,7 @@ spec: - name: cloud-credentials mountPath: /etc/openstack readOnly: true + # TODO(stephenfin): Remove in 4.22 - name: legacy-cacert mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config readOnly: true @@ -80,17 +81,26 @@ spec: terminationMessagePolicy: FallbackToLogsOnError volumes: - name: cloud-credentials - secret: - secretName: openstack-cloud-credentials - items: - - key: clouds.yaml - path: clouds.yaml + projected: + sources: + - secret: + name: openstack-cloud-credentials + items: + - key: cacert + path: ca.crt + optional: true + - secret: + name: openstack-cloud-credentials + items: + - key: clouds.yaml + path: clouds.yaml - name: config-cinderplugin configMap: name: cloud-conf items: - key: cloud.conf path: cloud.conf + # TODO(stephenfin): Remove in 4.22 - name: legacy-cacert configMap: name: cloud-conf From 28ab57c0a137bee3e394411b1631fe2ee743f79b Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 19 Feb 2025 14:20:33 +0000 Subject: [PATCH 6/9] openstack-cinder: Consume CA cert from credentials secret (controller) 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 for the old config map source to allow time for the cloud-credential-operator to update things in an upgrade scenario. This fallback can be removed in 4.22, as noted by the copious TODOs. Signed-off-by: Stephen Finucane --- pkg/openstack-cinder/config/cloudinfo.go | 8 +- pkg/openstack-cinder/config/configsync.go | 97 +++++++++++++------ .../config/configsync_test.go | 28 ++++-- pkg/openstack-cinder/config/const.go | 5 + 4 files changed, 95 insertions(+), 43 deletions(-) create mode 100644 pkg/openstack-cinder/config/const.go diff --git a/pkg/openstack-cinder/config/cloudinfo.go b/pkg/openstack-cinder/config/cloudinfo.go index 2e41047cd..70c47e4c3 100644 --- a/pkg/openstack-cinder/config/cloudinfo.go +++ b/pkg/openstack-cinder/config/cloudinfo.go @@ -13,8 +13,6 @@ import ( "github.com/openshift/csi-operator/pkg/version" ) -const caFile = "/etc/kubernetes/static-pod-resources/configmaps/cloud-config/ca-bundle.pem" - // CloudInfo caches data fetched from the user's openstack cloud type CloudInfo struct { ComputeZones []string @@ -87,10 +85,12 @@ func getCloudInfo() (*CloudInfo, error) { // our assets mount the CA file at a known path and we don't want to rely on other things // setting the 'cafile' value in our clouds.yaml file to the same path, so we explicitly // override this if a CA file is present + // NOTE(stephenfin): gophercloud (or rather, the clientconfig package) doesn't currently + // provide the way to override configuration other than via environment variables if _, err := os.Stat(caFile); err == nil { - // NOTE(stephenfin): gophercloud (or rather, the clientconfig package) doesn't currently - // provide the way to override configuration other than via environment variables os.Setenv("OS_CACERT", caFile) + } else if _, err := os.Stat(legacyCAFile); err == nil { // legacy path + os.Setenv("OS_CACERT", legacyCAFile) } ci.clients.computeClient, err = clientconfig.NewServiceClient(context.TODO(), "compute", opts) diff --git a/pkg/openstack-cinder/config/configsync.go b/pkg/openstack-cinder/config/configsync.go index bac78ac8b..576cbc87c 100644 --- a/pkg/openstack-cinder/config/configsync.go +++ b/pkg/openstack-cinder/config/configsync.go @@ -37,6 +37,13 @@ type configDestination struct { kubeClient kubernetes.Interface } +type caCertSource string + +const ( + configCACertSource caCertSource = "config" + secretCACertSource caCertSource = "secret" +) + // This ConfigSyncController generates the configuration file needed for the Cinder CSI controller // and driver, including optional configuration from the user, and save it to a config map in a // well-known location that both services can use @@ -159,27 +166,39 @@ func (c *ConfigSyncController) sync(ctx context.Context, syncCtx factory.SyncCon enableTopology = strconv.FormatBool(enableTopologyFeature) } - // ...and we watch the clusters-${NAME} namespace (on the control plane cluster) - configMapLister = c.controlPlaneInformers.InformersFor(c.controlPlaneNamespace).Core().V1().ConfigMaps().Lister() - - // FIXME(stephenfin): We extract the CA cert from the cloud-provider-config because we know - // it will exist here in both a standalone (IPI) deployment and in both clusters in a - // Hypershift deployment. However, this is less than ideal: cloud providers and CSI drivers are - // now separate things and we should be able to get this information from the credentials - // secret, along with our clouds.yaml. Fixing this will requires changes to - // cloud-credential-operator and hypershift (because the latter doesn't currently use the - // former). Once we have that feature, we should rejig this. - cloudProviderConfig := "cloud-provider-config" - if c.isHypershift { - // unfortunately hypershift uses a different name - cloudProviderConfig = "openstack-cloud-config" - } - caCert, err := getCACert(configMapLister, c.controlPlaneNamespace, cloudProviderConfig) + secretLister := c.controlPlaneInformers.InformersFor(c.controlPlaneNamespace).Core().V1().Secrets().Lister() + + var caCertSource caCertSource + + hasCACert, err := hasCACert(secretLister, c.controlPlaneNamespace, "openstack-cloud-credentials") if err != nil { return err } - generatedConfig, err := generateConfig(sourceConfig, caCert) + if hasCACert { + caCertSource = secretCACertSource + } else { + // TODO(stephenfin): Remove this fallback in 4.22. At that point, the + // cloud-credentials-operator will have ensured the root credential has + // been updated. + configMapLister = c.controlPlaneInformers.InformersFor(c.controlPlaneNamespace).Core().V1().ConfigMaps().Lister() + cloudProviderConfig := "cloud-provider-config" + if c.isHypershift { + // unfortunately hypershift uses a different name + cloudProviderConfig = "openstack-cloud-config" + } + + hasCACert, err = getCACertLegacy(configMapLister, c.controlPlaneNamespace, cloudProviderConfig) + if err != nil { + return err + } + + if hasCACert { + caCertSource = configCACertSource + } + } + + generatedConfig, err := generateConfig(sourceConfig, caCertSource) if err != nil { return err } @@ -195,9 +214,6 @@ func (c *ConfigSyncController) sync(ctx context.Context, syncCtx factory.SyncCon "enable_topology": enableTopology, }, } - if caCert != nil { - targetConfig.Data["ca-bundle.pem"] = *caCert - } _, _, err = resourceapply.ApplyConfigMap(ctx, configDestination.kubeClient.CoreV1(), c.eventRecorder, targetConfig) if err != nil { return err @@ -206,18 +222,35 @@ func (c *ConfigSyncController) sync(ctx context.Context, syncCtx factory.SyncCon return nil } -// getCACert gets the (optional) embedded CA Cert provided in a config map by either the Installer -// or the hypershift-operator so that we can inject it into our config files -func getCACert(configMapLister corelisters.ConfigMapLister, ns, name string) (*string, error) { +// hasCACert determines whether there is a CA Cert provided in the credentials +// secret by either the cloud-credential-operator or hypershift-operator so +// that we can inject it into our configuration +func hasCACert(secretLister corelisters.SecretLister, ns, name string) (bool, error) { + cm, err := secretLister.Secrets(ns).Get(name) + if err != nil { + return false, err + } + caCert, ok := cm.Data["cacert"] + if !ok || len(caCert) == 0 { + return false, nil + } + return true, nil +} + +// getCACertLegacy determines whether there is a CA Cert provided in the cloud +// provider config map by either the Installer or the hypershift-operator so +// that we can inject it into our config files. This is the legacy path for the +// transition period as the CA cert is now provided in the credentials secret +func getCACertLegacy(configMapLister corelisters.ConfigMapLister, ns, name string) (bool, error) { cm, err := configMapLister.ConfigMaps(ns).Get(name) if err != nil { - return nil, err + return false, err } caCert, ok := cm.Data["ca-bundle.pem"] if !ok || len(caCert) == 0 { - return nil, nil + return false, nil } - return &caCert, nil + return true, nil } // getSourceConfig extracts any source configuration present in any of the provided for the legacy, @@ -258,7 +291,7 @@ func getSourceConfig( // '[BlockStorage]' section (and only that section) will be used in generation. func generateConfig( sourceContent []byte, - caCert *string, + caCertSource caCertSource, ) (string, error) { var cfg *ini.File @@ -304,12 +337,18 @@ func generateConfig( } } - if caCert != nil { + if caCertSource != "" { // We don't have a (non-deprecated) way to inject the CA cert itself into the config // file, so instead we pass a file path. The path itself may look like a magic value but // its where we have configured both the deployment (for controller) and daemonset (for // driver) assets to mount the cert, if present. - _, err = global.NewKey("ca-file", "/etc/kubernetes/static-pod-resources/configmaps/cloud-config/ca-bundle.pem") + var path string + if caCertSource == secretCACertSource { + path = caFile + } else { // legacy path + path = legacyCAFile + } + _, err = global.NewKey("ca-file", path) if err != nil { return "", err } diff --git a/pkg/openstack-cinder/config/configsync_test.go b/pkg/openstack-cinder/config/configsync_test.go index 29e977619..d476587e4 100644 --- a/pkg/openstack-cinder/config/configsync_test.go +++ b/pkg/openstack-cinder/config/configsync_test.go @@ -6,7 +6,6 @@ import ( . "github.com/onsi/gomega" "github.com/onsi/gomega/format" - "k8s.io/utils/ptr" ) func TestGenerateConfigMap(t *testing.T) { @@ -14,11 +13,11 @@ func TestGenerateConfigMap(t *testing.T) { format.TruncatedDiff = false tc := []struct { - name string - source []byte - caCert *string - expected string - errMsg string + name string + source []byte + caCertSource caCertSource + expected string + errMsg string }{ { name: "Unset config", @@ -46,9 +45,18 @@ use-clouds = true clouds-file = /etc/openstack/clouds.yaml cloud = openstack`, }, { - name: "With CA cert", - source: nil, - caCert: ptr.To("not-so-secret CA data goes here"), + name: "With CA cert", + source: nil, + caCertSource: secretCACertSource, + expected: `[Global] +use-clouds = true +clouds-file = /etc/openstack/clouds.yaml +cloud = openstack +ca-file = /etc/openstack/ca.crt`, + }, { + name: "With CA cert (legacy)", + source: nil, + caCertSource: configCACertSource, expected: `[Global] use-clouds = true clouds-file = /etc/openstack/clouds.yaml @@ -71,7 +79,7 @@ cloud = openstack`, g := NewWithT(t) actual, err := generateConfig( []byte(tc.source), - tc.caCert, + tc.caCertSource, ) if tc.errMsg != "" { g.Expect(err).Should(MatchError(tc.errMsg)) diff --git a/pkg/openstack-cinder/config/const.go b/pkg/openstack-cinder/config/const.go new file mode 100644 index 000000000..26b09b852 --- /dev/null +++ b/pkg/openstack-cinder/config/const.go @@ -0,0 +1,5 @@ +package config + +// caFile/legacyCAFile is based on the path defined in the assets +const caFile = "/etc/openstack/ca.crt" +const legacyCAFile = "/etc/kubernetes/static-pod-resources/configmaps/cloud-config/ca-bundle.pem" From 1bd97623bf745ed140aabca40fffa3b0063758ce Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 13 Mar 2025 11:06:07 +0000 Subject: [PATCH 7/9] openstack-manila: Rename cacert mount This is going to be superseded in a coming change. Rename it in preparation. Signed-off-by: Stephen Finucane --- .../generated/hypershift/controller.yaml | 4 ++-- .../generated/hypershift/node.yaml | 6 ++--- .../generated/standalone/controller.yaml | 4 ++-- .../generated/standalone/node.yaml | 6 ++--- .../patches/controller_add_driver.yaml | 9 ++------ .../patches/controller_rename_config_map.yaml | 14 +++++------ .../patches/node_add_driver.yaml | 23 ++++++++----------- 7 files changed, 29 insertions(+), 37 deletions(-) diff --git a/assets/overlays/openstack-manila/generated/hypershift/controller.yaml b/assets/overlays/openstack-manila/generated/hypershift/controller.yaml index 7ab727ad2..7908665de 100644 --- a/assets/overlays/openstack-manila/generated/hypershift/controller.yaml +++ b/assets/overlays/openstack-manila/generated/hypershift/controller.yaml @@ -145,7 +145,7 @@ spec: - mountPath: /plugin name: socket-dir - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config - name: cacert + name: legacy-cacert - args: - --nodeid=$(NODE_ID) - --endpoint=unix://plugin/csi-nfs.sock @@ -370,7 +370,7 @@ spec: path: ca-bundle.pem name: openstack-cloud-config optional: true - name: cacert + name: legacy-cacert - name: hosted-kubeconfig secret: defaultMode: 420 diff --git a/assets/overlays/openstack-manila/generated/hypershift/node.yaml b/assets/overlays/openstack-manila/generated/hypershift/node.yaml index 4d75663e0..ecaa6a919 100644 --- a/assets/overlays/openstack-manila/generated/hypershift/node.yaml +++ b/assets/overlays/openstack-manila/generated/hypershift/node.yaml @@ -86,12 +86,12 @@ spec: name: plugin-dir - mountPath: /var/lib/kubelet/plugins/csi-nfsplugin name: fwd-plugin-dir - - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config - name: cacert - mountPath: /etc/selinux name: etc-selinux - mountPath: /sys/fs name: sys-fs + - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config + name: legacy-cacert - args: - --csi-address=/csi/csi.sock - --http-endpoint=127.0.0.1:10305 @@ -203,7 +203,7 @@ spec: path: ca-bundle.pem name: cloud-provider-config optional: true - name: cacert + name: legacy-cacert updateStrategy: rollingUpdate: maxUnavailable: 10% diff --git a/assets/overlays/openstack-manila/generated/standalone/controller.yaml b/assets/overlays/openstack-manila/generated/standalone/controller.yaml index 4ee32b697..8e36a298b 100644 --- a/assets/overlays/openstack-manila/generated/standalone/controller.yaml +++ b/assets/overlays/openstack-manila/generated/standalone/controller.yaml @@ -111,7 +111,7 @@ spec: - mountPath: /plugin name: socket-dir - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config - name: cacert + name: legacy-cacert - args: - --nodeid=$(NODE_ID) - --endpoint=unix://plugin/csi-nfs.sock @@ -315,4 +315,4 @@ spec: path: ca-bundle.pem name: cloud-provider-config optional: true - name: cacert + name: legacy-cacert diff --git a/assets/overlays/openstack-manila/generated/standalone/node.yaml b/assets/overlays/openstack-manila/generated/standalone/node.yaml index 4d75663e0..ecaa6a919 100644 --- a/assets/overlays/openstack-manila/generated/standalone/node.yaml +++ b/assets/overlays/openstack-manila/generated/standalone/node.yaml @@ -86,12 +86,12 @@ spec: name: plugin-dir - mountPath: /var/lib/kubelet/plugins/csi-nfsplugin name: fwd-plugin-dir - - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config - name: cacert - mountPath: /etc/selinux name: etc-selinux - mountPath: /sys/fs name: sys-fs + - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config + name: legacy-cacert - args: - --csi-address=/csi/csi.sock - --http-endpoint=127.0.0.1:10305 @@ -203,7 +203,7 @@ spec: path: ca-bundle.pem name: cloud-provider-config optional: true - name: cacert + name: legacy-cacert updateStrategy: rollingUpdate: maxUnavailable: 10% diff --git a/assets/overlays/openstack-manila/patches/controller_add_driver.yaml b/assets/overlays/openstack-manila/patches/controller_add_driver.yaml index 0b30487ab..8e6993dba 100644 --- a/assets/overlays/openstack-manila/patches/controller_add_driver.yaml +++ b/assets/overlays/openstack-manila/patches/controller_add_driver.yaml @@ -80,7 +80,7 @@ spec: volumeMounts: - name: socket-dir mountPath: /plugin - - name: cacert + - name: legacy-cacert mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config resources: requests: @@ -111,12 +111,7 @@ spec: volumes: - name: socket-dir emptyDir: {} - - name: cacert - # If present, extract ca-bundle.pem to - # /etc/kubernetes/static-pod-resources/configmaps/cloud-config - # Let the pod start when the ConfigMap does not exist or the certificate - # is not preset there. The certificate file will be created once the - # ConfigMap is created / the cerificate is added to it. + - name: legacy-cacert configMap: name: cloud-provider-config items: diff --git a/assets/overlays/openstack-manila/patches/controller_rename_config_map.yaml b/assets/overlays/openstack-manila/patches/controller_rename_config_map.yaml index aaaca5043..ec93740c4 100644 --- a/assets/overlays/openstack-manila/patches/controller_rename_config_map.yaml +++ b/assets/overlays/openstack-manila/patches/controller_rename_config_map.yaml @@ -2,10 +2,10 @@ spec: template: spec: volumes: - - configMap: - items: - - key: ca-bundle.pem - path: ca-bundle.pem - name: openstack-cloud-config - optional: true - name: cacert + - name: legacy-cacert + configMap: + items: + - key: ca-bundle.pem + path: ca-bundle.pem + name: openstack-cloud-config + optional: true diff --git a/assets/overlays/openstack-manila/patches/node_add_driver.yaml b/assets/overlays/openstack-manila/patches/node_add_driver.yaml index d131b6c9c..bbee2f659 100644 --- a/assets/overlays/openstack-manila/patches/node_add_driver.yaml +++ b/assets/overlays/openstack-manila/patches/node_add_driver.yaml @@ -63,12 +63,13 @@ spec: mountPath: /var/lib/kubelet/plugins/manila.csi.openstack.org - name: fwd-plugin-dir mountPath: /var/lib/kubelet/plugins/csi-nfsplugin - - name: cacert - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config - name: etc-selinux mountPath: /etc/selinux - name: sys-fs mountPath: /sys/fs + # credentials and configuration + - name: legacy-cacert + mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config ports: - name: healthz containerPort: 10305 @@ -100,17 +101,6 @@ spec: hostPath: path: /var/lib/kubelet/plugins/csi-nfsplugin type: DirectoryOrCreate - - name: cacert - # Extract ca-bundle.pem to /etc/kubernetes/static-pod-resources/configmaps/cloud-config if present. - # Let the pod start when the ConfigMap does not exist or the certificate - # is not preset there. The certificate file will be created once the - # ConfigMap is created / the cerificate is added to it. - configMap: - name: cloud-provider-config - items: - - key: ca-bundle.pem - path: ca-bundle.pem - optional: true - name: etc-selinux hostPath: path: /etc/selinux @@ -119,3 +109,10 @@ spec: hostPath: path: /sys/fs type: Directory + - name: legacy-cacert + configMap: + name: cloud-provider-config + items: + - key: ca-bundle.pem + path: ca-bundle.pem + optional: true From 6e8ff32444fd15d223421cdc9842f30d1c9176e1 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 13 Mar 2025 11:09:17 +0000 Subject: [PATCH 8/9] openstack-manila: Consume CA cert from credentials secret (assets) Again, do what we already did for openstack-cinder but for openstack-manila. Like the openstack-cinder change, we continue to allow consuming from the old location to ease upgrades. It's worth highlighting that this is a nice little step towards having the Manila CSI driver and controller source their credentials from a 'clouds.yaml' rather than a 'cloud.conf' file, which would let us remove a lot of logic currently found in the operator. Completing that effort is a job best left to another day though so a TODO is included for now. Signed-off-by: Stephen Finucane --- .../generated/hypershift/controller.yaml | 8 ++++++++ .../openstack-manila/generated/hypershift/node.yaml | 8 ++++++++ .../generated/standalone/controller.yaml | 8 ++++++++ .../openstack-manila/generated/standalone/node.yaml | 8 ++++++++ .../patches/controller_add_driver.yaml | 12 ++++++++++++ .../patches/controller_rename_config_map.yaml | 3 ++- .../openstack-manila/patches/node_add_driver.yaml | 12 ++++++++++++ 7 files changed, 58 insertions(+), 1 deletion(-) diff --git a/assets/overlays/openstack-manila/generated/hypershift/controller.yaml b/assets/overlays/openstack-manila/generated/hypershift/controller.yaml index 7908665de..ba0cc0e3e 100644 --- a/assets/overlays/openstack-manila/generated/hypershift/controller.yaml +++ b/assets/overlays/openstack-manila/generated/hypershift/controller.yaml @@ -144,6 +144,8 @@ spec: volumeMounts: - mountPath: /plugin name: socket-dir + - mountPath: /etc/openstack + name: cloud-credentials - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config name: legacy-cacert - args: @@ -364,6 +366,12 @@ spec: - name: metrics-serving-cert secret: secretName: manila-csi-driver-controller-metrics-serving-cert + - name: cloud-credentials + secret: + items: + - key: cacert + path: ca.crt + secretName: manila-cloud-credentials - configMap: items: - key: ca-bundle.pem diff --git a/assets/overlays/openstack-manila/generated/hypershift/node.yaml b/assets/overlays/openstack-manila/generated/hypershift/node.yaml index ecaa6a919..d09eb5fdb 100644 --- a/assets/overlays/openstack-manila/generated/hypershift/node.yaml +++ b/assets/overlays/openstack-manila/generated/hypershift/node.yaml @@ -90,6 +90,8 @@ spec: name: etc-selinux - mountPath: /sys/fs name: sys-fs + - mountPath: /etc/openstack + name: cloud-credentials - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config name: legacy-cacert - args: @@ -197,6 +199,12 @@ spec: path: /var/lib/kubelet/plugins/csi-nfsplugin type: DirectoryOrCreate name: fwd-plugin-dir + - name: cloud-credentials + secret: + items: + - key: cacert + path: ca.crt + secretName: manila-cloud-credentials - configMap: items: - key: ca-bundle.pem diff --git a/assets/overlays/openstack-manila/generated/standalone/controller.yaml b/assets/overlays/openstack-manila/generated/standalone/controller.yaml index 8e36a298b..68fe9477e 100644 --- a/assets/overlays/openstack-manila/generated/standalone/controller.yaml +++ b/assets/overlays/openstack-manila/generated/standalone/controller.yaml @@ -110,6 +110,8 @@ spec: volumeMounts: - mountPath: /plugin name: socket-dir + - mountPath: /etc/openstack + name: cloud-credentials - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config name: legacy-cacert - args: @@ -309,6 +311,12 @@ spec: - name: metrics-serving-cert secret: secretName: manila-csi-driver-controller-metrics-serving-cert + - name: cloud-credentials + secret: + items: + - key: cacert + path: ca.crt + secretName: manila-cloud-credentials - configMap: items: - key: ca-bundle.pem diff --git a/assets/overlays/openstack-manila/generated/standalone/node.yaml b/assets/overlays/openstack-manila/generated/standalone/node.yaml index ecaa6a919..d09eb5fdb 100644 --- a/assets/overlays/openstack-manila/generated/standalone/node.yaml +++ b/assets/overlays/openstack-manila/generated/standalone/node.yaml @@ -90,6 +90,8 @@ spec: name: etc-selinux - mountPath: /sys/fs name: sys-fs + - mountPath: /etc/openstack + name: cloud-credentials - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config name: legacy-cacert - args: @@ -197,6 +199,12 @@ spec: path: /var/lib/kubelet/plugins/csi-nfsplugin type: DirectoryOrCreate name: fwd-plugin-dir + - name: cloud-credentials + secret: + items: + - key: cacert + path: ca.crt + secretName: manila-cloud-credentials - configMap: items: - key: ca-bundle.pem diff --git a/assets/overlays/openstack-manila/patches/controller_add_driver.yaml b/assets/overlays/openstack-manila/patches/controller_add_driver.yaml index 8e6993dba..c8746cc16 100644 --- a/assets/overlays/openstack-manila/patches/controller_add_driver.yaml +++ b/assets/overlays/openstack-manila/patches/controller_add_driver.yaml @@ -80,6 +80,8 @@ spec: volumeMounts: - name: socket-dir mountPath: /plugin + - name: cloud-credentials + mountPath: /etc/openstack - name: legacy-cacert mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config resources: @@ -111,7 +113,17 @@ spec: volumes: - name: socket-dir emptyDir: {} + - name: cloud-credentials + secret: + secretName: manila-cloud-credentials + # TODO(stephenfin): We should also mount the clouds.yaml file + # here and start consuming that rather than converting it into + # the cloud.conf format + items: + - key: cacert + path: ca.crt - name: legacy-cacert + # TODO(stephenfin): Remove in 4.22 configMap: name: cloud-provider-config items: diff --git a/assets/overlays/openstack-manila/patches/controller_rename_config_map.yaml b/assets/overlays/openstack-manila/patches/controller_rename_config_map.yaml index ec93740c4..08d655bd7 100644 --- a/assets/overlays/openstack-manila/patches/controller_rename_config_map.yaml +++ b/assets/overlays/openstack-manila/patches/controller_rename_config_map.yaml @@ -2,10 +2,11 @@ spec: template: spec: volumes: + # TODO(stephenfin): Remove in 4.22 - name: legacy-cacert configMap: + name: openstack-cloud-config items: - key: ca-bundle.pem path: ca-bundle.pem - name: openstack-cloud-config optional: true diff --git a/assets/overlays/openstack-manila/patches/node_add_driver.yaml b/assets/overlays/openstack-manila/patches/node_add_driver.yaml index bbee2f659..a81c97fee 100644 --- a/assets/overlays/openstack-manila/patches/node_add_driver.yaml +++ b/assets/overlays/openstack-manila/patches/node_add_driver.yaml @@ -68,6 +68,8 @@ spec: - name: sys-fs mountPath: /sys/fs # credentials and configuration + - name: cloud-credentials + mountPath: /etc/openstack - name: legacy-cacert mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config ports: @@ -109,6 +111,16 @@ spec: hostPath: path: /sys/fs type: Directory + - name: cloud-credentials + secret: + secretName: manila-cloud-credentials + # TODO(stephenfin): We should also mount the clouds.yaml file + # here and start consuming that rather than converting it into + # the cloud.conf format + items: + - key: cacert + path: ca.crt + # TODO(stephenfin): Remove in 4.22 - name: legacy-cacert configMap: name: cloud-provider-config From e23fecd36e41f94c64bde1b555da366a0afffc8f Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 19 Feb 2025 17:13:56 +0000 Subject: [PATCH 9/9] openstack-manila: Consume CA cert from credentials secret (controller) Do what we previously did for the openstack-cinder controller but for the openstack-manila controller. In effect, we're really just reflecting the changes made in cluster-storage-operator in [1]. However, we do need to add some logic to detect where we are consuming our CA cert from so that we can match forthcoming changes to our assets. 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://pkg.go.dev/io/ioutil#ReadFile [3] https://pkg.go.dev/os#IsNotExist Signed-off-by: Stephen Finucane --- pkg/openstack-manila/client/openstack.go | 11 ++++-- pkg/openstack-manila/secret/secretsync.go | 41 ++++++++++++++++------- pkg/openstack-manila/util/const.go | 3 +- 3 files changed, 40 insertions(+), 15 deletions(-) diff --git a/pkg/openstack-manila/client/openstack.go b/pkg/openstack-manila/client/openstack.go index 37b0dda3f..fa19905ac 100644 --- a/pkg/openstack-manila/client/openstack.go +++ b/pkg/openstack-manila/client/openstack.go @@ -4,6 +4,7 @@ import ( "context" "crypto/tls" "crypto/x509" + "errors" "fmt" "net/http" "os" @@ -58,7 +59,7 @@ func (o *openStackClient) GetShareTypes() ([]sharetypes.ShareType, error) { provider.UserAgent = ua cert, err := getCloudProviderCert() - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, os.ErrNotExist) { return nil, fmt.Errorf("failed to get cloud provider CA certificate: %w", err) } @@ -120,5 +121,11 @@ func getCloudFromFile(filename string) (*clientconfig.Cloud, error) { } func getCloudProviderCert() ([]byte, error) { - return os.ReadFile(util.CertFile) + data, err := os.ReadFile(util.CertFile) + if err == nil || !errors.Is(err, os.ErrNotExist) { + return data, err + } + + // legacy path; remove in 4.22 + return os.ReadFile(util.LegacyCertFile) } diff --git a/pkg/openstack-manila/secret/secretsync.go b/pkg/openstack-manila/secret/secretsync.go index 20c1a907b..450ef1e6f 100644 --- a/pkg/openstack-manila/secret/secretsync.go +++ b/pkg/openstack-manila/secret/secretsync.go @@ -35,9 +35,10 @@ type SecretSyncController struct { const ( // Name of key with clouds.yaml in Secret provided by cloud-credentials-operator. cloudSecretKey = "clouds.yaml" - // Name of OpenStack in clouds.yaml - // Canonical path for custom ca certificates - cacertPath = "/etc/kubernetes/static-pod-resources/configmaps/cloud-config/ca-bundle.pem" + // Path for custom CA certificates when provided by cloud-credentials-operator. + defaultCACertPath = "/etc/openstack/ca.crt" + // Path for custom CA certificates when provided by Installer (legacy path). + legacyCACertPath = "/etc/kubernetes/static-pod-resources/configmaps/cloud-config/ca-bundle.pem" ) func NewSecretSyncController( @@ -115,11 +116,29 @@ func (c *SecretSyncController) translateSecret(cloudSecret *v1.Secret) (*v1.Secr data := cloudToConf(cloud) - // In the hypershift secret, the clouds.yaml field might not have the cacert defined. The content of the certificate - // is defined in the ca-bundle.pem field instead. - _, ok = cloudSecret.Data["ca-bundle.pem"] - if ok { - data["os-certAuthorityPath"] = []byte(cacertPath) + // Determine where our CA cert is stored. + // TODO(stephenfin): Remove most of this in 4.22 + var caCertPath string + if _, ok := cloudSecret.Data["cacert"]; ok { + // Option A: We have the CA cert in our credentials under the 'cacert' + // key which indicates a recent (>= 4.19) version of + // cluster-credential-operator (CCO) or hypershift + caCertPath = defaultCACertPath + } else if _, ok = cloudSecret.Data["ca-bundle.pem"]; ok { + // Option B: We have the CA cert in our credentials but under the + // 'ca-bundle.pem' key, which indicates an older (< 4.19) version of + // hypershift + caCertPath = legacyCACertPath + } else if cloud.CACertFile != "" { + // Option C: We have a non-empty 'cafile' field in our clouds.yaml. + // This means our root credential secret has this defined yet + // cloud-credential-operator (CCO) didn't populate the 'cacert' key of + // the secret. This indicates an older (< 4.19) version of CCO. + caCertPath = legacyCACertPath + } + + if caCertPath != "" { + data["os-certAuthorityPath"] = []byte(caCertPath) } secret := v1.Secret{ @@ -182,10 +201,8 @@ func cloudToConf(cloud clientconfig.Cloud) map[string][]byte { data["os-userDomainName"] = []byte(cloud.AuthInfo.UserDomainName) data["os-domainName"] = []byte(cloud.AuthInfo.UserDomainName) } - if cloud.CACertFile != "" { - // Replace the original cert authority path from clouds.yaml with the canonical one - data["os-certAuthorityPath"] = []byte(cacertPath) - } + + // We don't set os-certAuthorityPath here as it's handled separately. return data } diff --git a/pkg/openstack-manila/util/const.go b/pkg/openstack-manila/util/const.go index bdd82f42d..335664b5e 100644 --- a/pkg/openstack-manila/util/const.go +++ b/pkg/openstack-manila/util/const.go @@ -11,7 +11,8 @@ const ( // OpenStack config file name (as present in the operator Deployment) CloudConfigFilename = "/etc/openstack/clouds.yaml" - CertFile = "/etc/openstack-ca/ca-bundle.pem" + CertFile = "/etc/openstack/ca.crt" + LegacyCertFile = "/etc/openstack-ca/ca-bundle.pem" // Name of cloud in secret provided by cloud-credentials-operator CloudName = "openstack"