fix(iac): persist apply state per action#639
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens wfctl’s IaC apply flow by persisting state at per-action mutation boundaries (v2 apply path), making delete-state pruning more cancellation-resistant, and adding compensation logic to avoid orphaned resources or partial state when persistence/routing/validation fails.
Changes:
- Add
wfctlhelpers.ApplyPlanWithHooks(+ hook plumbing in replace/default replace) so wfctl can persist state immediately after each successful cloud mutation. - Update
wfctl infra applyto use action-boundary hooks for v2 providers, and add safer delete-state pruning + compensation behavior across v2 and legacy paths. - Enhance sensitive-output routing to return partially-hydrated results on failure, enabling cleanup/compensation.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| iac/wfctlhelpers/apply.go | Introduces apply hooks, integrates delete/apply callbacks, and guards provider-owned replace when delete hooks are active. |
| iac/wfctlhelpers/apply_replacer_dispatch_test.go | Extends replace dispatch tests to cover hook-driven replacer restrictions and preflight behavior. |
| iac/wfctlhelpers/apply_hooks_test.go | Adds tests validating hook ordering, delete-hook semantics, and JIT-output map updates after delete. |
| iac/sensitive/route.go | Returns partial hydrated map on routing errors to support compensating cleanup of partial writes. |
| iac/sensitive/route_test.go | Adds coverage for partial-hydration behavior on provider.Set errors. |
| cmd/wfctl/infra_validation.go | Refactors ProviderID validation to allow validating with an already-resolved driver. |
| cmd/wfctl/infra_apply.go | Wires v2 apply through ApplyPlanWithHooks, adds action-boundary persistence hooks, delete-state timeout deletion, and enhanced compensation/identity validation. |
| cmd/wfctl/infra_apply_v2_test.go | Adds extensive v2 apply behavior tests (early persistence, delete cancellation, sensitive routing rollback, identity normalization, update non-compensation). |
| cmd/wfctl/infra_apply_v2_loader_test.go | Ensures loader-seam v2 apply persists real state to disk (end-to-end). |
| cmd/wfctl/infra_apply_test.go | Adds legacy-path tests for update non-compensation, delete pruning behavior under partial failure, and cancellation-safe pruning. |
| cmd/wfctl/infra_apply_sensitive_routing_test.go | Expands compensation coverage when routing/persistence fails (including no-secrets-provider cases). |
| cmd/wfctl/infra_apply_plan_test.go | Adds precomputed-plan v2 hook persistence + drift-report test and delete-state retention test. |
| cmd/wfctl/infra_apply_allow_replace_test.go | Updates v2 dispatch seam usage to the new ApplyPlan-with-hooks seam. |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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
ApplyPlanWithHooksReview notes
ResourceReplacerremains allowed when no delete-state hook is active; with delete hooks active it aborts before any mutation because callback ordering is not enforceable generically.ApplyResult.Errorsdisables inferred delete-state pruning.Verification
GOWORK=off go test ./iac/wfctlhelpers ./iac/sensitive ./interfaces ./cmd/wfctl -run 'TestApplyPlanWithHooks|TestDoReplace|TestRoute_|TestApplyWithProvider_|TestApplyWithProviderAndStore_V2|TestInfraApplyPrecomputedPlan|TestApply_V2_LoaderSeamDispatch_EndToEnd|TestApplyWithProviderAndStore_ProtectedReplace_WithAllowReplace_Proceeds|TestPersistResourceWithSecretRouting|TestRequireSecretsProviderForSensitiveOutputs' -count=1git diff --checkAntagonistic review
Multiple adversarial review rounds were run. Final targeted review returned SHIP-IT after checking: