From 35d852fd9a2a925b989065504ced95b9221856d6 Mon Sep 17 00:00:00 2001
From: Pierre Souchay <pierresouchay@users.noreply.github.com>
Date: Mon, 22 Jun 2020 15:01:48 +0200
Subject: [PATCH] Returns DNS Error NSDOMAIN when DC does not exists (#8103)

This will allow to increase cache value when DC is not valid (aka
return SOA to avoid too many consecutive requests) and will
distinguish DC being temporarily not available from DC not existing.

Implements https://github.com/hashicorp/consul/issues/8102
---
 agent/dns.go      | 36 ++++++++++++++++++++++++++++--------
 agent/dns_test.go | 21 +++++++++++++++++++++
 2 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/agent/dns.go b/agent/dns.go
index 8d339ffc8a..bbaae35826 100644
--- a/agent/dns.go
+++ b/agent/dns.go
@@ -819,6 +819,18 @@ func (d *DNSServer) trimDomain(query string) string {
 	return strings.TrimSuffix(query, shorter)
 }
 
+// computeRCode Return the DNS Error code from Consul Error
+func (d *DNSServer) computeRCode(err error) int {
+	if err == nil {
+		return dns.RcodeSuccess
+	}
+	dErr := err.Error()
+	if dErr == structs.ErrNoDCPath.Error() || dErr == consul.ErrQueryNotFound.Error() {
+		return dns.RcodeNameError
+	}
+	return dns.RcodeServerFailure
+}
+
 // nodeLookup is used to handle a node query
 func (d *DNSServer) nodeLookup(cfg *dnsConfig, network, datacenter, node string, req, resp *dns.Msg, maxRecursionLevel int) {
 	// Only handle ANY, A, AAAA, and TXT type requests
@@ -839,7 +851,11 @@ func (d *DNSServer) nodeLookup(cfg *dnsConfig, network, datacenter, node string,
 	out, err := d.lookupNode(cfg, args)
 	if err != nil {
 		d.logger.Error("rpc error", "error", err)
-		resp.SetRcode(req, dns.RcodeServerFailure)
+		rCode := d.computeRCode(err)
+		if rCode == dns.RcodeNameError {
+			d.addSOA(cfg, resp)
+		}
+		resp.SetRcode(req, rCode)
 		return
 	}
 
@@ -1203,7 +1219,11 @@ func (d *DNSServer) serviceLookup(cfg *dnsConfig, lookup serviceLookup, req, res
 	out, err := d.lookupServiceNodes(cfg, lookup)
 	if err != nil {
 		d.logger.Error("rpc error", "error", err)
-		resp.SetRcode(req, dns.RcodeServerFailure)
+		rCode := d.computeRCode(err)
+		if rCode == dns.RcodeNameError {
+			d.addSOA(cfg, resp)
+		}
+		resp.SetRcode(req, rCode)
 		return
 	}
 
@@ -1297,12 +1317,12 @@ func (d *DNSServer) preparedQueryLookup(cfg *dnsConfig, network, datacenter, que
 	// If they give a bogus query name, treat that as a name error,
 	// not a full on server error. We have to use a string compare
 	// here since the RPC layer loses the type information.
-	if err != nil && err.Error() == consul.ErrQueryNotFound.Error() {
-		d.addSOA(cfg, resp)
-		resp.SetRcode(req, dns.RcodeNameError)
-		return
-	} else if err != nil {
-		resp.SetRcode(req, dns.RcodeServerFailure)
+	if err != nil {
+		rCode := d.computeRCode(err)
+		if rCode == dns.RcodeNameError {
+			d.addSOA(cfg, resp)
+		}
+		resp.SetRcode(req, rCode)
 		return
 	}
 
diff --git a/agent/dns_test.go b/agent/dns_test.go
index fb04b7fa86..fb6a5878bb 100644
--- a/agent/dns_test.go
+++ b/agent/dns_test.go
@@ -5790,6 +5790,27 @@ func TestDNS_AddressLookupIPV6(t *testing.T) {
 	}
 }
 
+func TestDNS_NonExistingDC(t *testing.T) {
+	t.Parallel()
+	a := NewTestAgent(t, "")
+	defer a.Shutdown()
+	testrpc.WaitForLeader(t, a.RPC, "dc1")
+
+	// lookup a non-existing node, we should receive a SOA
+	m := new(dns.Msg)
+	m.SetQuestion("consul.dc2.consul.", dns.TypeANY)
+
+	c := new(dns.Client)
+	in, _, err := c.Exchange(m, a.DNSAddr())
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	if in.Rcode != dns.RcodeNameError {
+		t.Fatalf("Expected RCode: %#v, had: %#v", dns.RcodeNameError, in.Rcode)
+	}
+}
+
 func TestDNS_NonExistingLookup(t *testing.T) {
 	t.Parallel()
 	a := NewTestAgent(t, "")