feature/appconnectors: start making it modular

Saves 45 KB.

Updates #12614

Change-Id: Iaeb73e69633878ce0a0f58c986024784bbe218f1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

appc/appconnector.go

@@ -18,13 +18,11 @@ import (
 	"sync"
 	"time"
 
-	"golang.org/x/net/dns/dnsmessage"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/views"
 	"tailscale.com/util/clientmetric"
 	"tailscale.com/util/dnsname"
 	"tailscale.com/util/execqueue"
-	"tailscale.com/util/mak"
 	"tailscale.com/util/slicesx"
 )
 
@@ -372,124 +370,6 @@ func (e *AppConnector) DomainRoutes() map[string][]netip.Addr {
 	return drCopy
 }
 
-// ObserveDNSResponse is a callback invoked by the DNS resolver when a DNS
-// 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 *AppConnector) ObserveDNSResponse(res []byte) error {
-	var p dnsmessage.Parser
-	if _, err := p.Start(res); err != nil {
-		return err
-	}
-	if err := p.SkipAllQuestions(); err != nil {
-		return err
-	}
-
-	// cnameChain tracks a chain of CNAMEs for a given query in order to reverse
-	// a CNAME chain back to the original query for flattening. The keys are
-	// CNAME record targets, and the value is the name the record answers, so
-	// for www.example.com CNAME example.com, the map would contain
-	// ["example.com"] = "www.example.com".
-	var cnameChain map[string]string
-
-	// addressRecords is a list of address records found in the response.
-	var addressRecords map[string][]netip.Addr
-
-	for {
-		h, err := p.AnswerHeader()
-		if err == dnsmessage.ErrSectionDone {
-			break
-		}
-		if err != nil {
-			return err
-		}
-
-		if h.Class != dnsmessage.ClassINET {
-			if err := p.SkipAnswer(); err != nil {
-				return err
-			}
-			continue
-		}
-
-		switch h.Type {
-		case dnsmessage.TypeCNAME, dnsmessage.TypeA, dnsmessage.TypeAAAA:
-		default:
-			if err := p.SkipAnswer(); err != nil {
-				return err
-			}
-			continue
-
-		}
-
-		domain := strings.TrimSuffix(strings.ToLower(h.Name.String()), ".")
-		if len(domain) == 0 {
-			continue
-		}
-
-		if h.Type == dnsmessage.TypeCNAME {
-			res, err := p.CNAMEResource()
-			if err != nil {
-				return err
-			}
-			cname := strings.TrimSuffix(strings.ToLower(res.CNAME.String()), ".")
-			if len(cname) == 0 {
-				continue
-			}
-			mak.Set(&cnameChain, cname, domain)
-			continue
-		}
-
-		switch h.Type {
-		case dnsmessage.TypeA:
-			r, err := p.AResource()
-			if err != nil {
-				return err
-			}
-			addr := netip.AddrFrom4(r.A)
-			mak.Set(&addressRecords, domain, append(addressRecords[domain], addr))
-		case dnsmessage.TypeAAAA:
-			r, err := p.AAAAResource()
-			if err != nil {
-				return err
-			}
-			addr := netip.AddrFrom16(r.AAAA)
-			mak.Set(&addressRecords, domain, append(addressRecords[domain], addr))
-		default:
-			if err := p.SkipAnswer(); err != nil {
-				return err
-			}
-			continue
-		}
-	}
-
-	e.mu.Lock()
-	defer e.mu.Unlock()
-
-	for domain, addrs := range addressRecords {
-		domain, isRouted := e.findRoutedDomainLocked(domain, cnameChain)
-
-		// domain and none of the CNAMEs in the chain are routed
-		if !isRouted {
-			continue
-		}
-
-		// advertise each address we have learned for the routed domain, that
-		// was not already known.
-		var toAdvertise []netip.Prefix
-		for _, addr := range addrs {
-			if !e.isAddrKnownLocked(domain, addr) {
-				toAdvertise = append(toAdvertise, netip.PrefixFrom(addr, addr.BitLen()))
-			}
-		}
-
-		if len(toAdvertise) > 0 {
-			e.logf("[v2] observed new routes for %s: %s", domain, toAdvertise)
-			e.scheduleAdvertisement(domain, toAdvertise...)
-		}
-	}
-	return nil
-}
-
 // starting from the given domain that resolved to an address, find it, or any
 // of the domains in the CNAME chain toward resolving it, that are routed
 // domains, returning the routed domain name and a bool indicating whether a

