From ab5e5d86775b3ce483beff469869aef2a60801e3 Mon Sep 17 00:00:00 2001 From: Michael Munch Date: Thu, 20 Aug 2026 19:51:35 +0200 Subject: [PATCH] fix(spiffe-oidc-discovery-provider): run under restricted PSA/SCC on OpenShift (#920) * fix(spiffe-oidc-discovery-provider): run under restricted PSA/SCC on OpenShift The OIDC discovery provider does not require any elevated privileges: it runs fine under OpenShift's built-in restricted-v2 SCC (non-root, no privilege escalation, all capabilities dropped, RuntimeDefault seccomp, read-only root filesystem) and mounts only restricted-compatible volumes (csi, configMap, emptyDir, secret, projected, downwardAPI). Despite this, on OpenShift the chart: - downgraded the spire-server namespace from restricted to privileged PSA whenever the OIDC provider was enabled, and - created a fully privileged SecurityContextConstraints (host network/IPC/ PID, privileged container, hostPath, arbitrary seccomp, RunAsAny) bound to the provider's ServiceAccount. Both contradict the chart's own Namespaces documentation, which specifies restricted PSA for spire-server, and violate least privilege for an internet-facing OIDC endpoint. Remove the privileged PSA override for the OIDC provider (spire-server stays restricted; the scc.podSecurityLabelSync=false label is retained) and drop the privileged SCC so the provider falls through to restricted-v2. With spire-server enforcing restricted PSA, the inline-CSI PodSecurity check reads the cluster-scoped CSIDriver's security.openshift.io/csi-ephemeral-volume-profile label. If the CSIDriver is not committed before the spire-server StatefulSet (which mounts the inline upstream.csi.spiffe.io volume) is admitted, the profile defaults to privileged and admission is denied. Under ArgoCD the CSIDriver and the server StatefulSet can land in the same sync wave, racing admission. Annotate the CSIDriver with argocd.argoproj.io/sync-wave: "-1" (OpenShift only) so it is applied before the default-wave server workloads; the annotation is inert for plain helm installs. Signed-off-by: Michael Munch * :sparkles: make CSIDriver sync-wave ordering configurable Add syncWave and csiDriverAnnotations values to the spiffe-csi-driver chart so the OpenShift argocd.argoproj.io/sync-wave annotation number can be overridden (e.g. when the chart is nested) and arbitrary annotations can be applied to the CSIDriver. Signed-off-by: Michael Munch --------- Signed-off-by: Michael Munch Co-authored-by: kfox1111 --- .../templates/_spire-server-namespace.yaml | 3 - .../spire/charts/spiffe-csi-driver/README.md | 102 +++++++++--------- .../templates/spiffe-csi-driver.yaml | 9 ++ .../charts/spiffe-csi-driver/values.yaml | 6 ++ .../scc-spire-oidc-discovery-provider.yaml | 42 -------- tests/unit/spire_test.go | 37 +++++++ 6 files changed, 104 insertions(+), 95 deletions(-) delete mode 100644 charts/spire/charts/spiffe-oidc-discovery-provider/templates/scc-spire-oidc-discovery-provider.yaml diff --git a/charts/spire-lib/templates/_spire-server-namespace.yaml b/charts/spire-lib/templates/_spire-server-namespace.yaml index 37a959b..f933d38 100644 --- a/charts/spire-lib/templates/_spire-server-namespace.yaml +++ b/charts/spire-lib/templates/_spire-server-namespace.yaml @@ -10,9 +10,6 @@ {{- $labels = mergeOverwrite $labels (include "spire-lib.namespace.default_server_labels" . | fromYaml) }} {{- if (dig "openshift" false .Values.global) }} {{- $_ := set $labels "security.openshift.io/scc.podSecurityLabelSync" "false" }} -{{- if (index .Values "spiffe-oidc-discovery-provider").enabled }} -{{- $_ := set $labels "pod-security.kubernetes.io/enforce" "privileged" }} -{{- end }} {{- end }} {{- end }} {{- $labels = mergeOverwrite $labels .Values.global.spire.namespaces.server.labels }} diff --git a/charts/spire/charts/spiffe-csi-driver/README.md b/charts/spire/charts/spiffe-csi-driver/README.md index a43873f..462826d 100644 --- a/charts/spire/charts/spiffe-csi-driver/README.md +++ b/charts/spire/charts/spiffe-csi-driver/README.md @@ -25,54 +25,56 @@ A Helm chart to install the SPIFFE CSI driver. ### SPIFFE CSI Driver Chart parameters -| Name | Description | Value | -| --------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------- | -| `pluginName` | Set the csi driver name deployed to Kubernetes. | `csi.spiffe.io` | -| `image.registry` | The OCI registry to pull the image from | `ghcr.io` | -| `image.repository` | The repository within the registry | `spiffe/spiffe-csi-driver` | -| `image.pullPolicy` | The image pull policy | `IfNotPresent` | -| `image.tag` | Overrides the image tag whose default is the chart appVersion | `""` | -| `resources` | Resource requests and limits for spiffe-csi-driver and its initContainers | `{}` | -| `extraEnvVars` | Extra environment variables to be added to the spiffe-csi-driver container | `[]` | -| `healthChecks.port` | The healthcheck port for spiffe-csi-driver | `9809` | -| `updateStrategy.type` | The update strategy to use to replace existing DaemonSet pods with new pods. Can be RollingUpdate or OnDelete. | `RollingUpdate` | -| `updateStrategy.rollingUpdate.maxUnavailable` | Max unavailable pods during update. Can be a number or a percentage. | `1` | -| `livenessProbe.initialDelaySeconds` | Initial delay seconds for livenessProbe | `5` | -| `livenessProbe.timeoutSeconds` | Timeout value in seconds for livenessProbe | `5` | -| `imagePullSecrets` | Image pull secret details for spiffe-csi-driver | `[]` | -| `nameOverride` | Name override for spiffe-csi-driver | `""` | -| `namespaceOverride` | Namespace to install spiffe-csi-driver | `""` | -| `serverNamespaceOverride` | Override the namespace that the spire-server is installed into | `""` | -| `validatingAdmissionPolicy.enabled` | When set to auto, the validatingAdmissionPolicy will be enabled when the pluginName == "upstream.csi.spiffe.io" and k8s >= 1.30.0. Valid options are [auto, true, false] | `auto` | -| `fullnameOverride` | Full name override for spiffe-csi-driver | `""` | -| `csiDriverLabels` | Labels to apply to the CSIDriver | `{}` | -| `initContainers` | Init Containers to apply to the CSI Driver DaemonSet | `[]` | -| `serviceAccount.create` | Specifies whether a service account should be created | `true` | -| `serviceAccount.annotations` | Annotations to add to the service account | `{}` | -| `serviceAccount.name` | The name of the service account to use. If not set and create is true, a name is generated. | `""` | -| `podAnnotations` | Pod annotations for spiffe-csi-driver | `{}` | -| `podLabels` | Labels to add to pods | `{}` | -| `podSecurityContext` | Security context for CSI driver pods | `{}` | -| `securityContext` | Security context for CSI driver containers | `{}` | -| `hostNetwork` | Enable hostNetwork for the DaemonSet | `false` | -| `nodeSelector` | Node selector for CSI driver pods | `{}` | -| `tolerations` | Tolerations for CSI driver pods | `[]` | -| `affinity` | Node affinity | `{}` | -| `nodeDriverRegistrar.image.registry` | The OCI registry to pull the image from | `registry.k8s.io` | -| `nodeDriverRegistrar.image.repository` | The repository within the registry | `sig-storage/csi-node-driver-registrar` | -| `nodeDriverRegistrar.image.pullPolicy` | The image pull policy | `IfNotPresent` | -| `nodeDriverRegistrar.image.tag` | Overrides the image tag | `v2.15.0` | -| `nodeDriverRegistrar.extraEnvVars` | Extra environment variables to be added to the nodeDriverRegistrar container | `[]` | -| `agentSocketPath` | The unix socket path to the spire-agent | `/run/spire/agent-sockets/spire-agent.sock` | -| `kubeletPath` | Path to kubelet file | `/var/lib/kubelet` | -| `priorityClassName` | Priority class assigned to daemonset pods. Can be auto set with global.recommendations.priorityClassName. | `""` | -| `restrictedScc.enabled` | Enables the creation of a SecurityContextConstraint based on the restricted SCC with CSI volume support | `false` | -| `restrictedScc.name` | Set the name of the restricted SCC with CSI support | `""` | -| `restrictedScc.version` | Version of the restricted SCC | `2` | -| `selinux.enabled` | Enable selinux support | `false` | -| `selinux.context` | Which selinux context to use | `container_file_t` | -| `selinux.image.registry` | The OCI registry to pull the image from | `registry.access.redhat.com` | -| `selinux.image.repository` | The repository within the registry | `ubi10/ubi-minimal` | -| `selinux.image.pullPolicy` | The image pull policy | `IfNotPresent` | -| `selinux.image.tag` | Overrides the image tag whose default is the chart appVersion | `10.1-1776834797` | +| Name | Description | Value | +| --------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------- | +| `pluginName` | Set the csi driver name deployed to Kubernetes. | `csi.spiffe.io` | +| `image.registry` | The OCI registry to pull the image from | `ghcr.io` | +| `image.repository` | The repository within the registry | `spiffe/spiffe-csi-driver` | +| `image.pullPolicy` | The image pull policy | `IfNotPresent` | +| `image.tag` | Overrides the image tag whose default is the chart appVersion | `""` | +| `resources` | Resource requests and limits for spiffe-csi-driver and its initContainers | `{}` | +| `extraEnvVars` | Extra environment variables to be added to the spiffe-csi-driver container | `[]` | +| `healthChecks.port` | The healthcheck port for spiffe-csi-driver | `9809` | +| `updateStrategy.type` | The update strategy to use to replace existing DaemonSet pods with new pods. Can be RollingUpdate or OnDelete. | `RollingUpdate` | +| `updateStrategy.rollingUpdate.maxUnavailable` | Max unavailable pods during update. Can be a number or a percentage. | `1` | +| `livenessProbe.initialDelaySeconds` | Initial delay seconds for livenessProbe | `5` | +| `livenessProbe.timeoutSeconds` | Timeout value in seconds for livenessProbe | `5` | +| `imagePullSecrets` | Image pull secret details for spiffe-csi-driver | `[]` | +| `nameOverride` | Name override for spiffe-csi-driver | `""` | +| `namespaceOverride` | Namespace to install spiffe-csi-driver | `""` | +| `serverNamespaceOverride` | Override the namespace that the spire-server is installed into | `""` | +| `validatingAdmissionPolicy.enabled` | When set to auto, the validatingAdmissionPolicy will be enabled when the pluginName == "upstream.csi.spiffe.io" and k8s >= 1.30.0. Valid options are [auto, true, false] | `auto` | +| `fullnameOverride` | Full name override for spiffe-csi-driver | `""` | +| `csiDriverLabels` | Labels to apply to the CSIDriver | `{}` | +| `csiDriverAnnotations` | Annotations to apply to the CSIDriver | `{}` | +| `syncWave` | The argocd.argoproj.io/sync-wave value applied to the CSIDriver on OpenShift, ensuring the CSI driver reconciles before workloads that depend on its csi-ephemeral-volume-profile label | `-1` | +| `initContainers` | Init Containers to apply to the CSI Driver DaemonSet | `[]` | +| `serviceAccount.create` | Specifies whether a service account should be created | `true` | +| `serviceAccount.annotations` | Annotations to add to the service account | `{}` | +| `serviceAccount.name` | The name of the service account to use. If not set and create is true, a name is generated. | `""` | +| `podAnnotations` | Pod annotations for spiffe-csi-driver | `{}` | +| `podLabels` | Labels to add to pods | `{}` | +| `podSecurityContext` | Security context for CSI driver pods | `{}` | +| `securityContext` | Security context for CSI driver containers | `{}` | +| `hostNetwork` | Enable hostNetwork for the DaemonSet | `false` | +| `nodeSelector` | Node selector for CSI driver pods | `{}` | +| `tolerations` | Tolerations for CSI driver pods | `[]` | +| `affinity` | Node affinity | `{}` | +| `nodeDriverRegistrar.image.registry` | The OCI registry to pull the image from | `registry.k8s.io` | +| `nodeDriverRegistrar.image.repository` | The repository within the registry | `sig-storage/csi-node-driver-registrar` | +| `nodeDriverRegistrar.image.pullPolicy` | The image pull policy | `IfNotPresent` | +| `nodeDriverRegistrar.image.tag` | Overrides the image tag | `v2.15.0` | +| `nodeDriverRegistrar.extraEnvVars` | Extra environment variables to be added to the nodeDriverRegistrar container | `[]` | +| `agentSocketPath` | The unix socket path to the spire-agent | `/run/spire/agent-sockets/spire-agent.sock` | +| `kubeletPath` | Path to kubelet file | `/var/lib/kubelet` | +| `priorityClassName` | Priority class assigned to daemonset pods. Can be auto set with global.recommendations.priorityClassName. | `""` | +| `restrictedScc.enabled` | Enables the creation of a SecurityContextConstraint based on the restricted SCC with CSI volume support | `false` | +| `restrictedScc.name` | Set the name of the restricted SCC with CSI support | `""` | +| `restrictedScc.version` | Version of the restricted SCC | `2` | +| `selinux.enabled` | Enable selinux support | `false` | +| `selinux.context` | Which selinux context to use | `container_file_t` | +| `selinux.image.registry` | The OCI registry to pull the image from | `registry.access.redhat.com` | +| `selinux.image.repository` | The repository within the registry | `ubi10/ubi-minimal` | +| `selinux.image.pullPolicy` | The image pull policy | `IfNotPresent` | +| `selinux.image.tag` | Overrides the image tag whose default is the chart appVersion | `10.1-1776834797` | diff --git a/charts/spire/charts/spiffe-csi-driver/templates/spiffe-csi-driver.yaml b/charts/spire/charts/spiffe-csi-driver/templates/spiffe-csi-driver.yaml index cd17fdd..3027602 100644 --- a/charts/spire/charts/spiffe-csi-driver/templates/spiffe-csi-driver.yaml +++ b/charts/spire/charts/spiffe-csi-driver/templates/spiffe-csi-driver.yaml @@ -3,10 +3,19 @@ {{- $_ := set $labels "security.openshift.io/csi-ephemeral-volume-profile" "restricted" }} {{- end }} {{- $labels = mergeOverwrite $labels .Values.csiDriverLabels }} +{{- $annotations := dict }} +{{- if (dig "openshift" false .Values.global) }} +{{- $_ := set $annotations "argocd.argoproj.io/sync-wave" (toString .Values.syncWave) }} +{{- end }} +{{- $annotations = mergeOverwrite $annotations .Values.csiDriverAnnotations }} apiVersion: storage.k8s.io/v1 kind: CSIDriver metadata: name: {{ .Values.pluginName | quote }} + {{- with $annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} {{- with $labels }} labels: {{- toYaml . | nindent 4 }} diff --git a/charts/spire/charts/spiffe-csi-driver/values.yaml b/charts/spire/charts/spiffe-csi-driver/values.yaml index 56a4a2f..d21d4c8 100644 --- a/charts/spire/charts/spiffe-csi-driver/values.yaml +++ b/charts/spire/charts/spiffe-csi-driver/values.yaml @@ -76,6 +76,12 @@ fullnameOverride: "" ## @param csiDriverLabels Labels to apply to the CSIDriver csiDriverLabels: {} +## @param csiDriverAnnotations Annotations to apply to the CSIDriver +csiDriverAnnotations: {} + +## @param syncWave The argocd.argoproj.io/sync-wave value applied to the CSIDriver on OpenShift, ensuring the CSI driver reconciles before workloads that depend on its csi-ephemeral-volume-profile label +syncWave: -1 + ## @param initContainers Init Containers to apply to the CSI Driver DaemonSet initContainers: [] diff --git a/charts/spire/charts/spiffe-oidc-discovery-provider/templates/scc-spire-oidc-discovery-provider.yaml b/charts/spire/charts/spiffe-oidc-discovery-provider/templates/scc-spire-oidc-discovery-provider.yaml deleted file mode 100644 index 6916687..0000000 --- a/charts/spire/charts/spiffe-oidc-discovery-provider/templates/scc-spire-oidc-discovery-provider.yaml +++ /dev/null @@ -1,42 +0,0 @@ -{{- if eq (.Values.global.openshift | toString) "true" }} -apiVersion: security.openshift.io/v1 -kind: SecurityContextConstraints -metadata: - name: {{ include "spiffe-oidc-discovery-provider.fullname" . }} -readOnlyRootFilesystem: true -runAsUser: - type: RunAsAny -seLinuxContext: - type: RunAsAny -supplementalGroups: - type: RunAsAny -users: - - system:serviceaccount:{{ include "spiffe-oidc-discovery-provider.namespace" . }}:{{ include "spiffe-oidc-discovery-provider.serviceAccountName" . }} - - system:serviceaccount:{{ include "spiffe-oidc-discovery-provider.namespace" . }}:{{ include "spiffe-oidc-discovery-provider.serviceAccountName" . }}-pre-delete -volumes: - - configMap - - csi - - downwardAPI - - emptyDir - - ephemeral - - hostPath - - projected - - secret -allowedCapabilities: null -allowHostDirVolumePlugin: true -allowHostIPC: true -allowHostNetwork: true -allowHostPID: true -allowHostPorts: true -allowPrivilegeEscalation: true -allowPrivilegedContainer: true -defaultAddCapabilities: null -fsGroup: - type: RunAsAny -groups: [] -priority: null -requiredDropCapabilities: null -seccompProfiles: - - '*' - -{{ end }} diff --git a/tests/unit/spire_test.go b/tests/unit/spire_test.go index c383e7a..e446a24 100644 --- a/tests/unit/spire_test.go +++ b/tests/unit/spire_test.go @@ -349,6 +349,43 @@ spire-server: Expect(objs[serverTmpl]).Should(ContainSubstring("init-jwt-svid-exec")) }) }) + Describe("spiffe-csi-driver.syncWave", func() { + csiTmpl := "spire/charts/spiffe-csi-driver/templates/spiffe-csi-driver.yaml" + It("renders the default sync-wave annotation on OpenShift", func() { + objs, err := ValueStringRender(chart, ` +global: + openshift: true +`) + Expect(err).Should(Succeed()) + Expect(objs[csiTmpl]).Should(ContainSubstring(`argocd.argoproj.io/sync-wave: "-1"`)) + }) + It("allows overriding the sync-wave number", func() { + objs, err := ValueStringRender(chart, ` +global: + openshift: true +spiffe-csi-driver: + syncWave: -2 +`) + Expect(err).Should(Succeed()) + Expect(objs[csiTmpl]).Should(ContainSubstring(`argocd.argoproj.io/sync-wave: "-2"`)) + }) + It("allows overriding the annotation via csiDriverAnnotations", func() { + objs, err := ValueStringRender(chart, ` +global: + openshift: true +spiffe-csi-driver: + csiDriverAnnotations: + argocd.argoproj.io/sync-wave: "-5" +`) + Expect(err).Should(Succeed()) + Expect(objs[csiTmpl]).Should(ContainSubstring(`argocd.argoproj.io/sync-wave: "-5"`)) + }) + It("does not render the sync-wave annotation when not on OpenShift", func() { + objs, err := ValueStringRender(chart, ``) + Expect(err).Should(Succeed()) + Expect(objs[csiTmpl]).ShouldNot(ContainSubstring("argocd.argoproj.io/sync-wave")) + }) + }) Describe("spire-server.externalServerSubject", func() { It("binds the external server's downstream RBAC to a ServiceAccount subject", func() { objs, err := ValueStringRender(chart, `