Testing DNS correctly deduplicates records

command/agent/dns.go

@@ -327,7 +327,15 @@ func (d *DNSServer) serviceLookup(datacenter, service, tag string, req, resp *dn
 
 // serviceARecords is used to add the A records for a service lookup
 func (d *DNSServer) serviceARecords(nodes structs.ServiceNodes, req, resp *dns.Msg) {
+	handled := make(map[string]struct{})
 	for _, node := range nodes {
+		// Avoid duplicate entries, possible if a node has
+		// the same service on multiple ports, etc.
+		if _, ok := handled[node.Address]; ok {
+			continue
+		}
+		handled[node.Address] = struct{}{}
+
 		ip := net.ParseIP(node.Address)
 		if ip == nil {
 			d.logger.Printf("[ERR] dns: failed to parse IP %v for %v", node.Address, node.Node)
@@ -348,7 +356,17 @@ func (d *DNSServer) serviceARecords(nodes structs.ServiceNodes, req, resp *dns.M
 
 // serviceARecords is used to add the SRV records for a service lookup
 func (d *DNSServer) serviceSRVRecords(dc string, nodes structs.ServiceNodes, req, resp *dns.Msg) {
+	handled := make(map[string]struct{})
 	for _, node := range nodes {
+		// Avoid duplicate entries, possible if a node has
+		// the same service the same port, etc.
+		tuple := fmt.Sprintf("%s:%d", node.Node, node.ServicePort)
+		if _, ok := handled[tuple]; ok {
+			continue
+		}
+		handled[tuple] = struct{}{}
+
+		// Add the SRV record
 		srvRec := &dns.SRV{
 			Hdr: dns.RR_Header{
 				Name:   req.Question[0].Name,
@@ -363,6 +381,13 @@ func (d *DNSServer) serviceSRVRecords(dc string, nodes structs.ServiceNodes, req
 		}
 		resp.Answer = append(resp.Answer, srvRec)
 
+		// Avoid duplicate A records, possible if a node has
+		// the same service on multiple ports, etc.
+		if _, ok := handled[node.Address]; ok {
+			continue
+		}
+		handled[node.Address] = struct{}{}
+
 		ip := net.ParseIP(node.Address)
 		if ip == nil {
 			d.logger.Printf("[ERR] dns: failed to parse IP %v for %v", node.Address, node.Node)

command/agent/dns_test.go

@@ -175,6 +175,109 @@ func TestDNS_ServiceLookup(t *testing.T) {
 	}
 }
 
+func TestDNS_ServiceLookup_Dedup(t *testing.T) {
+	dir, srv := makeDNSServer(t)
+	defer os.RemoveAll(dir)
+	defer srv.agent.Shutdown()
+
+	// Wait for leader
+	time.Sleep(100 * time.Millisecond)
+
+	// Register node
+	args := &structs.RegisterRequest{
+		Datacenter:  "dc1",
+		Node:        "foo",
+		Address:     "127.0.0.1",
+		ServiceName: "db",
+		ServiceTag:  "master",
+		ServicePort: 12345,
+	}
+	var out struct{}
+	if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	args = &structs.RegisterRequest{
+		Datacenter:  "dc1",
+		Node:        "foo",
+		Address:     "127.0.0.1",
+		ServiceID:   "db2",
+		ServiceName: "db",
+		ServiceTag:  "slave",
+		ServicePort: 12345,
+	}
+	if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	args = &structs.RegisterRequest{
+		Datacenter:  "dc1",
+		Node:        "foo",
+		Address:     "127.0.0.1",
+		ServiceID:   "db3",
+		ServiceName: "db",
+		ServiceTag:  "slave",
+		ServicePort: 12346,
+	}
+	if err := srv.agent.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, srv.agent.config.DNSAddr)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	if len(in.Answer) != 3 {
+		t.Fatalf("Bad: %#v", in)
+	}
+
+	aRec, ok := in.Answer[0].(*dns.A)
+	if !ok {
+		t.Fatalf("Bad: %#v", in.Answer[0])
+	}
+	if aRec.A.String() != "127.0.0.1" {
+		t.Fatalf("Bad: %#v", in.Answer[0])
+	}
+
+	srvRec, ok := in.Answer[1].(*dns.SRV)
+	if !ok {
+		t.Fatalf("Bad: %#v", in.Answer[1])
+	}
+	if srvRec.Port != 12345 {
+		t.Fatalf("Bad: %#v", srvRec)
+	}
+	if srvRec.Target != "foo.node.dc1.consul." {
+		t.Fatalf("Bad: %#v", srvRec)
+	}
+
+	srvRec, ok = in.Answer[2].(*dns.SRV)
+	if !ok {
+		t.Fatalf("Bad: %#v", in.Answer[1])
+	}
+	if srvRec.Port != 12346 {
+		t.Fatalf("Bad: %#v", srvRec)
+	}
+	if srvRec.Target != "foo.node.dc1.consul." {
+		t.Fatalf("Bad: %#v", srvRec)
+	}
+
+	aRec, ok = in.Extra[0].(*dns.A)
+	if !ok {
+		t.Fatalf("Bad: %#v", in.Extra[0])
+	}
+	if aRec.Hdr.Name != "foo.node.dc1.consul." {
+		t.Fatalf("Bad: %#v", in.Extra[0])
+	}
+	if aRec.A.String() != "127.0.0.1" {
+		t.Fatalf("Bad: %#v", in.Extra[0])
+	}
+}
+
 func TestDNS_Recurse(t *testing.T) {
 	dir, srv := makeDNSServer(t)
 	defer os.RemoveAll(dir)