From 37f75a393e0056cb12a7a0b480fc3f86b0570e8d Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Thu, 3 Aug 2017 11:39:50 -0500 Subject: [PATCH] Use sanitized version of node name of server in NS record, and start with "server" rather than "ns" --- agent/agent.go | 2 +- agent/consul/client.go | 2 +- agent/consul/server.go | 2 +- agent/consul/servers/manager.go | 8 ++++---- agent/consul/servers/router.go | 8 +++++--- agent/dns.go | 9 +++++++-- agent/dns_test.go | 16 +++++++++++----- 7 files changed, 30 insertions(+), 17 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 99ee456936..ebb3228f58 100644 --- a/agent/agent.go +++ b/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 diff --git a/agent/consul/client.go b/agent/consul/client.go index 1f625463d7..87476e7cfd 100644 --- a/agent/consul/client.go +++ b/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() } diff --git a/agent/consul/server.go b/agent/consul/server.go index bba2cf9807..694426c4d2 100644 --- a/agent/consul/server.go +++ b/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) diff --git a/agent/consul/servers/manager.go b/agent/consul/servers/manager.go index 7272414f8a..092efadd7b 100644 --- a/agent/consul/servers/manager.go +++ b/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 } diff --git a/agent/consul/servers/router.go b/agent/consul/servers/router.go index a3dfa9af02..f75f354285 100644 --- a/agent/consul/servers/router.go +++ b/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 } diff --git a/agent/dns.go b/agent/dns.go index 6195cb85c2..0678361706 100644 --- a/agent/dns.go +++ b/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{ diff --git a/agent/dns_test.go b/agent/dns_test.go index 995ddf6c71..1b75631844 100644 --- a/agent/dns_test.go +++ b/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()