appc/observe.go

@@ -0,0 +1,132 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build !ts_omit_appconnectors
+
+package appc
+
+import (
+	"net/netip"
+	"strings"
+
+	"golang.org/x/net/dns/dnsmessage"
+	"tailscale.com/util/mak"
+)
+
+// ObserveDNSResponse is a callback invoked by the DNS resolver when a DNS
+// 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 *AppConnector) ObserveDNSResponse(res []byte) error {
+	var p dnsmessage.Parser
+	if _, err := p.Start(res); err != nil {
+		return err
+	}
+	if err := p.SkipAllQuestions(); err != nil {
+		return err
+	}
+
+	// cnameChain tracks a chain of CNAMEs for a given query in order to reverse
+	// a CNAME chain back to the original query for flattening. The keys are
+	// CNAME record targets, and the value is the name the record answers, so
+	// for www.example.com CNAME example.com, the map would contain
+	// ["example.com"] = "www.example.com".
+	var cnameChain map[string]string
+
+	// addressRecords is a list of address records found in the response.
+	var addressRecords map[string][]netip.Addr
+
+	for {
+		h, err := p.AnswerHeader()
+		if err == dnsmessage.ErrSectionDone {
+			break
+		}
+		if err != nil {
+			return err
+		}
+
+		if h.Class != dnsmessage.ClassINET {
+			if err := p.SkipAnswer(); err != nil {
+				return err
+			}
+			continue
+		}
+
+		switch h.Type {
+		case dnsmessage.TypeCNAME, dnsmessage.TypeA, dnsmessage.TypeAAAA:
+		default:
+			if err := p.SkipAnswer(); err != nil {
+				return err
+			}
+			continue
+
+		}
+
+		domain := strings.TrimSuffix(strings.ToLower(h.Name.String()), ".")
+		if len(domain) == 0 {
+			continue
+		}
+
+		if h.Type == dnsmessage.TypeCNAME {
+			res, err := p.CNAMEResource()
+			if err != nil {
+				return err
+			}
+			cname := strings.TrimSuffix(strings.ToLower(res.CNAME.String()), ".")
+			if len(cname) == 0 {
+				continue
+			}
+			mak.Set(&cnameChain, cname, domain)
+			continue
+		}
+
+		switch h.Type {
+		case dnsmessage.TypeA:
+			r, err := p.AResource()
+			if err != nil {
+				return err
+			}
+			addr := netip.AddrFrom4(r.A)
+			mak.Set(&addressRecords, domain, append(addressRecords[domain], addr))
+		case dnsmessage.TypeAAAA:
+			r, err := p.AAAAResource()
+			if err != nil {
+				return err
+			}
+			addr := netip.AddrFrom16(r.AAAA)
+			mak.Set(&addressRecords, domain, append(addressRecords[domain], addr))
+		default:
+			if err := p.SkipAnswer(); err != nil {
+				return err
+			}
+			continue
+		}
+	}
+
+	e.mu.Lock()
+	defer e.mu.Unlock()
+
+	for domain, addrs := range addressRecords {
+		domain, isRouted := e.findRoutedDomainLocked(domain, cnameChain)
+
+		// domain and none of the CNAMEs in the chain are routed
+		if !isRouted {
+			continue
+		}
+
+		// advertise each address we have learned for the routed domain, that
+		// was not already known.
+		var toAdvertise []netip.Prefix
+		for _, addr := range addrs {
+			if !e.isAddrKnownLocked(domain, addr) {
+				toAdvertise = append(toAdvertise, netip.PrefixFrom(addr, addr.BitLen()))
+			}
+		}
+
+		if len(toAdvertise) > 0 {
+			e.logf("[v2] observed new routes for %s: %s", domain, toAdvertise)
+			e.scheduleAdvertisement(domain, toAdvertise...)
+		}
+	}
+	return nil
+}

