feat(secondary): Send NOTIFY messages after zone transfer (#7901)

* feat(secondary): Send NOTIFY messages after zone transfer

- Modified TransferIn() method to accept a transfer.Transfer parameter
- Added NOTIFY message sending after successful zone transfer in secondary plugin
- Updated Update() method to pass the transfer handler through the zone update cycle
- Added comprehensive tests for the secondary notify functionality

Closes #5669

Signed-off-by: liucongran <liucongran327@gmail.com>

* fix(secondary): Fix TransferIn method call in test

Update test to pass nil parameter to TransferIn method after signature change

Signed-off-by: liucongran <liucongran327@gmail.com>

* refactor(secondary): Clean up imports and add helper methods

- Reorder imports for consistency
- Add hasSOA() and getSOA() helper methods to Zone
- Remove unnecessary blank lines in tests

Signed-off-by: liucongran <liucongran327@gmail.com>

* fix(test): Fix variable declaration in secondary test

Change corefile variable assignment to use short declaration syntax (:=)
to fix compilation error.

Signed-off-by: liucongran <liucongran327@gmail.com>

* refactor(secondary): Use getSOA helper method in shouldTransfer

Replace direct SOA access with getSOA() helper method for consistency.

Signed-off-by: liucongran <liucongran327@gmail.com>

---------

Signed-off-by: liucongran <liucongran327@gmail.com>
Co-authored-by: liucongran <liucongran@cestc.cn>

plugin/file/file.go

@@ -22,7 +22,7 @@ type (
 	File struct {
 		Next plugin.Handler
 		Zones
-		transfer *transfer.Transfer
+		Xfer *transfer.Transfer
 
 		Fall fall.F
 	}
@@ -70,7 +70,7 @@ func (f File) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
 			log.Infof("Notify from %s for %s: checking transfer", state.IP(), zone)
 			ok, err := z.shouldTransfer()
 			if ok {
-				z.TransferIn()
+				z.TransferIn(f.Xfer)
 			} else {
 				log.Infof("Notify from %s for %s: no SOA serial increase seen", state.IP(), zone)
 			}

plugin/file/secondary.go

@@ -4,11 +4,13 @@ import (
 	"math/rand"
 	"time"
 
+	"github.com/coredns/coredns/plugin/transfer"
+
 	"github.com/miekg/dns"
 )
 
 // TransferIn retrieves the zone from the masters, parses it and sets it live.
-func (z *Zone) TransferIn() error {
+func (z *Zone) TransferIn(t *transfer.Transfer) error {
 	if len(z.TransferFrom) == 0 {
 		return nil
 	}
@@ -57,6 +59,13 @@ Transfer:
 	z.Expired = false
 	z.Unlock()
 	log.Infof("Transferred: %s from %s", z.origin, tr)
+
+	// Send notify messages to secondary servers
+	if t != nil {
+		if err := t.Notify(z.origin); err != nil {
+			log.Warningf("Failed sending notifies: %s", err)
+		}
+	}
 	return nil
 }
 
@@ -89,10 +98,11 @@ Transfer:
 	if serial == -1 {
 		return false, Err
 	}
-	if z.SOA == nil {
+	if !z.hasSOA() {
 		return true, Err
 	}
-	return less(z.SOA.Serial, uint32(serial)), Err // #nosec G115 -- serial fits in uint32 per DNS RFC
+	soa := z.getSOA()
+	return less(soa.Serial, uint32(serial)), Err // #nosec G115 -- serial fits in uint32 per DNS RFC
 }
 
 // less returns true of a is smaller than b when taking RFC 1982 serial arithmetic into account.
@@ -107,17 +117,18 @@ func less(a, b uint32) bool {
 // and uses the SOA parameters. Every refresh it will check for a new SOA number. If that fails (for all
 // server) it will retry every retry interval. If the zone failed to transfer before the expire, the zone
 // will be marked expired.
-func (z *Zone) Update(updateShutdown chan bool) error {
+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.SOA == nil {
+	for !z.hasSOA() {
 		time.Sleep(1 * time.Second)
 	}
 	retryActive := false
 
 Restart:
-	refresh := time.Second * time.Duration(z.SOA.Refresh)
+	soa := z.getSOA()
-	retry := time.Second * time.Duration(z.SOA.Retry)
+	refresh := time.Second * time.Duration(soa.Refresh)
-	expire := time.Second * time.Duration(z.SOA.Expire)
+	retry := time.Second * time.Duration(soa.Retry)
+	expire := time.Second * time.Duration(soa.Expire)
 
 	refreshTicker := time.NewTicker(refresh)
 	retryTicker := time.NewTicker(retry)
@@ -145,7 +156,7 @@ Restart:
 			}
 
 			if ok {
-				if err := z.TransferIn(); err != nil {
+				if err := z.TransferIn(t); err != nil {
 					// transfer failed, leave retryActive true
 					break
 				}
@@ -170,7 +181,7 @@ Restart:
 			}
 
 			if ok {
-				if err := z.TransferIn(); err != nil {
+				if err := z.TransferIn(t); err != nil {
 					// transfer failed
 					retryActive = true
 					break

plugin/file/secondary_test.go

@@ -113,7 +113,7 @@ func TestTransferIn(t *testing.T) {
 	z.origin = testZone
 	z.TransferFrom = []string{s.Addr}
 
-	if err := z.TransferIn(); err != nil {
+	if err := z.TransferIn(nil); err != nil {
 		t.Fatalf("Unable to run TransferIn: %v", err)
 	}
 	if z.SOA.String() != fmt.Sprintf("%s	3600	IN	SOA	bla. bla. 250 0 0 0 0", testZone) {

plugin/file/setup.go

@@ -29,10 +29,10 @@ func setup(c *caddy.Controller) error {
 		if t == nil {
 			return nil
 		}
-		f.transfer = t.(*transfer.Transfer) // if found this must be OK.
+		f.Xfer = t.(*transfer.Transfer) // if found this must be OK.
 		go func() {
 			for _, n := range zones.Names {
-				f.transfer.Notify(n)
+				f.Xfer.Notify(n)
 			}
 		}()
 		return nil
@@ -45,7 +45,7 @@ func setup(c *caddy.Controller) error {
 		}
 		go func() {
 			for _, n := range zones.Names {
-				f.transfer.Notify(n)
+				f.Xfer.Notify(n)
 			}
 		}()
 		return nil
@@ -55,7 +55,7 @@ func setup(c *caddy.Controller) error {
 		z := zones.Z[n]
 		c.OnShutdown(z.OnShutdown)
 		c.OnStartup(func() error {
-			z.StartupOnce.Do(func() { z.Reload(f.transfer) })
+			z.StartupOnce.Do(func() { z.Reload(f.Xfer) })
 			return nil
 		})
 	}

plugin/file/zone.go

@@ -182,3 +182,15 @@ 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()
+	return z.SOA
+}

plugin/secondary/setup.go

@@ -10,6 +10,7 @@ import (
 	clog "github.com/coredns/coredns/plugin/pkg/log"
 	"github.com/coredns/coredns/plugin/pkg/parse"
 	"github.com/coredns/coredns/plugin/pkg/upstream"
+	"github.com/coredns/coredns/plugin/transfer"
 )
 
 var log = clog.NewWithPlugin("secondary")
@@ -22,6 +23,17 @@ func setup(c *caddy.Controller) error {
 		return plugin.Error("secondary", err)
 	}
 
+	s := &Secondary{file.File{Zones: zones}}
+	var x *transfer.Transfer
+	c.OnStartup(func() error {
+		t := dnsserver.GetConfig(c).Handler("transfer")
+		if t != nil {
+			x = t.(*transfer.Transfer)
+			s.Xfer = x // if found this must be OK.
+		}
+		return nil
+	})
+
 	// Add startup functions to retrieve the zone and keep it up to date.
 	for i := range zones.Names {
 		n := zones.Names[i]
@@ -36,7 +48,7 @@ func setup(c *caddy.Controller) error {
 						dur := time.Millisecond * 250
 						max := time.Second * 10
 						for {
-							err := z.TransferIn()
+							err := z.TransferIn(x)
 							if err == nil {
 								break
 							}
@@ -52,7 +64,7 @@ func setup(c *caddy.Controller) error {
 							default:
 							}
 						}
-						z.Update(updateShutdown)
+						z.Update(updateShutdown, x)
 					}()
 				})
 				return nil
@@ -65,7 +77,8 @@ func setup(c *caddy.Controller) error {
 	}
 
 	dnsserver.GetConfig(c).AddPlugin(func(next plugin.Handler) plugin.Handler {
-		return Secondary{file.File{Next: next, Zones: zones}}
+		s.Next = next
+		return s
 	})
 
 	return nil

test/secondary_test.go

@@ -1,6 +1,7 @@
 package test
 
 import (
+	"os"
 	"testing"
 	"time"
 
@@ -215,3 +216,116 @@ www    IN AAAA ::1
 		}
 	}
 }
+
+func TestSecondaryZoneNotify(t *testing.T) {
+	// Now spin up the master server
+	name, rm, err := test.TempFile(".", `$ORIGIN example.org.
+@ 3600 IN SOA  sns.dns.icann.org. noc.dns.icann.org. (
+        2017042745 ; serial
+        7200       ; refresh (2 hours)
+        3600       ; retry (1 hour)
+        1209600    ; expire (2 weeks)
+        3600       ; minimum (1 hour)
+)
+
+  3600 IN NS   a.iana-servers.net.
+  3600 IN NS   b.iana-servers.net.
+`)
+	if err != nil {
+		t.Fatalf("Failed to create zone: %s", err)
+	}
+	defer rm()
+
+	corefileMaster := `example.org:53553 {
+    bind 127.0.0.1
+	file ` + name + ` {
+		reload 0.01s
+	}
+	transfer {
+		to 127.0.0.1:53554
+	}
+}`
+
+	master, _, _, err := CoreDNSServerAndPorts(corefileMaster)
+	if err != nil {
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
+	}
+	defer master.Stop()
+
+	corefileSecondary := `example.org:53554 {
+		bind 127.0.0.1
+		secondary {
+			transfer from 127.0.0.1:53553 
+		}
+		transfer {
+			to 127.0.0.1:53555
+		}
+	}`
+	secondary, _, _, err := CoreDNSServerAndPorts(corefileSecondary)
+	if err != nil {
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
+	}
+	defer secondary.Stop()
+
+	corefile := `example.org:53555 {
+        bind 127.0.0.1
+		secondary {
+			transfer from 127.0.0.1:53554
+		}
+	}`
+
+	svr, udp, _, err := CoreDNSServerAndPorts(corefile)
+	if err != nil {
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
+	}
+	defer svr.Stop()
+
+	m := new(dns.Msg)
+	m.SetQuestion("example.org.", dns.TypeSOA)
+
+	var r *dns.Msg
+	// This is now async; we need to wait for it to be transferred.
+	for range 10 {
+		r, _ = dns.Exchange(m, udp)
+		if len(r.Answer) != 0 {
+			break
+		}
+		time.Sleep(1000 * time.Microsecond)
+	}
+	if len(r.Answer) == 0 {
+		t.Fatalf("Expected answer section")
+	}
+
+	m = new(dns.Msg)
+	m.SetQuestion("www.example.org.", dns.TypeA)
+	r, _ = dns.Exchange(m, udp)
+	if len(r.Answer) != 0 {
+		t.Fatalf("Expected no answer section, got %d answers", len(r.Answer))
+	}
+
+	os.WriteFile(name, []byte(`$ORIGIN example.org.
+@ 3600 IN SOA  sns.dns.icann.org. noc.dns.icann.org. (
+        2017042746 ; serial
+        7200       ; refresh (2 hours)
+        3600       ; retry (1 hour)
+        1209600    ; expire (2 weeks)
+        3600       ; minimum (1 hour)
+)
+
+  3600 IN NS   a.iana-servers.net.
+  3600 IN NS   b.iana-servers.net.
+www    IN A    127.0.0.1
+`), 0644)
+
+	// This is now async; we need to wait for it to be transferred.
+	for range 10 {
+		r, _ = dns.Exchange(m, udp)
+		if len(r.Answer) != 0 {
+			break
+		}
+		time.Sleep(1000 * time.Microsecond)
+	}
+	if len(r.Answer) != 1 {
+		t.Fatalf("Expected one RR in answer section got %d", len(r.Answer))
+	}
+}