appc,cmd/sniproxy,ipn/ipnlocal: split sniproxy configuration code out of appc

The design changed during integration and testing, resulting in the
earlier implementation growing in the appc package to be intended now
only for the sniproxy implementation. That code is moved to it's final
location, and the current App Connector code is now renamed.

Updates tailscale/corp#15437

Signed-off-by: James Tucker <james@tailscale.com>

appc/appconnector.go

@@ -1,8 +1,12 @@
 // Copyright (c) Tailscale Inc & AUTHORS
 // SPDX-License-Identifier: BSD-3-Clause
 
-// Package appc implements App Connectors. An AppConnector provides domain
+// Package appc implements App Connectors.
-// oriented routing of traffic.
+// An AppConnector provides DNS domain oriented routing of traffic. An App
+// Connector becomes a DNS server for a peer, authoritative for the set of
+// configured domains. DNS resolution of the target domain triggers dynamic
+// publication of routes to ensure that traffic to the domain is routed through
+// the App Connector.
 package appc
 
 import (
@@ -17,12 +21,6 @@ import (
 	"tailscale.com/types/views"
 )
 
-/*
- * TODO(raggi): the sniproxy servicing portions of this package will be moved
- * into the sniproxy or deprecated at some point, when doing so is not
- * disruptive. At that time EmbeddedAppConnector can be renamed to AppConnector.
- */
-
 // RouteAdvertiser is an interface that allows the AppConnector to advertise
 // newly discovered routes that need to be served through the AppConnector.
 type RouteAdvertiser interface {
@@ -31,7 +29,7 @@ type RouteAdvertiser interface {
 	AdvertiseRoute(netip.Prefix) error
 }
 
-// EmbeddedAppConnector is an implementation of an AppConnector that performs
+// AppConnector is an implementation of an AppConnector that performs
 // its function as a subsystem inside of a tailscale node. At the control plane
 // side App Connector routing is configured in terms of domains rather than IP
 // addresses.
@@ -40,7 +38,7 @@ type RouteAdvertiser interface {
 // DNS requests for configured domains are observed. If the domains resolve to
 // routes not yet served by the AppConnector the local node configuration is
 // updated to advertise the new route.
-type EmbeddedAppConnector struct {
+type AppConnector struct {
 	logf            logger.Logf
 	routeAdvertiser RouteAdvertiser
 
@@ -51,9 +49,9 @@ type EmbeddedAppConnector struct {
 	domains map[string][]netip.Addr
 }
 
-// NewEmbeddedAppConnector creates a new EmbeddedAppConnector.
+// NewAppConnector creates a new AppConnector.
-func NewEmbeddedAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser) *EmbeddedAppConnector {
+func NewAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser) *AppConnector {
-	return &EmbeddedAppConnector{
+	return &AppConnector{
 		logf:            logger.WithPrefix(logf, "appc: "),
 		routeAdvertiser: routeAdvertiser,
 	}
@@ -62,7 +60,7 @@ func NewEmbeddedAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser)
 // UpdateDomains replaces the current set of configured domains with the
 // supplied set of domains. Domains must not contain a trailing dot, and should
 // be lower case.
-func (e *EmbeddedAppConnector) UpdateDomains(domains []string) {
+func (e *AppConnector) UpdateDomains(domains []string) {
 	e.mu.Lock()
 	defer e.mu.Unlock()
 
@@ -76,7 +74,7 @@ func (e *EmbeddedAppConnector) UpdateDomains(domains []string) {
 }
 
 // Domains returns the currently configured domain list.
-func (e *EmbeddedAppConnector) Domains() views.Slice[string] {
+func (e *AppConnector) Domains() views.Slice[string] {
 	e.mu.Lock()
 	defer e.mu.Unlock()
 
@@ -87,7 +85,7 @@ func (e *EmbeddedAppConnector) Domains() views.Slice[string] {
 // response is being returned over the PeerAPI. The response is parsed and
 // matched against the configured domains, if matched the routeAdvertiser is
 // advised to advertise the discovered route.
-func (e *EmbeddedAppConnector) ObserveDNSResponse(res []byte) {
+func (e *AppConnector) ObserveDNSResponse(res []byte) {
 	var p dnsmessage.Parser
 	if _, err := p.Start(res); err != nil {
 		return

appc/appconnector_test.go

@@ -14,7 +14,7 @@ import (
 )
 
 func TestUpdateDomains(t *testing.T) {
-	a := NewEmbeddedAppConnector(t.Logf, nil)
+	a := NewAppConnector(t.Logf, nil)
 	a.UpdateDomains([]string{"example.com"})
 	if got, want := a.Domains().AsSlice(), []string{"example.com"}; !slices.Equal(got, want) {
 		t.Errorf("got %v; want %v", got, want)
@@ -37,7 +37,7 @@ func TestUpdateDomains(t *testing.T) {
 
 func TestObserveDNSResponse(t *testing.T) {
 	rc := &routeCollector{}
-	a := NewEmbeddedAppConnector(t.Logf, rc)
+	a := NewAppConnector(t.Logf, rc)
 
 	// a has no domains configured, so it should not advertise any routes
 	a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8"))

cmd/sniproxy/handlers.go

@@ -1,7 +1,7 @@
 // Copyright (c) Tailscale Inc & AUTHORS
 // SPDX-License-Identifier: BSD-3-Clause
 
-package appc
+package main
 
 import (
 	"context"

cmd/sniproxy/handlers_test.go

@@ -1,7 +1,7 @@
 // Copyright (c) Tailscale Inc & AUTHORS
 // SPDX-License-Identifier: BSD-3-Clause
 
-package appc
+package main
 
 import (
 	"bytes"

cmd/sniproxy/server.go

@@ -1,8 +1,7 @@
 // Copyright (c) Tailscale Inc & AUTHORS
 // SPDX-License-Identifier: BSD-3-Clause
 
-// Package appc implements App Connectors.
+package main
-package appc
 
 import (
 	"expvar"

cmd/sniproxy/server_test.go

@@ -1,7 +1,7 @@
 // Copyright (c) Tailscale Inc & AUTHORS
 // SPDX-License-Identifier: BSD-3-Clause
 
-package appc
+package main
 
 import (
 	"net/netip"

cmd/sniproxy/sniproxy.go

@@ -22,8 +22,6 @@ import (
 	"strings"
 
 	"github.com/peterbourgon/ff/v3"
-	"golang.org/x/net/dns/dnsmessage"
-	"tailscale.com/appc"
 	"tailscale.com/client/tailscale"
 	"tailscale.com/hostinfo"
 	"tailscale.com/ipn"
@@ -38,8 +36,6 @@ import (
 
 const configCapKey = "tailscale.com/sniproxy"
 
-var tsMBox = dnsmessage.MustNewName("support.tailscale.com.")
-
 // portForward is the state for a single port forwarding entry, as passed to the --forward flag.
 type portForward struct {
 	Port        int
@@ -99,7 +95,7 @@ func main() {
 func run(ctx context.Context, ts *tsnet.Server, wgPort int, hostname string, promoteHTTPS bool, debugPort int, ports, forwards string) {
 	// Wire up Tailscale node + app connector server
 	hostinfo.SetApp("sniproxy")
-	var s server
+	var s sniproxy
 	s.ts = ts
 
 	s.ts.Port = uint16(wgPort)
@@ -110,7 +106,7 @@ func run(ctx context.Context, ts *tsnet.Server, wgPort int, hostname string, pro
 		log.Fatalf("LocalClient() failed: %v", err)
 	}
 	s.lc = lc
-	s.ts.RegisterFallbackTCPHandler(s.appc.HandleTCPFlow)
+	s.ts.RegisterFallbackTCPHandler(s.srv.HandleTCPFlow)
 
 	// Start special-purpose listeners: dns, http promotion, debug server
 	ln, err := s.ts.Listen("udp", ":53")
@@ -181,18 +177,18 @@ func run(ctx context.Context, ts *tsnet.Server, wgPort int, hostname string, pro
 			// on the command line. This is intentionally done after we advertise any routes
 			// because its never correct to advertise the nodes native IP addresses.
 			s.mergeConfigFromFlags(&c, ports, forwards)
-			s.appc.Configure(&c)
+			s.srv.Configure(&c)
 		}
 	}
 }
 
-type server struct {
+type sniproxy struct {
-	appc appc.Server
+	srv Server
 	ts  *tsnet.Server
 	lc  *tailscale.LocalClient
 }
 
-func (s *server) advertiseRoutesFromConfig(ctx context.Context, c *appctype.AppConnectorConfig) error {
+func (s *sniproxy) advertiseRoutesFromConfig(ctx context.Context, c *appctype.AppConnectorConfig) error {
 	// Collect the set of addresses to advertise, using a map
 	// to avoid duplicate entries.
 	addrs := map[netip.Addr]struct{}{}
@@ -224,7 +220,7 @@ func (s *server) advertiseRoutesFromConfig(ctx context.Context, c *appctype.AppC
 	return err
 }
 
-func (s *server) mergeConfigFromFlags(out *appctype.AppConnectorConfig, ports, forwards string) {
+func (s *sniproxy) mergeConfigFromFlags(out *appctype.AppConnectorConfig, ports, forwards string) {
 	ip4, ip6 := s.ts.TailscaleIPs()
 
 	sniConfigFromFlags := appctype.SNIProxyConfig{
@@ -276,18 +272,18 @@ func (s *server) mergeConfigFromFlags(out *appctype.AppConnectorConfig, ports, f
 	}
 }
 
-func (s *server) serveDNS(ln net.Listener) {
+func (s *sniproxy) serveDNS(ln net.Listener) {
 	for {
 		c, err := ln.Accept()
 		if err != nil {
 			log.Printf("serveDNS accept: %v", err)
 			return
 		}
-		go s.appc.HandleDNS(c.(nettype.ConnPacketConn))
+		go s.srv.HandleDNS(c.(nettype.ConnPacketConn))
 	}
 }
 
-func (s *server) promoteHTTPS(ln net.Listener) {
+func (s *sniproxy) promoteHTTPS(ln net.Listener) {
 	err := http.Serve(ln, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		http.Redirect(w, r, "https://"+r.Host+r.RequestURI, http.StatusFound)
 	}))

cmd/tailscaled/depaware.txt

@@ -216,7 +216,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         gvisor.dev/gvisor/pkg/tcpip/transport/udp                    from tailscale.com/net/tstun+
         gvisor.dev/gvisor/pkg/waiter                                 from gvisor.dev/gvisor/pkg/context+
         inet.af/peercred                                             from tailscale.com/ipn/ipnauth
-        inet.af/tcpproxy                                             from tailscale.com/appc
    W 💣 inet.af/wf                                                   from tailscale.com/wf
         nhooyr.io/websocket                                          from tailscale.com/derp/derphttp+
         nhooyr.io/websocket/internal/errd                            from nhooyr.io/websocket
@@ -321,7 +320,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/tstime/mono                                    from tailscale.com/net/tstun+
         tailscale.com/tstime/rate                                    from tailscale.com/wgengine/filter+
         tailscale.com/tsweb/varz                                     from tailscale.com/cmd/tailscaled
-        tailscale.com/types/appctype                                 from tailscale.com/appc+
+        tailscale.com/types/appctype                                 from tailscale.com/ipn/ipnlocal
         tailscale.com/types/dnstype                                  from tailscale.com/ipn/ipnlocal+
         tailscale.com/types/empty                                    from tailscale.com/ipn+
         tailscale.com/types/flagtype                                 from tailscale.com/cmd/tailscaled

ipn/ipnlocal/local.go

@@ -208,7 +208,7 @@ type LocalBackend struct {
 	httpTestClient *http.Client       // for controlclient. nil by default, used by tests.
 	ccGen          clientGen          // function for producing controlclient; lazily populated
 	sshServer      SSHServer          // or nil, initialized lazily.
-	appConnector   *appc.EmbeddedAppConnector // or nil, initialized when configured.
+	appConnector   *appc.AppConnector // or nil, initialized when configured.
 	webClient      webClient
 	notify         func(ipn.Notify)
 	cc             controlclient.Client
@@ -3250,7 +3250,7 @@ func (b *LocalBackend) reconfigAppConnectorLocked(nm *netmap.NetworkMap, prefs i
 	}
 
 	if b.appConnector == nil {
-		b.appConnector = appc.NewEmbeddedAppConnector(b.logf, b)
+		b.appConnector = appc.NewAppConnector(b.logf, b)
 	}
 	if nm == nil {
 		return
@@ -5476,7 +5476,7 @@ func (b *LocalBackend) DebugBreakDERPConns() error {
 // ObserveDNSResponse passes a DNS response from the PeerAPI DNS server to the
 // App Connector to enable route discovery.
 func (b *LocalBackend) ObserveDNSResponse(res []byte) {
-	var appConnector *appc.EmbeddedAppConnector
+	var appConnector *appc.AppConnector
 	b.mu.Lock()
 	if b.appConnector == nil {
 		b.mu.Unlock()

ipn/ipnlocal/local_test.go

@@ -1151,7 +1151,7 @@ func TestOfferingAppConnector(t *testing.T) {
 	if b.OfferingAppConnector() {
 		t.Fatal("unexpected offering app connector")
 	}
-	b.appConnector = appc.NewEmbeddedAppConnector(t.Logf, nil)
+	b.appConnector = appc.NewAppConnector(t.Logf, nil)
 	if !b.OfferingAppConnector() {
 		t.Fatal("unexpected not offering app connector")
 	}
@@ -1173,7 +1173,7 @@ func TestAppConnectorHostinfoService(t *testing.T) {
 	if hasAppConnectorService(b.peerAPIServicesLocked()) {
 		t.Fatal("unexpected app connector service")
 	}
-	b.appConnector = appc.NewEmbeddedAppConnector(t.Logf, nil)
+	b.appConnector = appc.NewAppConnector(t.Logf, nil)
 	if !hasAppConnectorService(b.peerAPIServicesLocked()) {
 		t.Fatal("expected app connector service")
 	}
@@ -1199,7 +1199,7 @@ func TestObserveDNSResponse(t *testing.T) {
 	b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8"))
 
 	rc := &routeCollector{}
-	b.appConnector = appc.NewEmbeddedAppConnector(t.Logf, rc)
+	b.appConnector = appc.NewAppConnector(t.Logf, rc)
 	b.appConnector.UpdateDomains([]string{"example.com"})
 
 	b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8"))

ipn/ipnlocal/peerapi_test.go

@@ -697,7 +697,7 @@ func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) {
 			e:            eng,
 			pm:           pm,
 			store:        pm.Store(),
-			appConnector: appc.NewEmbeddedAppConnector(t.Logf, rc),
+			appConnector: appc.NewAppConnector(t.Logf, rc),
 		},
 	}
 	h.ps.b.appConnector.UpdateDomains([]string{"example.com"})