Repository navigation
fix(pulumi): stop the no-destroy guard refusing every vm/k3s re-provision - #570
Merged
Merged
Conversation
…sion
SC nonces its remote commands on purpose: every install, readiness gate
and remote read carries the run timestamp in `triggers` via
TriggersForGuardedChange / TriggersForRead, so a host whose k3s was
stopped out of band is repaired instead of being reported as "no
changes". A replace on every run is therefore the normal shape of a vm
or k3s provision — and since the destructive-replace guard landed, it is
also an outright refusal: the second `sc provision` of a stack aborts
before touching anything.
Live-caught 2026-11-08 on a 3-node k3s stand. Nine resources flagged
(six k3s commands, three per-host readiness gates), every one of them a
nonce-only trigger diff, and the engine reported `keys=[]` — so the
guard's own explanation ("a changed REPLACE-forcing input") did not even
apply to what it was refusing.
The exemption is per-RESOURCE, not per-type, so it cannot go on the
safe-to-replace allowlist: a remote command carrying a `delete` script
runs that script against a live machine when it is replaced
(pulumi-command replaces as create-then-delete), and the k3s uninstall
and the vm ingress teardown are exactly that shape. The question is
whether this command has a delete, and the answer is in its inputs.
Also stop rendering `keys=[]` under a sentence blaming a changed input;
point at `triggers` instead. The grep-stable message prefix is unchanged.
Signed-off-by: Ilya Sadykov <smecsia@gmail.com>
smecsia
requested review from
Cre-eD,
Laboratory and
universe-ops
as code owners
November 8, 2026 13:51
Semgrep Scan ResultsRepository:
Scanned at 2026-11-08 13:51 UTC |
Security Scan ResultsRepository:
Scanned at 2026-11-08 13:52 UTC |
smecsia
enabled auto-merge (squash)
November 8, 2026 15:36
universe-ops
approved these changes
Nov 8, 2026
baasforger
approved these changes
Nov 8, 2026
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
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.
The bug
Since the destructive-replace guard landed (#556), the second
sc provisionof any vm or k3s parent stack aborts before touching anything.SC nonces its remote commands on purpose: every install, readiness gate and remote read carries the run timestamp in
triggersviapulumiVm.TriggersForGuardedChange/TriggersForRead, so a host whose k3s was stopped out of band is repaired rather than reported as "no changes" (the scripts are idempotent and carry their own args-marker guard — k3s's installer answers an unchanged config withalready running as k3s with the requested configuration). A replace on every run is therefore the normal shape of a vm/k3s provision, andisDestructiveReplaceflags anyOpReplaceon a resource with a live id unless its type is allowlisted.Live-caught 2026-11-08 on a 3-node k3s stand:
Six k3s commands and three per-host readiness gates, every one a nonce-only
triggersdiff. The engine reportedkeys=[]— so the guard's own explanation ("a changed REPLACE-forcing input (zone id, region, immutable name, …)") did not even apply to what it was refusing, which is its own small bug and is also fixed here.Why not the safe-to-replace allowlist
api.safeToReplaceTypesis per-TYPE, andcommand:remote:Commandis not uniformly safe. A command carrying adeletescript runs that script against a live machine when it is replaced, because pulumi-command replaces as create-then-delete. SC has exactly that shape in two places — the k3s uninstall (k3s/cluster.go'suninstallOnDestroy) and the vm ingress teardown (vm/caddy.go) — and both are deliberately registered with no triggers so that nothing but a destroy runs them. An input-driven replace of one of those (a changed connection address, a changed script) is a real teardown and is what the guard is for.So the question is per-RESOURCE, and the answer is in the inputs:
isStatelessRemoteCommandexempts a remote command only when neither the old nor the new state declares a non-emptydelete.Verified live, both directions
Against the same 3-node k3s stack, no
SC_ALLOW_DESTRUCTIVE_REPLACE:exit=0,{"replace":9,"same":6}, zero guard hits;StepEventStateMetadata.Inputson these events (inputKeys=6 hasDelete=false keys=[]), so the exemption is precise rather than degenerate;*-k3s-uninstallresources storedeleteas a plain 230-char string in their inputs, sohasCommandDeletesees it and they stay guarded.The chosen failure direction is documented in the code: if inputs ever become unreadable, a remote command becomes exempt rather than wedging every provision — its id names no cloud object (it is a random suffix on the resource name), so the guard was never protecting state here, only the delete script.
Tests
go test ./pkg/clouds/pulumi/... -count=1green. Added:TestIsDestructiveReplace_OpCoverage— the nonce-triggered command (safe) and the delete-carrying uninstall (guarded);TestIsStatelessRemoteCommand— wrong type, no delete key, empty/whitespace delete, delete on the new side only, non-string (secret placeholder) delete, nil states;TestDestructiveReplaceError_NamesTriggersWhenNoKeys— no morekeys=[], and the grep-stable message prefix is unchanged.