Cherry-pick: Implement notifies for transfer plugin (#3972) (#4142)

* Implement notifies for transfer plugin (#3972)

* Fix notifies in transfer plugin

Signed-off-by: Miek Gieben <miek@miek.nl>

* Make it compile

Signed-off-by: Miek Gieben <miek@miek.nl>

* Port more plugins

Signed-off-by: Miek Gieben <miek@miek.nl>

* golint

Signed-off-by: Miek Gieben <miek@miek.nl>

* Fix tests

Signed-off-by: Miek Gieben <miek@miek.nl>

* Fix notifies in transfer plugin

Signed-off-by: Miek Gieben <miek@miek.nl>

* Make it compile

Signed-off-by: Miek Gieben <miek@miek.nl>

* Port more plugins

Signed-off-by: Miek Gieben <miek@miek.nl>

* golint

Signed-off-by: Miek Gieben <miek@miek.nl>

* Fix tests

Signed-off-by: Miek Gieben <miek@miek.nl>

* Fix tests

Signed-off-by: Miek Gieben <miek@miek.nl>

* really fix test

Signed-off-by: Miek Gieben <miek@miek.nl>

* Implement ixfr fallback and unify file and auto for transfering

Signed-off-by: Miek Gieben <miek@miek.nl>

* Add transfer tests

copied and modified from #3452

Signed-off-by: Miek Gieben <miek@miek.nl>

* Test correct selection of plugin

Signed-off-by: Miek Gieben <miek@miek.nl>

* add upstream back in

Signed-off-by: Miek Gieben <miek@miek.nl>

* Implement ixfr fallback and unify file and auto for transfering

Signed-off-by: Miek Gieben <miek@miek.nl>

* fix test

Signed-off-by: Miek Gieben <miek@miek.nl>

* properly merge

Signed-off-by: Miek Gieben <miek@miek.nl>

* Remove plugin/kubernetes/setup_transfer_test.go

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

Co-authored-by: Miek Gieben <miek@miek.nl>

plugin/file/README.md

@@ -26,19 +26,16 @@ If you want to round-robin A and AAAA responses look at the *loadbalance* plugin
 
 ~~~
 file DBFILE [ZONES... ] {
-    transfer to ADDRESS...
     reload DURATION
 }
 ~~~
 
-* `transfer` enables zone transfers. It may be specified multiples times. `To` or `from` signals
-  the direction. **ADDRESS** must be denoted in CIDR notation (e.g., 127.0.0.1/32) or just as plain
-  addresses. The special wildcard `*` means: the entire internet (only valid for 'transfer to').
-  When an address is specified a notify message will be sent whenever the zone is reloaded.
 * `reload` interval to perform a reload of the zone if the SOA version changes. Default is one minute.
   Value of `0` means to not scan for changes and reload. For example, `30s` checks the zonefile every 30 seconds
   and reloads the zone when serial changes.
 
+If you need outgoing zone transfers, take a look at the *transfer* plugin.
+
 ## Examples
 
 Load the `example.org` zone from `example.org.signed` and allow transfers to the internet, but send
@@ -46,9 +43,9 @@ notifies to 10.240.1.1
 
 ~~~ corefile
 example.org {
-    file example.org.signed {
-        transfer to *
-        transfer to 10.240.1.1
+    file example.org.signed
+    transfer {
+        to * 10.240.1.1
     }
 }
 ~~~
@@ -57,9 +54,9 @@ Or use a single zone file for multiple zones:
 
 ~~~ corefile
 . {
-    file example.org.signed example.org example.net {
-        transfer to *
-        transfer to 10.240.1.1
+    file example.org.signed example.org example.net
+    transfer example.org example.net {
+        to * 10.240.1.1
     }
 }
 ~~~
@@ -94,4 +91,5 @@ example.org {
 
 ## Also See
 
-See the *loadbalance* plugin if you need simple record shuffling.
+See the *loadbalance* plugin if you need simple record shuffling. And the *transfer* plugin for zone
+transfers. Lastly the *root* plugin can help you specificy the location of the zone files.

plugin/file/file.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/coredns/coredns/plugin"
 	clog "github.com/coredns/coredns/plugin/pkg/log"
+	"github.com/coredns/coredns/plugin/transfer"
 	"github.com/coredns/coredns/request"
 
 	"github.com/miekg/dns"
@@ -20,6 +21,7 @@ type (
 	File struct {
 		Next plugin.Handler
 		Zones
+		transfer *transfer.Transfer
 	}
 
 	// Zones maps zone names to a *Zone.
@@ -77,11 +79,6 @@ func (f File) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
 		return dns.RcodeServerFailure, nil
 	}
 
-	if state.QType() == dns.TypeAXFR || state.QType() == dns.TypeIXFR {
-		xfr := Xfr{z}
-		return xfr.ServeDNS(ctx, w, r)
-	}
-
 	answer, ns, extra, result := z.Lookup(ctx, state, qname)
 
 	m := new(dns.Msg)

plugin/file/notify.go

@@ -1,10 +1,8 @@
 package file
 
 import (
-	"fmt"
 	"net"
 
-	"github.com/coredns/coredns/plugin/pkg/rcode"
 	"github.com/coredns/coredns/request"
 
 	"github.com/miekg/dns"
@@ -33,48 +31,3 @@ func (z *Zone) isNotify(state request.Request) bool {
 	}
 	return false
 }
-
-// Notify will send notifies to all configured TransferTo IP addresses.
-func (z *Zone) Notify() {
-	go notify(z.origin, z.TransferTo)
-}
-
-// notify sends notifies to the configured remote servers. It will try up to three times
-// before giving up on a specific remote. We will sequentially loop through "to"
-// until they all have replied (or have 3 failed attempts).
-func notify(zone string, to []string) error {
-	m := new(dns.Msg)
-	m.SetNotify(zone)
-	c := new(dns.Client)
-
-	for _, t := range to {
-		if t == "*" {
-			continue
-		}
-		if err := notifyAddr(c, m, t); err != nil {
-			log.Error(err.Error())
-		}
-	}
-	log.Infof("Sent notifies for zone %q to %v", zone, to)
-	return nil
-}
-
-func notifyAddr(c *dns.Client, m *dns.Msg, s string) error {
-	var err error
-
-	code := dns.RcodeServerFailure
-	for i := 0; i < 3; i++ {
-		ret, _, err := c.Exchange(m, s)
-		if err != nil {
-			continue
-		}
-		code = ret.Rcode
-		if code == dns.RcodeSuccess {
-			return nil
-		}
-	}
-	if err != nil {
-		return fmt.Errorf("notify for zone %q was not accepted by %q: %q", m.Question[0].Name, s, err)
-	}
-	return fmt.Errorf("notify for zone %q was not accepted by %q: rcode was %q", m.Question[0].Name, s, rcode.ToString(code))
-}

plugin/file/reload.go

@@ -3,10 +3,12 @@ package file
 import (
 	"os"
 	"time"
+
+	"github.com/coredns/coredns/plugin/transfer"
 )
 
 // Reload reloads a zone when it is changed on disk. If z.NoReload is true, no reloading will be done.
-func (z *Zone) Reload() error {
+func (z *Zone) Reload(t *transfer.Transfer) error {
 	if z.ReloadInterval == 0 {
 		return nil
 	}
@@ -40,7 +42,11 @@ func (z *Zone) Reload() error {
 				z.Unlock()
 
 				log.Infof("Successfully reloaded zone %q in %q with %d SOA serial", z.origin, zFile, z.Apex.SOA.Serial)
-				z.Notify()
+				if t != nil {
+					if err := t.Notify(z.origin); err != nil {
+						log.Warningf("Failed sending notifies: %s", err)
+					}
+				}
 
 			case <-z.reloadShutdown:
 				tick.Stop()

plugin/file/reload_test.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/coredns/coredns/plugin/test"
+	"github.com/coredns/coredns/plugin/transfer"
 	"github.com/coredns/coredns/request"
 
 	"github.com/miekg/dns"
@@ -30,7 +31,7 @@ func TestZoneReload(t *testing.T) {
 	}
 
 	z.ReloadInterval = 500 * time.Millisecond
-	z.Reload()
+	z.Reload(&transfer.Transfer{})
 	time.Sleep(time.Second)
 
 	ctx := context.TODO()

plugin/file/secondary_test.go

@@ -11,10 +11,6 @@ import (
 	"github.com/miekg/dns"
 )
 
-// TODO(miek): should test notifies as well, ie start test server (a real coredns one)...
-// setup other test server that sends notify, see if CoreDNS comes calling for a zone
-// transfer
-
 func TestLess(t *testing.T) {
 	const (
 		min  = 0

plugin/file/setup.go

@@ -8,8 +8,8 @@ import (
 	"github.com/coredns/caddy"
 	"github.com/coredns/coredns/core/dnsserver"
 	"github.com/coredns/coredns/plugin"
-	"github.com/coredns/coredns/plugin/pkg/parse"
 	"github.com/coredns/coredns/plugin/pkg/upstream"
+	"github.com/coredns/coredns/plugin/transfer"
 )
 
 func init() { plugin.Register("file", setup) }
@@ -20,26 +20,43 @@ func setup(c *caddy.Controller) error {
 		return plugin.Error("file", err)
 	}
 
-	// Add startup functions to notify the master(s).
-	for _, n := range zones.Names {
-		z := zones.Z[n]
-		c.OnStartup(func() error {
-			z.StartupOnce.Do(func() {
-				if len(z.TransferTo) > 0 {
-					z.Notify()
-				}
-				z.Reload()
-			})
+	f := File{Zones: zones}
+	// get the transfer plugin, so we can send notifies and send notifies on startup as well.
+	c.OnStartup(func() error {
+		t := dnsserver.GetConfig(c).Handler("transfer")
+		if t == nil {
 			return nil
-		})
-	}
+		}
+		f.transfer = t.(*transfer.Transfer) // if found this must be OK.
+		for _, n := range zones.Names {
+			f.transfer.Notify(n)
+		}
+		return nil
+	})
+
+	c.OnRestartFailed(func() error {
+		t := dnsserver.GetConfig(c).Handler("transfer")
+		if t == nil {
+			return nil
+		}
+		for _, n := range zones.Names {
+			f.transfer.Notify(n)
+		}
+		return nil
+	})
+
 	for _, n := range zones.Names {
 		z := zones.Z[n]
 		c.OnShutdown(z.OnShutdown)
+		c.OnStartup(func() error {
+			z.StartupOnce.Do(func() { z.Reload(f.transfer) })
+			return nil
+		})
 	}
 
 	dnsserver.GetConfig(c).AddPlugin(func(next plugin.Handler) plugin.Handler {
-		return File{Next: next, Zones: zones}
+		f.Next = next
+		return f
 	})
 
 	return nil
@@ -92,24 +109,14 @@ func fileParse(c *caddy.Controller) (Zones, error) {
 			names = append(names, origins[i])
 		}
 
-		t := []string{}
-		var e error
-
 		for c.NextBlock() {
 			switch c.Val() {
-			case "transfer":
-				t, _, e = parse.Transfer(c, false)
-				if e != nil {
-					return Zones{}, e
-				}
-
 			case "reload":
 				d, err := time.ParseDuration(c.RemainingArgs()[0])
 				if err != nil {
 					return Zones{}, plugin.Error("file", err)
 				}
 				reload = d
-
 			case "upstream":
 				// remove soon
 				c.RemainingArgs()
@@ -117,12 +124,6 @@ func fileParse(c *caddy.Controller) (Zones, error) {
 			default:
 				return Zones{}, c.Errf("unknown property '%s'", c.Val())
 			}
-
-			for _, origin := range origins {
-				if t != nil {
-					z[origin].TransferTo = append(z[origin].TransferTo, t...)
-				}
-			}
 		}
 	}
 

plugin/file/xfr.go

@@ -1,118 +1,45 @@
 package file
 
 import (
-	"context"
-	"fmt"
-	"sync"
-
-	"github.com/coredns/coredns/plugin"
 	"github.com/coredns/coredns/plugin/file/tree"
-	"github.com/coredns/coredns/request"
+	"github.com/coredns/coredns/plugin/transfer"
 
 	"github.com/miekg/dns"
 )
 
-// Xfr serves up an AXFR.
-type Xfr struct {
-	*Zone
+// Transfer implements the transfer.Transfer interface.
+func (f File) Transfer(zone string, serial uint32) (<-chan []dns.RR, error) {
+	z, ok := f.Zones.Z[zone]
+	if !ok || z == nil {
+		return nil, transfer.ErrNotAuthoritative
+	}
+	return z.Transfer(serial)
 }
 
-// ServeDNS implements the plugin.Handler interface.
-func (x Xfr) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
-	state := request.Request{W: w, Req: r}
-	if !x.TransferAllowed(state) {
-		return dns.RcodeServerFailure, nil
-	}
-	if state.QType() != dns.TypeAXFR && state.QType() != dns.TypeIXFR {
-		return 0, plugin.Error(x.Name(), fmt.Errorf("xfr called with non transfer type: %d", state.QType()))
-	}
-
-	// For IXFR we take the SOA in the IXFR message (if there), compare it what we have and then decide to do an
-	// AXFR or just reply with one SOA message back.
-	if state.QType() == dns.TypeIXFR {
-		code, _ := x.ServeIxfr(ctx, w, r)
-		if plugin.ClientWrite(code) {
-			return code, nil
-		}
-	}
-
+// 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) {
 	// get soa and apex
-	apex, err := x.ApexIfDefined()
+	apex, err := z.ApexIfDefined()
 	if err != nil {
-		return dns.RcodeServerFailure, nil
+		return nil, err
 	}
 
-	ch := make(chan *dns.Envelope)
-	tr := new(dns.Transfer)
-	wg := new(sync.WaitGroup)
-	wg.Add(1)
+	ch := make(chan []dns.RR)
 	go func() {
-		tr.Out(w, r, ch)
-		wg.Done()
+		if serial != 0 && apex[0].(*dns.SOA).Serial == serial { // ixfr fallback, only send SOA
+			ch <- []dns.RR{apex[0]}
+
+			close(ch)
+			return
+		}
+
+		ch <- apex
+		z.Walk(func(e *tree.Elem, _ map[uint16][]dns.RR) error { ch <- e.All(); return nil })
+		ch <- []dns.RR{apex[0]}
+
+		close(ch)
 	}()
 
-	rrs := []dns.RR{}
-	l := len(apex)
-
-	ch <- &dns.Envelope{RR: apex}
-
-	x.Walk(func(e *tree.Elem, _ map[uint16][]dns.RR) error {
-		rrs = append(rrs, e.All()...)
-		if len(rrs) > 500 {
-			ch <- &dns.Envelope{RR: rrs}
-			l += len(rrs)
-			rrs = []dns.RR{}
-		}
-		return nil
-	})
-
-	if len(rrs) > 0 {
-		ch <- &dns.Envelope{RR: rrs}
-		l += len(rrs)
-		rrs = []dns.RR{}
-	}
-
-	ch <- &dns.Envelope{RR: []dns.RR{apex[0]}} // closing SOA.
-	l++
-
-	close(ch) // Even though we close the channel here, we still have
-	wg.Wait() // to wait before we can return and close the connection.
-
-	log.Infof("Outgoing transfer of %d records of zone %s to %s done with %d SOA serial", l, x.origin, state.IP(), apex[0].(*dns.SOA).Serial)
-	return dns.RcodeSuccess, nil
-}
-
-// Name implements the plugin.Handler interface.
-func (x Xfr) Name() string { return "xfr" }
-
-// ServeIxfr checks if we need to serve a simpler IXFR for the incoming message.
-// See RFC 1995 Section 3: "... and the authority section containing the SOA record of client's version of the zone."
-// and Section 2, paragraph 4 where we only need to echo the SOA record back.
-// This function must be called when the qtype is IXFR. It returns a plugin.ClientWrite(code) == false, when it didn't
-// write anything and we should perform an AXFR.
-func (x Xfr) ServeIxfr(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
-	if len(r.Ns) != 1 {
-		return dns.RcodeServerFailure, nil
-	}
-	soa, ok := r.Ns[0].(*dns.SOA)
-	if !ok {
-		return dns.RcodeServerFailure, nil
-	}
-
-	x.RLock()
-	if x.Apex.SOA == nil {
-		x.RUnlock()
-		return dns.RcodeServerFailure, nil
-	}
-	serial := x.Apex.SOA.Serial
-	x.RUnlock()
-
-	if soa.Serial == serial { // Section 2, para 4; echo SOA back. We have the same zone
-		m := new(dns.Msg)
-		m.SetReply(r)
-		m.Answer = []dns.RR{soa}
-		w.WriteMsg(m)
-		return 0, nil
-	}
-	return dns.RcodeServerFailure, nil
+	return ch, nil
 }

plugin/file/zone.go

@@ -2,7 +2,6 @@ package file
 
 import (
 	"fmt"
-	"net"
 	"path/filepath"
 	"strings"
 	"sync"
@@ -10,7 +9,6 @@ import (
 
 	"github.com/coredns/coredns/plugin/file/tree"
 	"github.com/coredns/coredns/plugin/pkg/upstream"
-	"github.com/coredns/coredns/request"
 
 	"github.com/miekg/dns"
 )
@@ -26,7 +24,6 @@ type Zone struct {
 
 	sync.RWMutex
 
-	TransferTo   []string
 	StartupOnce  sync.Once
 	TransferFrom []string
 
@@ -58,7 +55,6 @@ func NewZone(name, file string) *Zone {
 // Copy copies a zone.
 func (z *Zone) Copy() *Zone {
 	z1 := NewZone(z.origin, z.file)
-	z1.TransferTo = z.TransferTo
 	z1.TransferFrom = z.TransferFrom
 	z1.Expired = z.Expired
 
@@ -69,7 +65,6 @@ func (z *Zone) Copy() *Zone {
 // CopyWithoutApex copies zone z without the Apex records.
 func (z *Zone) CopyWithoutApex() *Zone {
 	z1 := NewZone(z.origin, z.file)
-	z1.TransferTo = z.TransferTo
 	z1.TransferFrom = z.TransferFrom
 	z1.Expired = z.Expired
 
@@ -134,26 +129,6 @@ func (z *Zone) SetFile(path string) {
 	z.Unlock()
 }
 
-// TransferAllowed checks if incoming request for transferring the zone is allowed according to the ACLs.
-func (z *Zone) TransferAllowed(state request.Request) bool {
-	for _, t := range z.TransferTo {
-		if t == "*" {
-			return true
-		}
-		// If remote IP matches we accept.
-		remote := state.IP()
-		to, _, err := net.SplitHostPort(t)
-		if err != nil {
-			continue
-		}
-		if to == remote {
-			return true
-		}
-	}
-	// TODO(miek): future matching against IP/CIDR notations
-	return false
-}
-
 // 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) {
 	z.RLock()