Fix problem with exclude-suffix being hidden by DNS search path.

In some situations, a name ending with an exclude-suffix like "xyz.com" would be expanded by a search path into "xyz.com.<connected namespace>" and therefore not be excluded. Instead, the name was sent to the cluster to be resolved, causing an unnecessary load on its DNS server. Signed-off-by: Thomas Hallgren <[email protected]>
telepresenceio · Jan 10, 2025 · 198d06a · 198d06a
1 parent cd17f65
commit 198d06a
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 20 deletions.
diff --git a/CHANGELOG.yml b/CHANGELOG.yml
@@ -106,6 +106,12 @@ items:
         body: >-
           Using `telepresence list --namespace <ns> with a namespace different from the one that telepresence was
           connected to, would cause a deadlock, and then produce an empty list.
+      - type: bugfix
+        title: Fix problem with exclude-suffix being hidden by DNS search path.
+        body: >-
+          In some situations, a name ending with an exclude-suffix like "xyz.com" would be expanded by a search path
+          into "xyz.com.<connected namespace>" and therefore not be excluded. Instead, the name was sent to the cluster
+          to be resolved, causing an unnecessary load on its DNS server.
   - version: 2.21.1
     date: 2024-12-17
     notes:

diff --git a/docs/release-notes.md b/docs/release-notes.md
@@ -90,6 +90,12 @@ macOS based systems will often PTR queries using nameslike `b._dns-sd._udp`, lb.
 Using `telepresence list --namespace <ns> with a namespace different from the one that telepresence was connected to, would cause a deadlock, and then produce an empty list.
 </div>
 
+## <div style="display:flex;"><img src="images/bugfix.png" alt="bugfix" style="width:30px;height:fit-content;"/><div style="display:flex;margin-left:7px;">Fix problem with exclude-suffix being hidden by DNS search path.</div></div>
+<div style="margin-left: 15px">
+
+In some situations, a name ending with an exclude-suffix like "xyz.com" would be expanded by a search path into "xyz.com.<connected namespace>" and therefore not be excluded. Instead, the name was sent to the cluster to be resolved, causing an unnecessary load on its DNS server.
+</div>
+
 ## Version 2.21.1 <span style="font-size: 16px;">(December 17)</span>
 ## <div style="display:flex;"><img src="images/bugfix.png" alt="bugfix" style="width:30px;height:fit-content;"/><div style="display:flex;margin-left:7px;">[Allow ingest of serverless deployments without specifying an inject-container-ports annotation](https://github.com/telepresenceio/telepresence/issues/3741)</div></div>
 <div style="margin-left: 15px">

