From 9df55f59955e81da23b11acb757b0fc6764f062b 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 5bc74798bd..cd1edd6294 100644
--- a/agent/dns_test.go
+++ b/agent/dns_test.go
@@ -5791,6 +5791,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, "")