diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 3244deb5b..26d85fc8e 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -14,6 +14,9 @@ permissions: env: GOPRIVATE: github.com/GoCodeAlone/* GONOSUMCHECK: github.com/GoCodeAlone/* + # See ci.yml for rationale. Force in-memory diffcache in CI per the + # T3.5 rev3 lifecycle constraint. + WFCTL_DIFFCACHE: ":memory:" jobs: benchmark: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dcb275b51..6e3aa8831 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,6 +13,12 @@ permissions: env: GOPRIVATE: github.com/GoCodeAlone/* GONOSUMCHECK: github.com/GoCodeAlone/* + # CI runners are ephemeral; the iac/diffcache filesystem cache would + # cold-miss every job and waste a few ms on disk I/O. Force in-memory + # mode per the T3.5 rev3 lifecycle constraint — no filesystem writes + # in containerized runners. Operators running wfctl locally see the + # default filesystem cache at ~/.cache/wfctl/diff/. + WFCTL_DIFFCACHE: ":memory:" jobs: test: diff --git a/.github/workflows/dependency-update.yml b/.github/workflows/dependency-update.yml index 73c3d9385..407a0f1d3 100644 --- a/.github/workflows/dependency-update.yml +++ b/.github/workflows/dependency-update.yml @@ -10,6 +10,12 @@ on: - cron: '0 9 * * 1' workflow_dispatch: +env: + # See ci.yml for rationale. Force in-memory diffcache in CI per the + # T3.5 rev3 lifecycle constraint — no filesystem writes from this + # workflow's `go test ./...` step. + WFCTL_DIFFCACHE: ":memory:" + jobs: update-dependencies: name: Update Go Dependencies diff --git a/.github/workflows/pre-release.yml b/.github/workflows/pre-release.yml index 0f0b41ffb..cb913cb5e 100644 --- a/.github/workflows/pre-release.yml +++ b/.github/workflows/pre-release.yml @@ -15,6 +15,9 @@ permissions: env: GOPRIVATE: github.com/GoCodeAlone/* GONOSUMCHECK: github.com/GoCodeAlone/* + # See ci.yml for rationale. Force in-memory diffcache in CI per the + # T3.5 rev3 lifecycle constraint. + WFCTL_DIFFCACHE: ":memory:" jobs: test: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 66b2d5ab9..db345899f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -22,6 +22,9 @@ env: GOPRIVATE: github.com/GoCodeAlone/* GONOSUMCHECK: github.com/GoCodeAlone/* TAG_NAME: ${{ inputs.tag_name || github.ref_name }} + # See ci.yml for rationale. Force in-memory diffcache in CI per the + # T3.5 rev3 lifecycle constraint. + WFCTL_DIFFCACHE: ":memory:" jobs: test: diff --git a/cmd/wfctl/infra_apply.go b/cmd/wfctl/infra_apply.go index e1d7dd74c..94a32b93e 100644 --- a/cmd/wfctl/infra_apply.go +++ b/cmd/wfctl/infra_apply.go @@ -14,10 +14,32 @@ import ( "time" "github.com/GoCodeAlone/workflow/config" + "github.com/GoCodeAlone/workflow/iac/inputsnapshot" "github.com/GoCodeAlone/workflow/interfaces" "github.com/GoCodeAlone/workflow/platform" ) +// printDriftReportIfAny writes the canonical FormatStaleError output to w +// when result.InputDriftReport is non-empty. Safe to call when result is +// nil or InputDriftReport is empty/nil — both yield a no-op so callers +// don't need to defensively check the field before calling. +// +// Wired in by W-3a/T3.1.5 as a standalone helper; the actual call site in +// applyWithProviderAndStore (or its successor) lands when W-3b/T3.7 +// switches the in-process apply path through wfctlhelpers.ApplyPlan for +// v2 plugins. Until then this helper is exercised solely by the +// in-process drift test, NOT yet by any production caller — preserving +// the W-3a "zero runtime change for v1 plugins" invariant. +func printDriftReportIfAny(w io.Writer, result *interfaces.ApplyResult) { + if result == nil || len(result.InputDriftReport) == 0 { + return + } + // FormatStaleError emits the multi-line "plan stale: N input(s) changed" + // header + per-key fingerprint diff + trailing hint. Println adds the + // terminating newline so the next stderr line is not glued to the hint. + fmt.Fprintln(w, inputsnapshot.FormatStaleError(result.InputDriftReport)) +} + // infraApplyTroubleshootTimeout is the budget for a Troubleshoot call when // infra apply fails. Kept separate so tests can override it. var infraApplyTroubleshootTimeout = 30 * time.Second diff --git a/cmd/wfctl/infra_apply_in_process_test.go b/cmd/wfctl/infra_apply_in_process_test.go new file mode 100644 index 000000000..d45e95fd3 --- /dev/null +++ b/cmd/wfctl/infra_apply_in_process_test.go @@ -0,0 +1,127 @@ +package main + +import ( + "bytes" + "context" + "strings" + "testing" + + "github.com/GoCodeAlone/workflow/iac/wfctlhelpers" + "github.com/GoCodeAlone/workflow/interfaces" +) + +// fingerprintForTest is provided by infra_apply_plan_test.go in this +// package — it delegates to inputsnapshot.Compute so the production +// fingerprint algorithm is always used. Reused here to keep the +// in-process test consistent with the persisted-plan test. + +// inProcessFakeProvider satisfies interfaces.IaCProvider with no-op +// methods — TestApply_InProcess_PlanStaleDiagnostic_NamesChangedKeys +// exercises wfctlhelpers.ApplyPlan's drift postcondition independently +// of any per-action driver behavior. +type inProcessFakeProvider struct{} + +func (inProcessFakeProvider) Name() string { return "in-process-fake" } +func (inProcessFakeProvider) Version() string { return "0.0.0" } +func (inProcessFakeProvider) Initialize(_ context.Context, _ map[string]any) error { return nil } +func (inProcessFakeProvider) Capabilities() []interfaces.IaCCapabilityDeclaration { return nil } +func (inProcessFakeProvider) Plan(_ context.Context, _ []interfaces.ResourceSpec, _ []interfaces.ResourceState) (*interfaces.IaCPlan, error) { + return &interfaces.IaCPlan{}, nil +} +func (inProcessFakeProvider) Apply(_ context.Context, _ *interfaces.IaCPlan) (*interfaces.ApplyResult, error) { + return &interfaces.ApplyResult{}, nil +} +func (inProcessFakeProvider) Destroy(_ context.Context, _ []interfaces.ResourceRef) (*interfaces.DestroyResult, error) { + return nil, nil +} +func (inProcessFakeProvider) Status(_ context.Context, _ []interfaces.ResourceRef) ([]interfaces.ResourceStatus, error) { + return nil, nil +} +func (inProcessFakeProvider) DetectDrift(_ context.Context, _ []interfaces.ResourceRef) ([]interfaces.DriftResult, error) { + return nil, nil +} +func (inProcessFakeProvider) Import(_ context.Context, _ string, _ string) (*interfaces.ResourceState, error) { + return nil, nil +} +func (inProcessFakeProvider) ResolveSizing(_ string, _ interfaces.Size, _ *interfaces.ResourceHints) (*interfaces.ProviderSizing, error) { + return nil, nil +} +func (inProcessFakeProvider) ResourceDriver(_ string) (interfaces.ResourceDriver, error) { + return nil, nil +} +func (inProcessFakeProvider) SupportedCanonicalKeys() []string { return nil } +func (inProcessFakeProvider) BootstrapStateBackend(_ context.Context, _ map[string]any) (*interfaces.BootstrapResult, error) { + return nil, nil +} +func (inProcessFakeProvider) Close() error { return nil } + +// TestApply_InProcess_PlanStaleDiagnostic_NamesChangedKeys validates the +// full T3.1.5 chain: plan-time env value → wfctlhelpers.ApplyPlan +// captures InitialInputSnapshot at entry, then the deferred postcondition +// computes drift against the apply-time snapshot, populating +// result.InputDriftReport. cmd/wfctl/printDriftReportIfAny then formats +// the report onto stderr in the canonical FormatStaleError shape. +// +// This is the in-process counterpart to the persisted-`--plan` path +// covered by cmd/wfctl/infra.go (W-1/T1.5). Once W-3b/T3.7 wires +// wfctlhelpers.ApplyPlan into the in-process apply call site, this test +// exercises exactly the path operators run during `wfctl infra apply` +// against a v2-conformant IaCProvider plugin. +func TestApply_InProcess_PlanStaleDiagnostic_NamesChangedKeys(t *testing.T) { + const varName = "WFCTL_T315_INPROC_PASSWORD" + planFP := fingerprintForTest("old-value") + t.Setenv(varName, "new-value") // post-plan env value differs from plan-time fingerprint + plan := &interfaces.IaCPlan{ + InputSnapshot: map[string]string{varName: planFP}, + } + result, err := wfctlhelpers.ApplyPlan(context.Background(), inProcessFakeProvider{}, plan) + if err != nil { + t.Fatalf("ApplyPlan returned top-level error: %v", err) + } + // Postcondition assertion: exactly one entry naming the changed key. + if len(result.InputDriftReport) != 1 { + t.Fatalf("expected 1 drift entry; got %d (%+v)", + len(result.InputDriftReport), result.InputDriftReport) + } + if got := result.InputDriftReport[0].Name; got != varName { + t.Errorf("expected drift on %q; got %q", varName, got) + } + + // cmd/wfctl wiring assertion: printDriftReportIfAny formats the + // report onto the supplied writer in the canonical shape. + var buf bytes.Buffer + printDriftReportIfAny(&buf, result) + out := buf.String() + if !strings.Contains(out, "plan stale:") { + t.Errorf("expected canonical stale-error header in output; got %q", out) + } + if !strings.Contains(out, varName) { + t.Errorf("expected drift output to name %q; got %q", varName, out) + } + if !strings.Contains(out, "hint: ensure all env vars referenced") { + t.Errorf("expected canonical hint line in output; got %q", out) + } +} + +// TestPrintDriftReportIfAny_NoOpOnEmpty locks the no-op contract so +// callers don't need to guard the call: nil result, nil report, and +// empty-but-non-nil report all produce zero output. +func TestPrintDriftReportIfAny_NoOpOnEmpty(t *testing.T) { + cases := []struct { + name string + result *interfaces.ApplyResult + }{ + {"nil-result", nil}, + {"nil-report", &interfaces.ApplyResult{}}, + {"empty-report", &interfaces.ApplyResult{InputDriftReport: []interfaces.DriftEntry{}}}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + var buf bytes.Buffer + printDriftReportIfAny(&buf, c.result) + if buf.Len() != 0 { + t.Errorf("expected no output for %s; got %q", c.name, buf.String()) + } + }) + } +} diff --git a/docs/WFCTL.md b/docs/WFCTL.md index 93063e910..ffb432bca 100644 --- a/docs/WFCTL.md +++ b/docs/WFCTL.md @@ -2221,3 +2221,51 @@ deploy: runtime: minikube # minikube | kind | docker-desktop | k3d | remote registry: ghcr.io/myorg ``` + +--- + +## Diff Cache + +`wfctl infra plan` consults a per-resource diff cache so a Plan re-run against unchanged inputs can skip the (sometimes network-expensive) provider-side `Diff` call. The cache lives at the package `iac/diffcache/` and is keyed by `(plugin-version, resource-type, provider-id, sha256(config), sha256(outputs))`. + +**The cache is purely an amortization optimization, NOT a correctness mechanism.** Apply paths remain correct on a 100 % miss rate, which is exactly what CI sees on every fresh runner. Workflows MUST NOT depend on a cache hit for correctness or reproducibility. + +### Backend selection (`WFCTL_DIFFCACHE`) + +The env var `WFCTL_DIFFCACHE` selects the cache backend at process start: + +| Value | Backend | +|---------------|------------------------------------------------------------------------------------------------| +| `disabled` | No-op cache: every `Get` misses; `Put` is a no-op. Use this for fully-deterministic timing or to bypass any disk state. | +| `:memory:` | In-memory cache that lives only for the current process. **CI workflows in this repo set this explicitly** so containerized runners never write cache data to disk. | +| _(unset)_ or anything else | Filesystem cache rooted at `~/.cache/wfctl/diff/.json` (honors `XDG_CACHE_HOME` on Linux via `os.UserCacheDir`). The default for operators running `wfctl` locally. | + +If the user cache directory cannot be resolved, the helper falls back to the in-memory cache automatically — so the caller always gets a working `Cache`. + +### Eviction (filesystem backend) + +The filesystem cache enforces an LRU eviction policy on each `Put`: + +- **By count:** at most 1024 entries. +- **By total size:** at most 64 MiB on disk. + +Whichever cap is exceeded first triggers the eviction pass. The pass evicts the oldest 10 % of entries (sorted by mtime; secondary sort by path for determinism) so amortized cost stays bounded. + +### Corruption recovery + +If a cache file fails to parse on `Get` (truncated, partial-write, JSON syntax error, or a `schemaVersion` mismatch from a wfctl downgrade), the cache silently deletes the corrupt file and returns a miss. No error is surfaced to the caller; a single info-level log line fires the first time corruption is observed in a process so operators have a breadcrumb without log spam. + +### Plugin downgrade + +`PluginVersion` is part of the cache key, so a plugin downgrade naturally invalidates entries (cache key miss). Old entries persist on disk until the LRU eviction reclaims them; the size cap above bounds the disk waste. + +### CI configuration + +Per the T3.5 rev3 lifecycle constraint, **all CI workflows in this repository set `WFCTL_DIFFCACHE=:memory:` at the workflow level** so containerized runners never write to disk: + +```yaml +env: + WFCTL_DIFFCACHE: ":memory:" +``` + +Files: `.github/workflows/ci.yml`, `benchmark.yml`, `pre-release.yml`, `release.yml`, `dependency-update.yml`. New workflow files that invoke `go test` or `wfctl` should add the same env var. diff --git a/iac/diffcache/cache.go b/iac/diffcache/cache.go new file mode 100644 index 000000000..ccc1e643f --- /dev/null +++ b/iac/diffcache/cache.go @@ -0,0 +1,208 @@ +// Package diffcache caches per-resource Diff results so a wfctl invocation +// that re-runs a Plan against unchanged inputs can skip the (sometimes +// network-expensive) provider-side Diff call. The cache is purely an +// amortization optimization, NOT a correctness mechanism — apply paths +// remain correct on a 100% miss rate (which is exactly what CI sees on +// every fresh runner). +// +// # Storage backends +// +// Cache selection is driven by the WFCTL_DIFFCACHE env var, resolved by +// [New]: +// +// - `disabled` → noop cache (every Get misses; Put is a no-op). Use +// this when an operator wants fully-deterministic Plan/Apply timing +// with no shared state across invocations. +// - `:memory:` → in-memory cache that lives only for the current +// process. CI workflows in this repo set WFCTL_DIFFCACHE=:memory: +// explicitly so containerized runners never write to disk. +// - any other value (or unset) → filesystem cache rooted at +// `~/.cache/wfctl/diff/`. +// +// # CI ephemerality (load-bearing) +// +// CI runners are ephemeral — each job starts with an empty cache. +// Workflow correctness MUST NOT depend on a cache hit. The diff cache +// is an operator-local performance optimization for repeated +// `wfctl infra plan` invocations against the same checkout. +// +// # Cache key +// +// The cache key is a [Key] tuple of (PluginVersion, Type, ProviderID, +// SHAConfig, SHAOutputs). Plugin downgrades naturally invalidate +// entries since PluginVersion is part of the key — old entries persist +// on disk until the LRU eviction reclaims them; the size cap (1024 +// entries / 64 MiB) bounds the disk waste. +// +// # Schema versioning +// +// Each cache file embeds [cacheSchemaVersion] in a JSON envelope. On +// Get, a mismatched version is treated identically to file corruption: +// the entry is silently evicted and the caller re-Diffs. +// +// # Known limitations +// +// Windows: the filesystem cache uses [os.Rename] for the atomic +// publish step on Put. On Windows, [os.Rename] fails when the +// destination already exists, so updating an existing cache entry +// will fail (the entry is treated as a write failure and the +// operator gets a cache miss on the next Get — correct, since +// apply does not depend on cache hits). A future improvement is to +// vendor github.com/google/renameio for cross-platform atomic +// rename; deferred until there's a Windows-supported wfctl use +// case. +// +// # T3.5 / W-3a status +// +// This package ships in W-3a. The consumer that wires it into +// platform.ComputePlan lands in W-3b/T3.6f. Until then, the cache is +// callable but never invoked from production code paths. +package diffcache + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/GoCodeAlone/workflow/interfaces" +) + +// cacheSchemaVersion is the on-disk envelope version. Bump on any +// breaking change to the JSON shape; old files will be silently +// evicted on the next Get (same code path as corruption recovery). +const cacheSchemaVersion = 1 + +// defaultMaxEntries is the LRU eviction trigger by count. +const defaultMaxEntries = 1024 + +// defaultMaxBytes is the LRU eviction trigger by total on-disk size +// (64 MiB). Whichever cap (entries or bytes) is exceeded first triggers +// the eviction pass. +const defaultMaxBytes = 64 * 1024 * 1024 + +// evictionFraction is the fraction of cache entries to evict in one +// over-cap pass. 0.10 = oldest 10%; matches rev2 lifecycle constraint. +const evictionFraction = 0.10 + +// Key tuples the inputs to a single Diff. Two Keys are equal iff every +// field matches; the canonical sha256 fingerprint of the key +// determines the on-disk filename. +// +// JSON tags on the fields are for log / transcript serialization +// only — cache keying uses NUL-separated string concatenation in +// [keyFingerprint], not JSON marshaling. A reader checking the +// fingerprint shape should follow the keyFingerprint code, not the +// tags. +type Key struct { + // PluginVersion is the plugin's name@version string, e.g., + // "do@v0.10.0". Plugin downgrades naturally invalidate cache + // entries via this field. + PluginVersion string `json:"plugin_version"` + // Type is the canonical resource type, e.g., "infra.vpc". + Type string `json:"type"` + // ProviderID is the resource's cloud-side identifier; empty for + // net-new resources. + ProviderID string `json:"provider_id"` + // SHAConfig is the sha256-hex of canonical-marshal(spec.Config). + SHAConfig string `json:"sha_config"` + // SHAOutputs is the sha256-hex of canonical-marshal(currentState.Outputs); + // empty for net-new resources. + SHAOutputs string `json:"sha_outputs"` +} + +// Cache is the diff-result cache. Implementations are safe for +// concurrent use (the backing fs cache uses os-level atomic file ops; +// the in-memory cache locks internally). +type Cache interface { + // Get returns the cached DiffResult for key. The boolean is true + // iff the entry was found and successfully decoded; corruption, + // schema-version mismatch, and missing entries all yield + // (zero-value, false). + Get(key Key) (interfaces.DiffResult, bool) + // Put stores result under key. Errors during Put (e.g., disk + // full, serialization failure) are silently swallowed because + // cache misses are correct — apply behavior must not depend on + // Put success. + Put(key Key, result interfaces.DiffResult) +} + +// New returns a Cache configured by the WFCTL_DIFFCACHE env var. +// +// - "disabled" → [NewNoop] +// - ":memory:" → [NewMemory] +// - default → [NewFilesystem] rooted at the user cache directory +// (typically `~/.cache/wfctl/diff/`). +// +// When the user cache directory cannot be resolved, falls back to the +// in-memory cache so the caller still gets a working Cache (the +// filesystem path being unavailable is the operator's hint that disk +// caching is off). +func New() Cache { + switch strings.TrimSpace(os.Getenv("WFCTL_DIFFCACHE")) { + case "disabled": + return NewNoop() + case ":memory:": + return NewMemory() + } + dir, err := userCacheDir() + if err != nil { + return NewMemory() + } + return NewFilesystem(dir) +} + +// NewNoop returns a Cache that always misses. Put is a no-op. +func NewNoop() Cache { return &noopCache{} } + +// noopCache is the disabled-cache implementation. +type noopCache struct{} + +func (noopCache) Get(_ Key) (interfaces.DiffResult, bool) { return interfaces.DiffResult{}, false } +func (noopCache) Put(_ Key, _ interfaces.DiffResult) { /* no-op */ } + +// userCacheDir returns the diff-cache root under the user cache +// directory. Resolution follows os.UserCacheDir conventions per +// platform: +// +// - Linux: $XDG_CACHE_HOME or $HOME/.cache (e.g., ~/.cache/wfctl/diff/). +// - macOS: $HOME/Library/Caches (e.g., ~/Library/Caches/wfctl/diff/). +// - Windows: %LocalAppData% (e.g., C:\Users\\AppData\Local\wfctl\diff\). +func userCacheDir() (string, error) { + base, err := os.UserCacheDir() + if err != nil { + return "", fmt.Errorf("resolve user cache dir: %w", err) + } + return filepath.Join(base, "wfctl", "diff"), nil +} + +// keyFingerprint returns the sha256-hex fingerprint of the canonical +// key serialization. Exported as a package-level function so tests can +// assert key-stability without coupling to the filesystem cache. +func keyFingerprint(k Key) string { + // Concatenate fields with a NUL separator so per-field collisions + // are impossible (any field containing NUL would have to be + // deliberately injected; cloud IDs and resource types don't). + var b strings.Builder + b.WriteString(k.PluginVersion) + b.WriteByte(0) + b.WriteString(k.Type) + b.WriteByte(0) + b.WriteString(k.ProviderID) + b.WriteByte(0) + b.WriteString(k.SHAConfig) + b.WriteByte(0) + b.WriteString(k.SHAOutputs) + sum := sha256.Sum256([]byte(b.String())) + return hex.EncodeToString(sum[:]) +} + +// envelope is the JSON shape persisted to disk and used by the +// in-memory cache to validate on Get. The schemaVersion field gates +// silent eviction on a future schema bump. +type envelope struct { + SchemaVersion int `json:"schemaVersion"` + Result interfaces.DiffResult `json:"result"` +} diff --git a/iac/diffcache/cache_concurrent_test.go b/iac/diffcache/cache_concurrent_test.go new file mode 100644 index 000000000..e8a35e8ff --- /dev/null +++ b/iac/diffcache/cache_concurrent_test.go @@ -0,0 +1,59 @@ +package diffcache + +import ( + "path/filepath" + "sync" + "testing" + + "github.com/GoCodeAlone/workflow/interfaces" +) + +// TestCache_ConcurrentSameKeyPut verifies that multiple goroutines +// calling Put with the same Key do not race on a shared temp filename. +// Pre-fix, both Puts wrote to `.json.tmp` and one would clobber +// the other's temp file mid-write, producing either a Rename failure +// or, worse, a half-written final file. With os.CreateTemp's per-call +// unique suffix, each Put has its own temp path; the final Rename is +// racy in the sense that one payload "wins," but both payloads are +// equivalent (same Key → same canonical input) so the outcome is +// deterministic from the caller's perspective. +// +// The test asserts: (a) no panic, (b) no leftover *.tmp files in the +// cache dir after all goroutines finish, (c) the final cache file is +// readable and decodes successfully. Run under -race to catch any +// shared-state mutation that slipped through. +func TestCache_ConcurrentSameKeyPut(t *testing.T) { + dir := t.TempDir() + c := NewFilesystem(dir).(*filesystemCache) + key := Key{Type: "infra.vpc", ProviderID: "vpc-abc"} + const goroutines = 20 + + var wg sync.WaitGroup + wg.Add(goroutines) + for range goroutines { + go func() { + defer wg.Done() + c.Put(key, interfaces.DiffResult{NeedsUpdate: true}) + }() + } + wg.Wait() + + // No leftover temp files: every Put either renamed successfully + // (no orphan) or failed and cleaned up via os.Remove. + tmps, err := filepath.Glob(filepath.Join(dir, "*.tmp")) + if err != nil { + t.Fatalf("glob tmp: %v", err) + } + if len(tmps) != 0 { + t.Errorf("expected 0 leftover *.tmp files; found %d: %v", len(tmps), tmps) + } + + // Final cache file is readable + decodes. + got, hit := c.Get(key) + if !hit { + t.Fatal("expected hit after concurrent Puts of the same key") + } + if !got.NeedsUpdate { + t.Errorf("Get returned wrong value after concurrent Puts: %+v", got) + } +} diff --git a/iac/diffcache/cache_corruption_test.go b/iac/diffcache/cache_corruption_test.go new file mode 100644 index 000000000..98009aa75 --- /dev/null +++ b/iac/diffcache/cache_corruption_test.go @@ -0,0 +1,107 @@ +package diffcache + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/GoCodeAlone/workflow/interfaces" +) + +// TestCache_CorruptionRecovery_TruncatedFile verifies that a partially- +// written or truncated cache file does not crash Get and is silently +// removed so the next Put succeeds. The contract: Get returns +// (_, false) on parse failure; the file is deleted from disk. +func TestCache_CorruptionRecovery_TruncatedFile(t *testing.T) { + dir := t.TempDir() + c := NewFilesystem(dir).(*filesystemCache) + key := Key{Type: "infra.vpc", ProviderID: "vpc-abc"} + c.Put(key, interfaces.DiffResult{NeedsUpdate: true}) + + // Truncate the cache file to simulate a partial write or crash mid-Put. + path := c.pathFor(key) + if err := os.WriteFile(path, []byte("{not val"), 0o644); err != nil { + t.Fatalf("failed to truncate cache file: %v", err) + } + + if _, hit := c.Get(key); hit { + t.Errorf("expected miss on truncated file; got hit") + } + // The corrupt file should be removed so a fresh Put can succeed. + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Errorf("corrupt cache file should be deleted; stat err=%v", err) + } + + // Re-Put + Get round-trip must work after corruption recovery. + c.Put(key, interfaces.DiffResult{NeedsReplace: true}) + got, hit := c.Get(key) + if !hit { + t.Fatal("expected hit after re-Put following corruption") + } + if !got.NeedsReplace { + t.Errorf("post-recovery Get returned wrong value: %+v", got) + } +} + +// TestCache_CorruptionRecovery_SchemaVersionMismatch verifies that +// cache files written under a different schema version are silently +// evicted on Get — same code path as JSON corruption since we treat +// both as "cache file we cannot use." Lock the contract so a future +// schema bump (cacheSchemaVersion = 2) keeps the silent-eviction +// behavior. +func TestCache_CorruptionRecovery_SchemaVersionMismatch(t *testing.T) { + dir := t.TempDir() + c := NewFilesystem(dir).(*filesystemCache) + key := Key{Type: "infra.vpc", ProviderID: "vpc-abc"} + + // Write a valid-JSON file with a schemaVersion that doesn't match + // the current cacheSchemaVersion constant. Use a value >> current + // so it can't accidentally match a future bump. + path := c.pathFor(key) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + envelope := map[string]any{ + "schemaVersion": 9999, + "result": map[string]any{"needs_update": true}, + } + data, _ := json.Marshal(envelope) + if err := os.WriteFile(path, data, 0o644); err != nil { + t.Fatal(err) + } + + if _, hit := c.Get(key); hit { + t.Errorf("expected miss on schema-version mismatch; got hit") + } + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Errorf("schema-mismatched cache file should be deleted; stat err=%v", err) + } +} + +// TestCache_CorruptionRecovery_GetReturnsNoErrorOnParseFailure proves +// the no-error contract: callers Get a (zero, false) tuple, never a +// surfaced error. Cache failures must NEVER abort apply. +func TestCache_CorruptionRecovery_GetReturnsNoErrorOnParseFailure(t *testing.T) { + dir := t.TempDir() + c := NewFilesystem(dir).(*filesystemCache) + key := Key{Type: "x"} + path := c.pathFor(key) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("\x00\x00not-json\x00\x00"), 0o644); err != nil { + t.Fatal(err) + } + // We can't directly assert "no error returned" via the Get + // signature, but we can lock that hit is false and the file is + // removed (the canonical recovery path). A panic would also fail + // this test, which is the implicit contract this case anchors. + got, hit := c.Get(key) + if hit { + t.Error("expected miss on garbage bytes") + } + if got.NeedsUpdate || got.NeedsReplace || len(got.Changes) > 0 { + t.Errorf("zero-value DiffResult on miss; got %+v", got) + } +} diff --git a/iac/diffcache/cache_eviction_test.go b/iac/diffcache/cache_eviction_test.go new file mode 100644 index 000000000..5d491d166 --- /dev/null +++ b/iac/diffcache/cache_eviction_test.go @@ -0,0 +1,163 @@ +package diffcache + +import ( + "fmt" + "path/filepath" + "testing" + "time" + + "github.com/GoCodeAlone/workflow/interfaces" +) + +// TestCache_LRUEvictionByCount verifies the LRU eviction trigger when +// the entry count exceeds maxEntries. Per the rev2 lifecycle constraint +// (10% per-overflow batch), the eviction is amortized — for the test +// we drive it with a small cap (10 entries; 10% = 1 evicted per +// over-cap Put) to keep the test fast. +// +// Mtime-resolution assumption: the 1ms inter-Put sleep relies on the +// underlying filesystem providing mtime granularity ≤ 1ms (true on +// Linux ext4/btrfs/xfs, macOS APFS, Windows NTFS — the CI matrix). +// Coarse-mtime filesystems (FAT32 with 2s, SMB shares with ~1s) would +// produce indistinguishable mtimes for adjacent Puts and the +// secondary path-sort (by sha256 hash of the cache key) would decide +// eviction order — uncorrelated with insertion order. The package +// intentionally does not support such filesystems. +func TestCache_LRUEvictionByCount(t *testing.T) { + dir := t.TempDir() + c := &filesystemCache{ + dir: dir, + maxEntries: 10, + maxBytes: defaultMaxBytes, // not the trigger here + } + // Fill to the cap. Each Put with a different key creates a new file. + // 1ms between Puts so mtimes are distinguishable on the supported + // filesystems (see godoc above). + for i := range 10 { + c.Put(Key{Type: fmt.Sprintf("k%d", i)}, interfaces.DiffResult{}) + time.Sleep(time.Millisecond) + } + if got := countCacheFiles(t, dir); got != 10 { + t.Fatalf("expected 10 entries pre-overflow; got %d", got) + } + // Trigger overflow: one more Put → cap exceeded → evict 1 (10% of 10). + c.Put(Key{Type: "k_overflow"}, interfaces.DiffResult{}) + got := countCacheFiles(t, dir) + // After eviction (10% of 10 = 1) + new entry: 10 - 1 + 1 = 10. + if got != 10 { + t.Errorf("post-overflow count: got %d entries, want 10 (cap holds)", got) + } + // Oldest key (k0) should have been evicted. + if _, hit := c.Get(Key{Type: "k0"}); hit { + t.Errorf("oldest key (k0) should be evicted") + } + // Newest key (k_overflow) should still be present. + if _, hit := c.Get(Key{Type: "k_overflow"}); !hit { + t.Errorf("newest key (k_overflow) should remain") + } +} + +// TestCache_LRUEvictionBatchOf10Percent verifies that when the cap is +// large enough to make 10% a multi-entry batch, multiple oldest +// entries are evicted in one pass. This locks the "amortized cost" / +// "evict oldest 10% in one pass" rev2 constraint. +func TestCache_LRUEvictionBatchOf10Percent(t *testing.T) { + dir := t.TempDir() + c := &filesystemCache{ + dir: dir, + maxEntries: 100, + maxBytes: defaultMaxBytes, + } + for i := range 100 { + c.Put(Key{Type: fmt.Sprintf("k%03d", i)}, interfaces.DiffResult{}) + time.Sleep(time.Millisecond) + } + // Trigger overflow: one extra Put → evict 10 oldest (10% of 100) + + // add 1 → 100 - 10 + 1 = 91. + c.Put(Key{Type: "k_overflow"}, interfaces.DiffResult{}) + got := countCacheFiles(t, dir) + if got < 88 || got > 92 { + // Allow ±2 for filesystems with mtime resolution coarser than + // the 1ms sleep; the central tendency must be ~91. + t.Errorf("post-overflow count: got %d, want ~91 (allow ±2 for mtime precision)", got) + } + // First 5 oldest keys should be evicted. + for i := range 5 { + k := Key{Type: fmt.Sprintf("k%03d", i)} + if _, hit := c.Get(k); hit { + t.Errorf("k%03d should be in the evicted oldest 10%%", i) + } + } +} + +// TestCache_LRURefreshesOnGet verifies that Get touches the cache +// file's mtime so that frequently-read entries are NOT evicted as if +// they were stale. The contract: an entry written first but Get'd +// after a newer entry was Put should outlive the newer entry under +// LRU eviction. Without the mtime-refresh in Get, eviction would +// behave as FIFO-by-write rather than true LRU. +func TestCache_LRURefreshesOnGet(t *testing.T) { + dir := t.TempDir() + c := &filesystemCache{ + dir: dir, + maxEntries: 10, + maxBytes: defaultMaxBytes, + } + // Fill exactly to the cap. k0 is the oldest by write order. + for i := range 10 { + c.Put(Key{Type: fmt.Sprintf("k%d", i)}, interfaces.DiffResult{}) + time.Sleep(time.Millisecond) + } + // Get k0 — this MUST refresh mtime so k0 is now the youngest by + // access time, even though it was the oldest by write time. Without + // the refresh, k0 retains its original (oldest) mtime and gets + // evicted on the next over-cap Put. + if _, hit := c.Get(Key{Type: "k0"}); !hit { + t.Fatal("expected hit on k0 before eviction") + } + time.Sleep(time.Millisecond) + // Trigger overflow: one extra Put → evict 1 (10% of 10). Under true + // LRU (mtime refreshed on Get), k0 is now the freshest entry and k1 + // is the oldest, so k1 should be evicted, not k0. + c.Put(Key{Type: "k_overflow"}, interfaces.DiffResult{}) + if _, hit := c.Get(Key{Type: "k0"}); !hit { + t.Errorf("k0 was Get'd before eviction; LRU should retain it (Get must refresh mtime)") + } + if _, hit := c.Get(Key{Type: "k1"}); hit { + t.Errorf("k1 was the oldest by access time; LRU should have evicted it") + } +} + +// countCacheFiles returns the number of *.json files under dir. Used +// to assert eviction behavior without depending on cache internals. +func countCacheFiles(t *testing.T, dir string) int { + t.Helper() + matches, err := filepath.Glob(filepath.Join(dir, "*.json")) + if err != nil { + t.Fatalf("glob: %v", err) + } + return len(matches) +} + +// TestCache_EvictionTouchesNothingWhenUnderCap verifies the no-op +// case: Put when count < maxEntries leaves all existing files alone. +func TestCache_EvictionTouchesNothingWhenUnderCap(t *testing.T) { + dir := t.TempDir() + c := &filesystemCache{ + dir: dir, + maxEntries: 100, + maxBytes: defaultMaxBytes, + } + c.Put(Key{Type: "a"}, interfaces.DiffResult{}) + c.Put(Key{Type: "b"}, interfaces.DiffResult{}) + c.Put(Key{Type: "c"}, interfaces.DiffResult{}) + if got := countCacheFiles(t, dir); got != 3 { + t.Errorf("under-cap puts should not evict; got %d entries", got) + } + // All three should be retrievable. + for _, k := range []string{"a", "b", "c"} { + if _, hit := c.Get(Key{Type: k}); !hit { + t.Errorf("under-cap key %q should be retained", k) + } + } +} diff --git a/iac/diffcache/cache_filesystem.go b/iac/diffcache/cache_filesystem.go new file mode 100644 index 000000000..52818b71f --- /dev/null +++ b/iac/diffcache/cache_filesystem.go @@ -0,0 +1,234 @@ +package diffcache + +import ( + "encoding/json" + "errors" + "log" + "os" + "path/filepath" + "sort" + "sync" + "time" + + "github.com/GoCodeAlone/workflow/interfaces" +) + +// filesystemCache is the file-backed Cache implementation. Cache files +// live under cache.dir, named by the sha256 fingerprint of the Key +// with a .json extension, e.g., +// `/abcdef0123456789....json`. The schema-version envelope +// gates silent eviction on a future schema bump. +// +// Concurrency: Get is serialized through the read filesystem; Put +// uses an os.WriteFile that is atomic on POSIX-compliant filesystems +// for sizes <= PIPE_BUF, and for larger writes the worst case is a +// truncated file that the next Get treats as corruption (silent +// eviction). The eviction-on-overflow scan is guarded by evictMu so +// concurrent Puts don't multiply-evict. +// +// Logging: a single info-level log fires the first time corruption is +// observed in this process so an operator has a breadcrumb without +// log spam. +type filesystemCache struct { + dir string + maxEntries int + maxBytes int64 + evictMu sync.Mutex + corruptOnce sync.Once +} + +// NewFilesystem returns a Cache whose entries are persisted under dir. +// The directory is created on first Put if absent. Callers may pass +// any directory; the [New] factory uses `~/.cache/wfctl/diff/`. +func NewFilesystem(dir string) Cache { + return &filesystemCache{ + dir: dir, + maxEntries: defaultMaxEntries, + maxBytes: defaultMaxBytes, + } +} + +// pathFor returns the on-disk path for key. Exported (within-package) +// so tests can mutate the file directly to exercise the corruption +// recovery path. +func (c *filesystemCache) pathFor(k Key) string { + return filepath.Join(c.dir, keyFingerprint(k)+".json") +} + +func (c *filesystemCache) Get(k Key) (interfaces.DiffResult, bool) { + path := c.pathFor(k) + data, err := os.ReadFile(path) + if err != nil { + // Either file-not-found (cache miss, normal) or some other read + // error (permissions, transient I/O). Both are treated as miss + // — apply correctness must not hinge on Get success. + return interfaces.DiffResult{}, false + } + var env envelope + if err := json.Unmarshal(data, &env); err != nil { + c.handleCorruption(path, err) + return interfaces.DiffResult{}, false + } + if env.SchemaVersion != cacheSchemaVersion { + c.handleCorruption(path, errors.New("schema-version mismatch")) + return interfaces.DiffResult{}, false + } + // Refresh mtime so [maybeEvict] (which orders by mtime) treats this + // entry as recently-used. Without this, frequently-read but + // infrequently-rewritten entries get evicted as if they were stale — + // the cache would be FIFO-by-write, not LRU. We chose mtime-touch + // over a sidecar "last-accessed" file to keep the on-disk shape + // trivial; the small cost is one extra syscall per cache hit. + // Errors are intentionally ignored: a Chtimes failure degrades + // eviction precision (this entry may be evicted earlier than + // preferred) but never produces wrong cache results. + now := time.Now() + _ = os.Chtimes(path, now, now) + return env.Result, true +} + +// Put writes the cache entry atomically via write-temp-then-rename. +// POSIX rename(2) is atomic on the same filesystem — if the process +// crashes mid-write the temp file is orphaned but the final cache +// path is either the prior contents or the new contents, never a +// partial write. The corruption-recovery path in [Get] is still the +// safety net for cross-filesystem renames or NFS edge cases that +// don't honor atomicity, but with this pattern the corruption +// recovery essentially never fires in production. +// +// Concurrency: safe for concurrent use, including concurrent Puts +// of the same Key. Each Put uses [os.CreateTemp] to obtain a unique +// temp filename (`.json..tmp`) so two goroutines writing +// the same Key cannot clobber each other's temp file. The final +// rename is racy in the sense that one goroutine's payload "wins," +// but both payloads were derived from the caller's DiffResult so the +// outcome is deterministic from the caller's perspective. +// +// Windows portability: this implementation uses the bare [os.Rename] +// for the atomic publish step, which matches the precedent set by +// other rename sites in this repo (cmd/wfctl/update.go, +// cmd/wfctl/plugin_install.go). On Windows, [os.Rename] fails when +// the destination already exists, so an in-place cache update via +// Put will fail on Windows; the caller treats this as a write +// failure and proceeds without caching (correct, by the cache-as- +// amortization framing in the package godoc — apply remains correct +// on a 100% miss rate). A future improvement is to vendor +// github.com/google/renameio for cross-platform atomic rename; +// doing so here would introduce the first such dependency in the +// repo, so deferred until there's a Windows-supported wfctl use +// case. Tracked as a known limitation in the package godoc. +// +// Disk-side errors during Put are intentionally silent: the next Get +// will miss (correct), and the operator already has "stuff isn't +// working" signal from elsewhere. The cache-as-amortization framing +// in the package godoc sets the expectation. +func (c *filesystemCache) Put(k Key, result interfaces.DiffResult) { + if err := os.MkdirAll(c.dir, 0o755); err != nil { + return + } + env := envelope{SchemaVersion: cacheSchemaVersion, Result: result} + data, err := json.Marshal(env) + if err != nil { + return + } + path := c.pathFor(k) + // os.CreateTemp gives us a per-call unique tempfile name and an + // open *os.File. Pattern uses a "*" suffix so the random token + // lands before ".tmp", giving filenames like + // `.json.123456789.tmp` — easy to spot during eviction + // and unambiguous about origin. Two concurrent Puts of the same + // Key end up with two distinct temp files; whichever rename + // completes last wins for the final cache path. + tmpFile, err := os.CreateTemp(c.dir, filepath.Base(path)+".*.tmp") + if err != nil { + return + } + tmp := tmpFile.Name() + if _, err := tmpFile.Write(data); err != nil { + _ = tmpFile.Close() + _ = os.Remove(tmp) + return + } + if err := tmpFile.Close(); err != nil { + _ = os.Remove(tmp) + return + } + if err := os.Rename(tmp, path); err != nil { + // Best-effort cleanup of the orphaned temp file. On Windows + // this rename can fail when path already exists (see godoc + // above); on Unix it's atomic-replace. Either way the next + // Put may succeed and LRU eviction reclaims any orphans. + _ = os.Remove(tmp) + return + } + c.maybeEvict() +} + +// handleCorruption silently deletes the corrupt file and emits a +// once-per-process info log. The single log gives operators a +// breadcrumb without spamming on every Get when an attacker or a +// rogue tool has filled the cache directory with garbage files. +func (c *filesystemCache) handleCorruption(path string, cause error) { + _ = os.Remove(path) + c.corruptOnce.Do(func() { + log.Printf("info: diffcache: detected corrupted cache file %q (%v); silently re-Diffing. This message logs once per process.", path, cause) + }) +} + +// maybeEvict scans the cache directory and, if either cap is exceeded, +// evicts the oldest evictionFraction (10%) of entries by mtime. The +// scan + sort cost is amortized: a single Put after N entries +// triggers one full scan; the next eviction-fraction Puts after that +// see no scan because the cap is back below threshold. +func (c *filesystemCache) maybeEvict() { + c.evictMu.Lock() + defer c.evictMu.Unlock() + entries, err := os.ReadDir(c.dir) + if err != nil { + return + } + // Filter to *.json files (defensive in case of stray files). + type fileInfo struct { + path string + mtime int64 + size int64 + } + files := make([]fileInfo, 0, len(entries)) + var totalBytes int64 + for _, e := range entries { + if e.IsDir() || filepath.Ext(e.Name()) != ".json" { + continue + } + info, err := e.Info() + if err != nil { + continue + } + files = append(files, fileInfo{ + path: filepath.Join(c.dir, e.Name()), + mtime: info.ModTime().UnixNano(), + size: info.Size(), + }) + totalBytes += info.Size() + } + overCount := len(files) > c.maxEntries + overBytes := totalBytes > c.maxBytes + if !overCount && !overBytes { + return + } + // Sort oldest-first by mtime. Files written in close succession + // may share an mtime under some filesystems; the secondary sort + // by path makes the order deterministic. + sort.Slice(files, func(i, j int) bool { + if files[i].mtime != files[j].mtime { + return files[i].mtime < files[j].mtime + } + return files[i].path < files[j].path + }) + // Evict the oldest fraction in one pass. Always at least 1 so + // tiny caps still make progress. + evictCount := max(int(float64(len(files))*evictionFraction), 1) + evictCount = min(evictCount, len(files)) + for i := range evictCount { + _ = os.Remove(files[i].path) + } +} diff --git a/iac/diffcache/cache_inmemory.go b/iac/diffcache/cache_inmemory.go new file mode 100644 index 000000000..51fd62c3e --- /dev/null +++ b/iac/diffcache/cache_inmemory.go @@ -0,0 +1,68 @@ +package diffcache + +import ( + "slices" + "sync" + + "github.com/GoCodeAlone/workflow/interfaces" +) + +// memoryCache is the in-memory Cache implementation. Entries live for +// the current process; CI workflows in this repo set +// WFCTL_DIFFCACHE=:memory: explicitly so containerized runners never +// write cache data to disk. +// +// Concurrency: a single mutex guards the map. Diff results are small +// and operations are infrequent (one per resource per Plan), so a +// finer-grained scheme isn't justified. +// +// Mutation isolation: DiffResult contains a Changes slice ([]FieldChange) +// whose backing array would otherwise be shared between the cached +// value and the value returned to the caller. Both Put and Get +// deep-copy the Changes slice so a caller mutating the returned +// DiffResult cannot mutate the cached entry, and a caller mutating +// the original Put argument cannot mutate the cached entry either. +// FieldChange itself contains `Old`/`New` of type any — the package +// does not own those values; if a caller stores a pointer or +// mutable map there, the deep-copy stops at the slice level. By +// convention DiffResult.Changes carries scalar Old/New (strings, +// numbers, bools), so this is the right tradeoff between +// correctness and copy cost. +type memoryCache struct { + mu sync.Mutex + entries map[string]interfaces.DiffResult +} + +// NewMemory returns an in-memory Cache. The returned Cache does not +// enforce a size cap — process lifetime is the eviction horizon. +// This matches the rev2 lifecycle constraint that the in-memory mode +// is the CI default and is not relied on for correctness. +func NewMemory() Cache { + return &memoryCache{entries: map[string]interfaces.DiffResult{}} +} + +func (c *memoryCache) Get(k Key) (interfaces.DiffResult, bool) { + c.mu.Lock() + defer c.mu.Unlock() + r, ok := c.entries[keyFingerprint(k)] + if !ok { + return interfaces.DiffResult{}, false + } + return cloneDiffResult(r), true +} + +func (c *memoryCache) Put(k Key, result interfaces.DiffResult) { + c.mu.Lock() + defer c.mu.Unlock() + c.entries[keyFingerprint(k)] = cloneDiffResult(result) +} + +// cloneDiffResult returns a deep copy of r at the slice level. The +// scalar fields are value-copied by struct assignment; Changes is +// cloned via slices.Clone so the cached and returned values do not +// share the backing array. See [memoryCache] godoc for the +// Old/New tradeoff (any-typed; not deep-cloned beyond the slice). +func cloneDiffResult(r interfaces.DiffResult) interfaces.DiffResult { + r.Changes = slices.Clone(r.Changes) + return r +} diff --git a/iac/diffcache/cache_test.go b/iac/diffcache/cache_test.go new file mode 100644 index 000000000..33f83b581 --- /dev/null +++ b/iac/diffcache/cache_test.go @@ -0,0 +1,174 @@ +package diffcache + +import ( + "testing" + + "github.com/GoCodeAlone/workflow/interfaces" +) + +// TestCache_FilesystemRoundtrip verifies the basic Get/Put roundtrip on +// a filesystem-backed cache: Put a DiffResult under a Key, then Get the +// same key returns the same DiffResult with hit=true. +func TestCache_FilesystemRoundtrip(t *testing.T) { + dir := t.TempDir() + c := NewFilesystem(dir) + key := Key{ + PluginVersion: "do@v0.10.0", + Type: "infra.vpc", + ProviderID: "vpc-abc", + SHAConfig: "deadbeef", + SHAOutputs: "cafebabe", + } + want := interfaces.DiffResult{ + NeedsUpdate: true, + Changes: []interfaces.FieldChange{ + {Path: "size", Old: "s-1vcpu-1gb", New: "s-2vcpu-2gb"}, + }, + } + c.Put(key, want) + got, hit := c.Get(key) + if !hit { + t.Fatal("expected cache hit after Put") + } + if got.NeedsUpdate != want.NeedsUpdate { + t.Errorf("NeedsUpdate: got %v want %v", got.NeedsUpdate, want.NeedsUpdate) + } + if len(got.Changes) != 1 || got.Changes[0].Path != "size" { + t.Errorf("Changes: got %+v want one entry with Path=size", got.Changes) + } +} + +// TestCache_FilesystemMissOnDifferentKey verifies key isolation: a Put +// under one key does not service a Get under any of the 5 fields' +// distinct values. Each field must contribute to the key independently. +func TestCache_FilesystemMissOnDifferentKey(t *testing.T) { + dir := t.TempDir() + c := NewFilesystem(dir) + base := Key{PluginVersion: "v1", Type: "T", ProviderID: "P", SHAConfig: "C", SHAOutputs: "O"} + c.Put(base, interfaces.DiffResult{NeedsUpdate: true}) + + cases := map[string]Key{ + "diff-pluginversion": {PluginVersion: "v2", Type: "T", ProviderID: "P", SHAConfig: "C", SHAOutputs: "O"}, + "diff-type": {PluginVersion: "v1", Type: "U", ProviderID: "P", SHAConfig: "C", SHAOutputs: "O"}, + "diff-providerid": {PluginVersion: "v1", Type: "T", ProviderID: "Q", SHAConfig: "C", SHAOutputs: "O"}, + "diff-shaconfig": {PluginVersion: "v1", Type: "T", ProviderID: "P", SHAConfig: "D", SHAOutputs: "O"}, + "diff-shaoutputs": {PluginVersion: "v1", Type: "T", ProviderID: "P", SHAConfig: "C", SHAOutputs: "X"}, + } + for name, k := range cases { + t.Run(name, func(t *testing.T) { + if _, hit := c.Get(k); hit { + t.Errorf("expected miss for distinct key field; got hit on %+v", k) + } + }) + } +} + +// TestCache_MemoryRoundtrip is the in-memory cache's basic test. +// Same contract as the filesystem cache. +func TestCache_MemoryRoundtrip(t *testing.T) { + c := NewMemory() + key := Key{Type: "infra.vpc", ProviderID: "vpc-abc"} + want := interfaces.DiffResult{NeedsReplace: true} + c.Put(key, want) + got, hit := c.Get(key) + if !hit { + t.Fatal("expected cache hit after Put") + } + if got.NeedsReplace != want.NeedsReplace { + t.Errorf("NeedsReplace: got %v want %v", got.NeedsReplace, want.NeedsReplace) + } +} + +// TestCache_MemoryDeepCopiesChanges verifies that the in-memory +// cache returns a deep copy of the DiffResult so a caller mutating +// the returned Changes slice cannot leak that mutation back into +// the cached entry. Pre-fix, the cached value and the returned value +// shared the same backing array for Changes; mutating one mutated +// the other. +// +// The test pattern: Put a value with a single Change, Get it, mutate +// the returned Changes slice (both element-level and length-level +// via append-into-cap), Get again, assert the second Get returns +// the original unmodified value. Also verifies the symmetric case: +// mutating the original argument after Put does not affect the +// cached value. +func TestCache_MemoryDeepCopiesChanges(t *testing.T) { + c := NewMemory() + key := Key{Type: "infra.vpc"} + original := interfaces.DiffResult{ + NeedsUpdate: true, + Changes: []interfaces.FieldChange{ + {Path: "size", Old: "small", New: "large"}, + }, + } + c.Put(key, original) + + // Mutate the original argument after Put. A leaky implementation + // would let this mutation reach the cached value. + original.Changes[0].Path = "MUTATED-AFTER-PUT" + + // First Get + mutate the returned slice element + length. + got1, hit := c.Get(key) + if !hit { + t.Fatal("expected hit on first Get") + } + if got1.Changes[0].Path != "size" { + t.Errorf("first Get: cached value leaked from post-Put mutation; Path=%q want %q", + got1.Changes[0].Path, "size") + } + got1.Changes[0].Path = "MUTATED-VIA-GET" + got1.Changes = append(got1.Changes, interfaces.FieldChange{Path: "extra"}) + + // Second Get must see the original value, not the mutated one. + got2, hit := c.Get(key) + if !hit { + t.Fatal("expected hit on second Get") + } + if len(got2.Changes) != 1 { + t.Errorf("second Get: Changes len=%d want 1; mutation via append leaked into cache", len(got2.Changes)) + } + if len(got2.Changes) > 0 && got2.Changes[0].Path != "size" { + t.Errorf("second Get: Changes[0].Path=%q want %q; mutation via Get leaked into cache", + got2.Changes[0].Path, "size") + } +} + +// TestCache_NoopAlwaysMisses verifies the disabled cache: Put is a +// no-op, every Get returns hit=false. +func TestCache_NoopAlwaysMisses(t *testing.T) { + c := NewNoop() + key := Key{Type: "x"} + c.Put(key, interfaces.DiffResult{NeedsUpdate: true}) + if _, hit := c.Get(key); hit { + t.Error("noop cache should always miss") + } +} + +// TestCache_EnvDispatch verifies the New() factory's env-var driven +// dispatch: WFCTL_DIFFCACHE=disabled → noop; =:memory: → memory; +// default → filesystem (we just verify it's not noop/memory). +func TestCache_EnvDispatch(t *testing.T) { + t.Run("disabled", func(t *testing.T) { + t.Setenv("WFCTL_DIFFCACHE", "disabled") + c := New() + if _, ok := c.(*noopCache); !ok { + t.Errorf("WFCTL_DIFFCACHE=disabled should yield *noopCache; got %T", c) + } + }) + t.Run("memory", func(t *testing.T) { + t.Setenv("WFCTL_DIFFCACHE", ":memory:") + c := New() + if _, ok := c.(*memoryCache); !ok { + t.Errorf("WFCTL_DIFFCACHE=:memory: should yield *memoryCache; got %T", c) + } + }) + t.Run("default", func(t *testing.T) { + // Set HOME to a tempdir so we don't pollute the real cache dir. + t.Setenv("HOME", t.TempDir()) + t.Setenv("WFCTL_DIFFCACHE", "") + c := New() + if _, ok := c.(*filesystemCache); !ok { + t.Errorf("default should yield *filesystemCache; got %T", c) + } + }) +} diff --git a/iac/wfctlhelpers/apply.go b/iac/wfctlhelpers/apply.go new file mode 100644 index 000000000..871ce431b --- /dev/null +++ b/iac/wfctlhelpers/apply.go @@ -0,0 +1,369 @@ +// Package wfctlhelpers hosts the wfctl-side dispatch helper for v2 IaC +// plugins. wfctl calls [ApplyPlan] when a plugin manifest declares +// iacProvider.computePlanVersion: v2 (see plugin/sdk.IaCProvider). The +// helper iterates plan.Actions, fetches the matching ResourceDriver from +// the provider, and dispatches each action to a per-action sub-function +// (doCreate, doUpdate, doReplace, doDelete). +// +// Lifecycle inside W-3a: +// +// - T3.1 (this file's ApplyPlan + dispatch + skeleton sub-functions) +// - T3.1.5 — wraps ApplyPlan with the input-drift postcondition +// - T3.2 — fills doCreate with UpsertSupporter recovery +// - T3.3 — fills doUpdate + doDelete (the latent doDelete bug fix) +// - T3.4 — fills doReplace and populates ApplyResult.ReplaceIDMap +// +// Until W-3b lands the cmd/wfctl dispatch wiring, [ApplyPlan] has no +// in-tree caller — the helper ships in W-3a as foundation only and is +// exercised solely by this package's tests. +// +// # Per-action error-prefix policy +// +// Sub-functions follow a "decompose-then-prefix" rule for the strings +// recorded in [interfaces.ApplyResult].Errors[].Error: +// +// - doCreate, doUpdate, doDelete pass driver errors through +// unchanged. The ActionError struct already carries Resource + +// Action context fields, so a per-kind prefix would be redundant. +// - doCreate's upsert recovery path prefixes "upsert: " (e.g., +// "upsert: read after conflict: ...") because the failure is +// specifically about the recovery flow, not the original Create. +// - doReplace prefixes "replace: delete: " or "replace: create: " +// because a Replace decomposes into two driver calls — without +// the prefix, an operator reading result.Errors couldn't tell +// which sub-step failed. +// +// Tests in apply_update_delete_test.go and apply_replace_test.go +// lock this contract via exact-string assertions; future refactors +// that drop or rename a prefix fail loudly. +package wfctlhelpers + +import ( + "context" + "errors" + "fmt" + "log" + + "github.com/GoCodeAlone/workflow/iac/inputsnapshot" + "github.com/GoCodeAlone/workflow/interfaces" +) + +// ApplyPlan dispatches each plan action to the matching ResourceDriver on +// the provider. Per-action errors are recorded on result.Errors and do NOT +// abort the loop — apply best-effort across actions, surface every failure +// for the operator to triage. Context cancellation between actions IS +// respected: when ctx is canceled or its deadline expires, the loop stops +// at the next iteration boundary and returns ctx.Err() as the top-level +// error so a long apply terminates promptly on Ctrl-C / SIGTERM. +// +// At entry ApplyPlan captures result.InitialInputSnapshot by fingerprinting +// every name listed in plan.InputSnapshot through the OS env. After the +// dispatch loop completes — successfully or not — a deferred postcondition +// computes result.InputDriftReport against an apply-time snapshot taken +// through inputsnapshot.NewTolerantEnvProvider (sub-action env unsets are +// preserved, not flagged as drift). The postcondition is wrapped in +// recover() so a buggy env-provider closure cannot corrupt apply results; +// on panic, InputDriftReport is reset to nil and a warning is logged. +// +// The function is concurrency-safe with respect to its inputs: result is +// owned by ApplyPlan for the duration of the call and is not shared with +// the provider or driver implementations. +// +// T3.1 ships the dispatch skeleton; T3.1.5 added the postcondition above; +// T3.2/T3.3/T3.4 fill the per-action sub-functions with their full bodies. +func ApplyPlan(ctx context.Context, p interfaces.IaCProvider, plan *interfaces.IaCPlan) (*interfaces.ApplyResult, error) { + return applyPlanWithEnvProvider(ctx, p, plan, nil) +} + +// applyPlanWithEnvProvider is the same-package test seam used by +// apply_postcondition_test.go to inject a custom apply-time env provider +// into the deferred drift postcondition (e.g., a panicky closure that +// stresses the recover() guard). When applyTimeEnv is nil, the function +// uses inputsnapshot.NewTolerantEnvProvider(plan.InputSnapshot) — the +// production behavior. The seam stays unexported because the only +// sanctioned external entry point is ApplyPlan. +func applyPlanWithEnvProvider( + ctx context.Context, + p interfaces.IaCProvider, + plan *interfaces.IaCPlan, + applyTimeEnv func(string) (string, bool), +) (*interfaces.ApplyResult, error) { + inputNames := snapshotKeys(plan.InputSnapshot) + result := &interfaces.ApplyResult{ + PlanID: plan.ID, + InitialInputSnapshot: inputsnapshot.Snapshot(inputNames, inputsnapshot.OSEnvProvider), + } + + // Deferred drift postcondition — runs unconditionally (success OR + // per-action failure), wrapped in recover() so a buggy env-provider + // closure (e.g., one freed mid-flight) cannot corrupt the apply + // result. On panic, drop the report rather than ship a partial one. + defer func() { + defer func() { + if r := recover(); r != nil { + result.InputDriftReport = nil + log.Printf("warning: input-drift postcondition panicked: %v", r) + } + }() + // Resolve the apply-time env provider lazily so the production + // path's NewTolerantEnvProvider closure isn't constructed when a + // test override is in play. + env := applyTimeEnv + if env == nil { + // CRITICAL: factory MUST be invoked here, NOT passed by + // reference (NewTolerantEnvProvider returns the closure; + // passing the function value would call the factory itself + // every Snapshot call and short-circuit the planSnapshot + // closure-capture). The cycle-4 reviewer caught this exact + // bug-class in the rev3 pseudo-code. + env = inputsnapshot.NewTolerantEnvProvider(plan.InputSnapshot) + } + applyTimeSnap := inputsnapshot.Snapshot(inputNames, env) + result.InputDriftReport = inputsnapshot.ComputeDrift(plan.InputSnapshot, applyTimeSnap) + }() + + for _, action := range plan.Actions { + // Honor cancellation at the loop boundary. Drivers should also + // check ctx internally for in-flight work, but the loop check + // guarantees apply stops between actions even if a driver + // happens to ignore ctx. The deferred postcondition still runs + // on early return so InputDriftReport is populated even on a + // canceled apply. + if err := ctx.Err(); err != nil { + return result, err + } + d, err := p.ResourceDriver(action.Resource.Type) + if err != nil { + result.Errors = append(result.Errors, interfaces.ActionError{ + Resource: action.Resource.Name, + Action: action.Action, + Error: fmt.Sprintf("resolve driver: %v", err), + }) + continue + } + if err := dispatchAction(ctx, d, action, result); err != nil { + result.Errors = append(result.Errors, interfaces.ActionError{ + Resource: action.Resource.Name, + Action: action.Action, + Error: err.Error(), + }) + } + } + return result, nil +} + +// snapshotKeys returns the keys of m as an unordered slice. ComputeDrift +// sorts its output, and Snapshot iterates in any order, so no key sort +// is needed at this stage. Inlined helper to keep the dependency +// surface minimal and avoid pulling in slices/maps at the postcondition +// call site. +func snapshotKeys(m map[string]string) []string { + if len(m) == 0 { + return nil + } + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + return out +} + +// dispatchAction routes a single PlanAction to the per-kind sub-function. +// An unknown action kind returns an error which ApplyPlan records on +// result.Errors so an operator running a malformed plan sees a per-action +// diagnostic rather than a silent skip. +func dispatchAction(ctx context.Context, d interfaces.ResourceDriver, action interfaces.PlanAction, result *interfaces.ApplyResult) error { + switch action.Action { + case "create": + return doCreate(ctx, d, action, result) + case "update": + return doUpdate(ctx, d, action, result) + case "replace": + return doReplace(ctx, d, action, result) + case "delete": + return doDelete(ctx, d, action) + default: + return fmt.Errorf("unknown action %q", action.Action) + } +} + +// doCreate invokes Create and, on ErrResourceAlreadyExists, attempts an +// idempotent upsert recovery for drivers that opt in via the +// UpsertSupporter interface. The recovery path is: +// +// 1. Probe the driver for UpsertSupporter — if absent (interface +// assertion fails) or SupportsUpsert()==false, the original +// conflict surfaces unchanged. +// 2. Read the existing resource by Name + Type (no ProviderID — the +// driver's Read is responsible for locating by name when ProviderID +// is empty; SupportsUpsert()==true is the contract that this works). +// 3. Defensive: if Read returns an empty ProviderID, fail with a named +// diagnostic — Update with an empty ProviderID would route to a +// create-by-spec path on most drivers, defeating the upsert. +// 4. Update the existing resource with the desired spec, threading +// the ProviderID found in step 2. +// +// Recovery is single-pass: if the recovery Update itself returns +// ErrResourceAlreadyExists (a driver bug — Update with a known +// ProviderID should not conflict), the second conflict surfaces +// unchanged rather than retriggering the recovery loop. +// +// Error wrapping (in-package contract): +// +// - Read-after-conflict failures wrap both the original Create error +// and the Read error via errors.Join, so callers in this package +// can match either via errors.Is. +// - The doCreate return value preserves the wrap chain. ApplyPlan's +// dispatch loop, however, flattens errors to a string in +// result.Errors[].Error (see [ApplyPlan]) — external callers +// reading [interfaces.ApplyResult].Errors lose errors.Is matching +// and must inspect the canonical "upsert: read after conflict:" +// prefix instead. This boundary is deliberate: ActionError carries +// the per-resource action context fields the wrap chain otherwise +// duplicates. +func doCreate(ctx context.Context, d interfaces.ResourceDriver, action interfaces.PlanAction, result *interfaces.ApplyResult) error { + out, err := d.Create(ctx, action.Resource) + if errors.Is(err, interfaces.ErrResourceAlreadyExists) { + us, ok := d.(interfaces.UpsertSupporter) + if !ok || !us.SupportsUpsert() { + return err // no recovery available; surface the conflict + } + ref := interfaces.ResourceRef{Name: action.Resource.Name, Type: action.Resource.Type} + existing, readErr := d.Read(ctx, ref) + if readErr != nil { + return fmt.Errorf("upsert: read after conflict: %w", errors.Join(err, readErr)) + } + if existing == nil || existing.ProviderID == "" { + return fmt.Errorf("upsert: resource %q found by name but ProviderID is empty: %w", ref.Name, err) + } + ref.ProviderID = existing.ProviderID + out, err = d.Update(ctx, ref, action.Resource) + } + if err == nil && out != nil { + result.Resources = append(result.Resources, *out) + } + return err +} + +// doUpdate invokes Update with a ResourceRef carrying action.Current's +// ProviderID (when action.Current is non-nil), appending the driver's +// returned ResourceOutput to result.Resources on success. Driver errors +// pass through unchanged so the caller's per-action error wrapper +// (ApplyPlan's loop body) records them with the canonical action + +// resource fields. +// +// Defensive contract: doUpdate does NOT synthesize a precondition error +// when action.Current is nil — the driver is the authority on what an +// empty ProviderID means. ComputePlan upstream is responsible for never +// emitting an Update without action.Current; if it does, the driver's +// own typed validation surfaces the bug. +func doUpdate(ctx context.Context, d interfaces.ResourceDriver, action interfaces.PlanAction, result *interfaces.ApplyResult) error { + ref := refFromAction(action) + out, err := d.Update(ctx, ref, action.Resource) + if err != nil { + return err + } + if out != nil { + result.Resources = append(result.Resources, *out) + } + return nil +} + +// doReplace decomposes a Replace action into Delete-then-Create on the +// driver and propagates the new ProviderID through +// result.ReplaceIDMap[action.Resource.Name] so JIT substitution in W-5 +// can patch dependent resources whose configs reference the replaced +// resource by name. +// +// Failure semantics: +// - Delete fails → return wrapped "replace: delete: "; Create +// does NOT run; ReplaceIDMap is NOT populated for this resource. +// - Delete succeeds, ctx canceled before Create → return wrapped +// "replace: canceled after delete: "; Create does NOT run; +// ReplaceIDMap is NOT populated. The half-replaced state is the +// operator's recovery surface (same as the Create-fails case). +// - Delete succeeds, Create fails → return wrapped +// "replace: create: "; ReplaceIDMap stays empty for this +// resource. Operators inspect the apply log + the empty-for-this- +// name slot in ReplaceIDMap to know which resources are in a +// half-replaced state and need manual cloud restoration. +// - Both succeed → result.Resources gets the new output appended, +// result.ReplaceIDMap[action.Resource.Name] = new ProviderID. Map +// is lazily-initialized on first successful Replace so plans with +// no Replace actions don't carry an empty map through serialisation. +// +// The "replace: ..." prefix is essential because Replace decomposes +// into two driver calls — without it, an operator reading +// result.Errors couldn't tell whether the Delete or the Create failed. +// Other sub-functions (doCreate non-recovery path, doUpdate, doDelete) +// pass driver errors through unchanged. +func doReplace(ctx context.Context, d interfaces.ResourceDriver, action interfaces.PlanAction, result *interfaces.ApplyResult) error { + if err := d.Delete(ctx, refFromAction(action)); err != nil { + return fmt.Errorf("replace: delete: %w", err) + } + // Honor cancellation between Delete and Create. Without this guard + // a Ctrl-C / SIGTERM that arrives mid-Replace would still trigger + // the Create call, leaving the operator without the cleanest + // possible interruption point. The half-replaced state is still + // recoverable (Delete already happened; Create did not, so + // ReplaceIDMap stays empty) but cancellation propagates fast. + if err := ctx.Err(); err != nil { + return fmt.Errorf("replace: canceled after delete: %w", err) + } + out, err := d.Create(ctx, action.Resource) + if err != nil { + return fmt.Errorf("replace: create: %w", err) + } + if out != nil { + result.Resources = append(result.Resources, *out) + // Lazy-init: only allocate the map when there's an actual + // entry to record. ApplyResult.ReplaceIDMap stays nil for + // plans with no Replace actions, which the omitempty JSON tag + // then drops from the encoded form (covered by + // TestApplyResult_OmitEmptyContract in interfaces/iac_state_test.go). + if result.ReplaceIDMap == nil { + result.ReplaceIDMap = make(map[string]string) + } + result.ReplaceIDMap[action.Resource.Name] = out.ProviderID + } + return nil +} + +// doDelete invokes Delete with a ResourceRef carrying action.Current's +// ProviderID. This closes the latent gap documented in the design +// (DOProvider.Apply has no "case delete" arm today, so wfctl's +// state-prune action silently skipped cloud-resource deletion through +// the v1 dispatch path); under v2 dispatch wfctlhelpers.ApplyPlan +// always invokes the driver's Delete, ensuring state-prune is paired +// with a real cloud-side mutation. +// +// Driver errors pass through unchanged for the caller's per-action +// error wrapping. doDelete does not append to result.Resources — a +// successful delete has no resource to record. +func doDelete(ctx context.Context, d interfaces.ResourceDriver, action interfaces.PlanAction) error { + return d.Delete(ctx, refFromAction(action)) +} + +// refFromAction builds a ResourceRef from the action's resource identity, +// threading the ProviderID from action.Current when the plan was built +// from existing state. For net-new actions (action.Current == nil) the +// returned ref has an empty ProviderID, matching the contract that +// drivers locate by Name when ProviderID is absent. +// +// Invariant: callers MUST ensure action.Current's Name/Type match +// action.Resource — Replace plans assume same-name same-type. If a future +// plan generator emits a Replace where Current.Name != Resource.Name +// (e.g., a rename across replace), the Delete would target the new name +// with the old ProviderID — likely a "not found" or wrong-resource bug. +// This function does not enforce the invariant; the contract is upstream +// in ComputePlan. +func refFromAction(action interfaces.PlanAction) interfaces.ResourceRef { + ref := interfaces.ResourceRef{ + Name: action.Resource.Name, + Type: action.Resource.Type, + } + if action.Current != nil { + ref.ProviderID = action.Current.ProviderID + } + return ref +} diff --git a/iac/wfctlhelpers/apply_create_test.go b/iac/wfctlhelpers/apply_create_test.go new file mode 100644 index 000000000..cbb440429 --- /dev/null +++ b/iac/wfctlhelpers/apply_create_test.go @@ -0,0 +1,267 @@ +package wfctlhelpers + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/GoCodeAlone/workflow/interfaces" +) + +// fakeDriverWithUpsert is a ResourceDriver test double that: +// - returns createErr from Create (typically ErrResourceAlreadyExists) +// - returns readResult / readErr from Read +// - records whether Update was called via updateCalled +// - implements UpsertSupporter when supportsUpsert is true +// +// Used to exercise doCreate's upsert-recovery path. Embedded fakeDriver +// supplies the no-op other methods so the recorder is minimal. +type fakeDriverWithUpsert struct { + *fakeDriver + createErr error + readResult *interfaces.ResourceOutput + readErr error + updateCalled bool + updateOut *interfaces.ResourceOutput + supportsUpsert bool +} + +func (d *fakeDriverWithUpsert) Create(_ context.Context, _ interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + d.fakeDriver.createCount++ + if d.createErr != nil { + return nil, d.createErr + } + return &interfaces.ResourceOutput{}, nil +} + +func (d *fakeDriverWithUpsert) Read(_ context.Context, _ interfaces.ResourceRef) (*interfaces.ResourceOutput, error) { + if d.readErr != nil { + return nil, d.readErr + } + return d.readResult, nil +} + +func (d *fakeDriverWithUpsert) Update(_ context.Context, ref interfaces.ResourceRef, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + d.updateCalled = true + d.fakeDriver.updateCount++ + if d.updateOut != nil { + return d.updateOut, nil + } + return &interfaces.ResourceOutput{Name: spec.Name, Type: spec.Type, ProviderID: ref.ProviderID}, nil +} + +func (d *fakeDriverWithUpsert) SupportsUpsert() bool { return d.supportsUpsert } + +// upsertFakeProvider returns the fakeDriverWithUpsert from ResourceDriver +// regardless of resource type. +type upsertFakeProvider struct { + *fakeProvider + upsert *fakeDriverWithUpsert +} + +func newUpsertFakeProvider(d *fakeDriverWithUpsert) *upsertFakeProvider { + if d.fakeDriver == nil { + d.fakeDriver = &fakeDriver{} + } + return &upsertFakeProvider{fakeProvider: newFakeProvider(), upsert: d} +} + +func (p *upsertFakeProvider) ResourceDriver(_ string) (interfaces.ResourceDriver, error) { + return p.upsert, nil +} + +// TestApplyPlan_Create_UpsertOnAlreadyExists is the canonical T3.2 test: +// a Create that returns ErrResourceAlreadyExists must trigger +// Read + Update on a driver that opts in via UpsertSupporter. The +// recovery must clear the per-action error so result.Errors is empty. +func TestApplyPlan_Create_UpsertOnAlreadyExists(t *testing.T) { + d := &fakeDriverWithUpsert{ + createErr: interfaces.ErrResourceAlreadyExists, + readResult: &interfaces.ResourceOutput{ProviderID: "found-uuid"}, + supportsUpsert: true, + } + fp := newUpsertFakeProvider(d) + plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ + {Action: "create", Resource: spec("a", "infra.vpc")}, + }} + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if len(result.Errors) > 0 { + t.Errorf("upsert should recover; got errors: %v", result.Errors) + } + if !d.updateCalled { + t.Errorf("upsert path should call Update after Read; updateCalled=%v", d.updateCalled) + } +} + +// TestApplyPlan_Create_AlreadyExists_NoUpsertSupport verifies the +// fall-through: if the driver does NOT implement UpsertSupporter (or +// returns SupportsUpsert()==false), the original ErrResourceAlreadyExists +// surfaces unchanged via result.Errors. No Read or Update happens. +func TestApplyPlan_Create_AlreadyExists_NoUpsertSupport(t *testing.T) { + d := &fakeDriverWithUpsert{ + createErr: interfaces.ErrResourceAlreadyExists, + supportsUpsert: false, + } + fp := newUpsertFakeProvider(d) + plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ + {Action: "create", Resource: spec("a", "infra.vpc")}, + }} + result, _ := ApplyPlan(context.Background(), fp, plan) + if len(result.Errors) != 1 { + t.Fatalf("expected 1 per-action error; got %d (%v)", len(result.Errors), result.Errors) + } + if !strings.Contains(result.Errors[0].Error, "already exists") { + t.Errorf("expected ErrResourceAlreadyExists in error; got %q", result.Errors[0].Error) + } + if d.updateCalled { + t.Errorf("Update must not be called when SupportsUpsert returns false") + } +} + +// alreadyExistsBareDriver is a ResourceDriver that does NOT implement +// UpsertSupporter at all (no SupportsUpsert method). Used to exercise +// doCreate's `!ok` interface-assertion fall-through — distinct from +// the `SupportsUpsert()==false` path covered above. Behavioral +// outcome is identical (conflict surfaces unchanged, no Read/Update), +// but the code path through the type assertion is different. +type alreadyExistsBareDriver struct { + *fakeDriver +} + +func (d *alreadyExistsBareDriver) Create(_ context.Context, _ interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + d.fakeDriver.createCount++ + return nil, interfaces.ErrResourceAlreadyExists +} + +// TestApplyPlan_Create_AlreadyExists_DriverDoesNotImplementUpsertSupporter +// covers the `!ok` arm of doCreate's `us, ok := d.(interfaces.UpsertSupporter)` +// type assertion: a driver that does not implement UpsertSupporter at +// all. Distinct from TestApplyPlan_Create_AlreadyExists_NoUpsertSupport, +// which covers the `ok && !us.SupportsUpsert()` arm. +func TestApplyPlan_Create_AlreadyExists_DriverDoesNotImplementUpsertSupporter(t *testing.T) { + bare := &alreadyExistsBareDriver{fakeDriver: &fakeDriver{}} + // Sanity-check the test premise: bare must NOT satisfy UpsertSupporter. + // If a future refactor lifts SupportsUpsert onto the embedded fakeDriver, + // this test would silently switch to the "ok && !SupportsUpsert" branch + // and stop covering the `!ok` arm. The compile-time assertion locks + // the premise. + var _ interfaces.ResourceDriver = bare + if _, ok := any(bare).(interfaces.UpsertSupporter); ok { + t.Fatal("test premise broken: alreadyExistsBareDriver must not implement UpsertSupporter") + } + // Inject the bare driver via a one-off provider that returns it for + // any resource type. + fp := &bareDriverProvider{fakeProvider: newFakeProvider(), driver: bare} + plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ + {Action: "create", Resource: spec("a", "infra.vpc")}, + }} + result, _ := ApplyPlan(context.Background(), fp, plan) + if len(result.Errors) != 1 { + t.Fatalf("expected 1 per-action error; got %d (%v)", len(result.Errors), result.Errors) + } + if !strings.Contains(result.Errors[0].Error, "already exists") { + t.Errorf("expected ErrResourceAlreadyExists in error; got %q", result.Errors[0].Error) + } +} + +// bareDriverProvider returns the alreadyExistsBareDriver for any +// resource type so the test stays focused on the type-assertion +// fall-through. +type bareDriverProvider struct { + *fakeProvider + driver *alreadyExistsBareDriver +} + +func (p *bareDriverProvider) ResourceDriver(_ string) (interfaces.ResourceDriver, error) { + return p.driver, nil +} + +// TestApplyPlan_Create_UpsertReadFailureWraps verifies the diagnostic +// when Read fails after an ErrResourceAlreadyExists conflict: the error +// wraps both the original create error and the read error so callers +// can match either via errors.Is. +func TestApplyPlan_Create_UpsertReadFailureWraps(t *testing.T) { + readErr := errors.New("read failed: 503") + d := &fakeDriverWithUpsert{ + createErr: interfaces.ErrResourceAlreadyExists, + readErr: readErr, + supportsUpsert: true, + } + fp := newUpsertFakeProvider(d) + plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ + {Action: "create", Resource: spec("a", "infra.vpc")}, + }} + result, _ := ApplyPlan(context.Background(), fp, plan) + if len(result.Errors) != 1 { + t.Fatalf("expected 1 error; got %d (%v)", len(result.Errors), result.Errors) + } + got := result.Errors[0].Error + if !strings.Contains(got, "upsert: read after conflict") { + t.Errorf("expected canonical 'upsert: read after conflict' prefix; got %q", got) + } + if !strings.Contains(got, "503") { + t.Errorf("expected wrapped read error in message; got %q", got) + } + if d.updateCalled { + t.Errorf("Update must not be called when Read fails") + } +} + +// TestApplyPlan_Create_UpsertEmptyProviderIDFails verifies a defensive +// check: if Read finds a resource by name but its ProviderID is empty, +// the upsert path must NOT call Update with an empty ProviderID (which +// would route to a Create-by-spec semantics). Instead, fail with a +// diagnostic that names the resource. +func TestApplyPlan_Create_UpsertEmptyProviderIDFails(t *testing.T) { + d := &fakeDriverWithUpsert{ + createErr: interfaces.ErrResourceAlreadyExists, + readResult: &interfaces.ResourceOutput{ProviderID: ""}, // defensive: empty ID + supportsUpsert: true, + } + fp := newUpsertFakeProvider(d) + plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ + {Action: "create", Resource: spec("vpc-1", "infra.vpc")}, + }} + result, _ := ApplyPlan(context.Background(), fp, plan) + if len(result.Errors) != 1 { + t.Fatalf("expected 1 error; got %d (%v)", len(result.Errors), result.Errors) + } + got := result.Errors[0].Error + if !strings.Contains(got, "ProviderID is empty") { + t.Errorf("expected 'ProviderID is empty' in diagnostic; got %q", got) + } + if !strings.Contains(got, "vpc-1") { + t.Errorf("diagnostic should name the resource; got %q", got) + } + if d.updateCalled { + t.Errorf("Update must not be called with empty ProviderID") + } +} + +// TestApplyPlan_Create_UpsertAppendsResources verifies happy-path +// plumbing: a successful upsert recovery must also append the Update's +// output to result.Resources so downstream state-write paths see the +// recovered resource exactly as if Create had succeeded directly. +func TestApplyPlan_Create_UpsertAppendsResources(t *testing.T) { + d := &fakeDriverWithUpsert{ + createErr: interfaces.ErrResourceAlreadyExists, + readResult: &interfaces.ResourceOutput{ProviderID: "found-uuid"}, + updateOut: &interfaces.ResourceOutput{Name: "vpc-1", Type: "infra.vpc", ProviderID: "found-uuid", Status: "active"}, + supportsUpsert: true, + } + fp := newUpsertFakeProvider(d) + plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ + {Action: "create", Resource: spec("vpc-1", "infra.vpc")}, + }} + result, _ := ApplyPlan(context.Background(), fp, plan) + if len(result.Resources) != 1 { + t.Fatalf("expected 1 resource appended; got %d (%+v)", len(result.Resources), result.Resources) + } + if got := result.Resources[0].ProviderID; got != "found-uuid" { + t.Errorf("ProviderID: got %q want %q", got, "found-uuid") + } +} diff --git a/iac/wfctlhelpers/apply_postcondition_test.go b/iac/wfctlhelpers/apply_postcondition_test.go new file mode 100644 index 000000000..b7ef765b6 --- /dev/null +++ b/iac/wfctlhelpers/apply_postcondition_test.go @@ -0,0 +1,237 @@ +package wfctlhelpers + +import ( + "context" + "errors" + "os" + "testing" + + "github.com/GoCodeAlone/workflow/iac/inputsnapshot" + "github.com/GoCodeAlone/workflow/interfaces" +) + +// fingerprint delegates to inputsnapshot.Compute so this test file's +// fixtures always use the production fingerprint algorithm. If +// Compute's prefix length or hash algorithm ever changes, every +// expected-fingerprint constant flowing through here re-aligns +// automatically — keeping the cross-package guarantee that +// `apply_postcondition_test.go::fingerprint` and +// `cmd/wfctl/infra_apply_plan_test.go::fingerprintForTest` produce +// identical outputs. Mirrors that helper deliberately. +func fingerprint(value string) string { + snap := inputsnapshot.Compute([]string{"k"}, func(string) (string, bool) { return value, true }) + return snap["k"] +} + +// TestApplyPlan_InitialInputSnapshot_CapturedAtEntry verifies that +// ApplyPlan populates ApplyResult.InitialInputSnapshot at apply entry by +// fingerprinting every name listed in plan.InputSnapshot through the +// real OS env. T3.1.5 lands this capture; T3.1's skeleton did not. +func TestApplyPlan_InitialInputSnapshot_CapturedAtEntry(t *testing.T) { + t.Setenv("WFCTL_T315_FOO", "value-foo") + t.Setenv("WFCTL_T315_BAR", "value-bar") + plan := &interfaces.IaCPlan{ + InputSnapshot: map[string]string{ + "WFCTL_T315_FOO": fingerprint("value-foo"), + "WFCTL_T315_BAR": fingerprint("value-bar"), + }, + } + fp := newFakeProvider() + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if got, want := result.InitialInputSnapshot["WFCTL_T315_FOO"], fingerprint("value-foo"); got != want { + t.Errorf("InitialInputSnapshot[WFCTL_T315_FOO]: got %q want %q", got, want) + } + if got, want := result.InitialInputSnapshot["WFCTL_T315_BAR"], fingerprint("value-bar"); got != want { + t.Errorf("InitialInputSnapshot[WFCTL_T315_BAR]: got %q want %q", got, want) + } +} + +// TestApply_Postcondition_PanicDoesNotCorruptResult verifies that a +// panic inside the deferred postcondition (e.g., a buggy env-provider +// closure) does not crash apply or surface as the top-level error. +// Recovers and clears result.InputDriftReport so an operator sees an +// empty drift report rather than a partially-populated one. Uses the +// internal seam applyPlanWithEnvProvider to inject the panicky provider. +func TestApply_Postcondition_PanicDoesNotCorruptResult(t *testing.T) { + panickyEnv := func(_ string) (string, bool) { panic("env-provider closure freed") } + plan := &interfaces.IaCPlan{ + InputSnapshot: map[string]string{"FOO": fingerprint("v")}, + } + fp := newFakeProvider() + result, err := applyPlanWithEnvProvider(context.Background(), fp, plan, panickyEnv) + if err != nil { + t.Fatalf("apply should not surface postcondition panic: %v", err) + } + if result.InputDriftReport != nil { + t.Errorf("on postcondition panic, drift report should be nil; got %+v", result.InputDriftReport) + } +} + +// TestApply_Postcondition_FingerprintAfterEnvUnset_NoFalsePositive +// covers the cycle-3 sub-action cleanup case: an apply action unsets a +// credential env var (e.g., the post-apply security hardening pattern). +// The postcondition must NOT flag this as drift — the operator's mental +// model says the value at plan time is what matters; an unset post-apply +// is not "the env changed mid-flight" in the user-facing sense. The +// production NewTolerantEnvProvider preserves fingerprints for +// plan-time-set apply-time-unset vars via the in-package sentinel; this +// test exercises that path end-to-end through ApplyPlan. +func TestApply_Postcondition_FingerprintAfterEnvUnset_NoFalsePositive(t *testing.T) { + const varName = "WFCTL_T315_PG_PASSWORD" + t.Setenv(varName, "value") + planFP := fingerprint("value") + plan := &interfaces.IaCPlan{ + InputSnapshot: map[string]string{varName: planFP}, + Actions: []interfaces.PlanAction{ + {Action: "create", Resource: spec("a", "infra.vpc")}, + }, + } + // envUnsetingFakeProvider unsets the env var inside Create, simulating + // a sub-action cleanup that happens between snapshot capture and + // postcondition execution. + fp := &envUnsetingFakeProvider{ + fakeProvider: newFakeProvider(), + varToUnset: varName, + } + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if len(result.InputDriftReport) != 0 { + t.Errorf("post-apply env-unset must not trigger drift false-positive; got: %+v", result.InputDriftReport) + } + // Sanity: the sub-action did unset the var. + if _, set := os.LookupEnv(varName); set { + t.Errorf("test setup: sub-action did not unset %s", varName) + } +} + +// envUnsetingFakeProvider returns a fake driver whose Create method +// unsets a configured env var as a side-effect, simulating the +// sub-action credential-cleanup pattern that the tolerant env provider +// must accommodate without false-positive drift. +type envUnsetingFakeProvider struct { + *fakeProvider + varToUnset string +} + +func (p *envUnsetingFakeProvider) ResourceDriver(_ string) (interfaces.ResourceDriver, error) { + return &envUnsetingFakeDriver{fakeDriver: p.driver, varToUnset: p.varToUnset}, nil +} + +type envUnsetingFakeDriver struct { + *fakeDriver + varToUnset string +} + +func (d *envUnsetingFakeDriver) Create(ctx context.Context, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + out, err := d.fakeDriver.Create(ctx, spec) + if err != nil { + return out, err + } + _ = os.Unsetenv(d.varToUnset) + return out, nil +} + +// TestApplyPlan_PlanStaleDiagnostic_NamesChangedKeys is the canonical +// drift-detection test: an env var captured at plan time has a +// different value at apply time. The postcondition must record exactly +// one drift entry naming the changed key, with both fingerprints +// distinct. +func TestApplyPlan_PlanStaleDiagnostic_NamesChangedKeys(t *testing.T) { + const varName = "WFCTL_T315_STAGING_PG_PASSWORD" + planFP := fingerprint("old-value") + t.Setenv(varName, "new-value") // post-plan env value differs from plan-time fingerprint + plan := &interfaces.IaCPlan{ + InputSnapshot: map[string]string{varName: planFP}, + } + fp := newFakeProvider() + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if len(result.InputDriftReport) != 1 { + t.Fatalf("expected 1 drift entry, got %d (%+v)", len(result.InputDriftReport), result.InputDriftReport) + } + got := result.InputDriftReport[0] + if got.Name != varName { + t.Errorf("Name: got %q want %q", got.Name, varName) + } + if got.PlanFingerprint != planFP { + t.Errorf("PlanFingerprint: got %q want %q", got.PlanFingerprint, planFP) + } + if got.ApplyFingerprint == planFP || got.ApplyFingerprint == "" { + t.Errorf("ApplyFingerprint should be a distinct, non-empty value; got %q", got.ApplyFingerprint) + } +} + +// TestApplyPlan_NoDriftWhenInputSnapshotEmpty verifies the no-op case: +// a plan with no InputSnapshot must produce no drift report. Avoids +// the postcondition incorrectly synthesizing entries for vars that +// were never tracked. +func TestApplyPlan_NoDriftWhenInputSnapshotEmpty(t *testing.T) { + plan := &interfaces.IaCPlan{} + fp := newFakeProvider() + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if len(result.InputDriftReport) != 0 { + t.Errorf("empty InputSnapshot must yield empty drift; got %+v", result.InputDriftReport) + } +} + +// errFromDispatch is a sentinel for the "apply errored AND drift +// detected" interleave test. Confirms the deferred postcondition runs +// regardless of dispatch outcome. +var errFromDispatch = errors.New("dispatch failure (test)") + +// TestApplyPlan_PostconditionRunsEvenIfDispatchErrored verifies the +// "regardless of apply success/error path" contract on the +// InputDriftReport godoc. A driver that returns an error must still +// allow drift detection to populate the report — the postcondition is +// strictly best-effort but unconditional. +func TestApplyPlan_PostconditionRunsEvenIfDispatchErrored(t *testing.T) { + const varName = "WFCTL_T315_DRIFT_DURING_FAIL" + planFP := fingerprint("plan-time") + t.Setenv(varName, "apply-time") + plan := &interfaces.IaCPlan{ + InputSnapshot: map[string]string{varName: planFP}, + Actions: []interfaces.PlanAction{ + {Action: "create", Resource: spec("a", "infra.vpc")}, + }, + } + fp := &erroringFakeProvider{fakeProvider: newFakeProvider()} + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if len(result.Errors) != 1 { + t.Errorf("expected per-action error from erroring driver; got %d (%v)", len(result.Errors), result.Errors) + } + if len(result.InputDriftReport) != 1 { + t.Errorf("postcondition must populate drift report even when dispatch errors; got %+v", result.InputDriftReport) + } +} + +// erroringFakeProvider returns a driver whose Create returns +// errFromDispatch so apply produces a per-action error. +type erroringFakeProvider struct { + *fakeProvider +} + +func (p *erroringFakeProvider) ResourceDriver(_ string) (interfaces.ResourceDriver, error) { + return &erroringFakeDriver{fakeDriver: p.driver}, nil +} + +type erroringFakeDriver struct { + *fakeDriver +} + +func (d *erroringFakeDriver) Create(_ context.Context, _ interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + d.fakeDriver.createCount++ + return nil, errFromDispatch +} diff --git a/iac/wfctlhelpers/apply_replace_test.go b/iac/wfctlhelpers/apply_replace_test.go new file mode 100644 index 000000000..5e215bb84 --- /dev/null +++ b/iac/wfctlhelpers/apply_replace_test.go @@ -0,0 +1,316 @@ +package wfctlhelpers + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/GoCodeAlone/workflow/interfaces" +) + +// orderRecordingDriver records the order of Delete vs. Create +// invocations so doReplace's "delete THEN create" contract can be +// asserted exactly. createReturn is the canned Create output (carries +// the NEW ProviderID); deleteFn / createFn are optional hooks that +// override the default success behavior. +type orderRecordingDriver struct { + *fakeDriver + deleteAt int // sequence number when Delete was called (0 if not called) + createAt int // sequence number when Create was called (0 if not called) + step int + createReturn *interfaces.ResourceOutput + deleteErr error + createErr error +} + +func (d *orderRecordingDriver) Delete(_ context.Context, _ interfaces.ResourceRef) error { + d.fakeDriver.deleteCount++ + d.step++ + d.deleteAt = d.step + return d.deleteErr +} + +func (d *orderRecordingDriver) Create(_ context.Context, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + d.fakeDriver.createCount++ + d.step++ + d.createAt = d.step + if d.createErr != nil { + return nil, d.createErr + } + if d.createReturn != nil { + return d.createReturn, nil + } + return &interfaces.ResourceOutput{Name: spec.Name, Type: spec.Type, ProviderID: "fake-id-" + spec.Name}, nil +} + +// orderRecordingProvider returns the orderRecordingDriver for any +// resource type. +type orderRecordingProvider struct { + *fakeProvider + driver *orderRecordingDriver +} + +func newOrderRecordingProvider() *orderRecordingProvider { + base := newFakeProvider() + return &orderRecordingProvider{ + fakeProvider: base, + driver: &orderRecordingDriver{fakeDriver: base.driver}, + } +} + +func (p *orderRecordingProvider) ResourceDriver(_ string) (interfaces.ResourceDriver, error) { + return p.driver, nil +} + +// stateWithID is a helper that builds a ResourceState with a known +// ProviderID, used to seed the "old" side of a Replace action. +func stateWithID(name, providerID string) *interfaces.ResourceState { + return &interfaces.ResourceState{Name: name, ProviderID: providerID} +} + +// TestApplyPlan_Replace_DeletesThenCreates_PropagatesNewID is the +// canonical T3.4 test: Replace must (1) call Delete first, (2) call +// Create after the Delete, (3) thread the NEW ProviderID from Create +// into result.Resources. +func TestApplyPlan_Replace_DeletesThenCreates_PropagatesNewID(t *testing.T) { + fp := newOrderRecordingProvider() + fp.driver.createReturn = &interfaces.ResourceOutput{ + Name: "vpc", Type: "infra.vpc", ProviderID: "new-uuid", + } + plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ + {Action: "replace", Resource: spec("vpc", "infra.vpc"), Current: stateWithID("vpc", "old-uuid")}, + }} + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if fp.driver.deleteAt == 0 || fp.driver.createAt == 0 { + t.Errorf("Replace should call both Delete and Create; deleteAt=%d createAt=%d", + fp.driver.deleteAt, fp.driver.createAt) + } + if fp.driver.createAt < fp.driver.deleteAt { + t.Errorf("Create must run AFTER Delete; deleteAt=%d createAt=%d", + fp.driver.deleteAt, fp.driver.createAt) + } + if len(result.Resources) != 1 || result.Resources[0].ProviderID != "new-uuid" { + t.Errorf("expected new ProviderID in result.Resources, got %+v", result.Resources) + } +} + +// TestApplyPlan_Replace_PopulatesReplaceIDMap is the new-in-T3.4 +// contract: result.ReplaceIDMap[action.Resource.Name] must equal the +// new ProviderID returned by Create. Keyed by the *replaced* resource's +// Name (per T3.0.4 godoc — fixed during T3.0.4 review). Lazy-init on +// first Replace. +func TestApplyPlan_Replace_PopulatesReplaceIDMap(t *testing.T) { + fp := newOrderRecordingProvider() + fp.driver.createReturn = &interfaces.ResourceOutput{ + Name: "vpc", Type: "infra.vpc", ProviderID: "new-uuid", + } + plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ + {Action: "replace", Resource: spec("vpc", "infra.vpc"), Current: stateWithID("vpc", "old-uuid")}, + }} + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if got := result.ReplaceIDMap["vpc"]; got != "new-uuid" { + t.Errorf("ReplaceIDMap[vpc]: got %q want %q (full map: %+v)", got, "new-uuid", result.ReplaceIDMap) + } +} + +// TestApplyPlan_Replace_MultipleActionsAllPopulate verifies the map +// accumulates across actions: two Replace actions in one plan must +// produce two entries in result.ReplaceIDMap, each keyed by its +// replaced resource's name. +func TestApplyPlan_Replace_MultipleActionsAllPopulate(t *testing.T) { + plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ + {Action: "replace", Resource: spec("vpc", "infra.vpc"), Current: stateWithID("vpc", "old-vpc")}, + {Action: "replace", Resource: spec("db", "infra.database"), Current: stateWithID("db", "old-db")}, + }} + // Use a per-call provider that returns a fresh ID per resource so + // the map entries are distinguishable. + fp := &perResourceReplaceProvider{ + fakeProvider: newFakeProvider(), + newIDs: map[string]string{"vpc": "new-vpc-id", "db": "new-db-id"}, + } + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if got := result.ReplaceIDMap["vpc"]; got != "new-vpc-id" { + t.Errorf("ReplaceIDMap[vpc]: got %q want %q", got, "new-vpc-id") + } + if got := result.ReplaceIDMap["db"]; got != "new-db-id" { + t.Errorf("ReplaceIDMap[db]: got %q want %q", got, "new-db-id") + } +} + +// perResourceReplaceProvider mints a fresh new-ID per resource Name so +// MultipleActionsAllPopulate can distinguish the two map entries. Each +// driver is a one-shot recorder owned by the provider. +type perResourceReplaceProvider struct { + *fakeProvider + newIDs map[string]string + driver *perResourceReplaceDriver +} + +func (p *perResourceReplaceProvider) ResourceDriver(_ string) (interfaces.ResourceDriver, error) { + if p.driver == nil { + p.driver = &perResourceReplaceDriver{fakeDriver: p.fakeProvider.driver, newIDs: p.newIDs} + } + return p.driver, nil +} + +type perResourceReplaceDriver struct { + *fakeDriver + newIDs map[string]string +} + +func (d *perResourceReplaceDriver) Delete(_ context.Context, _ interfaces.ResourceRef) error { + d.fakeDriver.deleteCount++ + return nil +} + +func (d *perResourceReplaceDriver) Create(_ context.Context, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + d.fakeDriver.createCount++ + id := d.newIDs[spec.Name] + if id == "" { + id = "fallback-id" + } + return &interfaces.ResourceOutput{Name: spec.Name, Type: spec.Type, ProviderID: id}, nil +} + +// TestApplyPlan_Replace_DeleteFailsDoesNotCreate verifies that when +// the Delete sub-step of a Replace fails, Create is NOT called and +// result.ReplaceIDMap is NOT populated for that resource. The +// per-action error must surface with the canonical "replace: delete:" +// prefix that doReplace decorates (not bare driver error — Replace +// decomposes, so the prefix tells the operator which sub-step failed). +func TestApplyPlan_Replace_DeleteFailsDoesNotCreate(t *testing.T) { + fp := newOrderRecordingProvider() + fp.driver.deleteErr = errors.New("delete failed: 503") + plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ + {Action: "replace", Resource: spec("vpc", "infra.vpc"), Current: stateWithID("vpc", "old-uuid")}, + }} + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if fp.driver.createAt != 0 { + t.Errorf("Create must not run when Delete fails; createAt=%d", fp.driver.createAt) + } + if _, present := result.ReplaceIDMap["vpc"]; present { + t.Errorf("ReplaceIDMap must not contain vpc when Delete failed; got %+v", result.ReplaceIDMap) + } + if len(result.Errors) != 1 { + t.Fatalf("expected 1 per-action error; got %d (%v)", len(result.Errors), result.Errors) + } + if !strings.HasPrefix(result.Errors[0].Error, "replace: delete:") { + t.Errorf("expected canonical 'replace: delete:' prefix; got %q", result.Errors[0].Error) + } +} + +// TestApplyPlan_Replace_CtxCancelAfterDelete_SkipsCreate verifies the +// post-Delete cancellation guard: if ctx is canceled between Delete +// and Create, doReplace must NOT call Create — instead returning a +// wrapped error with the canonical "replace: canceled after delete:" +// prefix. The half-replaced state is the operator's recovery surface +// (Delete happened, Create did not, ReplaceIDMap stays empty for the +// resource). +func TestApplyPlan_Replace_CtxCancelAfterDelete_SkipsCreate(t *testing.T) { + // cancelOnDeleteDriver cancels the context inside Delete after + // recording the call but before returning. This simulates + // SIGTERM/SIGINT arriving exactly between the Delete and Create + // driver calls. + ctx, cancel := context.WithCancel(context.Background()) + fp := &cancelOnDeleteFakeProvider{ + fakeProvider: newFakeProvider(), + cancel: cancel, + } + plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ + {Action: "replace", Resource: spec("vpc", "infra.vpc"), Current: stateWithID("vpc", "old-uuid")}, + }} + result, err := ApplyPlan(ctx, fp, plan) + if err != nil { + // ApplyPlan's loop check catches the cancellation at the next + // iteration, but the per-action ctx check inside doReplace + // fires first. Either path yields a per-action error rather + // than a top-level error from this single-action plan. + t.Fatalf("ApplyPlan should not surface top-level error: %v", err) + } + if fp.driver.deleteCount != 1 { + t.Errorf("Delete should have run before cancellation; deleteCount=%d", fp.driver.deleteCount) + } + if fp.driver.createCount != 0 { + t.Errorf("Create must NOT run after ctx cancellation; createCount=%d", fp.driver.createCount) + } + if _, present := result.ReplaceIDMap["vpc"]; present { + t.Errorf("ReplaceIDMap must not contain vpc when Create skipped; got %+v", result.ReplaceIDMap) + } + if len(result.Errors) != 1 { + t.Fatalf("expected 1 per-action error; got %d (%v)", len(result.Errors), result.Errors) + } + if !strings.HasPrefix(result.Errors[0].Error, "replace: canceled after delete:") { + t.Errorf("expected canonical 'replace: canceled after delete:' prefix; got %q", result.Errors[0].Error) + } +} + +// cancelOnDeleteFakeProvider returns a driver whose Delete cancels a +// supplied context.CancelFunc as a side-effect, simulating exact +// post-Delete pre-Create cancellation. +type cancelOnDeleteFakeProvider struct { + *fakeProvider + cancel context.CancelFunc + driver *cancelOnDeleteDriver +} + +func (p *cancelOnDeleteFakeProvider) ResourceDriver(_ string) (interfaces.ResourceDriver, error) { + if p.driver == nil { + p.driver = &cancelOnDeleteDriver{fakeDriver: p.fakeProvider.driver, cancel: p.cancel} + } + return p.driver, nil +} + +type cancelOnDeleteDriver struct { + *fakeDriver + cancel context.CancelFunc +} + +func (d *cancelOnDeleteDriver) Delete(_ context.Context, _ interfaces.ResourceRef) error { + d.fakeDriver.deleteCount++ + d.cancel() // ctx is now canceled; Create must not run. + return nil +} + +// TestApplyPlan_Replace_CreateFailsLeavesMapEmpty verifies the +// post-Delete pre-Create failure window: Delete succeeded but Create +// failed → ReplaceIDMap stays empty for this resource (no spurious +// new-ID entry). The plan rollback note says operators inspect this +// state checkpoint to know which resources are in a half-replaced +// state; "absent from ReplaceIDMap + Resource was Replace target" is +// the canonical signal. +func TestApplyPlan_Replace_CreateFailsLeavesMapEmpty(t *testing.T) { + fp := newOrderRecordingProvider() + fp.driver.createErr = errors.New("create failed: 422") + plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ + {Action: "replace", Resource: spec("vpc", "infra.vpc"), Current: stateWithID("vpc", "old-uuid")}, + }} + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if fp.driver.deleteAt == 0 { + t.Errorf("Delete should still have run (failure was in Create, not Delete)") + } + if _, present := result.ReplaceIDMap["vpc"]; present { + t.Errorf("ReplaceIDMap must not contain vpc when Create failed; got %+v", result.ReplaceIDMap) + } + if len(result.Errors) != 1 { + t.Fatalf("expected 1 per-action error; got %d (%v)", len(result.Errors), result.Errors) + } + if !strings.HasPrefix(result.Errors[0].Error, "replace: create:") { + t.Errorf("expected canonical 'replace: create:' prefix; got %q", result.Errors[0].Error) + } +} diff --git a/iac/wfctlhelpers/apply_test.go b/iac/wfctlhelpers/apply_test.go new file mode 100644 index 000000000..5432740c7 --- /dev/null +++ b/iac/wfctlhelpers/apply_test.go @@ -0,0 +1,339 @@ +package wfctlhelpers + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/GoCodeAlone/workflow/interfaces" +) + +// fakeDriver counts CRUD-method invocations so dispatch tests can prove +// each per-action sub-function ran the right number of times. Counters +// are integers (not booleans) because the Replace action decomposes into +// Delete-then-Create on the driver — a boolean recorder can't tell +// "Replace dispatched correctly" apart from "explicit Create + explicit +// Delete also ran in the same plan." Each test gets a fresh instance via +// newFakeProvider(). +type fakeDriver struct { + createCount int + updateCount int + deleteCount int +} + +func (d *fakeDriver) Create(_ context.Context, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + d.createCount++ + return &interfaces.ResourceOutput{Name: spec.Name, Type: spec.Type, ProviderID: "fake-id-" + spec.Name}, nil +} + +func (d *fakeDriver) Read(_ context.Context, ref interfaces.ResourceRef) (*interfaces.ResourceOutput, error) { + return &interfaces.ResourceOutput{Name: ref.Name, Type: ref.Type, ProviderID: ref.ProviderID}, nil +} + +func (d *fakeDriver) Update(_ context.Context, ref interfaces.ResourceRef, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + d.updateCount++ + return &interfaces.ResourceOutput{Name: spec.Name, Type: spec.Type, ProviderID: ref.ProviderID}, nil +} + +func (d *fakeDriver) Delete(_ context.Context, _ interfaces.ResourceRef) error { + d.deleteCount++ + return nil +} + +func (d *fakeDriver) Diff(_ context.Context, _ interfaces.ResourceSpec, _ *interfaces.ResourceOutput) (*interfaces.DiffResult, error) { + return &interfaces.DiffResult{}, nil +} + +func (d *fakeDriver) HealthCheck(_ context.Context, _ interfaces.ResourceRef) (*interfaces.HealthResult, error) { + return &interfaces.HealthResult{Healthy: true}, nil +} + +func (d *fakeDriver) Scale(_ context.Context, _ interfaces.ResourceRef, _ int) (*interfaces.ResourceOutput, error) { + return nil, nil +} + +func (d *fakeDriver) SensitiveKeys() []string { return nil } + +// fakeProvider returns the same fakeDriver for every resource type so a +// single instance can record all dispatch invocations within one +// ApplyPlan call. The driverErr field, when non-nil, is returned from +// ResourceDriver instead of the driver — used by tests that need to +// exercise the resolve-driver-error path. +type fakeProvider struct { + driver *fakeDriver + driverErr error +} + +func newFakeProvider() *fakeProvider { return &fakeProvider{driver: &fakeDriver{}} } + +func (p *fakeProvider) Name() string { return "fake" } +func (p *fakeProvider) Version() string { return "0.0.0" } +func (p *fakeProvider) Initialize(_ context.Context, _ map[string]any) error { return nil } +func (p *fakeProvider) Capabilities() []interfaces.IaCCapabilityDeclaration { return nil } +func (p *fakeProvider) Plan(_ context.Context, _ []interfaces.ResourceSpec, _ []interfaces.ResourceState) (*interfaces.IaCPlan, error) { + return &interfaces.IaCPlan{}, nil +} +func (p *fakeProvider) Apply(_ context.Context, _ *interfaces.IaCPlan) (*interfaces.ApplyResult, error) { + return &interfaces.ApplyResult{}, nil +} +func (p *fakeProvider) Destroy(_ context.Context, _ []interfaces.ResourceRef) (*interfaces.DestroyResult, error) { + return nil, nil +} +func (p *fakeProvider) Status(_ context.Context, _ []interfaces.ResourceRef) ([]interfaces.ResourceStatus, error) { + return nil, nil +} +func (p *fakeProvider) DetectDrift(_ context.Context, _ []interfaces.ResourceRef) ([]interfaces.DriftResult, error) { + return nil, nil +} +func (p *fakeProvider) Import(_ context.Context, _ string, _ string) (*interfaces.ResourceState, error) { + return nil, nil +} +func (p *fakeProvider) ResolveSizing(_ string, _ interfaces.Size, _ *interfaces.ResourceHints) (*interfaces.ProviderSizing, error) { + return nil, nil +} +func (p *fakeProvider) ResourceDriver(_ string) (interfaces.ResourceDriver, error) { + if p.driverErr != nil { + return nil, p.driverErr + } + return p.driver, nil +} +func (p *fakeProvider) SupportedCanonicalKeys() []string { return nil } +func (p *fakeProvider) BootstrapStateBackend(_ context.Context, _ map[string]any) (*interfaces.BootstrapResult, error) { + return nil, nil +} +func (p *fakeProvider) Close() error { return nil } + +// spec is a minimal ResourceSpec helper for tests. +func spec(name, typ string) interfaces.ResourceSpec { + return interfaces.ResourceSpec{Name: name, Type: typ} +} + +// state is a minimal ResourceState helper for tests; sets a deterministic +// ProviderID so doUpdate / doDelete / doReplace have something to thread. +func state(name string) *interfaces.ResourceState { + return &interfaces.ResourceState{Name: name, ProviderID: "old-id-" + name} +} + +// TestApplyPlan_HandlesAllFourActions is the T3.1 dispatch smoke test: +// one PlanAction of each kind (create/update/replace/delete) must reach +// the ResourceDriver. Asserts via integer call counts so a missing +// "case replace" arm in dispatchAction is detectable: a 4-action plan +// [create, update, replace, delete] must produce exactly 2 Create, 1 +// Update, 2 Delete. If Replace dispatch were silently dropped, those +// counts would shift to 1/1/1 and the assertion would fail. +func TestApplyPlan_HandlesAllFourActions(t *testing.T) { + plan := &interfaces.IaCPlan{ + Actions: []interfaces.PlanAction{ + {Action: "create", Resource: spec("a", "infra.vpc")}, + {Action: "update", Resource: spec("b", "infra.vpc"), Current: state("b")}, + {Action: "replace", Resource: spec("c", "infra.vpc"), Current: state("c")}, + {Action: "delete", Resource: spec("d", "infra.vpc"), Current: state("d")}, + }, + } + fp := newFakeProvider() + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if len(result.Errors) != 0 { + t.Errorf("expected no errors, got %v", result.Errors) + } + // Replace = Delete + Create on the driver. + // Plan = [create, update, replace, delete] → + // Create: 1 (from create) + 1 (from replace) = 2 + // Update: 1 (from update) + // Delete: 1 (from delete) + 1 (from replace) = 2 + if got, want := fp.driver.createCount, 2; got != want { + t.Errorf("Create call count: got %d, want %d", got, want) + } + if got, want := fp.driver.updateCount, 1; got != want { + t.Errorf("Update call count: got %d, want %d", got, want) + } + if got, want := fp.driver.deleteCount, 2; got != want { + t.Errorf("Delete call count: got %d, want %d", got, want) + } +} + +// TestApplyPlan_ReplaceDispatchesViaDeleteThenCreate isolates Replace +// dispatch from explicit create/delete actions: a plan containing ONLY a +// single Replace action must produce exactly 1 Delete + 1 Create. If +// Replace were routed to default (unknown action) or to a different arm, +// these counts would not both be 1. +func TestApplyPlan_ReplaceDispatchesViaDeleteThenCreate(t *testing.T) { + plan := &interfaces.IaCPlan{ + Actions: []interfaces.PlanAction{ + {Action: "replace", Resource: spec("c", "infra.vpc"), Current: state("c")}, + }, + } + fp := newFakeProvider() + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if len(result.Errors) != 0 { + t.Errorf("expected no errors, got %v", result.Errors) + } + if got, want := fp.driver.deleteCount, 1; got != want { + t.Errorf("Replace.deleteCount: got %d, want %d", got, want) + } + if got, want := fp.driver.createCount, 1; got != want { + t.Errorf("Replace.createCount: got %d, want %d", got, want) + } + if got, want := fp.driver.updateCount, 0; got != want { + t.Errorf("Replace.updateCount: got %d, want %d", got, want) + } +} + +// TestApplyPlan_UnknownActionRecordsError verifies the dispatch's +// default-case behavior: an action with an unrecognised kind should not +// crash but must surface a per-action error in result.Errors so an +// operator running a malformed plan sees the diagnostic instead of a +// silently dropped action. +func TestApplyPlan_UnknownActionRecordsError(t *testing.T) { + plan := &interfaces.IaCPlan{ + Actions: []interfaces.PlanAction{ + {Action: "frobnicate", Resource: spec("x", "infra.vpc")}, + }, + } + fp := newFakeProvider() + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatalf("ApplyPlan should not return a top-level error for a per-action issue; got %v", err) + } + if len(result.Errors) != 1 { + t.Fatalf("expected 1 per-action error, got %d (%v)", len(result.Errors), result.Errors) + } + if result.Errors[0].Resource != "x" || result.Errors[0].Action != "frobnicate" { + t.Errorf("unexpected error shape: %+v", result.Errors[0]) + } +} + +// TestApplyPlan_PreservesPlanID checks the ApplyResult.PlanID echo. Future +// log-correlation paths rely on this so an operator can match an apply +// invocation back to the persisted plan. +func TestApplyPlan_PreservesPlanID(t *testing.T) { + plan := &interfaces.IaCPlan{ID: "plan-12345"} + fp := newFakeProvider() + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if result.PlanID != "plan-12345" { + t.Errorf("PlanID: got %q want %q", result.PlanID, "plan-12345") + } +} + +// errResolveDriver is a sentinel for the ResourceDriver-error test path. +var errResolveDriver = errors.New("driver lookup failed") + +// TestApplyPlan_ResolveDriverErrorRecordsActionError covers the branch +// where p.ResourceDriver returns an error: the per-action error must be +// recorded with the canonical "resolve driver:" prefix and the loop must +// continue to the next action so a single bad type doesn't abort apply. +func TestApplyPlan_ResolveDriverErrorRecordsActionError(t *testing.T) { + plan := &interfaces.IaCPlan{ + Actions: []interfaces.PlanAction{ + {Action: "create", Resource: spec("bad", "infra.unknown")}, + {Action: "create", Resource: spec("good", "infra.vpc")}, + }, + } + fp := newFakeProvider() + fp.driverErr = errResolveDriver + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatalf("top-level error not expected for per-action driver-resolve failure; got %v", err) + } + // fp.driverErr applies to BOTH actions in this test (driver lookup + // hits the same fake), so both record the resolve-driver error. The + // important contracts are: the loop continued past action[0] (so + // len(Errors)==2 not 1), and the prefix is canonical. + if len(result.Errors) != 2 { + t.Fatalf("expected 2 per-action errors (loop must continue past action[0]); got %d (%v)", + len(result.Errors), result.Errors) + } + for i, e := range result.Errors { + if !strings.HasPrefix(e.Error, "resolve driver:") { + t.Errorf("Errors[%d].Error: want prefix %q; got %q", i, "resolve driver:", e.Error) + } + if e.Action != "create" { + t.Errorf("Errors[%d].Action: got %q want %q", i, e.Action, "create") + } + } + if result.Errors[0].Resource != "bad" || result.Errors[1].Resource != "good" { + t.Errorf("expected Resource ordering [bad, good]; got [%s, %s]", + result.Errors[0].Resource, result.Errors[1].Resource) + } +} + +// TestApplyPlan_LoopContinuesAfterPerActionFailure verifies the +// best-effort contract: action[0]'s driver lookup fails, action[1]'s +// resource type uses a different driver path that succeeds. Both effects +// must be observable in the result. +func TestApplyPlan_LoopContinuesAfterPerActionFailure(t *testing.T) { + // Use a per-call fakeProvider variant whose ResourceDriver returns + // an error for one type and the real driver for the other. + fp := &selectiveFakeProvider{ + fakeProvider: newFakeProvider(), + errorType: "infra.unknown", + } + plan := &interfaces.IaCPlan{ + Actions: []interfaces.PlanAction{ + {Action: "create", Resource: spec("bad", "infra.unknown")}, + {Action: "create", Resource: spec("good", "infra.vpc")}, + }, + } + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if len(result.Errors) != 1 { + t.Fatalf("expected 1 error from action[0]; got %d (%v)", len(result.Errors), result.Errors) + } + if fp.driver.createCount != 1 { + t.Errorf("action[1] should have reached the driver; createCount=%d want 1", fp.driver.createCount) + } + // Successful action's output should be in result.Resources. + if len(result.Resources) != 1 || result.Resources[0].Name != "good" { + t.Errorf("expected one Resources entry for action[1]; got %+v", result.Resources) + } +} + +// selectiveFakeProvider returns errResolveDriver for one resource type +// and the wrapped fakeProvider's driver for any other type. Used by +// TestApplyPlan_LoopContinuesAfterPerActionFailure to exercise mixed +// success/failure ordering across a multi-action plan. +type selectiveFakeProvider struct { + *fakeProvider + errorType string +} + +func (p *selectiveFakeProvider) ResourceDriver(typ string) (interfaces.ResourceDriver, error) { + if typ == p.errorType { + return nil, errResolveDriver + } + return p.driver, nil +} + +// TestApplyPlan_CtxCancellationStopsLoop verifies the loop respects +// context cancellation between actions. Drivers may honor ctx individually, +// but the loop itself must check at the iteration boundary so a +// long-running multi-action apply terminates promptly on Ctrl-C / deadline. +func TestApplyPlan_CtxCancellationStopsLoop(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // cancel before invocation so the very first iteration aborts. + plan := &interfaces.IaCPlan{ + Actions: []interfaces.PlanAction{ + {Action: "create", Resource: spec("a", "infra.vpc")}, + {Action: "create", Resource: spec("b", "infra.vpc")}, + }, + } + fp := newFakeProvider() + _, err := ApplyPlan(ctx, fp, plan) + if !errors.Is(err, context.Canceled) { + t.Fatalf("expected context.Canceled top-level error; got %v", err) + } + if fp.driver.createCount != 0 { + t.Errorf("no driver calls should run after ctx cancellation; createCount=%d", fp.driver.createCount) + } +} diff --git a/iac/wfctlhelpers/apply_update_delete_test.go b/iac/wfctlhelpers/apply_update_delete_test.go new file mode 100644 index 000000000..44614653d --- /dev/null +++ b/iac/wfctlhelpers/apply_update_delete_test.go @@ -0,0 +1,259 @@ +package wfctlhelpers + +import ( + "context" + "errors" + "testing" + + "github.com/GoCodeAlone/workflow/interfaces" +) + +// providerIDCapturingDriver records the ResourceRef passed to Update / +// Delete so tests can assert ProviderID propagation from action.Current +// to the driver call. +type providerIDCapturingDriver struct { + *fakeDriver + updateRef interfaces.ResourceRef + updateSpec interfaces.ResourceSpec + deleteRef interfaces.ResourceRef + deleteErr error + updateErr error +} + +func (d *providerIDCapturingDriver) Update(_ context.Context, ref interfaces.ResourceRef, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + d.fakeDriver.updateCount++ + d.updateRef = ref + d.updateSpec = spec + if d.updateErr != nil { + return nil, d.updateErr + } + return &interfaces.ResourceOutput{Name: spec.Name, Type: spec.Type, ProviderID: ref.ProviderID}, nil +} + +func (d *providerIDCapturingDriver) Delete(_ context.Context, ref interfaces.ResourceRef) error { + d.fakeDriver.deleteCount++ + d.deleteRef = ref + return d.deleteErr +} + +// captureFakeProvider returns the providerIDCapturingDriver for any +// resource type so the caller can assert what ApplyPlan passed through. +type captureFakeProvider struct { + *fakeProvider + driver *providerIDCapturingDriver +} + +func newCaptureFakeProvider() *captureFakeProvider { + base := newFakeProvider() + return &captureFakeProvider{ + fakeProvider: base, + driver: &providerIDCapturingDriver{fakeDriver: base.driver}, + } +} + +func (p *captureFakeProvider) ResourceDriver(_ string) (interfaces.ResourceDriver, error) { + return p.driver, nil +} + +// TestApplyPlan_Update_PassesProviderID is the canonical T3.3 update +// test: the plan's update action carries action.Current with a known +// ProviderID; doUpdate must thread that ProviderID into the +// ResourceRef passed to driver.Update so the driver knows which +// resource to mutate. Threading it via Name alone would lose the +// upstream ID and force the driver to re-resolve. +func TestApplyPlan_Update_PassesProviderID(t *testing.T) { + const knownID = "do-vpc-abc123" + cur := &interfaces.ResourceState{ + Name: "vpc-1", + Type: "infra.vpc", + ProviderID: knownID, + } + plan := &interfaces.IaCPlan{ + Actions: []interfaces.PlanAction{ + {Action: "update", Resource: spec("vpc-1", "infra.vpc"), Current: cur}, + }, + } + fp := newCaptureFakeProvider() + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if len(result.Errors) != 0 { + t.Fatalf("unexpected errors: %v", result.Errors) + } + if got := fp.driver.updateRef.ProviderID; got != knownID { + t.Errorf("Update ResourceRef.ProviderID: got %q want %q", got, knownID) + } + if got := fp.driver.updateRef.Name; got != "vpc-1" { + t.Errorf("Update ResourceRef.Name: got %q want %q", got, "vpc-1") + } + if got := fp.driver.updateRef.Type; got != "infra.vpc" { + t.Errorf("Update ResourceRef.Type: got %q want %q", got, "infra.vpc") + } + // Driver returns a populated ResourceOutput; ApplyPlan must thread + // it through to result.Resources. + if len(result.Resources) != 1 || result.Resources[0].ProviderID != knownID { + t.Errorf("Resources: expected one entry with ProviderID %q; got %+v", knownID, result.Resources) + } +} + +// TestApplyPlan_Update_NilCurrentIsHandledDefensively verifies the +// edge case where a malformed plan's update action lacks +// action.Current. ComputePlan upstream should never emit such a plan, +// but doUpdate must not panic — it should call Update with an empty +// ProviderID (matching the skeleton's behavior) so the driver can +// surface its own typed validation error. +func TestApplyPlan_Update_NilCurrentIsHandledDefensively(t *testing.T) { + plan := &interfaces.IaCPlan{ + Actions: []interfaces.PlanAction{ + {Action: "update", Resource: spec("orphan", "infra.vpc"), Current: nil}, + }, + } + fp := newCaptureFakeProvider() + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if got := fp.driver.updateRef.ProviderID; got != "" { + t.Errorf("nil Current must yield empty ProviderID; got %q", got) + } + // Per-action result behavior: driver returned non-nil success → + // no errors, output appended. The contract is that doUpdate + // doesn't synthesize a precondition error; the driver is the + // authority on what an empty ProviderID means. + if len(result.Errors) != 0 { + t.Errorf("unexpected per-action errors for nil Current: %v", result.Errors) + } + // Lock the resource-append contract on the success path so a + // regression that made doUpdate skip the append on nil Current + // would fail this test loudly. + if len(result.Resources) != 1 { + t.Errorf("expected 1 Resources entry on driver-success path; got %d", len(result.Resources)) + } +} + +// TestApplyPlan_Delete_NilCurrentIsHandledDefensively mirrors the +// Update nil-Current contract for doDelete: a delete action without +// action.Current must not panic; the empty ProviderID flows to the +// driver, which is the authority on what to do (most drivers will +// surface a typed validation error). doDelete itself does not +// synthesize a precondition error — same defensive shape as doUpdate. +func TestApplyPlan_Delete_NilCurrentIsHandledDefensively(t *testing.T) { + plan := &interfaces.IaCPlan{ + Actions: []interfaces.PlanAction{ + {Action: "delete", Resource: spec("orphan", "infra.vpc"), Current: nil}, + }, + } + fp := newCaptureFakeProvider() + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if got := fp.driver.deleteRef.ProviderID; got != "" { + t.Errorf("nil Current must yield empty ProviderID on Delete; got %q", got) + } + if got := fp.driver.deleteRef.Name; got != "orphan" { + t.Errorf("Delete ResourceRef.Name: got %q want %q", got, "orphan") + } + if len(result.Errors) != 0 { + t.Errorf("unexpected per-action errors for nil Current Delete: %v", result.Errors) + } + // Sanity: driver was called exactly once (latent bug-fix contract + // from T3.3 — Delete must not be silently skipped). + if fp.driver.deleteCount != 1 { + t.Errorf("driver.Delete must be called exactly once; got %d", fp.driver.deleteCount) + } +} + +// TestApplyPlan_Delete_InvokesDriverDelete is the latent-bug-fix test +// from the design notes: today's DOProvider.Apply has no +// `case "delete":` arm, so wfctl's state-prune action silently skips +// cloud-resource deletion. doDelete CLOSES that gap by always calling +// driver.Delete. This test fails on the design's pre-T3.3 codepath. +func TestApplyPlan_Delete_InvokesDriverDelete(t *testing.T) { + const knownID = "do-vpc-xyz789" + cur := &interfaces.ResourceState{ + Name: "old-vpc", + Type: "infra.vpc", + ProviderID: knownID, + } + plan := &interfaces.IaCPlan{ + Actions: []interfaces.PlanAction{ + {Action: "delete", Resource: spec("old-vpc", "infra.vpc"), Current: cur}, + }, + } + fp := newCaptureFakeProvider() + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if len(result.Errors) != 0 { + t.Fatalf("unexpected errors: %v", result.Errors) + } + if fp.driver.deleteCount != 1 { + t.Errorf("driver.Delete must be called exactly once for delete action; got %d", fp.driver.deleteCount) + } + if got := fp.driver.deleteRef.ProviderID; got != knownID { + t.Errorf("Delete ResourceRef.ProviderID: got %q want %q", got, knownID) + } +} + +// TestApplyPlan_Delete_DriverErrorRecorded verifies that a driver-level +// delete failure is recorded in result.Errors with the action + +// resource captured, matching the per-action error contract used by +// the rest of the dispatch loop. +func TestApplyPlan_Delete_DriverErrorRecorded(t *testing.T) { + deleteErr := errors.New("delete failed: 503 service unavailable") + cur := &interfaces.ResourceState{Name: "old-vpc", Type: "infra.vpc", ProviderID: "do-id"} + plan := &interfaces.IaCPlan{ + Actions: []interfaces.PlanAction{ + {Action: "delete", Resource: spec("old-vpc", "infra.vpc"), Current: cur}, + }, + } + fp := newCaptureFakeProvider() + fp.driver.deleteErr = deleteErr + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if len(result.Errors) != 1 { + t.Fatalf("expected 1 per-action error; got %d (%v)", len(result.Errors), result.Errors) + } + if result.Errors[0].Action != "delete" { + t.Errorf("Action: got %q want %q", result.Errors[0].Action, "delete") + } + if result.Errors[0].Resource != "old-vpc" { + t.Errorf("Resource: got %q want %q", result.Errors[0].Resource, "old-vpc") + } + if got := result.Errors[0].Error; got == "" || got != deleteErr.Error() { + t.Errorf("Error: got %q want bare driver error %q", got, deleteErr.Error()) + } +} + +// TestApplyPlan_Update_DriverErrorRecorded mirrors the Delete error +// recording test — driver Update failures must surface in +// result.Errors with the canonical fields populated. +func TestApplyPlan_Update_DriverErrorRecorded(t *testing.T) { + updateErr := errors.New("update failed: 422 invalid spec") + cur := &interfaces.ResourceState{Name: "vpc-1", Type: "infra.vpc", ProviderID: "do-id"} + plan := &interfaces.IaCPlan{ + Actions: []interfaces.PlanAction{ + {Action: "update", Resource: spec("vpc-1", "infra.vpc"), Current: cur}, + }, + } + fp := newCaptureFakeProvider() + fp.driver.updateErr = updateErr + result, err := ApplyPlan(context.Background(), fp, plan) + if err != nil { + t.Fatal(err) + } + if len(result.Errors) != 1 { + t.Fatalf("expected 1 per-action error; got %d (%v)", len(result.Errors), result.Errors) + } + if result.Errors[0].Action != "update" { + t.Errorf("Action: got %q want %q", result.Errors[0].Action, "update") + } + if got := result.Errors[0].Error; got != updateErr.Error() { + t.Errorf("Error: got %q want bare driver error %q", got, updateErr.Error()) + } +} diff --git a/interfaces/iac_resource_driver.go b/interfaces/iac_resource_driver.go index 0efd2a369..058ceab0f 100644 --- a/interfaces/iac_resource_driver.go +++ b/interfaces/iac_resource_driver.go @@ -40,6 +40,26 @@ type ResourceAdoptionLocator interface { AdoptionRef(spec ResourceSpec) (ResourceRef, bool, error) } +// UpsertSupporter is an optional interface implemented by ResourceDrivers +// that support locating an existing resource by name alone (empty +// ProviderID) in their Read method. wfctlhelpers.ApplyPlan uses this hook +// to recover from Create that returns ErrResourceAlreadyExists: when the +// driver opts in via SupportsUpsert()==true, ApplyPlan calls Read with a +// name-only ResourceRef to obtain the existing ProviderID, then calls +// Update to bring the resource to the desired state — net effect of an +// idempotent upsert without requiring drivers to implement upsert +// natively. +// +// Drivers that do not implement this interface (or return false) yield +// the original ErrResourceAlreadyExists unchanged — ApplyPlan does NOT +// silently swallow the conflict. Implementations should return true only +// when their Read can locate a resource by Name + Type without a +// ProviderID; returning true while requiring a non-empty ProviderID in +// Read defeats the recovery path. +type UpsertSupporter interface { + SupportsUpsert() bool +} + // ResourceOutput is the concrete output of a provisioned or read resource. type ResourceOutput struct { Name string `json:"name"` diff --git a/interfaces/iac_state.go b/interfaces/iac_state.go index 668b834b1..c0dd3caf5 100644 --- a/interfaces/iac_state.go +++ b/interfaces/iac_state.go @@ -107,6 +107,37 @@ type ApplyResult struct { PlanID string `json:"plan_id"` Resources []ResourceOutput `json:"resources"` Errors []ActionError `json:"errors,omitempty"` + + // InitialInputSnapshot captures env-var fingerprints at the start of apply. + // Populated by wfctlhelpers.ApplyPlan (T3.1) at apply entry by snapshotting + // every name listed in plan.InputSnapshot through inputsnapshot.OSEnvProvider. + // Used by the deferred postcondition (T3.1.5) to compute the drift report + // against the apply-time snapshot. + InitialInputSnapshot map[string]string `json:"initial_input_snapshot,omitempty"` + + // InputDriftReport names env-vars whose fingerprint changed between plan + // and apply. Populated by the deferred postcondition in + // wfctlhelpers.ApplyPlan (T3.1.5) regardless of whether the apply + // itself succeeded or errored. Empty (or nil) means no drift detected. + // Entries are sorted by Name for deterministic comparison and stable + // diagnostic output; the sort guarantee is enforced by + // inputsnapshot.ComputeDrift (covered by + // TestComputeDrift_ResultIsSortedByName in + // iac/inputsnapshot/compute_drift_test.go). + InputDriftReport []DriftEntry `json:"input_drift_report,omitempty"` + + // ReplaceIDMap propagates new ProviderIDs from Replace actions to + // dependent resources whose Apply runs later in the same plan. + // Populated by doReplace (T3.4); consumed by JIT substitution in W-5 + // (T5.2/T5.3). + // + // Keyed by the *replaced* resource's Name (i.e., action.Resource.Name + // from the Replace PlanAction — the resource whose Delete-then-Create + // just produced a fresh ProviderID). The value is the new ProviderID + // returned from the post-Delete Create. Dependent resources reference + // the replaced resource by name in their config, so JIT substitution + // in W-5 translates "name → new ProviderID" via this map. + ReplaceIDMap map[string]string `json:"replace_id_map,omitempty"` } // DestroyResult summarises the outcome of a destroy operation. diff --git a/interfaces/iac_state_test.go b/interfaces/iac_state_test.go index 494bde584..90f4e0349 100644 --- a/interfaces/iac_state_test.go +++ b/interfaces/iac_state_test.go @@ -53,3 +53,103 @@ func TestPlanAction_ResolvedConfigHashField(t *testing.T) { t.Errorf("ResolvedConfigHash: got %q want %q", got.ResolvedConfigHash, realisticHash) } } + +// TestApplyResult_InputDriftReport_RoundTrip verifies the InputDriftReport +// field declared in T3.0.4. Field is populated by the deferred postcondition +// in wfctlhelpers.ApplyPlan (T3.1.5). +func TestApplyResult_InputDriftReport_RoundTrip(t *testing.T) { + r := ApplyResult{InputDriftReport: []DriftEntry{ + {Name: "STAGING_PG_PASSWORD", PlanFingerprint: "abc", ApplyFingerprint: "def"}, + }} + data, err := json.Marshal(r) + if err != nil { + t.Fatal(err) + } + var got ApplyResult + if err := json.Unmarshal(data, &got); err != nil { + t.Fatal(err) + } + if len(got.InputDriftReport) != 1 || got.InputDriftReport[0].Name != "STAGING_PG_PASSWORD" { + t.Errorf("InputDriftReport roundtrip failed: %+v", got) + } +} + +// TestApplyResult_InitialInputSnapshot_RoundTrip verifies the +// InitialInputSnapshot field declared in T3.0.4. Field is populated at apply +// entry by wfctlhelpers.ApplyPlan (T3.1) and consumed by the deferred +// postcondition (T3.1.5) when computing drift. +func TestApplyResult_InitialInputSnapshot_RoundTrip(t *testing.T) { + r := ApplyResult{InitialInputSnapshot: map[string]string{"FOO": "fp1234"}} + data, err := json.Marshal(r) + if err != nil { + t.Fatal(err) + } + var got ApplyResult + if err := json.Unmarshal(data, &got); err != nil { + t.Fatal(err) + } + if got.InitialInputSnapshot["FOO"] != "fp1234" { + t.Errorf("InitialInputSnapshot roundtrip failed: %+v", got) + } +} + +// TestApplyResult_ReplaceIDMap_RoundTrip verifies the ReplaceIDMap field +// declared in T3.0.4. Field is populated by doReplace (T3.4) and consumed +// by JIT substitution in W-5 (T5.2/T5.3). +func TestApplyResult_ReplaceIDMap_RoundTrip(t *testing.T) { + r := ApplyResult{ReplaceIDMap: map[string]string{"vpc": "new-uuid"}} + data, err := json.Marshal(r) + if err != nil { + t.Fatal(err) + } + var got ApplyResult + if err := json.Unmarshal(data, &got); err != nil { + t.Fatal(err) + } + if got.ReplaceIDMap["vpc"] != "new-uuid" { + t.Errorf("ReplaceIDMap roundtrip failed: %+v", got) + } +} + +// TestApplyResult_OmitEmptyContract locks in the omitempty JSON tag +// behavior on the three T3.0.4 fields. Both nil and empty-but-non-nil +// values must drop from the encoded form so plan/result transcripts stay +// lean and downstream consumers can treat "absent key" and "empty value" +// identically — matching the behavior already documented for +// IaCPlan.InputSnapshot and PlanAction.ResolvedConfigHash. +func TestApplyResult_OmitEmptyContract(t *testing.T) { + cases := map[string]ApplyResult{ + "nil-fields": {}, + "empty-non-nil-fields": { + InitialInputSnapshot: map[string]string{}, + InputDriftReport: []DriftEntry{}, + ReplaceIDMap: map[string]string{}, + }, + } + for name, r := range cases { + t.Run(name, func(t *testing.T) { + data, err := json.Marshal(r) + if err != nil { + t.Fatal(err) + } + s := string(data) + for _, key := range []string{"initial_input_snapshot", "input_drift_report", "replace_id_map"} { + if containsString(s, key) { + t.Errorf("expected %q to be omitted from %s; got %s", key, name, s) + } + } + }) + } +} + +// containsString is a tiny, dependency-free substring helper local to this +// test file so the omitempty test does not pull in strings just for one +// check (the file's other tests use only encoding/json + testing). +func containsString(s, substr string) bool { + for i := 0; i+len(substr) <= len(s); i++ { + if s[i:i+len(substr)] == substr { + return true + } + } + return false +} diff --git a/plugin/sdk/manifest.go b/plugin/sdk/manifest.go new file mode 100644 index 000000000..d9f22bfda --- /dev/null +++ b/plugin/sdk/manifest.go @@ -0,0 +1,136 @@ +// Package sdk hosts the plugin SDK manifest schema and helpers used by +// wfctl to discover plugin capabilities (currently: IaC dispatch version). +// +// The SDK manifest is intentionally additive over [plugin.PluginManifest]; +// it captures only the fields that wfctl reads at apply-time to choose +// between the v1 (legacy in-provider Apply) and v2 (wfctlhelpers.ApplyPlan) +// dispatch paths. +package sdk + +import ( + "bytes" + _ "embed" + "encoding/json" + "fmt" + "sync" + + "github.com/santhosh-tekuri/jsonschema/v6" +) + +// manifestSchemaJSON is the JSON Schema validating the SDK manifest. It is +// embedded so wfctl always validates against the schema version compiled +// into the binary, not whatever happens to be on disk. +// +//go:embed manifest_schema.json +var manifestSchemaJSON []byte + +// ManifestSchemaJSON returns the raw JSON Schema bytes used to validate +// SDK manifests. Exported for plugin authors and external tooling that +// want to validate plugin.json without depending on this package's +// ParseManifest entry point. +// +// Returns a copy so callers cannot mutate the embedded schema; the +// underlying slice from //go:embed is technically writable. +func ManifestSchemaJSON() []byte { + return bytes.Clone(manifestSchemaJSON) +} + +// Manifest captures the SDK-level fields wfctl reads from plugin.json. +// It is a strict subset of the full plugin.PluginManifest — only fields +// that gate apply-time dispatch live here. +type Manifest struct { + // Name is the plugin name. Carried for diagnostics; the SDK schema + // does not enforce shape (lowercase/hyphen rules live in plugin.PluginManifest). + Name string `json:"name"` + + // IaCProvider holds IaC-provider-specific manifest fields. Empty + // (zero-valued) when the plugin does not implement IaCProvider. + IaCProvider IaCProvider `json:"iacProvider"` +} + +// IaCProvider describes IaC-provider-specific manifest fields. +type IaCProvider struct { + // ComputePlanVersion selects the apply-time dispatch path: + // "" (default, treated as "v1"): legacy in-provider Apply switch. + // "v1": explicit legacy dispatch. + // "v2": route through wfctlhelpers.ApplyPlan + // (Replace + input-drift postcondition). + // Schema-validated against the enum ["v1","v2"]; "" passes validation + // because the field is optional. + ComputePlanVersion string `json:"computePlanVersion,omitempty"` +} + +// EffectiveComputePlanVersion returns the dispatch version, defaulting to +// "v1" when the manifest omits the field. Callers should always go through +// this accessor rather than reading ComputePlanVersion directly so the +// default-v1 contract stays in one place. +func (p IaCProvider) EffectiveComputePlanVersion() string { + if p.ComputePlanVersion == "" { + return "v1" + } + return p.ComputePlanVersion +} + +// compiledSchema is the parsed manifest schema. It is compiled lazily on +// first ParseManifest call and cached for the process lifetime; the schema +// is embedded and immutable, so a single compilation is correct. +// +// The compilation is guarded by sync.Once so concurrent callers cannot race +// on the cache pointer or on the jsonschema compiler's internal state. Both +// the success result and the error are captured so subsequent calls return +// the same outcome without re-compiling. +var ( + compiledSchema *jsonschema.Schema + compiledSchemaErr error + compiledSchemaOnce sync.Once +) + +// loadSchema compiles manifestSchemaJSON exactly once per process. Returns +// the compiled schema or an error wrapping the underlying compile failure +// (so failures surface with a clear "schema bug" diagnostic rather than as +// a generic "ParseManifest failed"). +func loadSchema() (*jsonschema.Schema, error) { + compiledSchemaOnce.Do(func() { + doc, err := jsonschema.UnmarshalJSON(bytes.NewReader(manifestSchemaJSON)) + if err != nil { + compiledSchemaErr = fmt.Errorf("sdk manifest schema: unmarshal: %w", err) + return + } + c := jsonschema.NewCompiler() + if err := c.AddResource("manifest.json", doc); err != nil { + compiledSchemaErr = fmt.Errorf("sdk manifest schema: add resource: %w", err) + return + } + s, err := c.Compile("manifest.json") + if err != nil { + compiledSchemaErr = fmt.Errorf("sdk manifest schema: compile: %w", err) + return + } + compiledSchema = s + }) + return compiledSchema, compiledSchemaErr +} + +// ParseManifest validates raw plugin.json bytes against the SDK schema and +// decodes them into a Manifest. Returns an error if the JSON is malformed +// or violates the schema (e.g., iacProvider.computePlanVersion not in +// {"v1","v2"}). Pure-additive: existing plugin.json files without an +// iacProvider key parse cleanly with a zero-valued IaCProvider. +func ParseManifest(data []byte) (*Manifest, error) { + s, err := loadSchema() + if err != nil { + return nil, err + } + doc, err := jsonschema.UnmarshalJSON(bytes.NewReader(data)) + if err != nil { + return nil, fmt.Errorf("manifest: invalid JSON: %w", err) + } + if err := s.Validate(doc); err != nil { + return nil, fmt.Errorf("manifest: schema validation failed: %w", err) + } + var m Manifest + if err := json.Unmarshal(data, &m); err != nil { + return nil, fmt.Errorf("manifest: decode: %w", err) + } + return &m, nil +} diff --git a/plugin/sdk/manifest_schema.json b/plugin/sdk/manifest_schema.json new file mode 100644 index 000000000..aec5a9767 --- /dev/null +++ b/plugin/sdk/manifest_schema.json @@ -0,0 +1,24 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/GoCodeAlone/workflow/plugin/sdk/manifest_schema.json", + "title": "Plugin SDK manifest", + "description": "Schema for the SDK-level plugin manifest (plugin.json) consumed by wfctl. Pure-additive; existing manifests without iacProvider remain valid. The root object permits additional properties so existing plugin.json files (with name/version/author/etc.) parse cleanly. The iacProvider sub-object is strict so typos like 'computeplanversion' or unknown keys are rejected at parse time, not silently downgraded to v1 dispatch.", + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "iacProvider": { + "type": "object", + "description": "IaC-provider-specific manifest fields. Only present for plugins that implement the IaCProvider interface.", + "additionalProperties": false, + "properties": { + "computePlanVersion": { + "type": "string", + "description": "Selects the apply-time dispatch path. v1 (default when omitted) routes through the legacy in-provider Apply switch. v2 routes through wfctlhelpers.ApplyPlan with full Replace + drift-postcondition support.", + "enum": ["v1", "v2"] + } + } + } + } +} diff --git a/plugin/sdk/manifest_test.go b/plugin/sdk/manifest_test.go new file mode 100644 index 000000000..4112e7bd9 --- /dev/null +++ b/plugin/sdk/manifest_test.go @@ -0,0 +1,137 @@ +package sdk + +import ( + "strings" + "sync" + "testing" +) + +// TestManifest_IaCProvider_ComputePlanVersion exercises the new +// iacProvider.computePlanVersion field. Cases: +// - default-v1: field omitted → EffectiveComputePlanVersion() == "v1" +// - explicit-v1: "v1" → "v1" +// - explicit-v2: "v2" → "v2" +// - rejected: "v3" → ParseManifest returns an error (schema-rejected) +func TestManifest_IaCProvider_ComputePlanVersion(t *testing.T) { + cases := map[string]struct { + in string + want string + wantErr bool + }{ + "default-v1": {`{"name":"x","iacProvider":{}}`, "v1", false}, + "explicit-v1": {`{"name":"x","iacProvider":{"computePlanVersion":"v1"}}`, "v1", false}, + "explicit-v2": {`{"name":"x","iacProvider":{"computePlanVersion":"v2"}}`, "v2", false}, + "rejected": {`{"name":"x","iacProvider":{"computePlanVersion":"v3"}}`, "", true}, + } + for name, c := range cases { + t.Run(name, func(t *testing.T) { + m, err := ParseManifest([]byte(c.in)) + if (err != nil) != c.wantErr { + t.Fatalf("err=%v wantErr=%v", err, c.wantErr) + } + if !c.wantErr && m.IaCProvider.EffectiveComputePlanVersion() != c.want { + t.Errorf("got %q want %q", m.IaCProvider.EffectiveComputePlanVersion(), c.want) + } + }) + } +} + +// TestManifest_IaCProvider_ComputePlanVersion_ZeroValue verifies that an +// IaCProvider with the zero value (empty string) reports v1, matching the +// "default-v1" case but exercising the accessor on a Go-zero-valued struct +// (no JSON involved). +func TestManifest_IaCProvider_ComputePlanVersion_ZeroValue(t *testing.T) { + var p IaCProvider + if got := p.EffectiveComputePlanVersion(); got != "v1" { + t.Errorf("zero IaCProvider.EffectiveComputePlanVersion() = %q, want %q", got, "v1") + } +} + +// TestManifest_IaCProvider_RejectsTypoKey verifies that a typo inside +// iacProvider (e.g., the lowercase "computeplanversion") is rejected by +// the schema rather than silently parsing to a zero-valued IaCProvider — +// which would produce a silent v1 dispatch downgrade. The root object +// stays permissive so plugin.json files with version/author/etc. still +// parse, but iacProvider is strict by design. +func TestManifest_IaCProvider_RejectsTypoKey(t *testing.T) { + cases := []string{ + `{"name":"x","iacProvider":{"computeplanversion":"v2"}}`, // lowercase typo + `{"name":"x","iacProvider":{"foo":"bar"}}`, // unknown key + } + for _, in := range cases { + t.Run(in, func(t *testing.T) { + _, err := ParseManifest([]byte(in)) + if err == nil { + t.Errorf("expected schema rejection for %q; got nil", in) + } + }) + } +} + +// TestManifest_RootPermitsAdditionalProperties verifies that the root +// object accepts unknown top-level keys so existing plugin.json files +// (which carry version/author/dependencies/etc.) parse cleanly through +// the SDK manifest. Pure-additive contract. +func TestManifest_RootPermitsAdditionalProperties(t *testing.T) { + in := `{"name":"x","version":"1.2.3","author":"jane","description":"hi","iacProvider":{"computePlanVersion":"v2"}}` + m, err := ParseManifest([]byte(in)) + if err != nil { + t.Fatalf("expected pass; got %v", err) + } + if m.IaCProvider.EffectiveComputePlanVersion() != "v2" { + t.Errorf("got %q want %q", m.IaCProvider.EffectiveComputePlanVersion(), "v2") + } +} + +// TestManifestSchemaJSON_ReturnsCopy verifies that mutating the slice +// returned from ManifestSchemaJSON cannot affect the embedded schema +// observed by subsequent callers. +func TestManifestSchemaJSON_ReturnsCopy(t *testing.T) { + a := ManifestSchemaJSON() + if len(a) == 0 { + t.Fatal("ManifestSchemaJSON() returned empty bytes") + } + a[0] = 0xFF // attempt to corrupt + b := ManifestSchemaJSON() + if b[0] == 0xFF { + t.Error("ManifestSchemaJSON returned a shared slice; embedded schema is mutable from callers") + } + if !strings.Contains(string(b), "computePlanVersion") { + t.Error("ManifestSchemaJSON copy lost the schema body") + } +} + +// TestParseManifest_ConcurrentRaceFree exercises the race detector against +// the lazy schema cache: 32 goroutines call ParseManifest simultaneously +// before any sequential call has populated the cache. Run under -race; a +// failure here means the sync.Once guard around compiledSchema regressed. +// +// Note: this test relies on the package-init ordering — Go's testing +// framework gives each test a fresh goroutine but the package globals +// persist across tests in the same binary, so by the time this test runs +// other tests may already have warmed the cache. The test is still useful +// because it stresses concurrent reads of the cached pointer and any +// internal state inside the jsonschema.Schema.Validate path. +func TestParseManifest_ConcurrentRaceFree(t *testing.T) { + const goroutines = 32 + const inputs = 4 + manifests := []string{ + `{"name":"a","iacProvider":{}}`, + `{"name":"b","iacProvider":{"computePlanVersion":"v1"}}`, + `{"name":"c","iacProvider":{"computePlanVersion":"v2"}}`, + `{"name":"d","iacProvider":{"computePlanVersion":"v3"}}`, // expected to error + } + var wg sync.WaitGroup + start := make(chan struct{}) + wg.Add(goroutines) + for i := range goroutines { + go func(idx int) { + defer wg.Done() + <-start // align goroutines for maximum concurrent pressure on loadSchema + in := manifests[idx%inputs] + _, _ = ParseManifest([]byte(in)) + }(i) + } + close(start) + wg.Wait() +}