Repository navigation
Helm chart follow-ups from PR #1481 review (worker isolation, migration readiness, port handling, post-processor) #1486
Description
Context
PR #1481 added the Helm chart that deploys the Scope app onto the Bicep-provisioned AKS cluster (#1477). Review surfaced several findings; the P1/quick ones were fixed directly in that PR (worker-scoped identity wiring, mutable latest image tags, migration image pinning, publish-images.yml path filters). The remaining findings below are valid but need more design work or a product decision, so they're tracked here for a follow-up PR instead of blocking #1481.
Remaining issues
1. Separate benchmark execution from the backend workload identity (P1)
Both ACP workers (coder-acp-copilot, coder-acp-claude-code) run benchmark-controlled agent code in the same container/pod that mounts the kv-secrets CSI volume and the common-secret envFrom. Per the companion Bicep work (#1477), the worker identity currently has Key Vault Secrets User (read-only) because the workers need their BYO GitHub token/Anthropic API key from the vault — but that means untrusted agent subprocess code (which merges the parent environment and shares its filesystem) can also read those secrets.
Needs: split the trusted queue processor from the untrusted executor (no creds/federated token in the executor, automountServiceAccountToken: false on the executor), which requires worker code changes beyond the chart. Already documented as a known limitation in deploy/helm/scope/WORKERS.md.
2. Preserve the API's required-migration readiness barrier (P1)
api/deployment.yaml's readiness probe (like its liveness probe) hits /health, which always returns 200 — it should use /ready, which checks required migrations. Combined with the db-migrate Job's current post-install,post-upgrade hook ordering, new API pods can receive traffic before required schema changes finish, and stay Ready even if the migration hook fails outright.
Needs: redesign the hook ordering (the Job currently runs post-install/post-upgrade specifically to avoid a chicken-and-egg problem with the ServiceAccount/SecretProviderClass not existing yet pre-install — see the comment in templates/jobs/db-migrate.yaml) alongside switching the readiness probe to /ready, so helm --wait doesn't deadlock.
3. Preserve existing worker settings when bootstrapping a single worker (P2)
scripts/bootstrap-workers.sh --skip-copilot --values-file <original-core-values-file> re-applies the original install-time values file without merging in a prior bootstrap run's worker overrides (e.g. workers.coderAcpCopilot.enabled=true). Since the original values file doesn't contain those overrides, Helm falls back to the chart default and disables the worker that's supposed to be left alone.
Needs: merge current chart defaults with the release's existing overrides when re-running bootstrap for a subset of workers (e.g. something like --reset-then-reuse-values, or reading helm get values to build the merge instead of relying solely on a user-supplied file). Add a sequential single-worker bootstrap regression test.
4. Include the post-processor in the completed-run pipeline (P2)
Both ACP workers enqueue to post-processor-queue and set run.postProcessorStatus: queued on completion, but neither this chart nor publish-images.yml includes apps/workers/post-processor or its version-registration mechanism. Enabling report-generator alone isn't sufficient — completed runs queue for post-processing with no consumer deployed, and automatic report triggering never happens.
Needs: publish/deploy the post-processor (new chart Deployment + add it to the publish workflow, with its version-registration step), or explicitly document/disable this pipeline for the minimal deployment.
5. Keep configurable ports consistent with the shipped nginx config (P2)
The portal image bakes in nginx.conf with listen 80 and proxy_pass http://api:80. The chart exposes portal.port/api.port as configurable values, but changing either doesn't update the baked-in nginx config, so overriding them breaks the portal (502s or unhealthy readiness).
Needs: either template the nginx listen/upstream config at container start (e.g. via apps/portal/docker-entrypoint.sh, which already does some env-driven config), or stop exposing portal.port/api.port as overridable since the shipped image can't honor arbitrary values.
Reference
- Chart:
deploy/helm/scope/ - Original review: Add Helm chart to deploy Scope app onto the Bicep-provisioned AKS cluster #1481
Activity