feat(iac): ResourceReplacer optional driver hook + DefaultReplace export#577
Merged
Conversation
Adds the interfaces.ResourceReplacer optional driver interface for resources requiring orchestration beyond naive Delete-then-Create (e.g., Droplets with attached Block Storage Volumes). Non-implementing drivers retain current behavior via DefaultReplace fallback. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Renames doReplace body to DefaultReplace (exported) so drivers implementing ResourceReplacer can delegate specific specs back to the engine-default Delete-then-Create without a sentinel-error round-trip. doReplace becomes a thin dispatch wrapper. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…prefix backstop doReplace now probes the driver for interfaces.ResourceReplacer: on opt-in, the driver's Replace(ctx, oldRef, spec) takes ownership of the full transition; non-opt-in drivers continue via DefaultReplace. Error-prefix backstop wraps non-conforming driver errors with "replace: driver: " so operator attribution is preserved regardless of per-plugin error-wrapping discipline. Conforming errors (starting with "replace:" or "<word> replace ") pass through unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ceErrorPrefix Adds apply_replacer_prefix_test.go covering all branches of wrapDriverReplaceError and hasReplaceErrorPrefix: "replace:" family pass-through, driver-owned "<word> replace " family, non-conforming backstop wrapping, and nil pass-through. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an opt-in ResourceReplacer driver hook to let IaC resource drivers own the full Replace transition when Delete-then-Create is insufficient (e.g., single-attach dependents), while preserving engine-default behavior for all existing drivers. Also exports the engine’s default Replace implementation for reuse by drivers and adds docs/tests around dispatch and error-prefix attribution.
Changes:
- Introduce
interfaces.ResourceReplaceroptional interface (Replace(ctx, oldRef, spec)) for driver-owned Replace orchestration. - Export
wfctlhelpers.DefaultReplaceand routedoReplacethrough the driver hook when available, with an error-prefix backstop wrapper. - Add ADR 0014 + changelog entry; add unit tests for dispatch and error-prefix handling.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
interfaces/iac_resource_driver.go |
Adds the ResourceReplacer optional driver interface with behavioral/error-prefix contract notes. |
interfaces/iac_resource_driver_test.go |
Adds compile-time interface shape assertion for ResourceReplacer. |
iac/wfctlhelpers/apply.go |
Exports DefaultReplace, adds doReplace dispatch to ResourceReplacer, and implements error-prefix backstop helpers. |
iac/wfctlhelpers/apply_replacer_prefix_test.go |
Adds tests for accepted error-prefix families and backstop wrapping behavior. |
iac/wfctlhelpers/apply_replacer_dispatch_test.go |
Adds tests verifying replacer dispatch vs fallback, but one test currently under-asserts intended behavior. |
decisions/0014-resource-replacer-driver-interface.md |
Adds ADR documenting motivation, decision, and consequences for the new hook. |
CHANGELOG.md |
Documents the new interface and exported DefaultReplace. |
Comment on lines
+89
to
+96
| err := doReplace(context.Background(), d, action, result) | ||
| if err == nil { | ||
| t.Fatal("expected error from failing Replacer") | ||
| } | ||
| // After Task 11 lands, this will assert "replace: driver: kaboom". | ||
| // For now assert only that an error was returned (pre-Task-11 compilation). | ||
| if err.Error() == "" { | ||
| t.Error("error message is empty") |
Comment on lines
+517
to
+521
| // head must be one alphanumeric token (no spaces, no colons). | ||
| for _, r := range head { | ||
| if r == ' ' || r == ':' { | ||
| return false | ||
| } |
| - `iac/wfctlhelpers/apply.go` — `doReplace` dispatch + `DefaultReplace` + `wrapDriverReplaceError`. | ||
| - `iac/wfctlhelpers/apply_replacer_dispatch_test.go` — dispatch tests. | ||
| - `iac/wfctlhelpers/apply_replacer_prefix_test.go` — prefix backstop tests. | ||
| - core-dump decisions/0010 — canonical source for this decision. |
- Strengthened TestDoReplace_ReplacerError_IsWrappedByBackstop to assert the 'replace: driver: ' backstop prefix on non-conforming driver errors (removed stale pre-Task-11 placeholder comment) - Clarified hasReplaceErrorPrefix comment: head is a single no-space/no-colon token (not strictly alphanumeric — hyphens/underscores/dots accepted) - Fixed ADR 0014 cross-reference to point to local decisions/0010 file instead of ambiguous out-of-repo reference Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
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
interfaces.ResourceReplaceroptional driver interface for resources owning single-attach dependents (e.g., DO Droplets with Block Storage Volumes). Drivers opt in by implementingReplace(ctx, oldRef, spec) → (*ResourceOutput, error)and take ownership of the full Replace transition.wfctlhelpers.DefaultReplaceexported (priordoReplacebody), callable directly by drivers that want engine-default Delete-then-Create for a specific spec without a sentinel-error round-trip.wrapDriverReplaceError) wraps non-conforming errors with"replace: driver: "for consistent operator attribution. Conforming prefixes pass through unchanged.DropletDriver.Replace adoption follows in workflow-plugin-digitalocean PR-3.
Test plan
go test ./interfaces/ ./iac/wfctlhelpers/ -vpassesgolangci-lint run ./interfaces/ ./iac/wfctlhelpers/— 0 issuesTestResourceReplacerInterfaceShapeTestDoReplace_DispatchesToReplacerWhenAvailable— Replacer.Replace invoked, DefaultReplace NOT firedTestDoReplace_DefaultReplaceWhenNoReplacerInterface— DefaultReplace fires for non-implementing driversTestWrapDriverReplaceError_*— conforming prefixes pass through, non-conforming get wrappedapply_replace_test.goandapply_replace_cascade_test.gotests remain green🤖 Generated via brainstorming → writing-plans pipeline