plugin/etcd: Fix multi record TXT lookups (#5293)

* fix multi-record txt

Signed-off-by: Chris O'Haver <cohaver@infoblox.com>

5 files changed, 40 insertions, 23 deletions

diff --git a/plugin/backend_lookup.go b/plugin/backend_lookup.go
index a6fd1ab2e..89db8aac6 100644
--- a/plugin/backend_lookup.go
+++ b/plugin/backend_lookup.go
@@ -343,7 +343,7 @@ func CNAME(ctx context.Context, b ServiceBackend, zone string, state request.Req
 // TXT returns TXT records from Backend or an error.
 func TXT(ctx context.Context, b ServiceBackend, zone string, state request.Request, previousRecords []dns.RR, opt Options) (records []dns.RR, truncated bool, err error) {
 
-	services, err := b.Services(ctx, state, true, opt)
+	services, err := b.Services(ctx, state, false, opt)
 	if err != nil {
 		return nil, false, err
 	}
@@ -398,9 +398,9 @@ func TXT(ctx context.Context, b ServiceBackend, zone string, state request.Reque
 			continue
 
 		case dns.TypeTXT:
-			if _, ok := dup[serv.Host]; !ok {
-				dup[serv.Host] = struct{}{}
-				return append(records, serv.NewTXT(state.QName())), truncated, nil
+			if _, ok := dup[serv.Text]; !ok {
+				dup[serv.Text] = struct{}{}
+				records = append(records, serv.NewTXT(state.QName()))
 			}
 
 		}
diff --git a/plugin/etcd/README.md b/plugin/etcd/README.md
index 463bc044c..6e83f2327 100644
--- a/plugin/etcd/README.md
+++ b/plugin/etcd/README.md
@@ -221,12 +221,14 @@ If you query the zone name for `SRV` now, you will get the following response:
 If you would like to use `TXT` records, you can set the following:
 ~~~
 % etcdctl put /skydns/local/skydns/x6 '{"ttl":60,"text":"this is a random text message."}'
+% etcdctl put /skydns/local/skydns/x7 '{"ttl":60,"text":"this is a another random text message."}'
 ~~~
 
 If you query the zone name for `TXT` now, you will get the following response:
 ~~~ sh
 % dig +short skydns.local TXT @localhost
 "this is a random text message."
+"this is a another random text message."
 ~~~
 
 ## See Also
diff --git a/plugin/etcd/cname_test.go b/plugin/etcd/cname_test.go
index 045a775f8..1e64d6dfd 100644
--- a/plugin/etcd/cname_test.go
+++ b/plugin/etcd/cname_test.go
@@ -22,7 +22,7 @@ func TestCnameLookup(t *testing.T) {
 		set(t, etc, serv.Key, 0, serv)
 		defer delete(t, etc, serv.Key)
 	}
-	for _, tc := range dnsTestCasesCname {
+	for i, tc := range dnsTestCasesCname {
 		m := tc.Msg()
 
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
@@ -34,17 +34,17 @@ func TestCnameLookup(t *testing.T) {
 
 		resp := rec.Msg
 		if err := test.Header(tc, resp); err != nil {
-			t.Error(err)
+			t.Errorf("Test %d: %v", i, err)
 			continue
 		}
 		if err := test.Section(tc, test.Answer, resp.Answer); err != nil {
-			t.Error(err)
+			t.Errorf("Test %d: %v", i, err)
 		}
 		if err := test.Section(tc, test.Ns, resp.Ns); err != nil {
-			t.Error(err)
+			t.Errorf("Test %d: %v", i, err)
 		}
 		if err := test.Section(tc, test.Extra, resp.Extra); err != nil {
-			t.Error(err)
+			t.Errorf("Test %d: %v", i, err)
 		}
 	}
 }
@@ -57,16 +57,18 @@ var servicesCname = []*msg.Service{
 	{Host: "cname5.region2.skydns.test", Key: "cname4.region2.skydns.test."},
 	{Host: "cname6.region2.skydns.test", Key: "cname5.region2.skydns.test."},
 	{Host: "endpoint.region2.skydns.test", Key: "cname6.region2.skydns.test."},
+	{Host: "10.240.0.1", Key: "endpoint.region2.skydns.test."},
+
 	{Host: "mainendpoint.region2.skydns.test", Key: "region2.skydns.test."},
+
 	{Host: "cname2.region3.skydns.test", Key: "cname3.region3.skydns.test."},
 	{Host: "cname1.region3.skydns.test", Key: "cname2.region3.skydns.test."},
-	{Host: "region3.skydns.test", Key: "cname1.region3.skydns.test."},
-	{Host: "", Key: "region3.skydns.test.", Text: "SOME-RECORD-TEXT"},
-	{Host: "10.240.0.1", Key: "endpoint.region2.skydns.test."},
+	{Host: "endpoint.region3.skydns.test", Key: "cname1.region3.skydns.test."},
+	{Host: "", Key: "endpoint.region3.skydns.test.", Text: "SOME-RECORD-TEXT"},
 }
 
 var dnsTestCasesCname = []test.Case{
-	{
+	{ // Test 0
 		Qname: "a.server1.dev.region1.skydns.test.", Qtype: dns.TypeSRV,
 		Answer: []dns.RR{
 			test.SRV("a.server1.dev.region1.skydns.test.	300	IN	SRV	10 100 0 cname1.region2.skydns.test."),
@@ -81,26 +83,26 @@ var dnsTestCasesCname = []test.Case{
 			test.A("endpoint.region2.skydns.test.	300	IN	A	10.240.0.1"),
 		},
 	},
-	{
+	{ // Test 1
 		Qname: "region2.skydns.test.", Qtype: dns.TypeCNAME,
 		Answer: []dns.RR{
 			test.CNAME("region2.skydns.test.	300	IN	CNAME	mainendpoint.region2.skydns.test."),
 		},
 	},
-	{
-		Qname: "region3.skydns.test.", Qtype: dns.TypeCNAME,
+	{ // Test 2
+		Qname: "endpoint.region3.skydns.test.", Qtype: dns.TypeCNAME,
 		Rcode: dns.RcodeSuccess,
 		Ns: []dns.RR{
 			test.SOA("skydns.test.	303	IN	SOA	ns.dns.skydns.test. hostmaster.skydns.test. 1546424605 7200 1800 86400 30"),
 		},
 	},
-	{
+	{ // Test 3
 		Qname: "cname3.region3.skydns.test.", Qtype: dns.TypeTXT,
 		Answer: []dns.RR{
 			test.CNAME("cname3.region3.skydns.test.	300	IN	CNAME	cname2.region3.skydns.test."),
 			test.CNAME("cname2.region3.skydns.test.	300	IN	CNAME	cname1.region3.skydns.test."),
-			test.CNAME("cname1.region3.skydns.test.	300	IN	CNAME	region3.skydns.test."),
-			test.TXT("region3.skydns.test. 300 IN TXT \"SOME-RECORD-TEXT\""),
+			test.CNAME("cname1.region3.skydns.test.	300	IN	CNAME	endpoint.region3.skydns.test."),
+			test.TXT("endpoint.region3.skydns.test. 300 IN TXT \"SOME-RECORD-TEXT\""),
 		},
 	},
 }
diff --git a/plugin/etcd/group_test.go b/plugin/etcd/group_test.go
index 831bdc2d5..2620bf20e 100644
--- a/plugin/etcd/group_test.go
+++ b/plugin/etcd/group_test.go
@@ -46,12 +46,15 @@ var servicesGroup = []*msg.Service{
 
 	{Host: "127.0.0.1", Key: "a.dom1.skydns.test.", Group: "g1"},
 	{Host: "127.0.0.2", Key: "b.sub.dom1.skydns.test.", Group: "g2"},
+
+	{Text: "foo", Key: "a.dom3.skydns.test.", Group: "g1"},
+	{Text: "bar", Key: "b.sub.dom3.skydns.test.", Group: "g1"},
 }
 
 var dnsTestCasesGroup = []test.Case{
 	// Groups
 	{
-		// hits the group 'g1' and only includes those records
+		// hits the group 'g1' and only includes those A records
 		Qname: "dom.skydns.test.", Qtype: dns.TypeA,
 		Answer: []dns.RR{
 			test.A("dom.skydns.test. 300 IN A 127.0.0.1"),
@@ -73,4 +76,12 @@ var dnsTestCasesGroup = []test.Case{
 			test.A("dom1.skydns.test. 300 IN A 127.0.0.1"),
 		},
 	},
+	{
+		// hits the group 'g1' and only includes those TXT records
+		Qname: "dom3.skydns.test.", Qtype: dns.TypeTXT,
+		Answer: []dns.RR{
+			test.TXT("dom3.skydns.test. 300 IN TXT bar"),
+			test.TXT("dom3.skydns.test. 300 IN TXT foo"),
+		},
+	},
 }
diff --git a/plugin/etcd/lookup_test.go b/plugin/etcd/lookup_test.go
index ef26d182d..0b689b083 100644
--- a/plugin/etcd/lookup_test.go
+++ b/plugin/etcd/lookup_test.go
@@ -28,7 +28,8 @@ var services = []*msg.Service{
 	{Host: "10.0.0.2", Port: 8080, Key: "b.server1.prod.region1.skydns.test."},
 	{Host: "::1", Port: 8080, Key: "b.server6.prod.region1.skydns.test."},
 	// TXT record in server1.
-	{Host: "", Port: 8080, Text: "sometext", Key: "txt.server1.prod.region1.skydns.test."},
+	{Text: "sometext", Key: "a.txt.server1.prod.region1.skydns.test."},
+	{Text: "moretext", Key: "b.txt.server1.prod.region1.skydns.test."},
 	// Unresolvable internal name.
 	{Host: "unresolvable.skydns.test", Key: "cname.prod.region1.skydns.test."},
 	// Priority.
@@ -145,6 +146,7 @@ var dnsTestCases = []test.Case{
 	{
 		Qname: "txt.server1.prod.region1.skydns.test.", Qtype: dns.TypeTXT,
 		Answer: []dns.RR{
+			test.TXT("txt.server1.prod.region1.skydns.test. 303 IN TXT moretext"),
 			test.TXT("txt.server1.prod.region1.skydns.test. 303 IN TXT sometext"),
 		},
 	},
@@ -337,7 +339,7 @@ func TestLookup(t *testing.T) {
 		defer delete(t, etc, serv.Key)
 	}
 
-	for _, tc := range dnsTestCases {
+	for i, tc := range dnsTestCases {
 		m := tc.Msg()
 
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
@@ -345,7 +347,7 @@ func TestLookup(t *testing.T) {
 
 		resp := rec.Msg
 		if err := test.SortAndCheck(resp, tc); err != nil {
-			t.Error(err)
+			t.Errorf("Test %d: %v", i, err)
 		}
 	}
 }