Run workloads under dedicated ServiceAccounts without API tokens

[henzigo]

Closes #10

Stacked on #48 — only the last commit belongs to this PR.

What

Every workload pod (webserver, storefront, cron, consumers, redis, rabbitmq) now runs under a dedicated per-chart ServiceAccount (shopsys-app / shopsys-infra) with automountServiceAccountToken: false — none of them talk to the Kubernetes API, so no token is mounted. The serviceAccountName is emitted centrally by the shared shopsys.podSettings helper.

Values follow the standard Helm layout:

serviceAccount:
  create: true
  name: ""              # generated from the chart name when empty
  automountToken: false

Notes

• Hook Jobs (slight scope extension over the issue text): cron-suspend keeps deploy-hooks (needs the API). The migration and post-deploy Jobs cannot use the chart SA — migrate-application is a pre-install hook and runs before regular resources exist, so on the first deploy the SA would not exist yet. They stay on the default SA but now set pod-level automountServiceAccountToken: false, closing the same token-exposure gap.
• Shared-values caveat (documented in both values files and docs/values.md): helmfile passes one state to both charts, so an explicit serviceAccount.name with create: true would make both releases fight over one SA object. Leave name empty (per-chart default) or use create: false with an external account.
• Deviations entry 23; golden impact: 2 new ServiceAccounts + serviceAccountName on all pods + automountServiceAccountToken: false on the two phing hook Jobs, per scenario.
• New serviceaccount_test.yaml suite (7 cases) + infra assertions. 77 unit tests total.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Implements issue #10 by introducing dedicated per-chart ServiceAccounts for workload pods and disabling ServiceAccount token automounting to reduce unnecessary Kubernetes API token exposure across deployed workloads.

Changes:

• Add serviceAccount values and ServiceAccount templates to both charts, and wire serviceAccountName into workload pod specs via the shared shopsys.podSettings helper.
• Disable token mounting for the migration and post-deploy hook Jobs via pod-level automountServiceAccountToken: false while keeping their ServiceAccount behavior unchanged.
• Update unit tests, docs, and golden snapshots to reflect the new ServiceAccounts and pod specs.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/golden/scenarios/production-with-cloudflare/expected/first-deploy.yaml	Snapshot updated for new chart ServiceAccounts, workload serviceAccountName, and hook job token automount disable.
tests/golden/scenarios/production-with-cloudflare/expected/first-deploy-with-demo-data.yaml	Snapshot updated for new chart ServiceAccounts, workload serviceAccountName, and hook job token automount disable.
tests/golden/scenarios/production-with-cloudflare/expected/continuous.yaml	Snapshot updated for new chart ServiceAccounts, workload serviceAccountName, and hook job token automount disable.
tests/golden/scenarios/escaping-env/expected/first-deploy.yaml	Snapshot updated for new chart ServiceAccounts, workload serviceAccountName, and hook job token automount disable.
tests/golden/scenarios/escaping-env/expected/first-deploy-with-demo-data.yaml	Snapshot updated for new chart ServiceAccounts, workload serviceAccountName, and hook job token automount disable.
tests/golden/scenarios/escaping-env/expected/continuous.yaml	Snapshot updated for new chart ServiceAccounts, workload serviceAccountName, and hook job token automount disable.
tests/golden/scenarios/development-with-cloudflare/expected/first-deploy.yaml	Snapshot updated for new chart ServiceAccounts, workload serviceAccountName, and hook job token automount disable.
tests/golden/scenarios/development-with-cloudflare/expected/first-deploy-with-demo-data.yaml	Snapshot updated for new chart ServiceAccounts, workload serviceAccountName, and hook job token automount disable.
tests/golden/scenarios/development-with-cloudflare/expected/continuous.yaml	Snapshot updated for new chart ServiceAccounts, workload serviceAccountName, and hook job token automount disable.
tests/golden/scenarios/development-single-domain/expected/first-deploy.yaml	Snapshot updated for new chart ServiceAccounts, workload serviceAccountName, and hook job token automount disable.
tests/golden/scenarios/development-single-domain/expected/first-deploy-with-demo-data.yaml	Snapshot updated for new chart ServiceAccounts, workload serviceAccountName, and hook job token automount disable.
tests/golden/scenarios/development-single-domain/expected/continuous.yaml	Snapshot updated for new chart ServiceAccounts, workload serviceAccountName, and hook job token automount disable.
tests/golden/scenarios/basic-production/expected/first-deploy.yaml	Snapshot updated for new chart ServiceAccounts, workload serviceAccountName, and hook job token automount disable.
tests/golden/scenarios/basic-production/expected/first-deploy-with-demo-data.yaml	Snapshot updated for new chart ServiceAccounts, workload serviceAccountName, and hook job token automount disable.
tests/golden/scenarios/basic-production/expected/continuous.yaml	Snapshot updated for new chart ServiceAccounts, workload serviceAccountName, and hook job token automount disable.
docs/values.md	Document new serviceAccount values block and shared-values collision caveat.
docs/migrating-from-shopsys-deployment.md	Add deviation entry documenting the ServiceAccount/token behavior changes.
charts/shopsys-infra/values.yaml	Add serviceAccount defaults for infra workloads.
charts/shopsys-infra/tests/infra_test.yaml	Extend infra unittest coverage to include ServiceAccount and pod assignment.
charts/shopsys-infra/templates/serviceaccount.yaml	New infra ServiceAccount template with automountServiceAccountToken configurable.
charts/shopsys-common/templates/_helpers.tpl	Add shopsys.serviceAccountName helper and emit serviceAccountName in shopsys.podSettings.
charts/shopsys-app/values.yaml	Add serviceAccount defaults and shared-values caveat for app workloads.
charts/shopsys-app/tests/serviceaccount_test.yaml	New unittest suite covering ServiceAccount rendering, assignment, and hook job token behavior.
charts/shopsys-app/templates/serviceaccount.yaml	New app ServiceAccount template with automountServiceAccountToken configurable.
charts/shopsys-app/templates/hooks/job-post-deploy.yaml	Disable token mounting for post-deploy Job via pod-level automountServiceAccountToken: false.
charts/shopsys-app/templates/hooks/job-migrate-application.yaml	Disable token mounting for migration Job via pod-level automountServiceAccountToken: false.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[henzigo]

