k8s/autopath: Add CNAMES (#771)

* Add unit tests & cnames

* more progress

* fix

* next mw dependent unit tests

* add tests for OnNXDOMAIN

* Add AAAA and ndots unit tests; fix request.NewWithQuestion

* Correct default value in README

* add CNAMEs to readme

* review

* fix autopath examples

* fix and test CNAME response order

middleware/kubernetes/README.md

@@ -156,7 +156,7 @@ specified).
 
   If no domains in the path produce an answer, a lookup on the bare question will be attempted.	
 
-  A successful response will contain a question section with the original question, and an answer section containing the record for the question that actually had an answer.  This means that the question and answer will not match. For example:
+  A successful response will contain a question section with the original question, and an answer section containing the record for the question that actually had an answer.  This means that the question and answer will not match. To avoid potential client confusion, a dynamically generated CNAME entry is added to join the two. For example:
 
   ```
     # host -v -t a google.com
@@ -168,6 +168,7 @@ specified).
     ;google.com.default.svc.cluster.local. IN A
 
     ;; ANSWER SECTION:
+    google.com.default.svc.cluster.local. 175 IN CNAME google.com.
     google.com.		175	IN	A	216.58.194.206
   ```
 
@@ -178,14 +179,16 @@ specified).
   ```
 	# host -t a google.com
 	google.com has address 216.58.194.206
+	google.com.default.svc.cluster.local is an alias for google.com.
 	
 	# host -t a google.com.default.svc.cluster.local.
 	google.com has address 216.58.194.206
+	google.com.default.svc.cluster.local is an alias for google.com.
   ```
  
   **NDOTS** (default: `0`) This provides an adjustable threshold to prevent server side lookups from triggering. If the number of dots before the first search domain is less than this number, then the search path will not executed on the server side.  When autopath is enabled with default settings, the search path is always conducted when the query is in the first search domain `<pod-namespace>.svc.<zone>.`.
 	
-  **RESPONSE** (default: `SERVFAIL`) This option causes the kubernetes middleware to return the given response instead of NXDOMAIN when the all searches in the path produce no results. Valid values: `NXDOMAIN`, `SERVFAIL` or `NOERROR`. Setting this to `SERVFAIL` or `NOERROR` should prevent the client from fruitlessly continuing the client side searches in the path after the server already checked them.
+  **RESPONSE** (default: `NOERROR`) This option causes the kubernetes middleware to return the given response instead of NXDOMAIN when the all searches in the path produce no results. Valid values: `NXDOMAIN`, `SERVFAIL` or `NOERROR`. Setting this to `SERVFAIL` or `NOERROR` should prevent the client from fruitlessly continuing the client side searches in the path after the server already checked them.
 
   **RESOLV-CONF** (default: `/etc/resolv.conf`) If specified, the kubernetes middleware uses this file to get the host's search domains. The kubernetes middleware performs a lookup on these domains if the in-cluster search domains in the path fail to produce an answer. If not specified, the values will be read from the local resolv.conf file (i.e the resolv.conf file in the pod containing CoreDNS).  In practice, this option should only need to be used if running CoreDNS outside of the cluster and the search path in /etc/resolv.conf does not match the cluster's "default" dns-policiy.
 
@@ -245,7 +248,7 @@ specified).
 		cidrs 10.0.0.0/24
 		pods verified
 		upstream 10.102.3.10:53
-		autopath 0 SERVFAIL cluster.conf
+		autopath 0 NOERROR cluster.conf
 	}
 
 

middleware/kubernetes/autopath.go

