From 7cd99da01392d2890f4dea34e62048585502366b Mon Sep 17 00:00:00 2001 From: houyuwushang Date: Wed, 15 Jul 2026 09:51:12 +0800 Subject: [PATCH] plugin/secondary: reset catalog members on ID change (#8281) Signed-off-by: houyuwushang --- plugin/secondary/catalog.go | 44 ++++++++---- plugin/secondary/catalog_test.go | 116 ++++++++++++++++++++++++++++--- plugin/secondary/secondary.go | 1 + 3 files changed, 135 insertions(+), 26 deletions(-) diff --git a/plugin/secondary/catalog.go b/plugin/secondary/catalog.go index 348fb6580..4a499cd03 100644 --- a/plugin/secondary/catalog.go +++ b/plugin/secondary/catalog.go @@ -58,11 +58,17 @@ func (s *Secondary) applyCatalog(origin string, cat *catalog.Catalog, catalogZon memberZones[member.Zone] = struct{}{} if existing, ok := s.Z[member.Zone]; ok { - if dyn, ok := s.dynamicZones[member.Zone]; ok && dyn.catalog == origin && existing != nil { + dyn, dynamic := s.dynamicZones[member.Zone] + if !dynamic || dyn.catalog != origin || existing == nil { + log.Warningf("Skipping catalog member zone %s from %s: zone already exists", member.Zone, origin) continue } - log.Warningf("Skipping catalog member zone %s from %s: zone already exists", member.Zone, origin) - continue + if dyn.memberID == member.ID { + continue + } + previousID := dyn.memberID + s.removeDynamicZoneLocked(member.Zone, origin) + log.Infof("Reset catalog member zone %s from %s after member ID changed from %s to %s", member.Zone, origin, previousID, member.ID) } z := file.NewZone(member.Zone, "stdin") @@ -75,7 +81,7 @@ func (s *Secondary) applyCatalog(origin string, cat *catalog.Catalog, catalogZon s.Z[member.Zone] = z s.Names = append(s.Names, member.Zone) s.zoneNames[z] = member.Zone - s.dynamicZones[member.Zone] = &dynamicZone{catalog: origin, shutdown: shutdown} + s.dynamicZones[member.Zone] = &dynamicZone{catalog: origin, memberID: member.ID, shutdown: shutdown} starts = append(starts, dynamicZoneStart{origin: member.Zone, zone: z, shutdown: shutdown}) log.Infof("Added catalog member zone %s from catalog %s", member.Zone, origin) } @@ -84,18 +90,9 @@ func (s *Secondary) applyCatalog(origin string, cat *catalog.Catalog, catalogZon if _, ok := memberZones[member]; ok { continue } - dyn, ok := s.dynamicZones[member] - if !ok || dyn.catalog != origin { - continue + if s.removeDynamicZoneLocked(member, origin) { + log.Infof("Removed catalog member zone %s from catalog %s", member, origin) } - dyn.stopOnce.Do(func() { close(dyn.shutdown) }) - delete(s.dynamicZones, member) - if z := s.Z[member]; z != nil { - delete(s.zoneNames, z) - } - delete(s.Z, member) - s.Names = removeZoneName(s.Names, member) - log.Infof("Removed catalog member zone %s from catalog %s", member, origin) } s.catalogMemberZones[origin] = memberZones s.zoneMu.Unlock() @@ -105,6 +102,23 @@ func (s *Secondary) applyCatalog(origin string, cat *catalog.Catalog, catalogZon } } +// removeDynamicZoneLocked removes a zone only when it belongs to catalog. +// The caller must hold s.zoneMu for writing. +func (s *Secondary) removeDynamicZoneLocked(zone, catalog string) bool { + dyn, ok := s.dynamicZones[zone] + if !ok || dyn.catalog != catalog { + return false + } + dyn.stopOnce.Do(func() { close(dyn.shutdown) }) + delete(s.dynamicZones, zone) + if z := s.Z[zone]; z != nil { + delete(s.zoneNames, z) + } + delete(s.Z, zone) + s.Names = removeZoneName(s.Names, zone) + return true +} + func (s *Secondary) ensureZoneStateLocked() { if s.Z == nil { s.Z = make(map[string]*file.Zone) diff --git a/plugin/secondary/catalog_test.go b/plugin/secondary/catalog_test.go index 999326a96..189dd1faf 100644 --- a/plugin/secondary/catalog_test.go +++ b/plugin/secondary/catalog_test.go @@ -2,6 +2,7 @@ package secondary import ( "context" + "fmt" "strings" "sync" "testing" @@ -95,6 +96,83 @@ func TestTransferInCatalogRemovesMemberZone(t *testing.T) { } } +func TestTransferInCatalogResetsMemberZoneOnIDChange(t *testing.T) { + const origin = "catalog.example." + zones := newTestTransferZones(map[string][]dns.RR{ + origin: catalogZoneRecordsWithMemberID(t, "a", 1), + "example.org.": memberZoneRecordsWithAddress(t, 1, "192.0.2.1"), + }) + + server := dnstest.NewServer(zones.handler()) + defer server.Close() + + z := file.NewZone(origin, "stdin") + z.TransferFrom = []string{server.Addr} + s := newTestSecondary(origin, z, true) + t.Cleanup(s.stopDynamicZones) + + if err := s.transferIn(origin, z, nil); err != nil { + t.Fatalf("initial transferIn returned error: %v", err) + } + waitForAnswer(t, s, "www.example.org.", dns.TypeA) + + s.zoneMu.RLock() + before := s.Z["example.org."] + beforeDynamic := s.dynamicZones["example.org."] + s.zoneMu.RUnlock() + if before == nil || beforeDynamic == nil { + t.Fatal("expected initial catalog member zone and dynamic state") + } + + zones.set(origin, catalogZoneRecordsWithMemberID(t, "a", 2)) + if err := s.transferIn(origin, z, nil); err != nil { + t.Fatalf("same-ID transferIn returned error: %v", err) + } + s.zoneMu.RLock() + sameIDZone := s.Z["example.org."] + sameIDDynamic := s.dynamicZones["example.org."] + s.zoneMu.RUnlock() + if sameIDZone != before || sameIDDynamic != beforeDynamic { + t.Fatal("expected unchanged member ID to preserve the dynamic zone") + } + select { + case <-beforeDynamic.shutdown: + t.Fatal("expected unchanged member ID to preserve the transfer loop") + default: + } + + zones.set(origin, catalogZoneRecordsWithMemberID(t, "b", 3)) + zones.set("example.org.", memberZoneRecordsWithAddress(t, 2, "192.0.2.2")) + if err := s.transferIn(origin, z, nil); err != nil { + t.Fatalf("updated transferIn returned error: %v", err) + } + + s.zoneMu.RLock() + after := s.Z["example.org."] + afterDynamic := s.dynamicZones["example.org."] + s.zoneMu.RUnlock() + if before == after { + t.Fatal("expected member node ID change to replace the dynamic zone") + } + if afterDynamic == nil || afterDynamic.memberID != "b" { + t.Fatalf("expected active member ID b, got %+v", afterDynamic) + } + select { + case <-beforeDynamic.shutdown: + default: + t.Fatal("expected member node ID change to stop the old transfer loop") + } + + msg := waitForAnswer(t, s, "www.example.org.", dns.TypeA) + a, ok := msg.Answer[0].(*dns.A) + if !ok { + t.Fatalf("expected A answer, got %T", msg.Answer[0]) + } + if a.A.String() != "192.0.2.2" { + t.Fatalf("expected reset zone answer 192.0.2.2, got %s", a.A.String()) + } +} + func TestTransferInCatalogRejectsInvalidCatalog(t *testing.T) { const origin = "catalog.example." rrs := catalogZoneRecords(t, false) @@ -212,20 +290,31 @@ func waitForAnswer(t *testing.T, s *Secondary, name string, qtype uint16) *dns.M func catalogZoneRecords(t *testing.T, includeVersion bool) []dns.RR { t.Helper() + if !includeVersion { + soa := mustRR(t, "catalog.example. 0 IN SOA invalid. hostmaster.invalid. 1 3600 600 604800 0") + return []dns.RR{ + soa, + mustRR(t, "catalog.example. 0 IN NS invalid."), + mustRR(t, "a.zones.catalog.example. 0 IN PTR example.org."), + mustRR(t, `group.a.zones.catalog.example. 0 IN TXT "default"`), + soa, + } + } + return catalogZoneRecordsWithMemberID(t, "a", 1) +} - soa := mustRR(t, "catalog.example. 0 IN SOA invalid. hostmaster.invalid. 1 3600 600 604800 0") +func catalogZoneRecordsWithMemberID(t *testing.T, id string, serial int) []dns.RR { + t.Helper() + + soa := mustRR(t, fmt.Sprintf("catalog.example. 0 IN SOA invalid. hostmaster.invalid. %d 3600 600 604800 0", serial)) rrs := []dns.RR{ soa, mustRR(t, "catalog.example. 0 IN NS invalid."), - } - if includeVersion { - rrs = append(rrs, mustRR(t, `version.catalog.example. 0 IN TXT "2"`)) - } - rrs = append(rrs, - mustRR(t, "a.zones.catalog.example. 0 IN PTR example.org."), - mustRR(t, `group.a.zones.catalog.example. 0 IN TXT "default"`), + mustRR(t, `version.catalog.example. 0 IN TXT "2"`), + mustRR(t, fmt.Sprintf("%s.zones.catalog.example. 0 IN PTR example.org.", id)), + mustRR(t, fmt.Sprintf(`group.%s.zones.catalog.example. 0 IN TXT "default"`, id)), soa, - ) + } return rrs } @@ -243,12 +332,17 @@ func catalogZoneRecordsWithoutMembers(t *testing.T) []dns.RR { func memberZoneRecords(t *testing.T) []dns.RR { t.Helper() + return memberZoneRecordsWithAddress(t, 1, "192.0.2.1") +} - soa := mustRR(t, "example.org. 0 IN SOA ns.example.org. hostmaster.example.org. 1 3600 600 604800 0") +func memberZoneRecordsWithAddress(t *testing.T, serial int, address string) []dns.RR { + t.Helper() + + soa := mustRR(t, fmt.Sprintf("example.org. 0 IN SOA ns.example.org. hostmaster.example.org. %d 3600 600 604800 0", serial)) return []dns.RR{ soa, mustRR(t, "example.org. 0 IN NS ns.example.org."), - mustRR(t, "www.example.org. 0 IN A 192.0.2.1"), + mustRR(t, fmt.Sprintf("www.example.org. 0 IN A %s", address)), soa, } } diff --git a/plugin/secondary/secondary.go b/plugin/secondary/secondary.go index 1241f1f43..0be2fa245 100644 --- a/plugin/secondary/secondary.go +++ b/plugin/secondary/secondary.go @@ -25,6 +25,7 @@ type Secondary struct { type dynamicZone struct { catalog string + memberID string shutdown chan bool stopOnce sync.Once }