diff --git a/.github/workflows/helm-test.yml b/.github/workflows/helm-test.yml index 378ec64210..1e4b80e5b7 100644 --- a/.github/workflows/helm-test.yml +++ b/.github/workflows/helm-test.yml @@ -496,6 +496,21 @@ jobs: --set preview.postgres.enabled=false \ --set externalSecret.enabled=false + # Binding sandbox access to the namespace's `default` ServiceAccount + # would hand it to every pod in the preview — Postgres and MinIO + # included — and the render would look perfectly healthy. + # values-preview.yaml turns serviceAccount.create on, so this case has + # to turn it back off — the point is the guard, not the default. + expect_fail "sandbox without a named ServiceAccount" "requires serviceAccount.create=true" \ + --set preview.host=h.example.com \ + --set preview.sandbox.enabled=true \ + --set preview.sandbox.roleName=some-role \ + --set serviceAccount.create=false + expect_fail "sandbox without a role" "preview.sandbox.roleName is required" \ + --set preview.host=h.example.com \ + --set preview.sandbox.enabled=true \ + --set serviceAccount.create=true + # image.command is a list, which --set cannot express cleanly. printf 'image:\n command: ["bun","run","deco"]\n' > /tmp/no-skip.yaml expect_fail "missing --skip-migrations" "requires --skip-migrations" \ diff --git a/deploy/helm/studio/Chart.yaml b/deploy/helm/studio/Chart.yaml index 1fb5c299fa..bb5b01487e 100644 --- a/deploy/helm/studio/Chart.yaml +++ b/deploy/helm/studio/Chart.yaml @@ -2,6 +2,12 @@ apiVersion: v2 name: chart-deco-studio description: Helm chart for deco Studio — supports inline secrets, external secretName, and AWS Secrets Manager via ExternalSecret type: application +# 0.14.2: optional hosted sandboxes for previews (preview.sandbox.enabled, +# default false). Renders one RoleBinding when on, nothing when off. Read the +# note in templates/preview-sandbox-rbac.yaml before enabling: the namespace it +# binds into is shared with production sandboxes and RBAC has no label scoping. +# Previews now also run under a named ServiceAccount rather than `default`. +# # 0.14.1: the preview ConfigMap had no sync-wave, so it landed at 0 — after the # migrate Job at -10 that consumes it via envFrom. The Job sat in # CreateContainerConfigError and the sync stalled on the hook with nothing on @@ -26,7 +32,7 @@ type: application # when disabled, so existing releases are unaffected. # 0.12.4: chart-managed API/worker dispatch roles now use # STUDIO_DISPATCH_ROLE; legacy overrides are rejected. -version: 0.14.1 +version: 0.14.2 appVersion: "latest" dependencies: diff --git a/deploy/helm/studio/templates/_helpers.tpl b/deploy/helm/studio/templates/_helpers.tpl index 43ace27ba8..9afb1d2fbf 100644 --- a/deploy/helm/studio/templates/_helpers.tpl +++ b/deploy/helm/studio/templates/_helpers.tpl @@ -341,6 +341,21 @@ Service/Deployment name for the preview's own MinIO. {{- printf "%s-minio" (include "chart-deco-studio.fullname" .) | trunc 63 | trimSuffix "-" -}} {{- end }} +{{/* +Guards the sandbox binding. Both checks stop a render that would look correct +and grant the wrong thing. +*/}} +{{- define "chart-deco-studio.validatePreviewSandbox" -}} +{{- if and .Values.preview.enabled .Values.preview.sandbox.enabled }} +{{- if not .Values.serviceAccount.create }} +{{- fail "chart-deco-studio: preview.sandbox.enabled=true requires serviceAccount.create=true — otherwise the binding lands on the namespace's `default` ServiceAccount, handing sandbox access to every pod in the preview, Postgres and MinIO included" -}} +{{- end }} +{{- if not .Values.preview.sandbox.roleName }} +{{- fail "chart-deco-studio: preview.sandbox.roleName is required when preview.sandbox.enabled=true" -}} +{{- end }} +{{- end }} +{{- end }} + {{/* Validates per-PR preview releases. Every failure here is something that would otherwise render a healthy-looking object that silently does nothing: an diff --git a/deploy/helm/studio/templates/preview-sandbox-rbac.yaml b/deploy/helm/studio/templates/preview-sandbox-rbac.yaml new file mode 100644 index 0000000000..1be8841f7d --- /dev/null +++ b/deploy/helm/studio/templates/preview-sandbox-rbac.yaml @@ -0,0 +1,49 @@ +{{- if and .Values.preview.enabled .Values.preview.sandbox.enabled }} +{{- /* +Lets a preview drive hosted agent sandboxes. + +READ THIS BEFORE ENABLING. The Role bound here lives in the sandbox namespace, +and that namespace holds the sandboxes of EVERY environment on the cluster — +production included. Kubernetes RBAC has no label scoping, so this grants reach +over all of them, not only the ones a preview creates. The housekeeper's +CLAIM_SELECTOR separates what it sweeps; it does not separate what a +ServiceAccount can touch. + +A preview runs un-merged code from any pull request carrying the preview label. +Binding it here means a label on a PR is enough to obtain pods/portforward and +HTTPRoute delete against production sandboxes. + +It is written this way because the sandbox namespace is not configurable — it is +hardcoded in the application and throughout the sandbox-env chart, on the stated +premise that "multiple envs share agent-sandbox-system". Until that changes, or +until the application talks to a controller API instead of the Kubernetes API +(which would replace this with a scoped token), there is no way to give previews +sandboxes without also giving them this reach. + +Off by default. Turn it on deliberately, and for as long as you actually need it. +*/ -}} +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: {{ include "chart-deco-studio.fullname" . }}-sandbox + namespace: {{ required "preview.sandbox.namespace is required when preview.sandbox.enabled=true" .Values.preview.sandbox.namespace }} + labels: + {{- include "chart-deco-studio.labels" . | nindent 4 }} + annotations: + # Ahead of the pods, so the ServiceAccount can already claim by the time + # anything tries to. + argocd.argoproj.io/sync-wave: "-30" + # Deliberately loud: this object is the one place where a preview reaches + # outside its own namespace, and into a shared production one. + decocms.com/grants-access-to: {{ .Values.preview.sandbox.namespace | quote }} +subjects: + - kind: ServiceAccount + name: {{ include "chart-deco-studio.serviceAccountName" . }} + namespace: {{ .Release.Namespace }} +roleRef: + kind: Role + # An existing Role, not one this chart defines: the sandbox-env chart owns the + # verb set, and a copy here would drift from it silently. + name: {{ required "preview.sandbox.roleName is required when preview.sandbox.enabled=true" .Values.preview.sandbox.roleName }} + apiGroup: rbac.authorization.k8s.io +{{- end }} diff --git a/deploy/helm/studio/templates/validations.yaml b/deploy/helm/studio/templates/validations.yaml index c5238cdeae..b30587f11a 100644 --- a/deploy/helm/studio/templates/validations.yaml +++ b/deploy/helm/studio/templates/validations.yaml @@ -9,3 +9,4 @@ This file only runs chart-level validations and renders no resources. {{- include "chart-deco-studio.validateClickhouse" . -}} {{- include "chart-deco-studio.validateIngress" . -}} {{- include "chart-deco-studio.validatePreview" . -}} +{{- include "chart-deco-studio.validatePreviewSandbox" . -}} diff --git a/deploy/helm/studio/values-preview.yaml b/deploy/helm/studio/values-preview.yaml index d8faf64030..be70e0497e 100644 --- a/deploy/helm/studio/values-preview.yaml +++ b/deploy/helm/studio/values-preview.yaml @@ -164,6 +164,12 @@ topologySpreadConstraints: [] # Spot is safe here in a way it is nowhere else on this cluster: the database # is an emptyDir Postgres that the PreSync Job re-migrates on the next sync, # and a preview holds no PVC at all. A reclaim costs a reviewer a restart. +# A named ServiceAccount rather than the namespace's `default`. Costs nothing +# when sandboxes are off, and is required when they are on: binding sandbox +# access to `default` would hand it to every pod here, Postgres and MinIO too. +serviceAccount: + create: true + nodeSelector: decocms.com/nodepool: studio-preview tolerations: @@ -207,8 +213,25 @@ configMap: AUTH_EMAIL_PASSWORD_ENABLED: "true" # Several reviewers behind one office IP trip the auth limiter otherwise. DISABLE_RATE_LIMIT: "true" - # No hosted sandboxes in a preview. Agent dispatch returns 409 link_offline + # No hosted sandboxes by default. Agent dispatch returns 409 link_offline # with no local daemon attached — a documented limitation, not a bug. + # + # To test agent behaviour in a preview, set preview.sandbox.enabled=true + # (see values.yaml — it grants reach over a shared production namespace) and + # replace this block with the settings below, pointed at an existing + # environment whose templates and warm pool the preview borrows: + # + # STUDIO_SANDBOX_PROVIDER: "cluster" + # STUDIO_SANDBOX_RUNNER: "agent-sandbox" + # STUDIO_SANDBOX_TEMPLATE_NAME: "" + # STUDIO_ENV: "" + # STUDIO_SANDBOX_PREVIEW_GATEWAY_NAME: "" + # STUDIO_SANDBOX_PREVIEW_GATEWAY_NAMESPACE: "agent-sandbox-system" + # STUDIO_SANDBOX_PREVIEW_URL_PATTERN: "https://{handle}./" + # + # Without STUDIO_SANDBOX_SENTINEL_TOKEN the claim takes the cold-start path + # instead of the warm pool — slower per sandbox, and it borrows no capacity + # from the environment being shared. STUDIO_SANDBOX_PROVIDER: "user-desktop" DISABLE_ORGFS_MOUNTS: "true" # Never let a preview provision real gateway keys. STUDIO_PROVISION_SECRET_KEY diff --git a/deploy/helm/studio/values.yaml b/deploy/helm/studio/values.yaml index 2bec0e2444..5cc3bb7af2 100644 --- a/deploy/helm/studio/values.yaml +++ b/deploy/helm/studio/values.yaml @@ -147,6 +147,23 @@ preview: limits: memory: "768Mi" + # Hosted agent sandboxes for a preview. OFF by default, and it should stay off + # unless someone is actively testing agent behaviour. + # + # Enabling binds this release's ServiceAccount to a Role in a namespace shared + # with production sandboxes — see templates/preview-sandbox-rbac.yaml for why + # that cannot currently be narrowed, and what would remove the need. + # + # The application also needs the matching STUDIO_SANDBOX_* configuration + # (provider, runner, template name, preview gateway) pointed at an existing + # environment; enabling this alone grants permission but changes no behaviour. + sandbox: + enabled: false + # Namespace holding the SandboxTemplates, warm pools and claims. + namespace: agent-sandbox-system + # An existing Role in that namespace, owned by the sandbox-env chart. + roleName: "" + # Resources for the short-lived migration Job. jobResources: requests: