Merge pull request #3572 from telepresenceio/thallgren/dns-tlds

Clean up confusion concerning DNS TLDs, search paths, and namespaces.

pkg/client/rootd/dns/resolved_linux.go

@@ -118,6 +118,9 @@ func (s *Server) updateLinkDomains(c context.Context, dev vif.Device) error {
 	// used as a filter that will direct queries for names ending with them to this resolver. Routes must be
 	// prefixed with "~".
 	for _, sfx := range s.includeSuffixes {
+		if !strings.HasSuffix(sfx, ".") {
+			sfx += "."
+		}
 		paths[i] = "~" + strings.TrimPrefix(sfx, ".")
 		i++
 	}

pkg/client/rootd/dns/server.go

@@ -69,6 +69,11 @@ var DefaultExcludeSuffixes = []string{
 	".ru",
 }
 
+type nsAndDomains struct {
+	domains   []string
+	namespace string
+}
+
 // Server is a DNS server which implements the github.com/miekg/dns Handler interface.
 type Server struct {
 	sync.RWMutex
@@ -96,8 +101,8 @@ type Server struct {
 	// used by the darwin resolver to keep track of files to add or remove.
 	domains map[string]struct{}
 
-	// searchPathCh receives requests to change the search path.
-	searchPathCh chan []string
+	// nsAndDomainsCh receives requests to change the top level domains and the search path.
+	nsAndDomainsCh chan nsAndDomains
 
 	includeSuffixes []string
 
@@ -176,7 +181,7 @@ func NewServer(config *rpc.DNSConfig, clusterLookup Resolver) *Server {
 		remoteIP:        config.RemoteIp,
 		dropSuffixes:    []string{tel2SubDomainDot},
 		search:          []string{tel2SubDomain},
-		searchPathCh:    make(chan []string, 5),
+		nsAndDomainsCh:  make(chan nsAndDomains, 5),
 		clusterDomain:   defaultClusterDomain,
 		clusterLookup:   clusterLookup,
 		ready:           make(chan struct{}),
@@ -187,16 +192,20 @@ func NewServer(config *rpc.DNSConfig, clusterLookup Resolver) *Server {
 	return s
 }
 
-// tel2SubDomain fixes a search-path problem when using Docker.
+// tel2SubDomain helps differentiate between single label and qualified DNS queries.
 //
-// Docker uses its own search-path for single label names. This means that the search path that is
-// declared in Telepresence DNS resolver is ignored, although the rest of the DNS-resolution works
-// OK. Since the search-path is likely to change during a session, a stable fake domain is needed
-// to emulate the search-path. That fake-domain can then be used in the search path declared in the
-// Docker config.
+// Dealing with single label names is tricky because what we really want is to receive the
+// name and then forward it verbatim to the DNS resolver in the cluster so that it can
+// add whatever search paths to it that it sees fit, but in order to receive single name
+// queries in the first place, our DNS resolver must have a search path that adds a domain
+// that the DNS system knows that we will handle.
 //
-// The tel2SubDomain fills this purpose and a request for "<single label name>.<tel2SubDomain>"
-// will be resolved as "<single label name>.<currently intercepted namespace>".
+// Example flow:
+// The user queries for the name "alpha". The DNS system on the host tries the search path
+// of our DNS resolver which contains "tel2-search" and creates the name "alpha.tel2-search".
+// The DNS system now discovers that our DNS resolver handles that domain, so we receive
+// the query. We then strip the "tel2-search" and send the original single label name to the
+// cluster, and we add it back before we forward the reply.
 const (
 	tel2SubDomain    = "tel2-search"
 	tel2SubDomainDot = tel2SubDomain + "."
@@ -412,23 +421,15 @@ func (s *Server) SetClusterDNS(dns *manager.DNS, remoteIP net.IP) {
 	s.Unlock()
 }
 
-// SetSearchPath updates the DNS search path used by the resolver.
-func (s *Server) SetSearchPath(ctx context.Context, paths, namespaces []string) {
-	allPaths := make([]string, 0, len(paths)+len(namespaces)+1)
-	allPaths = append(allPaths, tel2SubDomain)
-	if len(namespaces) > 0 {
-		// Provide direct access to intercepted namespaces
-		s.RLock()
-		for _, ns := range namespaces {
-			paths = append(paths, ns+".svc."+s.clusterDomain)
-		}
-		s.RUnlock()
+// SetTopLevelDomainsAndSearchPath updates the DNS top level domains and the search path used by the resolver.
+func (s *Server) SetTopLevelDomainsAndSearchPath(ctx context.Context, domains []string, namespace string) {
+	das := nsAndDomains{
+		domains:   domains,
+		namespace: namespace,
 	}
-	allPaths = append(allPaths, paths...)
-
 	select {
 	case <-ctx.Done():
-	case s.searchPathCh <- allPaths:
+	case s.nsAndDomainsCh <- das:
 	}
 }
 
@@ -493,46 +494,38 @@ func newLocalUDPListener(c context.Context) (net.PacketConn, error) {
 func (s *Server) processSearchPaths(g *dgroup.Group, processor func(context.Context, vif.Device) error, dev vif.Device) {
 	g.Go("SearchPaths", func(c context.Context) error {
 		s.performRecursionCheck(c)
-		prevPaths := s.search
-		unchanged := func(paths []string) bool {
-			if len(paths) != len(prevPaths) {
-				return false
-			}
-			for i, path := range paths {
-				if path != prevPaths[i] {
-					return false
-				}
-			}
-			return true
+		prevDas := nsAndDomains{
+			domains:   []string{},
+			namespace: "",
+		}
+		unchanged := func(das nsAndDomains) bool {
+			return das.namespace == prevDas.namespace && slices.Equal(das.domains, prevDas.domains)
 		}
 
 		for {
 			select {
 			case <-c.Done():
 				return nil
-			case paths := <-s.searchPathCh:
+			case das := <-s.nsAndDomainsCh:
 				// Only interested in the last one, and only if it differs
-				if len(s.searchPathCh) > 0 || unchanged(paths) {
+				if len(s.nsAndDomainsCh) > 0 || unchanged(das) {
 					continue
 				}
+				prevDas = das
 
-				dlog.Debugf(c, "%v -> %v", prevPaths, paths)
-				prevPaths = make([]string, len(paths))
-				copy(prevPaths, paths)
-
-				routes := make(map[string]struct{})
-				search := make([]string, 0)
-				for _, path := range paths {
-					path = strings.ToLower(path)
-					if path == tel2SubDomain || strings.ContainsRune(path, '.') {
-						search = append(search, path)
-					} else if path != "" {
-						routes[path] = struct{}{}
+				routes := make(map[string]struct{}, len(das.domains))
+				for _, domain := range das.domains {
+					if domain != "" {
+						routes[domain] = struct{}{}
 					}
 				}
 				s.Lock()
 				s.routes = routes
-				s.search = search
+
+				// The connected namespace must be included as a search path for the cases
+				// where it's up to the traffic-manager to resolve. It cannot resolve a single
+				// label name intended for other namespaces.
+				s.search = []string{tel2SubDomain, das.namespace}
 				s.Unlock()
 
 				if err := processor(c, dev); err != nil {

pkg/client/rootd/in_process.go

@@ -44,15 +44,15 @@ type InProcSession struct {
 	cancel context.CancelFunc
 }
 
-func (rd *InProcSession) Version(ctx context.Context, in *empty.Empty, opts ...grpc.CallOption) (*common.VersionInfo, error) {
+func (rd *InProcSession) Version(context.Context, *empty.Empty, ...grpc.CallOption) (*common.VersionInfo, error) {
 	return &common.VersionInfo{
 		ApiVersion: client.APIVersion,
 		Version:    client.Version(),
 		Name:       client.DisplayName,
 	}, nil
 }
 
-func (rd *InProcSession) Status(ctx context.Context, in *empty.Empty, opts ...grpc.CallOption) (*rpc.DaemonStatus, error) {
+func (rd *InProcSession) Status(context.Context, *empty.Empty, ...grpc.CallOption) (*rpc.DaemonStatus, error) {
 	nc := rd.getNetworkConfig()
 	return &rpc.DaemonStatus{
 		Version: &common.VersionInfo{
@@ -65,52 +65,52 @@ func (rd *InProcSession) Status(ctx context.Context, in *empty.Empty, opts ...gr
 	}, nil
 }
 
-func (rd *InProcSession) Quit(ctx context.Context, in *empty.Empty, opts ...grpc.CallOption) (*empty.Empty, error) {
+func (rd *InProcSession) Quit(context.Context, *empty.Empty, ...grpc.CallOption) (*empty.Empty, error) {
 	rd.cancel()
 	return &empty.Empty{}, nil
 }
 
-func (rd *InProcSession) Connect(ctx context.Context, in *rpc.OutboundInfo, opts ...grpc.CallOption) (*rpc.DaemonStatus, error) {
+func (rd *InProcSession) Connect(ctx context.Context, _ *rpc.OutboundInfo, opts ...grpc.CallOption) (*rpc.DaemonStatus, error) {
 	return rd.Status(ctx, nil, opts...)
 }
 
-func (rd *InProcSession) Disconnect(ctx context.Context, in *empty.Empty, opts ...grpc.CallOption) (*empty.Empty, error) {
+func (rd *InProcSession) Disconnect(context.Context, *empty.Empty, ...grpc.CallOption) (*empty.Empty, error) {
 	rd.cancel()
 	return &empty.Empty{}, nil
 }
 
-func (rd *InProcSession) GetNetworkConfig(ctx context.Context, in *empty.Empty, opts ...grpc.CallOption) (*rpc.NetworkConfig, error) {
+func (rd *InProcSession) GetNetworkConfig(context.Context, *empty.Empty, ...grpc.CallOption) (*rpc.NetworkConfig, error) {
 	return rd.getNetworkConfig(), nil
 }
 
-func (rd *InProcSession) SetDnsSearchPath(ctx context.Context, paths *rpc.Paths, opts ...grpc.CallOption) (*empty.Empty, error) {
-	rd.SetSearchPath(ctx, paths.Paths, paths.Namespaces)
+func (rd *InProcSession) SetDNSTopLevelDomains(ctx context.Context, in *rpc.Domains, _ ...grpc.CallOption) (*empty.Empty, error) {
+	rd.SetTopLevelDomains(ctx, in.Domains)
 	return &empty.Empty{}, nil
 }
 
-func (rd *InProcSession) SetDNSExcludes(ctx context.Context, in *rpc.SetDNSExcludesRequest, opts ...grpc.CallOption) (*empty.Empty, error) {
+func (rd *InProcSession) SetDNSExcludes(ctx context.Context, in *rpc.SetDNSExcludesRequest, _ ...grpc.CallOption) (*empty.Empty, error) {
 	rd.SetExcludes(ctx, in.Excludes)
 	return &empty.Empty{}, nil
 }
 
-func (rd *InProcSession) SetDNSMappings(ctx context.Context, in *rpc.SetDNSMappingsRequest, opts ...grpc.CallOption) (*empty.Empty, error) {
+func (rd *InProcSession) SetDNSMappings(ctx context.Context, in *rpc.SetDNSMappingsRequest, _ ...grpc.CallOption) (*empty.Empty, error) {
 	rd.SetMappings(ctx, in.Mappings)
 	return &empty.Empty{}, nil
 }
 
-func (rd *InProcSession) SetLogLevel(ctx context.Context, in *manager.LogLevelRequest, opts ...grpc.CallOption) (*empty.Empty, error) {
+func (rd *InProcSession) SetLogLevel(context.Context, *manager.LogLevelRequest, ...grpc.CallOption) (*empty.Empty, error) {
 	// No loglevel when session runs in the same process as the user daemon.
 	return &empty.Empty{}, nil
 }
 
-func (rd *InProcSession) WaitForNetwork(ctx context.Context, in *empty.Empty, opts ...grpc.CallOption) (*empty.Empty, error) {
+func (rd *InProcSession) WaitForNetwork(ctx context.Context, _ *empty.Empty, _ ...grpc.CallOption) (*empty.Empty, error) {
 	if err, ok := <-rd.networkReady(ctx); ok {
 		return &empty.Empty{}, status.Error(codes.Unavailable, err.Error())
 	}
 	return &empty.Empty{}, nil
 }
 
-func (rd *InProcSession) WaitForAgentIP(ctx context.Context, request *rpc.WaitForAgentIPRequest, opts ...grpc.CallOption) (*empty.Empty, error) {
+func (rd *InProcSession) WaitForAgentIP(ctx context.Context, request *rpc.WaitForAgentIPRequest, _ ...grpc.CallOption) (*empty.Empty, error) {
 	return rd.waitForAgentIP(ctx, request)
 }
 

pkg/client/rootd/service.go

@@ -165,9 +165,9 @@ func (s *Service) Quit(ctx context.Context, _ *emptypb.Empty) (*emptypb.Empty, e
 	return &emptypb.Empty{}, nil
 }
 
-func (s *Service) SetDnsSearchPath(ctx context.Context, paths *rpc.Paths) (*emptypb.Empty, error) {
+func (s *Service) SetDNSTopLevelDomains(ctx context.Context, domains *rpc.Domains) (*emptypb.Empty, error) {
 	err := s.WithSession(func(ctx context.Context, session *Session) error {
-		session.SetSearchPath(ctx, paths.Paths, paths.Namespaces)
+		session.SetTopLevelDomains(ctx, domains.Domains)
 		return nil
 	})
 	return &emptypb.Empty{}, err

pkg/client/rootd/session.go

@@ -390,7 +390,7 @@ func newSession(c context.Context, mi *rpc.OutboundInfo, mc connector.ManagerPro
 	dlog.Infof(c, "allow-conflicting subnets %v", s.allowConflictingSubnets)
 
 	s.dnsServer = dns.NewServer(mi.Dns, s.clusterLookup)
-	s.SetSearchPath(c, nil, nil)
+	s.SetTopLevelDomains(c, nil)
 	return s, nil
 }
 
@@ -1223,8 +1223,8 @@ func (s *Session) waitForProxyViaWorkloads(ctx context.Context) error {
 	return nil
 }
 
-func (s *Session) SetSearchPath(ctx context.Context, paths []string, namespaces []string) {
-	s.dnsServer.SetSearchPath(ctx, paths, namespaces)
+func (s *Session) SetTopLevelDomains(ctx context.Context, topLevelDomains []string) {
+	s.dnsServer.SetTopLevelDomainsAndSearchPath(ctx, topLevelDomains, s.namespace)
 }
 
 func (s *Session) SetExcludes(ctx context.Context, excludes []string) {

pkg/client/userd/trafficmgr/session.go

@@ -515,21 +515,16 @@ func connectError(t rpc.ConnectInfo_ErrType, err error) *rpc.ConnectInfo {
 // send it to the DNS-resolver in the daemon.
 func (s *session) updateDaemonNamespaces(c context.Context) {
 	s.wlWatcher.setNamespacesToWatch(c, s.GetCurrentNamespaces(true))
-	var namespaces []string
-	if s.Namespace != "" {
-		namespaces = []string{s.Namespace}
-	}
-	// Avoid being locked for the remainder of this function.
 
 	// Pass current mapped namespaces as plain names (no ending dot). The DNS-resolver will
 	// create special mapping for those, allowing names like myservice.mynamespace to be resolved
-	paths := s.GetCurrentNamespaces(false)
-	dlog.Debugf(c, "posting search paths %v and namespaces %v", paths, namespaces)
+	namespaces := s.GetCurrentNamespaces(false)
+	dlog.Debugf(c, "posting namespaces %v", namespaces)
 
-	if _, err := s.rootDaemon.SetDnsSearchPath(c, &rootdRpc.Paths{Paths: paths, Namespaces: namespaces}); err != nil {
-		dlog.Errorf(c, "error posting search paths %v and namespaces %v to root daemon: %v", paths, namespaces, err)
+	if _, err := s.rootDaemon.SetDNSTopLevelDomains(c, &rootdRpc.Domains{Domains: namespaces}); err != nil {
+		dlog.Errorf(c, "error posting namespaces %v to root daemon: %v", namespaces, err)
 	}
-	dlog.Debug(c, "search paths posted successfully")
+	dlog.Debug(c, "namespaces posted successfully")
 }
 
 func (s *session) Epilog(ctx context.Context) {
@@ -710,8 +705,6 @@ func (s *session) ensureWatchers(ctx context.Context,
 	wg.Add(len(namespaces))
 	for _, ns := range namespaces {
 		if ns == "" {
-			// Don't use tm.ActualNamespace here because the accessibility of the namespace
-			// is actually determined once the watcher starts
 			ns = s.Namespace
 		}
 		wgp := &wg
@@ -738,14 +731,8 @@ func (s *session) workloadInfoSnapshot(
 
 	var nss []string
 	if filter == rpc.ListRequest_INTERCEPTS {
-		// Special case, we don't care about namespaces in general. Instead, we use the intercepted namespaces
-		if s.Namespace != "" {
-			nss = []string{s.Namespace}
-		}
-		if len(nss) == 0 {
-			// No active intercepts
-			return &rpc.WorkloadInfoSnapshot{}, nil
-		}
+		// Special case, we don't care about namespaces in general. Instead, we use the connected namespace
+		nss = []string{s.Namespace}
 	} else {
 		nss = make([]string, 0, len(namespaces))
 		for _, ns := range namespaces {