+package kubernetes
+
+import "github.com/miekg/dns"
+
+// AutoPathWriter implements a ResponseWriter that also does the following:
+// * reverts question section of a packet to its original state.
+//   This is done to avoid the 'Question section mismatch:' error in client.
+// * Defers write to the client if the response code is NXDOMAIN.  This is needed
+//   to enable further search path probing if a search was not sucessful.
+// * Allow forced write to client regardless of response code.  This is needed to
+//   write the packet to the client if the final search in the path fails to
+//   produce results.
+// * Overwrites response code with AutoPathWriter.Rcode if the response code
+//   is NXDOMAIN (NameError).  This is needed to support the AutoPath.OnNXDOMAIN
+//   function, which returns a NOERROR to client instead of NXDOMAIN if the final
+//   search in the path fails to produce results.
+
+type AutoPathWriter struct {
+	dns.ResponseWriter
+	original dns.Question
+	Rcode    int
+	Sent     bool
+}
+
+// NewAutoPathWriter returns a pointer to a new AutoPathWriter
+func NewAutoPathWriter(w dns.ResponseWriter, r *dns.Msg) *AutoPathWriter {
+	return &AutoPathWriter{
+		ResponseWriter: w,
+		original:       r.Question[0],
+		Rcode:          dns.RcodeSuccess,
+	}
+}
+
+// WriteMsg writes to client, unless response will be NXDOMAIN
+func (apw *AutoPathWriter) WriteMsg(res *dns.Msg) error {
+	return apw.overrideMsg(res, false)
+}
+
+// ForceWriteMsg forces the write to client regardless of response code
+func (apw *AutoPathWriter) ForceWriteMsg(res *dns.Msg) error {
+	return apw.overrideMsg(res, true)
+}
+
+// overrideMsg overrides rcode, reverts question, adds CNAME, and calls the
+// underlying ResponseWriter's WriteMsg method unless the write is deferred,
+// or force = true.
+func (apw *AutoPathWriter) overrideMsg(res *dns.Msg, force bool) error {
+	if res.Rcode == dns.RcodeNameError {
+		res.Rcode = apw.Rcode
+	}
+	if res.Rcode != dns.RcodeSuccess && !force {
+		return nil
+	}
+	for _, a := range res.Answer {
+		if apw.original.Name == a.Header().Name {
+			continue
+		}
+		res.Answer = append(res.Answer, nil)
+		copy(res.Answer[1:], res.Answer)
+		res.Answer[0] = newCNAME(apw.original.Name, dns.Fqdn(a.Header().Name), a.Header().Ttl)
+	}
+	res.Question[0] = apw.original
+	apw.Sent = true
+	return apw.ResponseWriter.WriteMsg(res)
+}
+
+// Write is a wrapper that records the size of the message that gets written.
+func (apw *AutoPathWriter) Write(buf []byte) (int, error) {
+	n, err := apw.ResponseWriter.Write(buf)
+	return n, err
+}
+
+// Hijack implements dns.Hijacker. It simply wraps the underlying
+// ResponseWriter's Hijack method if there is one, or returns an error.
+func (apw *AutoPathWriter) Hijack() {
+	apw.ResponseWriter.Hijack()
+	return
+}

middleware/kubernetes/handler.go

@@ -6,7 +6,6 @@ import (
 
 	"github.com/coredns/coredns/middleware"
 	"github.com/coredns/coredns/middleware/pkg/dnsutil"
-	"github.com/coredns/coredns/middleware/rewrite"
 	"github.com/coredns/coredns/request"
 
 	"github.com/miekg/dns"
@@ -23,7 +22,6 @@ func (k Kubernetes) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.M
 	m := new(dns.Msg)
 	m.SetReply(r)
 	m.Authoritative, m.RecursionAvailable, m.Compress = true, true, true
-
 	// Check that query matches one of the zones served by this middleware,
 	// otherwise delegate to the next in the pipeline.
 	zone := middleware.Zones(k.Zones).Matches(state.Name())
@@ -40,10 +38,10 @@ func (k Kubernetes) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.M
 		// Set the zone to this specific request.
 		zone = state.Name()
 	}
-
 	records, extra, _, err := k.routeRequest(zone, state)
 
