From 50cbaf87a080532ba634bf643f64e5f573af2683 Mon Sep 17 00:00:00 2001 From: rpb-ant Date: Sun, 12 Apr 2026 13:34:36 -0400 Subject: [PATCH] plugin/file: introduce snapshot()/setData() accessors for zone data (#8040) Signed-off-by: Ryan Brewster --- plugin/file/lookup.go | 5 +--- plugin/file/reload.go | 8 ++---- plugin/file/secondary.go | 12 +++------ plugin/file/xfr.go | 14 ++++------- plugin/file/zone.go | 54 +++++++++++++++++++++------------------- 5 files changed, 41 insertions(+), 52 deletions(-) diff --git a/plugin/file/lookup.go b/plugin/file/lookup.go index 0948a0724..c8cc88e7d 100644 --- a/plugin/file/lookup.go +++ b/plugin/file/lookup.go @@ -37,10 +37,7 @@ func (z *Zone) Lookup(ctx context.Context, state request.Request, qname string) // If z is a secondary zone we might not have transferred it, meaning we have // all zone context setup, except the actual record. This means (for one thing) the apex // is empty and we don't have a SOA record. - z.RLock() - ap := z.Apex - tr := z.Tree - z.RUnlock() + ap, tr := z.snapshot() if ap.SOA == nil { return nil, nil, nil, ServerFailure } diff --git a/plugin/file/reload.go b/plugin/file/reload.go index 4db8e11d2..afba78a34 100644 --- a/plugin/file/reload.go +++ b/plugin/file/reload.go @@ -36,13 +36,9 @@ func (z *Zone) Reload(t *transfer.Transfer) error { continue } - // copy elements we need - z.Lock() - z.Apex = zone.Apex - z.Tree = zone.Tree - z.Unlock() + z.setData(zone.Apex, zone.Tree) - log.Infof("Successfully reloaded zone %q in %q with %d SOA serial", z.origin, zFile, z.SOA.Serial) + log.Infof("Successfully reloaded zone %q in %q with %d SOA serial", z.origin, zFile, zone.SOA.Serial) if t != nil { if err := t.Notify(z.origin); err != nil { log.Warningf("Failed sending notifies: %s", err) diff --git a/plugin/file/secondary.go b/plugin/file/secondary.go index 43be88bfa..2aaa43f38 100644 --- a/plugin/file/secondary.go +++ b/plugin/file/secondary.go @@ -53,11 +53,7 @@ Transfer: return Err } - z.Lock() - z.Tree = z1.Tree - z.Apex = z1.Apex - z.Expired = false - z.Unlock() + z.setData(z1.Apex, z1.Tree) log.Infof("Transferred: %s from %s", z.origin, tr) // Send notify messages to secondary servers @@ -98,10 +94,10 @@ Transfer: if serial == -1 { return false, Err } - if !z.hasSOA() { + soa := z.getSOA() + if soa == nil { return true, Err } - soa := z.getSOA() return less(soa.Serial, uint32(serial)), Err // #nosec G115 -- serial fits in uint32 per DNS RFC } @@ -119,7 +115,7 @@ func less(a, b uint32) bool { // will be marked expired. func (z *Zone) Update(updateShutdown chan bool, t *transfer.Transfer) error { // If we don't have a SOA, we don't have a zone, wait for it to appear. - for !z.hasSOA() { + for z.getSOA() == nil { time.Sleep(1 * time.Second) } retryActive := false diff --git a/plugin/file/xfr.go b/plugin/file/xfr.go index 75e202d48..d0a2bdc16 100644 --- a/plugin/file/xfr.go +++ b/plugin/file/xfr.go @@ -19,20 +19,16 @@ func (f File) Transfer(zone string, serial uint32) (<-chan []dns.RR, error) { // Transfer transfers a zone with serial in the returned channel and implements IXFR fallback, by just // sending a single SOA record. func (z *Zone) Transfer(serial uint32) (<-chan []dns.RR, error) { - // Snapshot apex and tree under one read lock so the goroutine walks the - // same generation the SOA came from, even if TransferIn swaps them mid-AXFR. - z.RLock() - apex, err := z.apexIfDefinedLocked() - t := z.Tree - z.RUnlock() + ap, t := z.snapshot() + apex, err := ap.records() if err != nil { return nil, err } ch := make(chan []dns.RR) go func() { - if serial != 0 && apex[0].(*dns.SOA).Serial == serial { // ixfr fallback, only send SOA - ch <- []dns.RR{apex[0]} + if serial != 0 && ap.SOA.Serial == serial { // ixfr fallback, only send SOA + ch <- []dns.RR{ap.SOA} close(ch) return @@ -40,7 +36,7 @@ func (z *Zone) Transfer(serial uint32) (<-chan []dns.RR, error) { ch <- apex t.Walk(func(e *tree.Elem, _ map[uint16][]dns.RR) error { ch <- e.All(); return nil }) - ch <- []dns.RR{apex[0]} + ch <- []dns.RR{ap.SOA} close(ch) }() diff --git a/plugin/file/zone.go b/plugin/file/zone.go index d0f792c34..bd7725342 100644 --- a/plugin/file/zone.go +++ b/plugin/file/zone.go @@ -142,34 +142,44 @@ func (z *Zone) SetFile(path string) { z.Unlock() } -// ApexIfDefined returns the apex nodes from z. The SOA record is the first record, if it does not exist, an error is returned. -func (z *Zone) ApexIfDefined() ([]dns.RR, error) { +// snapshot returns the apex and tree under a single read lock so callers see +// a consistent zone generation even if TransferIn or Reload swaps them. +func (z *Zone) snapshot() (Apex, *tree.Tree) { z.RLock() defer z.RUnlock() - return z.apexIfDefinedLocked() + return z.Apex, z.Tree } -// apexIfDefinedLocked is ApexIfDefined without locking; caller must hold z's read lock. -func (z *Zone) apexIfDefinedLocked() ([]dns.RR, error) { - if z.SOA == nil { +// setData atomically replaces the zone's apex and tree and clears the expired +// flag. It is the write-side counterpart to snapshot. +func (z *Zone) setData(ap Apex, t *tree.Tree) { + z.Lock() + z.Apex = ap + z.Tree = t + z.Expired = false + z.Unlock() +} + +// records returns the apex records in zone-file order (SOA, RRSIG(SOA), NS, +// RRSIG(NS)), or an error if no SOA is set. +func (a Apex) records() ([]dns.RR, error) { + if a.SOA == nil { return nil, fmt.Errorf("no SOA") } - - rrs := []dns.RR{z.SOA} - - if len(z.SIGSOA) > 0 { - rrs = append(rrs, z.SIGSOA...) - } - if len(z.NS) > 0 { - rrs = append(rrs, z.NS...) - } - if len(z.SIGNS) > 0 { - rrs = append(rrs, z.SIGNS...) - } - + rrs := make([]dns.RR, 0, 1+len(a.SIGSOA)+len(a.NS)+len(a.SIGNS)) + rrs = append(rrs, a.SOA) + rrs = append(rrs, a.SIGSOA...) + rrs = append(rrs, a.NS...) + rrs = append(rrs, a.SIGNS...) return rrs, nil } +// ApexIfDefined returns the apex nodes from z. The SOA record is the first record, if it does not exist, an error is returned. +func (z *Zone) ApexIfDefined() ([]dns.RR, error) { + ap, _ := z.snapshot() + return ap.records() +} + // NameFromRight returns the labels from the right, staring with the // origin and then i labels extra. When we are overshooting the name // the returned boolean is set to true. @@ -197,12 +207,6 @@ func (z *Zone) nameFromRight(qname string, i int) (string, bool) { return qname[n:], false } -func (z *Zone) hasSOA() bool { - z.RLock() - defer z.RUnlock() - return z.SOA != nil -} - func (z *Zone) getSOA() *dns.SOA { z.RLock() defer z.RUnlock()