From e5cadc561a8b21ce02fdbf2b405cfb64433ecffa Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Mon, 28 Jan 2019 14:59:58 -0500 Subject: [PATCH] Don't generate TXT records just to discard them (#5272) * Don't generate TXT records just to discard them * Add unit test for formatNodeRecord to ensure it prevents returning TXT records --- agent/dns.go | 40 +++++++++++++++++++++++++++++----------- agent/dns_test.go | 19 +++++++++++++++++++ 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/agent/dns.go b/agent/dns.go index 21f0cadc3..0b34a77bb 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -457,7 +457,7 @@ func (d *DNSServer) nameservers(edns bool, maxRecursionLevel int) (ns []dns.RR, } ns = append(ns, nsrr) - glue, meta := d.formatNodeRecord(nil, addr, fqdn, dns.TypeANY, d.config.NodeTTL, edns, maxRecursionLevel) + glue, meta := d.formatNodeRecord(nil, addr, fqdn, dns.TypeANY, d.config.NodeTTL, edns, maxRecursionLevel, d.config.NodeMetaTXT) extra = append(extra, glue...) if meta != nil && d.config.NodeMetaTXT { extra = append(extra, meta...) @@ -681,17 +681,26 @@ RPC: return } + generateMeta := false + metaInAnswer := false + if qType == dns.TypeANY || qType == dns.TypeTXT { + generateMeta = true + metaInAnswer = true + } else if d.config.NodeMetaTXT { + generateMeta = true + } + // Add the node record n := out.NodeServices.Node edns := req.IsEdns0() != nil addr := d.agent.TranslateAddress(datacenter, n.Address, n.TaggedAddresses) - records, meta := d.formatNodeRecord(out.NodeServices.Node, addr, req.Question[0].Name, qType, d.config.NodeTTL, edns, maxRecursionLevel) + records, meta := d.formatNodeRecord(out.NodeServices.Node, addr, req.Question[0].Name, qType, d.config.NodeTTL, edns, maxRecursionLevel, generateMeta) if records != nil { resp.Answer = append(resp.Answer, records...) } - if meta != nil && (qType == dns.TypeANY || qType == dns.TypeTXT) { + if meta != nil && metaInAnswer && generateMeta { resp.Answer = append(resp.Answer, meta...) - } else if meta != nil && d.config.NodeMetaTXT { + } else if meta != nil && generateMeta { resp.Extra = append(resp.Extra, meta...) } } @@ -722,7 +731,7 @@ func encodeKVasRFC1464(key, value string) (txt string) { // The return value is two slices. The first slice is the main answer slice (containing the A, AAAA, CNAME) RRs for the node // and the second slice contains any TXT RRs created from the node metadata. It is up to the caller to determine where the // generated RRs should go and if they should be used at all. -func (d *DNSServer) formatNodeRecord(node *structs.Node, addr, qName string, qType uint16, ttl time.Duration, edns bool, maxRecursionLevel int) (records, meta []dns.RR) { +func (d *DNSServer) formatNodeRecord(node *structs.Node, addr, qName string, qType uint16, ttl time.Duration, edns bool, maxRecursionLevel int, generateMeta bool) (records, meta []dns.RR) { // Parse the IP ip := net.ParseIP(addr) var ipv4 net.IP @@ -783,7 +792,7 @@ func (d *DNSServer) formatNodeRecord(node *structs.Node, addr, qName string, qTy } } - if node != nil { + if node != nil && generateMeta { for key, value := range node.Meta { txt := value if !strings.HasPrefix(strings.ToLower(key), "rfc1035-") { @@ -1016,7 +1025,7 @@ func (d *DNSServer) lookupServiceNodes(datacenter, service, tag string, connect Connect: connect, Datacenter: datacenter, ServiceName: service, - ServiceTags: []string{tag}, + ServiceTags: []string{tag}, TagFilter: tag != "", QueryOptions: structs.QueryOptions{ Token: d.agent.tokens.UserToken(), @@ -1246,9 +1255,18 @@ func (d *DNSServer) serviceNodeRecords(dc string, nodes structs.CheckServiceNode } handled[addr] = struct{}{} + generateMeta := false + metaInAnswer := false + if qType == dns.TypeANY || qType == dns.TypeTXT { + generateMeta = true + metaInAnswer = true + } else if d.config.NodeMetaTXT { + generateMeta = true + } + // Add the node record had_answer := false - records, meta := d.formatNodeRecord(node.Node, addr, qName, qType, ttl, edns, maxRecursionLevel) + records, meta := d.formatNodeRecord(node.Node, addr, qName, qType, ttl, edns, maxRecursionLevel, generateMeta) if records != nil { switch records[0].(type) { case *dns.CNAME: @@ -1263,10 +1281,10 @@ func (d *DNSServer) serviceNodeRecords(dc string, nodes structs.CheckServiceNode } } - if meta != nil && (qType == dns.TypeANY || qType == dns.TypeTXT) { + if meta != nil && generateMeta && metaInAnswer { resp.Answer = append(resp.Answer, meta...) had_answer = true - } else if meta != nil && d.config.NodeMetaTXT { + } else if meta != nil && generateMeta { resp.Extra = append(resp.Extra, meta...) } @@ -1367,7 +1385,7 @@ func (d *DNSServer) serviceSRVRecords(dc string, nodes structs.CheckServiceNodes } // Add the extra record - records, meta := d.formatNodeRecord(node.Node, addr, srvRec.Target, dns.TypeANY, ttl, edns, maxRecursionLevel) + records, meta := d.formatNodeRecord(node.Node, addr, srvRec.Target, dns.TypeANY, ttl, edns, maxRecursionLevel, d.config.NodeMetaTXT) 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 bb363aa7d..6f11399d7 100644 --- a/agent/dns_test.go +++ b/agent/dns_test.go @@ -6330,3 +6330,22 @@ func TestDNSInvalidRegex(t *testing.T) { } } + +func TestDNS_formatNodeRecord(t *testing.T) { + s := &DNSServer{} + + node := &structs.Node{ + Meta: map[string]string{ + "key": "value", + "key2": "value2", + }, + } + + records, meta := s.formatNodeRecord(node, "198.18.0.1", "test.node.consul", dns.TypeA, 5*time.Minute, false, 3, false) + require.Len(t, records, 1) + require.Len(t, meta, 0) + + records, meta = s.formatNodeRecord(node, "198.18.0.1", "test.node.consul", dns.TypeA, 5*time.Minute, false, 3, true) + require.Len(t, records, 1) + require.Len(t, meta, 2) +}