workflow#699: IaCProvider.Apply removal — design + plan + scope-lock (PR 1 of 10)#702
Merged
Merged
Conversation
… addressed) — pivot to typed Capabilities-RPC gate
…nsumer fanout, N2 v1 else-block, N3 smoke command, N4 wiring, N5 integration test, N6 regex tighten, N7 EffectiveComputePlanVersion)
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the design document, detailed multi-PR implementation plan, and a scope-lock hash file for workflow#699’s cascade to hard-remove IaCProvider.Apply from the workflow engine and IaC plugin ecosystem (proto/interface removal + load-time Capabilities gate + follow-on plugin/registry changes).
Changes:
- Introduces an implementation plan detailing a 10-PR cascade and task-by-task execution steps.
- Adds an approved design writeup documenting the decision, assumptions, rollback, and sequencing.
- Records a scope-lock hash intended to freeze the plan’s scope.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| docs/plans/2026-05-17-iac-provider-apply-removal.md.scope-lock | Adds a scope-lock hash for the plan |
| docs/plans/2026-05-17-iac-provider-apply-removal.md | Adds the detailed implementation plan for the 10-PR cascade |
| docs/plans/2026-05-17-iac-provider-apply-removal-design.md | Adds the design/decision record for removing IaCProvider.Apply |
|
|
||
| **Architecture:** 10-PR coordinated cascade. PR 1 ships workflow `v0.56.0-rc1` (proto deletion + interface deletion + load-time Capabilities-RPC gate). PRs 2-5 ship plugin `v2.0.0-rc1` tags in parallel (drop Apply method + iacserver handler). PR 6 runs conformance matrix + tags `workflow v0.56.0`. PRs 7-10 ship plugin `v2.0.0` final tags + registry manifest bumps as fan-out from PR 6. | ||
|
|
||
| **Tech Stack:** Go 1.24, gRPC (buf for proto), GoReleaser v2, GoCodeAlone/modular framework. No new dependencies introduced. |
Comment on lines
+458
to
+462
| ```bash | ||
| cd plugin/external/proto && buf generate && cd - | ||
| ``` | ||
|
|
||
| If `buf` is not installed: install per the existing project setup (see `CONTRIBUTING.md` or `Makefile` target `proto-gen`). |
|
|
||
| **Step 1: Add CI step** | ||
|
|
||
| Append to the test job in `.github/workflows/ci.yaml`: |
| @@ -0,0 +1 @@ | |||
| 237183aba33455a66d4c8d760096013c1ca31e14920b8f384ab3df654ad26083 | |||
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
…yResultFromPB (workflow#699 PR 1 task 2)
…orkflow#699 PR 1 task 1 fixup) Addresses code-reviewer Important findings 1-5 on Task 1 (commit 1df2231): 1. printDriftReportIfAny godoc — was 'NOT yet by any production caller'; now documents production drift-surfacing path on v2-only dispatch. 2. applyWithProviderAndStore godoc — was 'executes it via provider.Apply'; now names wfctlhelpers.ApplyPlanWithHooks + hooks-based persistence. 3. applyFromPrecomputedPlan godoc — was 'calls provider.Apply for each group'; now points at applyPrecomputedPlanWithStore. 4. applyPrecomputedPlanWithStore godoc — was 'executes via provider.Apply'; now names wfctlhelpers.ApplyPlanWithHooks + parity with sibling helper. 5. Twin inline comments at runInfraApply + applyPrecomputedPlanWithStore — was 'Provider.Apply can surface a top-level error'; now names applyV2ApplyPlanWithHooksFn and enumerates the failure sources.
… (workflow#699 PR 1 task 3)
…ion=v2 (workflow#699 PR 1 task 4)
…sult; CI lint guard (workflow#699 PR 1 task 5)
… load gate (workflow#699 PR 1 task 4 fixup) Addresses code-reviewer Task-4 findings on commit a081d4a: Critical: requiredOnlyServer in discover_typed_loader_test.go inherited UnimplementedIaCProviderRequiredServer's Capabilities default. Post-Task-6 (when the missing-Apply cascade resolves and CI compiles this package), TestDiscoverAndLoadIaCProvider_ReturnsTypedClient would have failed because the new gate would receive code.Unimplemented from the stub. Fix: add Capabilities method to requiredOnlyServer returning a configurable ComputePlanVersion (default "v2" → accept). startInProcessTypedServer now takes the cpv as a param so per-test variants can flip between accept/reject. Important: TestDiscoverAndLoadIaCProvider_LoadGate_WiredIntoDiscovery exercised the enforceCapabilitiesV2Gate var-seam directly — not the wiring through buildTypedIaCAdapterFrom. A refactor that removed the gate call site would have left the test green. Fix: new TestBuildTypedIaCAdapterFrom_LoadGate_RejectsV1Plugin drives a v1 plugin through the full buildTypedIaCAdapterFrom chain (Conn / ContractRegistry / newTypedIaCAdapter / Initialize / gate) and asserts the operator-facing workflow#699 error. Renamed the existing helper-only test to TestEnforceCapabilitiesV2Gate_HelperBehavior with honest framing.
…task 6) Resolves the intermediate "missing method Apply" cascade that Task 2 introduced when typedIaCAdapter.Apply was deleted. From task-2 commit (625de5b) through task-5 commit (c231daf), `go build ./cmd/wfctl/` and `go vet ./cmd/wfctl/...` failed at ~8 type-assertion / interface- assignment sites because the IaCProvider interface still declared Apply while no implementer (in-process stubs OR the typed adapter) carried the method. This is by design — the plan sequences proto + adapter + helper deletions BEFORE the interface trim so each layer's removal is self-contained and reviewable. Bisecting commits 625de5b..c231daf will encounter the cascade; the resolution lands here. Plan reference: docs/plans/2026-05-17-iac-provider-apply-removal.md Task 6 + Task 2 commit body ("compile may not be green yet — sequential PR within single branch").
…uiredServer (workflow#699 PR 1 task 7)
…1 task 8)
Multi-file v1-cleanup pass:
- iac/iactest/fakeprovider.go — delete DispatchVersion field + the now-orphaned
ComputePlanVersion method (ComputePlanVersionDeclarer is gone with Task 3).
- plugin/sdk/manifest.go — delete EffectiveComputePlanVersion accessor (cycle-2
N7: post-cutover "v1" is not a valid runtime value; authoritative gate is
the typed CapabilitiesResponse check in deploy_providers.go).
- plugin/external/proto/iac_proto_test.go — delete TestApplyResultActionsRoundTrip
(uses deleted pb.ApplyResult/ActionResult); update iacRequiredMethodsCheck
interface to drop Apply method.
- cmd/wfctl/{infra_apply_allow_replace,plan,v2}_test.go +
iac/conformance/scenarios_test.go — drop DispatchVersion: "v2" literals (the
field is gone).
- cmd/wfctl/infra_apply_v2_loader_test.go + infra_apply_jit_loader_test.go —
delete the now-undefined assertions
+ the obsolete ComputePlanVersion+Apply methods on the loader-seam stubs.
- cmd/wfctl/infra_apply_v2_test.go — delete TestApplyWithProviderAndStore_V1FallsThroughToProviderApply
+ TestApplyWithProviderAndStore_V1Path_DeclarerReturnsEmpty + v1RecordingProvider
+ dead ComputePlanVersion method on v2DriverProvider.
- cmd/wfctl/infra_apply_plan_test.go — rewrite TestInfraApplyConsumesPlan to capture
the plan via applyV2ApplyPlanWithHooksFn seam; delete TestInfraApplyPrecomputedPlan_PersistsState
+ TestInfraApplyPrecomputedPlan_FailedDeleteKeepsState + applyCaptureFull (v1-dispatch-
specific; v2 equivalent covered by TestInfraApplyPrecomputedPlan_V2PersistsStateThroughHooks).
- cmd/wfctl/plugin_audit_iac_test.go — drop "IaCProvider.Apply" from classifier
test data (no post-cutover plugin can advertise this).
- cmd/wfctl/deploy_providers.go — reword stale wfctlhelpers.DispatchVersion
comments in findIaCPluginDir (deferred from Task 3 — comment-only).
- interfaces/iac_state.go + iac_state_test.go + iac/wfctlhelpers/apply.go +
plugin/external/proto/iac_proto_test.go — reword stale godoc references to
deleted pb.ActionResult / applyResultFromPB (per code-reviewer Task 5
Minor advisories).
Test seam (installAsV2Dispatch) added to applyCapture / stateReturningProvider /
sizingCapture / applyFailProvider / plainFailProvider / recordingProvider /
readBackedFailingApplyProvider so v1-era tests can route through the v2
dispatch seam to the type's preserved .Apply method + synthesize the
OnResourceApplied/OnResourceDeleted/OnPlanComplete hook lifecycle for
state-persistence assertions.
Build + vet + full test suite green. The intentional nested go-test failure
(TestRunCIRunTestFallsBackToGoTestWhenNoConfiguredTests's inner TestFallbackRuns)
remains as designed — outer test asserts that fallback path errors.
Parallel cascade narrative (per code-reviewer Task 5 Minor advisory): commits
c231daf (Task 5) → 33f35da (Task 6) left plugin/external/proto/iac_proto_test.go
in a non-compiling state due to references to deleted pb.ApplyResult /
pb.ActionResult / pb.ApplyRequest. Same shape as the cmd/wfctl Task-2 cascade
(see commit 33f35da body); resolved by this commit.
…G (workflow#699 PR 1 task 9) Final PR-1 task — closes the IaCProvider.Apply hard-removal sequence: - cmd/iac-codemod/ — entire directory deleted (10 files: main.go, lint.go, refactor_apply.go, refactor_plan.go, add_validate_plan.go, main_test.go, lint_test.go, refactor_apply_test.go, refactor_plan_test.go, add_validate_plan_test.go). The codemod's reason-to-exist (migrate v1 Apply impls to v2 wfctlhelpers.ApplyPlan delegation) evaporated when IaCProvider.Apply was removed from the interface + proto in Tasks 5+6. - Makefile — deleted .PHONY entries build-iac-codemod + migrate-providers, the build-iac-codemod target, the migrate-providers target block, AWS/GCP/ AZURE override variables, and the iac-codemod entry in the clean rule. grep -c iac-codemod Makefile = 0 after edit. - CHANGELOG.md — Unreleased breaking-changes entry covering interface deletion, proto deletion, wfctlhelpers/dispatch deletion, codemod deletion, EffectiveComputePlanVersion deletion, load-time gate, CI lint guard, and minimum plugin versions (aws/gcp/azure/digitalocean v2.0.0+). - docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md — strike iac-codemod tool-references section as completed-and-removed; reference workflow#699 supersession. Pre-merge gate GREEN: - go build ./... → clean - go vet ./... → clean (modernization hints only) - go test ./... → all suites pass (except the intentional nested go-test failure in TestRunCIRunTestFallsBackToGoTestWhenNoConfiguredTests which the outer test asserts must fail). This completes the workflow#699 PR 1 cascade resolution. The branch is ready for v0.56.0-rc1 tag after merge.
Contributor
Author
Implementation complete — all 9 PR-1 tasks shippedBackground team Cascade summary
Pre-merge verification (per Task 9)
Cross-cutting prereqs (completed during execution)
Next steps
Plan lives at |
Comment on lines
+48
to
+53
| @if grep -qE '^\s*rpc Apply\s*\(' plugin/external/proto/iac.proto; then \ | ||
| echo "workflow#699: rpc Apply re-introduced in iac.proto; see decisions/0024-iac-typed-force-cutover.md"; \ | ||
| exit 1; \ | ||
| else \ | ||
| echo "workflow#699 guard: rpc Apply correctly absent"; \ | ||
| fi |
Comment on lines
+10
to
+26
| // TestInfraApply_V2OnlyDispatch_NoV1Branch asserts runInfraApply collapses | ||
| // to a single v2-only dispatch after workflow#699 removes provider.Apply. | ||
| // The presence of any conditional branch on a v1-vs-v2 selector is a | ||
| // regression: per ADR 0024, v2 is the only supported dispatch. | ||
| func TestInfraApply_V2OnlyDispatch_NoV1Branch(t *testing.T) { | ||
| t.Run("collapses dispatch when typedIaCAdapter declares no ComputePlanVersion method", func(t *testing.T) { | ||
| // stub provider satisfies the trimmed interfaces.IaCProvider | ||
| // (no Apply method) and has no ComputePlanVersion declarer. | ||
| // runInfraApply MUST route through wfctlhelpers.ApplyPlanWithHooks | ||
| // and MUST NOT type-assert against a v1 dispatch. | ||
| var p interfaces.IaCProvider = &stubV2OnlyProvider{} | ||
| if _, ok := p.(interface { | ||
| Apply(context.Context, *interfaces.IaCPlan) (*interfaces.ApplyResult, error) | ||
| }); ok { | ||
| t.Fatalf("provider unexpectedly satisfies legacy Apply interface") | ||
| } | ||
| }) |
Comment on lines
58
to
67
| @@ -67,8 +67,7 @@ func TestApplyWithProviderAndStore_PassesLiveProviderToComputePlan(t *testing.T) | |||
| // surfaced via the optional ComputePlanVersionDeclarer interface). | |||
| case "": | ||
| log.Printf("plugin %q: deprecation — manifest iacProvider.computePlanVersion is empty; declare \"v2\" explicitly (workflow#699)", pName) | ||
| case "v1": | ||
| log.Printf("plugin %q: deprecation — manifest iacProvider.computePlanVersion=\"v1\"; load-time gate will reject this (workflow#699)", pName) |
golang.org/x/tools demoted to indirect after cmd/iac-codemod deletion removed the direct dependency.
… clarity 4 material findings: - Makefile lint guard: \s is non-portable in POSIX ERE (BSD/busybox grep treats it as literal s); switch to [[:space:]] so the workflow#699 re-introduction guard fires correctly on every CI host. - deploy_providers.go:230,232 deprecation logs: clarify that runtime enforcement reads typed CapabilitiesResponse, not the manifest field (out-of-date manifest with v2 in Capabilities will still load). - infra_apply_v2_test.go:58-67: rewrite stale godoc that still described the deleted v1-vs-v2 manifest-driven dispatch decision; describe the current v2-only path + load-time enforcement. - infra_apply_v2_only_test.go: clarify the test's role as structural tripwire vs the runtime apply-path coverage in TestApplyWithProviderAndStore_V2RoutesThroughWfctlhelpers.
Comment on lines
126
to
+130
| // Until the follow-up: callers receive the value but discard it. The raw | ||
| // string is unconstrained — schema-validated values are {"", "v1", "v2"} | ||
| // per wfctlhelpers.DispatchVersionV2, but this loader path performs only | ||
| // minimal json.Unmarshal so MUST NOT assume. | ||
| // string is unconstrained — SDK schema-validated values are {"", "v1", | ||
| // "v2"}, but this loader path performs only minimal json.Unmarshal so | ||
| // MUST NOT assume. Per workflow#699, the authoritative gate is the | ||
| // typed CapabilitiesResponse check in discoverAndLoadIaCProvider. |
Comment on lines
+12
to
+14
| // declares Apply. If any implementation accidentally re-adds an Apply | ||
| // method that satisfies the legacy v1 dispatch signature, this test | ||
| // fires. |
Comment on lines
+56
to
+76
| // installAsV2Dispatch substitutes the global applyV2ApplyPlanWithHooksFn | ||
| // seam so the test's call to applyInfraModules / applyWithProviderAndStore / | ||
| // applyPrecomputedPlanWithStore routes through this stub's Apply method | ||
| // instead of the real wfctlhelpers.ApplyPlanWithHooks (which would | ||
| // dispatch per-action via ResourceDriver, a layer most v1-era tests don't | ||
| // stub). Also fires the appropriate OnResourceApplied / OnResourceDeleted | ||
| // hooks for each plan action so state-persistence assertions still pass | ||
| // without the per-action driver layer. Auto-restored on test cleanup. | ||
| // Per workflow#699 fixup. | ||
| func (f *applyCapture) installAsV2Dispatch(t testing.TB) { | ||
| t.Helper() | ||
| orig := applyV2ApplyPlanWithHooksFn | ||
| applyV2ApplyPlanWithHooksFn = func(ctx context.Context, p interfaces.IaCProvider, plan *interfaces.IaCPlan, hooks wfctlhelpers.ApplyPlanHooks) (*interfaces.ApplyResult, error) { | ||
| result, err := f.Apply(ctx, plan) | ||
| if hookErr := fireSyntheticHooks(ctx, p, plan, hooks, result); hookErr != nil && err == nil { | ||
| err = hookErr | ||
| } | ||
| return result, err | ||
| } | ||
| t.Cleanup(func() { applyV2ApplyPlanWithHooksFn = orig }) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
IaCProvider.Applyfrom interface + protoCapabilitiesRPC (eliminates need to backfilliacProvider.computePlanVersionin aws/gcp/azure plugin.jsons)Pipeline gates passed
237183aba334…recordedCascade overview (Scope Manifest)
This PR commits the design + plan + scope-lock. Implementation commits (Tasks 1-9 per the plan) land on this branch in subsequent commits.
Design + plan
docs/plans/2026-05-17-iac-provider-apply-removal-design.mddocs/plans/2026-05-17-iac-provider-apply-removal.mddocs/plans/2026-05-17-iac-provider-apply-removal.md.scope-lockTest plan
go build ./... && go vet ./... && go test ./...green at PR 1 task 9rpc Applyre-introductionv0.56.0-rc1after merge🤖 Generated with Claude Code