Skip to content

Allow a security context on every pod py-app renders - #31

Open
devjadeja-treets wants to merge 1 commit into
masterfrom
feature/dev-167-wire-security-contexts-through-every-py-app-pod-template
Open

devjadeja-treets wants to merge 1 commit into
masterfrom
feature/dev-167-wire-security-contexts-through-every-py-app-pod-template

Conversation

@devjadeja-treets

@devjadeja-treets devjadeja-treets commented Sep 9, 2026

Copy link
Copy Markdown
Member

py-app produces four pod specs and only one of them could be given a security context:

Template Workload Before After
deployment.yaml web container only pod + container
migrators.yaml Alembic / Scylla Jobs neither pod + container
taskiq-worker.yaml taskiq workers neither pod + container
taskiq-scheduler.yaml taskiq scheduler neither pod + container

There was also no podSecurityContext key anywhere in the chart, so runAsNonRoot, fsGroup
and seccompProfile had nowhere to go on any of the four — and pod level is where the kubelet
enforces runAsNonRoot.

Both values default to {} and both are documented in values.yaml.

This changes nothing on its own

With neither value set, rendered output is identical to the published 0.1.0 apart from the
helm.sh/chart label. There is no test job in this repo, so here are the two commands that make
that claim checkable:

cat > /tmp/tv.yaml <<'YAML'
image: {repository: example/app, tag: t1}
taskiq:
  workerCmd: ["python", "-m", "app", "worker"]
  schedulerCmd: ["python", "-m", "app", "scheduler"]
migrators:
  pg:
    command: ["alembic", "upgrade", "head"]
    resources: {requests: {cpu: 10m, memory: 64Mi}}
  scylla:
    command: ["python", "-m", "cli", "scylla", "up", "head"]
YAML

diff \
  <(helm template t oci://ghcr.io/treetscom/charts/py-app --version 0.1.0 -f /tmp/tv.yaml \
      2>/dev/null | grep -v 'helm.sh/chart: py-app-') \
  <(helm template t ./charts/py-app -f /tmp/tv.yaml \
      2>/dev/null | grep -v 'helm.sh/chart: py-app-')

That values set exercises all five workloads — web, taskiq worker, taskiq scheduler, and both
migrator Jobs, one with per-migrator resources and one without, since the new container block
sits 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-api is the only consumer setting securityContext today, and the value is
privileged: true (ArgoCD/apps/intree-api/base_values.yaml:71-72). It also has both taskiq
workloads and two migrators. Rendering its live prod values:

py-app 0.1.0 (published)   → 1 privileged container:  intree-api-app
py-app 0.2.0 (this branch) → 5 privileged containers: intree-api-app, -tkq,
                               -tkq-scheduler, -pg-mj, -scylla-mj

So Treetscom/ArgoCD#2179 (which removes that line) has to merge before the appset
targetRevision bump — not before this PR. Publishing 0.2.0 is safe on its own: the 21 appsets
stay pinned to 0.1.0 and pick up nothing until they are moved. That bump is a separate ArgoCD PR.

Version bump

0.1.00.2.0. Pushing to master publishes all three charts with no per-chart change
detection, and 21 appsets pin targetRevision: 0.1.0, so bumping here is what stops a publish
silently overwriting the tag every consumer is on. pgbouncer and i2g-operator are untouched
and still template cleanly.

Deliberate omission

readOnlyRootFilesystem is not among the documented examples. The Python services write to
/tmp at runtime — apns cert material in intree-api, media-api's upload processing — and this
chart has no volumeMounts support to give them an emptyDir there. Suggesting it would hand the
next caller a failure the chart cannot fix. Worth its own issue.

Closes DEV-167

@devjadeja-treets devjadeja-treets added the Security Impacts security label Sep 9, 2026
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
devjadeja-treets force-pushed the feature/dev-167-wire-security-contexts-through-every-py-app-pod-template branch from 6761866 to 021e3c1 Compare September 15, 2026 07:24
@devjadeja-treets devjadeja-treets self-assigned this Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Security Impacts security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant