From 185714ca7ba5ef1a127418eca8fc7801d204b885 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 e34cfbfe1..09eb11206 100644 --- a/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml @@ -52,7 +52,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: @@ -140,16 +140,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/ @@ -432,7 +432,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 983573f3d..37be7cb8e 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/ @@ -369,7 +369,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 872f4a999af9b3e76a00093209ccc41c5940639f 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 09eb11206..6b3242e81 100644 --- a/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml @@ -144,6 +144,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 37be7cb8e..bf89edefd 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 71bb7a5476f3956b3f7ad7e53807cbcd44587fcf 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 6b3242e81..4b26fa815 100644 --- a/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml @@ -148,7 +148,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 bf89edefd..778804d8b 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 f19a2be9d1334e95df22ccd52b25817f17c119ce 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 4b26fa815..586143174 100644 --- a/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml @@ -52,7 +52,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: @@ -142,15 +142,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/ @@ -451,7 +451,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 778804d8b..2bd759763 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/ @@ -388,4 +388,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 ae7353cbc152decf170cc4457164e5191de0af6f 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 586143174..7166c63a1 100644 --- a/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/hypershift/controller.yaml @@ -434,11 +434,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 2bd759763..3d6f0df7d 100644 --- a/assets/overlays/openstack-cinder/generated/standalone/controller.yaml +++ b/assets/overlays/openstack-cinder/generated/standalone/controller.yaml @@ -371,11 +371,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 537a976648ab7a25e4a30d5ce5f50a5ca8382b14 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 ca6dadbead14613231c7bd8c83bab563fcfee083 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 fed3f177d..ab52a3706 100644 --- a/assets/overlays/openstack-manila/generated/hypershift/controller.yaml +++ b/assets/overlays/openstack-manila/generated/hypershift/controller.yaml @@ -147,7 +147,7 @@ spec: - mountPath: /plugin name: socket-dir - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config - name: cacert + name: legacy-cacert - mountPath: /etc/openstack name: cloud-credentials readOnly: true @@ -389,7 +389,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 d4e48ab14..d29938ede 100644 --- a/assets/overlays/openstack-manila/generated/hypershift/node.yaml +++ b/assets/overlays/openstack-manila/generated/hypershift/node.yaml @@ -89,12 +89,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 @@ -212,7 +212,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 c6c82596c..993216bd6 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 - mountPath: /etc/openstack name: cloud-credentials readOnly: true @@ -332,4 +332,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 d4e48ab14..d29938ede 100644 --- a/assets/overlays/openstack-manila/generated/standalone/node.yaml +++ b/assets/overlays/openstack-manila/generated/standalone/node.yaml @@ -89,12 +89,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 @@ -212,7 +212,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 e235db34b..18ed2ba42 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 - name: cloud-credentials mountPath: /etc/openstack @@ -122,12 +122,7 @@ spec: path: clouds.yaml - 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 ecff0460f..5c86f04b0 100644 --- a/assets/overlays/openstack-manila/patches/node_add_driver.yaml +++ b/assets/overlays/openstack-manila/patches/node_add_driver.yaml @@ -66,12 +66,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 @@ -109,17 +110,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 @@ -128,3 +118,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 48052d2e4563e55882e15b1ee8768910dd34be46 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 | 3 ++- .../openstack-manila/generated/hypershift/node.yaml | 3 +++ .../generated/standalone/controller.yaml | 3 ++- .../openstack-manila/generated/standalone/node.yaml | 3 +++ .../patches/controller_add_driver.yaml | 12 ++++++++++++ .../patches/controller_rename_config_map.yaml | 3 ++- .../openstack-manila/patches/node_add_driver.yaml | 12 ++++++++++++ 7 files changed, 36 insertions(+), 3 deletions(-) diff --git a/assets/overlays/openstack-manila/generated/hypershift/controller.yaml b/assets/overlays/openstack-manila/generated/hypershift/controller.yaml index ab52a3706..5a1b2073b 100644 --- a/assets/overlays/openstack-manila/generated/hypershift/controller.yaml +++ b/assets/overlays/openstack-manila/generated/hypershift/controller.yaml @@ -146,11 +146,12 @@ 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 - mountPath: /etc/openstack name: cloud-credentials - readOnly: true - args: - --nodeid=$(NODE_ID) - --endpoint=unix://plugin/csi-nfs.sock diff --git a/assets/overlays/openstack-manila/generated/hypershift/node.yaml b/assets/overlays/openstack-manila/generated/hypershift/node.yaml index d29938ede..3ce6caaab 100644 --- a/assets/overlays/openstack-manila/generated/hypershift/node.yaml +++ b/assets/overlays/openstack-manila/generated/hypershift/node.yaml @@ -93,6 +93,9 @@ spec: name: etc-selinux - mountPath: /sys/fs name: sys-fs + - mountPath: /etc/openstack + name: cloud-credentials + readOnly: true - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config name: legacy-cacert - args: diff --git a/assets/overlays/openstack-manila/generated/standalone/controller.yaml b/assets/overlays/openstack-manila/generated/standalone/controller.yaml index 993216bd6..ce116be25 100644 --- a/assets/overlays/openstack-manila/generated/standalone/controller.yaml +++ b/assets/overlays/openstack-manila/generated/standalone/controller.yaml @@ -110,11 +110,12 @@ 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 - mountPath: /etc/openstack name: cloud-credentials - readOnly: true - args: - --nodeid=$(NODE_ID) - --endpoint=unix://plugin/csi-nfs.sock diff --git a/assets/overlays/openstack-manila/generated/standalone/node.yaml b/assets/overlays/openstack-manila/generated/standalone/node.yaml index d29938ede..3ce6caaab 100644 --- a/assets/overlays/openstack-manila/generated/standalone/node.yaml +++ b/assets/overlays/openstack-manila/generated/standalone/node.yaml @@ -93,6 +93,9 @@ spec: name: etc-selinux - mountPath: /sys/fs name: sys-fs + - mountPath: /etc/openstack + name: cloud-credentials + readOnly: true - mountPath: /etc/kubernetes/static-pod-resources/configmaps/cloud-config name: legacy-cacert - args: diff --git a/assets/overlays/openstack-manila/patches/controller_add_driver.yaml b/assets/overlays/openstack-manila/patches/controller_add_driver.yaml index 18ed2ba42..9713b2422 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 - name: cloud-credentials @@ -122,7 +124,17 @@ spec: path: clouds.yaml - 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 5c86f04b0..6b45322d8 100644 --- a/assets/overlays/openstack-manila/patches/node_add_driver.yaml +++ b/assets/overlays/openstack-manila/patches/node_add_driver.yaml @@ -71,6 +71,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: @@ -118,6 +120,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 19f1f5ab4b305cf31c92f654a5973ea8a64f4862 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/util/const.go | 3 ++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/pkg/openstack-manila/client/openstack.go b/pkg/openstack-manila/client/openstack.go index 37b0dda3f..3d869b14f 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 5.1 + return os.ReadFile(util.LegacyCertFile) } 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"