From 3b7b9b49d5d4a94dcb924b90595bbfd976f0d87c Mon Sep 17 00:00:00 2001
From: Miek Gieben <miek@miek.nl>
Date: Mon, 8 Aug 2016 21:42:39 -0700
Subject: [PATCH] Fix lookup test (#206)

* Always continue

* debug queries: more sane impl

This PR just add a msg.Service to debug instead of crafting an TXT RR
at that point. This way we lift on the normal way of generating debug
responses and don't muck with that implementation.

The tags=etcd is flaky as hell for some reason.
---
 middleware/etcd/README.md            | 22 +++++++++++----------
 middleware/etcd/etcd.go              |  2 +-
 middleware/etcd/lookup.go            | 29 +++++++++++++---------------
 middleware/etcd/proxy_lookup_test.go | 14 +++++++-------
 4 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/middleware/etcd/README.md b/middleware/etcd/README.md
index b932dc265..c95855ef0 100644
--- a/middleware/etcd/README.md
+++ b/middleware/etcd/README.md
@@ -113,18 +113,20 @@ process in the response. The general form looks like this:
 
     skydns.test.skydns.dom.a.	300	CH	TXT	"127.0.0.1:0(10,0,,false)[0,]"
 
-  This shows the complete key as the owername, the rdata of the TXT record has:
-  `host:port(priority,weight,txt content,mail)[targetstrip,group]`.
+This shows the complete key as the owername, the rdata of the TXT record has:
+`host:port(priority,weight,txt content,mail)[targetstrip,group]`.
 
-  Any errors seen doing parsing will show up like this:
+Errors when communicating with an upstream will be returned as: `host:0(0,0,error message,false)[0,]`.
+
+An example:
+
+    www.example.org.	0	CH	TXT	"www.example.org.:0(0,0, IN A: unreachable backend,false)[0,]"
+
+Signalling that an A record for www.example.org. was sought, but it failed with that error.
+
+Any errors seen doing parsing will show up like this:
 
     . 0 CH TXT "/skydns/local/skydns/r/a: invalid character '.' after object key:value pair"
 
-  which shows `a.r.skydns.local.` has a json encoding problem.
+which shows `a.r.skydns.local.` has a json encoding problem.
 
-  Errors when communicating with an upstream will be returned as:
-
-    . 0 CH TXT "example.org. IN A: unreachable backend"
-
-  Signalling that an A record for example.org. was sought, but it failed
-  with that error.
diff --git a/middleware/etcd/etcd.go b/middleware/etcd/etcd.go
index 63aae2967..54346225b 100644
--- a/middleware/etcd/etcd.go
+++ b/middleware/etcd/etcd.go
@@ -24,7 +24,7 @@ type Etcd struct {
 	Client     etcdc.KeysAPI
 	Ctx        context.Context
 	Inflight   *singleflight.Group
-	Stubmap    *map[string]proxy.Proxy // List of proxies for stub resolving.
+	Stubmap    *map[string]proxy.Proxy // list of proxies for stub resolving.
 	Debug      bool                    // Do we allow debug queries.
 }
 
diff --git a/middleware/etcd/lookup.go b/middleware/etcd/lookup.go
index 881c43a3a..2a33ff235 100644
--- a/middleware/etcd/lookup.go
+++ b/middleware/etcd/lookup.go
@@ -1,7 +1,6 @@
 package etcd
 
 import (
-	"errors"
 	"fmt"
 	"math"
 	"net"
@@ -74,11 +73,9 @@ func (e Etcd) A(zone string, state middleware.State, previousRecords []dns.RR, o
 			}
 			m1, e1 := e.Proxy.Lookup(state, target, state.QType())
 			if e1 != nil {
-				if opt.Debug != "" {
-					debugTxt := errorToTxt(errors.New(target + " IN " + state.Type() + ":" + e1.Error()))
-					records = append(records, debugTxt)
-					continue
-				}
+				debugMsg := msg.Service{Key: msg.Path(target, e.PathPrefix), Host: target, Text: " IN " + state.Type() + ": " + e1.Error()}
+				debug = append(debug, debugMsg)
+				continue
 			}
 			// Len(m1.Answer) > 0 here is well?
 			records = append(records, newRecord)
@@ -138,8 +135,8 @@ func (e Etcd) AAAA(zone string, state middleware.State, previousRecords []dns.RR
 			}
 			m1, e1 := e.Proxy.Lookup(state, target, state.QType())
 			if e1 != nil {
-				debugTxt := errorToTxt(errors.New(target + " IN " + state.Type() + ": " + e1.Error()))
-				records = append(records, debugTxt)
+				debugMsg := msg.Service{Key: msg.Path(target, e.PathPrefix), Host: target, Text: " IN " + state.Type() + ": " + e1.Error()}
+				debug = append(debug, debugMsg)
 				continue
 			}
 			// Len(m1.Answer) > 0 here is well?
@@ -203,8 +200,8 @@ func (e Etcd) SRV(zone string, state middleware.State, opt Options) (records, ex
 				if e1 == nil {
 					extra = append(extra, m1.Answer...)
 				} else {
-					debugTxt := errorToTxt(errors.New(srv.Target + " IN A: " + e1.Error()))
-					extra = append(extra, debugTxt)
+					debugMsg := msg.Service{Key: msg.Path(srv.Target, e.PathPrefix), Host: srv.Target, Text: " IN A: " + e1.Error()}
+					debug = append(debug, debugMsg)
 				}
 
 				m1, e1 = e.Proxy.Lookup(state, srv.Target, dns.TypeAAAA)
@@ -216,8 +213,8 @@ func (e Etcd) SRV(zone string, state middleware.State, opt Options) (records, ex
 						}
 					}
 				} else {
-					debugTxt := errorToTxt(errors.New(srv.Target + " IN AAAA: " + e1.Error()))
-					extra = append(extra, debugTxt)
+					debugMsg := msg.Service{Key: msg.Path(srv.Target, e.PathPrefix), Host: srv.Target, Text: " IN AAAA: " + e1.Error()}
+					debug = append(debug, debugMsg)
 				}
 				break
 			}
