From 4d0dae8debd39ad44dc25da884cc447d2d84bcde Mon Sep 17 00:00:00 2001
From: Miek Gieben <miek@miek.nl>
Date: Thu, 10 Aug 2017 19:27:54 +0100
Subject: [PATCH] middleware/autopath: some fixes (#883)

* middleware/autopath: some fixes

This fix a small issue in autopath, but unearthed a bigger one.
See #881.

* Fix test
---
 middleware/autopath/autopath.go      | 21 ++++++++++++++++-----
 middleware/autopath/autopath_test.go |  1 +
 middleware/autopath/setup.go         | 23 ++++++++++++-----------
 3 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/middleware/autopath/autopath.go b/middleware/autopath/autopath.go
index ecbdf0464..d859e5642 100644
--- a/middleware/autopath/autopath.go
+++ b/middleware/autopath/autopath.go
@@ -64,6 +64,11 @@ func (a *AutoPath) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Ms
 		return dns.RcodeServerFailure, middleware.Error(a.Name(), errors.New("can only deal with ClassINET"))
 	}
 
+	zone := middleware.Zones(a.Zones).Matches(state.Name())
+	if zone == "" {
+		return middleware.NextOrFailure(a.Name(), a.Next, ctx, w, r)
+	}
+
 	// Check if autopath should be done, searchFunc takes precedence over the local configured
 	// search path.
 	var err error
@@ -83,7 +88,7 @@ func (a *AutoPath) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Ms
 	origQName := state.QName()
 
 	// Establish base name of the query. I.e what was originally asked.
-	base, err := dnsutil.TrimZone(state.QName(), a.search[0]) // TOD(miek): we loose the original case of the query here.
+	base, err := dnsutil.TrimZone(state.QName(), a.search[0]) // TODO(miek): we loose the original case of the query here.
 	if err != nil {
 		return dns.RcodeServerFailure, err
 	}
@@ -91,15 +96,21 @@ func (a *AutoPath) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Ms
 	firstReply := new(dns.Msg)
 	firstRcode := 0
 	var firstErr error
+
+	ar := r.Copy()
 	// Walk the search path and see if we can get a non-nxdomain - if they all fail we return the first
 	// query we've done and return that as-is. This means the client will do the search path walk again...
 	for i, s := range searchpath {
-
 		newQName := base + "." + s
-		r.Question[0].Name = newQName
+		ar.Question[0].Name = newQName
 		nw := NewNonWriter(w)
 
-		rcode, err := middleware.NextOrFailure(a.Name(), a.Next, ctx, nw, r)
+		rcode, err := middleware.NextOrFailure(a.Name(), a.Next, ctx, nw, ar)
+		if err != nil {
+			// Return now - not sure if this is the best. We should also check if the write has happened.
+			return rcode, err
+
+		}
 		if i == 0 {
 			firstReply = nw.Msg
 			firstRcode = rcode
@@ -139,4 +150,4 @@ func (a *AutoPath) FirstInSearchPath(name string) bool {
 	return false
 }
 
-func (a AutoPath) Name() string { return "autopath" }
+func (a *AutoPath) Name() string { return "autopath" }
diff --git a/middleware/autopath/autopath_test.go b/middleware/autopath/autopath_test.go
index a536ee1fd..719389446 100644
--- a/middleware/autopath/autopath_test.go
+++ b/middleware/autopath/autopath_test.go
@@ -31,6 +31,7 @@ var autopathTestCases = []test.Case{
 
 func newTestAutoPath() *AutoPath {
 	ap := new(AutoPath)
+	ap.Zones = []string{"."}
 	ap.Next = nextHandler(map[string]int{
 		"b.example.org.": dns.RcodeNameError,
 		"b.com.":         dns.RcodeSuccess,
diff --git a/middleware/autopath/setup.go b/middleware/autopath/setup.go
index cbaf007fc..804b61855 100644
--- a/middleware/autopath/setup.go
+++ b/middleware/autopath/setup.go
@@ -5,7 +5,6 @@ import (
 
 	"github.com/coredns/coredns/core/dnsserver"
 	"github.com/coredns/coredns/middleware"
-	"github.com/coredns/coredns/middleware/kubernetes"
 
 	"github.com/mholt/caddy"
 	"github.com/miekg/dns"
@@ -20,7 +19,7 @@ func init() {
 }
 
 func setup(c *caddy.Controller) error {
-	ap, mw, err := autoPathParse(c)
+	ap, _, err := autoPathParse(c)
 	if err != nil {
 		return middleware.Error("autopath", err)
 	}
@@ -28,13 +27,15 @@ func setup(c *caddy.Controller) error {
 	c.OnStartup(func() error {
 		// Do this in OnStartup, so all middleware has been initialized.
 		// TODO(miek): fabricate test to proof this is not thread safe.
-		m := dnsserver.GetMiddleware(c, mw)
-		switch mw {
-		case "kubernetes":
-			if k, ok := m.(kubernetes.Kubernetes); ok {
-				ap.searchFunc = k.AutoPath
+		// TODO(miek): disable this for now: See https://github.com/coredns/coredns/issues/881
+		/*
+			switch mw {
+			case "kubernetes":
+				if k, ok := m.(kubernetes.Kubernetes); ok {
+					ap.searchFunc = k.AutoPath
+				}
 			}
-		}
+		*/
 		return nil
 	})
 
@@ -51,13 +52,13 @@ var allowedMiddleware = map[string]bool{
 }
 
 func autoPathParse(c *caddy.Controller) (*AutoPath, string, error) {
-	ap := new(AutoPath)
+	ap := &AutoPath{}
 	mw := ""
 
 	for c.Next() {
 		zoneAndresolv := c.RemainingArgs()
 		if len(zoneAndresolv) < 1 {
-			return nil, "", fmt.Errorf("no resolv-conf specified")
+			return ap, "", fmt.Errorf("no resolv-conf specified")
 		}
 		resolv := zoneAndresolv[len(zoneAndresolv)-1]
 		if resolv[0] == '@' {
@@ -69,7 +70,7 @@ func autoPathParse(c *caddy.Controller) (*AutoPath, string, error) {
 			// assume file on disk
 			rc, err := dns.ClientConfigFromFile(resolv)
 			if err != nil {
-				return nil, "", fmt.Errorf("failed to parse %q: %v", resolv, err)
+				return ap, "", fmt.Errorf("failed to parse %q: %v", resolv, err)
 			}
 			ap.search = rc.Search
 			middleware.Zones(ap.search).Normalize()