Fix NODATA/NXDOMAIN for unknown types in etcd (#113)

* Finish the nodata stuff. See issue #9
* middleware/etc: add response to SOA queries
* Remove and add a few TODOs

5 files changed, 47 insertions, 24 deletions

diff --git a/middleware/etcd/handler.go b/middleware/etcd/handler.go
index 91cd40e61..bcc4d582b 100644
--- a/middleware/etcd/handler.go
+++ b/middleware/etcd/handler.go
@@ -2,7 +2,6 @@ package etcd
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/miekg/coredns/middleware"
 
@@ -22,8 +21,7 @@ func (e Etcd) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
 	name := state.Name()
 	if e.Stubmap != nil && len(*e.Stubmap) > 0 {
 		for zone, _ := range *e.Stubmap {
-			// TODO(miek): use the Match function.
-			if strings.HasSuffix(name, zone) {
+			if middleware.Name(zone).Matches(name) {
 				stub := Stub{Etcd: e, Zone: zone}
 				return stub.ServeDNS(ctx, w, r)
 			}
@@ -56,15 +54,15 @@ func (e Etcd) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
 		records, extra, err = e.MX(zone, state)
 	case "SRV":
 		records, extra, err = e.SRV(zone, state)
+	case "SOA":
+		records = []dns.RR{e.SOA(zone, state)}
+	case "NS":
+		//TODO(miek): skydns had a thing that you specify this, should look for
+		//records otherwise synthesise them.
+		//records = e.NS(zone, state)
 	default:
-		// For SOA and NS we might still want this
-		// and use dns.<zones> as the name to put these
-		// also for stub
-		// rwrite and return
-		// Nodata response
-		// also catch other types, so that they return NODATA
-		// TODO(miek) nodata function see below
-		return 0, nil
+		// Do a fake A lookup, so we can distinguish betwen NODATA and NXDOMAIN
+		_, err = e.A(zone, state, nil)
 	}
 	if isEtcdNameError(err) {
 		return e.Err(zone, dns.RcodeNameError, state)
diff --git a/middleware/etcd/lookup.go b/middleware/etcd/lookup.go
index 794461097..403cb4e3b 100644
--- a/middleware/etcd/lookup.go
+++ b/middleware/etcd/lookup.go
@@ -3,6 +3,7 @@ package etcd
 import (
 	"math"
 	"net"
+	"time"
 
 	"github.com/miekg/coredns/middleware"
 	"github.com/miekg/coredns/middleware/etcd/msg"
@@ -29,9 +30,8 @@ func (e Etcd) A(zone string, state middleware.State, previousRecords []dns.RR) (
 		ip := net.ParseIP(serv.Host)
 		switch {
 		case ip == nil:
-			// Try to resolve as CNAME if it's not an IP, but only if we don't create loops.
-			// TODO(miek): lowercasing, use Match in middleware?
-			if state.Name() == dns.Fqdn(serv.Host) {
+			// TODO(miek): lowercasing? Should lowercase in everything see #85
+			if middleware.Name(state.Name()).Matches(dns.Fqdn(serv.Host)) {
 				// x CNAME x is a direct loop, don't add those
 				continue
 			}
@@ -90,8 +90,7 @@ func (e Etcd) AAAA(zone string, state middleware.State, previousRecords []dns.RR
 		switch {
 		case ip == nil:
 			// Try to resolve as CNAME if it's not an IP, but only if we don't create loops.
-			// TODO(miek): lowercasing, use Match in middleware/
-			if state.Name() == dns.Fqdn(serv.Host) {
+			if middleware.Name(state.Name()).Matches(dns.Fqdn(serv.Host)) {
 				// x CNAME x is a direct loop, don't add those
 				continue
 			}
@@ -111,7 +110,6 @@ func (e Etcd) AAAA(zone string, state middleware.State, previousRecords []dns.RR
 			if err == nil {
 				// Not only have we found something we should add the CNAME and the IP addresses.
 				if len(nextRecords) > 0 {
-					// TODO(miek): sorting here?
 					records = append(records, newRecord)
 					records = append(records, nextRecords...)
 				}
@@ -314,15 +312,21 @@ func (e Etcd) TXT(zone string, state middleware.State) (records []dns.RR, err er
 	return records, nil
 }
 
-// synthesis a SOA Record.
-// TODO(miek): finish
+// SOA Record returns a SOA record.
 func (e Etcd) SOA(zone string, state middleware.State) *dns.SOA {
 	header := dns.RR_Header{Name: zone, Rrtype: dns.TypeSOA, Ttl: 300, Class: dns.ClassINET}
-	return &dns.SOA{Hdr: header, Mbox: "hostmaster." + zone, Ns: "ns.dns." + zone}
+	return &dns.SOA{Hdr: header,
+		Mbox:    "hostmaster." + zone,
+		Ns:      "ns.dns." + zone,
+		Serial:  uint32(time.Now().Unix()),
+		Refresh: 14400,
+		Retry:   3600,
+		Expire:  604800,
+		Minttl:  60,
+	}
 }
 
-// TODO(miek): NS records, DS and DNSKEY ones...? prolly so that the signing will
-// work...
+// TODO(miek): DNSKEY and friends... intercepted by the DNSSEC middleware?
 
 func isDuplicateCNAME(r *dns.CNAME, records []dns.RR) bool {
 	for _, rec := range records {
diff --git a/middleware/etcd/lookup_test.go b/middleware/etcd/lookup_test.go
index fdd7ca95b..c1c990254 100644
--- a/middleware/etcd/lookup_test.go
+++ b/middleware/etcd/lookup_test.go
@@ -156,4 +156,26 @@ var dnsTestCases = []test.Case{
 		Qname: "a.server1.dev.region1.skydns.test.", Qtype: dns.TypeTXT,
 		Ns: []dns.RR{test.SOA("skydns.test. 300 SOA ns.dns.skydns.test. hostmaster.skydns.test. 0 0 0 0 0")},
 	},
+	// NODATA Test
+	{
+		Qname: "a.server1.dev.region1.skydns.test.", Qtype: dns.TypeHINFO,
+		Ns: []dns.RR{test.SOA("skydns.test. 300 SOA ns.dns.skydns.test. hostmaster.skydns.test. 0 0 0 0 0")},
+	},
+	// NXDOMAIN Test
+	{
+		Qname: "a.server1.nonexistent.region1.skydns.test.", Qtype: dns.TypeHINFO, Rcode: dns.RcodeNameError,
+		Ns: []dns.RR{test.SOA("skydns.test. 300 SOA ns.dns.skydns.test. hostmaster.skydns.test. 0 0 0 0 0")},
+	},
+	{
+		Qname: "skydns.test.", Qtype: dns.TypeSOA,
+		Answer: []dns.RR{test.SOA("skydns.test.	300	IN	SOA	ns.dns.skydns.test. hostmaster.skydns.test. 1460498836 14400 3600 604800 60")},
+	},
+	// TODO(miek)
+	// {
+	// Qname: "skydns.test.", Qtype: dns.TypeNS,
+	// },
+	{
+		Qname: "skydns_extra.test.", Qtype: dns.TypeSOA,
+		Answer: []dns.RR{test.SOA("skydns_extra.test. 300 IN SOA ns.dns.skydns_extra.test. hostmaster.skydns_extra.test. 1460498836 14400 3600 604800 60")},
+	},
 }
diff --git a/middleware/etcd/setup_test.go b/middleware/etcd/setup_test.go
index ea0ff43ed..0836f053e 100644
--- a/middleware/etcd/setup_test.go
+++ b/middleware/etcd/setup_test.go
@@ -39,7 +39,7 @@ func init() {
 		PathPrefix: "skydns",
 		Ctx:        context.Background(),
 		Inflight:   &singleflight.Group{},
-		Zones:      []string{"skydns.test."},
+		Zones:      []string{"skydns.test.", "skydns_extra.test."},
 		Client:     etcdc.NewKeysAPI(cli),
 	}
 }
diff --git a/middleware/replacer.go b/middleware/replacer.go
index c1f377d6e..a2fac3113 100644
--- a/middleware/replacer.go
+++ b/middleware/replacer.go
@@ -58,7 +58,6 @@ func NewReplacer(r *dns.Msg, rr *ResponseRecorder, emptyValue string) Replacer {
 	}
 
 	// Header placeholders (case-insensitive)
-	// TODO(miek): syntax for flags and document it
 	rep.replacements[headerReplacer+"id}"] = strconv.Itoa(int(r.Id))
 	rep.replacements[headerReplacer+"opcode}"] = strconv.Itoa(int(r.Opcode))
 	rep.replacements[headerReplacer+"do}"] = boolToString(state.Do())