Skip to content

sandbox apply: --dry-run to render a spec without applying it - #362

Open
joehorsnell wants to merge 1 commit into
mainfrom
joe/ENG-1187/sandbox-apply-dry-run
Open

sandbox apply: --dry-run to render a spec without applying it#362
joehorsnell wants to merge 1 commit into
mainfrom
joe/ENG-1187/sandbox-apply-dry-run

Conversation

@joehorsnell

Copy link
Copy Markdown
Contributor

What this does

Adds --dry-run=none|client|server to signadot sandbox apply. client renders the
spec, validates it, and prints it instead of applying it:

$ signadot sandbox apply -f .signadot/template.yaml --set tag=$(git rev-parse HEAD) --dry-run=client
name: acme-web-1234
spec:
  cluster: prod-eks
  forks:
    - forkOf:
        kind: Deployment
        name: web
        namespace: acme
      customizations:
        images:
          - image: ghcr.io/acme/web:5f2c1a0

Why

Rendering a template has always been local, but the only way to see what it produced
was to apply the sandbox. That leaves two things unserved: CI cannot show what it is
about to do, and a template cannot be checked without creating something.

The rendering step already existed and already ran locally — it just ran after
authentication. Moving it before authentication is most of this change, and it means
--dry-run=client works for someone who has never logged in, and fails identically
whether or not they have.

The output is a spec, not a report, so it can be reviewed, diffed, and passed straight
back to -f. TestDryRunOutputIsAFixedPoint pins that: re-rendering a rendered spec
changes nothing. That property is what lets a caller render in one step and apply
exactly those bytes in another, which is how the sandbox GitHub Action
(ENG-1187)
gets a rendered-spec output that is provably what was applied.

--dry-run=server is accepted by the flag and rejected at runtime, since it needs a
validateOnly parameter on the apiserver that does not exist yet
(ENG-1203).
Keeping it in the grammar now means adding it later needs no new spelling, and the
distinction between rendering locally and asking the server to validate is the one
kubectl users already know.

Scope

Deliberately small and self-contained. It is the first of two: the larger change on
#361 proposes a values document and a
built-in template so a sandbox can be described without authoring a template at all.
That one is a design decision and is still a draft. This one stands on its own for
anyone who already has a template.

One behaviour change beyond the new flag: the "sandbox spec must specify cluster" check
now happens before authentication rather than after, so a spec missing a cluster reports
that instead of an auth failure. Existing valid invocations are unaffected.

Test plan

  • go test ./internal/...
  • --dry-run=client renders --set substitutions and never builds an API client,
    so it cannot be reaching the network
  • Rendered output re-renders to itself byte for byte
  • Local validation reports a missing cluster, an unknown spec field, and an
    unexpanded @{var}
  • --dry-run=server is rejected with a message pointing at the follow-up
  • Reviewer sanity check: apply -f <your template> --dry-run=client against a real
    template, with and without being logged in

Getting the spec a template renders to meant applying the sandbox, so CI
had no way to show what it was about to do and no way to check a template
without creating something. Rendering already happens locally; it just ran
after authentication, so this moves it before and prints the result.

The output is a spec rather than a report, so it can be diffed and passed
straight back to -f. A test pins that, since a caller that renders in one
step and applies in another depends on it.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Risk: medium. Approved this PR; no reviewers were assigned. Cursor Bugbot and Cursor Security Agent were not running, and the remaining signals did not require human review.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Approver

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant