From c38906daad51c5b07fa8d786c956a889ad210faf Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Wed, 2 Aug 2017 16:44:40 -0500 Subject: [PATCH 01/14] Add NS records and A records for each server. Constructs ns host names using the advertise address of the server. --- agent/agent.go | 1 + agent/consul/client.go | 4 ++ agent/consul/server.go | 9 +++ agent/consul/servers/manager.go | 9 +++ agent/consul/servers/router.go | 22 ++++++ agent/dns.go | 46 ++++++++++++ agent/dns_test.go | 119 +++++++++++++++++++++++++++----- 7 files changed, 194 insertions(+), 16 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 98f02b692..99ee45693 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -65,6 +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 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 6ee73e12b..1f625463d 100644 --- a/agent/consul/client.go +++ b/agent/consul/client.go @@ -411,6 +411,10 @@ func (c *Client) Stats() map[string]map[string]string { return stats } +func (c *Client) ServerAddrs() []string { + return c.servers.GetServerAddrs() +} + // GetLANCoordinate returns the network coordinate of the current node, as // maintained by Serf. func (c *Client) GetLANCoordinate() (*coordinate.Coordinate, error) { diff --git a/agent/consul/server.go b/agent/consul/server.go index 3160ad73c..bba2cf980 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -1047,6 +1047,15 @@ func (s *Server) GetWANCoordinate() (*coordinate.Coordinate, error) { return s.serfWAN.GetCoordinate() } +func (s *Server) ServerAddrs() []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) + return nil + } + return ret +} + // Atomically sets a readiness state flag when leadership is obtained, to indicate that server is past its barrier write func (s *Server) setConsistentReadReady() { atomic.StoreInt32(&s.readyForConsistentReads, 1) diff --git a/agent/consul/servers/manager.go b/agent/consul/servers/manager.go index ef149d008..7272414f8 100644 --- a/agent/consul/servers/manager.go +++ b/agent/consul/servers/manager.go @@ -223,6 +223,15 @@ 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 + for _, server := range m.getServerList().servers { + ret = append(ret, server.Addr.String()) + } + return ret +} + // saveServerList is a convenience method which hides the locking semantics // of atomic.Value from the caller. func (m *Manager) saveServerList(l serverList) { diff --git a/agent/consul/servers/router.go b/agent/consul/servers/router.go index 315e3a55a..35d4580c1 100644 --- a/agent/consul/servers/router.go +++ b/agent/consul/servers/router.go @@ -489,3 +489,25 @@ func (r *Router) GetDatacenterMaps() ([]structs.DatacenterMap, error) { } return maps, nil } + +func (r *Router) FindServerAddrs(datacenter string) ([]string, error) { + r.RLock() + defer r.RUnlock() + + // Get the list of managers for this datacenter. This will usually just + // have one entry, but it's possible to have a user-defined area + WAN. + managers, ok := r.managers[datacenter] + if !ok { + return nil, fmt.Errorf("datacenter %v not found", datacenter) + } + + var ret []string + // Try each manager until we get a server. + for _, manager := range managers { + if manager.IsOffline() { + continue + } + ret = append(ret, manager.GetServerAddrs()...) + } + return ret, nil +} diff --git a/agent/dns.go b/agent/dns.go index bf01bd1e8..1434650ff 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -372,6 +372,7 @@ PARSE: INVALID: d.logger.Printf("[WARN] dns: QName invalid: %s", qName) d.addSOA(d.domain, resp) + d.addNSAndARecordsForDomain(resp) resp.SetRcode(req, dns.RcodeNameError) } @@ -414,6 +415,7 @@ RPC: // If we have no address, return not found! if out.NodeServices == nil { d.addSOA(d.domain, resp) + d.addNSAndARecordsForDomain(resp) resp.SetRcode(req, dns.RcodeNameError) return } @@ -427,6 +429,9 @@ RPC: if records != nil { resp.Answer = append(resp.Answer, records...) } + + // Add NS record and A record + d.addNSAndARecordsForDomain(resp) } // formatNodeRecord takes a Node and returns an A, AAAA, or CNAME record @@ -641,6 +646,7 @@ RPC: // If we have no nodes, return not found! if len(out.Nodes) == 0 { d.addSOA(d.domain, resp) + d.addNSAndARecordsForDomain(resp) resp.SetRcode(req, dns.RcodeNameError) return } @@ -656,6 +662,9 @@ RPC: d.serviceNodeRecords(datacenter, out.Nodes, req, resp, ttl) } + // Add NS and A records + d.addNSAndARecordsForDomain(resp) + // If the network is not TCP, restrict the number of responses if network != "tcp" { wasTrimmed := trimUDPResponse(d.config, req, resp) @@ -673,6 +682,40 @@ RPC: } } +// addNSAndARecordsForDomain uses the agent's advertise address to +func (d *DNSServer) addNSAndARecordsForDomain(msg *dns.Msg) { + serverAddrs := d.agent.delegate.ServerAddrs() + for _, addr := range serverAddrs { + ipAddrStr := strings.Split(addr, ":")[0] + nsName := "ns." + ipAddrStr + "." + d.domain + ip := net.ParseIP(ipAddrStr) + if ip != nil { + ns := &dns.NS{ + Hdr: dns.RR_Header{ + Name: d.domain, + Rrtype: dns.TypeNS, + Class: dns.ClassINET, + Ttl: 0, + }, + Ns: nsName, + } + msg.Ns = append(msg.Ns, ns) + + //add an A record for the NS record + a := &dns.A{ + Hdr: dns.RR_Header{ + Name: nsName, + Rrtype: dns.TypeA, + Class: dns.ClassINET, + Ttl: uint32(d.config.NodeTTL / time.Second), + }, + A: ip, + } + msg.Extra = append(msg.Extra, a) + } + } +} + // preparedQueryLookup is used to handle a prepared query. func (d *DNSServer) preparedQueryLookup(network, datacenter, query string, req, resp *dns.Msg) { // Execute the prepared query. @@ -710,6 +753,7 @@ RPC: // here since the RPC layer loses the type information. if err.Error() == consul.ErrQueryNotFound.Error() { d.addSOA(d.domain, resp) + d.addNSAndARecordsForDomain(resp) resp.SetRcode(req, dns.RcodeNameError) return } @@ -752,6 +796,7 @@ RPC: // If we have no nodes, return not found! if len(out.Nodes) == 0 { d.addSOA(d.domain, resp) + d.addNSAndARecordsForDomain(resp) resp.SetRcode(req, dns.RcodeNameError) return } @@ -776,6 +821,7 @@ RPC: // If the answer is empty and the response isn't truncated, return not found if len(resp.Answer) == 0 && !resp.Truncated { + d.addNSAndARecordsForDomain(resp) d.addSOA(d.domain, resp) return } diff --git a/agent/dns_test.go b/agent/dns_test.go index 90e905d6a..995ddf6c7 100644 --- a/agent/dns_test.go +++ b/agent/dns_test.go @@ -168,7 +168,7 @@ func TestDNS_NodeLookup(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Ns) != 1 { + if len(in.Ns) != 2 { t.Fatalf("Bad: %#v %#v", in, len(in.Answer)) } @@ -179,6 +179,14 @@ func TestDNS_NodeLookup(t *testing.T) { if soaRec.Hdr.Ttl != 0 { t.Fatalf("Bad: %#v", in.Ns[0]) } + + nsRec, ok := in.Ns[1].(*dns.NS) + if !ok { + t.Fatalf("Bad: %#v", in.Ns[1]) + } + if nsRec.Hdr.Ttl != 0 { + t.Fatalf("Bad: %#v", in.Ns[1]) + } } func TestDNS_CaseInsensitiveNodeLookup(t *testing.T) { @@ -619,7 +627,7 @@ func TestDNS_ServiceLookup(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Ns) != 1 { + if len(in.Ns) != 2 { t.Fatalf("Bad: %#v", in) } @@ -630,6 +638,14 @@ func TestDNS_ServiceLookup(t *testing.T) { if soaRec.Hdr.Ttl != 0 { t.Fatalf("Bad: %#v", in.Ns[0]) } + + nsRec, ok := in.Ns[1].(*dns.NS) + if !ok { + t.Fatalf("Bad: %#v", in.Ns[1]) + } + if nsRec.Hdr.Ttl != 0 { + t.Fatalf("Bad: %#v", in.Ns[1]) + } } } @@ -679,7 +695,6 @@ func TestDNS_ServiceLookupWithInternalServiceAddress(t *testing.T) { }, } verify.Values(t, "answer", in.Answer, wantAnswer) - wantExtra := []dns.RR{ &dns.CNAME{ Hdr: dns.RR_Header{Name: "foo.node.dc1.consul.", Rrtype: 0x5, Class: 0x1, Rdlength: 0x2}, @@ -689,6 +704,10 @@ func TestDNS_ServiceLookupWithInternalServiceAddress(t *testing.T) { Hdr: dns.RR_Header{Name: "db.service.consul.", Rrtype: 0x1, Class: 0x1, Rdlength: 0x4}, 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}, + A: []byte{0x7f, 0x0, 0x0, 0x1}, // 127.0.0.1 + }, } verify.Values(t, "extra", in.Extra, wantExtra) } @@ -842,7 +861,7 @@ func TestDNS_ExternalServiceToConsulCNAMELookup(t *testing.T) { t.Fatalf("Bad: %#v", in.Answer[0]) } - if len(in.Extra) != 2 { + if len(in.Extra) != 3 { t.Fatalf("Bad: %#v", in) } @@ -873,6 +892,20 @@ func TestDNS_ExternalServiceToConsulCNAMELookup(t *testing.T) { if aRec.Hdr.Ttl != 0 { t.Fatalf("Bad: %#v", in.Extra[1]) } + + aRec2, ok := in.Extra[2].(*dns.A) + if !ok { + t.Fatalf("Bad: %#v", in.Extra[2]) + } + if aRec2.Hdr.Name != "ns.127.0.0.1.consul." { + t.Fatalf("Bad: %#v", in.Extra[2]) + } + if aRec2.A.String() != "127.0.0.1" { + t.Fatalf("Bad: %#v", in.Extra[2]) + } + if aRec2.Hdr.Ttl != 0 { + t.Fatalf("Bad: %#v", in.Extra[2]) + } } } @@ -968,7 +1001,7 @@ func TestDNS_ExternalServiceToConsulCNAMENestedLookup(t *testing.T) { t.Fatalf("Bad: %#v", in.Answer[0]) } - if len(in.Extra) != 3 { + if len(in.Extra) != 4 { t.Fatalf("Bad: %#v", in) } @@ -1013,6 +1046,20 @@ func TestDNS_ExternalServiceToConsulCNAMENestedLookup(t *testing.T) { if aRec.Hdr.Ttl != 0 { t.Fatalf("Bad: %#v", in.Extra[2]) } + + aRec2, ok := in.Extra[3].(*dns.A) + if !ok { + t.Fatalf("Bad: %#v", in.Extra[3]) + } + if aRec2.Hdr.Name != "ns.127.0.0.1.consul." { + t.Fatalf("Bad: %#v", in.Extra[3]) + } + if aRec2.A.String() != "127.0.0.1" { + t.Fatalf("Bad: %#v", in.Extra[3]) + } + if aRec2.Hdr.Ttl != 0 { + t.Fatalf("Bad: %#v", in.Extra[3]) + } } } @@ -3758,7 +3805,7 @@ func TestDNS_NonExistingLookup(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Ns) != 1 { + if len(in.Ns) != 2 { t.Fatalf("Bad: %#v %#v", in, len(in.Answer)) } @@ -3769,6 +3816,14 @@ func TestDNS_NonExistingLookup(t *testing.T) { if soaRec.Hdr.Ttl != 0 { t.Fatalf("Bad: %#v", in.Ns[0]) } + + nsRec, ok := in.Ns[1].(*dns.NS) + if !ok { + t.Fatalf("Bad: %#v", in.Ns[1]) + } + if nsRec.Hdr.Ttl != 0 { + t.Fatalf("Bad: %#v", in.Ns[1]) + } } func TestDNS_NonExistingLookupEmptyAorAAAA(t *testing.T) { @@ -3859,21 +3914,28 @@ func TestDNS_NonExistingLookupEmptyAorAAAA(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Ns) != 1 { + if len(in.Ns) != 2 { t.Fatalf("Bad: %#v", in) } - - soaRec, ok := in.Ns[0].(*dns.SOA) + soaRec, ok := in.Ns[1].(*dns.SOA) if !ok { - t.Fatalf("Bad: %#v", in.Ns[0]) + t.Fatalf("Bad: %#v", in.Ns[1]) } if soaRec.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Ns[0]) + t.Fatalf("Bad: %#v", in.Ns[1]) } if in.Rcode != dns.RcodeSuccess { t.Fatalf("Bad: %#v", in) } + + nsRec, ok := in.Ns[0].(*dns.NS) + if !ok { + t.Fatalf("Bad: %#v", in.Ns[0]) + } + if nsRec.Hdr.Ttl != 0 { + t.Fatalf("Bad: %#v", in.Ns[0]) + } } // Check for ipv4 records on ipv6-only service directly and via the @@ -3893,18 +3955,26 @@ func TestDNS_NonExistingLookupEmptyAorAAAA(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Ns) != 1 { + if len(in.Ns) != 2 { t.Fatalf("Bad: %#v", in) } - soaRec, ok := in.Ns[0].(*dns.SOA) + nsRec, ok := in.Ns[0].(*dns.NS) if !ok { t.Fatalf("Bad: %#v", in.Ns[0]) } - if soaRec.Hdr.Ttl != 0 { + if nsRec.Hdr.Ttl != 0 { t.Fatalf("Bad: %#v", in.Ns[0]) } + soaRec, ok := in.Ns[1].(*dns.SOA) + if !ok { + t.Fatalf("Bad: %#v", in.Ns[1]) + } + if soaRec.Hdr.Ttl != 0 { + t.Fatalf("Bad: %#v", in.Ns[1]) + } + if in.Rcode != dns.RcodeSuccess { t.Fatalf("Bad: %#v", in) } @@ -3944,7 +4014,7 @@ func TestDNS_PreparedQuery_AllowStale(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Ns) != 1 { + if len(in.Ns) != 2 { t.Fatalf("Bad: %#v", in) } @@ -3955,6 +4025,15 @@ func TestDNS_PreparedQuery_AllowStale(t *testing.T) { if soaRec.Hdr.Ttl != 0 { t.Fatalf("Bad: %#v", in.Ns[0]) } + + nsRec, ok := in.Ns[1].(*dns.NS) + if !ok { + t.Fatalf("Bad: %#v", in.Ns[1]) + } + if nsRec.Hdr.Ttl != 0 { + t.Fatalf("Bad: %#v", in.Ns[1]) + } + } } @@ -3982,7 +4061,7 @@ func TestDNS_InvalidQueries(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Ns) != 1 { + if len(in.Ns) != 2 { t.Fatalf("Bad: %#v", in) } @@ -3993,6 +4072,14 @@ func TestDNS_InvalidQueries(t *testing.T) { if soaRec.Hdr.Ttl != 0 { t.Fatalf("Bad: %#v", in.Ns[0]) } + + nsRec, ok := in.Ns[1].(*dns.NS) + if !ok { + t.Fatalf("Bad: %#v", in.Ns[1]) + } + if nsRec.Hdr.Ttl != 0 { + t.Fatalf("Bad: %#v", in.Ns[1]) + } } } From 7e9d683ab1ca01d07b0e253c58bbb66ef462662f Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Thu, 3 Aug 2017 10:16:54 -0500 Subject: [PATCH 02/14] Removed a copy pasted irrelevant comment, and other code review feedback --- agent/consul/servers/router.go | 1 - agent/dns.go | 22 +++++++++++----------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/agent/consul/servers/router.go b/agent/consul/servers/router.go index 35d4580c1..a3dfa9af0 100644 --- a/agent/consul/servers/router.go +++ b/agent/consul/servers/router.go @@ -502,7 +502,6 @@ func (r *Router) FindServerAddrs(datacenter string) ([]string, error) { } var ret []string - // Try each manager until we get a server. for _, manager := range managers { if manager.IsOffline() { continue diff --git a/agent/dns.go b/agent/dns.go index 1434650ff..6195cb85c 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -372,7 +372,7 @@ PARSE: INVALID: d.logger.Printf("[WARN] dns: QName invalid: %s", qName) d.addSOA(d.domain, resp) - d.addNSAndARecordsForDomain(resp) + d.addAuthority(resp) resp.SetRcode(req, dns.RcodeNameError) } @@ -415,7 +415,7 @@ RPC: // If we have no address, return not found! if out.NodeServices == nil { d.addSOA(d.domain, resp) - d.addNSAndARecordsForDomain(resp) + d.addAuthority(resp) resp.SetRcode(req, dns.RcodeNameError) return } @@ -431,7 +431,7 @@ RPC: } // Add NS record and A record - d.addNSAndARecordsForDomain(resp) + d.addAuthority(resp) } // formatNodeRecord takes a Node and returns an A, AAAA, or CNAME record @@ -646,7 +646,7 @@ RPC: // If we have no nodes, return not found! if len(out.Nodes) == 0 { d.addSOA(d.domain, resp) - d.addNSAndARecordsForDomain(resp) + d.addAuthority(resp) resp.SetRcode(req, dns.RcodeNameError) return } @@ -663,7 +663,7 @@ RPC: } // Add NS and A records - d.addNSAndARecordsForDomain(resp) + d.addAuthority(resp) // If the network is not TCP, restrict the number of responses if network != "tcp" { @@ -682,8 +682,8 @@ RPC: } } -// addNSAndARecordsForDomain uses the agent's advertise address to -func (d *DNSServer) addNSAndARecordsForDomain(msg *dns.Msg) { +// 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 { ipAddrStr := strings.Split(addr, ":")[0] @@ -701,7 +701,7 @@ func (d *DNSServer) addNSAndARecordsForDomain(msg *dns.Msg) { } msg.Ns = append(msg.Ns, ns) - //add an A record for the NS record + // add an A record for the NS record a := &dns.A{ Hdr: dns.RR_Header{ Name: nsName, @@ -753,7 +753,7 @@ RPC: // here since the RPC layer loses the type information. if err.Error() == consul.ErrQueryNotFound.Error() { d.addSOA(d.domain, resp) - d.addNSAndARecordsForDomain(resp) + d.addAuthority(resp) resp.SetRcode(req, dns.RcodeNameError) return } @@ -796,7 +796,7 @@ RPC: // If we have no nodes, return not found! if len(out.Nodes) == 0 { d.addSOA(d.domain, resp) - d.addNSAndARecordsForDomain(resp) + d.addAuthority(resp) resp.SetRcode(req, dns.RcodeNameError) return } @@ -821,7 +821,7 @@ RPC: // If the answer is empty and the response isn't truncated, return not found if len(resp.Answer) == 0 && !resp.Truncated { - d.addNSAndARecordsForDomain(resp) + d.addAuthority(resp) d.addSOA(d.domain, resp) return } From 6bac9355fd1dbd75a3c9871c839ae6d724827e04 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Thu, 3 Aug 2017 11:39:50 -0500 Subject: [PATCH 03/14] 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 99ee45693..ebb3228f5 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 1f625463d..87476e7cf 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 bba2cf980..694426c4d 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 7272414f8..092efadd7 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 a3dfa9af0..f75f35428 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 6195cb85c..067836170 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 995ddf6c7..1b7563184 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() From bff45ee1daec80347879de9f41a46e7cf7f37304 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Thu, 3 Aug 2017 14:47:07 -0500 Subject: [PATCH 04/14] Unify regex used to identify invalid dns characters --- agent/agent.go | 8 ++------ agent/dns.go | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index ebb3228f5..99700f405 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -13,7 +13,6 @@ import ( "net/http" "os" "path/filepath" - "regexp" "strconv" "strings" "sync" @@ -51,9 +50,6 @@ const ( "service, but no reason was provided. This is a default message." ) -// dnsNameRe checks if a name or tag is dns-compatible. -var dnsNameRe = regexp.MustCompile(`^[a-zA-Z0-9\-]+$`) - // delegate defines the interface shared by both // consul.Client and consul.Server. type delegate interface { @@ -1370,7 +1366,7 @@ func (a *Agent) AddService(service *structs.NodeService, chkTypes []*structs.Che } // Warn if the service name is incompatible with DNS - if !dnsNameRe.MatchString(service.Service) { + if InvalidDnsRe.MatchString(service.Service) { a.logger.Printf("[WARN] Service name %q will not be discoverable "+ "via DNS due to invalid characters. Valid characters include "+ "all alpha-numerics and dashes.", service.Service) @@ -1378,7 +1374,7 @@ func (a *Agent) AddService(service *structs.NodeService, chkTypes []*structs.Che // Warn if any tags are incompatible with DNS for _, tag := range service.Tags { - if !dnsNameRe.MatchString(tag) { + if InvalidDnsRe.MatchString(tag) { a.logger.Printf("[DEBUG] Service tag %q will not be discoverable "+ "via DNS due to invalid characters. Valid characters include "+ "all alpha-numerics and dashes.", tag) diff --git a/agent/dns.go b/agent/dns.go index 067836170..8e458f666 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -32,7 +32,7 @@ const ( defaultMaxUDPSize = 512 ) -var invalidCharsRe = regexp.MustCompile(`[^A-Za-z0-9\\-]+`) +var InvalidDnsRe = regexp.MustCompile(`[^A-Za-z0-9\\-]+`) // DNSServer is used to wrap an Agent and expose various // service discovery endpoints using a DNS interface. @@ -691,7 +691,7 @@ func (d *DNSServer) addAuthority(msg *dns.Msg) { serverAddrs := d.agent.delegate.ServerAddrs() for name, addr := range serverAddrs { ipAddrStr := strings.Split(addr, ":")[0] - sanitizedName := invalidCharsRe.ReplaceAllString(name, "-") // does some basic sanitization of the name + sanitizedName := InvalidDnsRe.ReplaceAllString(name, "-") // does some basic sanitization of the name nsName := "server-" + sanitizedName + "." + d.domain ip := net.ParseIP(ipAddrStr) if ip != nil { From 450d8a69b578b51b02b4bcda6acf2ba63d645178 Mon Sep 17 00:00:00 2001 From: Frank Schroeder Date: Fri, 4 Aug 2017 13:24:04 +0200 Subject: [PATCH 05/14] dns: provide correct SOA and NS responses This patch changes the behavior of the DNS server as follows: * The SOA response contains the SOA record in the Answer section instead of the Authority section. It also contains NS records in the Authority and the corresponding A glue records in the Extra section. In addition, CNAMEs are added to the Extra section to make the MNAME of the SOA record resolvable. AAAA glue records are not yet supported. * The NS response returns up to three random servers from the consul cluster in the Answer section and the glue A records in the Extra section. AAAA glue records are not yet supported. --- agent/dns.go | 183 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 114 insertions(+), 69 deletions(-) diff --git a/agent/dns.go b/agent/dns.go index 8e458f666..90020741e 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -137,7 +137,7 @@ func (d *DNSServer) handlePtr(resp dns.ResponseWriter, req *dns.Msg) { // Only add the SOA if requested if req.Question[0].Qtype == dns.TypeSOA { - d.addSOA(d.domain, m) + d.addSOA(m) } datacenter := d.agent.config.Datacenter @@ -210,13 +210,40 @@ func (d *DNSServer) handleQuery(resp dns.ResponseWriter, req *dns.Msg) { m.Authoritative = true m.RecursionAvailable = (len(d.recursors) > 0) - // Only add the SOA if requested - if req.Question[0].Qtype == dns.TypeSOA { - d.addSOA(d.domain, m) - } + switch req.Question[0].Qtype { + case dns.TypeSOA: + ns, glue := d.nameservers() + m.Answer = append(m.Answer, d.soa()) + m.Ns = append(m.Ns, ns...) + m.Extra = append(m.Extra, glue...) - // Dispatch the correct handler - d.dispatch(network, req, m) + // add CNAMEs for "ns." to the Extra + // section to make the MNAME entry in the SOA + // record resolvable + for _, rr := range ns { + cname := &dns.CNAME{ + Hdr: dns.RR_Header{ + Name: "ns." + d.domain, + Rrtype: dns.TypeCNAME, + Class: dns.ClassINET, + Ttl: uint32(d.config.NodeTTL / time.Second), + }, + Target: rr.(*dns.NS).Ns, + } + m.Extra = append(m.Extra, cname) + } + + m.SetRcode(req, dns.RcodeSuccess) + + case dns.TypeNS: + ns, glue := d.nameservers() + m.Answer = ns + m.Extra = glue + m.SetRcode(req, dns.RcodeSuccess) + + default: + d.dispatch(network, req, m) + } // Handle EDNS if edns := req.IsEdns0(); edns != nil { @@ -229,24 +256,89 @@ func (d *DNSServer) handleQuery(resp dns.ResponseWriter, req *dns.Msg) { } } -// addSOA is used to add an SOA record to a message for the given domain -func (d *DNSServer) addSOA(domain string, msg *dns.Msg) { - soa := &dns.SOA{ +func (d *DNSServer) soa() *dns.SOA { + return &dns.SOA{ Hdr: dns.RR_Header{ - Name: domain, + Name: d.domain, Rrtype: dns.TypeSOA, Class: dns.ClassINET, Ttl: 0, }, - Ns: "ns." + domain, - Mbox: "postmaster." + domain, - Serial: uint32(time.Now().Unix()), + Ns: "ns." + d.domain, + Serial: uint32(time.Now().Unix()), + + // todo(fs): make these configurable + Mbox: "postmaster." + d.domain, Refresh: 3600, Retry: 600, Expire: 86400, Minttl: 0, } - msg.Ns = append(msg.Ns, soa) +} + +// addSOA is used to add an SOA record to a message for the given domain +func (d *DNSServer) addSOA(msg *dns.Msg) { + msg.Ns = append(msg.Ns, d.soa()) +} + +// nameservers returns the names and ip addresses of up to three random servers +// in the current cluster which serve as authoritative name servers for zone. +func (d *DNSServer) nameservers() (ns []dns.RR, extra []dns.RR) { + // get server names and store them in a map to randomize the output + servers := map[string]net.IP{} + for name, addr := range d.agent.delegate.ServerAddrs() { + ip := net.ParseIP(strings.Split(addr, ":")[0]) + if ip == nil { + continue + } + + // name is "name.dc" and domain is "consul." + // we want "name.node.dc.consul." + lastdot := strings.LastIndexByte(name, '.') + fqdn := name[:lastdot] + ".node" + name[lastdot:] + "." + d.domain + + // create a consistent, unique and sanitized name for the server + fqdn = dns.Fqdn(strings.ToLower(fqdn)) + + servers[fqdn] = ip + } + + if len(servers) == 0 { + return + } + + for name, ip := range servers { + // the name server record + nsrr := &dns.NS{ + Hdr: dns.RR_Header{ + Name: d.domain, + Rrtype: dns.TypeNS, + Class: dns.ClassINET, + Ttl: 0, + }, + Ns: name, + } + ns = append(ns, nsrr) + + // the glue record providing the ip address + a := &dns.A{ + Hdr: dns.RR_Header{ + Name: name, + Rrtype: dns.TypeA, + Class: dns.ClassINET, + Ttl: uint32(d.config.NodeTTL / time.Second), + }, + A: ip, + } + extra = append(extra, a) + + // don't provide more than 3 servers + if len(ns) >= 3 { + return + } + } + + return } // dispatch is used to parse a request and invoke the correct handler @@ -375,8 +467,7 @@ PARSE: return INVALID: d.logger.Printf("[WARN] dns: QName invalid: %s", qName) - d.addSOA(d.domain, resp) - d.addAuthority(resp) + d.addSOA(resp) resp.SetRcode(req, dns.RcodeNameError) } @@ -418,8 +509,7 @@ RPC: // If we have no address, return not found! if out.NodeServices == nil { - d.addSOA(d.domain, resp) - d.addAuthority(resp) + d.addSOA(resp) resp.SetRcode(req, dns.RcodeNameError) return } @@ -433,9 +523,6 @@ RPC: if records != nil { resp.Answer = append(resp.Answer, records...) } - - // Add NS record and A record - d.addAuthority(resp) } // formatNodeRecord takes a Node and returns an A, AAAA, or CNAME record @@ -649,8 +736,7 @@ RPC: // If we have no nodes, return not found! if len(out.Nodes) == 0 { - d.addSOA(d.domain, resp) - d.addAuthority(resp) + d.addSOA(resp) resp.SetRcode(req, dns.RcodeNameError) return } @@ -666,9 +752,6 @@ RPC: d.serviceNodeRecords(datacenter, out.Nodes, req, resp, ttl) } - // Add NS and A records - d.addAuthority(resp) - // If the network is not TCP, restrict the number of responses if network != "tcp" { wasTrimmed := trimUDPResponse(d.config, req, resp) @@ -681,46 +764,11 @@ RPC: // If the answer is empty and the response isn't truncated, return not found if len(resp.Answer) == 0 && !resp.Truncated { - d.addSOA(d.domain, resp) + d.addSOA(resp) return } } -// 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 name, addr := range serverAddrs { - ipAddrStr := strings.Split(addr, ":")[0] - sanitizedName := InvalidDnsRe.ReplaceAllString(name, "-") // does some basic sanitization of the name - nsName := "server-" + sanitizedName + "." + d.domain - ip := net.ParseIP(ipAddrStr) - if ip != nil { - ns := &dns.NS{ - Hdr: dns.RR_Header{ - Name: d.domain, - Rrtype: dns.TypeNS, - Class: dns.ClassINET, - Ttl: 0, - }, - Ns: nsName, - } - msg.Ns = append(msg.Ns, ns) - - // add an A record for the NS record - a := &dns.A{ - Hdr: dns.RR_Header{ - Name: nsName, - Rrtype: dns.TypeA, - Class: dns.ClassINET, - Ttl: uint32(d.config.NodeTTL / time.Second), - }, - A: ip, - } - msg.Extra = append(msg.Extra, a) - } - } -} - // preparedQueryLookup is used to handle a prepared query. func (d *DNSServer) preparedQueryLookup(network, datacenter, query string, req, resp *dns.Msg) { // Execute the prepared query. @@ -757,8 +805,7 @@ RPC: // not a full on server error. We have to use a string compare // here since the RPC layer loses the type information. if err.Error() == consul.ErrQueryNotFound.Error() { - d.addSOA(d.domain, resp) - d.addAuthority(resp) + d.addSOA(resp) resp.SetRcode(req, dns.RcodeNameError) return } @@ -800,8 +847,7 @@ RPC: // If we have no nodes, return not found! if len(out.Nodes) == 0 { - d.addSOA(d.domain, resp) - d.addAuthority(resp) + d.addSOA(resp) resp.SetRcode(req, dns.RcodeNameError) return } @@ -826,8 +872,7 @@ RPC: // If the answer is empty and the response isn't truncated, return not found if len(resp.Answer) == 0 && !resp.Truncated { - d.addAuthority(resp) - d.addSOA(d.domain, resp) + d.addSOA(resp) return } } From c7c4100503f383a87c43d6dedd1cdeb40dd0d3bb Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Fri, 4 Aug 2017 12:17:04 -0500 Subject: [PATCH 06/14] Don't add A records for NS requests, because the record being returned already resolves correctly. Also fixed all the unit tests, and ignored hostnames that don't meet valid dns hostname criteria --- agent/dns.go | 11 ++- agent/dns_test.go | 182 ++++++++++++++++++++++------------------------ 2 files changed, 93 insertions(+), 100 deletions(-) diff --git a/agent/dns.go b/agent/dns.go index 90020741e..ba65ce75d 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -236,9 +236,9 @@ func (d *DNSServer) handleQuery(resp dns.ResponseWriter, req *dns.Msg) { m.SetRcode(req, dns.RcodeSuccess) case dns.TypeNS: - ns, glue := d.nameservers() + ns, _ := d.nameservers() m.Answer = ns - m.Extra = glue + // no need to send A records with the IP address, since the ns record is a node name that resolves correctly m.SetRcode(req, dns.RcodeSuccess) default: @@ -295,7 +295,12 @@ func (d *DNSServer) nameservers() (ns []dns.RR, extra []dns.RR) { // name is "name.dc" and domain is "consul." // we want "name.node.dc.consul." lastdot := strings.LastIndexByte(name, '.') - fqdn := name[:lastdot] + ".node" + name[lastdot:] + "." + d.domain + nodeName := name[:lastdot] + if InvalidDnsRe.MatchString(nodeName) { + d.logger.Printf("[WARN] dns: Node name %q is not a valid dns host name, will not be added to NS record", nodeName) + continue + } + fqdn := nodeName + ".node" + name[lastdot:] + "." + d.domain // create a consistent, unique and sanitized name for the server fqdn = dns.Fqdn(strings.ToLower(fqdn)) diff --git a/agent/dns_test.go b/agent/dns_test.go index 1b7563184..4bf560055 100644 --- a/agent/dns_test.go +++ b/agent/dns_test.go @@ -168,7 +168,7 @@ func TestDNS_NodeLookup(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Ns) != 2 { + if len(in.Ns) != 1 { t.Fatalf("Bad: %#v %#v", in, len(in.Answer)) } @@ -180,13 +180,6 @@ func TestDNS_NodeLookup(t *testing.T) { t.Fatalf("Bad: %#v", in.Ns[0]) } - nsRec, ok := in.Ns[1].(*dns.NS) - if !ok { - t.Fatalf("Bad: %#v", in.Ns[1]) - } - if nsRec.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Ns[1]) - } } func TestDNS_CaseInsensitiveNodeLookup(t *testing.T) { @@ -627,7 +620,7 @@ func TestDNS_ServiceLookup(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Ns) != 2 { + if len(in.Ns) != 1 { t.Fatalf("Bad: %#v", in) } @@ -639,13 +632,6 @@ func TestDNS_ServiceLookup(t *testing.T) { t.Fatalf("Bad: %#v", in.Ns[0]) } - nsRec, ok := in.Ns[1].(*dns.NS) - if !ok { - t.Fatalf("Bad: %#v", in.Ns[1]) - } - if nsRec.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Ns[1]) - } } } @@ -706,10 +692,6 @@ func TestDNS_ServiceLookupWithInternalServiceAddress(t *testing.T) { Hdr: dns.RR_Header{Name: "db.service.consul.", Rrtype: 0x1, Class: 0x1, Rdlength: 0x4}, A: []byte{0x7f, 0x0, 0x0, 0x1}, // 127.0.0.1 }, - &dns.A{ - 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 - }, } verify.Values(t, "extra", in.Extra, wantExtra) } @@ -864,7 +846,7 @@ func TestDNS_ExternalServiceToConsulCNAMELookup(t *testing.T) { t.Fatalf("Bad: %#v", in.Answer[0]) } - if len(in.Extra) != 3 { + if len(in.Extra) != 2 { t.Fatalf("Bad: %#v", in) } @@ -896,22 +878,59 @@ func TestDNS_ExternalServiceToConsulCNAMELookup(t *testing.T) { t.Fatalf("Bad: %#v", in.Extra[1]) } - aRec2, ok := in.Extra[2].(*dns.A) - if !ok { - t.Fatalf("Bad: %#v", in.Extra[2]) - } - if aRec2.Hdr.Name != "server-test-node-dc1.consul." { - t.Fatalf("Bad: %#v", in.Extra[2]) - } - if aRec2.A.String() != "127.0.0.1" { - t.Fatalf("Bad: %#v", in.Extra[2]) - } - if aRec2.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Extra[2]) - } } } +func TestDNS_NSRecords(t *testing.T) { + t.Parallel() + cfg := TestConfig() + cfg.Domain = "CONSUL." + cfg.NodeName = "foo" + a := NewTestAgent(t.Name(), cfg) + defer a.Shutdown() + + // Register node + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo", + Address: "127.0.0.1", + TaggedAddresses: map[string]string{ + "wan": "127.0.0.2", + }, + } + + var out struct{} + if err := a.RPC("Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } + + m := new(dns.Msg) + m.SetQuestion("something.node.consul.", dns.TypeNS) + + c := new(dns.Client) + addr, _ := a.Config.ClientListener("", a.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) + } + + nsRec, ok := in.Answer[0].(*dns.NS) + if !ok { + t.Fatalf("Bad: %#v", in.Answer[0]) + } + if nsRec.Ns != "foo.node.dc1.consul." { + t.Fatalf("Bad: %#v", in.Answer[0]) + } + if nsRec.Hdr.Ttl != 0 { + t.Fatalf("Bad: %#v", in.Answer[0]) + } + +} + func TestDNS_ExternalServiceToConsulCNAMENestedLookup(t *testing.T) { t.Parallel() cfg := TestConfig() @@ -1006,7 +1025,7 @@ func TestDNS_ExternalServiceToConsulCNAMENestedLookup(t *testing.T) { t.Fatalf("Bad: %#v", in.Answer[0]) } - if len(in.Extra) != 4 { + if len(in.Extra) != 3 { t.Fatalf("Bad: %#v", in) } @@ -1051,20 +1070,6 @@ func TestDNS_ExternalServiceToConsulCNAMENestedLookup(t *testing.T) { if aRec.Hdr.Ttl != 0 { t.Fatalf("Bad: %#v", in.Extra[2]) } - - aRec2, ok := in.Extra[3].(*dns.A) - if !ok { - t.Fatalf("Bad: %#v", in.Extra[3]) - } - if aRec2.Hdr.Name != "server-test-node-dc1.consul." { - t.Fatalf("Bad: %#v", in.Extra[3]) - } - if aRec2.A.String() != "127.0.0.1" { - t.Fatalf("Bad: %#v", in.Extra[3]) - } - if aRec2.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Extra[3]) - } } } @@ -3811,7 +3816,7 @@ func TestDNS_NonExistingLookup(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Ns) != 2 { + if len(in.Ns) != 1 { t.Fatalf("Bad: %#v %#v", in, len(in.Answer)) } @@ -3822,14 +3827,6 @@ func TestDNS_NonExistingLookup(t *testing.T) { if soaRec.Hdr.Ttl != 0 { t.Fatalf("Bad: %#v", in.Ns[0]) } - - nsRec, ok := in.Ns[1].(*dns.NS) - if !ok { - t.Fatalf("Bad: %#v", in.Ns[1]) - } - if nsRec.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Ns[1]) - } } func TestDNS_NonExistingLookupEmptyAorAAAA(t *testing.T) { @@ -3920,28 +3917,21 @@ func TestDNS_NonExistingLookupEmptyAorAAAA(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Ns) != 2 { + if len(in.Ns) != 1 { t.Fatalf("Bad: %#v", in) } - soaRec, ok := in.Ns[1].(*dns.SOA) + soaRec, ok := in.Ns[0].(*dns.SOA) if !ok { - t.Fatalf("Bad: %#v", in.Ns[1]) + t.Fatalf("Bad: %#v", in.Ns[0]) } if soaRec.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Ns[1]) + t.Fatalf("Bad: %#v", in.Ns[0]) } if in.Rcode != dns.RcodeSuccess { t.Fatalf("Bad: %#v", in) } - nsRec, ok := in.Ns[0].(*dns.NS) - if !ok { - t.Fatalf("Bad: %#v", in.Ns[0]) - } - if nsRec.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Ns[0]) - } } // Check for ipv4 records on ipv6-only service directly and via the @@ -3961,24 +3951,16 @@ func TestDNS_NonExistingLookupEmptyAorAAAA(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Ns) != 2 { + if len(in.Ns) != 1 { t.Fatalf("Bad: %#v", in) } - nsRec, ok := in.Ns[0].(*dns.NS) + soaRec, ok := in.Ns[0].(*dns.SOA) if !ok { t.Fatalf("Bad: %#v", in.Ns[0]) } - if nsRec.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Ns[0]) - } - - soaRec, ok := in.Ns[1].(*dns.SOA) - if !ok { - t.Fatalf("Bad: %#v", in.Ns[1]) - } if soaRec.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Ns[1]) + t.Fatalf("Bad: %#v", in.Ns[0]) } if in.Rcode != dns.RcodeSuccess { @@ -4020,7 +4002,7 @@ func TestDNS_PreparedQuery_AllowStale(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Ns) != 2 { + if len(in.Ns) != 1 { t.Fatalf("Bad: %#v", in) } @@ -4032,14 +4014,6 @@ func TestDNS_PreparedQuery_AllowStale(t *testing.T) { t.Fatalf("Bad: %#v", in.Ns[0]) } - nsRec, ok := in.Ns[1].(*dns.NS) - if !ok { - t.Fatalf("Bad: %#v", in.Ns[1]) - } - if nsRec.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Ns[1]) - } - } } @@ -4067,7 +4041,7 @@ func TestDNS_InvalidQueries(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Ns) != 2 { + if len(in.Ns) != 1 { t.Fatalf("Bad: %#v", in) } @@ -4079,13 +4053,6 @@ func TestDNS_InvalidQueries(t *testing.T) { t.Fatalf("Bad: %#v", in.Ns[0]) } - nsRec, ok := in.Ns[1].(*dns.NS) - if !ok { - t.Fatalf("Bad: %#v", in.Ns[1]) - } - if nsRec.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Ns[1]) - } } } @@ -4781,3 +4748,24 @@ func TestDNS_Compression_Recurse(t *testing.T) { t.Fatalf("doesn't look compressed: %d vs. %d", compressed, unc) } } + +func TestDNSInvalidRegex(t *testing.T) { + tests := []struct { + desc string + in string + invalid bool + }{ + {"Valid Hostname", "testnode", false}, + {"Valid Hostname", "test-node", false}, + {"Invalid Hostname with special chars", "test#$$!node", true}, + {"Invalid Hostname with special chars in the end", "test-node%^", true}, + } + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + if got, want := InvalidDnsRe.MatchString(test.in), test.invalid; got != want { + t.Fatalf("Expected %v to return %v", test.in, want) + } + }) + + } +} From 52075bda1cd0028de4cee56c0f7d8e5ad9180bdf Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Fri, 4 Aug 2017 16:53:42 -0500 Subject: [PATCH 07/14] Added back glue records in NS response, expanded unit test. Also reused same function used in node lookup for adding A/AAAA records in the extra section of the NS response --- agent/dns.go | 28 +++++++++------------------- agent/dns_test.go | 19 +++++++++++++++++-- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/agent/dns.go b/agent/dns.go index ba65ce75d..f28edae4e 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -212,7 +212,7 @@ func (d *DNSServer) handleQuery(resp dns.ResponseWriter, req *dns.Msg) { switch req.Question[0].Qtype { case dns.TypeSOA: - ns, glue := d.nameservers() + ns, glue := d.nameservers(req.IsEdns0() != nil) m.Answer = append(m.Answer, d.soa()) m.Ns = append(m.Ns, ns...) m.Extra = append(m.Extra, glue...) @@ -236,9 +236,9 @@ func (d *DNSServer) handleQuery(resp dns.ResponseWriter, req *dns.Msg) { m.SetRcode(req, dns.RcodeSuccess) case dns.TypeNS: - ns, _ := d.nameservers() + ns, glue := d.nameservers(req.IsEdns0() != nil) m.Answer = ns - // no need to send A records with the IP address, since the ns record is a node name that resolves correctly + m.Extra = glue m.SetRcode(req, dns.RcodeSuccess) default: @@ -283,7 +283,7 @@ func (d *DNSServer) addSOA(msg *dns.Msg) { // nameservers returns the names and ip addresses of up to three random servers // in the current cluster which serve as authoritative name servers for zone. -func (d *DNSServer) nameservers() (ns []dns.RR, extra []dns.RR) { +func (d *DNSServer) nameservers(edns bool) (ns []dns.RR, extra []dns.RR) { // get server names and store them in a map to randomize the output servers := map[string]net.IP{} for name, addr := range d.agent.delegate.ServerAddrs() { @@ -324,18 +324,8 @@ func (d *DNSServer) nameservers() (ns []dns.RR, extra []dns.RR) { Ns: name, } ns = append(ns, nsrr) - // the glue record providing the ip address - a := &dns.A{ - Hdr: dns.RR_Header{ - Name: name, - Rrtype: dns.TypeA, - Class: dns.ClassINET, - Ttl: uint32(d.config.NodeTTL / time.Second), - }, - A: ip, - } - extra = append(extra, a) + extra = append(extra, d.formatNodeRecord(ip.String(), name, dns.TypeANY, d.config.NodeTTL, edns)...) // don't provide more than 3 servers if len(ns) >= 3 { @@ -523,7 +513,7 @@ RPC: n := out.NodeServices.Node edns := req.IsEdns0() != nil addr := d.agent.TranslateAddress(datacenter, n.Address, n.TaggedAddresses) - records := d.formatNodeRecord(out.NodeServices.Node, addr, + records := d.formatNodeRecord(addr, req.Question[0].Name, qType, d.config.NodeTTL, edns) if records != nil { resp.Answer = append(resp.Answer, records...) @@ -531,7 +521,7 @@ RPC: } // formatNodeRecord takes a Node and returns an A, AAAA, or CNAME record -func (d *DNSServer) formatNodeRecord(node *structs.Node, addr, qName string, qType uint16, ttl time.Duration, edns bool) (records []dns.RR) { +func (d *DNSServer) formatNodeRecord(addr, qName string, qType uint16, ttl time.Duration, edns bool) (records []dns.RR) { // Parse the IP ip := net.ParseIP(addr) var ipv4 net.IP @@ -911,7 +901,7 @@ func (d *DNSServer) serviceNodeRecords(dc string, nodes structs.CheckServiceNode handled[addr] = struct{}{} // Add the node record - records := d.formatNodeRecord(node.Node, addr, qName, qType, ttl, edns) + records := d.formatNodeRecord(addr, qName, qType, ttl, edns) if records != nil { resp.Answer = append(resp.Answer, records...) } @@ -955,7 +945,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, edns) + records := d.formatNodeRecord(addr, srvRec.Target, dns.TypeANY, ttl, edns) if len(records) > 0 { // Use the node address if it doesn't differ from the service address if addr == node.Node.Address { diff --git a/agent/dns_test.go b/agent/dns_test.go index 4bf560055..5d0929ef3 100644 --- a/agent/dns_test.go +++ b/agent/dns_test.go @@ -885,7 +885,7 @@ func TestDNS_NSRecords(t *testing.T) { t.Parallel() cfg := TestConfig() cfg.Domain = "CONSUL." - cfg.NodeName = "foo" + cfg.NodeName = "server1" a := NewTestAgent(t.Name(), cfg) defer a.Shutdown() @@ -922,13 +922,28 @@ func TestDNS_NSRecords(t *testing.T) { if !ok { t.Fatalf("Bad: %#v", in.Answer[0]) } - if nsRec.Ns != "foo.node.dc1.consul." { + if nsRec.Ns != "server1.node.dc1.consul." { t.Fatalf("Bad: %#v", in.Answer[0]) } if nsRec.Hdr.Ttl != 0 { t.Fatalf("Bad: %#v", in.Answer[0]) } + if len(in.Extra) != 1 { + t.Fatalf("Bad: %#v", in.Extra) + } + + aRec, ok := in.Extra[0].(*dns.A) + if !ok { + t.Fatalf("Bad: %#v", in.Extra) + } + if aRec.A.String() != "127.0.0.1" { + t.Fatalf("Bad: %#v", in.Extra) + } + if aRec.Hdr.Name != "server1.node.dc1.consul." { + t.Fatalf("Bad: %#v", in.Extra) + } + } func TestDNS_ExternalServiceToConsulCNAMENestedLookup(t *testing.T) { From 393a0eae93c6672cfa8d2d1e2fc8c414a3fffdcd Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Sun, 6 Aug 2017 18:18:30 -0500 Subject: [PATCH 08/14] Added test case with IPV6 bind address for NS records, rewrote tests to use verify library and other code review feedback --- agent/dns.go | 7 +++- agent/dns_test.go | 83 +++++++++++++++++++++++++++++++++++------------ 2 files changed, 68 insertions(+), 22 deletions(-) diff --git a/agent/dns.go b/agent/dns.go index f28edae4e..3245a4ea8 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -287,7 +287,12 @@ func (d *DNSServer) nameservers(edns bool) (ns []dns.RR, extra []dns.RR) { // get server names and store them in a map to randomize the output servers := map[string]net.IP{} for name, addr := range d.agent.delegate.ServerAddrs() { - ip := net.ParseIP(strings.Split(addr, ":")[0]) + host, _, err := net.SplitHostPort(addr) + if err != nil { + d.logger.Println("[WARN] Unable to parse address %v, got error: %v", addr, err) + continue + } + ip := net.ParseIP(host) if ip == nil { continue } diff --git a/agent/dns_test.go b/agent/dns_test.go index 5d0929ef3..152f5f237 100644 --- a/agent/dns_test.go +++ b/agent/dns_test.go @@ -914,36 +914,75 @@ func TestDNS_NSRecords(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Answer) != 1 { - t.Fatalf("Bad: %#v", in) + wantAnswer := []dns.RR{ + &dns.NS{ + Hdr: dns.RR_Header{Name: "consul.", Rrtype: dns.TypeNS, Class: dns.ClassINET, Ttl: 0, Rdlength: 0x13}, + Ns: "server1.node.dc1.consul.", + }, + } + verify.Values(t, "answer", in.Answer, wantAnswer) + wantExtra := []dns.RR{ + &dns.A{ + Hdr: dns.RR_Header{Name: "server1.node.dc1.consul.", Rrtype: dns.TypeA, Class: dns.ClassINET, Rdlength: 0x4, Ttl: 0}, + A: net.ParseIP("127.0.0.1").To4(), + }, } - nsRec, ok := in.Answer[0].(*dns.NS) - if !ok { - t.Fatalf("Bad: %#v", in.Answer[0]) - } - if nsRec.Ns != "server1.node.dc1.consul." { - t.Fatalf("Bad: %#v", in.Answer[0]) - } - if nsRec.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Answer[0]) + verify.Values(t, "extra", in.Extra, wantExtra) + +} + +func TestDNS_NSRecords_IPV6(t *testing.T) { + t.Parallel() + cfg := TestConfig() + cfg.Domain = "CONSUL." + cfg.NodeName = "server1" + cfg.AdvertiseAddr = "::1" + cfg.AdvertiseAddrWan = "::1" + a := NewTestAgent(t.Name(), cfg) + defer a.Shutdown() + + // Register node + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo", + Address: "127.0.0.1", + TaggedAddresses: map[string]string{ + "wan": "127.0.0.2", + }, } - if len(in.Extra) != 1 { - t.Fatalf("Bad: %#v", in.Extra) + var out struct{} + if err := a.RPC("Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) } - aRec, ok := in.Extra[0].(*dns.A) - if !ok { - t.Fatalf("Bad: %#v", in.Extra) + m := new(dns.Msg) + m.SetQuestion("server1.node.dc1.consul.", dns.TypeNS) + + c := new(dns.Client) + addr, _ := a.Config.ClientListener("", a.Config.Ports.DNS) + in, _, err := c.Exchange(m, addr.String()) + if err != nil { + t.Fatalf("err: %v", err) } - if aRec.A.String() != "127.0.0.1" { - t.Fatalf("Bad: %#v", in.Extra) + + wantAnswer := []dns.RR{ + &dns.NS{ + Hdr: dns.RR_Header{Name: "consul.", Rrtype: dns.TypeNS, Class: dns.ClassINET, Ttl: 0, Rdlength: 0x2}, + Ns: "server1.node.dc1.consul.", + }, } - if aRec.Hdr.Name != "server1.node.dc1.consul." { - t.Fatalf("Bad: %#v", in.Extra) + verify.Values(t, "answer", in.Answer, wantAnswer) + wantExtra := []dns.RR{ + &dns.AAAA{ + Hdr: dns.RR_Header{Name: "server1.node.dc1.consul.", Rrtype: dns.TypeAAAA, Class: dns.ClassINET, Rdlength: 0x10, Ttl: 0}, + AAAA: net.ParseIP("::1"), + }, } + verify.Values(t, "extra", in.Extra, wantExtra) + } func TestDNS_ExternalServiceToConsulCNAMENestedLookup(t *testing.T) { @@ -4773,7 +4812,9 @@ func TestDNSInvalidRegex(t *testing.T) { {"Valid Hostname", "testnode", false}, {"Valid Hostname", "test-node", false}, {"Invalid Hostname with special chars", "test#$$!node", true}, - {"Invalid Hostname with special chars in the end", "test-node%^", true}, + {"Invalid Hostname with special chars in the end", "testnode%^", true}, + {"Whitespace", " ", true}, + {"Only special chars", "./$", true}, } for _, test := range tests { t.Run(test.desc, func(t *testing.T) { From e1bcbc6832c6f528e379529e6b78c773dacee067 Mon Sep 17 00:00:00 2001 From: Frank Schroeder Date: Mon, 7 Aug 2017 11:09:04 +0200 Subject: [PATCH 09/14] dns: drop CNAME for primary name server --- agent/dns.go | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/agent/dns.go b/agent/dns.go index 3245a4ea8..279e1144a 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -216,23 +216,6 @@ func (d *DNSServer) handleQuery(resp dns.ResponseWriter, req *dns.Msg) { m.Answer = append(m.Answer, d.soa()) m.Ns = append(m.Ns, ns...) m.Extra = append(m.Extra, glue...) - - // add CNAMEs for "ns." to the Extra - // section to make the MNAME entry in the SOA - // record resolvable - for _, rr := range ns { - cname := &dns.CNAME{ - Hdr: dns.RR_Header{ - Name: "ns." + d.domain, - Rrtype: dns.TypeCNAME, - Class: dns.ClassINET, - Ttl: uint32(d.config.NodeTTL / time.Second), - }, - Target: rr.(*dns.NS).Ns, - } - m.Extra = append(m.Extra, cname) - } - m.SetRcode(req, dns.RcodeSuccess) case dns.TypeNS: From 98de22e13e7085b8aaaeb0776366d013bfc4073b Mon Sep 17 00:00:00 2001 From: Frank Schroeder Date: Mon, 7 Aug 2017 11:09:41 +0200 Subject: [PATCH 10/14] dns: we do not support zone transfers --- agent/dns.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/agent/dns.go b/agent/dns.go index 279e1144a..fb2d2cacb 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -224,6 +224,9 @@ func (d *DNSServer) handleQuery(resp dns.ResponseWriter, req *dns.Msg) { m.Extra = glue m.SetRcode(req, dns.RcodeSuccess) + case dns.TypeAXFR: + m.SetRcode(req, dns.RcodeNotImplemented) + default: d.dispatch(network, req, m) } From 7b39af2b2de30309c6ade45a28b41d3d596401e8 Mon Sep 17 00:00:00 2001 From: Frank Schroeder Date: Mon, 7 Aug 2017 11:09:58 +0200 Subject: [PATCH 11/14] dns: postmaster -> hostmaster --- agent/dns.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/dns.go b/agent/dns.go index fb2d2cacb..4808b9ac8 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -254,7 +254,7 @@ func (d *DNSServer) soa() *dns.SOA { Serial: uint32(time.Now().Unix()), // todo(fs): make these configurable - Mbox: "postmaster." + d.domain, + Mbox: "hostmaster." + d.domain, Refresh: 3600, Retry: 600, Expire: 86400, From b571cb8097a31ed700b3613afd0737c88ab94f0f Mon Sep 17 00:00:00 2001 From: Frank Schroeder Date: Mon, 7 Aug 2017 11:10:22 +0200 Subject: [PATCH 12/14] dns: keep NS names in consul domain --- agent/dns.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/agent/dns.go b/agent/dns.go index 4808b9ac8..92e956341 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -272,7 +272,7 @@ func (d *DNSServer) addSOA(msg *dns.Msg) { func (d *DNSServer) nameservers(edns bool) (ns []dns.RR, extra []dns.RR) { // get server names and store them in a map to randomize the output servers := map[string]net.IP{} - for name, addr := range d.agent.delegate.ServerAddrs() { + for _, addr := range d.agent.delegate.ServerAddrs() { host, _, err := net.SplitHostPort(addr) if err != nil { d.logger.Println("[WARN] Unable to parse address %v, got error: %v", addr, err) @@ -283,17 +283,9 @@ func (d *DNSServer) nameservers(edns bool) (ns []dns.RR, extra []dns.RR) { continue } - // name is "name.dc" and domain is "consul." - // we want "name.node.dc.consul." - lastdot := strings.LastIndexByte(name, '.') - nodeName := name[:lastdot] - if InvalidDnsRe.MatchString(nodeName) { - d.logger.Printf("[WARN] dns: Node name %q is not a valid dns host name, will not be added to NS record", nodeName) - continue - } - fqdn := nodeName + ".node" + name[lastdot:] + "." + d.domain - // create a consistent, unique and sanitized name for the server + r := strings.NewReplacer(".", "-", ":", "-") + fqdn := "server-" + r.Replace(host) + "." + d.domain fqdn = dns.Fqdn(strings.ToLower(fqdn)) servers[fqdn] = ip From 2df084968ce7e39c4c082d7d6db8b778d2c106ed Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Mon, 7 Aug 2017 16:02:33 -0500 Subject: [PATCH 13/14] Go back to using .node.dc.consul as the name of the ns record being returned. --- agent/dns.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/agent/dns.go b/agent/dns.go index 92e956341..4808b9ac8 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -272,7 +272,7 @@ func (d *DNSServer) addSOA(msg *dns.Msg) { func (d *DNSServer) nameservers(edns bool) (ns []dns.RR, extra []dns.RR) { // get server names and store them in a map to randomize the output servers := map[string]net.IP{} - for _, addr := range d.agent.delegate.ServerAddrs() { + for name, addr := range d.agent.delegate.ServerAddrs() { host, _, err := net.SplitHostPort(addr) if err != nil { d.logger.Println("[WARN] Unable to parse address %v, got error: %v", addr, err) @@ -283,9 +283,17 @@ func (d *DNSServer) nameservers(edns bool) (ns []dns.RR, extra []dns.RR) { continue } + // name is "name.dc" and domain is "consul." + // we want "name.node.dc.consul." + lastdot := strings.LastIndexByte(name, '.') + nodeName := name[:lastdot] + if InvalidDnsRe.MatchString(nodeName) { + d.logger.Printf("[WARN] dns: Node name %q is not a valid dns host name, will not be added to NS record", nodeName) + continue + } + fqdn := nodeName + ".node" + name[lastdot:] + "." + d.domain + // create a consistent, unique and sanitized name for the server - r := strings.NewReplacer(".", "-", ":", "-") - fqdn := "server-" + r.Replace(host) + "." + d.domain fqdn = dns.Fqdn(strings.ToLower(fqdn)) servers[fqdn] = ip From 0f4986dcc79334c848aee1888e8de75ccc547958 Mon Sep 17 00:00:00 2001 From: Frank Schroeder Date: Tue, 8 Aug 2017 13:55:58 +0200 Subject: [PATCH 14/14] dns: minor cleanups --- agent/dns.go | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/agent/dns.go b/agent/dns.go index 4808b9ac8..64e9226a3 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -278,22 +278,23 @@ func (d *DNSServer) nameservers(edns bool) (ns []dns.RR, extra []dns.RR) { d.logger.Println("[WARN] Unable to parse address %v, got error: %v", addr, err) continue } + ip := net.ParseIP(host) if ip == nil { continue } - // name is "name.dc" and domain is "consul." - // we want "name.node.dc.consul." + // Use "NODENAME.node.DC.DOMAIN" as a unique name for the server + // since we use that name in other places as well. + // 'name' is "NODENAME.DC" so we need to split it + // to construct the server name. lastdot := strings.LastIndexByte(name, '.') - nodeName := name[:lastdot] + nodeName, dc := name[:lastdot], name[lastdot:] if InvalidDnsRe.MatchString(nodeName) { d.logger.Printf("[WARN] dns: Node name %q is not a valid dns host name, will not be added to NS record", nodeName) continue } - fqdn := nodeName + ".node" + name[lastdot:] + "." + d.domain - - // create a consistent, unique and sanitized name for the server + fqdn := nodeName + ".node" + dc + "." + d.domain fqdn = dns.Fqdn(strings.ToLower(fqdn)) servers[fqdn] = ip @@ -304,19 +305,21 @@ func (d *DNSServer) nameservers(edns bool) (ns []dns.RR, extra []dns.RR) { } for name, ip := range servers { - // the name server record + // NS record nsrr := &dns.NS{ Hdr: dns.RR_Header{ Name: d.domain, Rrtype: dns.TypeNS, Class: dns.ClassINET, - Ttl: 0, + Ttl: uint32(d.config.NodeTTL / time.Second), }, Ns: name, } ns = append(ns, nsrr) - // the glue record providing the ip address - extra = append(extra, d.formatNodeRecord(ip.String(), name, dns.TypeANY, d.config.NodeTTL, edns)...) + + // A or AAAA glue record + glue := d.formatNodeRecord(ip.String(), name, dns.TypeANY, d.config.NodeTTL, edns) + extra = append(extra, glue...) // don't provide more than 3 servers if len(ns) >= 3 { @@ -504,8 +507,7 @@ RPC: n := out.NodeServices.Node edns := req.IsEdns0() != nil addr := d.agent.TranslateAddress(datacenter, n.Address, n.TaggedAddresses) - records := d.formatNodeRecord(addr, - req.Question[0].Name, qType, d.config.NodeTTL, edns) + records := d.formatNodeRecord(addr, req.Question[0].Name, qType, d.config.NodeTTL, edns) if records != nil { resp.Answer = append(resp.Answer, records...) }