feat(chart): make pod and sidecar securityContext configurable

[yugstar]

What

Makes the csi-driver-spiffe DaemonSet's pod- and container-level securityContext configurable through values.yaml, instead of hardcoding it in the template.

New values (each defaults to the value that was previously hardcoded):

Value	Container / scope
podSecurityContext	pod-level
securityContext	cert-manager-csi-driver-spiffe
nodeDriverRegistrarSecurityContext	node-driver-registrar
livenessProbeSecurityContext	liveness-probe

Why

templates/daemonset.yaml hardcodes privileged: true and runAsUser: 0 on the main container (and runAsUser: 0 on both sidecars), with no .Values.* overrides. Operators subject to Pod Security Standards (Baseline/Restricted) or security-scanning policies cannot adjust the posture without forking the chart.

This is the companion to cert-manager/csi-driver#672 — it applies the identical change to the sibling CSI chart, using the same value names and layout so the two charts stay consistent. It also mirrors the convention already used in the main cert-manager chart (securityContext / containerSecurityContext).

What changes by default

Nothing. Every new value defaults to the exact value that was previously hardcoded, so helm template output is semantically identical (verified by parsing the before/after manifests and asserting deep-equality, plus a helm-unittest golden suite). Mounting is unaffected: the pods-mount-dir mount and its mountPropagation: "Bidirectional" are untouched, and the main container keeps privileged: true by default — Kubernetes only permits Bidirectional mount propagation on privileged containers, so dropping it outright would stop the driver pods from starting.

What becomes possible (opt-in)

securityContext:
  runAsUser: 0
  privileged: false
  allowPrivilegeEscalation: false
  readOnlyRootFilesystem: true
  capabilities:
    add: [SYS_ADMIN]
    drop: [ALL]

Testing

• helm template before/after manifests are semantically identical (default render unchanged).
• make verify-helm-unittest — new tests/securitycontext_test.yaml (defaults preserved + opt-in overrides + null-clears-block); full suite 31/31.
• make verify-helm-values, verify-helm-lint, verify-helm-kubeconform, verify-pod-security-standards all pass.
• values.schema.json and README.md regenerated via make generate-helm-schema generate-helm-docs.

Open question

I reused the value names from cert-manager/csi-driver#672 for cross-chart consistency. If maintainers prefer a different layout, happy to adjust — the default render is identical either way.

The csi-driver-spiffe Helm chart now allows configuring the pod- and container-level `securityContext` via the `podSecurityContext`, `securityContext`, `nodeDriverRegistrarSecurityContext` and `livenessProbeSecurityContext` values. Defaults are unchanged.