-	if k.AutoPath.Enabled && k.IsNameError(err) {
+	// Check for Autopath search eligibility
+	if k.AutoPath.Enabled && k.IsNameError(err) && (state.QType() == dns.TypeA || state.QType() == dns.TypeAAAA) {
 		p := k.findPodWithIP(state.IP())
 		for p != nil {
 			name, path, ok := splitSearch(zone, state.QName(), p.Namespace)
@@ -53,38 +51,50 @@ func (k Kubernetes) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.M
 			if (dns.CountLabel(name) - 1) < k.AutoPath.NDots {
 				break
 			}
-			// Search "svc.cluster.local" and "cluster.local"
+			origQName := state.QName()
+			// Search "svc.cluster.local." and "cluster.local."
 			for i := 0; i < 2; i++ {
 				path = strings.Join(dns.SplitDomainName(path)[1:], ".")
-				state = state.NewWithQuestion(strings.Join([]string{name, path}, "."), state.QType())
-				records, extra, _, err = k.routeRequest(zone, state)
+				newstate := state.NewWithQuestion(strings.Join([]string{name, path}, "."), state.QType())
+				records, extra, _, err = k.routeRequest(zone, newstate)
 				if !k.IsNameError(err) {
+					records = append(records, nil)
+					copy(records[1:], records)
+					records[0] = newCNAME(origQName, records[0].Header().Name, records[0].Header().Ttl)
 					break
 				}
 			}
 			if !k.IsNameError(err) {
 				break
 			}
-			// Fallthrough with the host search path (if set)
-			wr := rewrite.NewResponseReverter(w, r)
+			// Try host search path (if set) in the next middleware
+			apw := NewAutoPathWriter(w, r)
 			for _, hostsearch := range k.AutoPath.HostSearchPath {
-				r = state.NewWithQuestion(strings.Join([]string{name, hostsearch}, "."), state.QType()).Req
-				rcode, nextErr := middleware.NextOrFailure(k.Name(), k.Next, ctx, wr, r)
-				if rcode == dns.RcodeSuccess {
+				newstate := state.NewWithQuestion(strings.Join([]string{name, hostsearch}, "."), state.QType())
+				rcode, nextErr := middleware.NextOrFailure(k.Name(), k.Next, ctx, apw, newstate.Req)
+				if apw.Sent {
 					return rcode, nextErr
 				}
 			}
 			// Search . in this middleware
-			state = state.NewWithQuestion(strings.Join([]string{name, "."}, ""), state.QType())
-			records, extra, _, err = k.routeRequest(zone, state)
+			newstate := state.NewWithQuestion(strings.Join([]string{name, "."}, ""), state.QType())
+			records, extra, _, err = k.routeRequest(zone, newstate)
 			if !k.IsNameError(err) {
+				records = append(records, nil)
+				copy(records[1:], records)
+				records[0] = newCNAME(origQName, records[0].Header().Name, records[0].Header().Ttl)
 				break
 			}
 			// Search . in the next middleware
-			r = state.Req
-			rcode, nextErr := middleware.NextOrFailure(k.Name(), k.Next, ctx, wr, r)
-			if rcode == dns.RcodeNameError {
-				rcode = k.AutoPath.OnNXDOMAIN
+			apw.Rcode = k.AutoPath.OnNXDOMAIN
+			newstate = state.NewWithQuestion(strings.Join([]string{name, "."}, ""), state.QType())
+			r = newstate.Req
+			rcode, nextErr := middleware.NextOrFailure(k.Name(), k.Next, ctx, apw, r)
+			if !apw.Sent && nextErr == nil {
+				r = dnsutil.Dedup(r)
+				state.SizeAndDo(r)
+				r, _ = state.Scrub(r)
+				apw.ForceWriteMsg(r)
 			}
 			return rcode, nextErr
 		}
@@ -115,6 +125,11 @@ func (k Kubernetes) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.M
 	return dns.RcodeSuccess, nil
 }
 
+func newCNAME(name string, target string, ttl uint32) *dns.CNAME {
+	// TODO factor this out and put in dnsutil
+	return &dns.CNAME{Hdr: dns.RR_Header{Name: name, Rrtype: dns.TypeCNAME, Class: dns.ClassINET, Ttl: ttl}, Target: dns.Fqdn(target)}
+}
+
 func (k *Kubernetes) routeRequest(zone string, state request.Request) (records []dns.RR, extra []dns.RR, debug []dns.RR, err error) {
 	switch state.Type() {
 	case "A":

middleware/kubernetes/kubernetes.go

@@ -110,13 +110,14 @@ var errResolvConfReadErr = errors.New("resolv.conf read error")
 
 // Services implements the ServiceBackend interface.
 func (k *Kubernetes) Services(state request.Request, exact bool, opt middleware.Options) (svcs []msg.Service, debug []msg.Service, err error) {
-
 	r, e := k.parseRequest(state.Name(), state.QType())
 	if e != nil {
 		return nil, nil, e
 	}
-
 	switch state.Type() {
+	case "AAAA":
+		// AAAA not implemented
+		return nil, nil, errNoItems
 	case "A", "CNAME":
 		if state.Type() == "A" && isDefaultNS(state.Name(), r) {
 			// If this is an A request for "ns.dns", respond with a "fake" record for coredns.
@@ -362,7 +363,6 @@ func (k *Kubernetes) Entries(r recordRequest) ([]msg.Service, error) {
 	if (!symbolContainsWildcard(r.namespace)) && (len(k.Namespaces) > 0) && (!dnsstrings.StringInSlice(r.namespace, k.Namespaces)) {
 		return nil, errNsNotExposed
 	}
-
 	services, pods, err := k.get(r)
 	if err != nil {
 		return nil, err
@@ -464,7 +464,7 @@ func ipFromPodName(podname string) string {
 }
 
 func (k *Kubernetes) findPodWithIP(ip string) (p *api.Pod) {
-	if k.PodMode != PodModeVerified {
+	if !k.AutoPath.Enabled {
 		return nil
 	}
 	objList := k.APIConn.PodIndex(ip)
@@ -533,7 +533,6 @@ func (k *Kubernetes) get(r recordRequest) (services []service, pods []pod, err e
 
 func (k *Kubernetes) findServices(r recordRequest) ([]service, error) {
 	serviceList := k.APIConn.ServiceList()
-
 	var resultItems []service
 
 	nsWildcard := symbolContainsWildcard(r.namespace)

middleware/kubernetes/setup.go

@@ -59,6 +59,7 @@ func kubernetesParse(c *caddy.Controller) (*Kubernetes, error) {
 		ResyncPeriod:   defaultResyncPeriod,
 		interfaceAddrs: &interfaceAddrs{},
 		PodMode:        PodModeDisabled,
+		Proxy:          proxy.Proxy{},
 	}
 
 	for c.Next() {
@@ -244,5 +245,5 @@ const (
 	defaultPodMode        = PodModeDisabled
 	defautNdots           = 0
 	defaultResolvConfFile = "/etc/resolv.conf"
-	defaultOnNXDOMAIN     = dns.RcodeServerFailure
+	defaultOnNXDOMAIN     = dns.RcodeSuccess
 )

request/request.go

@@ -29,7 +29,7 @@ type Request struct {
 // section in the request.
 func (r *Request) NewWithQuestion(name string, typ uint16) Request {
 	req1 := Request{W: r.W, Req: r.Req.Copy()}
-	req1.Req.Question[0] = dns.Question{Name: dns.Fqdn(name), Qtype: dns.ClassINET, Qclass: typ}
+	req1.Req.Question[0] = dns.Question{Name: dns.Fqdn(name), Qclass: dns.ClassINET, Qtype: typ}
 	return req1
 }