Skip to content

Commit e5acccc

Browse files
authored
fix(iac): let drivers suppress new-resource creates (#753)
1 parent 644200a commit e5acccc

5 files changed

Lines changed: 180 additions & 6 deletions

File tree

cmd/wfctl/iac_typed_adapter.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,29 @@ func (d *typedResourceDriver) SensitiveKeys() []string {
738738
return append([]string(nil), resp.GetKeys()...)
739739
}
740740

741+
func (d *typedResourceDriver) AdoptionRef(spec interfaces.ResourceSpec) (interfaces.ResourceRef, bool, error) {
742+
switch spec.Type {
743+
case "infra.dns", "infra.dns_delegation":
744+
ref := interfaces.ResourceRef{Name: spec.Name, Type: spec.Type}
745+
if providerID, _ := spec.Config["provider_id"].(string); providerID != "" {
746+
ref.ProviderID = providerID
747+
} else if domain, _ := spec.Config["domain"].(string); domain != "" {
748+
ref.ProviderID = domain
749+
} else {
750+
ref.ProviderID = spec.Name
751+
}
752+
return ref, true, nil
753+
default:
754+
if !boolFromAny(spec.Config["adopt_existing"]) {
755+
return interfaces.ResourceRef{}, false, nil
756+
}
757+
if spec.Name == "" {
758+
return interfaces.ResourceRef{}, false, fmt.Errorf("%s adoption requires resource name", spec.Type)
759+
}
760+
return interfaces.ResourceRef{Name: spec.Name, Type: spec.Type}, true, nil
761+
}
762+
}
763+
741764
func (d *typedResourceDriver) SupportsConfigAdoption() bool {
742765
return true
743766
}
@@ -1389,5 +1412,6 @@ var (
13891412
_ interfaces.ProviderCredentialRevoker = (*typedIaCAdapter)(nil)
13901413
_ interfaces.ProviderMigrationRepairer = (*typedIaCAdapter)(nil)
13911414
_ interfaces.ResourceDriver = (*typedResourceDriver)(nil)
1415+
_ interfaces.ResourceAdoptionLocator = (*typedResourceDriver)(nil)
13921416
_ interfaces.Troubleshooter = (*typedResourceDriver)(nil)
13931417
)

cmd/wfctl/iac_typed_adapter_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,34 @@ func TestTypedAdapter_EndToEnd_NameVersionEnumerateAll(t *testing.T) {
244244
}
245245
}
246246

247+
func TestTypedResourceDriver_AdoptionRefDNSDelegationUsesDomain(t *testing.T) {
248+
driver := &typedResourceDriver{resourceType: "infra.dns_delegation"}
249+
locator, ok := any(driver).(interfaces.ResourceAdoptionLocator)
250+
if !ok {
251+
t.Fatal("typedResourceDriver must expose ResourceAdoptionLocator for external IaC drivers")
252+
}
253+
254+
ref, ok, err := locator.AdoptionRef(interfaces.ResourceSpec{
255+
Name: "gocodealone-tech-delegation",
256+
Type: "infra.dns_delegation",
257+
Config: map[string]any{"domain": "gocodealone.tech"},
258+
})
259+
if err != nil {
260+
t.Fatalf("AdoptionRef returned error: %v", err)
261+
}
262+
if !ok {
263+
t.Fatal("AdoptionRef returned ok=false for dns delegation")
264+
}
265+
want := interfaces.ResourceRef{
266+
Name: "gocodealone-tech-delegation",
267+
Type: "infra.dns_delegation",
268+
ProviderID: "gocodealone.tech",
269+
}
270+
if ref != want {
271+
t.Fatalf("AdoptionRef = %+v, want %+v", ref, want)
272+
}
273+
}
274+
247275
// ─── In-process gRPC test fixture ───────────────────────────────────────────
248276

249277
// ─── ADR-0029 capability-extension tests ─────────────────────────────────