[cert-manager-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign sgtcodfish for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[cert-manager-prow]

Hi @yugstar. Thanks for your PR.

I'm waiting for a cert-manager member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[wallrj-cyberark]

/ok-to-test

[Copilot]

⚠️ Not ready to approve

securityContext: null is explicitly tested/supported by the template but is rejected by the current values.schema.json type definition.

Pull request overview

This PR makes the csi-driver-spiffe Helm chart’s DaemonSet pod- and container-level securityContext configurable via values.yaml, replacing previously hardcoded security context blocks while keeping default rendered manifests semantically unchanged.

Changes:

• Adds new values for pod and per-container security contexts (podSecurityContext, securityContext, nodeDriverRegistrarSecurityContext, livenessProbeSecurityContext) with defaults matching prior hardcoded settings.
• Updates the DaemonSet template to render security contexts from .Values.* (and omit them when values are unset/null).
• Adds helm-unittest coverage and regenerates schema/docs to reflect new values.

File summaries

File	Description
deploy/charts/csi-driver-spiffe/values.yaml	Introduces new securityContext-related values with defaults matching prior template behavior.
deploy/charts/csi-driver-spiffe/values.schema.json	Adds schema entries for new values (but currently rejects securityContext: null).
deploy/charts/csi-driver-spiffe/tests/securitycontext_test.yaml	Adds tests to ensure defaults are preserved and overrides/null behavior works.
deploy/charts/csi-driver-spiffe/templates/daemonset.yaml	Switches hardcoded securityContext blocks to value-driven rendering via with + toYaml.
deploy/charts/csi-driver-spiffe/README.md	Documents the new values and their defaults.

Copilot's findings

• Files reviewed: 5/5 changed files
• Comments generated: 1

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[yugstar]

Thanks for the review. securityContext: null is actually accepted today — Helm treats an explicit null on an optional value as "unset", so the key is dropped during value coalescing before schema validation. helm template -f with securityContext: null renders fine and omits the block, while genuinely invalid types are still rejected (a string gives at '/securityContext': got string, want object).

I also tried the suggested ["object", "null"]. The schema is generated from values.yaml by helm-tool, and # +docs:type=object,null makes it emit no type at all — which would over-permissively accept invalid values like a bare string. So the current type: object is the stricter/correct option and the null case already works. Happy to revisit if you'd prefer a different approach.

[wallrj-cyberark]

Hi @yugstar — thank you for the work on this. Before reviewing the chart changes in detail, I spent some time empirically testing whether the driver actually functions with privileged: false, which is what the originating issue (cert-manager/csi-driver#583) is asking for. The results are written up here: cert-manager/csi-driver#583 (comment)

The short version: every variant I could construct with privileged: false either fails Kubernetes admission, fails to start, or silently delivers an empty volume to the workload pod. The driver structurally requires mountPropagation: Bidirectional, which Kubernetes only permits on privileged containers, and a Helm values knob exposing privileged cannot change that.

I also looked for legitimate operator use cases for the other knobs in this PR (podSecurityContext, the sidecar fields, per-container runAsUser) and could not find one for this daemonset specifically:

• Pod-level fields: the main container must run as root with privileged: true, so runAsNonRoot, runAsUser, runAsGroup, fsGroup, seccompProfile and appArmorProfile are either ignored by the privileged container (seccomp/apparmor) or would break it (runAsNonRoot). The sidecars already override at container level.
• Sidecar securityContext (nodeDriverRegistrarSecurityContext, livenessProbeSecurityContext): the current hardcoded values are already maximally restrictive (allowPrivilegeEscalation: false, capabilities: drop: ALL, readOnlyRootFilesystem: true, non-root). Configurability can only weaken them.
• Main container securityContext: this is the dangerous one — the only field operators would want to change to satisfy a scanner is privileged: false, which produces the silent-failure mode in the issue write-up.

I think the right outcome is to close this PR and instead address #583 with either (1) documentation in the chart README explaining why privileged: true is structural, so operators can write scanner exemptions, or (2) an architectural change to csi-lib so the driver no longer requires Bidirectional mount propagation. The latter is non-trivial and is not a Helm change.

Happy to be shown wrong if you have an operator scenario in mind that the table in the issue comment does not cover — I would rather find a working unprivileged path than close this PR if one exists.

---

Edit — walking back the secondary point above

I overstated the case when I said I "could not find a legitimate operator use case" for podSecurityContext, the sidecar fields, or per-container runAsUser. After more thought there are real reasons an operator might need them, even while the main container's privileged: true requirement stays put:

• AppArmor profiles apply to privileged containers. AppArmor is an LSM independent of capabilities — the kernel enforces a profile if one is set; the operator just has to write it permissively enough for the privileged operations to work. Hardened distros like Bottlerocket and Talos ship AppArmor policies; until Kubernetes 1.30 promoted appArmorProfile to a first-class SecurityContext field there was no way to apply them via Helm, and trust-manager's #836 merged the same shape for that chart.
• SELinux labels apply to privileged containers too. Multi-tenant clusters mandating specific seLinuxOptions.level values for tenancy isolation cannot apply them via the hardcoded chart.
• The sidecars (node-driver-registrar, liveness-probe) are not privileged. Operators on platforms with a custom Localhost seccomp profile have no way to apply it today — the chart hardcodes RuntimeDefault on the sidecars. The per-sidecar fields in this PR are how they would.
• runAsUser pinning on the sidecars is a real requirement on OpenShift SCC ranges and in environments with audit-log UID alignment policies.
• fsGroup at pod level for the hostPath ownership of csi-data-dir / plugin-dir in clusters with strict ownership policies. Niche but valid.

So the narrower recommendation: keep podSecurityContext, keep the sidecar fields, keep per-container runAsUser. Drop only the main container's securityContext exposure — the silent-failure footgun documented in the issue write-up is specific to that one knob and does not generalise to the rest of the PR.

Apologies for the overshoot in the previous comment.

[yugstar]

Thank you for taking the time to test this end to end. The writeup in #583 is really helpful and convincing. I have updated both this PR and the companion cert-manager/csi-driver#672 to match your recommendation.

What changed:

• Dropped the configurable securityContext on the main driver container. It is back to the hardcoded privileged: true, with a comment in the daemonset explaining that the container mounts pods-mount-dir with mountPropagation: Bidirectional, which Kubernetes only allows for privileged containers. That removes the privileged: false footgun you documented.
• Kept the pod-level securityContext and the two sidecar securityContext knobs (node-driver-registrar, liveness-probe). Those are the legitimate operator cases you pointed out, and they match the trust-manager #836 precedent.

The values schema, README and helm unittests were regenerated and trimmed accordingly. Happy to also document the privileged requirement in the chart README if you would prefer that over the template comment.