wipfran/conn25-dynamic-peerapi-dns-resolvers

8 files changed, 83 insertions, 188 deletions

diff --git a/appc/conn25.go b/appc/conn25.go
index fd1748fa6..20796612e 100644
--- a/appc/conn25.go
+++ b/appc/conn25.go
@@ -10,7 +10,6 @@ import (
 	"tailscale.com/ipn/ipnext"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/appctype"
-	"tailscale.com/util/mak"
 	"tailscale.com/util/set"
 )
 
@@ -53,50 +52,21 @@ func PickConnector(nb ipnext.NodeBackend, app appctype.Conn25Attr) []tailcfg.Nod
 	return matches
 }
 
-// PickSplitDNSPeers looks at the netmap peers capabilities and finds which peers
-// want to be connectors for which domains.
-func PickSplitDNSPeers(hasCap func(c tailcfg.NodeCapability) bool, self tailcfg.NodeView, peers map[tailcfg.NodeID]tailcfg.NodeView) map[string][]tailcfg.NodeView {
-	var m map[string][]tailcfg.NodeView
+func AppNameByDomain(hasCap func(c tailcfg.NodeCapability) bool, self tailcfg.NodeView) map[string]string {
 	if !hasCap(AppConnectorsExperimentalAttrName) {
-		return m
+		return nil
 	}
 	apps, err := tailcfg.UnmarshalNodeCapViewJSON[appctype.AppConnectorAttr](self.CapMap(), AppConnectorsExperimentalAttrName)
 	if err != nil {
-		return m
+		return nil
 	}
-	tagToDomain := make(map[string][]string)
+	appNamesByDomain := map[string]string{}
 	for _, app := range apps {
-		for _, tag := range app.Connectors {
-			tagToDomain[tag] = append(tagToDomain[tag], app.Domains...)
+		for _, domain := range app.Domains {
+			// in the case of multiple apps specifying the same domain (which is misconfiguration
+			// that should be validated at point of input) last write wins.
+			appNamesByDomain[domain] = app.Name
 		}
 	}
-	// NodeIDs are Comparable, and we have a map of NodeID to NodeView anyway, so
-	// use a Set of NodeIDs to deduplicate, and populate into a []NodeView later.
-	var work map[string]set.Set[tailcfg.NodeID]
-	for _, peer := range peers {
-		if !isEligibleConnector(peer) {
-			continue
-		}
-		for _, t := range peer.Tags().All() {
-			domains := tagToDomain[t]
-			for _, domain := range domains {
-				if work[domain] == nil {
-					mak.Set(&work, domain, set.Set[tailcfg.NodeID]{})
-				}
-				work[domain].Add(peer.ID())
-			}
-		}
-	}
-
-	// Populate m. Make a []tailcfg.NodeView from []tailcfg.NodeID using the peers map.
-	// And sort it to our preference.
-	for domain, ids := range work {
-		nodes := make([]tailcfg.NodeView, 0, ids.Len())
-		for id := range ids {
-			nodes = append(nodes, peers[id])
-		}
-		sortByPreference(nodes)
-		mak.Set(&m, domain, nodes)
-	}
-	return m
+	return appNamesByDomain
 }
diff --git a/appc/conn25_test.go b/appc/conn25_test.go
index fc14caf36..872d51b34 100644
--- a/appc/conn25_test.go
+++ b/appc/conn25_test.go
@@ -4,8 +4,6 @@
 package appc
 
 import (
-	"encoding/json"
-	"reflect"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -15,125 +13,6 @@ import (
 	"tailscale.com/types/opt"
 )
 
-func TestPickSplitDNSPeers(t *testing.T) {
-	getBytesForAttr := func(name string, domains []string, tags []string) []byte {
-		attr := appctype.AppConnectorAttr{
-			Name:       name,
-			Domains:    domains,
-			Connectors: tags,
-		}
-		bs, err := json.Marshal(attr)
-		if err != nil {
-			t.Fatalf("test setup: %v", err)
-		}
-		return bs
-	}
-	appOneBytes := getBytesForAttr("app1", []string{"example.com"}, []string{"tag:one"})
-	appTwoBytes := getBytesForAttr("app2", []string{"a.example.com"}, []string{"tag:two"})
-	appThreeBytes := getBytesForAttr("app3", []string{"woo.b.example.com", "hoo.b.example.com"}, []string{"tag:three1", "tag:three2"})
-	appFourBytes := getBytesForAttr("app4", []string{"woo.b.example.com", "c.example.com"}, []string{"tag:four1", "tag:four2"})
-
-	makeNodeView := func(id tailcfg.NodeID, name string, tags []string) tailcfg.NodeView {
-		return (&tailcfg.Node{
-			ID:       id,
-			Name:     name,
-			Tags:     tags,
-			Hostinfo: (&tailcfg.Hostinfo{AppConnector: opt.NewBool(true)}).View(),
-		}).View()
-	}
-	nvp1 := makeNodeView(1, "p1", []string{"tag:one"})
-	nvp2 := makeNodeView(2, "p2", []string{"tag:four1", "tag:four2"})
-	nvp3 := makeNodeView(3, "p3", []string{"tag:two", "tag:three1"})
-	nvp4 := makeNodeView(4, "p4", []string{"tag:two", "tag:three2", "tag:four2"})
-
-	for _, tt := range []struct {
-		name   string
-		want   map[string][]tailcfg.NodeView
-		peers  []tailcfg.NodeView
-		config []tailcfg.RawMessage
-	}{
-		{
-			name: "empty",
-		},
-		{
-			name:   "bad-config", // bad config should return a nil map rather than error.
-			config: []tailcfg.RawMessage{tailcfg.RawMessage(`hey`)},
-		},
-		{
-			name:   "no-peers",
-			config: []tailcfg.RawMessage{tailcfg.RawMessage(appOneBytes)},
-		},
-		{
-			name:   "peers-that-are-not-connectors",
-			config: []tailcfg.RawMessage{tailcfg.RawMessage(appOneBytes)},
-			peers: []tailcfg.NodeView{
-				(&tailcfg.Node{
-					ID:   5,
-					Name: "p5",
-					Tags: []string{"tag:one"},
-				}).View(),
-				(&tailcfg.Node{
-					ID:   6,
-					Name: "p6",
-					Tags: []string{"tag:one"},
-				}).View(),
-			},
-		},
-		{
-			name:   "peers-that-dont-match-tags",
-			config: []tailcfg.RawMessage{tailcfg.RawMessage(appOneBytes)},
-			peers: []tailcfg.NodeView{
-				makeNodeView(5, "p5", []string{"tag:seven"}),
-				makeNodeView(6, "p6", nil),
-			},
-		},
-		{
-			name: "matching-tagged-connector-peers",
-			config: []tailcfg.RawMessage{
-				tailcfg.RawMessage(appOneBytes),
-				tailcfg.RawMessage(appTwoBytes),
-				tailcfg.RawMessage(appThreeBytes),
-				tailcfg.RawMessage(appFourBytes),
-			},
-			peers: []tailcfg.NodeView{
-				nvp1,
-				nvp2,
-				nvp3,
-				nvp4,
-				makeNodeView(5, "p5", nil),
-			},
-			want: map[string][]tailcfg.NodeView{
-				// p5 has no matching tags and so doesn't appear
-				"example.com":       {nvp1},
-				"a.example.com":     {nvp3, nvp4},
-				"woo.b.example.com": {nvp2, nvp3, nvp4},
-				"hoo.b.example.com": {nvp3, nvp4},
-				"c.example.com":     {nvp2, nvp4},
-			},
-		},
-	} {
-		t.Run(tt.name, func(t *testing.T) {
-			selfNode := &tailcfg.Node{}
-			if tt.config != nil {
-				selfNode.CapMap = tailcfg.NodeCapMap{
-					tailcfg.NodeCapability(AppConnectorsExperimentalAttrName): tt.config,
-				}
-			}
-			selfView := selfNode.View()
-			peers := map[tailcfg.NodeID]tailcfg.NodeView{}
-			for _, p := range tt.peers {
-				peers[p.ID()] = p
-			}
-			got := PickSplitDNSPeers(func(_ tailcfg.NodeCapability) bool {
-				return true
-			}, selfView, peers)
-			if !reflect.DeepEqual(got, tt.want) {
-				t.Fatalf("got %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
 type testNodeBackend struct {
 	ipnext.NodeBackend
 	peers []tailcfg.NodeView
diff --git a/feature/conn25/conn25.go b/feature/conn25/conn25.go
index e716c09d0..d2eb29fdf 100644
--- a/feature/conn25/conn25.go
+++ b/feature/conn25/conn25.go
@@ -27,6 +27,7 @@ import (
 	"tailscale.com/feature"
 	"tailscale.com/ipn/ipnext"
 	"tailscale.com/ipn/ipnlocal"
+	"tailscale.com/net/dns/resolver"
 	"tailscale.com/net/packet"
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/net/tstun"
@@ -151,6 +152,24 @@ func (e *extension) installHooks(dph *datapathHandler) error {
 		return e.conn25.mapDNSResponse(bs)
 	})
 
+	if !resolver.FranNewDynamicResolverThing.IsSet() {
+		resolver.FranNewDynamicResolverThing.Set(func(appName string) (string, error) {
+			if !e.conn25.isConfigured() {
+				return "", errors.New("conn25 not configured")
+			}
+			cfg := e.conn25.client.getConfig()
+			app, ok := cfg.appsByName[appName]
+			if !ok {
+				return "", errors.New("no app found for app name")
+			}
+			_, urlBase := e.pickConnectorURLBase(app)
+			if urlBase == "" {
+				return "", errors.New("no peer found for app")
+			}
+			return urlBase + "/dns-query", nil
+		})
+	}
+
 	// Intercept packets from the tun device and from WireGuard
 	// to perform DNAT and SNAT.
 	tun.PreFilterPacketOutboundToWireGuardAppConnectorIntercept = func(p *packet.Parsed, _ *tstun.Wrapper) filter.Response {
@@ -801,13 +820,7 @@ func makePeerAPIReq(ctx context.Context, httpClient *http.Client, urlBase string
 	return nil
 }
 
-func (e *extension) sendAddressAssignment(ctx context.Context, as addrs) (tailcfg.NodeView, error) {
-	app, ok := e.conn25.client.getConfig().appsByName[as.app]
-	if !ok {
-		e.conn25.client.logf("App not found for app: %s (domain: %s)", as.app, as.domain)
-		return tailcfg.NodeView{}, errors.New("app not found")
-	}
-
+func (e *extension) pickConnectorURLBase(app appctype.Conn25Attr) (tailcfg.NodeView, string) {
 	nb := e.host.NodeBackend()
 	peers := appc.PickConnector(nb, app)
 	var urlBase string
@@ -819,6 +832,16 @@ func (e *extension) sendAddressAssignment(ctx context.Context, as addrs) (tailcf
 			break
 		}
 	}
+	return conn, urlBase
+}
+
+func (e *extension) sendAddressAssignment(ctx context.Context, as addrs) (tailcfg.NodeView, error) {
+	app, ok := e.conn25.client.getConfig().appsByName[as.app]
+	if !ok {
+		e.conn25.client.logf("App not found for app: %s (domain: %s)", as.app, as.domain)
+		return tailcfg.NodeView{}, errors.New("app not found")
+	}
+	conn, urlBase := e.pickConnectorURLBase(app)
 	if urlBase == "" {
 		return tailcfg.NodeView{}, errors.New("no connector peer found to handle address assignment")
 	}
diff --git a/ipn/ipnlocal/node_backend.go b/ipn/ipnlocal/node_backend.go
index 75550b3d5..408ca2435 100644
--- a/ipn/ipnlocal/node_backend.go
+++ b/ipn/ipnlocal/node_backend.go
@@ -6,7 +6,6 @@ package ipnlocal
 import (
 	"cmp"
 	"context"
-	"fmt"
 	"net/netip"
 	"slices"
 	"sync"
@@ -864,18 +863,11 @@ func dnsConfigForNetmap(nm *netmap.NetworkMap, peers map[tailcfg.NodeID]tailcfg.
 	addSplitDNSRoutes(nm.DNS.Routes)
 
 	// Add split DNS routes for conn25
-	conn25DNSTargets := appc.PickSplitDNSPeers(nm.HasCap, nm.SelfNode, peers)
-	if conn25DNSTargets != nil {
+	appMap := appc.AppNameByDomain(nm.HasCap, nm.SelfNode)
+	if appMap != nil {
 		var m map[string][]*dnstype.Resolver
-		for domain, candidateSplitDNSPeers := range conn25DNSTargets {
-			for _, peer := range candidateSplitDNSPeers {
-				base := peerAPIBase(nm, peer)
-				if base == "" {
-					continue
-				}
-				mak.Set(&m, domain, []*dnstype.Resolver{{Addr: fmt.Sprintf("%s/dns-query", base)}})
-				break // Just make one resolver for the first peer we can get a peerAPIBase for.
-			}
+		for domain, appName := range appMap {
+			mak.Set(&m, domain, []*dnstype.Resolver{{Addr: appName, IsFranNewDynamicResolverThing: true}})
 		}
 		if m != nil {
 			addSplitDNSRoutes(m)
diff --git a/net/dns/resolver/forwarder.go b/net/dns/resolver/forwarder.go
index ed7ff78f7..992539b1b 100644
--- a/net/dns/resolver/forwarder.go
+++ b/net/dns/resolver/forwarder.go
@@ -47,6 +47,9 @@ import (
 	"tailscale.com/version"
 )
 
+// TODO comment, name
+var FranNewDynamicResolverThing feature.Hook[func(string) (string, error)]
+
 // headerBytes is the number of bytes in a DNS message header.
 const headerBytes = 12
 
@@ -1004,7 +1007,30 @@ func (f *forwarder) resolvers(domain dnsname.FQDN) []resolverAndDelay {
 	f.mu.Unlock()
 	for _, route := range routes {
 		if route.Suffix == "." || route.Suffix.Contains(domain) {
-			return route.Resolvers
+			triedToResolveResolver := false
+			resolvers := []resolverAndDelay{}
+			for _, r := range route.Resolvers {
+				if r.name.IsFranNewDynamicResolverThing {
+					triedToResolveResolver = true
+					fx := FranNewDynamicResolverThing.Get()
+					if fx != nil {
+						url, err := fx(r.name.Addr)
+						if err != nil {
+							continue
+						}
+						r.name.Addr = url
+					}
+				}
+				resolvers = append(resolvers, r)
+			}
+			// if there turned out to actually be no resolvers for this route Suffix then
+			// if the user configured that on purpose?? let it be, but if it's because
+			// there's a dynamic resolver that might have covered this domain but elected
+			// not to or was unable to do so here, then the route doesn't count.
+			if triedToResolveResolver && len(resolvers) == 0 {
+				continue
+			}
+			return resolvers
 		}
 	}
 	return cloudHostFallback // or nil if no fallback
diff --git a/types/dnstype/dnstype.go b/types/dnstype/dnstype.go
index 1cd38d383..149b159fd 100644
--- a/types/dnstype/dnstype.go
+++ b/types/dnstype/dnstype.go
@@ -41,6 +41,8 @@ type Resolver struct {
 	// there are situations where it is preferable to still use a Split DNS server and/or
 	// global DNS server instead of the exit node.
 	UseWithExitNode bool `json:",omitempty"`
+
+	IsFranNewDynamicResolverThing bool `json:",omitempty"`
 }
 
 // IPPort returns r.Addr as an IP address and port if either
diff --git a/types/dnstype/dnstype_clone.go b/types/dnstype/dnstype_clone.go
index e690ebaec..d95cd9479 100644
--- a/types/dnstype/dnstype_clone.go
+++ b/types/dnstype/dnstype_clone.go
@@ -23,9 +23,10 @@ func (src *Resolver) Clone() *Resolver {
 
 // A compilation failure here means this code must be regenerated, with the command at the top of this file.
 var _ResolverCloneNeedsRegeneration = Resolver(struct {
-	Addr                string
-	BootstrapResolution []netip.Addr
-	UseWithExitNode     bool
+	Addr                          string
+	BootstrapResolution           []netip.Addr
+	UseWithExitNode               bool
+	IsFranNewDynamicResolverThing bool
 }{})
 
 // Clone duplicates src into dst and reports whether it succeeded.
diff --git a/types/dnstype/dnstype_view.go b/types/dnstype/dnstype_view.go
index c91feb6b8..a9618fa53 100644
--- a/types/dnstype/dnstype_view.go
+++ b/types/dnstype/dnstype_view.go
@@ -113,12 +113,14 @@ func (v ResolverView) BootstrapResolution() views.Slice[netip.Addr] {
 // exit node is in use. Normally, DNS resolution is delegated to the exit node but
 // there are situations where it is preferable to still use a Split DNS server and/or
 // global DNS server instead of the exit node.
-func (v ResolverView) UseWithExitNode() bool      { return v.ж.UseWithExitNode }
-func (v ResolverView) Equal(v2 ResolverView) bool { return v.ж.Equal(v2.ж) }
+func (v ResolverView) UseWithExitNode() bool               { return v.ж.UseWithExitNode }
+func (v ResolverView) IsFranNewDynamicResolverThing() bool { return v.ж.IsFranNewDynamicResolverThing }
+func (v ResolverView) Equal(v2 ResolverView) bool          { return v.ж.Equal(v2.ж) }
 
 // A compilation failure here means this code must be regenerated, with the command at the top of this file.
 var _ResolverViewNeedsRegeneration = Resolver(struct {
-	Addr                string
-	BootstrapResolution []netip.Addr
-	UseWithExitNode     bool
+	Addr                          string
+	BootstrapResolution           []netip.Addr
+	UseWithExitNode               bool
+	IsFranNewDynamicResolverThing bool
 }{})