From 2cef64e8fccc72963ceaf7de3afa935a00092056 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Thu, 21 May 2026 10:08:23 -0400 Subject: [PATCH] fix(iac): let drivers suppress new-resource creates --- cmd/wfctl/iac_typed_adapter.go | 24 ++++++++++ cmd/wfctl/iac_typed_adapter_test.go | 28 ++++++++++++ platform/differ.go | 71 +++++++++++++++++++++++++++-- platform/differ_test.go | 44 ++++++++++++++++++ platform/fake_provider_test.go | 19 +++++++- 5 files changed, 180 insertions(+), 6 deletions(-) diff --git a/cmd/wfctl/iac_typed_adapter.go b/cmd/wfctl/iac_typed_adapter.go index 99790388..828b3685 100644 --- a/cmd/wfctl/iac_typed_adapter.go +++ b/cmd/wfctl/iac_typed_adapter.go @@ -738,6 +738,29 @@ func (d *typedResourceDriver) SensitiveKeys() []string { return append([]string(nil), resp.GetKeys()...) } +func (d *typedResourceDriver) AdoptionRef(spec interfaces.ResourceSpec) (interfaces.ResourceRef, bool, error) { + switch spec.Type { + case "infra.dns", "infra.dns_delegation": + ref := interfaces.ResourceRef{Name: spec.Name, Type: spec.Type} + if providerID, _ := spec.Config["provider_id"].(string); providerID != "" { + ref.ProviderID = providerID + } else if domain, _ := spec.Config["domain"].(string); domain != "" { + ref.ProviderID = domain + } else { + ref.ProviderID = spec.Name + } + return ref, true, nil + default: + if !boolFromAny(spec.Config["adopt_existing"]) { + return interfaces.ResourceRef{}, false, nil + } + if spec.Name == "" { + return interfaces.ResourceRef{}, false, fmt.Errorf("%s adoption requires resource name", spec.Type) + } + return interfaces.ResourceRef{Name: spec.Name, Type: spec.Type}, true, nil + } +} + func (d *typedResourceDriver) SupportsConfigAdoption() bool { return true } @@ -1389,5 +1412,6 @@ var ( _ interfaces.ProviderCredentialRevoker = (*typedIaCAdapter)(nil) _ interfaces.ProviderMigrationRepairer = (*typedIaCAdapter)(nil) _ interfaces.ResourceDriver = (*typedResourceDriver)(nil) + _ interfaces.ResourceAdoptionLocator = (*typedResourceDriver)(nil) _ interfaces.Troubleshooter = (*typedResourceDriver)(nil) ) diff --git a/cmd/wfctl/iac_typed_adapter_test.go b/cmd/wfctl/iac_typed_adapter_test.go index a495314f..c01f9d3b 100644 --- a/cmd/wfctl/iac_typed_adapter_test.go +++ b/cmd/wfctl/iac_typed_adapter_test.go @@ -244,6 +244,34 @@ func TestTypedAdapter_EndToEnd_NameVersionEnumerateAll(t *testing.T) { } } +func TestTypedResourceDriver_AdoptionRefDNSDelegationUsesDomain(t *testing.T) { + driver := &typedResourceDriver{resourceType: "infra.dns_delegation"} + locator, ok := any(driver).(interfaces.ResourceAdoptionLocator) + if !ok { + t.Fatal("typedResourceDriver must expose ResourceAdoptionLocator for external IaC drivers") + } + + ref, ok, err := locator.AdoptionRef(interfaces.ResourceSpec{ + Name: "gocodealone-tech-delegation", + Type: "infra.dns_delegation", + Config: map[string]any{"domain": "gocodealone.tech"}, + }) + if err != nil { + t.Fatalf("AdoptionRef returned error: %v", err) + } + if !ok { + t.Fatal("AdoptionRef returned ok=false for dns delegation") + } + want := interfaces.ResourceRef{ + Name: "gocodealone-tech-delegation", + Type: "infra.dns_delegation", + ProviderID: "gocodealone.tech", + } + if ref != want { + t.Fatalf("AdoptionRef = %+v, want %+v", ref, want) + } +} + // ─── In-process gRPC test fixture ─────────────────────────────────────────── // ─── ADR-0029 capability-extension tests ───────────────────────────────── diff --git a/platform/differ.go b/platform/differ.go index c9cd831f..aebfe024 100644 --- a/platform/differ.go +++ b/platform/differ.go @@ -4,6 +4,7 @@ import ( "context" "crypto/sha256" "encoding/json" + "errors" "fmt" "os" "sort" @@ -108,11 +109,13 @@ func ComputePlan(ctx context.Context, p interfaces.IaCProvider, desired []interf hash = configHash(spec.Config) } if rs, exists := currentMap[spec.Name]; !exists { - creates = append(creates, interfaces.PlanAction{ - Action: "create", - Resource: spec, - ResolvedConfigHash: hash, - }) + create, err := classifyCreate(ctx, p, spec, hash) + if err != nil { + return interfaces.IaCPlan{}, err + } + if create != nil { + creates = append(creates, *create) + } } else { candidates = append(candidates, modCandidate{ spec: spec, @@ -350,6 +353,64 @@ func classifyModification(ctx context.Context, p interfaces.IaCProvider, spec in return nil } +// classifyCreate decides whether a desired resource absent from local state +// should produce a create action. Drivers that opt in to ResourceAdoptionLocator +// get one chance to locate/read an external resource and Diff it before create. +// Nil provider, nil driver, or non-adoption drivers preserve legacy create +// behavior. +func classifyCreate(ctx context.Context, p interfaces.IaCProvider, spec interfaces.ResourceSpec, hash string) (*interfaces.PlanAction, error) { + create := &interfaces.PlanAction{ + Action: "create", + Resource: spec, + ResolvedConfigHash: hash, + } + if p == nil { + return create, nil + } + driver := resourceDriverForCreate(p, spec.Type) + if driver == nil { + return create, nil + } + locator, ok := driver.(interfaces.ResourceAdoptionLocator) + if !ok { + return create, nil + } + ref, ok, err := locator.AdoptionRef(spec) + if err != nil { + return nil, fmt.Errorf("provider.AdoptionRef(%q/%q): %w", spec.Type, spec.Name, err) + } + if !ok { + return create, nil + } + current, err := driver.Read(ctx, ref) + if err != nil { + if errors.Is(err, interfaces.ErrResourceNotFound) { + return create, nil + } + return nil, fmt.Errorf("provider.Read(%q/%q): %w", spec.Type, spec.Name, err) + } + if current == nil { + return create, nil + } + diff, err := driver.Diff(ctx, spec, current) + if err != nil { + return nil, fmt.Errorf("provider.Diff(%q/%q): %w", spec.Type, spec.Name, err) + } + if diff == nil || (!diff.NeedsUpdate && !diff.NeedsReplace && !hasForceNew(diff.Changes)) { + return nil, nil + } + create.Changes = diff.Changes + return create, nil +} + +func resourceDriverForCreate(p interfaces.IaCProvider, resourceType string) interfaces.ResourceDriver { + driver, err := p.ResourceDriver(resourceType) + if err != nil { + return nil + } + return driver +} + // resourceStateToOutput converts the persisted ResourceState into the // *interfaces.ResourceOutput shape that ResourceDriver.Diff expects. // Sensitive map is not reconstructed here — drivers that need it should diff --git a/platform/differ_test.go b/platform/differ_test.go index 0ea0bb57..28283844 100644 --- a/platform/differ_test.go +++ b/platform/differ_test.go @@ -58,6 +58,50 @@ func TestDiffer_NewResource(t *testing.T) { } } +func TestComputePlan_NewResource_AdoptionNoopDiffSkipsCreate(t *testing.T) { + desired := []interfaces.ResourceSpec{ + {Name: "delegation", Type: "infra.dns_delegation", Config: map[string]any{"domain": "example.com"}}, + } + driver := &fakeDriver{ + diff: &interfaces.DiffResult{NeedsUpdate: false}, + adopt: true, + readOutput: &interfaces.ResourceOutput{Name: "delegation", Type: "infra.dns_delegation", ProviderID: "example.com"}, + } + provider := &fakeProvider{driver: driver} + + plan, err := platform.ComputePlan(context.Background(), provider, desired, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(plan.Actions) != 0 { + t.Fatalf("expected no actions for driver no-op new resource, got %+v", plan.Actions) + } + if got := driver.diffCallCount.Load(); got != 1 { + t.Fatalf("Diff call count = %d, want 1", got) + } +} + +func TestComputePlan_NewResource_NoAdoptionDoesNotCallDiff(t *testing.T) { + desired := []interfaces.ResourceSpec{ + {Name: "delegation", Type: "infra.dns_delegation", Config: map[string]any{"domain": "example.com"}}, + } + driver := &fakeDriver{diff: &interfaces.DiffResult{NeedsUpdate: true}} + provider := &fakeProvider{driver: driver} + + plan, err := platform.ComputePlan(context.Background(), provider, desired, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(plan.Actions) != 1 || plan.Actions[0].Action != "create" { + t.Fatalf("expected one create action, got %+v", plan.Actions) + } + if got := driver.diffCallCount.Load(); got != 0 { + t.Fatalf("Diff call count = %d, want 0 without adoption opt-in", got) + } +} + func TestDiffer_DeletedResource(t *testing.T) { desired := []interfaces.ResourceSpec{} current := []interfaces.ResourceState{ diff --git a/platform/fake_provider_test.go b/platform/fake_provider_test.go index e745ddaa..26415e37 100644 --- a/platform/fake_provider_test.go +++ b/platform/fake_provider_test.go @@ -90,6 +90,9 @@ type fakeDriver struct { diff *interfaces.DiffResult diffErr error diffCallCount atomic.Int64 + adopt bool + adoptRef interfaces.ResourceRef + readOutput *interfaces.ResourceOutput } // Compile-time interface conformance check. @@ -99,7 +102,7 @@ func (d *fakeDriver) Create(_ context.Context, _ interfaces.ResourceSpec) (*inte return nil, nil } func (d *fakeDriver) Read(_ context.Context, _ interfaces.ResourceRef) (*interfaces.ResourceOutput, error) { - return nil, nil + return d.readOutput, nil } func (d *fakeDriver) Update(_ context.Context, _ interfaces.ResourceRef, _ interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { return nil, nil @@ -119,3 +122,17 @@ func (d *fakeDriver) Scale(_ context.Context, _ interfaces.ResourceRef, _ int) ( return nil, nil } func (d *fakeDriver) SensitiveKeys() []string { return nil } + +func (d *fakeDriver) AdoptionRef(spec interfaces.ResourceSpec) (interfaces.ResourceRef, bool, error) { + if !d.adopt { + return interfaces.ResourceRef{}, false, nil + } + ref := d.adoptRef + if ref.Name == "" { + ref.Name = spec.Name + } + if ref.Type == "" { + ref.Type = spec.Type + } + return ref, true, nil +}