From ad76aef5fcf46dcdb50d8529bbf085a1578d3bda Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Mon, 8 Aug 2016 19:18:55 -0700 Subject: [PATCH] Fix stubzone retention (#198) Make the receiver a pointer so that the uptdateStubZones map update will retain the stubzones found, unlike the current case where the update will be applied and then promptly forgotten, because it is working on a copy. Add test/etcd_test.go to test a large part of the code. This didn't catch the chaos middleware hack though. The chaos middleware zones are now *not* automatically added. You have to take care of that by yourself (docs updates). When using debug queries and falling through to the next middleware in etcd, restore the original (with o-o.debug) query before passing it on. --- conf/chaosCorefile | 4 ++ conf/etcdCorefile | 10 ++++ core/setup/etcd.go | 24 ++++---- middleware/chaos/README.md | 8 +-- middleware/etcd/debug.go | 3 +- middleware/etcd/debug_test.go | 10 ++-- middleware/etcd/etcd.go | 29 +++++---- middleware/etcd/handler.go | 46 ++++++++------- middleware/etcd/lookup.go | 50 ++++++++-------- middleware/etcd/multi_test.go | 6 +- middleware/etcd/setup_test.go | 8 +-- middleware/etcd/stub.go | 2 +- middleware/etcd/stub_cycle_test.go | 1 + middleware/etcd/stub_handler.go | 2 +- middleware/etcd/stub_test.go | 1 + middleware/test/helpers.go | 1 + server/server.go | 14 ----- test/etcd_test.go | 95 +++++++++++++++++++++++++++++- 18 files changed, 210 insertions(+), 104 deletions(-) create mode 100644 conf/chaosCorefile create mode 100644 conf/etcdCorefile diff --git a/conf/chaosCorefile b/conf/chaosCorefile new file mode 100644 index 000000000..c7ebd9f80 --- /dev/null +++ b/conf/chaosCorefile @@ -0,0 +1,4 @@ +.:1053 authors.bind:1053 { + chaos CoreDNS-001 "Miek Gieben" miek@miek.nl + proxy . 8.8.8.8:53 +} diff --git a/conf/etcdCorefile b/conf/etcdCorefile new file mode 100644 index 000000000..8133696f1 --- /dev/null +++ b/conf/etcdCorefile @@ -0,0 +1,10 @@ +.:1053 { + etcd skydns.local { + stubzones + path /skydns + endpoint http://localhost:2379 + upstream 8.8.8.8:53 8.8.4.4:53 + debug + } + proxy . 8.8.8.8:53 +} diff --git a/core/setup/etcd.go b/core/setup/etcd.go index 80df2ae4d..2160b540c 100644 --- a/core/setup/etcd.go +++ b/core/setup/etcd.go @@ -38,7 +38,7 @@ func Etcd(c *Controller) (middleware.Middleware, error) { }, nil } -func etcdParse(c *Controller) (etcd.Etcd, bool, error) { +func etcdParse(c *Controller) (*etcd.Etcd, bool, error) { stub := make(map[string]proxy.Proxy) etc := etcd.Etcd{ Proxy: proxy.New([]string{"8.8.8.8:53", "8.8.4.4:53"}), @@ -72,19 +72,19 @@ func etcdParse(c *Controller) (etcd.Etcd, bool, error) { etc.Debug = true case "path": if !c.NextArg() { - return etcd.Etcd{}, false, c.ArgErr() + return &etcd.Etcd{}, false, c.ArgErr() } etc.PathPrefix = c.Val() case "endpoint": args := c.RemainingArgs() if len(args) == 0 { - return etcd.Etcd{}, false, c.ArgErr() + return &etcd.Etcd{}, false, c.ArgErr() } endpoints = args case "upstream": args := c.RemainingArgs() if len(args) == 0 { - return etcd.Etcd{}, false, c.ArgErr() + return &etcd.Etcd{}, false, c.ArgErr() } for i := 0; i < len(args); i++ { h, p, e := net.SplitHostPort(args[i]) @@ -97,7 +97,7 @@ func etcdParse(c *Controller) (etcd.Etcd, bool, error) { case "tls": // cert key cacertfile args := c.RemainingArgs() if len(args) != 3 { - return etcd.Etcd{}, false, c.ArgErr() + return &etcd.Etcd{}, false, c.ArgErr() } tlsCertFile, tlsKeyFile, tlsCAcertFile = args[0], args[1], args[2] } @@ -109,19 +109,19 @@ func etcdParse(c *Controller) (etcd.Etcd, bool, error) { etc.Debug = true case "path": if !c.NextArg() { - return etcd.Etcd{}, false, c.ArgErr() + return &etcd.Etcd{}, false, c.ArgErr() } etc.PathPrefix = c.Val() case "endpoint": args := c.RemainingArgs() if len(args) == 0 { - return etcd.Etcd{}, false, c.ArgErr() + return &etcd.Etcd{}, false, c.ArgErr() } endpoints = args case "upstream": args := c.RemainingArgs() if len(args) == 0 { - return etcd.Etcd{}, false, c.ArgErr() + return &etcd.Etcd{}, false, c.ArgErr() } for i := 0; i < len(args); i++ { h, p, e := net.SplitHostPort(args[i]) @@ -133,7 +133,7 @@ func etcdParse(c *Controller) (etcd.Etcd, bool, error) { case "tls": // cert key cacertfile args := c.RemainingArgs() if len(args) != 3 { - return etcd.Etcd{}, false, c.ArgErr() + return &etcd.Etcd{}, false, c.ArgErr() } tlsCertFile, tlsKeyFile, tlsCAcertFile = args[0], args[1], args[2] } @@ -141,13 +141,13 @@ func etcdParse(c *Controller) (etcd.Etcd, bool, error) { } client, err := newEtcdClient(endpoints, tlsCertFile, tlsKeyFile, tlsCAcertFile) if err != nil { - return etcd.Etcd{}, false, err + return &etcd.Etcd{}, false, err } etc.Client = client - return etc, stubzones, nil + return &etc, stubzones, nil } } - return etcd.Etcd{}, false, nil + return &etcd.Etcd{}, false, nil } func newEtcdClient(endpoints []string, tlsCert, tlsKey, tlsCACert string) (etcdc.KeysAPI, error) { diff --git a/middleware/chaos/README.md b/middleware/chaos/README.md index 1720e918a..82c4ae4dd 100644 --- a/middleware/chaos/README.md +++ b/middleware/chaos/README.md @@ -1,7 +1,7 @@ # chaos The `chaos` middleware allows CoreDNS to response to TXT queries in CH class. -Useful for retrieving version or author information from the server. +Useful for retrieving version or author information from the server. If ## Syntax @@ -12,9 +12,9 @@ chaos [version] [authors...] * `version` the version to return, defaults to CoreDNS. * `authors` what authors to return. No default. -Note this middleware can only be specified for a zone once. This is because it hijacks -the zones `version.bind`, `version.server`, `authors.bind`, `hostname.bind` and -`id.server`, which means it can only be routed to one middleware. +Note that you have to make sure that this middleware will get actual queries for the +following zones: `version.bind`, `version.server`, `authors.bind`, `hostname.bind` and +`id.server`. ## Examples diff --git a/middleware/etcd/debug.go b/middleware/etcd/debug.go index 58e896680..d5dfd6811 100644 --- a/middleware/etcd/debug.go +++ b/middleware/etcd/debug.go @@ -12,11 +12,12 @@ const debugName = "o-o.debug." // isDebug checks if name is a debugging name, i.e. starts with o-o.debug. // it return the empty string if it is not a debug message, otherwise it will return the -// name with o-o.debug. stripped off. +// name with o-o.debug. stripped off. Must be called with name lowercased. func isDebug(name string) string { if len(name) == len(debugName) { return "" } + name = strings.ToLower(name) debug := strings.HasPrefix(name, debugName) if !debug { return "" diff --git a/middleware/etcd/debug_test.go b/middleware/etcd/debug_test.go index d07ffe986..91796816f 100644 --- a/middleware/etcd/debug_test.go +++ b/middleware/etcd/debug_test.go @@ -4,6 +4,7 @@ package etcd import ( "sort" + "strings" "testing" "github.com/miekg/coredns/middleware" @@ -13,17 +14,17 @@ import ( "github.com/miekg/dns" ) -func TestisDebug(t *testing.T) { +func TestIsDebug(t *testing.T) { if ok := isDebug("o-o.debug.miek.nl."); ok != "miek.nl." { t.Errorf("expected o-o.debug.miek.nl. to be debug") } - if ok := isDebug("o-o.Debug.miek.nl."); ok != "miek.nl." { + if ok := isDebug(strings.ToLower("o-o.Debug.miek.nl.")); ok != "miek.nl." { t.Errorf("expected o-o.Debug.miek.nl. to be debug") } - if ok := isDebug("i-o.Debug.miek.nl."); ok != "" { + if ok := isDebug("i-o.debug.miek.nl."); ok != "" { t.Errorf("expected i-o.Debug.miek.nl. to be non-debug") } - if ok := isDebug("i-o.Debug."); ok != "" { + if ok := isDebug(strings.ToLower("i-o.Debug.")); ok != "" { t.Errorf("expected o-o.Debug. to be non-debug") } } @@ -35,6 +36,7 @@ func TestDebugLookup(t *testing.T) { } etc.Debug = true defer func() { etc.Debug = false }() + for _, tc := range dnsTestCasesDebug { m := tc.Msg() diff --git a/middleware/etcd/etcd.go b/middleware/etcd/etcd.go index d903a2a66..63aae2967 100644 --- a/middleware/etcd/etcd.go +++ b/middleware/etcd/etcd.go @@ -26,35 +26,34 @@ type Etcd struct { Inflight *singleflight.Group Stubmap *map[string]proxy.Proxy // List of proxies for stub resolving. Debug bool // Do we allow debug queries. - debug string // Should we return debugging information, if so, contains original qname. } // Records looks up records in etcd. If exact is true, it will lookup just // this name. This is used when find matches when completing SRV lookups // for instance. -func (g Etcd) Records(name string, exact bool) ([]msg.Service, error) { - path, star := msg.PathWithWildcard(name, g.PathPrefix) - r, err := g.Get(path, true) +func (e *Etcd) Records(name string, exact bool) ([]msg.Service, error) { + path, star := msg.PathWithWildcard(name, e.PathPrefix) + r, err := e.Get(path, true) if err != nil { return nil, err } - segments := strings.Split(msg.Path(name, g.PathPrefix), "/") + segments := strings.Split(msg.Path(name, e.PathPrefix), "/") switch { case exact && r.Node.Dir: return nil, nil case r.Node.Dir: - return g.loopNodes(r.Node.Nodes, segments, star, nil) + return e.loopNodes(r.Node.Nodes, segments, star, nil) default: - return g.loopNodes([]*etcdc.Node{r.Node}, segments, false, nil) + return e.loopNodes([]*etcdc.Node{r.Node}, segments, false, nil) } } // Get is a wrapper for client.Get that uses SingleInflight to suppress multiple outstanding queries. -func (g Etcd) Get(path string, recursive bool) (*etcdc.Response, error) { - resp, err := g.Inflight.Do(path, func() (interface{}, error) { - ctx, cancel := context.WithTimeout(g.Ctx, etcdTimeout) +func (e *Etcd) Get(path string, recursive bool) (*etcdc.Response, error) { + resp, err := e.Inflight.Do(path, func() (interface{}, error) { + ctx, cancel := context.WithTimeout(e.Ctx, etcdTimeout) defer cancel() - r, e := g.Client.Get(ctx, path, &etcdc.GetOptions{Sort: false, Recursive: recursive}) + r, e := e.Client.Get(ctx, path, &etcdc.GetOptions{Sort: false, Recursive: recursive}) if e != nil { return nil, e } @@ -74,14 +73,14 @@ func (g Etcd) Get(path string, recursive bool) (*etcdc.Response, error) { // loopNodes recursively loops through the nodes and returns all the values. The nodes' keyname // will be match against any wildcards when star is true. -func (g Etcd) loopNodes(ns []*etcdc.Node, nameParts []string, star bool, bx map[msg.Service]bool) (sx []msg.Service, err error) { +func (e Etcd) loopNodes(ns []*etcdc.Node, nameParts []string, star bool, bx map[msg.Service]bool) (sx []msg.Service, err error) { if bx == nil { bx = make(map[msg.Service]bool) } Nodes: for _, n := range ns { if n.Dir { - nodes, err := g.loopNodes(n.Nodes, nameParts, star, bx) + nodes, err := e.loopNodes(n.Nodes, nameParts, star, bx) if err != nil { return nil, err } @@ -114,7 +113,7 @@ Nodes: bx[b] = true serv.Key = n.Key - serv.Ttl = g.TTL(n, serv) + serv.Ttl = e.TTL(n, serv) if serv.Priority == 0 { serv.Priority = priority } @@ -125,7 +124,7 @@ Nodes: // TTL returns the smaller of the etcd TTL and the service's // TTL. If neither of these are set (have a zero value), a default is used. -func (g Etcd) TTL(node *etcdc.Node, serv *msg.Service) uint32 { +func (e *Etcd) TTL(node *etcdc.Node, serv *msg.Service) uint32 { etcdTTL := uint32(node.TTL) if etcdTTL == 0 && serv.Ttl == 0 { diff --git a/middleware/etcd/handler.go b/middleware/etcd/handler.go index cef319dbf..132dba370 100644 --- a/middleware/etcd/handler.go +++ b/middleware/etcd/handler.go @@ -10,7 +10,8 @@ import ( "golang.org/x/net/context" ) -func (e Etcd) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { +func (e *Etcd) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { + opt := Options{} state := middleware.State{W: w, Req: r} if state.QClass() != dns.ClassINET { return dns.RcodeServerFailure, fmt.Errorf("can only deal with ClassINET") @@ -18,7 +19,7 @@ func (e Etcd) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i name := state.Name() if e.Debug { if debug := isDebug(name); debug != "" { - e.debug = r.Question[0].Name + opt.Debug = r.Question[0].Name state.Clear() state.Req.Question[0].Name = debug } @@ -41,6 +42,9 @@ func (e Etcd) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i if e.Next == nil { return dns.RcodeServerFailure, nil } + if opt.Debug != "" { + r.Question[0].Name = opt.Debug + } return e.Next.ServeDNS(ctx, w, r) } @@ -51,47 +55,47 @@ func (e Etcd) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i ) switch state.Type() { case "A": - records, debug, err = e.A(zone, state, nil) + records, debug, err = e.A(zone, state, nil, opt) case "AAAA": - records, debug, err = e.AAAA(zone, state, nil) + records, debug, err = e.AAAA(zone, state, nil, opt) case "TXT": - records, debug, err = e.TXT(zone, state) + records, debug, err = e.TXT(zone, state, opt) case "CNAME": - records, debug, err = e.CNAME(zone, state) + records, debug, err = e.CNAME(zone, state, opt) case "PTR": - records, debug, err = e.PTR(zone, state) + records, debug, err = e.PTR(zone, state, opt) case "MX": - records, extra, debug, err = e.MX(zone, state) + records, extra, debug, err = e.MX(zone, state, opt) case "SRV": - records, extra, debug, err = e.SRV(zone, state) + records, extra, debug, err = e.SRV(zone, state, opt) case "SOA": - records, debug, err = e.SOA(zone, state) + records, debug, err = e.SOA(zone, state, opt) case "NS": if state.Name() == zone { - records, extra, debug, err = e.NS(zone, state) + records, extra, debug, err = e.NS(zone, state, opt) break } fallthrough default: // Do a fake A lookup, so we can distinguish between NODATA and NXDOMAIN - _, debug, err = e.A(zone, state, nil) + _, debug, err = e.A(zone, state, nil, opt) } - if e.debug != "" { + if opt.Debug != "" { // Substitute this name with the original when we return the request. state.Clear() - state.Req.Question[0].Name = e.debug + state.Req.Question[0].Name = opt.Debug } if isEtcdNameError(err) { - return e.Err(zone, dns.RcodeNameError, state, debug, err) + return e.Err(zone, dns.RcodeNameError, state, debug, err, opt) } if err != nil { - return e.Err(zone, dns.RcodeServerFailure, state, debug, err) + return e.Err(zone, dns.RcodeServerFailure, state, debug, err, opt) } if len(records) == 0 { - return e.Err(zone, dns.RcodeSuccess, state, debug, err) + return e.Err(zone, dns.RcodeSuccess, state, debug, err, opt) } m := new(dns.Msg) @@ -99,7 +103,7 @@ func (e Etcd) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i m.Authoritative, m.RecursionAvailable, m.Compress = true, true, true m.Answer = append(m.Answer, records...) m.Extra = append(m.Extra, extra...) - if e.debug != "" { + if opt.Debug != "" { m.Extra = append(m.Extra, servicesToTxt(debug)...) } @@ -111,12 +115,12 @@ func (e Etcd) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i } // Err write an error response to the client. -func (e Etcd) Err(zone string, rcode int, state middleware.State, debug []msg.Service, err error) (int, error) { +func (e *Etcd) Err(zone string, rcode int, state middleware.State, debug []msg.Service, err error, opt Options) (int, error) { m := new(dns.Msg) m.SetRcode(state.Req, rcode) m.Authoritative, m.RecursionAvailable, m.Compress = true, true, true - m.Ns, _, _ = e.SOA(zone, state) - if e.debug != "" { + m.Ns, _, _ = e.SOA(zone, state, opt) + if opt.Debug != "" { m.Extra = servicesToTxt(debug) txt := errorToTxt(err) if txt != nil { diff --git a/middleware/etcd/lookup.go b/middleware/etcd/lookup.go index f60aeec83..f111b4240 100644 --- a/middleware/etcd/lookup.go +++ b/middleware/etcd/lookup.go @@ -12,20 +12,24 @@ import ( "github.com/miekg/dns" ) -func (e Etcd) records(state middleware.State, exact bool) (services, debug []msg.Service, err error) { +type Options struct { + Debug string +} + +func (e Etcd) records(state middleware.State, exact bool, opt Options) (services, debug []msg.Service, err error) { services, err = e.Records(state.Name(), exact) if err != nil { return } - if e.debug != "" { + if opt.Debug != "" { debug = services } services = msg.Group(services) return } -func (e Etcd) A(zone string, state middleware.State, previousRecords []dns.RR) (records []dns.RR, debug []msg.Service, err error) { - services, debug, err := e.records(state, false) +func (e Etcd) A(zone string, state middleware.State, previousRecords []dns.RR, opt Options) (records []dns.RR, debug []msg.Service, err error) { + services, debug, err := e.records(state, false, opt) if err != nil { return nil, debug, err } @@ -50,7 +54,7 @@ func (e Etcd) A(zone string, state middleware.State, previousRecords []dns.RR) ( } state1 := copyState(state, serv.Host, state.QType()) - nextRecords, nextDebug, err := e.A(zone, state1, append(previousRecords, newRecord)) + nextRecords, nextDebug, err := e.A(zone, state1, append(previousRecords, newRecord), opt) if err == nil { // Not only have we found something we should add the CNAME and the IP addresses. @@ -84,8 +88,8 @@ func (e Etcd) A(zone string, state middleware.State, previousRecords []dns.RR) ( return records, debug, nil } -func (e Etcd) AAAA(zone string, state middleware.State, previousRecords []dns.RR) (records []dns.RR, debug []msg.Service, err error) { - services, debug, err := e.records(state, false) +func (e Etcd) AAAA(zone string, state middleware.State, previousRecords []dns.RR, opt Options) (records []dns.RR, debug []msg.Service, err error) { + services, debug, err := e.records(state, false, opt) if err != nil { return nil, debug, err } @@ -110,7 +114,7 @@ func (e Etcd) AAAA(zone string, state middleware.State, previousRecords []dns.RR } state1 := copyState(state, serv.Host, state.QType()) - nextRecords, nextDebug, err := e.AAAA(zone, state1, append(previousRecords, newRecord)) + nextRecords, nextDebug, err := e.AAAA(zone, state1, append(previousRecords, newRecord), opt) if err == nil { // Not only have we found something we should add the CNAME and the IP addresses. @@ -147,8 +151,8 @@ func (e Etcd) AAAA(zone string, state middleware.State, previousRecords []dns.RR // SRV returns SRV records from etcd. // If the Target is not a name but an IP address, a name is created on the fly. -func (e Etcd) SRV(zone string, state middleware.State) (records, extra []dns.RR, debug []msg.Service, err error) { - services, debug, err := e.records(state, false) +func (e Etcd) SRV(zone string, state middleware.State, opt Options) (records, extra []dns.RR, debug []msg.Service, err error) { + services, debug, err := e.records(state, false, opt) if err != nil { return nil, nil, nil, err } @@ -206,7 +210,7 @@ func (e Etcd) SRV(zone string, state middleware.State) (records, extra []dns.RR, // Internal name, we should have some info on them, either v4 or v6 // Clients expect a complete answer, because we are a recursor in their view. state1 := copyState(state, srv.Target, dns.TypeA) - addr, debugAddr, e1 := e.A(zone, state1, nil) + addr, debugAddr, e1 := e.A(zone, state1, nil, opt) if e1 == nil { extra = append(extra, addr...) debug = append(debug, debugAddr...) @@ -231,8 +235,8 @@ func (e Etcd) SRV(zone string, state middleware.State) (records, extra []dns.RR, // MX returns MX records from etcd. // If the Target is not a name but an IP address, a name is created on the fly. -func (e Etcd) MX(zone string, state middleware.State) (records, extra []dns.RR, debug []msg.Service, err error) { - services, debug, err := e.records(state, false) +func (e Etcd) MX(zone string, state middleware.State, opt Options) (records, extra []dns.RR, debug []msg.Service, err error) { + services, debug, err := e.records(state, false, opt) if err != nil { return nil, nil, debug, err } @@ -271,7 +275,7 @@ func (e Etcd) MX(zone string, state middleware.State) (records, extra []dns.RR, } // Internal name state1 := copyState(state, mx.Mx, dns.TypeA) - addr, debugAddr, e1 := e.A(zone, state1, nil) + addr, debugAddr, e1 := e.A(zone, state1, nil, opt) if e1 == nil { extra = append(extra, addr...) debug = append(debug, debugAddr...) @@ -290,8 +294,8 @@ func (e Etcd) MX(zone string, state middleware.State) (records, extra []dns.RR, return records, extra, debug, nil } -func (e Etcd) CNAME(zone string, state middleware.State) (records []dns.RR, debug []msg.Service, err error) { - services, debug, err := e.records(state, true) +func (e Etcd) CNAME(zone string, state middleware.State, opt Options) (records []dns.RR, debug []msg.Service, err error) { + services, debug, err := e.records(state, true, opt) if err != nil { return nil, debug, err } @@ -306,8 +310,8 @@ func (e Etcd) CNAME(zone string, state middleware.State) (records []dns.RR, debu } // PTR returns the PTR records, only services that have a domain name as host are included. -func (e Etcd) PTR(zone string, state middleware.State) (records []dns.RR, debug []msg.Service, err error) { - services, debug, err := e.records(state, true) +func (e Etcd) PTR(zone string, state middleware.State, opt Options) (records []dns.RR, debug []msg.Service, err error) { + services, debug, err := e.records(state, true, opt) if err != nil { return nil, debug, err } @@ -320,8 +324,8 @@ func (e Etcd) PTR(zone string, state middleware.State) (records []dns.RR, debug return records, debug, nil } -func (e Etcd) TXT(zone string, state middleware.State) (records []dns.RR, debug []msg.Service, err error) { - services, debug, err := e.records(state, false) +func (e Etcd) TXT(zone string, state middleware.State, opt Options) (records []dns.RR, debug []msg.Service, err error) { + services, debug, err := e.records(state, false, opt) if err != nil { return nil, debug, err } @@ -335,14 +339,14 @@ func (e Etcd) TXT(zone string, state middleware.State) (records []dns.RR, debug return records, debug, nil } -func (e Etcd) NS(zone string, state middleware.State) (records, extra []dns.RR, debug []msg.Service, err error) { +func (e Etcd) NS(zone string, state middleware.State, opt Options) (records, extra []dns.RR, debug []msg.Service, err error) { // NS record for this zone live in a special place, ns.dns.. Fake our lookup. // only a tad bit fishy... old := state.QName() state.Clear() state.Req.Question[0].Name = "ns.dns." + zone - services, debug, err := e.records(state, false) + services, debug, err := e.records(state, false, opt) if err != nil { return nil, nil, debug, err } @@ -368,7 +372,7 @@ func (e Etcd) NS(zone string, state middleware.State) (records, extra []dns.RR, } // SOA Record returns a SOA record. -func (e Etcd) SOA(zone string, state middleware.State) ([]dns.RR, []msg.Service, error) { +func (e Etcd) SOA(zone string, state middleware.State, opt Options) ([]dns.RR, []msg.Service, error) { header := dns.RR_Header{Name: zone, Rrtype: dns.TypeSOA, Ttl: 300, Class: dns.ClassINET} soa := &dns.SOA{Hdr: header, diff --git a/middleware/etcd/multi_test.go b/middleware/etcd/multi_test.go index bcb411d6f..56e8501fd 100644 --- a/middleware/etcd/multi_test.go +++ b/middleware/etcd/multi_test.go @@ -14,13 +14,13 @@ import ( ) func TestMultiLookup(t *testing.T) { - etcMulti := etc + etcMulti := *etc etcMulti.Zones = []string{"skydns.test.", "miek.nl."} etcMulti.Next = test.ErrorHandler() for _, serv := range servicesMulti { - set(t, etcMulti, serv.Key, 0, serv) - defer delete(t, etcMulti, serv.Key) + set(t, &etcMulti, serv.Key, 0, serv) + defer delete(t, &etcMulti, serv.Key) } for _, tc := range dnsTestCasesMulti { m := tc.Msg() diff --git a/middleware/etcd/setup_test.go b/middleware/etcd/setup_test.go index bc126761e..5b60c6038 100644 --- a/middleware/etcd/setup_test.go +++ b/middleware/etcd/setup_test.go @@ -20,7 +20,7 @@ import ( ) var ( - etc Etcd + etc *Etcd client etcdc.KeysAPI ctxt context.Context ) @@ -32,7 +32,7 @@ func init() { Endpoints: []string{"http://localhost:2379"}, } cli, _ := etcdc.New(etcdCfg) - etc = Etcd{ + etc = &Etcd{ Proxy: proxy.New([]string{"8.8.8.8:53"}), PathPrefix: "skydns", Ctx: context.Background(), @@ -42,7 +42,7 @@ func init() { } } -func set(t *testing.T, e Etcd, k string, ttl time.Duration, m *msg.Service) { +func set(t *testing.T, e *Etcd, k string, ttl time.Duration, m *msg.Service) { b, err := json.Marshal(m) if err != nil { t.Fatal(err) @@ -51,7 +51,7 @@ func set(t *testing.T, e Etcd, k string, ttl time.Duration, m *msg.Service) { e.Client.Set(ctxt, path, string(b), &etcdc.SetOptions{TTL: ttl}) } -func delete(t *testing.T, e Etcd, k string) { +func delete(t *testing.T, e *Etcd, k string) { path, _ := msg.PathWithWildcard(k, e.PathPrefix) e.Client.Delete(ctxt, path, &etcdc.DeleteOptions{Recursive: false}) } diff --git a/middleware/etcd/stub.go b/middleware/etcd/stub.go index 2d4c14c24..2daaff77c 100644 --- a/middleware/etcd/stub.go +++ b/middleware/etcd/stub.go @@ -13,7 +13,7 @@ import ( "github.com/miekg/dns" ) -func (e Etcd) UpdateStubZones() { +func (e *Etcd) UpdateStubZones() { go func() { for { e.updateStubZones() diff --git a/middleware/etcd/stub_cycle_test.go b/middleware/etcd/stub_cycle_test.go index 282c25408..eae769418 100644 --- a/middleware/etcd/stub_cycle_test.go +++ b/middleware/etcd/stub_cycle_test.go @@ -17,6 +17,7 @@ func TestStubCycle(t *testing.T) { defer delete(t, etc, serv.Key) } etc.updateStubZones() + defer func() { etc.Stubmap = nil }() for _, tc := range dnsTestCasesCycleStub { m := tc.Msg() diff --git a/middleware/etcd/stub_handler.go b/middleware/etcd/stub_handler.go index 514eff236..9d8778219 100644 --- a/middleware/etcd/stub_handler.go +++ b/middleware/etcd/stub_handler.go @@ -12,7 +12,7 @@ import ( // Stub wraps an Etcd. We have this type so that it can have a ServeDNS method. type Stub struct { - Etcd + *Etcd Zone string // for what zone (and thus what nameservers are we called) } diff --git a/middleware/etcd/stub_test.go b/middleware/etcd/stub_test.go index d36d36e87..7b704b3cd 100644 --- a/middleware/etcd/stub_test.go +++ b/middleware/etcd/stub_test.go @@ -19,6 +19,7 @@ func TestStubLookup(t *testing.T) { defer delete(t, etc, serv.Key) } etc.updateStubZones() + defer func() { etc.Stubmap = nil }() for _, tc := range dnsTestCasesStub { m := tc.Msg() diff --git a/middleware/test/helpers.go b/middleware/test/helpers.go index a01d7a306..01a6f156b 100644 --- a/middleware/test/helpers.go +++ b/middleware/test/helpers.go @@ -21,6 +21,7 @@ func (p RRSet) Len() int { return len(p) } func (p RRSet) Swap(i, j int) { p[i], p[j] = p[j], p[i] } func (p RRSet) Less(i, j int) bool { return p[i].String() < p[j].String() } +// If the TTL of a record is 303 we don't care what the TTL is. type Case struct { Qname string Qtype uint16 diff --git a/server/server.go b/server/server.go index ab9ce884f..0ba6f7f01 100644 --- a/server/server.go +++ b/server/server.go @@ -16,7 +16,6 @@ import ( "time" "github.com/miekg/coredns/middleware" - "github.com/miekg/coredns/middleware/chaos" "github.com/miekg/coredns/middleware/metrics" "github.com/miekg/dns" @@ -111,19 +110,6 @@ func New(addr string, configs []Config, gracefulTimeout time.Duration) (*Server, } s.zones[conf.Host] = z - - // A bit of a hack. Loop through the middlewares of this zone and check if - // they have enabled the chaos middleware. If so add the special chaos zones. - Middleware: - for _, mid := range z.config.Middleware { - fn := mid(nil) - if _, ok := fn.(chaos.Chaos); ok { - for _, ch := range []string{"authors.bind.", "version.bind.", "version.server.", "hostname.bind.", "id.server."} { - s.zones[ch] = z - } - break Middleware - } - } } return s, nil diff --git a/test/etcd_test.go b/test/etcd_test.go index 96be524e4..ceec936e7 100644 --- a/test/etcd_test.go +++ b/test/etcd_test.go @@ -2,10 +2,103 @@ package test -import "testing" +import ( + "encoding/json" + "io/ioutil" + "log" + "testing" + "time" + + "github.com/miekg/coredns/middleware" + "github.com/miekg/coredns/middleware/etcd" + "github.com/miekg/coredns/middleware/etcd/msg" + "github.com/miekg/coredns/middleware/proxy" + "github.com/miekg/coredns/middleware/test" + + etcdc "github.com/coreos/etcd/client" + "github.com/miekg/dns" + "golang.org/x/net/context" +) + +func etcdMiddleware() *etcd.Etcd { + etcdCfg := etcdc.Config{ + Endpoints: []string{"http://localhost:2379"}, + } + cli, _ := etcdc.New(etcdCfg) + client := etcdc.NewKeysAPI(cli) + return &etcd.Etcd{Client: client} +} // This test starts two coredns servers (and needs etcd). Configure a stubzones in both (that will loop) and // will then test if we detect this loop. func TestEtcdStubForwarding(t *testing.T) { // TODO(miek) } + +func TestEtcdStubAndProxyLookup(t *testing.T) { + corefile := `.:0 { + etcd skydns.local { + stubzones + path /skydns + endpoint http://localhost:2379 + upstream 8.8.8.8:53 8.8.4.4:53 + } + proxy . 8.8.8.8:53 +}` + + etc := etcdMiddleware() + ex, _, udp, err := Server(t, corefile) + if err != nil { + t.Fatalf("Could get server: %s", err) + } + defer ex.Stop() + + log.SetOutput(ioutil.Discard) + + var ctx = context.TODO() + for _, serv := range servicesStub { // adds example.{net,org} as stubs + set(ctx, t, etc, serv.Key, 0, serv) + defer delete(ctx, t, etc, serv.Key) + } + + p := proxy.New([]string{udp}) // use udp port from the server + state := middleware.State{W: &test.ResponseWriter{}, Req: new(dns.Msg)} + resp, err := p.Lookup(state, "example.com.", dns.TypeA) + if err != nil { + t.Error("Expected to receive reply, but didn't") + return + } + if len(resp.Answer) == 0 { + t.Error("Expected to at least one RR in the answer section, got none") + } + if resp.Answer[0].Header().Rrtype != dns.TypeA { + t.Error("Expected RR to A, got: %d", resp.Answer[0].Header().Rrtype) + } + if resp.Answer[0].(*dns.A).A.String() != "93.184.216.34" { + t.Error("Expected 93.184.216.34, got: %d", resp.Answer[0].(*dns.A).A.String()) + } +} + +var servicesStub = []*msg.Service{ + // Two tests, ask a question that should return servfail because remote it no accessible + // and one with edns0 option added, that should return refused. + {Host: "127.0.0.1", Port: 666, Key: "b.example.org.stub.dns.skydns.test."}, + // Actual test that goes out to the internet. + {Host: "199.43.132.53", Key: "a.example.net.stub.dns.skydns.test."}, +} + +// Copied from middleware/etcd/setup_test.go +func set(ctx context.Context, t *testing.T, e *etcd.Etcd, k string, ttl time.Duration, m *msg.Service) { + b, err := json.Marshal(m) + if err != nil { + t.Fatal(err) + } + path, _ := msg.PathWithWildcard(k, e.PathPrefix) + e.Client.Set(ctx, path, string(b), &etcdc.SetOptions{TTL: ttl}) +} + +// Copied from middleware/etcd/setup_test.go +func delete(ctx context.Context, t *testing.T, e *etcd.Etcd, k string) { + path, _ := msg.PathWithWildcard(k, e.PathPrefix) + e.Client.Delete(ctx, path, &etcdc.DeleteOptions{Recursive: false}) +}