From 384691eca42b8feeb6459dbbab6be6c4b2d4d72d Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 5 May 2026 06:55:35 -0400 Subject: [PATCH 1/4] refactor(wfctl): remoteIaCProvider delegates Plan/Apply to wfctlhelpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the v1-wire monolithic IaCProvider.Plan / IaCProvider.Apply proxy bodies with the canonical 2-statement / 1-statement delegations recognized by cmd/iac-codemod's lint analyzers: - Plan() -> plan, err := platform.ComputePlan(ctx, r, desired, current) return &plan, err - Apply() -> return wfctlhelpers.ApplyPlan(ctx, r, plan) Plan delegation drives per-resource Diff dispatch through remoteResourceDriver, preserving the gRPC fan-out for the classification path. Apply delegation routes per-action through ResourceDriver.{Create,Update,Delete,Replace}, which the proxy already forwards over gRPC. The legacy plugin-side IaCProvider.Apply endpoint is no longer reachable through wfctl; both branches of the dispatch gate in infra_apply.go now route through the same per-driver dispatch. Test updates: - deploy_providers_remote_iac_test.go: replace TestRemoteIaC_Plan / TestRemoteIaC_Apply (which asserted on the now-gone IaCProvider.* wire methods) with delegation-path tests that pin platform.ComputePlan net-new + delete classification and wfctlhelpers.ApplyPlan per-driver dispatch + per-action error decomposition. - deploy_providers_dispatch_matrix_test.go: drop the Plan / Apply rows from the dispatch matrix (no longer applicable; their coverage moves to TestDispatchMatrix_RemoteResourceDriver and the canonical tests). Validates per cmd/iac-codemod lint: - Pre-refactor: 3 findings on deploy_providers.go (Plan / Apply non-canonical + ProviderValidator missing). - Post-refactor: 1 finding (ProviderValidator — orthogonal, pre-existing, out of W-Refactor scope). Closes the W-8 lint surface for remoteIaCProvider per docs/plans/2026-05-05-iac-deferred-cleanup.md PR 5 / Task 5.1. Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/wfctl/deploy_providers.go | 75 ++++---- .../deploy_providers_dispatch_matrix_test.go | 29 +-- cmd/wfctl/deploy_providers_remote_iac_test.go | 182 +++++++++++------- 3 files changed, 158 insertions(+), 128 deletions(-) diff --git a/cmd/wfctl/deploy_providers.go b/cmd/wfctl/deploy_providers.go index f5c3d3b51..7bd2cd14f 100644 --- a/cmd/wfctl/deploy_providers.go +++ b/cmd/wfctl/deploy_providers.go @@ -18,7 +18,9 @@ import ( "time" "github.com/GoCodeAlone/workflow/config" + "github.com/GoCodeAlone/workflow/iac/wfctlhelpers" "github.com/GoCodeAlone/workflow/interfaces" + "github.com/GoCodeAlone/workflow/platform" "github.com/GoCodeAlone/workflow/plugin" "github.com/GoCodeAlone/workflow/plugin/external" "google.golang.org/grpc/codes" @@ -337,43 +339,42 @@ func (r *remoteIaCProvider) Capabilities() []interfaces.IaCCapabilityDeclaration return caps } -func (r *remoteIaCProvider) Plan(_ context.Context, desired []interfaces.ResourceSpec, current []interfaces.ResourceState) (*interfaces.IaCPlan, error) { - desiredAny, err := jsonToAny(desired) - if err != nil { - return nil, fmt.Errorf("IaCProvider.Plan: marshal desired: %w", err) - } - currentAny, err := jsonToAny(current) - if err != nil { - return nil, fmt.Errorf("IaCProvider.Plan: marshal current: %w", err) - } - res, err := r.invoker.InvokeService("IaCProvider.Plan", map[string]any{ - "desired": desiredAny, - "current": currentAny, - }) - if err != nil { - return nil, err - } - var plan interfaces.IaCPlan - if err := anyToStruct(res, &plan); err != nil { - return nil, fmt.Errorf("IaCProvider.Plan: decode result: %w", err) - } - return &plan, nil -} - -func (r *remoteIaCProvider) Apply(_ context.Context, plan *interfaces.IaCPlan) (*interfaces.ApplyResult, error) { - planAny, err := jsonToAny(plan) - if err != nil { - return nil, fmt.Errorf("IaCProvider.Apply: marshal plan: %w", err) - } - res, err := r.invoker.InvokeService("IaCProvider.Apply", map[string]any{"plan": planAny}) - if err != nil { - return nil, err - } - var result interfaces.ApplyResult - if err := anyToStruct(res, &result); err != nil { - return nil, fmt.Errorf("IaCProvider.Apply: decode result: %w", err) - } - return &result, nil +// Plan delegates to platform.ComputePlan, the canonical wfctl-side diff +// engine. The pre-W-Refactor body proxied a monolithic IaCProvider.Plan +// call to the plugin via InvokeService — a v1-wire shape that platform/ +// differ.go superseded once Diff dispatch landed (W-3 / W-7). Delegating +// here keeps the wfctl-loaded provider's Plan body in lockstep with the +// canonical Plan flow that infra_apply.go's computeInfraPlan var already +// uses; the lint analyzer in cmd/iac-codemod/lint.go::AssertPlanDelegates +// ToHelper now reports zero findings on this file. The 2-statement +// `plan, err := platform.ComputePlan(...); return &plan, err` shape is +// the canonical form recognized by isAlreadyDelegatedPlanBody (the +// rewriter's idempotency check). ResourceDriver.Diff for per-resource +// classification still hits the plugin via remoteResourceDriver, so the +// gRPC fan-out is preserved. +func (r *remoteIaCProvider) Plan(ctx context.Context, desired []interfaces.ResourceSpec, current []interfaces.ResourceState) (*interfaces.IaCPlan, error) { + plan, err := platform.ComputePlan(ctx, r, desired, current) + return &plan, err +} + +// Apply delegates to wfctlhelpers.ApplyPlan, the canonical per-action +// dispatcher. The pre-W-Refactor body proxied a monolithic IaCProvider. +// Apply call to the plugin via InvokeService — a v1-wire shape that +// wfctlhelpers.ApplyPlan supersedes by fanning out per-action through +// ResourceDriver.{Create,Update,Delete,Replace} (which remoteResource +// Driver still proxies via gRPC). The single-statement +// `return wfctlhelpers.ApplyPlan(ctx, p, plan)` shape is the canonical +// form recognized by isAlreadyDelegatedApplyBody. +// +// Note: this delegation also activates wfctlhelpers.ApplyPlan's drift +// postcondition + per-action error decomposition for any plugin loaded +// via this proxy, regardless of plugin.json's iacProvider.compute +// PlanVersion declaration. The dispatch gate in infra_apply.go that +// branches on DispatchVersionFor remains, but both branches now route +// through the same per-driver dispatch — the v1 monolithic Apply +// endpoint is no longer reachable through wfctl. +func (r *remoteIaCProvider) Apply(ctx context.Context, plan *interfaces.IaCPlan) (*interfaces.ApplyResult, error) { + return wfctlhelpers.ApplyPlan(ctx, r, plan) } func (r *remoteIaCProvider) Destroy(_ context.Context, refs []interfaces.ResourceRef) (*interfaces.DestroyResult, error) { diff --git a/cmd/wfctl/deploy_providers_dispatch_matrix_test.go b/cmd/wfctl/deploy_providers_dispatch_matrix_test.go index e1d699b9e..e293415df 100644 --- a/cmd/wfctl/deploy_providers_dispatch_matrix_test.go +++ b/cmd/wfctl/deploy_providers_dispatch_matrix_test.go @@ -248,26 +248,15 @@ func TestDispatchMatrix_RemoteIaCProvider(t *testing.T) { requiredKeys []string invoke func(p *remoteIaCProvider, ri *recordingInvoker) error }{ - { - name: "Plan", - wantMethod: "IaCProvider.Plan", - requiredKeys: []string{"desired", "current"}, - invoke: func(p *remoteIaCProvider, ri *recordingInvoker) error { - ri.resp = map[string]any{} - _, err := p.Plan(ctx, nil, nil) - return err - }, - }, - { - name: "Apply", - wantMethod: "IaCProvider.Apply", - requiredKeys: []string{"plan"}, - invoke: func(p *remoteIaCProvider, ri *recordingInvoker) error { - ri.resp = map[string]any{} - _, err := p.Apply(ctx, &interfaces.IaCPlan{}) - return err - }, - }, + // Plan and Apply intentionally absent from this matrix. + // Per W-Refactor (PR 5), remoteIaCProvider.Plan delegates to + // platform.ComputePlan and Apply delegates to wfctlhelpers.ApplyPlan. + // Neither method forwards a monolithic IaCProvider.Plan / + // IaCProvider.Apply RPC anymore — the per-action driver dispatch + // (ResourceDriver.Diff / Create / Update / Delete) covers the + // underlying gRPC traffic and is exercised by + // TestDispatchMatrix_RemoteResourceDriver. Canonical-delegation + // behavior is pinned in deploy_providers_remote_iac_test.go. { name: "Destroy", wantMethod: "IaCProvider.Destroy", diff --git a/cmd/wfctl/deploy_providers_remote_iac_test.go b/cmd/wfctl/deploy_providers_remote_iac_test.go index fbc87ca40..8ca09bc46 100644 --- a/cmd/wfctl/deploy_providers_remote_iac_test.go +++ b/cmd/wfctl/deploy_providers_remote_iac_test.go @@ -67,81 +67,108 @@ func TestRemoteIaC_Capabilities_Error(t *testing.T) { } // ── Plan ────────────────────────────────────────────────────────────────────── - -func samplePlanResponse() map[string]any { - return map[string]any{ - "id": "plan-abc", - "actions": []any{ - map[string]any{ - "action": "create", - "resource": map[string]any{ - "name": "db", - "type": "infra.database", - "config": map[string]any{}, - }, - }, - }, - "created_at": time.Now().Format(time.RFC3339Nano), - } -} - -func TestRemoteIaC_Plan(t *testing.T) { - si := &stubInvoker{resp: samplePlanResponse()} +// +// Pre-W-Refactor, Plan() proxied a monolithic IaCProvider.Plan call to +// the plugin via InvokeService. After W-Refactor (PR 5), Plan() delegates +// to platform.ComputePlan, which classifies actions locally: +// - net-new resources (no current state) → emit "create" without +// dispatching ResourceDriver.Diff (no InvokeService traffic); +// - removed-from-desired resources → emit "delete" without dispatching +// Diff (also no InvokeService traffic); +// - updates/replaces fan out via remoteResourceDriver.Diff → +// InvokeService("ResourceDriver.Diff", ...). +// +// The test below pins the create-only path because it exercises the +// canonical-delegation contract without standing up the full diff-cache +// + Diff dispatch. The delegation-via-driver path is covered by the +// canonical test below + the existing remote-driver tests. + +func TestRemoteIaC_Plan_DelegatesToComputePlan_NetNewResource(t *testing.T) { + // stubInvoker tracks the LAST InvokeService call. With ComputePlan + // delegation, a net-new resource emits "create" without touching the + // invoker — confirming the per-resource Diff fan-out is bypassed for + // creates and the v1-monolithic IaCProvider.Plan wire is gone. + si := &stubInvoker{} p := newIaCProvider(si) desired := []interfaces.ResourceSpec{ {Name: "db", Type: "infra.database", Config: map[string]any{"engine": "postgres"}}, } - current := []interfaces.ResourceState{ - {Name: "old-db", Type: "infra.database", ProviderID: "pid-old"}, - } - - plan, err := p.Plan(context.Background(), desired, current) + // No current state → "db" is net-new → ComputePlan emits "create" + // without calling driver.Diff. + plan, err := p.Plan(context.Background(), desired, nil) if err != nil { t.Fatalf("Plan: unexpected error: %v", err) } - if si.method != "IaCProvider.Plan" { - t.Errorf("method: got %q, want IaCProvider.Plan", si.method) - } - // Args must include desired and current as slices - if _, ok := si.args["desired"]; !ok { - t.Error("missing arg key 'desired'") - } - if _, ok := si.args["current"]; !ok { - t.Error("missing arg key 'current'") + if si.method != "" { + t.Errorf("invoker.method: net-new create path should not call InvokeService; got %q", si.method) } - if plan.ID != "plan-abc" { - t.Errorf("plan ID: got %q", plan.ID) + if plan == nil { + t.Fatal("Plan returned nil plan") } if len(plan.Actions) != 1 { - t.Fatalf("expected 1 action, got %d", len(plan.Actions)) + t.Fatalf("expected 1 action (create), got %d", len(plan.Actions)) } if plan.Actions[0].Action != "create" { - t.Errorf("action: got %q", plan.Actions[0].Action) + t.Errorf("action: got %q, want %q", plan.Actions[0].Action, "create") + } + if plan.Actions[0].Resource.Name != "db" { + t.Errorf("action resource name: got %q, want %q", plan.Actions[0].Resource.Name, "db") } } -func TestRemoteIaC_Plan_Error(t *testing.T) { - si := &stubInvoker{err: fmt.Errorf("rpc error")} +func TestRemoteIaC_Plan_DelegatesToComputePlan_DeleteEmittedForRemoved(t *testing.T) { + // A resource present in current state but absent from desired must + // emit "delete". ComputePlan emits deletes without calling driver.Diff. + si := &stubInvoker{} p := newIaCProvider(si) - _, err := p.Plan(context.Background(), nil, nil) - if err == nil { - t.Fatal("expected error") + + current := []interfaces.ResourceState{ + {Name: "old-db", Type: "infra.database", ProviderID: "pid-old"}, + } + plan, err := p.Plan(context.Background(), nil, current) + if err != nil { + t.Fatalf("Plan: unexpected error: %v", err) + } + if si.method != "" { + t.Errorf("invoker.method: delete path should not call InvokeService; got %q", si.method) + } + if plan == nil { + t.Fatal("Plan returned nil plan") + } + if len(plan.Actions) != 1 { + t.Fatalf("expected 1 action (delete), got %d", len(plan.Actions)) + } + if plan.Actions[0].Action != "delete" { + t.Errorf("action: got %q, want %q", plan.Actions[0].Action, "delete") } } // ── Apply ───────────────────────────────────────────────────────────────────── - -func TestRemoteIaC_Apply(t *testing.T) { +// +// Pre-W-Refactor, Apply() proxied a monolithic IaCProvider.Apply call to +// the plugin via InvokeService. After W-Refactor (PR 5), Apply() delegates +// to wfctlhelpers.ApplyPlan, which fans out per-action through +// ResourceDriver.{Create,Update,Delete,Replace}. Through the remoteIaC +// Provider those calls hit InvokeService("ResourceDriver.Create", ...) etc. +// +// The tests below pin the canonical delegation by: +// (a) verifying the wire-method is the per-driver call (not the legacy +// IaCProvider.Apply); and +// (b) verifying the ApplyResult is populated with the correct PlanID +// (set by ApplyPlan from plan.ID) regardless of the per-driver +// response shape. + +func TestRemoteIaC_Apply_DelegatesToApplyPlan_PerDriverDispatch(t *testing.T) { + // ApplyPlan dispatches Create on a single-create plan via + // remoteResourceDriver, which invokes "ResourceDriver.Create" through + // the stub invoker. The legacy "IaCProvider.Apply" wire is gone. si := &stubInvoker{resp: map[string]any{ - "plan_id": "plan-abc", - "resources": []any{ - map[string]any{ - "provider_id": "pid-123", - "name": "db", - "type": "infra.database", - "status": "running", - }, + "output": map[string]any{ + "provider_id": "pid-123", + "name": "db", + "type": "infra.database", + "status": "running", }, }} p := newIaCProvider(si) @@ -152,35 +179,48 @@ func TestRemoteIaC_Apply(t *testing.T) { {Action: "create", Resource: interfaces.ResourceSpec{Name: "db", Type: "infra.database"}}, }, } - result, err := p.Apply(context.Background(), plan) if err != nil { t.Fatalf("Apply: unexpected error: %v", err) } - if si.method != "IaCProvider.Apply" { - t.Errorf("method: got %q, want IaCProvider.Apply", si.method) + if si.method == "IaCProvider.Apply" { + t.Error("invoker.method: legacy IaCProvider.Apply wire should not be invoked after W-Refactor") } - // Plan must be wrapped under "plan" key - if _, ok := si.args["plan"]; !ok { - t.Error("missing arg key 'plan'") + if !strings.HasPrefix(si.method, "ResourceDriver.") { + t.Errorf("invoker.method: expected ResourceDriver.* per-driver dispatch, got %q", si.method) } - if result.PlanID != "plan-abc" { - t.Errorf("PlanID: got %q", result.PlanID) - } - if len(result.Resources) != 1 { - t.Fatalf("expected 1 resource, got %d", len(result.Resources)) + if result == nil { + t.Fatal("Apply returned nil result") } - if result.Resources[0].Name != "db" { - t.Errorf("resource name: got %q", result.Resources[0].Name) + if result.PlanID != "plan-abc" { + t.Errorf("PlanID: got %q, want %q (ApplyPlan stamps plan.ID onto result)", result.PlanID, "plan-abc") } } -func TestRemoteIaC_Apply_Error(t *testing.T) { - si := &stubInvoker{err: fmt.Errorf("apply failed")} +func TestRemoteIaC_Apply_DelegatesToApplyPlan_RecordsErrorsPerAction(t *testing.T) { + // When the underlying driver returns an error, ApplyPlan records it + // in result.Errors rather than returning the error from the top-level + // call (per the per-action error decomposition contract). This pins + // the canonical-delegation behavior — pre-refactor the same error + // would have been returned directly. + si := &stubInvoker{err: fmt.Errorf("driver create failed")} p := newIaCProvider(si) - _, err := p.Apply(context.Background(), &interfaces.IaCPlan{ID: "p1"}) - if err == nil { - t.Fatal("expected error") + + plan := &interfaces.IaCPlan{ + ID: "p1", + Actions: []interfaces.PlanAction{ + {Action: "create", Resource: interfaces.ResourceSpec{Name: "db", Type: "infra.database"}}, + }, + } + result, err := p.Apply(context.Background(), plan) + if err != nil { + t.Fatalf("Apply: unexpected top-level error (wfctlhelpers.ApplyPlan records per-action errors): %v", err) + } + if result == nil { + t.Fatal("Apply returned nil result") + } + if len(result.Errors) == 0 { + t.Error("expected ApplyResult.Errors to contain the per-action driver error") } } From 46235718d37eb3d695aa8495c4dae18803dbd1b4 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 5 May 2026 06:55:44 -0400 Subject: [PATCH 2/4] docs(adr): add ADR 010 - platform-vs-provider conformance scenarios MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codify the two classes of conformance scenario surfaced during W-7 implementation: 8 of 12 scenarios assert against cfg.Provider() (per- provider behavior); 4 of 12 scenarios bypass cfg.Provider and exercise platform-shared surfaces (inputsnapshot, jitsubst, wfctlhelpers) directly. Documents the rationale (some root-cause issues live at the platform layer, not at the per-provider layer; wrapping in cfg.Provider() would be vestigial), the consequences (future scenarios must classify at design time and document the choice in the body comment), and the alternatives considered (bypass validateConfig / split into a separate package — both rejected). Per docs/plans/2026-05-05-iac-deferred-cleanup.md PR 5 / Task 5.2. Co-Authored-By: Claude Opus 4.7 (1M context) --- ...tform-vs-provider-conformance-scenarios.md | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 docs/adr/010-platform-vs-provider-conformance-scenarios.md diff --git a/docs/adr/010-platform-vs-provider-conformance-scenarios.md b/docs/adr/010-platform-vs-provider-conformance-scenarios.md new file mode 100644 index 000000000..1eb291b1f --- /dev/null +++ b/docs/adr/010-platform-vs-provider-conformance-scenarios.md @@ -0,0 +1,51 @@ +# ADR 010: Platform-vs-Provider Conformance Scenario Classification + +**Status:** Accepted +**Date:** 2026-05-05 +**Context:** W-7 (workflow PR #535) shipped 12 conformance scenarios in `iac/conformance/`. During implementation, 4 scenarios diverged from the typical pattern of asserting against `cfg.Provider()`; instead they exercise platform-shared surfaces directly. + +## Decision + +We codify TWO classes of conformance scenario: + +**Provider-level scenarios** (8 of 12): assert against `cfg.Provider()`. Exercise per-provider behavior (Diff, Apply, ResourceDriver lookup, etc.). Scenarios: +- Scenario_NeedsReplaceTriggersReplaceAction +- Scenario_DeleteActionInApplyInvokesDriverDelete +- Scenario_DiffSurvivesGRPCRoundTrip +- Scenario_OutputsRefreshDetectsNewFields +- Scenario_CrossResourceConstraintRejection +- Scenario_OutputsConsistencyAcrossReadCycles +- Scenario_ReplaceCascadePreservesDependents +- Scenario_UpsertOnAlreadyExists + +**Platform-level scenarios** (4 of 12): bypass `cfg.Provider()`; exercise platform-shared surfaces (`inputsnapshot`, `jitsubst`, `wfctlhelpers`). cfg.Provider is required by Run's validateConfig precondition but intentionally NOT invoked. Scenarios: +- Scenario_PlanStaleDiagnostic — exercises `inputsnapshot.NewStaleError` +- Scenario_InfraOutputCrossModuleResolution — exercises `jitsubst.ResolveSpec` +- Scenario_ProtectedReplaceWithoutOverride — exercises `wfctlhelpers.ValidateAllowReplaceProtected` +- Scenario_ProtectedReplaceWithOverride — exercises `wfctlhelpers.ValidateAllowReplaceProtected` + +Each platform-level scenario carries a body comment naming the platform surface it exercises and explaining why cfg.Provider is unused. + +## Rationale + +Some root-cause issues from the IaC conformance plan (e.g. plan-stale-diagnostic, JIT secret resolution, --allow-replace gate) live at the platform layer (cross-provider-shared code), not at the per-provider layer. Conformance scenarios for those issues SHOULD test the platform surface directly — wrapping in `cfg.Provider()` calls would be vestigial. + +The 4-of-12 ratio is acceptable; the boundary is non-arbitrary (each platform-level scenario tests code that lives in `iac/` or `iac/wfctlhelpers/`, not in any specific provider). + +## Consequences + +- Future contributors adding conformance scenarios must classify their scenario at design time and document the choice in the body comment. +- The Run dispatcher does not need code changes for this classification; it already validates `cfg.Provider != nil` regardless. Platform-level scenarios pass a NoopProvider to satisfy validateConfig. +- If future iteration wants typed enforcement, a `Scenario.Platform bool` field could be added and consulted by Run for richer reporting. Out of scope for this ADR. + +## Alternatives Considered + +- **Bypass validateConfig for platform-level scenarios.** Rejected: the precondition is universal; making it conditional adds complexity without benefit. +- **Move platform-level scenarios out of conformance/ into a separate package.** Rejected: scenarios are conceptually about provider-conformance to the IaC contract; platform-shared surfaces are part of that contract. + +## References + +- Workflow PR #535 (W-7) — original implementation +- Workflow PR #538 (W-8) — codemod that surfaced the pattern in lint reports +- IaC conformance plan: `docs/plans/2026-05-03-iac-conformance-and-replace.md` +- Deferred cleanup design: `docs/plans/2026-05-05-iac-deferred-cleanup-design.md` From 454429585ff022f253ab21d3502be6ac81e53a04 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 5 May 2026 06:57:53 -0400 Subject: [PATCH 3/4] chore(example): go mod tidy after aws-sdk-go-v2 indirect bumps Refresh example/go.mod + go.sum to match the latest indirect aws-sdk-go-v2 patch versions (1.41.5 -> 1.41.6, config 1.32.14 -> 1.32.16, etc.). No direct dependency change; the Go Mod Tidy CI gate flagged drift after recent dependabot indirect bumps landed on main. Co-Authored-By: Claude Opus 4.7 (1M context) --- example/go.mod | 29 ++++++++++++------------- example/go.sum | 58 ++++++++++++++++++++++++-------------------------- 2 files changed, 42 insertions(+), 45 deletions(-) diff --git a/example/go.mod b/example/go.mod index 50e77be98..67383ef3d 100644 --- a/example/go.mod +++ b/example/go.mod @@ -38,33 +38,32 @@ require ( github.com/Workiva/go-datastructures v1.1.7 // indirect github.com/andybalholm/brotli v1.2.1 // indirect github.com/armon/go-metrics v0.4.1 // indirect - github.com/aws/aws-sdk-go-v2 v1.41.5 // indirect + github.com/aws/aws-sdk-go-v2 v1.41.6 // indirect github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.8 // indirect - github.com/aws/aws-sdk-go-v2/config v1.32.14 // indirect - github.com/aws/aws-sdk-go-v2/credentials v1.19.14 // indirect - github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.21 // indirect - github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.21 // indirect - github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.21 // indirect - github.com/aws/aws-sdk-go-v2/internal/ini v1.8.6 // indirect - github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.22 // indirect + github.com/aws/aws-sdk-go-v2/config v1.32.16 // indirect + github.com/aws/aws-sdk-go-v2/credentials v1.19.15 // indirect + github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.22 // indirect + github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.22 // indirect + github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.22 // indirect + github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.23 // indirect github.com/aws/aws-sdk-go-v2/service/apigatewayv2 v1.34.1 // indirect github.com/aws/aws-sdk-go-v2/service/applicationautoscaling v1.41.14 // indirect github.com/aws/aws-sdk-go-v2/service/codebuild v1.68.13 // indirect github.com/aws/aws-sdk-go-v2/service/ec2 v1.297.0 // indirect github.com/aws/aws-sdk-go-v2/service/ecs v1.78.0 // indirect github.com/aws/aws-sdk-go-v2/service/eks v1.82.0 // indirect - github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.7 // indirect + github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.8 // indirect github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.9.13 // indirect - github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.21 // indirect + github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.22 // indirect github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.21 // indirect github.com/aws/aws-sdk-go-v2/service/kinesis v1.43.5 // indirect github.com/aws/aws-sdk-go-v2/service/route53 v1.62.5 // indirect github.com/aws/aws-sdk-go-v2/service/s3 v1.99.0 // indirect - github.com/aws/aws-sdk-go-v2/service/signin v1.0.9 // indirect - github.com/aws/aws-sdk-go-v2/service/sso v1.30.15 // indirect - github.com/aws/aws-sdk-go-v2/service/ssooidc v1.35.19 // indirect - github.com/aws/aws-sdk-go-v2/service/sts v1.41.10 // indirect - github.com/aws/smithy-go v1.24.3 // indirect + github.com/aws/aws-sdk-go-v2/service/signin v1.0.10 // indirect + github.com/aws/aws-sdk-go-v2/service/sso v1.30.16 // indirect + github.com/aws/aws-sdk-go-v2/service/ssooidc v1.35.20 // indirect + github.com/aws/aws-sdk-go-v2/service/sts v1.42.0 // indirect + github.com/aws/smithy-go v1.25.0 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/bits-and-blooms/bitset v1.24.4 // indirect github.com/cenkalti/backoff/v4 v4.3.0 // indirect diff --git a/example/go.sum b/example/go.sum index 1b38659ff..1b47cd7ad 100644 --- a/example/go.sum +++ b/example/go.sum @@ -86,24 +86,22 @@ github.com/antithesishq/antithesis-sdk-go v0.7.0 h1:uWDG8BqLD1lI2ps38WDz2vXflrTX github.com/antithesishq/antithesis-sdk-go v0.7.0/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= github.com/armon/go-metrics v0.4.1 h1:hR91U9KYmb6bLBYLQjyM+3j+rcd/UhE+G78SFnF8gJA= github.com/armon/go-metrics v0.4.1/go.mod h1:E6amYzXo6aW1tqzoZGT755KkbgrJsSdpwZ+3JqfkOG4= -github.com/aws/aws-sdk-go-v2 v1.41.5 h1:dj5kopbwUsVUVFgO4Fi5BIT3t4WyqIDjGKCangnV/yY= -github.com/aws/aws-sdk-go-v2 v1.41.5/go.mod h1:mwsPRE8ceUUpiTgF7QmQIJ7lgsKUPQOUl3o72QBrE1o= +github.com/aws/aws-sdk-go-v2 v1.41.6 h1:1AX0AthnBQzMx1vbmir3Y4WsnJgiydmnJjiLu+LvXOg= +github.com/aws/aws-sdk-go-v2 v1.41.6/go.mod h1:dy0UzBIfwSeot4grGvY1AqFWN5zgziMmWGzysDnHFcQ= github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.8 h1:eBMB84YGghSocM7PsjmmPffTa+1FBUeNvGvFou6V/4o= github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.8/go.mod h1:lyw7GFp3qENLh7kwzf7iMzAxDn+NzjXEAGjKS2UOKqI= -github.com/aws/aws-sdk-go-v2/config v1.32.14 h1:opVIRo/ZbbI8OIqSOKmpFaY7IwfFUOCCXBsUpJOwDdI= -github.com/aws/aws-sdk-go-v2/config v1.32.14/go.mod h1:U4/V0uKxh0Tl5sxmCBZ3AecYny4UNlVmObYjKuuaiOo= -github.com/aws/aws-sdk-go-v2/credentials v1.19.14 h1:n+UcGWAIZHkXzYt87uMFBv/l8THYELoX6gVcUvgl6fI= -github.com/aws/aws-sdk-go-v2/credentials v1.19.14/go.mod h1:cJKuyWB59Mqi0jM3nFYQRmnHVQIcgoxjEMAbLkpr62w= -github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.21 h1:NUS3K4BTDArQqNu2ih7yeDLaS3bmHD0YndtA6UP884g= -github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.21/go.mod h1:YWNWJQNjKigKY1RHVJCuupeWDrrHjRqHm0N9rdrWzYI= -github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.21 h1:Rgg6wvjjtX8bNHcvi9OnXWwcE0a2vGpbwmtICOsvcf4= -github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.21/go.mod h1:A/kJFst/nm//cyqonihbdpQZwiUhhzpqTsdbhDdRF9c= -github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.21 h1:PEgGVtPoB6NTpPrBgqSE5hE/o47Ij9qk/SEZFbUOe9A= -github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.21/go.mod h1:p+hz+PRAYlY3zcpJhPwXlLC4C+kqn70WIHwnzAfs6ps= -github.com/aws/aws-sdk-go-v2/internal/ini v1.8.6 h1:qYQ4pzQ2Oz6WpQ8T3HvGHnZydA72MnLuFK9tJwmrbHw= -github.com/aws/aws-sdk-go-v2/internal/ini v1.8.6/go.mod h1:O3h0IK87yXci+kg6flUKzJnWeziQUKciKrLjcatSNcY= -github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.22 h1:rWyie/PxDRIdhNf4DzRk0lvjVOqFJuNnO8WwaIRVxzQ= -github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.22/go.mod h1:zd/JsJ4P7oGfUhXn1VyLqaRZwPmZwg44Jf2dS84Dm3Y= +github.com/aws/aws-sdk-go-v2/config v1.32.16 h1:Q0iQ7quUgJP0F/SCRTieScnaMdXr9h/2+wze1u3cNeM= +github.com/aws/aws-sdk-go-v2/config v1.32.16/go.mod h1:duCCnJEFqpt2RC6no1iK6q+8HpwOAkiUua0pY507dQc= +github.com/aws/aws-sdk-go-v2/credentials v1.19.15 h1:fyvgWTszojq8hEnMi8PPBTvZdTtEVmAVyo+NFLHBhH4= +github.com/aws/aws-sdk-go-v2/credentials v1.19.15/go.mod h1:gJiYyMOjNg8OEdRWOf3CrFQxM2a98qmrtjx1zuiQfB8= +github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.22 h1:IOGsJ1xVWhsi+ZO7/NW8OuZZBtMJLZbk4P5HDjJO0jQ= +github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.22/go.mod h1:b+hYdbU+jGKfXE8kKM6g1+h+L/Go3vMvzlxBsiuGsxg= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.22 h1:GmLa5Kw1ESqtFpXsx5MmC84QWa/ZrLZvlJGa2y+4kcQ= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.22/go.mod h1:6sW9iWm9DK9YRpRGga/qzrzNLgKpT2cIxb7Vo2eNOp0= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.22 h1:dY4kWZiSaXIzxnKlj17nHnBcXXBfac6UlsAx2qL6XrU= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.22/go.mod h1:KIpEUx0JuRZLO7U6cbV204cWAEco2iC3l061IxlwLtI= +github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.23 h1:FPXsW9+gMuIeKmz7j6ENWcWtBGTe1kH8r9thNt5Uxx4= +github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.23/go.mod h1:7J8iGMdRKk6lw2C+cMIphgAnT8uTwBwNOsGkyOCm80U= github.com/aws/aws-sdk-go-v2/service/apigatewayv2 v1.34.1 h1:KdeMmKNbHOcFsiFe8BhLYE2A8crvEWnetrs8GkF2od8= github.com/aws/aws-sdk-go-v2/service/apigatewayv2 v1.34.1/go.mod h1:QdVi7bF8U8HE8FWQ77pa4wcAMFnxI/UPx4mmQ4azRQs= github.com/aws/aws-sdk-go-v2/service/applicationautoscaling v1.41.14 h1:0aYQ2UaSB1ccXZXUQ4a5XanrHEykKNzMLFgLEDhf8PU= @@ -118,12 +116,12 @@ github.com/aws/aws-sdk-go-v2/service/eks v1.82.0 h1:AvBDUgHffBd4AErnQY6sB9u5vY/9 github.com/aws/aws-sdk-go-v2/service/eks v1.82.0/go.mod h1:xdUh6tdF9A8hc+PE84kmHbF/zsVPNiKnc6oLgulq1Eo= github.com/aws/aws-sdk-go-v2/service/iam v1.53.7 h1:n9YLiWtX3+6pTLZWvRJmtq5JIB9NA/KFelyCg5fOlTU= github.com/aws/aws-sdk-go-v2/service/iam v1.53.7/go.mod h1:sP46Vo6MeJcM4s0ZXcG2PFmfiSyixhIuC/74W52yKuk= -github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.7 h1:5EniKhLZe4xzL7a+fU3C2tfUN4nWIqlLesfrjkuPFTY= -github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.7/go.mod h1:x0nZssQ3qZSnIcePWLvcoFisRXJzcTVvYpAAdYX8+GI= +github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.8 h1:HtOTYcbVcGABLOVuPYaIihj6IlkqubBwFj10K5fxRek= +github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.8/go.mod h1:VsK9abqQeGlzPgUr+isNWzPlK2vKe9INMLWnY65f5Xs= github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.9.13 h1:JRaIgADQS/U6uXDqlPiefP32yXTda7Kqfx+LgspooZM= github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.9.13/go.mod h1:CEuVn5WqOMilYl+tbccq8+N2ieCy0gVn3OtRb0vBNNM= -github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.21 h1:c31//R3xgIJMSC8S6hEVq+38DcvUlgFY0FM6mSI5oto= -github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.21/go.mod h1:r6+pf23ouCB718FUxaqzZdbpYFyDtehyZcmP5KL9FkA= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.22 h1:PUmZeJU6Y1Lbvt9WFuJ0ugUK2xn6hIWUBBbKuOWF30s= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.22/go.mod h1:nO6egFBoAaoXze24a2C0NjQCvdpk8OueRoYimvEB9jo= github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.21 h1:ZlvrNcHSFFWURB8avufQq9gFsheUgjVD9536obIknfM= github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.21/go.mod h1:cv3TNhVrssKR0O/xxLJVRfd2oazSnZnkUeTf6ctUwfQ= github.com/aws/aws-sdk-go-v2/service/kinesis v1.43.5 h1:LxgRVyuY+5DEPSX7kmin/V7toE8MWZ9U8n2dqRtX+RE= @@ -132,16 +130,16 @@ github.com/aws/aws-sdk-go-v2/service/route53 v1.62.5 h1:Z+/OLsb85Kpq7TVLCspskqeP github.com/aws/aws-sdk-go-v2/service/route53 v1.62.5/go.mod h1:TmxGowuBYwjmHFOsEDxaZdsQE62JJzOmtiWafTi/czg= github.com/aws/aws-sdk-go-v2/service/s3 v1.99.0 h1:hlSuz394kV0vhv9drL5lhuEFbEOEP1VyQpy15qWh1Pk= github.com/aws/aws-sdk-go-v2/service/s3 v1.99.0/go.mod h1:uoA43SdFwacedBfSgfFSjjCvYe8aYBS7EnU5GZ/YKMM= -github.com/aws/aws-sdk-go-v2/service/signin v1.0.9 h1:QKZH0S178gCmFEgst8hN0mCX1KxLgHBKKY/CLqwP8lg= -github.com/aws/aws-sdk-go-v2/service/signin v1.0.9/go.mod h1:7yuQJoT+OoH8aqIxw9vwF+8KpvLZ8AWmvmUWHsGQZvI= -github.com/aws/aws-sdk-go-v2/service/sso v1.30.15 h1:lFd1+ZSEYJZYvv9d6kXzhkZu07si3f+GQ1AaYwa2LUM= -github.com/aws/aws-sdk-go-v2/service/sso v1.30.15/go.mod h1:WSvS1NLr7JaPunCXqpJnWk1Bjo7IxzZXrZi1QQCkuqM= -github.com/aws/aws-sdk-go-v2/service/ssooidc v1.35.19 h1:dzztQ1YmfPrxdrOiuZRMF6fuOwWlWpD2StNLTceKpys= -github.com/aws/aws-sdk-go-v2/service/ssooidc v1.35.19/go.mod h1:YO8TrYtFdl5w/4vmjL8zaBSsiNp3w0L1FfKVKenZT7w= -github.com/aws/aws-sdk-go-v2/service/sts v1.41.10 h1:p8ogvvLugcR/zLBXTXrTkj0RYBUdErbMnAFFp12Lm/U= -github.com/aws/aws-sdk-go-v2/service/sts v1.41.10/go.mod h1:60dv0eZJfeVXfbT1tFJinbHrDfSJ2GZl4Q//OSSNAVw= -github.com/aws/smithy-go v1.24.3 h1:XgOAaUgx+HhVBoP4v8n6HCQoTRDhoMghKqw4LNHsDNg= -github.com/aws/smithy-go v1.24.3/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= +github.com/aws/aws-sdk-go-v2/service/signin v1.0.10 h1:a1Fq/KXn75wSzoJaPQTgZO0wHGqE9mjFnylnqEPTchA= +github.com/aws/aws-sdk-go-v2/service/signin v1.0.10/go.mod h1:p6+MXNxW7IA6dMgHfTAzljuwSKD0NCm/4lbS4t6+7vI= +github.com/aws/aws-sdk-go-v2/service/sso v1.30.16 h1:x6bKbmDhsgSZwv6q19wY/u3rLk/3FGjJWyqKcIRufpE= +github.com/aws/aws-sdk-go-v2/service/sso v1.30.16/go.mod h1:CudnEVKRtLn0+3uMV0yEXZ+YZOKnAtUJ5DmDhilVnIw= +github.com/aws/aws-sdk-go-v2/service/ssooidc v1.35.20 h1:oK/njaL8GtyEihkWMD4k3VgHCT64RQKkZwh0DG5j8ak= +github.com/aws/aws-sdk-go-v2/service/ssooidc v1.35.20/go.mod h1:JHs8/y1f3zY7U5WcuzoJ/yAYGYtNIVPKLIbp61euvmg= +github.com/aws/aws-sdk-go-v2/service/sts v1.42.0 h1:ks8KBcZPh3PYISr5dAiXCM5/Thcuxk8l+PG4+A0exds= +github.com/aws/aws-sdk-go-v2/service/sts v1.42.0/go.mod h1:pFw33T0WLvXU3rw1WBkpMlkgIn54eCB5FYLhjDc9Foo= +github.com/aws/smithy-go v1.25.0 h1:Sz/XJ64rwuiKtB6j98nDIPyYrV1nVNJ4YU74gttcl5U= +github.com/aws/smithy-go v1.25.0/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= From 24a99b7d5fab6804d73ebafb016946dc99fa0603 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 5 May 2026 07:08:53 -0400 Subject: [PATCH 4/4] fix(wfctl): preserve v1 dispatch in remoteIaCProvider proxy (PR #556 R1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #556 R1 review correction (Copilot inline comment): the initial delegation-only implementation collapsed both v1 and v2 routes into the v2 path through this proxy, silently breaking plugins that declare computePlanVersion: "" / "v1" / unknown in plugin.json. The dispatch gate in infra_apply.go's DispatchVersionFor still treats those plugins as legacy v1 — but the proxy was running their Plan/Apply through platform.ComputePlan / wfctlhelpers.ApplyPlan regardless, bypassing the plugin's own monolithic Plan/Apply endpoints. Restore the manifest-conditional branch in both Plan and Apply: - computePlanVersion == "v2" → delegates to platform.ComputePlan (Plan) / wfctlhelpers.ApplyPlan (Apply) — the canonical wfctl-side path with per-driver fan-out + drift postcondition. - "" / "v1" / unknown → proxies the legacy IaCProvider.Plan / IaCProvider.Apply call to the plugin via InvokeService, preserving the v1 contract per dispatch.go's "default-to-v1" doctrine. The proxy now agrees with infra_apply.go's DispatchVersionFor gate. Apply the canonical-delegation lint SkipMarker (// wfctl:skip-iac-codemod) on both methods with justification: the analyzer expects unconditional v2 forms for plugin-side providers; this is a wfctl-side proxy that supports BOTH versions. The marker also honors ProviderValidator (R-A10) since that analyzer consults Plan/ Apply skip markers when classifying provider-shaped types. Test surface: - Restore TestRemoteIaC_Plan_V1Default + TestRemoteIaC_Apply_V1Default pinning the legacy IaCProvider.Plan / IaCProvider.Apply wire shape (default-constructed *remoteIaCProvider has computePlanVersion: ""). - Keep the V2 delegation tests (TestRemoteIaC_Plan_V2_* and TestRemoteIaC_Apply_V2_*) with explicit p.computePlanVersion = wfctlhelpers.DispatchVersionV2. - Restore Plan / Apply rows in the dispatch matrix (TestDispatchMatrix_RemoteIaCProvider) since the v1-default branch preserves the original wire signature. Codemod lint validation post-fix: - Findings: 0 - Skipped: 3 (Plan / Apply / type-level ProviderValidator suppressed transitively through Plan/Apply markers) All wfctl tests pass: go test ./cmd/wfctl/... -count=1 -race (10.18s). Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/wfctl/deploy_providers.go | 114 +++++++--- .../deploy_providers_dispatch_matrix_test.go | 36 +++- cmd/wfctl/deploy_providers_remote_iac_test.go | 197 ++++++++++++++---- 3 files changed, 263 insertions(+), 84 deletions(-) diff --git a/cmd/wfctl/deploy_providers.go b/cmd/wfctl/deploy_providers.go index 7bd2cd14f..79c7d8a75 100644 --- a/cmd/wfctl/deploy_providers.go +++ b/cmd/wfctl/deploy_providers.go @@ -339,42 +339,90 @@ func (r *remoteIaCProvider) Capabilities() []interfaces.IaCCapabilityDeclaration return caps } -// Plan delegates to platform.ComputePlan, the canonical wfctl-side diff -// engine. The pre-W-Refactor body proxied a monolithic IaCProvider.Plan -// call to the plugin via InvokeService — a v1-wire shape that platform/ -// differ.go superseded once Diff dispatch landed (W-3 / W-7). Delegating -// here keeps the wfctl-loaded provider's Plan body in lockstep with the -// canonical Plan flow that infra_apply.go's computeInfraPlan var already -// uses; the lint analyzer in cmd/iac-codemod/lint.go::AssertPlanDelegates -// ToHelper now reports zero findings on this file. The 2-statement -// `plan, err := platform.ComputePlan(...); return &plan, err` shape is -// the canonical form recognized by isAlreadyDelegatedPlanBody (the -// rewriter's idempotency check). ResourceDriver.Diff for per-resource -// classification still hits the plugin via remoteResourceDriver, so the -// gRPC fan-out is preserved. +// Plan branches on the plugin manifest's iacProvider.computePlanVersion: +// +// - "v2" — delegates to platform.ComputePlan, the canonical wfctl-side +// diff engine. ResourceDriver.Diff for per-resource classification +// hits the plugin via remoteResourceDriver, so the gRPC fan-out is +// preserved while wfctl owns the plan-shape contract. +// - "" / "v1" / unknown — proxies the legacy monolithic IaCProvider.Plan +// call to the plugin via InvokeService, preserving the v1 contract +// for plugins that compute their own plans. This is the safe-default +// branch per dispatch.go's "default-to-v1" doctrine. +// +// The branch mirrors infra_apply.go's DispatchVersionFor gate so the +// wfctl-side proxy honors the same plugin-author-controlled routing that +// the in-tree apply path honors. PR #556 R1 review correction: the +// initial implementation collapsed both branches into the v2 path, which +// silently bypassed v1 plugins' own Plan endpoint via this proxy. +// +// wfctl:skip-iac-codemod (manifest-conditional dispatch is intentional — +// canonical-delegation lint expects unconditional v2 forms for plugin- +// side providers; this is a wfctl-side proxy that supports BOTH versions +// per dispatch.go's "default-to-v1" safety net.) func (r *remoteIaCProvider) Plan(ctx context.Context, desired []interfaces.ResourceSpec, current []interfaces.ResourceState) (*interfaces.IaCPlan, error) { - plan, err := platform.ComputePlan(ctx, r, desired, current) - return &plan, err -} - -// Apply delegates to wfctlhelpers.ApplyPlan, the canonical per-action -// dispatcher. The pre-W-Refactor body proxied a monolithic IaCProvider. -// Apply call to the plugin via InvokeService — a v1-wire shape that -// wfctlhelpers.ApplyPlan supersedes by fanning out per-action through -// ResourceDriver.{Create,Update,Delete,Replace} (which remoteResource -// Driver still proxies via gRPC). The single-statement -// `return wfctlhelpers.ApplyPlan(ctx, p, plan)` shape is the canonical -// form recognized by isAlreadyDelegatedApplyBody. + if r.computePlanVersion == wfctlhelpers.DispatchVersionV2 { + plan, err := platform.ComputePlan(ctx, r, desired, current) + return &plan, err + } + desiredAny, err := jsonToAny(desired) + if err != nil { + return nil, fmt.Errorf("IaCProvider.Plan: marshal desired: %w", err) + } + currentAny, err := jsonToAny(current) + if err != nil { + return nil, fmt.Errorf("IaCProvider.Plan: marshal current: %w", err) + } + res, err := r.invoker.InvokeService("IaCProvider.Plan", map[string]any{ + "desired": desiredAny, + "current": currentAny, + }) + if err != nil { + return nil, err + } + var plan interfaces.IaCPlan + if err := anyToStruct(res, &plan); err != nil { + return nil, fmt.Errorf("IaCProvider.Plan: decode result: %w", err) + } + return &plan, nil +} + +// Apply branches on the plugin manifest's iacProvider.computePlanVersion, +// matching the same dispatch policy as Plan above: +// +// - "v2" — delegates to wfctlhelpers.ApplyPlan, which fans out per- +// action through ResourceDriver.{Create,Update,Delete,Replace} (the +// remoteResourceDriver proxies each over gRPC) and runs the +// drift-postcondition + per-action error decomposition. +// - "" / "v1" / unknown — proxies the legacy monolithic IaCProvider. +// Apply call to the plugin via InvokeService, preserving the v1 +// contract for plugins that own their own apply orchestration. +// +// The branch keeps the v1/v2 routing contract documented in dispatch.go +// intact: the dispatch gate in infra_apply.go and this proxy now agree. +// PR #556 R1 review correction (Copilot inline finding) — the initial +// implementation collapsed both branches into the v2 path, which +// silently broke v1 plugins routed through this proxy. // -// Note: this delegation also activates wfctlhelpers.ApplyPlan's drift -// postcondition + per-action error decomposition for any plugin loaded -// via this proxy, regardless of plugin.json's iacProvider.compute -// PlanVersion declaration. The dispatch gate in infra_apply.go that -// branches on DispatchVersionFor remains, but both branches now route -// through the same per-driver dispatch — the v1 monolithic Apply -// endpoint is no longer reachable through wfctl. +// wfctl:skip-iac-codemod (manifest-conditional dispatch is intentional — +// see Plan above for the canonical-delegation-lint rationale.) func (r *remoteIaCProvider) Apply(ctx context.Context, plan *interfaces.IaCPlan) (*interfaces.ApplyResult, error) { - return wfctlhelpers.ApplyPlan(ctx, r, plan) + if r.computePlanVersion == wfctlhelpers.DispatchVersionV2 { + return wfctlhelpers.ApplyPlan(ctx, r, plan) + } + planAny, err := jsonToAny(plan) + if err != nil { + return nil, fmt.Errorf("IaCProvider.Apply: marshal plan: %w", err) + } + res, err := r.invoker.InvokeService("IaCProvider.Apply", map[string]any{"plan": planAny}) + if err != nil { + return nil, err + } + var result interfaces.ApplyResult + if err := anyToStruct(res, &result); err != nil { + return nil, fmt.Errorf("IaCProvider.Apply: decode result: %w", err) + } + return &result, nil } func (r *remoteIaCProvider) Destroy(_ context.Context, refs []interfaces.ResourceRef) (*interfaces.DestroyResult, error) { diff --git a/cmd/wfctl/deploy_providers_dispatch_matrix_test.go b/cmd/wfctl/deploy_providers_dispatch_matrix_test.go index e293415df..b4abe597d 100644 --- a/cmd/wfctl/deploy_providers_dispatch_matrix_test.go +++ b/cmd/wfctl/deploy_providers_dispatch_matrix_test.go @@ -248,15 +248,35 @@ func TestDispatchMatrix_RemoteIaCProvider(t *testing.T) { requiredKeys []string invoke func(p *remoteIaCProvider, ri *recordingInvoker) error }{ - // Plan and Apply intentionally absent from this matrix. - // Per W-Refactor (PR 5), remoteIaCProvider.Plan delegates to - // platform.ComputePlan and Apply delegates to wfctlhelpers.ApplyPlan. - // Neither method forwards a monolithic IaCProvider.Plan / - // IaCProvider.Apply RPC anymore — the per-action driver dispatch - // (ResourceDriver.Diff / Create / Update / Delete) covers the - // underlying gRPC traffic and is exercised by - // TestDispatchMatrix_RemoteResourceDriver. Canonical-delegation + // Plan and Apply exercise the v1-default branch: per W-Refactor + // (PR 5), remoteIaCProvider's Plan / Apply are manifest-conditional + // — they proxy IaCProvider.Plan / IaCProvider.Apply via + // InvokeService when computePlanVersion is "" / "v1" / unknown, + // and delegate to platform.ComputePlan / wfctlhelpers.ApplyPlan + // when computePlanVersion == "v2". The matrix below pins the v1 + // wire shape (the default constructed *remoteIaCProvider has + // computePlanVersion: "" so the legacy proxy fires). v2 branch // behavior is pinned in deploy_providers_remote_iac_test.go. + { + name: "Plan", + wantMethod: "IaCProvider.Plan", + requiredKeys: []string{"desired", "current"}, + invoke: func(p *remoteIaCProvider, ri *recordingInvoker) error { + ri.resp = map[string]any{} + _, err := p.Plan(ctx, nil, nil) + return err + }, + }, + { + name: "Apply", + wantMethod: "IaCProvider.Apply", + requiredKeys: []string{"plan"}, + invoke: func(p *remoteIaCProvider, ri *recordingInvoker) error { + ri.resp = map[string]any{} + _, err := p.Apply(ctx, &interfaces.IaCPlan{}) + return err + }, + }, { name: "Destroy", wantMethod: "IaCProvider.Destroy", diff --git a/cmd/wfctl/deploy_providers_remote_iac_test.go b/cmd/wfctl/deploy_providers_remote_iac_test.go index 8ca09bc46..dd90800e3 100644 --- a/cmd/wfctl/deploy_providers_remote_iac_test.go +++ b/cmd/wfctl/deploy_providers_remote_iac_test.go @@ -8,10 +8,14 @@ import ( "testing" "time" + "github.com/GoCodeAlone/workflow/iac/wfctlhelpers" "github.com/GoCodeAlone/workflow/interfaces" ) // newIaCProvider builds a remoteIaCProvider backed by the given stubInvoker. +// Defaults to empty computePlanVersion (the safe-default v1 branch in +// dispatch.go's "default-to-v1" doctrine). Tests that need the v2 branch +// set p.computePlanVersion = wfctlhelpers.DispatchVersionV2 directly. func newIaCProvider(si *stubInvoker) *remoteIaCProvider { return &remoteIaCProvider{invoker: si} } @@ -68,40 +72,98 @@ func TestRemoteIaC_Capabilities_Error(t *testing.T) { // ── Plan ────────────────────────────────────────────────────────────────────── // -// Pre-W-Refactor, Plan() proxied a monolithic IaCProvider.Plan call to -// the plugin via InvokeService. After W-Refactor (PR 5), Plan() delegates -// to platform.ComputePlan, which classifies actions locally: -// - net-new resources (no current state) → emit "create" without -// dispatching ResourceDriver.Diff (no InvokeService traffic); -// - removed-from-desired resources → emit "delete" without dispatching -// Diff (also no InvokeService traffic); -// - updates/replaces fan out via remoteResourceDriver.Diff → -// InvokeService("ResourceDriver.Diff", ...). +// Plan() is manifest-conditional after W-Refactor (PR 5): +// - computePlanVersion == "v2" → delegates to platform.ComputePlan +// (wfctl owns plan classification; ResourceDriver.Diff dispatches +// remotely on a per-resource basis); +// - otherwise (default v1) → proxies the legacy monolithic +// IaCProvider.Plan call to the plugin via InvokeService. // -// The test below pins the create-only path because it exercises the -// canonical-delegation contract without standing up the full diff-cache -// + Diff dispatch. The delegation-via-driver path is covered by the -// canonical test below + the existing remote-driver tests. +// Tests below pin BOTH branches. -func TestRemoteIaC_Plan_DelegatesToComputePlan_NetNewResource(t *testing.T) { +// v1-default branch: legacy proxy to plugin. + +func samplePlanResponse() map[string]any { + return map[string]any{ + "id": "plan-abc", + "actions": []any{ + map[string]any{ + "action": "create", + "resource": map[string]any{ + "name": "db", + "type": "infra.database", + "config": map[string]any{}, + }, + }, + }, + "created_at": time.Now().Format(time.RFC3339Nano), + } +} + +func TestRemoteIaC_Plan_V1Default_ProxiesIaCProviderPlan(t *testing.T) { + si := &stubInvoker{resp: samplePlanResponse()} + p := newIaCProvider(si) // default computePlanVersion == "" + + desired := []interfaces.ResourceSpec{ + {Name: "db", Type: "infra.database", Config: map[string]any{"engine": "postgres"}}, + } + current := []interfaces.ResourceState{ + {Name: "old-db", Type: "infra.database", ProviderID: "pid-old"}, + } + + plan, err := p.Plan(context.Background(), desired, current) + if err != nil { + t.Fatalf("Plan: unexpected error: %v", err) + } + if si.method != "IaCProvider.Plan" { + t.Errorf("v1-default branch must proxy IaCProvider.Plan; got %q", si.method) + } + if _, ok := si.args["desired"]; !ok { + t.Error("missing arg key 'desired'") + } + if _, ok := si.args["current"]; !ok { + t.Error("missing arg key 'current'") + } + if plan.ID != "plan-abc" { + t.Errorf("plan ID: got %q", plan.ID) + } + if len(plan.Actions) != 1 { + t.Fatalf("expected 1 action, got %d", len(plan.Actions)) + } + if plan.Actions[0].Action != "create" { + t.Errorf("action: got %q", plan.Actions[0].Action) + } +} + +func TestRemoteIaC_Plan_V1Default_PropagatesError(t *testing.T) { + si := &stubInvoker{err: fmt.Errorf("rpc error")} + p := newIaCProvider(si) + _, err := p.Plan(context.Background(), nil, nil) + if err == nil { + t.Fatal("expected error from v1 IaCProvider.Plan proxy") + } +} + +// v2 branch: delegates to platform.ComputePlan. + +func TestRemoteIaC_Plan_V2_DelegatesToComputePlan_NetNewResource(t *testing.T) { // stubInvoker tracks the LAST InvokeService call. With ComputePlan // delegation, a net-new resource emits "create" without touching the - // invoker — confirming the per-resource Diff fan-out is bypassed for - // creates and the v1-monolithic IaCProvider.Plan wire is gone. + // invoker — confirming the v2 branch routes through wfctl-side + // classification rather than the v1 IaCProvider.Plan wire. si := &stubInvoker{} p := newIaCProvider(si) + p.computePlanVersion = wfctlhelpers.DispatchVersionV2 desired := []interfaces.ResourceSpec{ {Name: "db", Type: "infra.database", Config: map[string]any{"engine": "postgres"}}, } - // No current state → "db" is net-new → ComputePlan emits "create" - // without calling driver.Diff. plan, err := p.Plan(context.Background(), desired, nil) if err != nil { t.Fatalf("Plan: unexpected error: %v", err) } if si.method != "" { - t.Errorf("invoker.method: net-new create path should not call InvokeService; got %q", si.method) + t.Errorf("v2 branch + net-new create should not hit InvokeService; got %q", si.method) } if plan == nil { t.Fatal("Plan returned nil plan") @@ -117,11 +179,10 @@ func TestRemoteIaC_Plan_DelegatesToComputePlan_NetNewResource(t *testing.T) { } } -func TestRemoteIaC_Plan_DelegatesToComputePlan_DeleteEmittedForRemoved(t *testing.T) { - // A resource present in current state but absent from desired must - // emit "delete". ComputePlan emits deletes without calling driver.Diff. +func TestRemoteIaC_Plan_V2_DelegatesToComputePlan_DeleteEmittedForRemoved(t *testing.T) { si := &stubInvoker{} p := newIaCProvider(si) + p.computePlanVersion = wfctlhelpers.DispatchVersionV2 current := []interfaces.ResourceState{ {Name: "old-db", Type: "infra.database", ProviderID: "pid-old"}, @@ -131,7 +192,7 @@ func TestRemoteIaC_Plan_DelegatesToComputePlan_DeleteEmittedForRemoved(t *testin t.Fatalf("Plan: unexpected error: %v", err) } if si.method != "" { - t.Errorf("invoker.method: delete path should not call InvokeService; got %q", si.method) + t.Errorf("v2 branch + delete path should not hit InvokeService; got %q", si.method) } if plan == nil { t.Fatal("Plan returned nil plan") @@ -146,23 +207,73 @@ func TestRemoteIaC_Plan_DelegatesToComputePlan_DeleteEmittedForRemoved(t *testin // ── Apply ───────────────────────────────────────────────────────────────────── // -// Pre-W-Refactor, Apply() proxied a monolithic IaCProvider.Apply call to -// the plugin via InvokeService. After W-Refactor (PR 5), Apply() delegates -// to wfctlhelpers.ApplyPlan, which fans out per-action through -// ResourceDriver.{Create,Update,Delete,Replace}. Through the remoteIaC -// Provider those calls hit InvokeService("ResourceDriver.Create", ...) etc. +// Apply() is manifest-conditional after W-Refactor (PR 5): +// - computePlanVersion == "v2" → delegates to wfctlhelpers.ApplyPlan +// (per-action driver dispatch + drift postcondition); +// - otherwise (default v1) → proxies the legacy monolithic +// IaCProvider.Apply call to the plugin via InvokeService. // -// The tests below pin the canonical delegation by: -// (a) verifying the wire-method is the per-driver call (not the legacy -// IaCProvider.Apply); and -// (b) verifying the ApplyResult is populated with the correct PlanID -// (set by ApplyPlan from plan.ID) regardless of the per-driver -// response shape. - -func TestRemoteIaC_Apply_DelegatesToApplyPlan_PerDriverDispatch(t *testing.T) { +// Tests below pin BOTH branches. + +// v1-default branch: legacy proxy to plugin. + +func TestRemoteIaC_Apply_V1Default_ProxiesIaCProviderApply(t *testing.T) { + si := &stubInvoker{resp: map[string]any{ + "plan_id": "plan-abc", + "resources": []any{ + map[string]any{ + "provider_id": "pid-123", + "name": "db", + "type": "infra.database", + "status": "running", + }, + }, + }} + p := newIaCProvider(si) // default computePlanVersion == "" + + plan := &interfaces.IaCPlan{ + ID: "plan-abc", + Actions: []interfaces.PlanAction{ + {Action: "create", Resource: interfaces.ResourceSpec{Name: "db", Type: "infra.database"}}, + }, + } + result, err := p.Apply(context.Background(), plan) + if err != nil { + t.Fatalf("Apply: unexpected error: %v", err) + } + if si.method != "IaCProvider.Apply" { + t.Errorf("v1-default branch must proxy IaCProvider.Apply; got %q", si.method) + } + if _, ok := si.args["plan"]; !ok { + t.Error("missing arg key 'plan'") + } + if result.PlanID != "plan-abc" { + t.Errorf("PlanID: got %q", result.PlanID) + } + if len(result.Resources) != 1 { + t.Fatalf("expected 1 resource, got %d", len(result.Resources)) + } + if result.Resources[0].Name != "db" { + t.Errorf("resource name: got %q", result.Resources[0].Name) + } +} + +func TestRemoteIaC_Apply_V1Default_PropagatesError(t *testing.T) { + si := &stubInvoker{err: fmt.Errorf("apply failed")} + p := newIaCProvider(si) + _, err := p.Apply(context.Background(), &interfaces.IaCPlan{ID: "p1"}) + if err == nil { + t.Fatal("expected error from v1 IaCProvider.Apply proxy") + } +} + +// v2 branch: delegates to wfctlhelpers.ApplyPlan (per-action driver dispatch). + +func TestRemoteIaC_Apply_V2_DelegatesToApplyPlan_PerDriverDispatch(t *testing.T) { // ApplyPlan dispatches Create on a single-create plan via // remoteResourceDriver, which invokes "ResourceDriver.Create" through - // the stub invoker. The legacy "IaCProvider.Apply" wire is gone. + // the stub invoker. The v1 monolithic "IaCProvider.Apply" wire is + // not used in the v2 branch. si := &stubInvoker{resp: map[string]any{ "output": map[string]any{ "provider_id": "pid-123", @@ -172,6 +283,7 @@ func TestRemoteIaC_Apply_DelegatesToApplyPlan_PerDriverDispatch(t *testing.T) { }, }} p := newIaCProvider(si) + p.computePlanVersion = wfctlhelpers.DispatchVersionV2 plan := &interfaces.IaCPlan{ ID: "plan-abc", @@ -184,10 +296,10 @@ func TestRemoteIaC_Apply_DelegatesToApplyPlan_PerDriverDispatch(t *testing.T) { t.Fatalf("Apply: unexpected error: %v", err) } if si.method == "IaCProvider.Apply" { - t.Error("invoker.method: legacy IaCProvider.Apply wire should not be invoked after W-Refactor") + t.Error("v2 branch must NOT invoke legacy IaCProvider.Apply wire") } if !strings.HasPrefix(si.method, "ResourceDriver.") { - t.Errorf("invoker.method: expected ResourceDriver.* per-driver dispatch, got %q", si.method) + t.Errorf("v2 branch: expected ResourceDriver.* per-driver dispatch, got %q", si.method) } if result == nil { t.Fatal("Apply returned nil result") @@ -197,14 +309,13 @@ func TestRemoteIaC_Apply_DelegatesToApplyPlan_PerDriverDispatch(t *testing.T) { } } -func TestRemoteIaC_Apply_DelegatesToApplyPlan_RecordsErrorsPerAction(t *testing.T) { +func TestRemoteIaC_Apply_V2_DelegatesToApplyPlan_RecordsErrorsPerAction(t *testing.T) { // When the underlying driver returns an error, ApplyPlan records it // in result.Errors rather than returning the error from the top-level - // call (per the per-action error decomposition contract). This pins - // the canonical-delegation behavior — pre-refactor the same error - // would have been returned directly. + // call (per the per-action error decomposition contract). si := &stubInvoker{err: fmt.Errorf("driver create failed")} p := newIaCProvider(si) + p.computePlanVersion = wfctlhelpers.DispatchVersionV2 plan := &interfaces.IaCPlan{ ID: "p1",