From b9d1e7042a7bf466d18a1d63a2844528570d2143 Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Tue, 19 Jun 2018 10:08:16 -0400 Subject: [PATCH] Make filtering out TXT RRs only apply when they would end up in Additional section ANY queries are no longer affected. --- agent/config/config.go | 2 +- agent/dns.go | 24 +++++++++++------------ agent/dns_test.go | 15 ++++++++------ website/source/docs/agent/options.html.md | 9 +++++---- 4 files changed, 27 insertions(+), 23 deletions(-) diff --git a/agent/config/config.go b/agent/config/config.go index 48227d955..13f7db6a9 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -360,7 +360,7 @@ type DNS struct { RecursorTimeout *string `json:"recursor_timeout,omitempty" hcl:"recursor_timeout" mapstructure:"recursor_timeout"` ServiceTTL map[string]string `json:"service_ttl,omitempty" hcl:"service_ttl" mapstructure:"service_ttl"` UDPAnswerLimit *int `json:"udp_answer_limit,omitempty" hcl:"udp_answer_limit" mapstructure:"udp_answer_limit"` - NodeMetaTXT *bool `json:"additional_node_meta_txt,omitempty" hcl:"additional_node_meta_txt" mapstructure:"additional_node_meta_txt"` + NodeMetaTXT *bool `json:"enable_additional_node_meta_txt,omitempty" hcl:"enable_additional_node_meta_txt" mapstructure:"enable_additional_node_meta_txt"` } type HTTPConfig struct { diff --git a/agent/dns.go b/agent/dns.go index 1b8c2e20c..993511d0d 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -376,7 +376,7 @@ func (d *DNSServer) nameservers(edns bool) (ns []dns.RR, extra []dns.RR) { } ns = append(ns, nsrr) - glue := d.formatNodeRecord(nil, addr, fqdn, dns.TypeANY, d.config.NodeTTL, edns) + glue := d.formatNodeRecord(nil, addr, fqdn, dns.TypeANY, d.config.NodeTTL, edns, false) extra = append(extra, glue...) // don't provide more than 3 servers @@ -584,7 +584,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, req.Question[0].Name, qType, d.config.NodeTTL, edns) + records := d.formatNodeRecord(out.NodeServices.Node, addr, req.Question[0].Name, qType, d.config.NodeTTL, edns, true) if records != nil { resp.Answer = append(resp.Answer, records...) } @@ -612,7 +612,7 @@ func encodeKVasRFC1464(key, value string) (txt string) { } // formatNodeRecord takes a Node and returns an A, AAAA, TXT 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(node *structs.Node, addr, qName string, qType uint16, ttl time.Duration, edns, answer bool) (records []dns.RR) { // Parse the IP ip := net.ParseIP(addr) var ipv4 net.IP @@ -673,17 +673,17 @@ func (d *DNSServer) formatNodeRecord(node *structs.Node, addr, qName string, qTy } } - node_meta_txt := true + node_meta_txt := false if node == nil { node_meta_txt = false - } else if qType == dns.TypeANY { - // Since any RR type is requested allow the configuration to - // determine whether or not node meta gets added as TXT records + } else if answer { + node_meta_txt = true + } else { + // Use configuration when the TXT RR would + // end up in the Additional section of the + // DNS response node_meta_txt = d.config.NodeMetaTXT - } else if qType != dns.TypeTXT { - // qType isn't TXT or ANY so avoid emitting the TXT records - node_meta_txt = false } if node_meta_txt { @@ -1158,7 +1158,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(node.Node, addr, qName, qType, ttl, edns, true) if records != nil { resp.Answer = append(resp.Answer, records...) count++ @@ -1207,7 +1207,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(node.Node, addr, srvRec.Target, dns.TypeANY, ttl, edns, false) 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 454d598c3..a171132e2 100644 --- a/agent/dns_test.go +++ b/agent/dns_test.go @@ -473,7 +473,7 @@ func TestDNS_NodeLookup_TXT(t *testing.T) { } func TestDNS_NodeLookup_TXT_DontSuppress(t *testing.T) { - a := NewTestAgent(t.Name(), `dns_config = { additional_node_meta_txt = false }`) + a := NewTestAgent(t.Name(), `dns_config = { enable_additional_node_meta_txt = false }`) defer a.Shutdown() args := &structs.RegisterRequest{ @@ -555,11 +555,10 @@ func TestDNS_NodeLookup_ANY(t *testing.T) { }, } verify.Values(t, "answer", in.Answer, wantAnswer) - } -func TestDNS_NodeLookup_ANY_SuppressTXT(t *testing.T) { - a := NewTestAgent(t.Name(), `dns_config = { additional_node_meta_txt = false }`) +func TestDNS_NodeLookup_ANY_DontSuppressTXT(t *testing.T) { + a := NewTestAgent(t.Name(), `dns_config = { enable_additional_node_meta_txt = false }`) defer a.Shutdown() args := &structs.RegisterRequest{ @@ -590,6 +589,10 @@ func TestDNS_NodeLookup_ANY_SuppressTXT(t *testing.T) { Hdr: dns.RR_Header{Name: "bar.node.consul.", Rrtype: dns.TypeA, Class: dns.ClassINET, Rdlength: 0x4}, A: []byte{0x7f, 0x0, 0x0, 0x1}, // 127.0.0.1 }, + &dns.TXT{ + Hdr: dns.RR_Header{Name: "bar.node.consul.", Rrtype: dns.TypeTXT, Class: dns.ClassINET, Rdlength: 0xa}, + Txt: []string{"key=value"}, + }, } verify.Values(t, "answer", in.Answer, wantAnswer) } @@ -4695,7 +4698,7 @@ func TestDNS_ServiceLookup_FilterACL(t *testing.T) { } func TestDNS_ServiceLookup_MetaTXT(t *testing.T) { - a := NewTestAgent(t.Name(), `dns_config = { additional_node_meta_txt = true }`) + a := NewTestAgent(t.Name(), `dns_config = { enable_additional_node_meta_txt = true }`) defer a.Shutdown() args := &structs.RegisterRequest{ @@ -4740,7 +4743,7 @@ func TestDNS_ServiceLookup_MetaTXT(t *testing.T) { } func TestDNS_ServiceLookup_SuppressTXT(t *testing.T) { - a := NewTestAgent(t.Name(), `dns_config = { additional_node_meta_txt = false }`) + a := NewTestAgent(t.Name(), `dns_config = { enable_additional_node_meta_txt = false }`) defer a.Shutdown() // Register a node with a service. diff --git a/website/source/docs/agent/options.html.md b/website/source/docs/agent/options.html.md index 1e1e274b9..3f259e9ef 100644 --- a/website/source/docs/agent/options.html.md +++ b/website/source/docs/agent/options.html.md @@ -778,10 +778,11 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass be increasingly uncommon to need to change this value with modern resolvers). - * `additional_node_meta_txt` - If set - to false, node metadata will not be synthesized into TXT records and returned except for queries specifically for - TXT records. By default, TXT records will be generated for node queries with an ANY query type or for SRV queries - of services. + * `enable_additional_node_meta_txt` - + When set to true, Consul will add TXT records for Node metadata into the Additional section of the DNS responses for several + query types such as SRV queries. When set to false those records are emitted. This does not impact the behavior of those + same TXT records when they would be added to the Answer section of the response like when querying with type TXT or ANY. This + defaults to true. * `domain` Equivalent to the [`-domain` command-line flag](#_domain).