From 6e6969775758e4aa0808176445eb76563f03f2f6 Mon Sep 17 00:00:00 2001 From: Tobias Harnickell Date: Thu, 18 Sep 2025 14:31:14 +0200 Subject: [PATCH 1/4] fix(provider/gcp): bundle record-type swap changes The change teaches the Google provider to bundle record-type swaps into a single Cloud DNS change set. It also carries the ownership forward. * Add new `extractTypeSwaps` helper to pair create/delete endpoints by name * Modify `extractTypeSwaps` to skip TXT records * Modify `extractTypeSwaps` to copy owner label of the prior RRset onto the new record before both ends of the swap are appended to the change * Modify `ApplyChanges` to receive the new `extractTypeSwaps` helper's result for one astomic delete+add per type change * Add `TestGoogleApplyChangesTypeSwap` to set up A to CNAME transition * Add tests * Assert helper finds a swap * Verify that zone holds the CNAME * Verify that create endpoint retains original owner label Signed-off-by: Tobias Harnickell --- provider/google/google.go | 106 ++++++++++++++++++++++++++++++++- provider/google/google_test.go | 72 ++++++++++++++++++++++ 2 files changed, 175 insertions(+), 3 deletions(-) diff --git a/provider/google/google.go b/provider/google/google.go index fd521fc0ef..54a324f1d9 100644 --- a/provider/google/google.go +++ b/provider/google/google.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "sort" + "strings" "time" "cloud.google.com/go/compute/metadata" @@ -230,20 +231,119 @@ func (p *GoogleProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, err return endpoints, nil } +type typeSwap struct { + old *endpoint.Endpoint + new *endpoint.Endpoint +} + // ApplyChanges applies a given set of changes in a given zone. func (p *GoogleProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) error { + swaps, creates, deletes := p.extractTypeSwaps(changes.Create, changes.Delete) + change := &dns.Change{} - change.Additions = append(change.Additions, p.newFilteredRecords(changes.Create)...) + for _, swap := range swaps { + change.Deletions = append(change.Deletions, p.newFilteredRecords([]*endpoint.Endpoint{swap.old})...) + change.Additions = append(change.Additions, p.newFilteredRecords([]*endpoint.Endpoint{swap.new})...) + } + + change.Additions = append(change.Additions, p.newFilteredRecords(creates)...) change.Additions = append(change.Additions, p.newFilteredRecords(changes.UpdateNew)...) - change.Deletions = append(change.Deletions, p.newFilteredRecords(changes.UpdateOld)...) - change.Deletions = append(change.Deletions, p.newFilteredRecords(changes.Delete)...) + change.Deletions = append(change.Deletions, p.newFilteredRecords(changes.UpdateOld)...) + change.Deletions = append(change.Deletions, p.newFilteredRecords(deletes)...) return p.submitChange(ctx, change) } +func (p *GoogleProvider) extractTypeSwaps(create, deleteEndpoints []*endpoint.Endpoint) ([]typeSwap, []*endpoint.Endpoint, []*endpoint.Endpoint) { + swaps := make([]typeSwap, 0) + remainingCreates := make([]*endpoint.Endpoint, 0, len(create)) + remainingDeletes := make([]*endpoint.Endpoint, 0) + + deletesByName := map[string][]*endpoint.Endpoint{} + + for _, del := range deleteEndpoints { + if !eligibleForTypeSwap(del) { + remainingDeletes = append(remainingDeletes, del) + continue + } + key := swapKey(del.DNSName) + deletesByName[key] = append(deletesByName[key], del) + } + + for _, createEP := range create { + if !eligibleForTypeSwap(createEP) { + remainingCreates = append(remainingCreates, createEP) + continue + } + + key := swapKey(createEP.DNSName) + dels, ok := deletesByName[key] + if !ok { + remainingCreates = append(remainingCreates, createEP) + continue + } + + matchIdx := -1 + for i, del := range dels { + if del.RecordType != createEP.RecordType { + matchIdx = i + break + } + } + + if matchIdx == -1 { + remainingCreates = append(remainingCreates, createEP) + continue + } + + matched := dels[matchIdx] + remaining := append([]*endpoint.Endpoint{}, dels[:matchIdx]...) + remaining = append(remaining, dels[matchIdx+1:]...) + if len(remaining) > 0 { + deletesByName[key] = remaining + } else { + delete(deletesByName, key) + } + + if matched.Labels != nil { + if createEP.Labels == nil { + createEP.Labels = map[string]string{} + } + if owner, ok := matched.Labels[endpoint.OwnerLabelKey]; ok { + if _, exists := createEP.Labels[endpoint.OwnerLabelKey]; !exists { + createEP.Labels[endpoint.OwnerLabelKey] = owner + } + } + } + + swaps = append(swaps, typeSwap{ + old: matched, + new: createEP, + }) + } + + for _, leftovers := range deletesByName { + remainingDeletes = append(remainingDeletes, leftovers...) + } + + return swaps, remainingCreates, remainingDeletes +} + +func eligibleForTypeSwap(ep *endpoint.Endpoint) bool { + if ep == nil { + return false + } + + return ep.RecordType != endpoint.RecordTypeTXT +} + +func swapKey(name string) string { + return strings.ToLower(provider.EnsureTrailingDot(name)) +} + // SupportedRecordType returns true if the record type is supported by the provider func (p *GoogleProvider) SupportedRecordType(recordType string) bool { switch recordType { diff --git a/provider/google/google_test.go b/provider/google/google_test.go index 2596325446..ae92064382 100644 --- a/provider/google/google_test.go +++ b/provider/google/google_test.go @@ -403,6 +403,78 @@ func TestGoogleApplyChanges(t *testing.T) { }) } +func TestGoogleApplyChangesTypeSwap(t *testing.T) { + provider := newGoogleProvider( + t, + endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), + provider.NewZoneIDFilter([]string{""}), + false, + []*endpoint.Endpoint{}, + nil, + nil, + ) + + existing := endpoint.NewEndpointWithTTL( + "typeswap.zone-1.ext-dns-test-2.gcp.zalan.do", + endpoint.RecordTypeA, + endpoint.TTL(180), + "203.0.113.10", + ) + + setupGoogleRecords(t, provider, []*endpoint.Endpoint{existing}) + + records, err := provider.Records(context.Background()) + require.NoError(t, err) + require.Len(t, records, 1) + + deleteRecord := endpoint.NewEndpointWithTTL( + existing.DNSName, + existing.RecordType, + existing.RecordTTL, + records[0].Targets..., + ) + deleteRecord.Labels = map[string]string{endpoint.OwnerLabelKey: "owner-1"} + + createRecord := endpoint.NewEndpoint( + existing.DNSName, + endpoint.RecordTypeCNAME, + "target.typeswap.example.", + ) + createRecord.Labels = nil + require.True(t, provider.domainFilter.Match(createRecord.DNSName)) + + changes := &plan.Changes{ + Create: []*endpoint.Endpoint{createRecord}, + Delete: []*endpoint.Endpoint{deleteRecord}, + } + + swaps, remainingCreates, remainingDeletes := provider.extractTypeSwaps(changes.Create, changes.Delete) + require.Len(t, swaps, 1) + require.Empty(t, remainingCreates) + require.Empty(t, remainingDeletes) + deletionRecords := provider.newFilteredRecords([]*endpoint.Endpoint{swaps[0].old}) + additionRecords := provider.newFilteredRecords([]*endpoint.Endpoint{swaps[0].new}) + require.Len(t, deletionRecords, 1) + require.Len(t, additionRecords, 1) + + require.NoError(t, provider.ApplyChanges(context.Background(), changes)) + + updatedRecords, err := provider.Records(context.Background()) + require.NoError(t, err) + + expectedRecord := endpoint.NewEndpointWithTTL( + existing.DNSName, + endpoint.RecordTypeCNAME, + endpoint.TTL(defaultTTL), + "target.typeswap.example.", + ) + + validateEndpoints(t, updatedRecords, []*endpoint.Endpoint{expectedRecord}) + + require.NotNil(t, createRecord.Labels) + assert.Equal(t, "owner-1", createRecord.Labels[endpoint.OwnerLabelKey]) +} + func TestGoogleApplyChangesDryRun(t *testing.T) { originalEndpoints := []*endpoint.Endpoint{ endpoint.NewEndpointWithTTL("update-test.zone-1.ext-dns-test-2.gcp.zalan.do", endpoint.RecordTypeA, defaultTTL, "8.8.8.8"), From 3887b0ee3b8d8f69097885baddce5a2fddcd652b Mon Sep 17 00:00:00 2001 From: Tobias Harnickell Date: Fri, 19 Sep 2025 08:59:07 +0200 Subject: [PATCH 2/4] feat(providers/google): Preserve swap owner label * Maintain TXT ownership by skipping TXT in type swaps * Carry owner label forward when swapping record types * Add tests for swap logic * Refactor swap extraction for clarity Signed-off-by: Tobias Harnickell --- provider/google/google.go | 61 +++++++++++++++++----------- provider/google/google_test.go | 74 ++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 24 deletions(-) diff --git a/provider/google/google.go b/provider/google/google.go index 54a324f1d9..a6b68b4805 100644 --- a/provider/google/google.go +++ b/provider/google/google.go @@ -262,15 +262,21 @@ func (p *GoogleProvider) extractTypeSwaps(create, deleteEndpoints []*endpoint.En remainingCreates := make([]*endpoint.Endpoint, 0, len(create)) remainingDeletes := make([]*endpoint.Endpoint, 0) - deletesByName := map[string][]*endpoint.Endpoint{} + // Index deletions by name then by record type + typeBucketByName := make(map[string]map[string][]*endpoint.Endpoint) for _, del := range deleteEndpoints { if !eligibleForTypeSwap(del) { remainingDeletes = append(remainingDeletes, del) continue } - key := swapKey(del.DNSName) - deletesByName[key] = append(deletesByName[key], del) + nameKey := swapKey(del.DNSName) + buckets := typeBucketByName[nameKey] + if buckets == nil { + buckets = make(map[string][]*endpoint.Endpoint) + typeBucketByName[nameKey] = buckets + } + buckets[del.RecordType] = append(buckets[del.RecordType], del) } for _, createEP := range create { @@ -279,35 +285,41 @@ func (p *GoogleProvider) extractTypeSwaps(create, deleteEndpoints []*endpoint.En continue } - key := swapKey(createEP.DNSName) - dels, ok := deletesByName[key] + nameKey := swapKey(createEP.DNSName) + buckets, ok := typeBucketByName[nameKey] if !ok { remainingCreates = append(remainingCreates, createEP) continue } - matchIdx := -1 - for i, del := range dels { - if del.RecordType != createEP.RecordType { - matchIdx = i - break + // Find any delete with a different type to form a swap. + var matched *endpoint.Endpoint + var matchedType string + for delType, list := range buckets { + if delType == createEP.RecordType || len(list) == 0 { + continue } + matched = list[0] + matchedType = delType + + if len(list) == 1 { + delete(buckets, delType) + } else { + buckets[delType] = list[1:] + } + break } - if matchIdx == -1 { + if matched == nil { remainingCreates = append(remainingCreates, createEP) continue } - matched := dels[matchIdx] - remaining := append([]*endpoint.Endpoint{}, dels[:matchIdx]...) - remaining = append(remaining, dels[matchIdx+1:]...) - if len(remaining) > 0 { - deletesByName[key] = remaining - } else { - delete(deletesByName, key) + if len(buckets) == 0 { + delete(typeBucketByName, nameKey) } + // Ensure create endpoint has owner label if matched.Labels != nil { if createEP.Labels == nil { createEP.Labels = map[string]string{} @@ -319,19 +331,20 @@ func (p *GoogleProvider) extractTypeSwaps(create, deleteEndpoints []*endpoint.En } } - swaps = append(swaps, typeSwap{ - old: matched, - new: createEP, - }) + swaps = append(swaps, typeSwap{old: matched, new: createEP}) + _ = matchedType // silence unused if future logging added } - for _, leftovers := range deletesByName { - remainingDeletes = append(remainingDeletes, leftovers...) + for _, byType := range typeBucketByName { + for _, list := range byType { + remainingDeletes = append(remainingDeletes, list...) + } } return swaps, remainingCreates, remainingDeletes } +// Returns true only for non-TXT records func eligibleForTypeSwap(ep *endpoint.Endpoint) bool { if ep == nil { return false diff --git a/provider/google/google_test.go b/provider/google/google_test.go index ae92064382..ad929ceab3 100644 --- a/provider/google/google_test.go +++ b/provider/google/google_test.go @@ -705,6 +705,80 @@ func TestSoftErrListRecordsConflict(t *testing.T) { require.Empty(t, records) } +func TestGoogleExtractTypeSwapsIgnoreTXT(t *testing.T) { + gp := newGoogleProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), provider.NewZoneIDFilter([]string{""}), false, []*endpoint.Endpoint{}, nil, nil) + + name := "txt-swap-test.zone-1.ext-dns-test-2.gcp.zalan.do" + + // Create A, delete TXT -> no swap + createA := endpoint.NewEndpoint(name, endpoint.RecordTypeA, "1.2.3.4") + deleteTXT := endpoint.NewEndpoint(name, endpoint.RecordTypeTXT, "txt-value") + swaps, remCreates, remDeletes := gp.extractTypeSwaps([]*endpoint.Endpoint{createA}, []*endpoint.Endpoint{deleteTXT}) + require.Len(t, swaps, 0) + require.Len(t, remCreates, 1) + require.Len(t, remDeletes, 1) + + // Create TXT, delete A -> no swap + createTXT := endpoint.NewEndpoint(name, endpoint.RecordTypeTXT, "txt-value") + deleteA := endpoint.NewEndpoint(name, endpoint.RecordTypeA, "1.2.3.4") + swaps, remCreates, remDeletes = gp.extractTypeSwaps([]*endpoint.Endpoint{createTXT}, []*endpoint.Endpoint{deleteA}) + require.Len(t, swaps, 0) + require.Len(t, remCreates, 1) + require.Len(t, remDeletes, 1) +} + +func TestGoogleExtractTypeSwapsConsumesSingleDelete(t *testing.T) { + gp := newGoogleProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), provider.NewZoneIDFilter([]string{""}), false, []*endpoint.Endpoint{}, nil, nil) + + name := "multi-delete-swap.zone-1.ext-dns-test-2.gcp.zalan.do" + + delA := endpoint.NewEndpoint(name, endpoint.RecordTypeA, "192.0.2.1") + delA.Labels = map[string]string{endpoint.OwnerLabelKey: "owner-a"} + delAAAA := endpoint.NewEndpoint(name, endpoint.RecordTypeAAAA, "2001:db8::1") + + createCNAME := endpoint.NewEndpoint(name, endpoint.RecordTypeCNAME, "target.example.") + + swaps, remCreates, remDeletes := gp.extractTypeSwaps([]*endpoint.Endpoint{createCNAME}, []*endpoint.Endpoint{delA, delAAAA}) + + require.Len(t, swaps, 1) + require.Len(t, remCreates, 0) + require.Len(t, remDeletes, 1) + assert.Equal(t, delAAAA, remDeletes[0]) + assert.NotNil(t, swaps[0].new.Labels) + assert.Equal(t, "owner-a", swaps[0].new.Labels[endpoint.OwnerLabelKey]) +} + +func TestGoogleExtractTypeSwapsCaseInsensitiveName(t *testing.T) { + gp := newGoogleProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), provider.NewZoneIDFilter([]string{""}), false, []*endpoint.Endpoint{}, nil, nil) + + // Mixed case create vs lower-case delete + createName := "CaseSwap.ZONE-1.ext-dns-test-2.gcp.zalan.do" + deleteName := strings.ToLower(createName) + + delA := endpoint.NewEndpoint(deleteName, endpoint.RecordTypeA, "198.51.100.10") + createCNAME := endpoint.NewEndpoint(createName, endpoint.RecordTypeCNAME, "target.caseswap.example.") + + swaps, remCreates, remDeletes := gp.extractTypeSwaps([]*endpoint.Endpoint{createCNAME}, []*endpoint.Endpoint{delA}) + require.Len(t, swaps, 1) + require.Len(t, remCreates, 0) + require.Len(t, remDeletes, 0) + assert.Equal(t, delA, swaps[0].old) + assert.Equal(t, createCNAME, swaps[0].new) +} + +func TestGoogleExtractTypeSwapsSameTypeNoSwap(t *testing.T) { + gp := newGoogleProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), provider.NewZoneIDFilter([]string{""}), false, []*endpoint.Endpoint{}, nil, nil) + + name := "same-type.zone-1.ext-dns-test-2.gcp.zalan.do" + delA := endpoint.NewEndpoint(name, endpoint.RecordTypeA, "203.0.113.55") + createA := endpoint.NewEndpoint(name, endpoint.RecordTypeA, "203.0.113.56") + + swaps, remCreates, remDeletes := gp.extractTypeSwaps([]*endpoint.Endpoint{createA}, []*endpoint.Endpoint{delA}) + require.Len(t, swaps, 0) + require.Len(t, remCreates, 1) + require.Len(t, remDeletes, 1) +} + func sortChangesByName(cs *dns.Change) { sort.SliceStable(cs.Additions, func(i, j int) bool { return cs.Additions[i].Name < cs.Additions[j].Name From 0756708d4a42878a29f2408ff3e0917bb4c1aa5c Mon Sep 17 00:00:00 2001 From: Tobias Harnickell Date: Sun, 21 Sep 2025 12:04:59 +0200 Subject: [PATCH 3/4] feat(providers/google): Cleanup Google Provider * Fix `TestGoogleExtractTypeSwapsConsumesSingleDelete` flakiness by asserting A-record as the chosen swap source * Remove unused `matchedType` variable * Initialize `remainingDeletes` with `make([]*endpoint.Endpoint, 0, len(deleteEndpoints))` so "no swaps" mirrors the preallocation used for `remainingCreates` * Document invariants and rationale for `extractTypeSwaps` in code comment * Evaluate buckets deterministically by `typeSwapTypePriority`, owner-label preference and stable tie-preakters (leftover deletions also deterministically emitted) Signed-off-by: Tobias Harnickell --- provider/google/google.go | 84 ++++++++++++++++++++++++++++------ provider/google/google_test.go | 13 +++--- 2 files changed, 77 insertions(+), 20 deletions(-) diff --git a/provider/google/google.go b/provider/google/google.go index a6b68b4805..6080446532 100644 --- a/provider/google/google.go +++ b/provider/google/google.go @@ -236,6 +236,12 @@ type typeSwap struct { new *endpoint.Endpoint } +var typeSwapTypePriority = map[string]int{ + endpoint.RecordTypeA: 0, + endpoint.RecordTypeAAAA: 1, + endpoint.RecordTypeCNAME: 2, +} + // ApplyChanges applies a given set of changes in a given zone. func (p *GoogleProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) error { swaps, creates, deletes := p.extractTypeSwaps(changes.Create, changes.Delete) @@ -257,10 +263,13 @@ func (p *GoogleProvider) ApplyChanges(ctx context.Context, changes *plan.Changes return p.submitChange(ctx, change) } +// extractTypeSwaps pairs create/delete records of differing types so we can swap +// them while preserving ownership labels, skipping TXT records and preferring +// higher-priority, labeled deletions. func (p *GoogleProvider) extractTypeSwaps(create, deleteEndpoints []*endpoint.Endpoint) ([]typeSwap, []*endpoint.Endpoint, []*endpoint.Endpoint) { swaps := make([]typeSwap, 0) remainingCreates := make([]*endpoint.Endpoint, 0, len(create)) - remainingDeletes := make([]*endpoint.Endpoint, 0) + remainingDeletes := make([]*endpoint.Endpoint, 0, len(deleteEndpoints)) // Index deletions by name then by record type typeBucketByName := make(map[string]map[string][]*endpoint.Endpoint) @@ -293,21 +302,40 @@ func (p *GoogleProvider) extractTypeSwaps(create, deleteEndpoints []*endpoint.En } // Find any delete with a different type to form a swap. - var matched *endpoint.Endpoint - var matchedType string + var ( + matched *endpoint.Endpoint + matchedType string + matchedIndex int + bestTypeRank int + bestOwnerRank int + ) + + bestTypeRank = len(typeSwapTypePriority) + 1 + bestOwnerRank = 1 + for delType, list := range buckets { if delType == createEP.RecordType || len(list) == 0 { continue } - matched = list[0] - matchedType = delType - if len(list) == 1 { - delete(buckets, delType) - } else { - buckets[delType] = list[1:] + typeRank := swapTypePriority(delType) + + for idx, candidate := range list { + ownerRank := 1 + if candidate.Labels != nil { + if _, ok := candidate.Labels[endpoint.OwnerLabelKey]; ok { + ownerRank = 0 + } + } + + if matched == nil || typeRank < bestTypeRank || (typeRank == bestTypeRank && ownerRank < bestOwnerRank) || (typeRank == bestTypeRank && ownerRank == bestOwnerRank && idx < matchedIndex) || (typeRank == bestTypeRank && ownerRank == bestOwnerRank && idx == matchedIndex && delType < matchedType) { + matched = candidate + matchedType = delType + matchedIndex = idx + bestTypeRank = typeRank + bestOwnerRank = ownerRank + } } - break } if matched == nil { @@ -315,6 +343,15 @@ func (p *GoogleProvider) extractTypeSwaps(create, deleteEndpoints []*endpoint.En continue } + // Consume the matched deletion from the bucket. + bucket := buckets[matchedType] + switch len(bucket) { + case 1: + delete(buckets, matchedType) + default: + buckets[matchedType] = append(bucket[:matchedIndex], bucket[matchedIndex+1:]...) + } + if len(buckets) == 0 { delete(typeBucketByName, nameKey) } @@ -332,12 +369,23 @@ func (p *GoogleProvider) extractTypeSwaps(create, deleteEndpoints []*endpoint.En } swaps = append(swaps, typeSwap{old: matched, new: createEP}) - _ = matchedType // silence unused if future logging added } - for _, byType := range typeBucketByName { - for _, list := range byType { - remainingDeletes = append(remainingDeletes, list...) + nameKeys := make([]string, 0, len(typeBucketByName)) + for name := range typeBucketByName { + nameKeys = append(nameKeys, name) + } + sort.Strings(nameKeys) + + for _, name := range nameKeys { + byType := typeBucketByName[name] + typeKeys := make([]string, 0, len(byType)) + for recordType := range byType { + typeKeys = append(typeKeys, recordType) + } + sort.Strings(typeKeys) + for _, recordType := range typeKeys { + remainingDeletes = append(remainingDeletes, byType[recordType]...) } } @@ -357,6 +405,14 @@ func swapKey(name string) string { return strings.ToLower(provider.EnsureTrailingDot(name)) } +func swapTypePriority(recordType string) int { + if priority, ok := typeSwapTypePriority[recordType]; ok { + return priority + } + + return len(typeSwapTypePriority) + 1 +} + // SupportedRecordType returns true if the record type is supported by the provider func (p *GoogleProvider) SupportedRecordType(recordType string) bool { switch recordType { diff --git a/provider/google/google_test.go b/provider/google/google_test.go index ad929ceab3..79fc8b7b23 100644 --- a/provider/google/google_test.go +++ b/provider/google/google_test.go @@ -714,7 +714,7 @@ func TestGoogleExtractTypeSwapsIgnoreTXT(t *testing.T) { createA := endpoint.NewEndpoint(name, endpoint.RecordTypeA, "1.2.3.4") deleteTXT := endpoint.NewEndpoint(name, endpoint.RecordTypeTXT, "txt-value") swaps, remCreates, remDeletes := gp.extractTypeSwaps([]*endpoint.Endpoint{createA}, []*endpoint.Endpoint{deleteTXT}) - require.Len(t, swaps, 0) + require.Empty(t, swaps) require.Len(t, remCreates, 1) require.Len(t, remDeletes, 1) @@ -722,7 +722,7 @@ func TestGoogleExtractTypeSwapsIgnoreTXT(t *testing.T) { createTXT := endpoint.NewEndpoint(name, endpoint.RecordTypeTXT, "txt-value") deleteA := endpoint.NewEndpoint(name, endpoint.RecordTypeA, "1.2.3.4") swaps, remCreates, remDeletes = gp.extractTypeSwaps([]*endpoint.Endpoint{createTXT}, []*endpoint.Endpoint{deleteA}) - require.Len(t, swaps, 0) + require.Empty(t, swaps) require.Len(t, remCreates, 1) require.Len(t, remDeletes, 1) } @@ -741,7 +741,8 @@ func TestGoogleExtractTypeSwapsConsumesSingleDelete(t *testing.T) { swaps, remCreates, remDeletes := gp.extractTypeSwaps([]*endpoint.Endpoint{createCNAME}, []*endpoint.Endpoint{delA, delAAAA}) require.Len(t, swaps, 1) - require.Len(t, remCreates, 0) + assert.Equal(t, delA, swaps[0].old) + require.Empty(t, remCreates) require.Len(t, remDeletes, 1) assert.Equal(t, delAAAA, remDeletes[0]) assert.NotNil(t, swaps[0].new.Labels) @@ -760,8 +761,8 @@ func TestGoogleExtractTypeSwapsCaseInsensitiveName(t *testing.T) { swaps, remCreates, remDeletes := gp.extractTypeSwaps([]*endpoint.Endpoint{createCNAME}, []*endpoint.Endpoint{delA}) require.Len(t, swaps, 1) - require.Len(t, remCreates, 0) - require.Len(t, remDeletes, 0) + require.Empty(t, remCreates) + require.Empty(t, remDeletes) assert.Equal(t, delA, swaps[0].old) assert.Equal(t, createCNAME, swaps[0].new) } @@ -774,7 +775,7 @@ func TestGoogleExtractTypeSwapsSameTypeNoSwap(t *testing.T) { createA := endpoint.NewEndpoint(name, endpoint.RecordTypeA, "203.0.113.56") swaps, remCreates, remDeletes := gp.extractTypeSwaps([]*endpoint.Endpoint{createA}, []*endpoint.Endpoint{delA}) - require.Len(t, swaps, 0) + require.Empty(t, swaps) require.Len(t, remCreates, 1) require.Len(t, remDeletes, 1) } From a7d3b07b494014188171156e9cd309b7d1dd7388 Mon Sep 17 00:00:00 2001 From: Tobias Harnickell Date: Sun, 21 Sep 2025 12:38:54 +0200 Subject: [PATCH 4/4] chore(provider/google): Improve readability Split unnecessarily long lines. Signed-off-by: Tobias Harnickell --- provider/google/google.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/provider/google/google.go b/provider/google/google.go index 6080446532..190a0a3d5b 100644 --- a/provider/google/google.go +++ b/provider/google/google.go @@ -266,7 +266,9 @@ func (p *GoogleProvider) ApplyChanges(ctx context.Context, changes *plan.Changes // extractTypeSwaps pairs create/delete records of differing types so we can swap // them while preserving ownership labels, skipping TXT records and preferring // higher-priority, labeled deletions. -func (p *GoogleProvider) extractTypeSwaps(create, deleteEndpoints []*endpoint.Endpoint) ([]typeSwap, []*endpoint.Endpoint, []*endpoint.Endpoint) { +func (p *GoogleProvider) extractTypeSwaps( + create, deleteEndpoints []*endpoint.Endpoint, +) ([]typeSwap, []*endpoint.Endpoint, []*endpoint.Endpoint) { swaps := make([]typeSwap, 0) remainingCreates := make([]*endpoint.Endpoint, 0, len(create)) remainingDeletes := make([]*endpoint.Endpoint, 0, len(deleteEndpoints)) @@ -328,7 +330,21 @@ func (p *GoogleProvider) extractTypeSwaps(create, deleteEndpoints []*endpoint.En } } - if matched == nil || typeRank < bestTypeRank || (typeRank == bestTypeRank && ownerRank < bestOwnerRank) || (typeRank == bestTypeRank && ownerRank == bestOwnerRank && idx < matchedIndex) || (typeRank == bestTypeRank && ownerRank == bestOwnerRank && idx == matchedIndex && delType < matchedType) { + better := matched == nil + if !better && typeRank != bestTypeRank { + better = typeRank < bestTypeRank + } + if !better && ownerRank != bestOwnerRank { + better = ownerRank < bestOwnerRank + } + if !better && idx != matchedIndex { + better = idx < matchedIndex + } + if !better && delType != matchedType { + better = delType < matchedType + } + + if better { matched = candidate matchedType = delType matchedIndex = idx