feat(preview): optional hosted sandboxes, off by default - #6212
Merged
Conversation
A preview cannot drive agents today: SANDBOX_START creates a SandboxClaim through the Kubernetes API and the preview's ServiceAccount has no rights in the sandbox namespace, so it fails with a 403 that surfaces as an unhandled rejection. This adds the binding, and keeps it off by default, because what it grants is not narrow. The Role lives in a namespace shared by every environment on the cluster, production included, and Kubernetes RBAC has no label scoping — so a preview bound there reaches production sandboxes too. A preview runs un-merged code from any PR carrying a label. That is the whole trade, and it is written at the top of the template rather than buried in a values comment. It is shaped this way because the sandbox namespace is hardcoded in the application and across the sandbox-env chart, on the premise that multiple envs share it. A namespace-per-environment, or the move to a controller API with a scoped token, removes the need for this entirely. Two guards, because both failures render something that looks correct: serviceAccount.create must be on (otherwise the binding lands on and every pod in the preview gets sandbox access, Postgres and MinIO included), and roleName must be set (an empty roleRef binds nothing). Permission and configuration stay separate: enabling this grants rights but changes no behaviour until the STUDIO_SANDBOX_* block is pointed at an existing environment, which values-preview.yaml now spells out.
…ables values-preview.yaml now sets serviceAccount.create=true, so the negative case rendered successfully and the assertion reported a missing guard.
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
1 participant
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.
A preview cannot drive agents today.
SANDBOX_STARTcreates aSandboxClaimthrough the Kubernetes API, and the preview's ServiceAccount has no rights in the sandbox namespace:Observed on #6205.
What this grants, plainly
The Role being bound lives in
agent-sandbox-system, which holds the sandboxes of every environment on the cluster, production included:Kubernetes RBAC has no label scoping. A preview bound there gets
pods/portforward, anddelete/patchon HTTPRoutes, across all sandboxes in that namespace — not only the ones it creates. The housekeeper'sCLAIM_SELECTORseparates what it sweeps; it does not separate what a ServiceAccount can touch.A preview runs un-merged code from any PR carrying the
previewlabel. So enabling this makes a label on a PR sufficient to reach production sandboxes. That is the trade, and it is stated at the top of the template rather than buried in a values comment.Default is
false. Nothing changes unless someone turns it on.Why it cannot be narrowed today
The sandbox namespace is hardcoded — in the application, and across 16 files of the
sandbox-envchart — on the premise recorded inlifecycle.ts:A
sandbox-envinstalled into its own namespace would need application changes, not just a differenthelm install. The move to a controller API instead of the Kubernetes API removes the need entirely, by replacing cluster RBAC with a tenant-scoped token.Two guards
Both cover a render that would look healthy and grant the wrong thing:
serviceAccount.createmust be truedefaultSA, giving sandbox access to every pod in the preview — Postgres and MinIO includedroleNamemust be setroleRefbinds nothing, silentlyAsserted in
helm-test.ymlon the message text, not just the exit code.Permission and configuration are separate
Enabling
preview.sandbox.enabledgrants rights and changes no behaviour. The preview still needs theSTUDIO_SANDBOX_*block pointed at an existing environment —values-preview.yamlnow spells out exactly which settings, and notes that withoutSTUDIO_SANDBOX_SENTINEL_TOKENthe claim takes the cold-start path rather than borrowing the shared warm pool.Side effect worth having
Previews now run under a named ServiceAccount instead of
default. Postgres and MinIO stay ondefault, so even with sandboxes enabled they hold no sandbox rights — only the app, the worker and the migrate Job do.Verification
main.helm lintclean.Summary by cubic
Adds optional hosted agent sandboxes for previews. Previously
SANDBOX_STARTfailed with 403; now, when enabled, previews can claim and drive sandboxes. Default stays off to avoid giving preview code reach into production sandboxes.agent-sandbox-system, grantingpods/portforwardand HTTPRoute delete/patch across that namespace.serviceAccount.create=trueand a non-emptypreview.sandbox.roleName; CI asserts both failure paths.preview.sandbox.enabled=trueonly grants rights; the app must still setSTUDIO_SANDBOX_*(provider, runner, template, gateway) to a real environment.default.Rollout
preview.sandbox.enabled=true,preview.sandbox.namespace(defaults toagent-sandbox-system), andpreview.sandbox.roleNameto a Role from thesandbox-envchart; also configure theSTUDIO_SANDBOX_*block.Written for commit 4cdbe21. Summary will update on new commits.