Use sanitized version of node name of server in NS record, and start with "server" rather than "ns"

agent/agent.go

@@ -65,7 +65,7 @@ type delegate interface {
 	JoinLAN(addrs []string) (n int, err error)
 	RemoveFailedNode(node string) error
 	RPC(method string, args interface{}, reply interface{}) error
-	ServerAddrs() []string
+	ServerAddrs() map[string]string
 	SnapshotRPC(args *structs.SnapshotRequest, in io.Reader, out io.Writer, replyFn structs.SnapshotReplyFn) error
 	Shutdown() error
 	Stats() map[string]map[string]string

agent/consul/client.go

@@ -411,7 +411,7 @@ func (c *Client) Stats() map[string]map[string]string {
 	return stats
 }
 
-func (c *Client) ServerAddrs() []string {
+func (c *Client) ServerAddrs() map[string]string {
 	return c.servers.GetServerAddrs()
 }
 

agent/consul/server.go

@@ -1047,7 +1047,7 @@ func (s *Server) GetWANCoordinate() (*coordinate.Coordinate, error) {
 	return s.serfWAN.GetCoordinate()
 }
 
-func (s *Server) ServerAddrs() []string {
+func (s *Server) ServerAddrs() map[string]string {
 	ret, err := s.router.FindServerAddrs(s.config.Datacenter)
 	if err != nil {
 		s.logger.Printf("[WARN] Unexpected state, no server addresses for datacenter %v, got error: %v", s.config.Datacenter, err)

agent/consul/servers/manager.go

@@ -223,11 +223,11 @@ func (m *Manager) getServerList() serverList {
 	return m.listValue.Load().(serverList)
 }
 
-// GetServerAddrs returns a slice with all server addresses
-func (m *Manager) GetServerAddrs() []string {
-	var ret []string
+// GetServerAddrs returns a map from node name to address for all servers
+func (m *Manager) GetServerAddrs() map[string]string {
+	ret := make(map[string]string)
 	for _, server := range m.getServerList().servers {
-		ret = append(ret, server.Addr.String())
+		ret[server.Name] = server.Addr.String()
 	}
 	return ret
 }

agent/consul/servers/router.go

@@ -490,7 +490,7 @@ func (r *Router) GetDatacenterMaps() ([]structs.DatacenterMap, error) {
 	return maps, nil
 }
 
-func (r *Router) FindServerAddrs(datacenter string) ([]string, error) {
+func (r *Router) FindServerAddrs(datacenter string) (map[string]string, error) {
 	r.RLock()
 	defer r.RUnlock()
 
@@ -501,12 +501,14 @@ func (r *Router) FindServerAddrs(datacenter string) ([]string, error) {
 		return nil, fmt.Errorf("datacenter %v not found", datacenter)
 	}
 
-	var ret []string
+	ret := make(map[string]string)
 	for _, manager := range managers {
 		if manager.IsOffline() {
 			continue
 		}
-		ret = append(ret, manager.GetServerAddrs()...)
+		for name, addr := range manager.GetServerAddrs() {
+			ret[name] = addr
+		}
 	}
 	return ret, nil
 }

agent/dns.go

@@ -9,6 +9,8 @@ import (
 	"sync/atomic"
 	"time"
 
+	"regexp"
+
 	"github.com/armon/go-metrics"
 	"github.com/hashicorp/consul/agent/consul"
 	"github.com/hashicorp/consul/agent/consul/structs"
@@ -30,6 +32,8 @@ const (
 	defaultMaxUDPSize = 512
 )
 
+var invalidCharsRe = regexp.MustCompile(`[^A-Za-z0-9\\-]+`)
+
 // DNSServer is used to wrap an Agent and expose various
 // service discovery endpoints using a DNS interface.
 type DNSServer struct {
@@ -685,9 +689,10 @@ RPC:
 // addAuthority adds NS records and corresponding A records with the IP addresses of servers
 func (d *DNSServer) addAuthority(msg *dns.Msg) {
 	serverAddrs := d.agent.delegate.ServerAddrs()
-	for _, addr := range serverAddrs {
+	for name, addr := range serverAddrs {
 		ipAddrStr := strings.Split(addr, ":")[0]
-		nsName := "ns." + ipAddrStr + "." + d.domain
+		sanitizedName := invalidCharsRe.ReplaceAllString(name, "-") // does some basic sanitization of the name
+		nsName := "server-" + sanitizedName + "." + d.domain
 		ip := net.ParseIP(ipAddrStr)
 		if ip != nil {
 			ns := &dns.NS{

agent/dns_test.go

@@ -651,7 +651,9 @@ func TestDNS_ServiceLookup(t *testing.T) {
 
 func TestDNS_ServiceLookupWithInternalServiceAddress(t *testing.T) {
 	t.Parallel()
-	a := NewTestAgent(t.Name(), nil)
+	cfg := TestConfig()
+	cfg.NodeName = "my.test-node"
+	a := NewTestAgent(t.Name(), cfg)
 	defer a.Shutdown()
 
 	// Register a node with a service.
@@ -705,7 +707,7 @@ func TestDNS_ServiceLookupWithInternalServiceAddress(t *testing.T) {
 			A:   []byte{0x7f, 0x0, 0x0, 0x1}, // 127.0.0.1
 		},
 		&dns.A{
-			Hdr: dns.RR_Header{Name: "ns.127.0.0.1.consul.", Rrtype: 0x1, Class: 0x1, Rdlength: 0x4},
+			Hdr: dns.RR_Header{Name: "server-my-test-node-dc1.consul.", Rrtype: 0x1, Class: 0x1, Rdlength: 0x4},
 			A:   []byte{0x7f, 0x0, 0x0, 0x1}, // 127.0.0.1
 		},
 	}
@@ -788,6 +790,7 @@ func TestDNS_ExternalServiceToConsulCNAMELookup(t *testing.T) {
 	t.Parallel()
 	cfg := TestConfig()
 	cfg.Domain = "CONSUL."
+	cfg.NodeName = "test node"
 	a := NewTestAgent(t.Name(), cfg)
 	defer a.Shutdown()
 
@@ -897,7 +900,7 @@ func TestDNS_ExternalServiceToConsulCNAMELookup(t *testing.T) {
 		if !ok {
 			t.Fatalf("Bad: %#v", in.Extra[2])
 		}
-		if aRec2.Hdr.Name != "ns.127.0.0.1.consul." {
+		if aRec2.Hdr.Name != "server-test-node-dc1.consul." {
 			t.Fatalf("Bad: %#v", in.Extra[2])
 		}
 		if aRec2.A.String() != "127.0.0.1" {
@@ -911,7 +914,9 @@ func TestDNS_ExternalServiceToConsulCNAMELookup(t *testing.T) {
 
 func TestDNS_ExternalServiceToConsulCNAMENestedLookup(t *testing.T) {
 	t.Parallel()
-	a := NewTestAgent(t.Name(), nil)
+	cfg := TestConfig()
+	cfg.NodeName = "test-node"
+	a := NewTestAgent(t.Name(), cfg)
 	defer a.Shutdown()
 
 	// Register the initial node with a service
@@ -1051,7 +1056,7 @@ func TestDNS_ExternalServiceToConsulCNAMENestedLookup(t *testing.T) {
 		if !ok {
 			t.Fatalf("Bad: %#v", in.Extra[3])
 		}
-		if aRec2.Hdr.Name != "ns.127.0.0.1.consul." {
+		if aRec2.Hdr.Name != "server-test-node-dc1.consul." {
 			t.Fatalf("Bad: %#v", in.Extra[3])
 		}
 		if aRec2.A.String() != "127.0.0.1" {
@@ -2741,6 +2746,7 @@ func testDNS_ServiceLookup_responseLimits(t *testing.T, answerLimit int, qType u
 	expectedService, expectedQuery, expectedQueryID int) (bool, error) {
 	cfg := TestConfig()
 	cfg.DNSConfig.UDPAnswerLimit = answerLimit
+	cfg.NodeName = "test-node"
 	a := NewTestAgent(t.Name(), cfg)
 	defer a.Shutdown()