Add AXFR test

Test shouldTransfer by upping a testserver and sending the SOA query.

Remove state from DefaultErrorHandler and just get it from the request.

Add more logging to show what is going on.



This also adds the infrastructure for future tests.

middleware/file/file.go

@@ -47,11 +47,16 @@ func (f File) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
 			m.Authoritative, m.RecursionAvailable, m.Compress = true, true, true
 			w.WriteMsg(m)
 
-			if ok, _ := z.shouldTransfer(); ok {
-				log.Printf("[INFO] Valid notify from %s for %s: initiating transfer", state.IP(), zone)
+			log.Printf("[INFO] Notify from %s for %s: checking transfer", state.IP(), zone)
+			ok, err := z.shouldTransfer()
+			if ok {
 				z.TransferIn()
+			} else {
+				log.Printf("[INFO] Notify from %s for %s: no serial increase seen", state.IP(), zone)
+			}
+			if err != nil {
+				log.Printf("[WARNING] Notify from %s for %s: failed primary check: %s", state.IP(), zone, err)
 			}
-
 			return dns.RcodeSuccess, nil
 		}
 		log.Printf("[INFO] Dropping notify from %s for %s", state.IP(), zone)

middleware/file/notify.go

@@ -42,7 +42,6 @@ func notify(zone string, to []string) error {
 	c := new(dns.Client)
 
 	for _, t := range to {
-		// TODO(miek): these ACLs thingies not to be formalized.
 		if t == "*" {
 			continue
 		}

middleware/file/secondary.go

@@ -57,6 +57,8 @@ Transfer:
 	}
 
 	z.Tree = z1.Tree
+	z.SOA = z1.SOA
+	z.SIG = z1.SIG
 	*z.Expired = false
 	log.Printf("[INFO] Transferred: %s", z.name)
 	return nil
@@ -73,6 +75,7 @@ func (z *Zone) shouldTransfer() (bool, error) {
 	var Err error
 	serial := -1
 
+Transfer:
 	for _, tr := range z.TransferFrom {
 		Err = nil
 		ret, err := middleware.Exchange(c, m, tr)
@@ -83,6 +86,7 @@ func (z *Zone) shouldTransfer() (bool, error) {
 		for _, a := range ret.Answer {
 			if a.Header().Rrtype == dns.TypeSOA {
 				serial = int(a.(*dns.SOA).Serial)
+				break Transfer
 			}
 		}
 	}
@@ -94,8 +98,10 @@ func (z *Zone) shouldTransfer() (bool, error) {
 
 // less return true of a is smaller than b when taking RFC 1982 serial arithmetic into account.
 func less(a, b uint32) bool {
-	// TODO(miek): implement!
-	return a < b
+	if a < b {
+		return (b - a) <= MaxSerialIncrement
+	}
+	return (a - b) > MaxSerialIncrement
 }
 
 // Update updates the secondary zone according to its SOA. It will run for the life time of the server
@@ -103,24 +109,29 @@ func less(a, b uint32) bool {
 // server) it wil retry every retry interval. If the zone failed to transfer before the expire, the zone
 // will be marked expired.
 func (z *Zone) Update() error {
-	// TODO(miek): if SOA changes we need to redo this with possible different timer values.
-	// TODO(miek): yeah...
+	// If we don't have a SOA, we don't have a zone, wait for it to appear.
 	for z.SOA == nil {
 		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)
-	retryActive := false
 
-	// TODO(miek): check max as well?
 	if refresh < time.Hour {
 		refresh = time.Hour
 	}
 	if retry < time.Hour {
 		retry = time.Hour
 	}
+	if refresh > 24*time.Hour {
+		refresh = 24 * time.Hour
+	}
+	if retry > 12*time.Hour {
+		retry = 12 * time.Hour
+	}
 
 	refreshTicker := time.NewTicker(refresh)
 	retryTicker := time.NewTicker(retry)
@@ -132,7 +143,6 @@ func (z *Zone) Update() error {
 			if !retryActive {
 				break
 			}
-			// TODO(miek): should actually keep track of last succesfull transfer
 			*z.Expired = true
 
 		case <-retryTicker.C:
@@ -141,23 +151,40 @@ func (z *Zone) Update() error {
 			}
 			ok, err := z.shouldTransfer()
 			if err != nil && ok {
-				log.Printf("[INFO] Refreshing zone: %s: initiating transfer", z.name)
-				z.TransferIn()
+				if err := z.TransferIn(); err != nil {
+					// transfer failed, leave retryActive true
+					break
+				}
 				retryActive = false
+				// transfer OK, possible new SOA, stop timers and redo
+				refreshTicker.Stop()
+				retryTicker.Stop()
+				expireTicker.Stop()
+				goto Restart
 			}
 
 		case <-refreshTicker.C:
 			ok, err := z.shouldTransfer()
 			retryActive = err != nil
 			if err != nil && ok {
-				log.Printf("[INFO] Refreshing zone: %s: initiating transfer", z.name)
-				z.TransferIn()
+				if err := z.TransferIn(); err != nil {
+					// transfer failed
+					retryActive = true
+					break
 				}
-		}
-	}
-
+				retryActive = false
+				// transfer OK, possible new SOA, stop timers and redo
 				refreshTicker.Stop()
 				retryTicker.Stop()
 				expireTicker.Stop()
+				goto Restart
+			}
+		}
+	}
 	return nil
 }
+
+// The maximum difference between two serial numbers. If the difference between
+// two serials is greater than this number, the smaller one is considered
+// greater.
+const MaxSerialIncrement uint32 = 2147483647

middleware/file/secondary_test.go

@@ -0,0 +1,128 @@
+package file
+
+import (
+	"net"
+	"sync"
+	"testing"
+	"time"
+
+	"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
+// tranfer
+
+func TestLess(t *testing.T) {
+	const (
+		min  = 0
+		max  = 4294967295
+		low  = 12345
+		high = 4000000000
+	)
+
+	if less(min, max) {
+		t.Fatalf("less: should be false")
+	}
+	if !less(max, min) {
+		t.Fatalf("less: should be true")
+	}
+	if !less(high, low) {
+		t.Fatalf("less: should be true")
+	}
+	if !less(7, 9) {
+		t.Fatalf("less; should be true")
+	}
+}
+
+func TCPServer(laddr string) (*dns.Server, string, error) {
+	l, err := net.Listen("tcp", laddr)
+	if err != nil {
+		return nil, "", err
+	}
+
+	server := &dns.Server{Listener: l, ReadTimeout: time.Hour, WriteTimeout: time.Hour}
+
+	waitLock := sync.Mutex{}
+	waitLock.Lock()
+	server.NotifyStartedFunc = waitLock.Unlock
+
+	go func() {
+		server.ActivateAndServe()
+		l.Close()
+	}()
+
+	waitLock.Lock()
+	return server, l.Addr().String(), nil
+}
+
+func UDPServer(laddr string) (*dns.Server, string, chan bool, error) {
+	pc, err := net.ListenPacket("udp", laddr)
+	if err != nil {
+		return nil, "", nil, err
+	}
+	server := &dns.Server{PacketConn: pc, ReadTimeout: time.Hour, WriteTimeout: time.Hour}
+
+	waitLock := sync.Mutex{}
+	waitLock.Lock()
+	server.NotifyStartedFunc = waitLock.Unlock
+
+	stop := make(chan bool)
+
+	go func() {
+		server.ActivateAndServe()
+		close(stop)
+		pc.Close()
+	}()
+
+	waitLock.Lock()
+	return server, pc.LocalAddr().String(), stop, nil
+}
+
+type soa struct {
+	serial uint32
+}
+
+func (s *soa) Handler(w dns.ResponseWriter, req *dns.Msg) {
+	m := new(dns.Msg)
+	m.SetReply(req)
+	m.Answer = make([]dns.RR, 1)
+	m.Answer[0] = &dns.SOA{Hdr: dns.RR_Header{Name: m.Question[0].Name, Rrtype: dns.TypeSOA, Class: dns.ClassINET, Ttl: 100}, Ns: "bla.", Mbox: "bla.", Serial: s.serial}
+	w.WriteMsg(m)
+}
+
+func TestShouldTransfer(t *testing.T) {
+	soa := soa{250}
+
+	dns.HandleFunc("secondary.miek.nl.", soa.Handler)
+	defer dns.HandleRemove("secondary.miek.nl.")
+
+	s, addrstr, err := TCPServer("127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("unable to run test server: %v", err)
+	}
+	defer s.Shutdown()
+
+	z := new(Zone)
+	z.name = "secondary.miek.nl."
+	z.TransferFrom = []string{addrstr}
+
+	// Serial smaller
+	z.SOA = &dns.SOA{Hdr: dns.RR_Header{Name: "secondary.miek.nl.", Rrtype: dns.TypeSOA, Class: dns.ClassINET, Ttl: 100}, Ns: "bla.", Mbox: "bla.", Serial: soa.serial - 1}
+	should, err := z.shouldTransfer()
+	if err != nil {
+		t.Fatalf("unable to run shouldTransfer: %v", err)
+	}
+	if !should {
+		t.Fatalf("shouldTransfer should return true for serial: %q", soa.serial-1)
+	}
+	// Serial equal
+	z.SOA = &dns.SOA{Hdr: dns.RR_Header{Name: "secondary.miek.nl.", Rrtype: dns.TypeSOA, Class: dns.ClassINET, Ttl: 100}, Ns: "bla.", Mbox: "bla.", Serial: soa.serial}
+	should, err = z.shouldTransfer()
+	if err != nil {
+		t.Fatalf("unable to run shouldTransfer: %v", err)
+	}
+	if should {
+		t.Fatalf("shouldTransfer should return false for serial: %d", soa.serial)
+	}
+}

middleware/file/zone_test.go

@@ -1,3 +1,4 @@
 package file
 
 // TODO tests here.
+// see secondary_test.go for some infrastructure stuff.

middleware/prometheus/README.md

@@ -14,6 +14,9 @@ and a label `qtype` which old the query type.
 The `response_rcode_count_total` has an extra label `rcode` which holds the rcode
 of the response.
 
+If monitoring is enabled queries that do not enter the middleware chain are exported
+under the fake domain "dropped" (without a closing dot).
+
 Restarting CoreDNS will stop the monitoring. This is a bug. Also [this upstream
 Caddy bug](https://github.com/mholt/caddy/issues/675).
 

server/server.go

@@ -16,12 +16,11 @@ import (
 	"sync"
 	"time"
 
-	"golang.org/x/net/context"
-
-	"github.com/miekg/coredns/middleware"
 	"github.com/miekg/coredns/middleware/chaos"
 	"github.com/miekg/coredns/middleware/prometheus"
+
 	"github.com/miekg/dns"
+	"golang.org/x/net/context"
 )
 
 // Server represents an instance of a server, which serves
@@ -332,17 +331,18 @@ func (s *Server) ServeDNS(w dns.ResponseWriter, r *dns.Msg) {
 // DefaultErrorFunc responds to an HTTP request with a simple description
 // of the specified HTTP status code.
 func DefaultErrorFunc(w dns.ResponseWriter, r *dns.Msg, rcode int) {
+	qtype := dns.Type(r.Question[0].Qtype).String()
+
 	// this code is duplicated a few times, TODO(miek)
 	rc := dns.RcodeToString[rcode]
 	if rc == "" {
 		rc = "RCODE" + strconv.Itoa(rcode)
 	}
+
 	answer := new(dns.Msg)
 	answer.SetRcode(r, rcode)
-
-	state := middleware.State{W: w, Req: r}
 	// Default zone to dropped (without closing dot, so no zone) here to not blow up this metric.
-	metrics.Report(dropped, state.Type(), rc, answer.Len(), time.Now())
+	metrics.Report(dropped, qtype, rc, answer.Len(), time.Now())
 
 	w.WriteMsg(answer)
 }