From 2daf48e42d4d3d71dff931b0aea4ac3dae410039 Mon Sep 17 00:00:00 2001 From: liucongran <33312307+crliu3227@users.noreply.github.com> Date: Sun, 8 Mar 2026 13:15:44 +0800 Subject: [PATCH] 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 * fix(secondary): Fix TransferIn method call in test Update test to pass nil parameter to TransferIn method after signature change Signed-off-by: liucongran * 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 * 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 * refactor(secondary): Use getSOA helper method in shouldTransfer Replace direct SOA access with getSOA() helper method for consistency. Signed-off-by: liucongran --------- Signed-off-by: liucongran Co-authored-by: liucongran --- plugin/file/file.go | 4 +- plugin/file/secondary.go | 31 ++++++--- plugin/file/secondary_test.go | 2 +- plugin/file/setup.go | 8 +-- plugin/file/zone.go | 12 ++++ plugin/secondary/setup.go | 19 +++++- test/secondary_test.go | 114 ++++++++++++++++++++++++++++++++++ 7 files changed, 170 insertions(+), 20 deletions(-) diff --git a/plugin/file/file.go b/plugin/file/file.go index bbbfe8279..c3b541e6c 100644 --- a/plugin/file/file.go +++ b/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) } diff --git a/plugin/file/secondary.go b/plugin/file/secondary.go index 9da5f7865..ef85d5b7c 100644 --- a/plugin/file/secondary.go +++ b/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) - retry := time.Second * time.Duration(z.SOA.Retry) - expire := time.Second * time.Duration(z.SOA.Expire) + soa := z.getSOA() + refresh := time.Second * time.Duration(soa.Refresh) + 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 diff --git a/plugin/file/secondary_test.go b/plugin/file/secondary_test.go index 3e36c4b4e..a5bd0819f 100644 --- a/plugin/file/secondary_test.go +++ b/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) { diff --git a/plugin/file/setup.go b/plugin/file/setup.go index eabfcc855..f863ddf84 100644 --- a/plugin/file/setup.go +++ b/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 }) } diff --git a/plugin/file/zone.go b/plugin/file/zone.go index e7afc795b..4cb1a1f6b 100644 --- a/plugin/file/zone.go +++ b/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 +} diff --git a/plugin/secondary/setup.go b/plugin/secondary/setup.go index 112270103..f87e9d949 100644 --- a/plugin/secondary/setup.go +++ b/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 diff --git a/test/secondary_test.go b/test/secondary_test.go index d1fcb2bbd..8f03b1db0 100644 --- a/test/secondary_test.go +++ b/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)) + } +}