From 288b2390b7540c8c48971de3b2546ccb8c5713d1 Mon Sep 17 00:00:00 2001 From: universe-ops Date: Sun, 8 Nov 2026 20:22:53 +0300 Subject: [PATCH 1/3] docs(plugin-authoring): never test an Output's zero-ness via value methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live-caught on `kubernetes-caddy`'s `isPulumiOutputSet` predicate (ee4afc84, merged with #563): `pulumi.StringOutput`'s `ElementType()` is a value method returning a constant, so it answers identically on a zero-value struct and a fully-resolved one. The predicate was unconditionally true; the GKE-only branch always won; `caddyfilePrefix` + `trustedProxies` were accepted, validated and silently discarded on every non-GKE cluster. The generalisation past `isXSet`: `pulumi.StringOutput` is `struct{ *OutputState }`, and any value method on a struct-with-embedded- pointer has this property. The rule that catches the whole class is "the only field that distinguishes a zero Output is the embedded pointer — test `out.OutputState != nil`, never a method". Also include the preferred shape: check the SOURCE value, not the Output derived from it, when the source is in scope at the call site (`if record.Value != "" { ... }`). Comes from the `scCoreRegistrar. NewRecord` fix in sc-plugin-gcp 2026-11-08 where the source string was in scope and the predicate on it is both clearer and immune to this failure mode. Section 6.7 of the Gotchas chapter. Placement matches the other "this was live-caught in prod" gotchas — brief, with the anti- pattern code first, correct pattern second, source-preferred form third, and the incident citation last so a reader diagnosing a similar shape can jump straight to the fix. Signed-off-by: Ilya Sadykov --- docs/docs/guides/plugin-authoring.md | 57 ++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/docs/docs/guides/plugin-authoring.md b/docs/docs/guides/plugin-authoring.md index 5200e6b5..9bcb8024 100644 --- a/docs/docs/guides/plugin-authoring.md +++ b/docs/docs/guides/plugin-authoring.md @@ -624,6 +624,63 @@ list. **Copy the one from the repo you're linting**, not the one you remember. Running `gci` with the wrong section list is a lint-only diff — `gofmt` doesn't catch it — and it fails CI from an operator-invisible place. +### 6.7 Never test an Output's zero-ness via its value methods + +`pulumi.StringOutput` is a struct with a single embedded pointer field — +`struct { *OutputState }`. `ElementType()` is a *value* method that +returns the constant `reflect.TypeOf("")`. It answers identically on a +zero-value struct and a fully-resolved one, because the method can't +see whether the embedded pointer was ever assigned. **Any value method +on a struct-with-embedded-pointer has this property**, so the rule that +catches the whole class is: + +> The only field that distinguishes a zero Output is the embedded +> pointer. Test `out.OutputState != nil`, never a method. + +The anti-pattern — don't: + +```go +// isPulumiOutputSet tests a value method that returns a constant for ALL instances of +// the type — zero-value or populated. The predicate reads correctly but is +// unconditionally true, and the "set" branch always wins. +func isPulumiOutputSet(out pulumi.StringOutput) bool { + return out.ElementType() != nil +} +``` + +The correct shape — test the embedded pointer: + +```go +// out.OutputState is the only field that distinguishes a zero Output +// from a populated one. The zero value is `pulumi.StringOutput{}` with +// *OutputState == nil. +func isOutputSet(out pulumi.StringOutput) bool { + return out.OutputState != nil +} +``` + +The sharper framing: **check the source, not the output.** When the +value comes from a plain string you already hold in scope, the +predicate on the source is always clearer and never has this failure +mode: + +```go +// Preferred: predicate on the source string, not on an Output derived from it. +// Use this when the source value is in scope at the call site. +var valueOut pulumi.StringOutput = record.ValueOut +if record.Value != "" { + valueOut = pulumi.String(record.Value).ToStringOutput() +} +``` + +Live-caught (api `ee4afc84`, 2026-11-08): a `kubernetes-caddy` driver's +`isXSet`-over-output predicate was unconditionally true, so the +GKE-only branch always won and `caddyfilePrefix` + `trustedProxies` +were accepted, validated and **silently discarded** on every non-GKE +cluster. The fleet-wide lesson is the first framing above — the second +form (check the source) is the preferred shape at every call site +where the source is in scope. + --- ## 7. Prior art From 57e547067acfec7f8f5253dbb36fe251d01903aa Mon Sep 17 00:00:00 2001 From: universe-ops Date: Sun, 8 Nov 2026 21:20:24 +0300 Subject: [PATCH 2/3] =?UTF-8?q?docs(plugin-authoring):=20=C2=A76.8=20?= =?UTF-8?q?=E2=80=94=20a=20helper=20that=20bypasses=20the=20framework=20gi?= =?UTF-8?q?ves=20up=20the=20resource=20graph?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sibling trap to §6.7 (just added in this PR). Different shape, same class — a surface that looks like it carries intent, but the intent never reaches the engine. A helper that mutates state inside a bare `sdk.All(...).ApplyT(...)` side effect — no `RegisterResource` — is outside the Pulumi graph. Any `ResourceOption` it accepts goes nowhere. `sdk.DependsOn` on the call site is documentation-only. The two-second check: `grep -n '' ` returning only the declaration. The decl without a read is the whole fingerprint. Live-caught on the sc-ru-clouds session's Caddy-edge batch commits 1-2 (2026-11-08): `PatchDeployment`'s `Opts []sdk.ResourceOption` was dead for a year. A Caddy pod rolled 9 s before the Service annotation it was rolled for landed (pod template 16:16:30, annotation 16:16:39) — the `sdk.DependsOn([]sdk.Resource{svc})` two call sites passed through was ignored. Fix was to compute the hash from the Service's applied annotation so `sdk.ToStringMapOutput`'s own output dependency provides the ordering. Credit: sc-ru-clouds found the root cause + generalised the trap ("a helper that bypasses the framework gives up the resource graph"). This section is their framing, verbatim where it matters. Section layout matches §6.7: anti-pattern + correct-shape-depends-on- what-the-helper-does + incident citation. Rule at the end states the generalisable form so a future reader diagnosing a similar shape knows what to look for. Signed-off-by: Ilya Sadykov --- docs/docs/guides/plugin-authoring.md | 80 ++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/docs/docs/guides/plugin-authoring.md b/docs/docs/guides/plugin-authoring.md index 9bcb8024..7caef4a8 100644 --- a/docs/docs/guides/plugin-authoring.md +++ b/docs/docs/guides/plugin-authoring.md @@ -681,6 +681,86 @@ cluster. The fleet-wide lesson is the first framing above — the second form (check the source) is the preferred shape at every call site where the source is in scope. +### 6.8 A helper that bypasses the framework gives up the resource graph + +The sibling trap to §6.7: a surface that *looks* like it carries +ordering intent (`Opts []sdk.ResourceOption`) but never reaches the +Pulumi engine silently discards `sdk.DependsOn`, `sdk.Provider`, every +other `ResourceOption` the caller passed. Ordering becomes fiction — +the code reads correctly, a reviewer sees `sdk.DependsOn([]sdk.Resource{x})` +and infers "waits for x", but at runtime the apply fires whenever its +own inputs resolve, in whatever order the engine happens to schedule. + +The root cause is architectural: only a `RegisterResource` call +attaches a resource (and its `ResourceOption`s) to the Pulumi graph. +A helper that mutates state inside a bare `sdk.All(...).ApplyT(...)` +side effect — no `RegisterResource` — is OUTSIDE the graph. Any +`ResourceOption` the helper accepts goes nowhere. + +The two-second check: **`grep -n '' ` returning only the +declaration.** If the only occurrence of `Opts` in a helper's file is +the struct field definition, the helper never used it — the +`ResourceOption`s travelled from the call site to a dead end. + +The anti-pattern — don't: + +```go +// DeploymentPatchArgs carries options the caller believes order the patch. +// The helper below does not call RegisterResource; the Opts never leave +// this struct. DependsOn here is a documentation-only annotation. +type DeploymentPatchArgs struct { + ... + Opts []sdk.ResourceOption +} + +func PatchDeployment(ctx *sdk.Context, args DeploymentPatchArgs) error { + return sdk.All(args.Service.Metadata).ApplyTWithContext( + ctx.Context(), + func(_ context.Context, all []any) error { + // mutate k8s state here; no RegisterResource call → outside the graph + return doPatch(all[0].(metav1.ObjectMeta)) + }, + ).(sdk.AnyOutput).Result() +} + +// Call site — the DependsOn is a lie. +err := PatchDeployment(ctx, DeploymentPatchArgs{ + Opts: []sdk.ResourceOption{sdk.DependsOn([]sdk.Resource{svc})}, + ... +}) +``` + +The correct shape depends on what the helper is actually doing: + +- **If the helper registers a resource**: pass `Opts` through to + `RegisterResource` (or to a pulumi SDK call that does). This is the + common case — most helpers *should* register resources, and `Opts` + reaches the engine naturally. +- **If the helper is a bare `ApplyT` side effect** (like a patch + computed from another resource's applied state): drop the + `ResourceOption` field entirely and order the apply via its input + outputs. `sdk.ToStringMapOutput` and similar `*Output` types + preserve the source's output dependencies — an `ApplyT` on an + output cannot run until the source has resolved, which is the only + ordering mechanism an apply-driven helper has. + +Live-caught (Caddy-edge batch commits 1-2, 2026-11-08): `PatchDeployment` +(`pkg/clouds/pulumi/kubernetes/deployment_patch.go`) carried an unused +`Opts []sdk.ResourceOption` for over a year. Two call sites +(`kube_run.go`, `gcp/gke_autopilot_stack.go`) passed +`sdk.DependsOn([]sdk.Resource{sc.Service})` through it. The ordering +never fired: a Caddy pod rolled 9s before the Service annotation it +was rolled for became visible (pod template 16:16:30, annotation +16:16:39). Fix was to compute the pod-template hash from the Service's +applied annotation — `sdk.ToStringMapOutput` preserves the +annotation's output dependency, so the patch's apply blocks on it. +Dead `Opts` field removed; `PatchDeployment`'s doc comment now states +that `DependsOn` cannot order it and what to do instead. + +Generalisable rule: **a helper that doesn't call `RegisterResource` +cannot order work via `ResourceOption`s. Order via output +dependencies on its inputs, or make the helper register a resource.** + --- ## 7. Prior art From 3b75845f2ddec751595fd3ac85875aba9858f855 Mon Sep 17 00:00:00 2001 From: universe-ops Date: Sun, 8 Nov 2026 22:17:04 +0300 Subject: [PATCH 3/3] =?UTF-8?q?docs(plugin-authoring):=20=C2=A76.7=20?= =?UTF-8?q?=E2=80=94=20pulumi=20SDK=20citation=20+=20UNKNOWN-await=20empir?= =?UTF-8?q?ical=20probe?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two amendments on top of §6.7: 1. **The SDK citation.** `ElementType()` is a method on the value type `StringOutput` returning a package-level `reflect.Type`, so `out.ElementType() != nil` is a constant true for every StringOutput including a zero value. Full citation: `pulumi/sdk/v3@v3.245.0/go/pulumi/types_builtins.go:9471`: func (StringOutput) ElementType() reflect.Type { return stringType } A reader hitting the rule for the first time wants to see where the constant lives, not just trust the rule. 2. **The UNKNOWN-await empirical probe.** The predicate-reader failure mode is already in the body (wrong-branch-selected); the sibling failure mode is an awaiter reading the zero-value Output as UNKNOWN rather than empty. Probed live against pulumi SDK v3.245.0: value= known=false secret=false err= A Caddy pod handed `CADDYFILE_PREFIX=` never comes up, and this is why the bug is invisible in preview: previews don't distinguish "will resolve to known" from "will never resolve". Credit: both citations come from sc-ru-clouds' direct SDK + live probe during the Caddy-edge batch work, 2026-11-08. The §6.7 I shipped has the rule right; these amendments make it teach-able. Signed-off-by: Ilya Sadykov --- docs/docs/guides/plugin-authoring.md | 36 ++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/docs/docs/guides/plugin-authoring.md b/docs/docs/guides/plugin-authoring.md index 7caef4a8..672ad42a 100644 --- a/docs/docs/guides/plugin-authoring.md +++ b/docs/docs/guides/plugin-authoring.md @@ -628,11 +628,20 @@ remember. Running `gci` with the wrong section list is a lint-only diff — `pulumi.StringOutput` is a struct with a single embedded pointer field — `struct { *OutputState }`. `ElementType()` is a *value* method that -returns the constant `reflect.TypeOf("")`. It answers identically on a -zero-value struct and a fully-resolved one, because the method can't -see whether the embedded pointer was ever assigned. **Any value method -on a struct-with-embedded-pointer has this property**, so the rule that -catches the whole class is: +returns a package-level `reflect.Type` constant — the full citation is +`pulumi/sdk/v3@v3.245.0/go/pulumi/types_builtins.go:9471`: + +```go +func (StringOutput) ElementType() reflect.Type { return stringType } +``` + +So `out.ElementType() != nil` is a constant true for every +`StringOutput` in existence, including `var o sdk.StringOutput`. The +method answers identically on a zero-value struct and a fully- +resolved one, because the method can't see whether the embedded +pointer was ever assigned. **Any value method on a struct-with- +embedded-pointer has this property**, so the rule that catches the +whole class is: > The only field that distinguishes a zero Output is the embedded > pointer. Test `out.OutputState != nil`, never a method. @@ -681,6 +690,23 @@ cluster. The fleet-wide lesson is the first framing above — the second form (check the source) is the preferred shape at every call site where the source is in scope. +**Why this is invisible in preview.** A zero-value `sdk.StringOutput` +doesn't await as the empty string — it awaits as UNKNOWN. Probed live +against the shipped SDK (pulumi/sdk/v3@v3.245.0): + +``` +value= known=false secret=false err= +``` + +So the downstream failure modes differ by who consumes the zero +Output: a predicate-reader gets the wrong branch selected (the +`caddyfilePrefix` discard above); an awaiter gets UNKNOWN forever, +which lands on the wire as an unresolved placeholder — a Caddy pod +handed `CADDYFILE_PREFIX=` never comes up. Previews don't +distinguish "will resolve to known" from "will never resolve", so +neither shape surfaces until apply-time, which is why these bugs can +live for a year before anyone measures. + ### 6.8 A helper that bypasses the framework gives up the resource graph The sibling trap to §6.7: a surface that *looks* like it carries