From be2ffee9b5461fcb05d7d4f7b8795ad5a6623093 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 17 May 2026 01:11:30 -0400 Subject: [PATCH] feat(wfctl): validate iacProvider.computePlanVersion in plugin manifest (#693) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2.1 follow-up to workflow#640 Phase 2 cascade (PR #694, v0.54.0). Per ADR 0024 + ADR 0040 hard-cutover discipline + Phase 1 Assumption 8: findIaCPluginDir previously json.Unmarshal'd the matching plugin manifest and returned the raw iacProvider.computePlanVersion string without validation. A typo (V2, v2.0, two, v3) would silently route through the v1 dispatch path via wfctlhelpers.DispatchVersionFor's empty/unknown default — breaking the Phase 2 hard-cutover contract. Validation gate now hard-fails on the matching plugin manifest when computePlanVersion is not in {"", "v1", "v2"}. Operators see the misconfiguration loudly with an actionable error instead of silent fallback. Empty/missing field still permitted (defaults to v1 dispatch). Per workflow#693 issue body's 3-option design space, picked option 3 (lightweight enum check, minimum viable) over option 1 (full pluginmanifest package, ~200 LOC overkill) and option 2 (reuse existing schema/ JSON Schema validator, ~50 LOC). YAGNI applies — the field's domain is a 3-element enum closed by the proto spec. 7 test cases (table-driven): empty, v1, v2 pass; V2 (uppercase typo), v2.0 (decimal typo), two (word typo), v3 (future Phase 2.3 tag pre-introduction) all reject with the actionable error. Closes #693. --- cmd/wfctl/deploy_providers.go | 17 +++++ ...loy_providers_compute_plan_version_test.go | 74 +++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 cmd/wfctl/deploy_providers_compute_plan_version_test.go diff --git a/cmd/wfctl/deploy_providers.go b/cmd/wfctl/deploy_providers.go index 59642501..098fcd62 100644 --- a/cmd/wfctl/deploy_providers.go +++ b/cmd/wfctl/deploy_providers.go @@ -151,6 +151,23 @@ func findIaCPluginDir(pluginDir, providerName string) (name, computePlanVersion if m.Capabilities.IaCProvider.Name != providerName { continue } + // Per workflow#693 (Phase 2.1 follow-up to #640): validate + // iacProvider.computePlanVersion ∈ {"", "v1", "v2"} on the + // matching plugin manifest. A typo (e.g. "V2", "v2.0", "two") + // would silently route through the v1 dispatch path via + // wfctlhelpers.DispatchVersionFor's empty/unknown default, + // breaking the Phase 2 hard-cutover contract per ADR 0024 + + // ADR 0040. Hard-fail so operators see the misconfiguration + // loudly instead of silently dispatching to the wrong path. + switch m.IaCProvider.ComputePlanVersion { + case "", "v1", "v2": + // valid + default: + return "", "", false, fmt.Errorf( + "plugin %q manifest has invalid iacProvider.computePlanVersion %q (must be \"\", \"v1\", or \"v2\")", + pluginName, m.IaCProvider.ComputePlanVersion, + ) + } binaryPath := filepath.Join(pluginDir, pluginName, pluginName) _, statErr := os.Stat(binaryPath) return pluginName, m.IaCProvider.ComputePlanVersion, statErr == nil, nil diff --git a/cmd/wfctl/deploy_providers_compute_plan_version_test.go b/cmd/wfctl/deploy_providers_compute_plan_version_test.go new file mode 100644 index 00000000..158b0be6 --- /dev/null +++ b/cmd/wfctl/deploy_providers_compute_plan_version_test.go @@ -0,0 +1,74 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestFindIaCPluginDir_ComputePlanVersionValidation pins the workflow#693 +// (Phase 2.1 follow-up to #640) manifest validation gate: invalid values +// of iacProvider.computePlanVersion on the matching plugin manifest must +// hard-fail at findIaCPluginDir so operators see misconfiguration loudly +// instead of silently routing through the v1 dispatch fallback (which +// would break the Phase 2 hard-cutover contract per ADR 0024 + ADR 0040). +func TestFindIaCPluginDir_ComputePlanVersionValidation(t *testing.T) { + tests := []struct { + name string + computePlanVer string + wantErrSubstring string + wantVersionReturn string + }{ + {name: "empty defaults to v1 dispatch", computePlanVer: "", wantVersionReturn: ""}, + {name: "v1 explicit", computePlanVer: "v1", wantVersionReturn: "v1"}, + {name: "v2 explicit (Phase 2)", computePlanVer: "v2", wantVersionReturn: "v2"}, + {name: "typo uppercase rejected", computePlanVer: "V2", wantErrSubstring: `invalid iacProvider.computePlanVersion "V2"`}, + {name: "typo decimal rejected", computePlanVer: "v2.0", wantErrSubstring: `invalid iacProvider.computePlanVersion "v2.0"`}, + {name: "typo word rejected", computePlanVer: "two", wantErrSubstring: `invalid iacProvider.computePlanVersion "two"`}, + {name: "phase-2.3 future-tag rejected pre-introduction", computePlanVer: "v3", wantErrSubstring: `invalid iacProvider.computePlanVersion "v3"`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pluginDir := t.TempDir() + pluginName := "workflow-plugin-test-" + tt.name + pluginName = strings.ReplaceAll(pluginName, " ", "-") + subDir := filepath.Join(pluginDir, pluginName) + if mkErr := os.Mkdir(subDir, 0o755); mkErr != nil { + t.Fatalf("mkdir: %v", mkErr) + } + manifest := `{ + "name": "` + pluginName + `", + "version": "1.0.0", + "capabilities": {"iacProvider": {"name": "test-provider"}}, + "iacProvider": {"computePlanVersion": "` + tt.computePlanVer + `"} + }` + if writeErr := os.WriteFile(filepath.Join(subDir, "plugin.json"), []byte(manifest), 0o644); writeErr != nil { + t.Fatalf("write manifest: %v", writeErr) + } + + name, gotVer, _, err := findIaCPluginDir(pluginDir, "test-provider") + + if tt.wantErrSubstring != "" { + if err == nil { + t.Fatalf("expected error containing %q, got nil (name=%q ver=%q)", tt.wantErrSubstring, name, gotVer) + } + if !strings.Contains(err.Error(), tt.wantErrSubstring) { + t.Errorf("error mismatch:\n got: %v\n want substring: %q", err, tt.wantErrSubstring) + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if name != pluginName { + t.Errorf("name = %q; want %q", name, pluginName) + } + if gotVer != tt.wantVersionReturn { + t.Errorf("computePlanVersion = %q; want %q", gotVer, tt.wantVersionReturn) + } + }) + } +}