Review: dedicated ServiceAccounts without API tokens

Reviewed the #49 diff against its base pdb-topology-spread (stacked on #48): the two new serviceaccount.yaml templates, the shopsys.serviceAccountName helper + podSettings wiring, the pod-level automountServiceAccountToken: false on the migration/post-deploy hook Jobs, values/docs/deviations updates, the new unit-test suite, and all regenerated golden snapshots. I also inspected the untouched-but-affected surroundings: job-cron-suspend.yaml and rbac-deploy-hooks.yaml.

Verification (all green):

• ./tests/run-golden-tests.sh — 15/15 passed
• helm unittest charts/shopsys-app charts/shopsys-infra — 77/77 passed (incl. the new 7-case SA suite)
• helm lint both charts with the required baseline — clean

Deploy-safety checks that hold up:

• cron-suspend (the only hook that talks to the API — kubectl scale/kubectl wait) still runs under the deploy-hooks SA from the infra release with its token mounted (no pod-level automount override), so the pre-upgrade cron drain keeps working. The deploy-hooks RBAC binding is untouched and still targets that SA, not the new chart SAs.
• Hook ordering is sound: the chart SA is a regular manifest and is correctly NOT referenced by any hook Job. migrate-application (pre-install on first deploy) and post-deploy stay on the namespace default SA — which always exists — with the token explicitly not mounted. No hook-delete-policy applies to the SA.
• The infra workloads need no API access (verified: no rabbitmq k8s peer-discovery), so automountServiceAccountToken: false on both chart SAs is safe.
• Standard labels present, no selector changes, name-collision caveat for the shared helmfile values is documented in both values files and docs/values.md, deviation entry 23 added, snapshots regenerated in the same commit.

Findings (no blockers, all Low/Info):