diff --git a/docs/release-notes.mdx b/docs/release-notes.mdx
@@ -80,6 +80,10 @@ namespaceSelector:
 	<Title type="bugfix">Using the --namespace option with telepresence causes a deadlock.</Title>
 	<Body>Using `telepresence list --namespace <ns> with a namespace different from the one that telepresence was connected to, would cause a deadlock, and then produce an empty list.</Body>
 </Note>
+<Note>
+	<Title type="bugfix">Fix problem with exclude-suffix being hidden by DNS search path.</Title>
+	<Body>In some situations, a name ending with an exclude-suffix like "xyz.com" would be expanded by a search path into "xyz.com.<connected namespace>" and therefore not be excluded. Instead, the name was sent to the cluster to be resolved, causing an unnecessary load on its DNS server.</Body>
+</Note>
 ## Version 2.21.1 <span style={{fontSize:'16px'}}>(December 17)</span>
 <Note>
 	<Title type="bugfix" docs="https://github.com/telepresenceio/telepresence/issues/3741">Allow ingest of serverless deployments without specifying an inject-container-ports annotation</Title>

diff --git a/integration_test/kubeconfig_extension_test.go b/integration_test/kubeconfig_extension_test.go
@@ -315,7 +315,7 @@ func (s *notConnectedSuite) Test_DNSSuffixRules() {
 			defaults.IncludeSuffixes,
 			defaults.ExcludeSuffixes,
 			[]string{
-				`Lookup A "` + randomName + randomDomain,
+				`Lookup A "` + randomName + randomDomain + `."`,
 			},
 		},
 		{
@@ -325,8 +325,8 @@ func (s *notConnectedSuite) Test_DNSSuffixRules() {
 			nil,
 			nil,
 			[]string{
-				`Cluster DNS included by include-suffix "` + randomDomain + `" for name "` + randomName + randomDomain,
-				`Lookup A "` + randomName + randomDomain,
+				`Cluster DNS included by include-suffix "` + randomDomain + `" for name "` + randomName + randomDomain + `"`,
+				`Lookup A "` + randomName + randomDomain + `."`,
 			},
 			true,
 			[]string{randomDomain},
@@ -340,8 +340,8 @@ func (s *notConnectedSuite) Test_DNSSuffixRules() {
 			nil,
 			[]string{randomDomain},
 			[]string{
-				`Cluster DNS included by include-suffix "` + randomDomain + `" for name "` + randomName + randomDomain,
-				`Lookup A "` + randomName + randomDomain,
+				`Cluster DNS included by include-suffix "` + randomDomain + `" for name "` + randomName + randomDomain + `"`,
+				`Lookup A "` + randomName + randomDomain + `."`,
 			},
 			true,
 			[]string{randomDomain},
@@ -355,8 +355,8 @@ func (s *notConnectedSuite) Test_DNSSuffixRules() {
 			nil,
 			[]string{randomDomain2},
 			[]string{
-				`Cluster DNS included by include-suffix "` + randomDomain + `" for name "` + randomName + randomDomain,
-				`Lookup A "` + randomName + randomDomain,
+				`Cluster DNS included by include-suffix "` + randomDomain + `" for name "` + randomName + randomDomain + `"`,
+				`Lookup A "` + randomName + randomDomain + `."`,
 			},
 			true,
 			[]string{randomDomain},
@@ -456,7 +456,7 @@ func (s *notConnectedSuite) Test_DNSSuffixRules() {
 			_, _ = net.DefaultResolver.LookupIPAddr(short, tt.domainName)
 
 			// Give query time to reach telepresence and produce a log entry
-			dtime.SleepWithContext(ctx, 100*time.Millisecond)
+			dtime.SleepWithContext(ctx, 500*time.Millisecond)
 
 			for _, wl := range tt.wantedLogEntry {
 				_, err = rootLog.Seek(pos, io.SeekStart)

diff --git a/pkg/client/rootd/dns/server.go b/pkg/client/rootd/dns/server.go
@@ -227,32 +227,48 @@ func (s *Server) shouldDoClusterLookup(query string) bool {
 
 	// Skip configured exclude-suffixes unless also matched by an include-suffix
 	// that is longer (i.e. more specific).
-	for _, es := range s.ExcludeSuffixes {
-		if strings.HasSuffix(name, es) {
-			// Exclude unless more specific include.
-			for _, is := range s.IncludeSuffixes {
-				if len(is) >= len(es) && strings.HasSuffix(name, is) {
-					dlog.Debugf(s.ctx,
-						"Cluster DNS included by include-suffix %q (overriding exclude-suffix %q) for name %q", is, es, name)
-					return true
+	suffixExcluded := func(n string) (included, excluded bool) {
+		for _, es := range s.ExcludeSuffixes {
+			if strings.HasSuffix(n, es) {
+				// Exclude unless more specific include.
+				for _, is := range s.IncludeSuffixes {
+					if len(is) >= len(es) && strings.HasSuffix(n, is) {
+						dlog.Debugf(s.ctx,
+							"Cluster DNS included by include-suffix %q (overriding exclude-suffix %q) for name %q", is, es, n)
+						return true, false
+					}
 				}
+				dlog.Debugf(s.ctx, "Cluster DNS excluded by exclude-suffix %q for name %q", es, n)
+				return false, true
 			}
-			dlog.Debugf(s.ctx, "Cluster DNS excluded by exclude-suffix %q for name %q", es, name)
-			return false
 		}
+		return false, false
+	}
+
+	if include, exclude := suffixExcluded(name); include || exclude {
+		return include
 	}
 
 	// Always include configured search paths
+	ln := len(name)
 	for _, sfx := range s.search {
-		if strings.HasSuffix(name, sfx) {
+		li := ln - len(sfx) - 1
+		if li > 0 && name[li] == '.' && strings.HasSuffix(name, sfx) {
+			if include, exclude := suffixExcluded(name[:li]); include || exclude {
+				return include
+			}
 			dlog.Debugf(s.ctx, "Cluster DNS included by search %q of name %q", sfx, name)
 			return true
 		}
 	}
 
 	// Always include configured routes
 	for sfx := range s.routes {
-		if strings.HasSuffix(name, sfx) {
+		li := ln - len(sfx) - 1
+		if li > 0 && name[li] == '.' && strings.HasSuffix(name, sfx) {
+			if include, exclude := suffixExcluded(name[:li]); include || exclude {
+				return include
+			}
 			dlog.Debugf(s.ctx, "Cluster DNS included by namespace %q of name %q", sfx, name)
 			return true
 		}
@@ -267,6 +283,11 @@ func (s *Server) shouldDoClusterLookup(query string) bool {
 	// Always include configured includeSuffixes
 	for _, sfx := range s.IncludeSuffixes {
 		if strings.HasSuffix(name, sfx) {
+			if sfx[0] == '.' {
+				if include, exclude := suffixExcluded(strings.TrimSuffix(name, sfx)); include || exclude {
+					return include
+				}
+			}
 			dlog.Debugf(s.ctx,
 				"Cluster DNS included by include-suffix %q for name %q", sfx, name)
 			return true