Skip to content

docs(plugin-authoring): never test an Output's zero-ness via value methods - #574

Open
smecsia wants to merge 3 commits into
mainfrom
feat/plugin-authoring-output-zero-ness
Open

smecsia wants to merge 3 commits into
mainfrom
feat/plugin-authoring-output-zero-ness

Conversation

@smecsia

@smecsia smecsia commented Nov 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Add gotcha §6.7 to the plugin-authoring guide: pulumi.StringOutput's
value methods like ElementType() return a constant on both the
zero-value struct and a fully-resolved one. Any isXSet-style predicate
that calls a value method is unconditionally true.

Why

Live-caught on kubernetes-caddy (ee4afc8, merged with #563): a
value-method predicate silently discarded caddyfilePrefix +
trustedProxies 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:

The only field that distinguishes a zero Output is the embedded
pointer. Test out.OutputState != nil, never a method.

What

  • §6.7 added to §6 "The gotchas" with:
    • Anti-pattern code first (don't call a value method on the Output)
    • Correct pattern second (test out.OutputState != nil)
    • Preferred shape third: check the SOURCE value if in scope, not the
      Output — immune to this failure mode. Comes from the
      scCoreRegistrar.NewRecord fix in sc-plugin-gcp 2026-11-08 where
      the source string was in scope and record.Value != \"\" is both
      clearer and correct.
    • Incident citation last so a future reader diagnosing a similar
      shape can jump straight to the fix.

Credit

Peer flagged + handed off during sc-plugin-gcp phase-5 session
2026-11-08; the generalisation above "predicate on an Output" to "any
value method on struct-with-embedded-pointer" is their framing.

Test plan

  • File renders cleanly (no new section breaking the §6 structure,
    TOC unchanged since mdbook doesn't auto-TOC headings here)
  • Section follows the same shape as §6.5-6.6: brief, anti-pattern
    • correct pattern + incident citation

…thods

Live-caught on `kubernetes-caddy`'s `isPulumiOutputSet` predicate
(ee4afc8, 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 <smecsia@gmail.com>
@github-actions

github-actions Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

Semgrep Scan Results

Repository: api | Commit: f453a77

Check Status Details
⚠️ Semgrep Warning 11 warning(s), 15 total

Scanned at 2026-11-08 18:21 UTC

@github-actions

github-actions Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

Security Scan Results

Repository: api | Commit: f453a77

Check Status Details
✅ Secret Scan Pass No secrets detected
✅ Dependencies (Trivy) Pass 1 total (no critical/high)
✅ Dependencies (Grype) Pass 1 total (no critical/high)
📦 SBOM Generated 517 components (CycloneDX)

Scanned at 2026-11-08 18:23 UTC

universe-ops
universe-ops previously approved these changes Nov 8, 2026
@universe-ops
universe-ops enabled auto-merge (squash) November 8, 2026 18:14
baasforger
baasforger previously approved these changes Nov 8, 2026
…ives up the resource graph

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 '<field>' <file>` 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 <smecsia@gmail.com>
@universe-ops
universe-ops dismissed stale reviews from baasforger and themself via 57e5470 November 8, 2026 18:20
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.

3 participants