From fb11ed138d0a427d004b13bc7f52e58a59993a70 Mon Sep 17 00:00:00 2001 From: Evan Vetere Date: Fri, 2 Oct 2026 22:01:33 -0400 Subject: [PATCH] fix: Keep a name another record set owns on delete When two record sets of one type list the same name, the later writer replaces the RRset and leaves its own ownership comment. Deleting the earlier record set still removed every name in its spec, so the later record set's name stopped resolving while it kept reporting Programmed. Skip a spec-listed name whose ownership comment names another record set. A name with no ownership comment is still deleted, as before. Fixes #188 --- internal/dns/pdns/client.go | 11 +++++++-- internal/dns/pdns/pdns_test.go | 41 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/internal/dns/pdns/client.go b/internal/dns/pdns/client.go index 1c48cbd..023ecdd 100644 --- a/internal/dns/pdns/client.go +++ b/internal/dns/pdns/client.go @@ -596,9 +596,16 @@ func (c *Client) DeleteRecordSet(ctx context.Context, zone dnsv1alpha1.DNSZone, targets := make(map[string]struct{}, len(recordSet.Spec.Records)) for i := range recordSet.Spec.Records { qualified := ownername.Qualify(recordSet.Spec.Records[i].Name, zoneName) - if _, found := existing[rrsetKey{name: qualified, typ: recordType}]; found { - targets[qualified] = struct{}{} + current, found := existing[rrsetKey{name: qualified, typ: recordType}] + if !found { + continue + } + // Another record set rewrote this name after this one did, and still + // asks for it, so the name is no longer this record set's to delete. + if owner := rrsetOwnerRef(current); owner != "" && owner != ownerRef { + continue } + targets[qualified] = struct{}{} } // Owner names this record set wrote that its spec no longer mentions. diff --git a/internal/dns/pdns/pdns_test.go b/internal/dns/pdns/pdns_test.go index 57397dd..2d31dba 100644 --- a/internal/dns/pdns/pdns_test.go +++ b/internal/dns/pdns/pdns_test.go @@ -1330,6 +1330,47 @@ func TestDeleteRecordSet_DeletesSpecAndOwnedNamesInOnePatch(t *testing.T) { } } +// Two record sets of one type can list the same name. The later writer's +// ownership comment says the name is now its own, so deleting the earlier +// record set must leave that name in place (#188). +func TestDeleteRecordSet_KeepsNameAnotherRecordSetRewrote(t *testing.T) { + t.Parallel() + + stub, c := newPDNSStub(t, zoneResponse{ + Name: exampleCom, + RRSets: []zoneRRset{ + { + Name: "www.example.com.", + Type: "A", + Records: []zoneRRsetRecord{{Content: "5.6.7.8"}}, + Comments: []zoneRRsetComment{{Account: ACCOUNT_OWNER, Content: "default:rs2"}}, + }, + { + Name: "mine.example.com.", + Type: "A", + Records: []zoneRRsetRecord{{Content: "1.2.3.4"}}, + Comments: []zoneRRsetComment{{Account: ACCOUNT_OWNER, Content: "default:rs"}}, + }, + }, + }) + + if err := c.DeleteRecordSet(context.Background(), testZone, aRecordSet(1, "www", "mine")); err != nil { + t.Fatalf("DeleteRecordSet error: %v", err) + } + + var names []string + for _, patch := range stub.patches { + for _, rr := range patch.RRSets { + if rr.ChangeType == changeTypeDelete { + names = append(names, rr.Name) + } + } + } + if !reflect.DeepEqual(names, []string{"mine.example.com."}) { + t.Fatalf("deleted %v, want only the name this record set still owns", names) + } +} + // A patch that replaces and deletes one RRset says two things at once. PowerDNS // would refuse it for the duplicate REPLACE the clear adds, rejecting every // other change travelling with it, so it is refused here with the RRset named.