diff --git a/plugin/secondary/README.md b/plugin/secondary/README.md index d7ef1bdb9..91f93c760 100644 --- a/plugin/secondary/README.md +++ b/plugin/secondary/README.md @@ -27,7 +27,7 @@ A working syntax would be: ~~~ secondary [zones...] { transfer from ADDRESS [ADDRESS...] - catalog + catalog [MEMBER-ZONES...] fallthrough [ZONES...] } ~~~ @@ -38,7 +38,13 @@ secondary [zones...] { * `catalog` treats the transferred zone as an RFC 9432 catalog zone. After each successful catalog transfer, CoreDNS adds and removes the catalog member zones and transfers those member zones from - the same primary servers. + the same primary servers. Optional **MEMBER-ZONES** restrict which member zone names are accepted; + each name also matches its subdomains. With no **MEMBER-ZONES**, all member zones are accepted for + backward compatibility. RFC 9432 Section 7 recommends configuring this restriction because a + catalog producer otherwise controls which zones the consumer serves. A member in another catalog + remains a name clash unless the current catalog's `coo` property points to the newly updated + catalog. During that ownership migration, CoreDNS preserves the current zone data only when both + catalogs use the same member node label. * `fallthrough` If a query for a record in the zone results in NXDOMAIN, the query will be passed to the next plugin in the chain. If **[ZONES...]** are listed, then only queries for those zones @@ -74,6 +80,17 @@ example.net { } ~~~ +Restrict a catalog consumer to member zones at or below `example.org` and `internal.example`. + +~~~ corefile +catalog.example { + secondary { + transfer from 10.1.2.1 + catalog example.org internal.example + } +} +~~~ + ## Bugs Only AXFR is supported and the retrieved zone is not committed to disk. @@ -81,4 +98,4 @@ Only AXFR is supported and the retrieved zone is not committed to disk. ## See Also See the *transfer* plugin to enable zone transfers _to_ other servers. -And RFC 5936 detailing the AXFR protocol. +RFC 5936 details the AXFR protocol, and RFC 9432 defines DNS catalog zones. diff --git a/plugin/secondary/catalog.go b/plugin/secondary/catalog.go index 4a499cd03..9a06b193f 100644 --- a/plugin/secondary/catalog.go +++ b/plugin/secondary/catalog.go @@ -50,39 +50,52 @@ type dynamicZoneStart struct { func (s *Secondary) applyCatalog(origin string, cat *catalog.Catalog, catalogZone *file.Zone, t *transfer.Transfer) { memberZones := make(map[string]struct{}, len(cat.Members)) var starts []dynamicZoneStart + rejected := 0 s.zoneMu.Lock() s.ensureZoneStateLocked() for _, member := range cat.Members { + if !s.catalogMemberAllowed(origin, member.Zone) { + rejected++ + continue + } memberZones[member.Zone] = struct{}{} if existing, ok := s.Z[member.Zone]; ok { dyn, dynamic := s.dynamicZones[member.Zone] - if !dynamic || dyn.catalog != origin || existing == nil { + if !dynamic || existing == nil { log.Warningf("Skipping catalog member zone %s from %s: zone already exists", member.Zone, origin) continue } - if dyn.memberID == member.ID { + + if dyn.catalog == origin { + if dyn.memberID == member.ID { + continue + } + previousID := dyn.memberID + start := s.replaceDynamicZoneLocked(member, origin, catalogZone, nil) + starts = append(starts, start) + log.Infof("Reset catalog member zone %s from %s after member ID changed from %s to %s", member.Zone, origin, previousID, 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) + + s.catalogMu.RLock() + sourceMember, ok := catalogMember(s.catalogs[dyn.catalog], member.Zone) + if !ok || sourceMember.ChangeOfOwnership != origin { + s.catalogMu.RUnlock() + log.Warningf("Skipping catalog member zone %s from %s: zone already exists", member.Zone, origin) + continue + } + start, preserved := s.migrateCatalogMemberLocked(existing, dyn, sourceMember, member, origin, catalogZone) + s.catalogMu.RUnlock() + starts = append(starts, start) + s.logCatalogMigration(member.Zone, dyn.catalog, origin, sourceMember.ID, member.ID, preserved) + continue } - z := file.NewZone(member.Zone, "stdin") - if catalogZone != nil { - z.TransferFrom = append([]string(nil), catalogZone.TransferFrom...) - } - z.Upstream = upstream.New() - - shutdown := make(chan bool) - s.Z[member.Zone] = z - s.Names = append(s.Names, member.Zone) - s.zoneNames[z] = member.Zone - s.dynamicZones[member.Zone] = &dynamicZone{catalog: origin, memberID: member.ID, shutdown: shutdown} - starts = append(starts, dynamicZoneStart{origin: member.Zone, zone: z, shutdown: shutdown}) + start := s.replaceDynamicZoneLocked(member, origin, catalogZone, nil) + starts = append(starts, start) log.Infof("Added catalog member zone %s from catalog %s", member.Zone, origin) } @@ -97,11 +110,91 @@ func (s *Secondary) applyCatalog(origin string, cat *catalog.Catalog, catalogZon s.catalogMemberZones[origin] = memberZones s.zoneMu.Unlock() + if rejected > 0 { + log.Warningf("Skipped %d member zones from catalog %s: outside configured member zones", rejected, origin) + } for _, start := range starts { go s.transferAndUpdate(start.origin, start.zone, t, start.shutdown) } } +func (s *Secondary) catalogMemberAllowed(origin, member string) bool { + zones, ok := s.catalogZones[origin] + return ok && (len(zones) == 0 || zones.Matches(member) != "") +} + +func catalogMember(cat *catalog.Catalog, zone string) (catalog.Member, bool) { + if cat == nil { + return catalog.Member{}, false + } + for _, member := range cat.Members { + if member.Zone == zone { + return member, true + } + } + return catalog.Member{}, false +} + +// migrateCatalogMemberLocked moves ownership to target. State is retained only +// when the active, source, and target member IDs all describe the same member. +func (s *Secondary) migrateCatalogMemberLocked(existing *file.Zone, dyn *dynamicZone, source, target catalog.Member, targetOrigin string, targetZone *file.Zone) (dynamicZoneStart, bool) { + preserved := dyn.memberID == source.ID && source.ID == target.ID + var preserveFrom *file.Zone + if preserved { + preserveFrom = existing + } + return s.replaceDynamicZoneLocked(target, targetOrigin, targetZone, preserveFrom), preserved +} + +// replaceDynamicZoneLocked atomically installs a new transfer generation. A +// stopped generation may still finish an in-flight transfer, but it can only +// update the detached Zone pointer and cannot overwrite the new owner. +func (s *Secondary) replaceDynamicZoneLocked(member catalog.Member, origin string, catalogZone, preserveFrom *file.Zone) dynamicZoneStart { + if dyn, ok := s.dynamicZones[member.Zone]; ok { + dyn.stopOnce.Do(func() { close(dyn.shutdown) }) + } + + z := file.NewZone(member.Zone, "stdin") + if catalogZone != nil { + z.TransferFrom = append([]string(nil), catalogZone.TransferFrom...) + } + if preserveFrom != nil { + preserveDynamicZoneState(z, preserveFrom) + } + z.Upstream = upstream.New() + + if previous, ok := s.Z[member.Zone]; ok { + if previous != nil { + delete(s.zoneNames, previous) + } + } else { + s.Names = append(s.Names, member.Zone) + } + shutdown := make(chan bool) + s.Z[member.Zone] = z + s.zoneNames[z] = member.Zone + s.dynamicZones[member.Zone] = &dynamicZone{catalog: origin, memberID: member.ID, shutdown: shutdown} + return dynamicZoneStart{origin: member.Zone, zone: z, shutdown: shutdown} +} + +func preserveDynamicZoneState(target, source *file.Zone) { + // Dynamic transfers replace the live Apex and Tree instead of mutating them, + // so this snapshot remains isolated from any old in-flight transfer. + source.RLock() + target.Apex = source.Apex + target.Tree = source.Tree + target.Expired = source.Expired + source.RUnlock() +} + +func (s *Secondary) logCatalogMigration(zone, source, target, sourceID, targetID string, preserved bool) { + if preserved { + log.Infof("Migrated catalog member zone %s from catalog %s to %s, preserving state for member ID %s", zone, source, target, sourceID) + return + } + log.Infof("Migrated catalog member zone %s from catalog %s to %s with state reset after member identity changed from %s to %s", zone, source, target, sourceID, targetID) +} + // 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 { diff --git a/plugin/secondary/catalog_test.go b/plugin/secondary/catalog_test.go index 189dd1faf..4a62c3b6d 100644 --- a/plugin/secondary/catalog_test.go +++ b/plugin/secondary/catalog_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/coredns/coredns/plugin" "github.com/coredns/coredns/plugin/file" "github.com/coredns/coredns/plugin/pkg/dnstest" "github.com/coredns/coredns/plugin/pkg/fall" @@ -67,6 +68,51 @@ func TestTransferInCatalog(t *testing.T) { } } +func TestTransferInCatalogRestrictsMemberZones(t *testing.T) { + const ( + origin = "catalog.example." + allowed = "tenant.allowed.example." + denied = "tenant.denied.example." + ) + zones := newTestTransferZones(map[string][]dns.RR{ + origin: catalogZoneRecordsForMembers(t, origin, []catalogRecordMember{ + {id: "a", zone: allowed}, + {id: "b", zone: denied}, + }, 1), + allowed: memberZoneRecordsFor(t, allowed, 1, "192.0.2.1"), + denied: memberZoneRecordsFor(t, denied, 1, "192.0.2.2"), + }) + + server := dnstest.NewServer(zones.handler()) + defer server.Close() + + z := file.NewZone(origin, "stdin") + z.TransferFrom = []string{server.Addr} + s := newTestSecondary(origin, z, true) + s.catalogZones[origin] = plugin.Zones{"allowed.example."} + t.Cleanup(s.stopDynamicZones) + + if err := s.transferIn(origin, z, nil); err != nil { + t.Fatalf("transferIn returned error: %v", err) + } + waitForAnswer(t, s, "www."+allowed, dns.TypeA) + + s.zoneMu.RLock() + _, allowedDynamic := s.dynamicZones[allowed] + _, deniedDynamic := s.dynamicZones[denied] + _, admittedDenied := s.catalogMemberZones[origin][denied] + s.zoneMu.RUnlock() + if !allowedDynamic { + t.Fatalf("expected member zone %s to match configured suffix", allowed) + } + if deniedDynamic || admittedDenied { + t.Fatalf("expected member zone %s to be rejected", denied) + } + if _, _, ok := s.lookupZone("www." + denied); ok { + t.Fatalf("expected rejected member zone %s not to be served", denied) + } +} + func TestTransferInCatalogRemovesMemberZone(t *testing.T) { const origin = "catalog.example." zones := newTestTransferZones(map[string][]dns.RR{ @@ -173,6 +219,217 @@ func TestTransferInCatalogResetsMemberZoneOnIDChange(t *testing.T) { } } +func TestTransferInCatalogMigratesMemberZoneWithSameID(t *testing.T) { + fixture := newCatalogMigrationFixture(t, "a", "a", "new.catalog.example.", true) + + if err := fixture.s.transferIn(fixture.oldOrigin, fixture.oldCatalog, nil); err != nil { + t.Fatalf("old catalog transferIn returned error: %v", err) + } + waitForAddress(t, fixture.s, fixture.member, "192.0.2.1") + + fixture.s.zoneMu.RLock() + oldDynamic := fixture.s.dynamicZones[fixture.member] + fixture.s.zoneMu.RUnlock() + if oldDynamic == nil { + t.Fatal("expected member zone to belong to the old catalog") + } + + if err := fixture.s.transferIn(fixture.newOrigin, fixture.newCatalog, nil); err != nil { + t.Fatalf("new catalog transferIn returned error: %v", err) + } + + fixture.s.zoneMu.RLock() + newDynamic := fixture.s.dynamicZones[fixture.member] + fixture.s.zoneMu.RUnlock() + if newDynamic == nil || newDynamic.catalog != fixture.newOrigin || newDynamic.memberID != "a" { + t.Fatalf("expected member zone to migrate to %s with ID a, got %+v", fixture.newOrigin, newDynamic) + } + select { + case <-oldDynamic.shutdown: + default: + t.Fatal("expected migration to stop the old catalog transfer loop") + } + + // The target transfer is paused, so this answer can only come from state + // carried over from the old catalog. + waitForAddress(t, fixture.s, fixture.member, "192.0.2.1") + fixture.releaseTarget() + waitForAddress(t, fixture.s, fixture.member, "192.0.2.2") +} + +func TestTransferInCatalogRejectsOwnershipMigrationOutsideMemberZones(t *testing.T) { + fixture := newCatalogMigrationFixture(t, "a", "a", "new.catalog.example.", false) + fixture.s.catalogZones[fixture.newOrigin] = plugin.Zones{"other.example."} + + if err := fixture.s.transferIn(fixture.oldOrigin, fixture.oldCatalog, nil); err != nil { + t.Fatalf("old catalog transferIn returned error: %v", err) + } + waitForAddress(t, fixture.s, fixture.member, "192.0.2.1") + + fixture.s.zoneMu.RLock() + before := fixture.s.dynamicZones[fixture.member] + fixture.s.zoneMu.RUnlock() + if before == nil || before.catalog != fixture.oldOrigin { + t.Fatal("expected member zone to belong to the old catalog") + } + + if err := fixture.s.transferIn(fixture.newOrigin, fixture.newCatalog, nil); err != nil { + t.Fatalf("new catalog transferIn returned error: %v", err) + } + + fixture.s.zoneMu.RLock() + after := fixture.s.dynamicZones[fixture.member] + _, admitted := fixture.s.catalogMemberZones[fixture.newOrigin][fixture.member] + fixture.s.zoneMu.RUnlock() + if after == nil || after != before || after.catalog != fixture.oldOrigin { + t.Fatalf("expected rejected migration to preserve ownership by %s, got %+v", fixture.oldOrigin, after) + } + if admitted { + t.Fatal("expected target catalog not to admit an out-of-scope member") + } + select { + case <-before.shutdown: + t.Fatal("expected rejected migration to preserve the old transfer loop") + default: + } +} + +func TestTransferInCatalogWaitsForTargetUpdateAfterSourceAddsCOO(t *testing.T) { + fixture := newCatalogMigrationFixture(t, "a", "a", "", true) + + if err := fixture.s.transferIn(fixture.oldOrigin, fixture.oldCatalog, nil); err != nil { + t.Fatalf("old catalog transferIn returned error: %v", err) + } + waitForAddress(t, fixture.s, fixture.member, "192.0.2.1") + + if err := fixture.s.transferIn(fixture.newOrigin, fixture.newCatalog, nil); err != nil { + t.Fatalf("new catalog transferIn returned error: %v", err) + } + fixture.s.zoneMu.RLock() + before := fixture.s.dynamicZones[fixture.member] + fixture.s.zoneMu.RUnlock() + if before == nil || before.catalog != fixture.oldOrigin { + t.Fatalf("expected missing coo to leave ownership with %s, got %+v", fixture.oldOrigin, before) + } + + fixture.oldZones.set(fixture.oldOrigin, catalogZoneRecordsFor(t, fixture.oldOrigin, "a", fixture.member, fixture.newOrigin, 2)) + if err := fixture.s.transferIn(fixture.oldOrigin, fixture.oldCatalog, nil); err != nil { + t.Fatalf("updated old catalog transferIn returned error: %v", err) + } + + fixture.s.zoneMu.RLock() + afterSourceUpdate := fixture.s.dynamicZones[fixture.member] + fixture.s.zoneMu.RUnlock() + if afterSourceUpdate != before || afterSourceUpdate.catalog != fixture.oldOrigin { + t.Fatalf("expected source coo update to wait for a target catalog update, got %+v", afterSourceUpdate) + } + select { + case <-before.shutdown: + t.Fatal("expected source coo update to preserve the old transfer loop") + default: + } + waitForAddress(t, fixture.s, fixture.member, "192.0.2.1") + + fixture.newZones.set(fixture.newOrigin, catalogZoneRecordsFor(t, fixture.newOrigin, "a", fixture.member, "", 2)) + if err := fixture.s.transferIn(fixture.newOrigin, fixture.newCatalog, nil); err != nil { + t.Fatalf("updated target catalog transferIn returned error: %v", err) + } + + fixture.s.zoneMu.RLock() + afterTargetUpdate := fixture.s.dynamicZones[fixture.member] + fixture.s.zoneMu.RUnlock() + if afterTargetUpdate == nil || afterTargetUpdate.catalog != fixture.newOrigin { + t.Fatalf("expected target catalog update to complete migration to %s, got %+v", fixture.newOrigin, afterTargetUpdate) + } + select { + case <-before.shutdown: + default: + t.Fatal("expected completed coo handshake to stop the old transfer loop") + } + waitForAddress(t, fixture.s, fixture.member, "192.0.2.1") + fixture.releaseTarget() + waitForAddress(t, fixture.s, fixture.member, "192.0.2.2") +} + +func TestTransferInCatalogRejectsUncoordinatedMigration(t *testing.T) { + tests := []struct { + name string + coo string + }{ + {name: "missing coo"}, + {name: "wrong coo", coo: "other.catalog.example."}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fixture := newCatalogMigrationFixture(t, "a", "a", test.coo, false) + + if err := fixture.s.transferIn(fixture.oldOrigin, fixture.oldCatalog, nil); err != nil { + t.Fatalf("old catalog transferIn returned error: %v", err) + } + waitForAddress(t, fixture.s, fixture.member, "192.0.2.1") + + fixture.s.zoneMu.RLock() + before := fixture.s.dynamicZones[fixture.member] + fixture.s.zoneMu.RUnlock() + if err := fixture.s.transferIn(fixture.newOrigin, fixture.newCatalog, nil); err != nil { + t.Fatalf("new catalog transferIn returned error: %v", err) + } + + fixture.s.zoneMu.RLock() + after := fixture.s.dynamicZones[fixture.member] + fixture.s.zoneMu.RUnlock() + if after != before || after == nil || after.catalog != fixture.oldOrigin { + t.Fatalf("expected uncoordinated migration to preserve old ownership, got before=%+v after=%+v", before, after) + } + select { + case <-before.shutdown: + t.Fatal("expected uncoordinated migration to preserve the old transfer loop") + default: + } + waitForAddress(t, fixture.s, fixture.member, "192.0.2.1") + }) + } +} + +func TestTransferInCatalogMigrationResetsStateForDifferentID(t *testing.T) { + fixture := newCatalogMigrationFixture(t, "a", "b", "new.catalog.example.", true) + + if err := fixture.s.transferIn(fixture.oldOrigin, fixture.oldCatalog, nil); err != nil { + t.Fatalf("old catalog transferIn returned error: %v", err) + } + waitForAddress(t, fixture.s, fixture.member, "192.0.2.1") + + fixture.s.zoneMu.RLock() + oldDynamic := fixture.s.dynamicZones[fixture.member] + fixture.s.zoneMu.RUnlock() + if err := fixture.s.transferIn(fixture.newOrigin, fixture.newCatalog, nil); err != nil { + t.Fatalf("new catalog transferIn returned error: %v", err) + } + + fixture.s.zoneMu.RLock() + memberZone := fixture.s.Z[fixture.member] + newDynamic := fixture.s.dynamicZones[fixture.member] + fixture.s.zoneMu.RUnlock() + if newDynamic == nil || newDynamic.catalog != fixture.newOrigin || newDynamic.memberID != "b" { + t.Fatalf("expected member zone to migrate to %s with ID b, got %+v", fixture.newOrigin, newDynamic) + } + select { + case <-oldDynamic.shutdown: + default: + t.Fatal("expected migration to stop the old catalog transfer loop") + } + memberZone.RLock() + soa := memberZone.SOA + memberZone.RUnlock() + if soa != nil { + t.Fatal("expected different member ID to reset old zone data before the target transfer") + } + + fixture.releaseTarget() + waitForAddress(t, fixture.s, fixture.member, "192.0.2.2") +} + func TestTransferInCatalogRejectsInvalidCatalog(t *testing.T) { const origin = "catalog.example." rrs := catalogZoneRecords(t, false) @@ -222,8 +479,9 @@ func TestTransferInSkipsCatalogParseForRegularZone(t *testing.T) { } type testTransferZones struct { - mu sync.RWMutex - records map[string][]dns.RR + mu sync.RWMutex + records map[string][]dns.RR + axfrGates map[string]<-chan struct{} } func newTestTransferZones(records map[string][]dns.RR) *testTransferZones { @@ -236,6 +494,15 @@ func (z *testTransferZones) set(zone string, rrs []dns.RR) { z.mu.Unlock() } +func (z *testTransferZones) blockAXFR(zone string, gate <-chan struct{}) { + z.mu.Lock() + if z.axfrGates == nil { + z.axfrGates = make(map[string]<-chan struct{}) + } + z.axfrGates[zone] = gate + z.mu.Unlock() +} + func (z *testTransferZones) handler() dns.HandlerFunc { return func(w dns.ResponseWriter, req *dns.Msg) { m := new(dns.Msg) @@ -244,6 +511,7 @@ func (z *testTransferZones) handler() dns.HandlerFunc { qname := strings.ToLower(dns.Fqdn(req.Question[0].Name)) z.mu.RLock() rrs := append([]dns.RR(nil), z.records[qname]...) + gate := z.axfrGates[qname] z.mu.RUnlock() switch req.Question[0].Qtype { @@ -255,6 +523,9 @@ func (z *testTransferZones) handler() dns.HandlerFunc { } } case dns.TypeAXFR: + if gate != nil { + <-gate + } m.Answer = rrs } } @@ -262,10 +533,80 @@ func (z *testTransferZones) handler() dns.HandlerFunc { } } +type catalogMigrationFixture struct { + s *Secondary + oldOrigin string + newOrigin string + member string + oldCatalog *file.Zone + newCatalog *file.Zone + oldZones *testTransferZones + newZones *testTransferZones + release func() +} + +func newCatalogMigrationFixture(t *testing.T, oldID, newID, coo string, blockTarget bool) *catalogMigrationFixture { + t.Helper() + + const ( + oldOrigin = "old.catalog.example." + newOrigin = "new.catalog.example." + member = "example.org." + ) + + oldZones := newTestTransferZones(map[string][]dns.RR{ + oldOrigin: catalogZoneRecordsFor(t, oldOrigin, oldID, member, coo, 1), + member: memberZoneRecordsWithAddress(t, 1, "192.0.2.1"), + }) + newZones := newTestTransferZones(map[string][]dns.RR{ + newOrigin: catalogZoneRecordsFor(t, newOrigin, newID, member, "", 1), + member: memberZoneRecordsWithAddress(t, 2, "192.0.2.2"), + }) + + var releaseOnce sync.Once + targetGate := make(chan struct{}) + release := func() { releaseOnce.Do(func() { close(targetGate) }) } + if blockTarget { + newZones.blockAXFR(member, targetGate) + } + + oldServer := dnstest.NewMultipleServer(oldZones.handler()) + t.Cleanup(oldServer.Close) + newServer := dnstest.NewMultipleServer(newZones.handler()) + t.Cleanup(newServer.Close) + t.Cleanup(release) + + oldCatalog := file.NewZone(oldOrigin, "stdin") + oldCatalog.TransferFrom = []string{oldServer.Addr} + newCatalog := file.NewZone(newOrigin, "stdin") + newCatalog.TransferFrom = []string{newServer.Addr} + s := newSecondary(file.Zones{ + Z: map[string]*file.Zone{oldOrigin: oldCatalog, newOrigin: newCatalog}, + Names: []string{oldOrigin, newOrigin}, + }, fall.F{}, map[string]plugin.Zones{oldOrigin: nil, newOrigin: nil}) + t.Cleanup(s.stopDynamicZones) + + return &catalogMigrationFixture{ + s: s, + oldOrigin: oldOrigin, + newOrigin: newOrigin, + member: member, + oldCatalog: oldCatalog, + newCatalog: newCatalog, + oldZones: oldZones, + newZones: newZones, + release: release, + } +} + +func (f *catalogMigrationFixture) releaseTarget() { + f.release() +} + func newTestSecondary(origin string, z *file.Zone, catalog bool) *Secondary { - catalogZones := map[string]struct{}{} + catalogZones := map[string]plugin.Zones{} if catalog { - catalogZones[origin] = struct{}{} + catalogZones[origin] = nil } return newSecondary(file.Zones{Z: map[string]*file.Zone{origin: z}, Names: []string{origin}}, fall.F{}, catalogZones) } @@ -288,6 +629,22 @@ func waitForAnswer(t *testing.T, s *Secondary, name string, qtype uint16) *dns.M return nil } +func waitForAddress(t *testing.T, s *Secondary, zone, address string) { + t.Helper() + + deadline := time.Now().Add(time.Second) + for time.Now().Before(deadline) { + msg := waitForAnswer(t, s, "www."+zone, dns.TypeA) + if len(msg.Answer) == 1 { + if a, ok := msg.Answer[0].(*dns.A); ok && a.A.String() == address { + return + } + } + time.Sleep(10 * time.Millisecond) + } + t.Fatalf("timed out waiting for %s to answer with %s", zone, address) +} + func catalogZoneRecords(t *testing.T, includeVersion bool) []dns.RR { t.Helper() if !includeVersion { @@ -305,17 +662,39 @@ func catalogZoneRecords(t *testing.T, includeVersion bool) []dns.RR { func catalogZoneRecordsWithMemberID(t *testing.T, id string, serial int) []dns.RR { t.Helper() + return catalogZoneRecordsFor(t, "catalog.example.", id, "example.org.", "", serial) +} - soa := mustRR(t, fmt.Sprintf("catalog.example. 0 IN SOA invalid. hostmaster.invalid. %d 3600 600 604800 0", serial)) +func catalogZoneRecordsFor(t *testing.T, origin, id, member, coo string, serial int) []dns.RR { + t.Helper() + return catalogZoneRecordsForMembers(t, origin, []catalogRecordMember{{id: id, zone: member, coo: coo}}, serial) +} + +type catalogRecordMember struct { + id string + zone string + coo string +} + +func catalogZoneRecordsForMembers(t *testing.T, origin string, members []catalogRecordMember, serial int) []dns.RR { + t.Helper() + + soa := mustRR(t, fmt.Sprintf("%s 0 IN SOA invalid. hostmaster.invalid. %d 3600 600 604800 0", origin, serial)) rrs := []dns.RR{ soa, - mustRR(t, "catalog.example. 0 IN NS invalid."), - 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, + mustRR(t, fmt.Sprintf("%s 0 IN NS invalid.", origin)), + mustRR(t, fmt.Sprintf(`version.%s 0 IN TXT "2"`, origin)), } - return rrs + for _, member := range members { + rrs = append(rrs, + mustRR(t, fmt.Sprintf("%s.zones.%s 0 IN PTR %s", member.id, origin, member.zone)), + mustRR(t, fmt.Sprintf(`group.%s.zones.%s 0 IN TXT "default"`, member.id, origin)), + ) + if member.coo != "" { + rrs = append(rrs, mustRR(t, fmt.Sprintf("coo.%s.zones.%s 0 IN PTR %s", member.id, origin, member.coo))) + } + } + return append(rrs, soa) } func catalogZoneRecordsWithoutMembers(t *testing.T) []dns.RR { @@ -337,12 +716,17 @@ func memberZoneRecords(t *testing.T) []dns.RR { func memberZoneRecordsWithAddress(t *testing.T, serial int, address string) []dns.RR { t.Helper() + return memberZoneRecordsFor(t, "example.org.", serial, address) +} - soa := mustRR(t, fmt.Sprintf("example.org. 0 IN SOA ns.example.org. hostmaster.example.org. %d 3600 600 604800 0", serial)) +func memberZoneRecordsFor(t *testing.T, origin string, serial int, address string) []dns.RR { + t.Helper() + + soa := mustRR(t, fmt.Sprintf("%s 0 IN SOA ns.%s hostmaster.%s %d 3600 600 604800 0", origin, origin, origin, serial)) return []dns.RR{ soa, - mustRR(t, "example.org. 0 IN NS ns.example.org."), - mustRR(t, fmt.Sprintf("www.example.org. 0 IN A %s", address)), + mustRR(t, fmt.Sprintf("%s 0 IN NS ns.%s", origin, origin)), + mustRR(t, fmt.Sprintf("www.%s 0 IN A %s", origin, address)), soa, } } diff --git a/plugin/secondary/secondary.go b/plugin/secondary/secondary.go index 0be2fa245..6cd6bf983 100644 --- a/plugin/secondary/secondary.go +++ b/plugin/secondary/secondary.go @@ -4,6 +4,7 @@ package secondary import ( "sync" + "github.com/coredns/coredns/plugin" "github.com/coredns/coredns/plugin/file" "github.com/coredns/coredns/plugin/pkg/catalog" ) @@ -19,7 +20,7 @@ type Secondary struct { catalogMu sync.RWMutex catalogs map[string]*catalog.Catalog - catalogZones map[string]struct{} + catalogZones map[string]plugin.Zones catalogMemberZones map[string]map[string]struct{} } diff --git a/plugin/secondary/setup.go b/plugin/secondary/setup.go index e2bc96f30..105c644a9 100644 --- a/plugin/secondary/setup.go +++ b/plugin/secondary/setup.go @@ -1,6 +1,7 @@ package secondary import ( + "fmt" "sync" "time" @@ -71,7 +72,7 @@ func setup(c *caddy.Controller) error { return nil } -func newSecondary(zones file.Zones, fall fall.F, catalogZones map[string]struct{}) *Secondary { +func newSecondary(zones file.Zones, fall fall.F, catalogZones map[string]plugin.Zones) *Secondary { s := &Secondary{ File: file.File{Zones: zones, Fall: fall}, zoneNames: make(map[*file.Zone]string, len(zones.Z)), @@ -128,11 +129,11 @@ func waitForTransferRetry(updateShutdown <-chan bool, dur time.Duration) bool { } } -func secondaryParse(c *caddy.Controller) (file.Zones, fall.F, map[string]struct{}, error) { +func secondaryParse(c *caddy.Controller) (file.Zones, fall.F, map[string]plugin.Zones, error) { z := make(map[string]*file.Zone) names := []string{} fall := fall.F{} - catalogZones := map[string]struct{}{} + catalogZones := map[string]plugin.Zones{} for c.Next() { if c.Val() == "secondary" { // secondary [origin] @@ -155,11 +156,12 @@ func secondaryParse(c *caddy.Controller) (file.Zones, fall.F, map[string]struct{ } hasTransfer = true case "catalog": - if len(c.RemainingArgs()) != 0 { - return file.Zones{}, fall, nil, c.ArgErr() + memberZones, err := catalogMemberZonesFromArgs(c.RemainingArgs()) + if err != nil { + return file.Zones{}, fall, nil, err } for _, origin := range origins { - catalogZones[origin] = struct{}{} + catalogZones[origin] = mergeCatalogMemberZones(catalogZones, origin, memberZones) } case "fallthrough": fall.SetZonesFromArgs(c.RemainingArgs()) @@ -181,3 +183,36 @@ func secondaryParse(c *caddy.Controller) (file.Zones, fall.F, map[string]struct{ } return file.Zones{Z: z, Names: names}, fall, catalogZones, nil } + +func catalogMemberZonesFromArgs(args []string) (plugin.Zones, error) { + zones := make(plugin.Zones, 0, len(args)) + for _, arg := range args { + normalized := plugin.Host(arg).NormalizeExact() + if len(normalized) == 0 { + return nil, fmt.Errorf("invalid catalog member zone %q", arg) + } + zones = append(zones, normalized...) + } + return zones, nil +} + +func mergeCatalogMemberZones(config map[string]plugin.Zones, origin string, zones plugin.Zones) plugin.Zones { + existing, configured := config[origin] + if configured && (len(existing) == 0 || len(zones) == 0) { + return nil + } + + merged := append(plugin.Zones(nil), existing...) + seen := make(map[string]struct{}, len(merged)+len(zones)) + for _, zone := range merged { + seen[zone] = struct{}{} + } + for _, zone := range zones { + if _, ok := seen[zone]; ok { + continue + } + merged = append(merged, zone) + seen[zone] = struct{}{} + } + return merged +} diff --git a/plugin/secondary/setup_test.go b/plugin/secondary/setup_test.go index 135b83fcb..6d6ca31d4 100644 --- a/plugin/secondary/setup_test.go +++ b/plugin/secondary/setup_test.go @@ -1,9 +1,11 @@ package secondary import ( + "slices" "testing" "github.com/coredns/caddy" + "github.com/coredns/coredns/plugin" "github.com/coredns/coredns/plugin/pkg/fall" ) @@ -14,7 +16,7 @@ func TestSecondaryParse(t *testing.T) { transferFrom string zones []string fall fall.F - catalogZones []string + catalogZones map[string]plugin.Zones }{ { `secondary { @@ -45,12 +47,47 @@ func TestSecondaryParse(t *testing.T) { "127.0.0.1:53", []string{"catalog.example."}, fall.F{}, - []string{"catalog.example."}, + map[string]plugin.Zones{"catalog.example.": nil}, }, { `secondary catalog.example { transfer from 127.0.0.1 - catalog extra + catalog EXAMPLE.ORG internal.example + }`, + false, + "127.0.0.1:53", + []string{"catalog.example."}, + fall.F{}, + map[string]plugin.Zones{"catalog.example.": {"example.org.", "internal.example."}}, + }, + { + `secondary catalog.example { + transfer from 127.0.0.1 + catalog example.org EXAMPLE.ORG + catalog internal.example + }`, + false, + "127.0.0.1:53", + []string{"catalog.example."}, + fall.F{}, + map[string]plugin.Zones{"catalog.example.": {"example.org.", "internal.example."}}, + }, + { + `secondary catalog.example { + transfer from 127.0.0.1 + catalog example.org + catalog + }`, + false, + "127.0.0.1:53", + []string{"catalog.example."}, + fall.F{}, + map[string]plugin.Zones{"catalog.example.": nil}, + }, + { + `secondary catalog.example { + transfer from 127.0.0.1 + catalog : }`, true, "", @@ -131,10 +168,14 @@ func TestSecondaryParse(t *testing.T) { if len(catalogZones) != len(test.catalogZones) { t.Fatalf("Test %d catalog zone count mismatch: expected %d, got %d", i, len(test.catalogZones), len(catalogZones)) } - for _, name := range test.catalogZones { - if _, ok := catalogZones[name]; !ok { + for name, expected := range test.catalogZones { + actual, ok := catalogZones[name] + if !ok { t.Fatalf("Test %d expected catalog zone %q", i, name) } + if !slices.Equal(actual, expected) { + t.Fatalf("Test %d catalog member zones for %q mismatch: expected %v, got %v", i, name, expected, actual) + } } } }