Avoid unnecessary DNS lookups from traffic-manager

The traffic-manager will make attempts to lookup using the client
namespace when a lookup fails. This commit ensures that it never makes
an attempt to add the namespace to a name that already has that suffix.

Signed-off-by: Thomas Hallgren <thomas@datawire.io>

cmd/traffic/cmd/manager/service.go

@@ -780,6 +780,39 @@ func (s *service) WatchDial(session *rpc.SessionInfo, stream rpc.Manager_WatchDi
 	}
 }
 
+// hasDomainSuffix checks if the given name is suffixed with the given suffix. The following
+// rules apply:
+//
+//   - The name must end with a dot.
+//   - The suffix may optionally end with a dot.
+//   - The suffix may not be empty.
+//   - The suffix match must follow after a dot in the name, or match the whole name.
+func hasDomainSuffix(name, suffix string) bool {
+	sl := len(suffix)
+	if sl == 0 {
+		return false
+	}
+	nl := len(name)
+	sfp := nl - sl
+	if sfp < 0 {
+		return false
+	}
+	if name[nl-1] != '.' {
+		return false
+	}
+	if suffix[sl-1] != '.' {
+		if sfp == 0 {
+			return false
+		}
+		sfp--
+		name = name[0 : nl-1]
+	}
+	if sfp == 0 {
+		return name == suffix
+	}
+	return name[sfp-1] == '.' && name[sfp:] == suffix
+}
+
 func (s *service) LookupDNS(ctx context.Context, request *rpc.DNSRequest) (*rpc.DNSResponse, error) {
 	ctx = managerutil.WithSessionInfo(ctx, request.GetSession())
 	qType := uint16(request.Type)
@@ -816,8 +849,8 @@ func (s *service) LookupDNS(ctx context.Context, request *rpc.DNSRequest) (*rpc.
 		dlog.Debugf(ctx, "LookupDNS on traffic-manager: %s", name)
 		rrs, rCode, err = dnsproxy.Lookup(ctx, qType, name)
 		if err != nil {
-			// Could still be x.y.<client namespace>
-			if client != nil && nDots > 1 && client.Namespace != tmNamespace && !strings.HasSuffix(name, s.ClusterInfo().ClusterDomain()) {
+			// Could still be x.y.<client namespace>, but let's avoid x.<cluster domain>.<client namespace> and x.<client-namespace>.<client namespace>
+			if client != nil && nDots > 1 && client.Namespace != tmNamespace && !hasDomainSuffix(name, s.ClusterInfo().ClusterDomain()) && !hasDomainSuffix(name, client.Namespace) {
 				name += client.Namespace + "."
 				restoreName = true
 				dlog.Debugf(ctx, "LookupDNS on traffic-manager: %s", name)

cmd/traffic/cmd/manager/service_test.go

@@ -1,4 +1,4 @@
-package manager_test
+package manager
 
 import (
 	"context"
@@ -23,7 +23,6 @@ import (
 	"github.com/datawire/dlib/dlog"
 	"github.com/datawire/k8sapi/pkg/k8sapi"
 	rpc "github.com/telepresenceio/telepresence/rpc/v2/manager"
-	"github.com/telepresenceio/telepresence/v2/cmd/traffic/cmd/manager"
 	"github.com/telepresenceio/telepresence/v2/cmd/traffic/cmd/manager/managerutil"
 	"github.com/telepresenceio/telepresence/v2/cmd/traffic/cmd/manager/mutator"
 	testdata "github.com/telepresenceio/telepresence/v2/cmd/traffic/cmd/manager/test"
@@ -329,7 +328,7 @@ func getTestClientConn(ctx context.Context, t *testing.T) *grpc.ClientConn {
 	agentmap.GeneratorConfigFunc = env.GeneratorConfig
 
 	s := grpc.NewServer()
-	mgr, g, err := manager.NewServiceFunc(ctx)
+	mgr, g, err := NewServiceFunc(ctx)
 	if err != nil {
 		t.Fatalf("failed to build manager: %v", err)
 	}
@@ -353,3 +352,74 @@ func getTestClientConn(ctx context.Context, t *testing.T) *grpc.ClientConn {
 	})
 	return conn
 }
+
+func Test_hasDomainSuffix(t *testing.T) {
+	tests := []struct {
+		name   string
+		qn     string
+		suffix string
+		want   bool
+	}{
+		{
+			"empty suffix",
+			"aa.bb.",
+			"",
+			false,
+		},
+		{
+			"suffix with dot",
+			"aa.bb.",
+			"bb.",
+			true,
+		},
+		{
+			"suffix without dot",
+			"aa.bb.",
+			"bb",
+			true,
+		},
+		{
+			"suffix partial match",
+			"aa.bb.",
+			"b.",
+			false,
+		},
+		{
+			"suffix partial match no dot",
+			"foo.bar.",
+			"b",
+			false,
+		},
+		{
+			"name without dot",
+			"aa.bb",
+			"bb",
+			false,
+		},
+		{
+			"equal",
+			"a.",
+			"a.",
+			true,
+		},
+		{
+			"equal no dot",
+			"a.",
+			"a",
+			true,
+		},
+		{
+			"empty qn",
+			".",
+			"a",
+			false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := hasDomainSuffix(tt.qn, tt.suffix); got != tt.want {
+				t.Errorf("hasDomainSuffix() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}