From c6601b73ce3d16fd85b942357eccd6eb93786b9b Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 16 Jun 2026 20:27:40 -0400 Subject: [PATCH 1/2] fix: clean duplicate workflow DNS markers --- internal/drivers/dns.go | 29 +++++----- internal/drivers/dns_test.go | 106 +++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 15 deletions(-) diff --git a/internal/drivers/dns.go b/internal/drivers/dns.go index efc0c5e..b6bb96e 100644 --- a/internal/drivers/dns.go +++ b/internal/drivers/dns.go @@ -326,15 +326,15 @@ func (d *DNSDriver) applyRecords(ctx context.Context, zoneID, zoneName string, d } } if cleanupManagedMarkers { - for _, record := range current { - if !isWorkflowManagedMarker(record, zoneName) { - continue - } - if _, ok := desiredKeys[recordKey(record, zoneName)]; ok { - continue - } - if err := d.client.DeleteRecord(ctx, zoneID, record.ID); err != nil { - return fmt.Errorf("delete stale workflow managed marker %q in zone %q: %w", record.Name, zoneID, err) + for _, records := range currentByKey { + for _, record := range records { + if !isWorkflowManagedMarker(record, zoneName) { + continue + } + if err := d.client.DeleteRecord(ctx, zoneID, record.ID); err != nil { + return fmt.Errorf("delete stale workflow managed marker %q in zone %q: %w", record.Name, zoneID, err) + } + desiredKeys[recordKey(record, zoneName)] = struct{}{} } } } @@ -605,12 +605,11 @@ func diffRecords(current, desired []Record, manageUnlisted bool, domain string) } } if !manageUnlisted && hasDesiredWorkflowMarker { - for _, record := range current { - if !isWorkflowManagedMarker(record, domain) { - continue - } - if _, ok := desiredKeys[recordKey(record, domain)]; !ok { - changes = append(changes, interfaces.FieldChange{Path: "records", Old: recordOutput(record), New: nil}) + for _, records := range currentByKey { + for _, record := range records { + if isWorkflowManagedMarker(record, domain) { + changes = append(changes, interfaces.FieldChange{Path: "records", Old: recordOutput(record), New: nil}) + } } } } diff --git a/internal/drivers/dns_test.go b/internal/drivers/dns_test.go index 9bedcd6..9f025fd 100644 --- a/internal/drivers/dns_test.go +++ b/internal/drivers/dns_test.go @@ -650,6 +650,66 @@ func TestDNSDriver_DiffDetectsStaleWorkflowManagedMarkersWhenUnlistedRecordsAreP } } +func TestDNSDriver_DiffDetectsDuplicateWorkflowManagedMarkersWithSameKey(t *testing.T) { + driver := NewDNSDriverWithClient(&fakeCFClient{}) + current := &interfaces.ResourceOutput{ + Name: "gigbagg.rocks", + Type: "infra.dns", + ProviderID: "zone", + Outputs: map[string]any{ + "domain": "gigbagg.rocks", + "records": []map[string]any{ + { + "id": "current-marker", + "type": "TXT", + "name": "_workflow-dns-managed.gigbagg.rocks", + "data": `"heritage=wfinfra-v1 managed_by=wfctl state_dir=.state/cloudflare-staging/ resource=cf-gigbagg-rocks"`, + "ttl": 300, + }, + { + "id": "duplicate-marker", + "type": "TXT", + "name": "_workflow-dns-managed.gigbagg.rocks", + "data": `"heritage=wfinfra-v1 managed_by=wfctl state_dir=.state/cloudflare-staging/ resource=cf-gigbagg-rocks"`, + "ttl": 300, + }, + }, + }, + } + diff, err := driver.Diff(context.Background(), interfaces.ResourceSpec{ + Name: "gigbagg.rocks", + Type: "infra.dns", + Config: map[string]any{ + "domain": "gigbagg.rocks", + "manage_unlisted": false, + "records": []any{ + map[string]any{ + "type": "TXT", + "name": "_workflow-dns-managed", + "data": `"heritage=wfinfra-v1 managed_by=wfctl state_dir=.state/cloudflare-staging/ resource=cf-gigbagg-rocks"`, + "ttl": 300, + }, + }, + }, + }, current) + if err != nil { + t.Fatalf("Diff: %v", err) + } + if !diff.NeedsUpdate { + t.Fatalf("diff.NeedsUpdate = false, want true for duplicate workflow marker") + } + if len(diff.Changes) != 1 { + t.Fatalf("changes len = %d, want 1: %#v", len(diff.Changes), diff.Changes) + } + old, ok := diff.Changes[0].Old.(map[string]any) + if !ok { + t.Fatalf("change old = %#v, want record output", diff.Changes[0].Old) + } + if got, want := old["id"], "duplicate-marker"; got != want { + t.Fatalf("deleted marker id = %q, want %q", got, want) + } +} + func TestDNSDriver_UpdatePreservesUnlistedRecordsByDefault(t *testing.T) { fake := &fakeCFClient{ zone: &Zone{ID: "zone", Name: "example.com"}, @@ -891,6 +951,52 @@ func TestDNSDriver_UpdateDeletesStaleWorkflowManagedMarkersWhenUnlistedRecordsAr } } +func TestDNSDriver_UpdateDeletesDuplicateWorkflowManagedMarkersWithSameKey(t *testing.T) { + fake := &fakeCFClient{ + zone: &Zone{ID: "zone", Name: "gigbagg.rocks"}, + records: []Record{ + { + ID: "current-marker", + Type: "TXT", + Name: "_workflow-dns-managed.gigbagg.rocks", + Data: "heritage=wfinfra-v1 managed_by=wfctl state_dir=.state/cloudflare-staging/ resource=cf-gigbagg-rocks", + TTL: 300, + }, + { + ID: "duplicate-marker", + Type: "TXT", + Name: "_workflow-dns-managed.gigbagg.rocks", + Data: "heritage=wfinfra-v1 managed_by=wfctl state_dir=.state/cloudflare-staging/ resource=cf-gigbagg-rocks", + TTL: 300, + }, + {ID: "unmanaged", Type: "TXT", Name: "gigbagg.rocks", Data: "external", TTL: 300}, + }, + } + driver := NewDNSDriverWithClient(fake) + _, err := driver.Update(context.Background(), interfaces.ResourceRef{Name: "gigbagg.rocks", Type: "infra.dns", ProviderID: "zone"}, interfaces.ResourceSpec{ + Name: "gigbagg.rocks", + Type: "infra.dns", + Config: map[string]any{ + "domain": "gigbagg.rocks", + "manage_unlisted": false, + "records": []any{ + map[string]any{ + "type": "TXT", + "name": "_workflow-dns-managed", + "data": `"heritage=wfinfra-v1 managed_by=wfctl state_dir=.state/cloudflare-staging/ resource=cf-gigbagg-rocks"`, + "ttl": 300, + }, + }, + }, + }) + if err != nil { + t.Fatalf("Update: %v", err) + } + if len(fake.deletedRecords) != 1 || fake.deletedRecords[0] != "duplicate-marker" { + t.Fatalf("deletedRecords = %#v, want duplicate-marker only", fake.deletedRecords) + } +} + func TestDNSDriver_MissingRecordsErrorsBeforeMutation(t *testing.T) { fake := &fakeCFClient{zone: &Zone{ID: "zone", Name: "example.com"}} driver := NewDNSDriverWithClient(fake) From 189d3329097a21e7736542df2940480703aafa0d Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 16 Jun 2026 20:41:27 -0400 Subject: [PATCH 2/2] fix: plan marker cleanup with managed records --- internal/drivers/dns.go | 25 ++++++++++----- internal/drivers/dns_test.go | 60 ++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 8 deletions(-) diff --git a/internal/drivers/dns.go b/internal/drivers/dns.go index b6bb96e..8d0164f 100644 --- a/internal/drivers/dns.go +++ b/internal/drivers/dns.go @@ -579,6 +579,7 @@ func diffRecords(current, desired []Record, manageUnlisted bool, domain string) var changes []interfaces.FieldChange currentByKey := recordsByKey(current, domain) desiredKeys := map[string]struct{}{} + deletedCurrent := map[string]struct{}{} hasDesiredWorkflowMarker := false for _, record := range desired { key := recordKey(record, domain) @@ -597,22 +598,26 @@ func diffRecords(current, desired []Record, manageUnlisted bool, domain string) changes = append(changes, interfaces.FieldChange{Path: "records", Old: recordOutput(current), New: recordOutput(record)}) } } - if manageUnlisted { - for _, record := range current { - if _, ok := desiredKeys[recordKey(record, domain)]; !ok { - changes = append(changes, interfaces.FieldChange{Path: "records", Old: recordOutput(record), New: nil}) - } - } - } - if !manageUnlisted && hasDesiredWorkflowMarker { + if hasDesiredWorkflowMarker { for _, records := range currentByKey { for _, record := range records { if isWorkflowManagedMarker(record, domain) { changes = append(changes, interfaces.FieldChange{Path: "records", Old: recordOutput(record), New: nil}) + deletedCurrent[recordIdentity(record, domain)] = struct{}{} } } } } + if manageUnlisted { + for _, record := range current { + if _, ok := deletedCurrent[recordIdentity(record, domain)]; ok { + continue + } + if _, ok := desiredKeys[recordKey(record, domain)]; !ok { + changes = append(changes, interfaces.FieldChange{Path: "records", Old: recordOutput(record), New: nil}) + } + } + } return changes } @@ -660,6 +665,10 @@ func recordKey(record Record, domain string) string { return strings.Join(parts, "\x00") } +func recordIdentity(record Record, domain string) string { + return strings.Join([]string{record.ID, recordKey(record, domain)}, "\x00") +} + func canonicalName(name, domain string) string { return strings.ToLower(normalizeRecordName(strings.TrimSpace(name), strings.TrimSpace(domain))) } diff --git a/internal/drivers/dns_test.go b/internal/drivers/dns_test.go index 9f025fd..514cbb0 100644 --- a/internal/drivers/dns_test.go +++ b/internal/drivers/dns_test.go @@ -710,6 +710,66 @@ func TestDNSDriver_DiffDetectsDuplicateWorkflowManagedMarkersWithSameKey(t *test } } +func TestDNSDriver_DiffDetectsDuplicateWorkflowManagedMarkersWhenManagingUnlistedRecords(t *testing.T) { + driver := NewDNSDriverWithClient(&fakeCFClient{}) + current := &interfaces.ResourceOutput{ + Name: "gigbagg.rocks", + Type: "infra.dns", + ProviderID: "zone", + Outputs: map[string]any{ + "domain": "gigbagg.rocks", + "records": []map[string]any{ + { + "id": "current-marker", + "type": "TXT", + "name": "_workflow-dns-managed.gigbagg.rocks", + "data": `"heritage=wfinfra-v1 managed_by=wfctl state_dir=.state/cloudflare-staging/ resource=cf-gigbagg-rocks"`, + "ttl": 300, + }, + { + "id": "duplicate-marker", + "type": "TXT", + "name": "_workflow-dns-managed.gigbagg.rocks", + "data": `"heritage=wfinfra-v1 managed_by=wfctl state_dir=.state/cloudflare-staging/ resource=cf-gigbagg-rocks"`, + "ttl": 300, + }, + }, + }, + } + diff, err := driver.Diff(context.Background(), interfaces.ResourceSpec{ + Name: "gigbagg.rocks", + Type: "infra.dns", + Config: map[string]any{ + "domain": "gigbagg.rocks", + "manage_unlisted": true, + "records": []any{ + map[string]any{ + "type": "TXT", + "name": "_workflow-dns-managed", + "data": `"heritage=wfinfra-v1 managed_by=wfctl state_dir=.state/cloudflare-staging/ resource=cf-gigbagg-rocks"`, + "ttl": 300, + }, + }, + }, + }, current) + if err != nil { + t.Fatalf("Diff: %v", err) + } + if !diff.NeedsUpdate { + t.Fatalf("diff.NeedsUpdate = false, want true for duplicate workflow marker") + } + if len(diff.Changes) != 1 { + t.Fatalf("changes len = %d, want 1: %#v", len(diff.Changes), diff.Changes) + } + old, ok := diff.Changes[0].Old.(map[string]any) + if !ok { + t.Fatalf("change old = %#v, want record output", diff.Changes[0].Old) + } + if got, want := old["id"], "duplicate-marker"; got != want { + t.Fatalf("deleted marker id = %q, want %q", got, want) + } +} + func TestDNSDriver_UpdatePreservesUnlistedRecordsByDefault(t *testing.T) { fake := &fakeCFClient{ zone: &Zone{ID: "zone", Name: "example.com"},