plugin/file: introduce snapshot()/setData() accessors for zone data (#8040)

Signed-off-by: Ryan Brewster <rpb@anthropic.com>

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, tr := z.snapshot()
-	ap := z.Apex
-	tr := z.Tree
-	z.RUnlock()
 	if ap.SOA == nil {
 		return nil, nil, nil, ServerFailure
 	}

plugin/file/reload.go

@@ -36,13 +36,9 @@ func (z *Zone) Reload(t *transfer.Transfer) error {
 					continue
 				}
 
-				// copy elements we need
+				z.setData(zone.Apex, zone.Tree)
-				z.Lock()
-				z.Apex = zone.Apex
-				z.Tree = zone.Tree
-				z.Unlock()
 
-				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)

plugin/file/secondary.go

@@ -53,11 +53,7 @@ Transfer:
 		return Err
 	}
 
-	z.Lock()
+	z.setData(z1.Apex, z1.Tree)
-	z.Tree = z1.Tree
-	z.Apex = z1.Apex
-	z.Expired = false
-	z.Unlock()
 	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

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
+	ap, t := z.snapshot()
-	// same generation the SOA came from, even if TransferIn swaps them mid-AXFR.
+	apex, err := ap.records()
-	z.RLock()
-	apex, err := z.apexIfDefinedLocked()
-	t := z.Tree
-	z.RUnlock()
 	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
+		if serial != 0 && ap.SOA.Serial == serial { // ixfr fallback, only send SOA
-			ch <- []dns.RR{apex[0]}
+			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)
 	}()

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.
+// snapshot returns the apex and tree under a single read lock so callers see
-func (z *Zone) ApexIfDefined() ([]dns.RR, error) {
+// 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.
+// setData atomically replaces the zone's apex and tree and clears the expired
-func (z *Zone) apexIfDefinedLocked() ([]dns.RR, error) {
+// flag. It is the write-side counterpart to snapshot.
-	if z.SOA == nil {
+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 := make([]dns.RR, 0, 1+len(a.SIGSOA)+len(a.NS)+len(a.SIGNS))
-	rrs := []dns.RR{z.SOA}
+	rrs = append(rrs, a.SOA)
-
+	rrs = append(rrs, a.SIGSOA...)
-	if len(z.SIGSOA) > 0 {
+	rrs = append(rrs, a.NS...)
-		rrs = append(rrs, z.SIGSOA...)
+	rrs = append(rrs, a.SIGNS...)
-	}
-	if len(z.NS) > 0 {
-		rrs = append(rrs, z.NS...)
-	}
-	if len(z.SIGNS) > 0 {
-		rrs = append(rrs, z.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()