From f9db3870975690de66e782f2cfb0906a94b78f2f 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 98f02b6924..99ee456936 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 6ee73e12b3..1f625463d7 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 3160ad73ce..bba2cf9807 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 ef149d0087..7272414f8a 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 315e3a55af..35d4580c1c 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 bf01bd1e85..1434650ff1 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 90e905d6aa..995ddf6c71 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 794d1afe441cc2ad0b5326a7cd6acac01534c192 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 35d4580c1c..a3dfa9af02 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 1434650ff1..6195cb85c2 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 37f75a393e0056cb12a7a0b480fc3f86b0570e8d 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 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() From 824fc4ee20b1ebe076f6e25bd8e6ecc89d584498 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 ebb3228f58..99700f405c 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 0678361706..8e458f6668 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 7ea11c2f45a3e045ded8115a81f9c9d44a55d462 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 8e458f6668..90020741e6 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 f01f17bda3f8e2902e19b80d0e7faaafd9591d99 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 90020741e6..ba65ce75dc 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 1b75631844..4bf5600553 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 76319f751d09a8c77ff9cff8302ce089b17a4ec5 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 ba65ce75dc..f28edae4e1 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 4bf5600553..5d0929ef3c 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 7f34dc08a577555e36a19530063a56f6abd71aa2 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 f28edae4e1..3245a4ea8c 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 5d0929ef3c..152f5f237b 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 76b2538915cc9528043bbcbee8f2213c2841b842 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 3245a4ea8c..279e1144a9 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 60608b455d9296b12478e78765461b30aadb12b2 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 279e1144a9..fb2d2cacbd 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 f17bf78bb1b5a4bfa6d019a4674c0c7feeb0c46e 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 fb2d2cacbd..4808b9ac82 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 8a9653bdf8f3fdf27e590e727fceeeebdceb8830 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 4808b9ac82..92e956341c 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 72ae8c8f33ba6526f78fc3616e90b0850c8379c2 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 92e956341c..4808b9ac82 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 9fa237ddb6e2b1a19296a701e1fd5e63b15af0fc 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 4808b9ac82..64e9226a38 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...) }