feat(iac): DriftConfigDetector optional interface + applied-config-source sentinel + caller wiring#569
Merged
Merged
Conversation
…(TDD-gates T1.5) Adds a compat smoke test that verifies the wire-format fallback story: when a remote plugin returns "method not found" for DetectDriftWithApplied, the wfctl caller must fall through to legacy IaCProvider.DetectDrift rather than propagating the error. Red phase — test fails until T6 adds DetectDriftWithApplied to remoteIaCProvider. Plan: workflow-plugin-digitalocean#worktree-agent-ad141ad23872c3337 docs/plans/2026-05-06-iac-state-truth-and-tc2-closeout.md Task 1 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ugin-digitalocean#47) Adds interfaces.DriftConfigDetector as an OPTIONAL plugin-side interface following the established repo pattern (ComputePlanVersionDeclarer, ProviderValidator, Enumerator, UpsertSupporter). IaCProvider.DetectDrift is unchanged; sibling plugins (aws/gcp/azure) require zero code changes. Callers type-assert and fall back to existence-only DetectDrift on the negative case. Providers that implement DriftConfigDetector can return DriftClassConfig with field-level detail when applied config is available. ADR 0007 records the decision and the rejected required-signature alternative. Plan: workflow-plugin-digitalocean#worktree-agent-ad141ad23872c3337 docs/plans/2026-05-06-iac-state-truth-and-tc2-closeout.md Task 2 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…doption provenance)
Adds AppliedConfigSource string (json:"applied_config_source,omitempty") to
ResourceState to discriminate user-supplied config ("apply") from
adoption-shaped Outputs reflow ("adoption"). Empty string = legacy state,
treated as "adoption" (conservative default).
DriftConfigDetector.DetectDriftWithApplied uses this field (via
buildAppliedSpecMap) to refuse false-positive config-drift on adopted
resources. JSON compat is bidirectional: omitempty ensures old state files
decode cleanly; new state files are silently tolerated by old code.
ADR 0010 records the decision and the rejected magic-key-in-AppliedConfig alternative.
Plan: workflow-plugin-digitalocean#worktree-agent-ad141ad23872c3337 docs/plans/2026-05-06-iac-state-truth-and-tc2-closeout.md Task 3
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n sites Sets AppliedConfigSource="apply" in applyWithProviderAndStore and applyPrecomputedPlanWithStore (user-supplied config from ResourceSpec.Config). Sets AppliedConfigSource="adoption" in resourceStateFromLiveOutput (Outputs reflow via liveConfigFromOutputs). DriftConfigDetector consumers use this field (via buildAppliedSpecMap) to refuse false-positive config-drift on adoption-shaped entries. Plan: workflow-plugin-digitalocean#worktree-agent-ad141ad23872c3337 docs/plans/2026-05-06-iac-state-truth-and-tc2-closeout.md Task 4 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds buildAppliedSpecMap(states, refs) that walks ResourceState entries and emits only "apply"-provenance entries as the applied map for DetectDriftWithApplied. Adoption and legacy (empty AppliedConfigSource) entries are omitted to prevent false-positive config-drift on adoption-shaped Outputs. Returns nil when no safe entries exist so callers can short-circuit. Plan: workflow-plugin-digitalocean#worktree-agent-ad141ad23872c3337 docs/plans/2026-05-06-iac-state-truth-and-tc2-closeout.md Task 5 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ertion with legacy fallback Adds DetectDriftWithApplied to remoteIaCProvider: dispatches to the v2 RPC when the remote plugin supports it; falls back to legacy DetectDrift when the remote returns "method not found" (isMethodNotFound sentinel). This is the wire-format counterpart of the caller-side type-assertion. Wires caller-side type-assertion in: - runInfraApplyRefreshPhase (infra_apply_refresh.go) - driftGroup closure in driftInfraModules (infra_status_drift.go) Both sites use buildAppliedSpecMap to filter states for safe "apply"- provenance entries only before calling DetectDriftWithApplied. T1 TDD gate now passes (TestRemoteIaC_OptionalDriftConfigDetector_FallsBackOnLegacyPlugin). Plan: workflow-plugin-digitalocean#worktree-agent-ad141ad23872c3337 docs/plans/2026-05-06-iac-state-truth-and-tc2-closeout.md Task 6 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the IaC drift detection pipeline to optionally support config-drift classification by (a) adding an opt-in interfaces.DriftConfigDetector capability and (b) introducing an AppliedConfigSource provenance field on persisted ResourceState so drift detection can safely ignore adoption-shaped applied configs. It also wires the new capability through wfctl’s drift callers and the remote (plugin) dispatch layer with backward-compatible fallbacks.
Changes:
- Add optional
interfaces.DriftConfigDetectorinterface and wire wfctl callers + remote provider dispatch with legacy fallback behavior. - Add
ResourceState.AppliedConfigSourcesentinel to distinguish “apply” vs “adoption” provenance and prevent false-positive config drift. - Add
buildAppliedSpecMaphelper plus tests, and setAppliedConfigSourceat all relevant state write sites.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| interfaces/iac_state.go | Adds AppliedConfigSource field to ResourceState with documented semantics. |
| interfaces/iac_state_test.go | Adds JSON compatibility/round-trip tests for AppliedConfigSource. |
| interfaces/iac_provider.go | Introduces optional DriftConfigDetector interface for drift-with-applied detection. |
| interfaces/iac_provider_test.go | Adds test pinning the optional-interface type-assertion behavior. |
| cmd/wfctl/infra_status_drift.go | Updates drift status path to use optional drift-with-applied when supported. |
| cmd/wfctl/infra_drift_applied.go | Adds helper to construct per-resource applied-config map filtered by provenance. |
| cmd/wfctl/infra_drift_applied_test.go | Adds unit tests for applied-config map construction and copy semantics. |
| cmd/wfctl/infra_apply.go | Writes AppliedConfigSource at apply/adoption state persistence sites. |
| cmd/wfctl/infra_apply_test.go | Adds tests asserting state persistence records correct provenance. |
| cmd/wfctl/infra_apply_refresh.go | Updates refresh-phase drift check to use optional drift-with-applied path. |
| cmd/wfctl/deploy_providers.go | Adds remote dispatch for DetectDriftWithApplied with method-not-found fallback. |
| cmd/wfctl/deploy_providers_remote_iac_test.go | Adds happy-path test for remote DetectDriftWithApplied. |
| cmd/wfctl/deploy_providers_remote_iac_compat_test.go | Adds compat test ensuring fallback to legacy DetectDrift when RPC is missing. |
| decisions/0007-iac-driftconfigdetector-optional-interface.md | Adds ADR documenting the optional-interface design decision. |
| decisions/0010-applied-config-source-on-resourcestate.md | Adds ADR documenting the AppliedConfigSource sentinel decision. |
Comment on lines
+33
to
+38
| // configDetector is a local interface alias for DriftConfigDetector to avoid | ||
| // importing the full interfaces package name in the type-assertion switch. | ||
| // It matches interfaces.DriftConfigDetector exactly. | ||
| type configDetector interface { | ||
| DetectDriftWithApplied(ctx context.Context, refs []interfaces.ResourceRef, applied map[string]map[string]any) ([]interfaces.DriftResult, error) | ||
| } |
Comment on lines
+56
to
+65
| // Use DriftConfigDetector when the provider supports it (optional interface). | ||
| // Falls back to existence-only DetectDrift on the negative type-assertion. | ||
| var results []interfaces.DriftResult | ||
| var err error | ||
| if d, ok := provider.(configDetector); ok { | ||
| appliedMap := buildAppliedSpecMap(states, refs) | ||
| results, err = d.DetectDriftWithApplied(ctx, refs, appliedMap) | ||
| } else { | ||
| results, err = provider.DetectDrift(ctx, refs) | ||
| } |
Comment on lines
+103
to
+110
| // Use DriftConfigDetector when the provider supports it (optional interface). | ||
| // Falls back to existence-only DetectDrift on the negative type-assertion. | ||
| type configDetectorLocal interface { | ||
| DetectDriftWithApplied(ctx context.Context, refs []interfaces.ResourceRef, applied map[string]map[string]any) ([]interfaces.DriftResult, error) | ||
| } | ||
| var results []interfaces.DriftResult | ||
| if d, ok := provider.(configDetectorLocal); ok { | ||
| appliedMap := buildAppliedSpecMap(states, g.refs) |
| - **Status:** Accepted | ||
| - **Date:** 2026-05-06 | ||
| - **Deciders:** Claude (autonomous design pipeline), Jon Langevin (mandate) | ||
| - **Refs:** workflow-plugin-digitalocean#47, docs/plans/2026-05-06-iac-state-truth-and-tc2-closeout-design.md |
| - **Status:** Accepted | ||
| - **Date:** 2026-05-06 | ||
| - **Deciders:** Claude (autonomous design pipeline), Jon Langevin (mandate) | ||
| - **Refs:** ADR 0007, docs/plans/2026-05-06-iac-state-truth-and-tc2-closeout-design.md |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
…Map short-circuit Addresses Copilot review: - Remove redundant configDetector alias in infra_apply_refresh.go; use interfaces.DriftConfigDetector directly (no type alias drift risk). - Add nil-appliedMap short-circuit in both call sites (infra_apply_refresh.go and infra_status_drift.go): when buildAppliedSpecMap returns nil (no "apply"-provenance entries), fall through to legacy DetectDrift to avoid unnecessary RPC round-trips and remote-plugin method-not-found fallbacks. - Use interfaces.DriftConfigDetector in infra_status_drift.go instead of duplicated function-local interface definition. - Update ADR cross-references to use GitHub URLs for the plan doc in the DO plugin repo (the doc doesn't exist in this repo). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.DriftConfigDetectorinterface for config-drift classification (parallel to ComputePlanVersionDeclarer pattern).IaCProvider.DetectDriftis unchanged; sibling plugins (aws/gcp/azure) require zero code changes.AppliedConfigSourcefield onResourceStateto discriminate "apply" (user-supplied config) vs "adoption" (Outputs reflow) provenance. Empty string = legacy state, conservative default to adoption treatment.buildAppliedSpecMaphelper that filters states to "apply"-provenance entries only, preventing false-positive config-drift on adopted resources.remoteIaCProvider.DetectDriftWithApplied) with method-not-found fallback to legacyDetectDrift.runInfraApplyRefreshPhaseanddriftInfraModules.AppliedConfigSourceat all three write sites:applyWithProviderAndStore,applyPrecomputedPlanWithStore(both "apply"), andresourceStateFromLiveOutput("adoption").Scope
PR 1 of 5 in the IaC state-truth + TC2 closeout plan series:
docs/plans/2026-05-06-iac-state-truth-and-tc2-closeout.mdADRs
decisions/0007-iac-driftconfigdetector-optional-interface.md— records optional-declarer pattern choice over required-signature changedecisions/0010-applied-config-source-on-resourcestate.md— records field-on-ResourceState choice over magic-key in AppliedConfigPre-flight checks
Out-of-tree plugin survey:
gh search code "iac.provider" --owner GoCodeAlone --extension json --limit 50returned only GoCodeAlone-controlled plugins:workflow-plugin-digitaloceanandworkflow-plugin-aws. No third-party iac.provider plugins found. Survey confirms the optional-declarer pattern affects only known plugins under our control.Manifest-field tolerance smoke: Tested wfctl's
iacPluginManifeststruct (usesjson.Unmarshalwith no schema validation per code comment at line 110). Go'sencoding/jsonsilently ignores unknown fields by design. A manifest with"iacProvider": {"driftConfigSupported": true}parses cleanly in any wfctl version. Minimum wfctl version: none (manifest field is future-only signal for DO plugin PR 2; current wfctl ignores it gracefully).Test plan
go test ./...PASS in worktreego test -race ./...PASS in worktree (interfaces + cmd/wfctl)go test ./...— zero failures across all packages🤖 Generated with Claude Code
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com