platform/differ.go

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"crypto/sha256"
66
"encoding/json"
7+
"errors"
78
"fmt"
89
"os"
910
"sort"
@@ -108,11 +109,13 @@ func ComputePlan(ctx context.Context, p interfaces.IaCProvider, desired []interf
108109
hash = configHash(spec.Config)
109110
}
110111
if rs, exists := currentMap[spec.Name]; !exists {
111-
creates = append(creates, interfaces.PlanAction{
112-
Action: "create",
113-
Resource: spec,
114-
ResolvedConfigHash: hash,
115-
})
112+
create, err := classifyCreate(ctx, p, spec, hash)
113+
if err != nil {
114+
return interfaces.IaCPlan{}, err
115+
}
116+
if create != nil {
117+
creates = append(creates, *create)
118+
}
116119
} else {
117120
candidates = append(candidates, modCandidate{
118121
spec: spec,
@@ -350,6 +353,64 @@ func classifyModification(ctx context.Context, p interfaces.IaCProvider, spec in
350353
return nil
351354
}
352355

356+
// classifyCreate decides whether a desired resource absent from local state
357+
// should produce a create action. Drivers that opt in to ResourceAdoptionLocator
358+
// get one chance to locate/read an external resource and Diff it before create.
359+
// Nil provider, nil driver, or non-adoption drivers preserve legacy create
360+
// behavior.
361+
func classifyCreate(ctx context.Context, p interfaces.IaCProvider, spec interfaces.ResourceSpec, hash string) (*interfaces.PlanAction, error) {
362+
create := &interfaces.PlanAction{
363+
Action: "create",
364+
Resource: spec,
365+
ResolvedConfigHash: hash,
366+
}
367+
if p == nil {
368+
return create, nil
369+
}
370+
driver := resourceDriverForCreate(p, spec.Type)
371+
if driver == nil {
372+
return create, nil
373+
}
374+
locator, ok := driver.(interfaces.ResourceAdoptionLocator)
375+
if !ok {
376+
return create, nil
377+
}
378+
ref, ok, err := locator.AdoptionRef(spec)
379+
if err != nil {
380+
return nil, fmt.Errorf("provider.AdoptionRef(%q/%q): %w", spec.Type, spec.Name, err)
381+
}
382+
if !ok {
383+
return create, nil
384+
}
385+
current, err := driver.Read(ctx, ref)
386+
if err != nil {
387+
if errors.Is(err, interfaces.ErrResourceNotFound) {
388+
return create, nil
389+
}
390+
return nil, fmt.Errorf("provider.Read(%q/%q): %w", spec.Type, spec.Name, err)
391+
}
392+
if current == nil {
393+
return create, nil
394+
}
395+
diff, err := driver.Diff(ctx, spec, current)
396+
if err != nil {
397+
return nil, fmt.Errorf("provider.Diff(%q/%q): %w", spec.Type, spec.Name, err)
398+
}
399+
if diff == nil || (!diff.NeedsUpdate && !diff.NeedsReplace && !hasForceNew(diff.Changes)) {
400+
return nil, nil
401+
}
402+
create.Changes = diff.Changes
403+
return create, nil
404+
}
405+
406+
func resourceDriverForCreate(p interfaces.IaCProvider, resourceType string) interfaces.ResourceDriver {
407+
driver, err := p.ResourceDriver(resourceType)
408+
if err != nil {
409+
return nil
410+
}
411+
return driver
412+
}
413+
353414
// resourceStateToOutput converts the persisted ResourceState into the
354415
// *interfaces.ResourceOutput shape that ResourceDriver.Diff expects.
355416
// Sensitive map is not reconstructed here — drivers that need it should

platform/differ_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,50 @@ func TestDiffer_NewResource(t *testing.T) {
5858
}
5959
}
6060

61+
func TestComputePlan_NewResource_AdoptionNoopDiffSkipsCreate(t *testing.T) {
62+
desired := []interfaces.ResourceSpec{
63+
{Name: "delegation", Type: "infra.dns_delegation", Config: map[string]any{"domain": "example.com"}},
64+
}
65+
driver := &fakeDriver{
66+
diff: &interfaces.DiffResult{NeedsUpdate: false},
67+
adopt: true,
68+
readOutput: &interfaces.ResourceOutput{Name: "delegation", Type: "infra.dns_delegation", ProviderID: "example.com"},
69+
}
70+
provider := &fakeProvider{driver: driver}
71+
72+
plan, err := platform.ComputePlan(context.Background(), provider, desired, nil)
73+
if err != nil {
74+
t.Fatalf("unexpected error: %v", err)
75+
}
76+
77+
if len(plan.Actions) != 0 {
78+
t.Fatalf("expected no actions for driver no-op new resource, got %+v", plan.Actions)
79+
}
80+
if got := driver.diffCallCount.Load(); got != 1 {
81+
t.Fatalf("Diff call count = %d, want 1", got)
82+
}
83+
}
84+
85+
func TestComputePlan_NewResource_NoAdoptionDoesNotCallDiff(t *testing.T) {
86+
desired := []interfaces.ResourceSpec{
87+
{Name: "delegation", Type: "infra.dns_delegation", Config: map[string]any{"domain": "example.com"}},
88+
}
89+
driver := &fakeDriver{diff: &interfaces.DiffResult{NeedsUpdate: true}}
90+
provider := &fakeProvider{driver: driver}
91+
92+
plan, err := platform.ComputePlan(context.Background(), provider, desired, nil)
93+
if err != nil {
94+
t.Fatalf("unexpected error: %v", err)
95+
}
96+
97+
if len(plan.Actions) != 1 || plan.Actions[0].Action != "create" {
98+
t.Fatalf("expected one create action, got %+v", plan.Actions)
99+
}
100+
if got := driver.diffCallCount.Load(); got != 0 {
101+
t.Fatalf("Diff call count = %d, want 0 without adoption opt-in", got)
102+
}
103+
}
104+
61105
func TestDiffer_DeletedResource(t *testing.T) {
62106
desired := []interfaces.ResourceSpec{}
63107
current := []interfaces.ResourceState{

platform/fake_provider_test.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,9 @@ type fakeDriver struct {
9090
diff *interfaces.DiffResult
9191
diffErr error
9292
diffCallCount atomic.Int64
93+
adopt bool
94+
adoptRef interfaces.ResourceRef
95+
readOutput *interfaces.ResourceOutput
9396
}
9497

9598
// Compile-time interface conformance check.
@@ -99,7 +102,7 @@ func (d *fakeDriver) Create(_ context.Context, _ interfaces.ResourceSpec) (*inte
99102
return nil, nil
100103
}
101104
func (d *fakeDriver) Read(_ context.Context, _ interfaces.ResourceRef) (*interfaces.ResourceOutput, error) {
102-
return nil, nil
105+
return d.readOutput, nil
103106
}
104107
func (d *fakeDriver) Update(_ context.Context, _ interfaces.ResourceRef, _ interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) {
105108
return nil, nil
@@ -119,3 +122,17 @@ func (d *fakeDriver) Scale(_ context.Context, _ interfaces.ResourceRef, _ int) (
119122
return nil, nil
120123
}
121124
func (d *fakeDriver) SensitiveKeys() []string { return nil }
125+
126+
func (d *fakeDriver) AdoptionRef(spec interfaces.ResourceSpec) (interfaces.ResourceRef, bool, error) {
127+
if !d.adopt {
128+
return interfaces.ResourceRef{}, false, nil
129+
}
130+
ref := d.adoptRef
131+
if ref.Name == "" {
132+
ref.Name = spec.Name
133+
}
134+
if ref.Type == "" {
135+
ref.Type = spec.Type
136+
}
137+
return ref, true, nil
138+
}

0 commit comments

Comments
 (0)