appc/observe_disabled.go

@@ -0,0 +1,8 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build ts_omit_appconnectors
+
+package appc
+
+func (e *AppConnector) ObserveDNSResponse(res []byte) error { return nil }

cmd/tailscaled/depaware-min.txt

@@ -164,7 +164,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
      💣 tailscale.com/util/hashx                                     from tailscale.com/util/deephash
         tailscale.com/util/httpm                                     from tailscale.com/ipn/ipnlocal+
         tailscale.com/util/lineiter                                  from tailscale.com/hostinfo+
-        tailscale.com/util/mak                                       from tailscale.com/appc+
+        tailscale.com/util/mak                                       from tailscale.com/control/controlclient+
         tailscale.com/util/multierr                                  from tailscale.com/cmd/tailscaled+
         tailscale.com/util/must                                      from tailscale.com/logpolicy+
         tailscale.com/util/nocasemaps                                from tailscale.com/types/ipproto

cmd/tailscaled/depaware-minbox.txt

@@ -190,7 +190,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
      💣 tailscale.com/util/hashx                                     from tailscale.com/util/deephash
         tailscale.com/util/httpm                                     from tailscale.com/ipn/ipnlocal+
         tailscale.com/util/lineiter                                  from tailscale.com/hostinfo+
-        tailscale.com/util/mak                                       from tailscale.com/appc+
+        tailscale.com/util/mak                                       from tailscale.com/control/controlclient+
         tailscale.com/util/multierr                                  from tailscale.com/cmd/tailscaled+
         tailscale.com/util/must                                      from tailscale.com/logpolicy+
         tailscale.com/util/nocasemaps                                from tailscale.com/types/ipproto

cmd/tailscaled/depaware.txt

@@ -271,6 +271,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/envknob                                        from tailscale.com/client/local+
         tailscale.com/envknob/featureknob                            from tailscale.com/client/web+
         tailscale.com/feature                                        from tailscale.com/feature/wakeonlan+
+        tailscale.com/feature/appconnectors                          from tailscale.com/feature/condregister
         tailscale.com/feature/buildfeatures                          from tailscale.com/wgengine/magicsock+
         tailscale.com/feature/capture                                from tailscale.com/feature/condregister
         tailscale.com/feature/clientupdate                           from tailscale.com/feature/condregister

feature/appconnectors/appconnectors.go

@@ -0,0 +1,39 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+// Package appconnectors registers support for Tailscale App Connectors.
+package appconnectors
+
+import (
+	"encoding/json"
+	"net/http"
+
+	"tailscale.com/ipn/ipnlocal"
+	"tailscale.com/tailcfg"
+)
+
+func init() {
+	ipnlocal.RegisterC2N("GET /appconnector/routes", handleC2NAppConnectorDomainRoutesGet)
+}
+
+// handleC2NAppConnectorDomainRoutesGet handles returning the domains
+// that the app connector is responsible for, as well as the resolved
+// IP addresses for each domain. If the node is not configured as
+// an app connector, an empty map is returned.
+func handleC2NAppConnectorDomainRoutesGet(b *ipnlocal.LocalBackend, w http.ResponseWriter, r *http.Request) {
+	logf := b.Logger()
+	logf("c2n: GET /appconnector/routes received")
+
+	var res tailcfg.C2NAppConnectorDomainRoutesResponse
+	appConnector := b.AppConnector()
+	if appConnector == nil {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(res)
+		return
+	}
+
+	res.Domains = appConnector.DomainRoutes()
+
+	w.Header().Set("Content-Type", "application/json")
+	json.NewEncoder(w).Encode(res)
+}

