fix(helm): render pod-level securityContext on the migration Job - #35482
Conversation
|
|
Greptile SummaryThis PR applies the chart’s pod-level security context to the migration Job so hardened Kubernetes clusters can admit migration pods.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| helm/litellm-helm/templates/migrations-job.yaml | Correctly renders the configured pod security context on the migration Job using the established Deployment pattern. |
| helm/litellm-helm/tests/migrations-job_tests.yaml | Adds focused assertions for pod-level security context rendering and separation from container-level settings. |
Reviews (2): Last reviewed commit: "fix(helm): render pod-level securityCont..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The litellm-helm proxy Deployment renders a pod-level securityContext from .Values.podSecurityContext, but the Prisma migration Job rendered only the container-level securityContext from .Values.securityContext. Clusters that enforce pod-level admission policies (OPA Gatekeeper K8sPSPAllowedUsers, or a PSP-style fsGroup MustRunAs rule) therefore admitted the Deployment and denied the Job, which blocks install and upgrade because the Job runs as an ArgoCD PreSync or Helm pre-install/pre-upgrade hook. The Job now renders the same pod-level securityContext the Deployment does. Charts that leave podSecurityContext unset render an empty securityContext, matching what the Deployment already emitted, so default installs are unchanged. Resolves LIT-4928
952cc4f to
daa3eaf
Compare
|
@greptileai please review the current head Rebased onto current |
TLDR
Problem this solves:
securityContextHow it solves it:
.Values.podSecurityContext, like the DeploymentRelevant issues
Linear ticket
Resolves LIT-4928
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
The proof is a real Kubernetes cluster, not
helm template, because the failure is an admission decision. Akindcluster (k8s v1.36.1) enforces aValidatingAdmissionPolicythat mirrors the reported GatekeeperK8sPSPAllowedUsersconstraint (fsGroupruleMustRunAs, ranges 1-65535), so no Gatekeeper install is needed and the denial message is reproduced verbatim.Before:
704b9da8ab(origin/litellm_internal_staging). After:daa3eaf443Each leg runs in its own namespace and is bracketed by a gate that creates an unhardened pod and requires it to be denied, both immediately before the install and immediately after; a leg whose gate does not deny is discarded rather than reported. This caught two would-be false passes: one where the policy had not finished propagating to a freshly created namespace, and one where another cluster on the same machine had taken over the shared kubeconfig's current-context, so every run is pinned to an explicit
KUBECONFIGwritten bykind get kubeconfig --name lit4928.Cluster and policy setup
Values used for both legs, matching the reported configuration (a plain Postgres pod runs in each namespace as the migration target):
Before the fix, at
704b9da8ab; install is blockedNot one migration pod is ever created, and the denial text matches the report.
After the fix, at
daa3eaf443; install completesThe Job pod is admitted carrying the configured
fsGroup, the pre-install hook completes, and the release reachesdeployedunder the same policy that rejected it before.Rendered output matches the Deployment
With
podSecurityContextleft at its default the Job renderssecurityContext: {}, which is exactly what the Deployment already emitted, so existing installs see no change.Chart gates
Mutation check on the two new tests
Against the chart at
704b9da8abwith only the new tests copied in (fix absent), and against a copy whose pod-level context reads.Values.securityContextinstead, each mutation asserted as applied before running:Both new tests fail on both mutants and pass on the fix.
Type
🐛 Bug Fix
Changes
helm/litellm-helm/templates/migrations-job.yamlrendersspec.template.spec.securityContextfrom.Values.podSecurityContext, the same expression and position the proxy Deployment already uses. Two regression tests land in the existinghelm/litellm-helm/tests/migrations-job_tests.yamlsuite, which theHelm unit testworkflow already runs: one pins the pod-level context to the configured value, the other sets pod-level and container-level contexts to different values and pins both, so a fix applied at the wrong level or from the wrong values key fails.Two things deliberately left out of scope. The
helm/litellmchart has nosecurityContextsupport in any template, so it has no inconsistency to correct here; adding hardening to it is a feature rather than this regression. Separately, running thelitellm-databaseimage underrunAsUser: 10000leavesHOME=/unwritable, so Prisma logs aPermissionErroron/.cacheand exits 0 without applying the schema. That reproduces from a bare pod with no chart involved and applies equally to the proxy Deployment, so it is an image-level limitation rather than anything this template change introduces or can fix.Final Attestation