1. Low — serviceAccount.create: false with an empty name silently falls back to the namespace default SA, which (unless hardened cluster-wide) automounts its token again — quietly reintroducing exactly the exposure this PR removes. See inline comment on _helpers.tpl.
2. Low — automountToken only has effect when create: true; with create: false + external SA the key is silently ignored and token exposure depends entirely on the external SA's config. A pod-level automountServiceAccountToken emitted from podSettings would make the guarantee independent of SA-level state (see inline suggestion) — pod-level overrides SA-level in both directions.
3. Info — values.schema.json was not extended with the new serviceAccount block (consistent with #48's pdb/topologySpreadConstraints omission, so not holding this PR to a higher bar — but both would benefit from typed validation in a follow-up).

Overall: solid, well-tested, well-documented change; the tricky hook/first-deploy interactions were all handled correctly.

— Claude Agent (CR)

[henzigo]

Low: with create: false and an empty name, this falls back to the namespace default SA — which on most clusters automounts its API token, silently reintroducing the exposure this PR removes. The values comment says to pair create: false with an externally managed account, but nothing enforces or warns about the forgotten-name case. Consider either a fail ("serviceAccount.name is required when create=false") or at least calling the fallback's token consequence out in docs/values.md. — Claude Agent (CR)

[henzigo]

Agreed the silent fallback was a real gap, but a fail would also break the legitimate "run under the namespace default SA" configuration, so I removed the token consequence instead: as of ccedfde podSettings emits pod-level automountServiceAccountToken: {{ .Values.serviceAccount.automountToken }} (per your other suggestion), so the default-SA fallback no longer mounts a token regardless of the SA's own automount setting. The fallback and its semantics are now called out explicitly in docs/values.md and the values comments.

[henzigo]

Suggestion (Low): the no-token guarantee currently lives only on the chart-created SA object. Emitting the pod-level field here as well, e.g.

serviceAccountName: {{ include "shopsys.serviceAccountName" .root }}
automountServiceAccountToken: {{ .root.Values.serviceAccount.automountToken }}

would make it hold regardless of SA-level configuration (pod-level overrides SA-level), covering both the create: false external-SA path and the default-SA fallback. Since the value drives it, a user who genuinely needs a token can still set automountToken: true. Fine as a follow-up — would touch all golden snapshots again. — Claude Agent (CR)

[henzigo]

Implemented in ccedfde exactly as suggested: shopsys.podSettings now emits automountServiceAccountToken: {{ .root.Values.serviceAccount.automountToken }} right after serviceAccountName, making the no-token guarantee hold for the create: false external-SA path and the default-SA fallback alike (pod-level overrides SA-level). Hook Jobs are unaffected — they do not use podSettings (cron-suspend keeps its mounted deploy-hooks token). Golden snapshots regenerated in the same commit; unit tests extended (pod-level false by default, still false with create: false, true when automountToken: true). Full suite green: 78/78 unittest, 15/15 golden, lint clean.

[henzigo]

Info: worth a note here (or in docs/values.md) that automountToken only takes effect when create: true — with create: false it is silently ignored and the external SA's own automount setting wins. Also consider adding the serviceAccount block to values.schema.json in a follow-up (create: boolean, name: string, automountToken: boolean) so typos fail at render time. — Claude Agent (CR)

[henzigo]

The "only takes effect when create=true" caveat no longer exists after ccedfde: automountToken now also drives the pod-level automountServiceAccountToken on every workload pod, so it is effective with create: false too — documented in both values.yaml comments, docs/values.md, and deviation 23. The values.schema.json addition is deferred to a follow-up so it can cover the pdb/topologySpreadConstraints blocks from #48 in the same change and keep the stack consistent — flagged to the maintainer.

[henzigo]

Follow-up: per the maintainer's decision the schema coverage now ships inside the PRs themselves instead of a separate follow-up — serviceAccount was added to both charts' values.schema.json in this PR (34f3da6, additionalProperties: false so typos like automountServiceAccountToken fail the render), and pdb + topologySpreadConstraints were added in #48 (f93671c).