feature/buildfeatures/feature_appconnectors_disabled.go

@@ -0,0 +1,13 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+// Code generated by gen.go; DO NOT EDIT.
+
+//go:build ts_omit_appconnectors
+
+package buildfeatures
+
+// HasAppConnectors is whether the binary was built with support for modular feature "App Connectors support".
+// Specifically, it's whether the binary was NOT built with the "ts_omit_appconnectors" build tag.
+// It's a const so it can be used for dead code elimination.
+const HasAppConnectors = false

feature/buildfeatures/feature_appconnectors_enabled.go

@@ -0,0 +1,13 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+// Code generated by gen.go; DO NOT EDIT.
+
+//go:build !ts_omit_appconnectors
+
+package buildfeatures
+
+// HasAppConnectors is whether the binary was built with support for modular feature "App Connectors support".
+// Specifically, it's whether the binary was NOT built with the "ts_omit_appconnectors" build tag.
+// It's a const so it can be used for dead code elimination.
+const HasAppConnectors = true

feature/condregister/maybe_appconnectors.go

@@ -0,0 +1,8 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build !ts_omit_appconnectors
+
+package condregister
+
+import _ "tailscale.com/feature/appconnectors"

feature/featuretags/featuretags.go

@@ -88,6 +88,7 @@ type FeatureMeta struct {
 // excluded via build tags, and a description of each.
 var Features = map[FeatureTag]FeatureMeta{
 	"acme":          {"ACME", "ACME TLS certificate management", nil},
+	"appconnectors": {"AppConnectors", "App Connectors support", nil},
 	"aws":           {"AWS", "AWS integration", nil},
 	"bird":          {"Bird", "Bird BGP integration", nil},
 	"captiveportal": {"CaptivePortal", "Captive portal detection", nil},

ipn/ipnlocal/c2n.go

@@ -51,9 +51,6 @@ var c2nHandlers = map[methodAndPath]c2nHandler{
 	// SSH
 	req("/ssh/usernames"): handleC2NSSHUsernames,
 
-	// App Connectors.
-	req("GET /appconnector/routes"): handleC2NAppConnectorDomainRoutesGet,
-
 	// Linux netfilter.
 	req("POST /netfilter-kind"): handleC2NSetNetfilterKind,
 }
@@ -294,27 +291,6 @@ func handleC2NSockStats(b *LocalBackend, w http.ResponseWriter, r *http.Request)
 	fmt.Fprintf(w, "debug info: %v\n", sockstats.DebugInfo())
 }
 
-// handleC2NAppConnectorDomainRoutesGet handles returning the domains
-// that the app connector is responsible for, as well as the resolved
-// IP addresses for each domain. If the node is not configured as
-// an app connector, an empty map is returned.
-func handleC2NAppConnectorDomainRoutesGet(b *LocalBackend, w http.ResponseWriter, r *http.Request) {
-	b.logf("c2n: GET /appconnector/routes received")
-
-	var res tailcfg.C2NAppConnectorDomainRoutesResponse
-	appConnector := b.AppConnector()
-	if appConnector == nil {
-		w.Header().Set("Content-Type", "application/json")
-		json.NewEncoder(w).Encode(res)
-		return
-	}
-
-	res.Domains = appConnector.DomainRoutes()
-
-	w.Header().Set("Content-Type", "application/json")
-	json.NewEncoder(w).Encode(res)
-}
-
 func handleC2NSetNetfilterKind(b *LocalBackend, w http.ResponseWriter, r *http.Request) {
 	b.logf("c2n: POST /netfilter-kind received")
 

ipn/ipnlocal/local.go

@@ -398,9 +398,10 @@ type LocalBackend struct {
 }
 
 // HealthTracker returns the health tracker for the backend.
-func (b *LocalBackend) HealthTracker() *health.Tracker {
-	return b.health
-}
+func (b *LocalBackend) HealthTracker() *health.Tracker { return b.health }
+
+// Logger returns the logger for the backend.
+func (b *LocalBackend) Logger() logger.Logf { return b.logf }
 
 // UserMetricsRegistry returns the usermetrics registry for the backend
 func (b *LocalBackend) UserMetricsRegistry() *usermetric.Registry {
@@ -4154,6 +4155,9 @@ func (b *LocalBackend) SetUseExitNodeEnabled(actor ipnauth.Actor, v bool) (ipn.P
 // MaybeClearAppConnector clears the routes from any AppConnector if
 // AdvertiseRoutes has been set in the MaskedPrefs.
 func (b *LocalBackend) MaybeClearAppConnector(mp *ipn.MaskedPrefs) error {
+	if !buildfeatures.HasAppConnectors {
+		return nil
+	}
 	var err error
 	if ac := b.AppConnector(); ac != nil && mp.AdvertiseRoutesSet {
 		err = ac.ClearRoutes()
@@ -4770,6 +4774,9 @@ func (b *LocalBackend) blockEngineUpdates(block bool) {
 // current network map and preferences.
 // b.mu must be held.
 func (b *LocalBackend) reconfigAppConnectorLocked(nm *netmap.NetworkMap, prefs ipn.PrefsView) {
+	if !buildfeatures.HasAppConnectors {
+		return
+	}
 	const appConnectorCapName = "tailscale.com/app-connectors"
 	defer func() {
 		if b.hostinfo != nil {
@@ -4943,7 +4950,9 @@ func (b *LocalBackend) authReconfig() {
 	b.logf("[v1] authReconfig: ra=%v dns=%v 0x%02x: %v", prefs.RouteAll(), prefs.CorpDNS(), flags, err)
 
 	b.initPeerAPIListener()
-	b.readvertiseAppConnectorRoutes()
+	if buildfeatures.HasAppConnectors {
+		b.readvertiseAppConnectorRoutes()
+	}
 }
 
 // shouldUseOneCGNATRoute reports whether we should prefer to make one big
@@ -6363,6 +6372,9 @@ func (b *LocalBackend) OfferingExitNode() bool {
 // OfferingAppConnector reports whether b is currently offering app
 // connector services.
 func (b *LocalBackend) OfferingAppConnector() bool {
+	if !buildfeatures.HasAppConnectors {
+		return false
+	}
 	b.mu.Lock()
 	defer b.mu.Unlock()
 	return b.appConnector != nil
@@ -6372,6 +6384,9 @@ func (b *LocalBackend) OfferingAppConnector() bool {
 //
 // TODO(nickkhyl): move app connectors to [nodeBackend], or perhaps a feature package?
 func (b *LocalBackend) AppConnector() *appc.AppConnector {
+	if !buildfeatures.HasAppConnectors {
+		return nil
+	}
 	b.mu.Lock()
 	defer b.mu.Unlock()
 	return b.appConnector
@@ -6917,6 +6932,9 @@ 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) error {
+	if !buildfeatures.HasAppConnectors {
+		return nil
+	}
 	var appConnector *appc.AppConnector
 	b.mu.Lock()
 	if b.appConnector == nil {
@@ -7020,6 +7038,9 @@ func namespaceKeyForCurrentProfile(pm *profileManager, key ipn.StateKey) ipn.Sta
 const routeInfoStateStoreKey ipn.StateKey = "_routeInfo"
 
 func (b *LocalBackend) storeRouteInfo(ri *appc.RouteInfo) error {
+	if !buildfeatures.HasAppConnectors {
+		return feature.ErrUnavailable
+	}
 	b.mu.Lock()
 	defer b.mu.Unlock()
 	if b.pm.CurrentProfile().ID() == "" {
@@ -7034,6 +7055,9 @@ func (b *LocalBackend) storeRouteInfo(ri *appc.RouteInfo) error {
 }
 
 func (b *LocalBackend) readRouteInfoLocked() (*appc.RouteInfo, error) {
+	if !buildfeatures.HasAppConnectors {
+		return nil, feature.ErrUnavailable
+	}
 	if b.pm.CurrentProfile().ID() == "" {
 		return &appc.RouteInfo{}, nil
 	}

ipn/ipnlocal/peerapi.go

@@ -745,7 +745,7 @@ func (h *peerAPIHandler) handleDNSQuery(w http.ResponseWriter, r *http.Request)
 	// TODO(raggi): consider pushing the integration down into the resolver
 	// instead to avoid re-parsing the DNS response for improved performance in
 	// the future.
-	if h.ps.b.OfferingAppConnector() {
+	if buildfeatures.HasAppConnectors && h.ps.b.OfferingAppConnector() {
 		if err := h.ps.b.ObserveDNSResponse(res); err != nil {
 			h.logf("ObserveDNSResponse error: %v", err)
 			// This is not fatal, we probably just failed to parse the upstream

ipn/localapi/localapi.go

@@ -72,7 +72,6 @@ var handler = map[string]LocalAPIHandler{
 	// The other /localapi/v0/NAME handlers are exact matches and contain only NAME
 	// without a trailing slash:
 	"alpha-set-device-attrs":       (*Handler).serveSetDeviceAttrs, // see tailscale/corp#24690
-	"appc-route-info":              (*Handler).serveGetAppcRouteInfo,
 	"bugreport":                    (*Handler).serveBugReport,
 	"check-ip-forwarding":          (*Handler).serveCheckIPForwarding,
 	"check-prefs":                  (*Handler).serveCheckPrefs,
@@ -113,6 +112,12 @@ var handler = map[string]LocalAPIHandler{
 	"whois":                        (*Handler).serveWhoIs,
 }
 
+func init() {
+	if buildfeatures.HasAppConnectors {
+		Register("appc-route-info", (*Handler).serveGetAppcRouteInfo)
+	}
+}
+
 // Register registers a new LocalAPI handler for the given name.
 func Register(name string, fn LocalAPIHandler) {
 	if _, ok := handler[name]; ok {
@@ -934,11 +939,13 @@ func (h *Handler) servePrefs(w http.ResponseWriter, r *http.Request) {
 			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
 		}
-		if err := h.b.MaybeClearAppConnector(mp); err != nil {
-			w.Header().Set("Content-Type", "application/json")
-			w.WriteHeader(http.StatusInternalServerError)
-			json.NewEncoder(w).Encode(resJSON{Error: err.Error()})
-			return
+		if buildfeatures.HasAppConnectors {
+			if err := h.b.MaybeClearAppConnector(mp); err != nil {
+				w.Header().Set("Content-Type", "application/json")
+				w.WriteHeader(http.StatusInternalServerError)
+				json.NewEncoder(w).Encode(resJSON{Error: err.Error()})
+				return
+			}
 		}
 		var err error
 		prefs, err = h.b.EditPrefsAs(mp, h.Actor)
@@ -1666,6 +1673,10 @@ func (h *Handler) serveShutdown(w http.ResponseWriter, r *http.Request) {
 }
 
 func (h *Handler) serveGetAppcRouteInfo(w http.ResponseWriter, r *http.Request) {
+	if !buildfeatures.HasAppConnectors {
+		http.Error(w, feature.ErrUnavailable.Error(), http.StatusNotImplemented)
+		return
+	}
 	if r.Method != httpm.GET {
 		http.Error(w, "only GET allowed", http.StatusMethodNotAllowed)
 		return