Merge pull request #2695 from hashicorp/b-srv-cname

Fixes issue with missing CNAME for services with non-IP addresses set in SRV responses.

command/agent/dns.go

@@ -876,8 +876,7 @@ func (d *DNSServer) serviceSRVRecords(dc string, nodes structs.CheckServiceNodes
 
 		// Add the extra record
 		records := d.formatNodeRecord(node.Node, addr, srvRec.Target, dns.TypeANY, ttl)
-
+		if len(records) > 0 {
-		if records != nil {
 			// Use the node address if it doesn't differ from the service address
 			if addr == node.Node.Address {
 				resp.Extra = append(resp.Extra, records...)
@@ -900,6 +899,10 @@ func (d *DNSServer) serviceSRVRecords(dc string, nodes structs.CheckServiceNodes
 					srvRec.Target = fmt.Sprintf("%s.addr.%s.%s", hex.EncodeToString(record.AAAA), dc, d.domain)
 					record.Hdr.Name = srvRec.Target
 					resp.Extra = append(resp.Extra, record)
+
+				// Something else (probably a CNAME; just add the records).
+				default:
+					resp.Extra = append(resp.Extra, records...)
 				}
 			}
 		}

command/agent/dns_test.go

@@ -988,7 +988,7 @@ func TestDNS_ExternalServiceToConsulCNAMENestedLookup(t *testing.T) {
 	}
 }
 
-func TestDNS_ServiceLookup_ServiceAddress(t *testing.T) {
+func TestDNS_ServiceLookup_ServiceAddress_A(t *testing.T) {
 	dir, srv := makeDNSServer(t)
 	defer os.RemoveAll(dir)
 	defer srv.agent.Shutdown()
@@ -1083,6 +1083,101 @@ func TestDNS_ServiceLookup_ServiceAddress(t *testing.T) {
 	}
 }
 
+func TestDNS_ServiceLookup_ServiceAddress_CNAME(t *testing.T) {
+	dir, srv := makeDNSServer(t)
+	defer os.RemoveAll(dir)
+	defer srv.agent.Shutdown()
+
+	testutil.WaitForLeader(t, srv.agent.RPC, "dc1")
+
+	// Register a node with a service whose address isn't an IP.
+	{
+		args := &structs.RegisterRequest{
+			Datacenter: "dc1",
+			Node:       "foo",
+			Address:    "127.0.0.1",
+			Service: &structs.NodeService{
+				Service: "db",
+				Tags:    []string{"master"},
+				Address: "www.google.com",
+				Port:    12345,
+			},
+		}
+
+		var out struct{}
+		if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+	}
+
+	// Register an equivalent prepared query.
+	var id string
+	{
+		args := &structs.PreparedQueryRequest{
+			Datacenter: "dc1",
+			Op:         structs.PreparedQueryCreate,
+			Query: &structs.PreparedQuery{
+				Name: "test",
+				Service: structs.ServiceQuery{
+					Service: "db",
+				},
+			},
+		}
+		if err := srv.agent.RPC("PreparedQuery.Apply", args, &id); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+	}
+
+	// Look up the service directly and via prepared query.
+	questions := []string{
+		"db.service.consul.",
+		id + ".query.consul.",
+	}
+	for _, question := range questions {
+		m := new(dns.Msg)
+		m.SetQuestion(question, dns.TypeSRV)
+
+		c := new(dns.Client)
+		addr, _ := srv.agent.config.ClientListener("", srv.agent.config.Ports.DNS)
+		in, _, err := c.Exchange(m, addr.String())
+		if err != nil {
+			t.Fatalf("err: %v", err)
+		}
+
+		if len(in.Answer) != 1 {
+			t.Fatalf("Bad: %#v", in)
+		}
+
+		srvRec, ok := in.Answer[0].(*dns.SRV)
+		if !ok {
+			t.Fatalf("Bad: %#v", in.Answer[0])
+		}
+		if srvRec.Port != 12345 {
+			t.Fatalf("Bad: %#v", srvRec)
+		}
+		if srvRec.Target != "foo.node.dc1.consul." {
+			t.Fatalf("Bad: %#v", srvRec)
+		}
+		if srvRec.Hdr.Ttl != 0 {
+			t.Fatalf("Bad: %#v", in.Answer[0])
+		}
+
+		cnameRec, ok := in.Extra[0].(*dns.CNAME)
+		if !ok {
+			t.Fatalf("Bad: %#v", in.Extra[0])
+		}
+		if cnameRec.Hdr.Name != "foo.node.dc1.consul." {
+			t.Fatalf("Bad: %#v", in.Extra[0])
+		}
+		if cnameRec.Target != "www.google.com." {
+			t.Fatalf("Bad: %#v", in.Extra[0])
+		}
+		if cnameRec.Hdr.Ttl != 0 {
+			t.Fatalf("Bad: %#v", in.Extra[0])
+		}
+	}
+}
+
 func TestDNS_ServiceLookup_ServiceAddressIPV6(t *testing.T) {
 	dir, srv := makeDNSServer(t)
 	defer os.RemoveAll(dir)