From 4d76faa4b454f8b82a124e628eca5e00f1622a92 Mon Sep 17 00:00:00 2001 From: Chris O'Haver Date: Mon, 4 Apr 2022 14:59:16 -0400 Subject: [PATCH] plugin/etcd: Fix multi record TXT lookups (#5293) * fix multi-record txt Signed-off-by: Chris O'Haver --- plugin/backend_lookup.go | 8 ++++---- plugin/etcd/README.md | 2 ++ plugin/etcd/cname_test.go | 32 +++++++++++++++++--------------- plugin/etcd/group_test.go | 13 ++++++++++++- plugin/etcd/lookup_test.go | 8 +++++--- 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) } } }