@@ -276,8 +273,8 @@ func (e Etcd) MX(zone string, state middleware.State, opt Options) (records, ext
 				if e1 == nil {
 					extra = append(extra, m1.Answer...)
 				} else {
-					debugTxt := errorToTxt(errors.New(mx.Mx + " IN A: " + e1.Error()))
-					extra = append(extra, debugTxt)
+					debugMsg := msg.Service{Key: msg.Path(mx.Mx, e.PathPrefix), Host: mx.Mx, Text: " IN A: " + e1.Error()}
+					debug = append(debug, debugMsg)
 				}
 				m1, e1 = e.Proxy.Lookup(state, mx.Mx, dns.TypeAAAA)
 				if e1 == nil {
@@ -288,8 +285,8 @@ func (e Etcd) MX(zone string, state middleware.State, opt Options) (records, ext
 						}
 					}
 				} else {
-					debugTxt := errorToTxt(errors.New(mx.Mx + " IN AAAA: " + e1.Error()))
-					extra = append(extra, debugTxt)
+					debugMsg := msg.Service{Key: msg.Path(mx.Mx, e.PathPrefix), Host: mx.Mx, Text: " IN AAAA: " + e1.Error()}
+					debug = append(debug, debugMsg)
 				}
 				break
 			}
diff --git a/middleware/etcd/proxy_lookup_test.go b/middleware/etcd/proxy_lookup_test.go
index 868bf80d6..2a52cba06 100644
--- a/middleware/etcd/proxy_lookup_test.go
+++ b/middleware/etcd/proxy_lookup_test.go
@@ -22,11 +22,11 @@ func TestProxyLookupFailDebug(t *testing.T) {
 
 	prxy := etc.Proxy
 	etc.Proxy = proxy.New([]string{"127.0.0.0:154"})
-	etc.Debug = true
-
-	defer func() { etc.Debug = false }()
 	defer func() { etc.Proxy = prxy }()
 
+	etc.Debug = true
+	defer func() { etc.Debug = false }()
+
 	for _, tc := range dnsTestCasesProxy {
 		m := tc.Msg()
 
@@ -58,20 +58,20 @@ func TestProxyLookupFailDebug(t *testing.T) {
 	}
 }
 
-// Note the key is encoded as DNS name, while in "reality" it is a etcd path.
 var servicesProxy = []*msg.Service{
 	{Host: "www.example.org", Key: "a.dom.skydns.test."},
 }
 
 var dnsTestCasesProxy = []test.Case{
 	{
-		Qname: "dom.skydns.test.", Qtype: dns.TypeSRV,
+		Qname: "o-o.debug.dom.skydns.test.", Qtype: dns.TypeSRV,
 		Answer: []dns.RR{
 			test.SRV("dom.skydns.test. 300 IN SRV 10 100 0 www.example.org."),
 		},
 		Extra: []dns.RR{
-			test.TXT(".	0	CH	TXT	\"www.example.org. IN A: unreachable backend\""),
-			test.TXT(".	0	CH	TXT	\"www.example.org. IN AAAA: unreachable backend\""),
+			test.TXT("a.dom.skydns.test.	300	CH	TXT	\"www.example.org:0(10,0,,false)[0,]\""),
+			test.TXT("www.example.org.	0	CH	TXT	\"www.example.org.:0(0,0, IN A: unreachable backend,false)[0,]\""),
+			test.TXT("www.example.org.	0	CH	TXT	\"www.example.org.:0(0,0, IN AAAA: unreachable backend,false)[0,]\""),
 		},
 	},
 }