From 6c6ce14d5d6ccc556a21f36d82a24f5a24eddc3a Mon Sep 17 00:00:00 2001
From: Dhia Ayachi <dhia@hashicorp.com>
Date: Tue, 18 Jul 2023 10:03:41 -0400
Subject: [PATCH] Fix a bug that wrongly trims domains when there is an overlap
 with DC 1.16.x (#18164)

Fix a bug that wrongly trims domains when there is an overlap with DC name (#17160)

* Fix a bug that wrongly trims domains when there is an overlap with DC name

Before this change, when DC name and domain/alt-domain overlap, the domain name incorrectly trimmed from the query.

Example:

Given: datacenter = dc-test, alt-domain = test.consul.
Querying for "test-node.node.dc-test.consul" will faile, because the
code was trimming "test.consul" instead of just ".consul"

This change, fixes the issue by adding dot (.) before trimming

* trimDomain: ensure domain trimmed without modyfing original domains

* update changelog

---------

Co-authored-by: Alex Simenduev <shamil.si@gmail.com>
---
 .changelog/17160.txt |  3 +++
 agent/dns.go         |  2 +-
 agent/dns_test.go    | 39 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 .changelog/17160.txt

diff --git a/.changelog/17160.txt b/.changelog/17160.txt
new file mode 100644
index 000000000..666a6e8f2
--- /dev/null
+++ b/.changelog/17160.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+Fix a bug that wrongly trims domains when there is an overlap with DC name.
+```
diff --git a/agent/dns.go b/agent/dns.go
index cb1e3c310..5804dc97d 100644
--- a/agent/dns.go
+++ b/agent/dns.go
@@ -1055,7 +1055,7 @@ func (d *DNSServer) trimDomain(query string) string {
 		longer, shorter = shorter, longer
 	}
 
-	if strings.HasSuffix(query, longer) {
+	if strings.HasSuffix(query, "."+strings.TrimLeft(longer, ".")) {
 		return strings.TrimSuffix(query, longer)
 	}
 	return strings.TrimSuffix(query, shorter)
diff --git a/agent/dns_test.go b/agent/dns_test.go
index 46a7e758c..ef5364964 100644
--- a/agent/dns_test.go
+++ b/agent/dns_test.go
@@ -7071,6 +7071,45 @@ func TestDNS_AltDomains_Overlap(t *testing.T) {
 	}
 }
 
+func TestDNS_AltDomain_DCName_Overlap(t *testing.T) {
+	if testing.Short() {
+		t.Skip("too slow for testing.Short")
+	}
+
+	// this tests the DC name overlap with the consul domain/alt-domain
+	// we should get response when DC suffix is a prefix of consul alt-domain
+	t.Parallel()
+	a := NewTestAgent(t, `
+		datacenter = "dc-test"
+		node_name = "test-node"
+		alt_domain = "test.consul."
+	`)
+	defer a.Shutdown()
+	testrpc.WaitForLeader(t, a.RPC, "dc-test")
+
+	questions := []string{
+		"test-node.node.dc-test.consul.",
+		"test-node.node.dc-test.test.consul.",
+	}
+
+	for _, question := range questions {
+		m := new(dns.Msg)
+		m.SetQuestion(question, dns.TypeA)
+
+		c := new(dns.Client)
+		in, _, err := c.Exchange(m, a.DNSAddr())
+		if err != nil {
+			t.Fatalf("err: %v", err)
+		}
+
+		require.Len(t, in.Answer, 1)
+
+		aRec, ok := in.Answer[0].(*dns.A)
+		require.True(t, ok)
+		require.Equal(t, aRec.A.To4().String(), "127.0.0.1")
+	}
+}
+
 func TestDNS_PreparedQuery_AllowStale(t *testing.T) {
 	if testing.Short() {
 		t.Skip("too slow for testing.Short")