From f27b2cf569c40b09a416d154aa760d279aaa9799 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Wed, 1 Nov 2023 16:56:30 -0700 Subject: [PATCH] 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 --- appc/{embedded.go => appconnector.go} | 30 +++++++++---------- ...{embedded_test.go => appconnector_test.go} | 4 +-- {appc => cmd/sniproxy}/handlers.go | 2 +- {appc => cmd/sniproxy}/handlers_test.go | 2 +- appc/appc.go => cmd/sniproxy/server.go | 3 +- .../sniproxy/server_test.go | 2 +- cmd/sniproxy/sniproxy.go | 28 ++++++++--------- cmd/tailscaled/depaware.txt | 3 +- ipn/ipnlocal/local.go | 12 ++++---- ipn/ipnlocal/local_test.go | 6 ++-- ipn/ipnlocal/peerapi_test.go | 2 +- 11 files changed, 43 insertions(+), 51 deletions(-) rename appc/{embedded.go => appconnector.go} (82%) rename appc/{embedded_test.go => appconnector_test.go} (97%) rename {appc => cmd/sniproxy}/handlers.go (99%) rename {appc => cmd/sniproxy}/handlers_test.go (99%) rename appc/appc.go => cmd/sniproxy/server.go (99%) rename appc/appc_test.go => cmd/sniproxy/server_test.go (99%) diff --git a/appc/embedded.go b/appc/appconnector.go similarity index 82% rename from appc/embedded.go rename to appc/appconnector.go index 84192e2c1..82ac8af83 100644 --- a/appc/embedded.go +++ b/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 -// oriented routing of traffic. +// Package appc implements App Connectors. +// 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. -func NewEmbeddedAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser) *EmbeddedAppConnector { - return &EmbeddedAppConnector{ +// NewAppConnector creates a new AppConnector. +func NewAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser) *AppConnector { + 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 diff --git a/appc/embedded_test.go b/appc/appconnector_test.go similarity index 97% rename from appc/embedded_test.go rename to appc/appconnector_test.go index 7b1687ea6..bf9f92859 100644 --- a/appc/embedded_test.go +++ b/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")) diff --git a/appc/handlers.go b/cmd/sniproxy/handlers.go similarity index 99% rename from appc/handlers.go rename to cmd/sniproxy/handlers.go index 0d017309b..a7303cc8f 100644 --- a/appc/handlers.go +++ b/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" diff --git a/appc/handlers_test.go b/cmd/sniproxy/handlers_test.go similarity index 99% rename from appc/handlers_test.go rename to cmd/sniproxy/handlers_test.go index d8229d004..4f9fc6a34 100644 --- a/appc/handlers_test.go +++ b/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" diff --git a/appc/appc.go b/cmd/sniproxy/server.go similarity index 99% rename from appc/appc.go rename to cmd/sniproxy/server.go index 6da47c3c7..b322b6f4b 100644 --- a/appc/appc.go +++ b/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 appc +package main import ( "expvar" diff --git a/appc/appc_test.go b/cmd/sniproxy/server_test.go similarity index 99% rename from appc/appc_test.go rename to cmd/sniproxy/server_test.go index 9bb69db2b..d56f2aa75 100644 --- a/appc/appc_test.go +++ b/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" diff --git a/cmd/sniproxy/sniproxy.go b/cmd/sniproxy/sniproxy.go index e94566772..fa83aaf4a 100644 --- a/cmd/sniproxy/sniproxy.go +++ b/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 { - appc appc.Server - ts *tsnet.Server - lc *tailscale.LocalClient +type sniproxy struct { + 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) })) diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 4c945a67d..c65e635c0 100644 --- a/cmd/tailscaled/depaware.txt +++ b/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 diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 291e22c18..1a08e6f21 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -205,10 +205,10 @@ type LocalBackend struct { conf *conffile.Config // latest parsed config, or nil if not in declarative mode pm *profileManager // mu guards access filterHash deephash.Sum - 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. + 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.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() diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index c9e511cc0..9568bd2fe 100644 --- a/ipn/ipnlocal/local_test.go +++ b/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")) diff --git a/ipn/ipnlocal/peerapi_test.go b/ipn/ipnlocal/peerapi_test.go index ebfa858d8..8f54affc9 100644 --- a/ipn/ipnlocal/peerapi_test.go +++ b/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"})