From 225cdd1ca387b409c07269db44100643fccb620c Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Wed, 6 Apr 2016 22:29:33 +0100 Subject: [PATCH] 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 | 11 ++- middleware/file/notify.go | 1 - middleware/file/secondary.go | 57 +++++++++---- middleware/file/secondary_test.go | 128 ++++++++++++++++++++++++++++++ middleware/file/zone_test.go | 1 + middleware/prometheus/README.md | 3 + server/server.go | 12 +-- 7 files changed, 188 insertions(+), 25 deletions(-) create mode 100644 middleware/file/secondary_test.go diff --git a/middleware/file/file.go b/middleware/file/file.go index 7ec690f3c..fcb1ce6e2 100644 --- a/middleware/file/file.go +++ b/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) diff --git a/middleware/file/notify.go b/middleware/file/notify.go index a88ca9192..b369f6ad1 100644 --- a/middleware/file/notify.go +++ b/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 } diff --git a/middleware/file/secondary.go b/middleware/file/secondary.go index dbea7ea3d..057d6a4f3 100644 --- a/middleware/file/secondary.go +++ b/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 } } } - - refreshTicker.Stop() - retryTicker.Stop() - expireTicker.Stop() 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 diff --git a/middleware/file/secondary_test.go b/middleware/file/secondary_test.go new file mode 100644 index 000000000..3533df042 --- /dev/null +++ b/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) + } +} diff --git a/middleware/file/zone_test.go b/middleware/file/zone_test.go index 5421d1dd3..e7eaef4b8 100644 --- a/middleware/file/zone_test.go +++ b/middleware/file/zone_test.go @@ -1,3 +1,4 @@ package file // TODO tests here. +// see secondary_test.go for some infrastructure stuff. diff --git a/middleware/prometheus/README.md b/middleware/prometheus/README.md index 3b16b3b94..919050e7b 100644 --- a/middleware/prometheus/README.md +++ b/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). diff --git a/server/server.go b/server/server.go index 6f821d62d..67cc35ba5 100644 --- a/server/server.go +++ b/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) }