Allow a security context on every pod py-app renders - #31
Open
devjadeja-treets wants to merge 1 commit into
Open
devjadeja-treets wants to merge 1 commit into
devjadeja-treets wants to merge 1 commit into
Conversation
py-app produces four pod specs and only one of them could be given a security
context: deployment.yaml honoured .Values.securityContext, while
migrators.yaml, taskiq-worker.yaml and taskiq-scheduler.yaml silently ignored
it. There was also no podSecurityContext key anywhere in the chart, so
runAsNonRoot, fsGroup and seccompProfile had nowhere to go on any of the four
-- pod level is where the kubelet enforces runAsNonRoot.
Adds podSecurityContext at pod level and wires securityContext at container
level across all four templates. Both default to {} and both are documented in
values.yaml with the settings a caller is most likely to want.
readOnlyRootFilesystem is deliberately not among them: the Python services
write to /tmp at runtime and this chart has no volumeMounts support to give
them an emptyDir there, so suggesting it would hand callers a foot-gun.
No defaults are set, so this changes nothing on its own. With neither value
provided, `helm template` output is identical to the published 0.1.0 apart
from the helm.sh/chart version label, checked against a values set that
exercises all five workloads -- web, taskiq worker, taskiq scheduler, and the
pg and scylla migrator Jobs, with and without per-migrator resources. With
both contexts set they land on all five at the right level and the result
passes `kubectl create --dry-run=client`.
Version bumped to 0.2.0. Consumers pin targetRevision in appsets/defs, so
nothing picks this up until those move -- which matters, because intree-api is
currently the only consumer setting securityContext and the value is
`privileged: true`. Rendering its live prod values against this chart takes it
from 1 privileged container to 5. ArgoCD#2179 removes that line and has to
merge before the appset bump, not before this.
Change-Id: 964edff23912c3c8aa1d2253a2e61328
Signed-off-by: Dev Jadeja <dev@treets.com>
devjadeja-treets
force-pushed
the
feature/dev-167-wire-security-contexts-through-every-py-app-pod-template
branch
from
September 15, 2026 07:24
6761866 to
021e3c1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
py-appproduces four pod specs and only one of them could be given a security context:deployment.yamlmigrators.yamltaskiq-worker.yamltaskiq-scheduler.yamlThere was also no
podSecurityContextkey anywhere in the chart, sorunAsNonRoot,fsGroupand
seccompProfilehad nowhere to go on any of the four — and pod level is where the kubeletenforces
runAsNonRoot.Both values default to
{}and both are documented invalues.yaml.This changes nothing on its own
With neither value set, rendered output is identical to the published
0.1.0apart from thehelm.sh/chartlabel. There is no test job in this repo, so here are the two commands that makethat claim checkable:
That values set exercises all five workloads — web, taskiq worker, taskiq scheduler, and both
migrator Jobs, one with per-migrator
resourcesand one without, since the new container blocksits directly below that stanza. With both contexts set they land on all five at the right level
and the result passes
kubectl create --dry-run=client.Sequencing: read before bumping the appsets
intree-apiis the only consumer settingsecurityContexttoday, and the value isprivileged: true(ArgoCD/apps/intree-api/base_values.yaml:71-72). It also has both taskiqworkloads and two migrators. Rendering its live prod values:
So Treetscom/ArgoCD#2179 (which removes that line) has to merge before the appset
targetRevisionbump — not before this PR. Publishing0.2.0is safe on its own: the 21 appsetsstay pinned to
0.1.0and pick up nothing until they are moved. That bump is a separate ArgoCD PR.Version bump
0.1.0→0.2.0. Pushing tomasterpublishes all three charts with no per-chart changedetection, and 21 appsets pin
targetRevision: 0.1.0, so bumping here is what stops a publishsilently overwriting the tag every consumer is on.
pgbouncerandi2g-operatorare untouchedand still template cleanly.
Deliberate omission
readOnlyRootFilesystemis not among the documented examples. The Python services write to/tmpat runtime — apns cert material inintree-api,media-api's upload processing — and thischart has no
volumeMountssupport to give them an emptyDir there. Suggesting it would hand thenext caller a failure the chart cannot fix. Worth its own issue.
Closes DEV-167