From 22c2be5bf194f29614ecc4349128816b4143b3a0 Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Mon, 2 Jul 2018 16:58:52 -0400 Subject: [PATCH] Fix some edge cases and add some tests. --- agent/dns.go | 13 +++- agent/dns_test.go | 184 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 196 insertions(+), 1 deletion(-) diff --git a/agent/dns.go b/agent/dns.go index 77d155f57f..3aa408f7b8 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -1144,6 +1144,7 @@ func (d *DNSServer) serviceNodeRecords(dc string, nodes structs.CheckServiceNode handled := make(map[string]struct{}) edns := req.IsEdns0() != nil haveCNAME := false + allowCNAME := true count := 0 for _, node := range nodes { @@ -1173,10 +1174,20 @@ func (d *DNSServer) serviceNodeRecords(dc string, nodes structs.CheckServiceNode // only allow at most 1 CNAME record switch records[0].(type) { case *dns.CNAME: - if haveCNAME { + if haveCNAME || !allowCNAME { continue } else { haveCNAME = true + allowCNAME = false + } + default: + if haveCNAME { + // clear the slice - this removes the 1 and only CNAME + // in favor of returning records without a CNAME + resp.Answer = nil + } else { + // we don't exactly have one but we don't want one now either + allowCNAME = false } } diff --git a/agent/dns_test.go b/agent/dns_test.go index bc6fdaadb0..e7111a1d98 100644 --- a/agent/dns_test.go +++ b/agent/dns_test.go @@ -946,6 +946,190 @@ func TestDNS_ServiceReverseLookupNodeAddress(t *testing.T) { } } +func TestDNS_ServiceLookupNoMultiCNAME(t *testing.T) { + t.Parallel() + a := NewTestAgent(t.Name(), "") + defer a.Shutdown() + + // Register a node with a service. + { + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo", + Address: "198.18.0.1", + Service: &structs.NodeService{ + Service: "db", + Port: 12345, + Address: "foo.node.consul", + }, + } + + var out struct{} + require.NoError(t, a.RPC("Catalog.Register", args, &out)) + } + + // Register a second node node with the same service. + { + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "bar", + Address: "198.18.0.2", + Service: &structs.NodeService{ + Service: "db", + Port: 12345, + Address: "bar.node.consul", + }, + } + + var out struct{} + if err := a.RPC("Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } + } + + m := new(dns.Msg) + m.SetQuestion("db.service.consul.", dns.TypeANY) + + c := new(dns.Client) + in, _, err := c.Exchange(m, a.DNSAddr()) + require.NoError(t, err) + + // expect a CNAME and an A RR + require.Len(t, in.Answer, 2) + require.IsType(t, &dns.CNAME{}, in.Answer[0]) + require.IsType(t, &dns.A{}, in.Answer[1]) +} + +func TestDNS_ServiceLookupPreferNoCNAME(t *testing.T) { + t.Parallel() + a := NewTestAgent(t.Name(), "") + defer a.Shutdown() + + // Register a node with a service. + { + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo", + Address: "198.18.0.1", + Service: &structs.NodeService{ + Service: "db", + Port: 12345, + Address: "198.18.0.1", + }, + } + + var out struct{} + require.NoError(t, a.RPC("Catalog.Register", args, &out)) + } + + // Register a second node node with the same service. + { + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "bar", + Address: "198.18.0.2", + Service: &structs.NodeService{ + Service: "db", + Port: 12345, + Address: "bar.node.consul", + }, + } + + var out struct{} + if err := a.RPC("Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } + } + + m := new(dns.Msg) + m.SetQuestion("db.service.consul.", dns.TypeANY) + + c := new(dns.Client) + in, _, err := c.Exchange(m, a.DNSAddr()) + require.NoError(t, err) + + // expect a CNAME and an A RR + require.Len(t, in.Answer, 1) + aRec, ok := in.Answer[0].(*dns.A) + require.Truef(t, ok, "Not an A RR") + + require.Equal(t, "db.service.consul.", aRec.Hdr.Name) + require.Equal(t, "198.18.0.1", aRec.A.String()) +} + +func TestDNS_ServiceLookupMultiAddrNoCNAME(t *testing.T) { + t.Parallel() + a := NewTestAgent(t.Name(), "") + defer a.Shutdown() + + // Register a node with a service. + { + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo", + Address: "198.18.0.1", + Service: &structs.NodeService{ + Service: "db", + Port: 12345, + Address: "198.18.0.1", + }, + } + + var out struct{} + require.NoError(t, a.RPC("Catalog.Register", args, &out)) + } + + // Register a second node node with the same service. + { + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "bar", + Address: "198.18.0.2", + Service: &structs.NodeService{ + Service: "db", + Port: 12345, + Address: "bar.node.consul", + }, + } + + var out struct{} + if err := a.RPC("Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } + } + + // Register a second node node with the same service. + { + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "baz", + Address: "198.18.0.3", + Service: &structs.NodeService{ + Service: "db", + Port: 12345, + Address: "198.18.0.3", + }, + } + + var out struct{} + if err := a.RPC("Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } + } + + m := new(dns.Msg) + m.SetQuestion("db.service.consul.", dns.TypeANY) + + c := new(dns.Client) + in, _, err := c.Exchange(m, a.DNSAddr()) + require.NoError(t, err) + + // expect a CNAME and an A RR + require.Len(t, in.Answer, 2) + require.IsType(t, &dns.A{}, in.Answer[0]) + require.IsType(t, &dns.A{}, in.Answer[1]) +} + func TestDNS_ServiceLookup(t *testing.T) { t.Parallel() a := NewTestAgent(t.Name(), "")