diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt index 2281d3819..ea0e08b19 100644 --- a/cmd/k8s-operator/depaware.txt +++ b/cmd/k8s-operator/depaware.txt @@ -813,7 +813,6 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/ipn/ipnlocal from tailscale.com/ipn/localapi+ tailscale.com/ipn/ipnstate from tailscale.com/client/local+ tailscale.com/ipn/localapi from tailscale.com/tsnet - tailscale.com/ipn/policy from tailscale.com/ipn/ipnlocal tailscale.com/ipn/store from tailscale.com/ipn/ipnlocal+ L tailscale.com/ipn/store/awsstore from tailscale.com/ipn/store tailscale.com/ipn/store/kubestore from tailscale.com/cmd/k8s-operator+ @@ -861,7 +860,6 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/net/netknob from tailscale.com/logpolicy+ 💣 tailscale.com/net/netmon from tailscale.com/control/controlclient+ 💣 tailscale.com/net/netns from tailscale.com/derp/derphttp+ - W 💣 tailscale.com/net/netstat from tailscale.com/portlist tailscale.com/net/netutil from tailscale.com/client/local+ tailscale.com/net/netx from tailscale.com/control/controlclient+ tailscale.com/net/packet from tailscale.com/net/connstats+ @@ -885,7 +883,6 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/net/udprelay/status from tailscale.com/client/local tailscale.com/omit from tailscale.com/ipn/conffile tailscale.com/paths from tailscale.com/client/local+ - 💣 tailscale.com/portlist from tailscale.com/ipn/ipnlocal tailscale.com/posture from tailscale.com/ipn/ipnlocal tailscale.com/proxymap from tailscale.com/tsd+ 💣 tailscale.com/safesocket from tailscale.com/client/local+ @@ -931,7 +928,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/util/cmpver from tailscale.com/clientupdate+ tailscale.com/util/ctxkey from tailscale.com/client/tailscale/apitype+ 💣 tailscale.com/util/deephash from tailscale.com/ipn/ipnlocal+ - L 💣 tailscale.com/util/dirwalk from tailscale.com/metrics+ + L 💣 tailscale.com/util/dirwalk from tailscale.com/metrics tailscale.com/util/dnsname from tailscale.com/appc+ tailscale.com/util/eventbus from tailscale.com/tsd+ tailscale.com/util/execqueue from tailscale.com/appc+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 70be690ee..acd8e0459 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -278,6 +278,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/feature/debugportmapper from tailscale.com/feature/condregister tailscale.com/feature/drive from tailscale.com/feature/condregister L tailscale.com/feature/linuxdnsfight from tailscale.com/feature/condregister + tailscale.com/feature/portlist from tailscale.com/feature/condregister tailscale.com/feature/portmapper from tailscale.com/feature/condregister/portmapper tailscale.com/feature/relayserver from tailscale.com/feature/condregister tailscale.com/feature/syspolicy from tailscale.com/feature/condregister+ @@ -299,7 +300,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/ipn/ipnserver from tailscale.com/cmd/tailscaled tailscale.com/ipn/ipnstate from tailscale.com/client/local+ tailscale.com/ipn/localapi from tailscale.com/ipn/ipnserver+ - tailscale.com/ipn/policy from tailscale.com/ipn/ipnlocal + tailscale.com/ipn/policy from tailscale.com/feature/portlist tailscale.com/ipn/store from tailscale.com/cmd/tailscaled+ L tailscale.com/ipn/store/awsstore from tailscale.com/ipn/store L tailscale.com/ipn/store/kubestore from tailscale.com/ipn/store @@ -360,7 +361,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/net/udprelay/status from tailscale.com/client/local+ tailscale.com/omit from tailscale.com/ipn/conffile tailscale.com/paths from tailscale.com/client/local+ - 💣 tailscale.com/portlist from tailscale.com/ipn/ipnlocal + 💣 tailscale.com/portlist from tailscale.com/feature/portlist tailscale.com/posture from tailscale.com/ipn/ipnlocal tailscale.com/proxymap from tailscale.com/tsd+ 💣 tailscale.com/safesocket from tailscale.com/client/local+ diff --git a/cmd/tailscaled/deps_test.go b/cmd/tailscaled/deps_test.go index 35975b57c..24a393124 100644 --- a/cmd/tailscaled/deps_test.go +++ b/cmd/tailscaled/deps_test.go @@ -185,3 +185,16 @@ func TestOmitDBus(t *testing.T) { }, }.Check(t) } + +func TestOmitPortlist(t *testing.T) { + deptest.DepChecker{ + GOOS: "linux", + GOARCH: "amd64", + Tags: "ts_omit_portlist,ts_include_cli", + OnDep: func(dep string) { + if strings.Contains(dep, "portlist") { + t.Errorf("unexpected dep: %q", dep) + } + }, + }.Check(t) +} diff --git a/cmd/tsidp/depaware.txt b/cmd/tsidp/depaware.txt index 4fd7c8020..69904c976 100644 --- a/cmd/tsidp/depaware.txt +++ b/cmd/tsidp/depaware.txt @@ -255,7 +255,6 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar tailscale.com/ipn/ipnlocal from tailscale.com/ipn/localapi+ tailscale.com/ipn/ipnstate from tailscale.com/client/local+ tailscale.com/ipn/localapi from tailscale.com/tsnet - tailscale.com/ipn/policy from tailscale.com/ipn/ipnlocal tailscale.com/ipn/store from tailscale.com/ipn/ipnlocal+ L tailscale.com/ipn/store/awsstore from tailscale.com/ipn/store L tailscale.com/ipn/store/kubestore from tailscale.com/ipn/store @@ -292,7 +291,6 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar tailscale.com/net/netknob from tailscale.com/logpolicy+ 💣 tailscale.com/net/netmon from tailscale.com/control/controlclient+ 💣 tailscale.com/net/netns from tailscale.com/derp/derphttp+ - W 💣 tailscale.com/net/netstat from tailscale.com/portlist tailscale.com/net/netutil from tailscale.com/client/local+ tailscale.com/net/netx from tailscale.com/control/controlclient+ tailscale.com/net/packet from tailscale.com/ipn/ipnlocal+ @@ -316,7 +314,6 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar tailscale.com/net/udprelay/status from tailscale.com/client/local tailscale.com/omit from tailscale.com/ipn/conffile tailscale.com/paths from tailscale.com/client/local+ - 💣 tailscale.com/portlist from tailscale.com/ipn/ipnlocal tailscale.com/posture from tailscale.com/ipn/ipnlocal tailscale.com/proxymap from tailscale.com/tsd+ 💣 tailscale.com/safesocket from tailscale.com/client/local+ @@ -361,7 +358,7 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar tailscale.com/util/cmpver from tailscale.com/clientupdate+ tailscale.com/util/ctxkey from tailscale.com/client/tailscale/apitype+ 💣 tailscale.com/util/deephash from tailscale.com/ipn/ipnlocal+ - L 💣 tailscale.com/util/dirwalk from tailscale.com/metrics+ + L 💣 tailscale.com/util/dirwalk from tailscale.com/metrics tailscale.com/util/dnsname from tailscale.com/appc+ tailscale.com/util/eventbus from tailscale.com/ipn/localapi+ tailscale.com/util/execqueue from tailscale.com/appc+ diff --git a/feature/buildfeatures/feature_portlist_disabled.go b/feature/buildfeatures/feature_portlist_disabled.go new file mode 100644 index 000000000..934061fd8 --- /dev/null +++ b/feature/buildfeatures/feature_portlist_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_portlist + +package buildfeatures + +// HasPortList is whether the binary was built with support for modular feature "Optionally advertise listening service ports". +// Specifically, it's whether the binary was NOT built with the "ts_omit_portlist" build tag. +// It's a const so it can be used for dead code elimination. +const HasPortList = false diff --git a/feature/buildfeatures/feature_portlist_enabled.go b/feature/buildfeatures/feature_portlist_enabled.go new file mode 100644 index 000000000..c1dc1c163 --- /dev/null +++ b/feature/buildfeatures/feature_portlist_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_portlist + +package buildfeatures + +// HasPortList is whether the binary was built with support for modular feature "Optionally advertise listening service ports". +// Specifically, it's whether the binary was NOT built with the "ts_omit_portlist" build tag. +// It's a const so it can be used for dead code elimination. +const HasPortList = true diff --git a/feature/condregister/maybe_portlist.go b/feature/condregister/maybe_portlist.go new file mode 100644 index 000000000..1be56f177 --- /dev/null +++ b/feature/condregister/maybe_portlist.go @@ -0,0 +1,8 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !ts_omit_portlist + +package condregister + +import _ "tailscale.com/feature/portlist" diff --git a/feature/featuretags/featuretags.go b/feature/featuretags/featuretags.go index 6f8c4ac17..d1752a80c 100644 --- a/feature/featuretags/featuretags.go +++ b/feature/featuretags/featuretags.go @@ -114,6 +114,7 @@ var Features = map[FeatureTag]FeatureMeta{ Desc: "Outbound localhost HTTP/SOCK5 proxy support", Deps: []FeatureTag{"netstack"}, }, + "portlist": {"PortList", "Optionally advertise listening service ports", nil}, "portmapper": {"PortMapper", "NAT-PMP/PCP/UPnP port mapping support", nil}, "netstack": {"Netstack", "gVisor netstack (userspace networking) support (TODO; not yet omittable)", nil}, "networkmanager": { diff --git a/feature/portlist/portlist.go b/feature/portlist/portlist.go new file mode 100644 index 000000000..7d69796ff --- /dev/null +++ b/feature/portlist/portlist.go @@ -0,0 +1,157 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +// Package portlist contains code to poll the local system for open ports +// and report them to the control plane, if enabled on the tailnet. +package portlist + +import ( + "context" + "sync/atomic" + + "tailscale.com/envknob" + "tailscale.com/ipn" + "tailscale.com/ipn/ipnext" + "tailscale.com/ipn/ipnlocal" + "tailscale.com/ipn/policy" + "tailscale.com/portlist" + "tailscale.com/tailcfg" + "tailscale.com/types/logger" + "tailscale.com/util/eventbus" + "tailscale.com/version" +) + +func init() { + ipnext.RegisterExtension("portlist", newExtension) +} + +func newExtension(logf logger.Logf, sb ipnext.SafeBackend) (ipnext.Extension, error) { + busClient := sb.Sys().Bus.Get().Client("portlist") + e := &Extension{ + sb: sb, + busClient: busClient, + logf: logger.WithPrefix(logf, "portlist: "), + pub: eventbus.Publish[ipnlocal.PortlistServices](busClient), + pollerDone: make(chan struct{}), + wakePoller: make(chan struct{}), + } + e.ctx, e.ctxCancel = context.WithCancel(context.Background()) + return e, nil +} + +// Extension implements the portlist extension. +type Extension struct { + ctx context.Context + ctxCancel context.CancelFunc + pollerDone chan struct{} // close-only chan when poller goroutine exits + wakePoller chan struct{} // best effort chan to wake poller from sleep + busClient *eventbus.Client + pub *eventbus.Publisher[ipnlocal.PortlistServices] + logf logger.Logf + sb ipnext.SafeBackend + host ipnext.Host // from Init + + shieldsUp atomic.Bool + shouldUploadServicesAtomic atomic.Bool +} + +func (e *Extension) Name() string { return "portlist" } +func (e *Extension) Shutdown() error { + e.ctxCancel() + e.busClient.Close() + <-e.pollerDone + return nil +} + +func (e *Extension) Init(h ipnext.Host) error { + if !envknob.BoolDefaultTrue("TS_PORTLIST") { + return ipnext.SkipExtension + } + + e.host = h + h.Hooks().ShouldUploadServices.Set(e.shouldUploadServicesAtomic.Load) + h.Hooks().ProfileStateChange.Add(e.onChangeProfile) + h.Hooks().OnSelfChange.Add(e.onSelfChange) + + // TODO(nickkhyl): remove this after the profileManager refactoring. + // See tailscale/tailscale#15974. + // This same workaround appears in feature/taildrop/ext.go. + profile, prefs := h.Profiles().CurrentProfileState() + e.onChangeProfile(profile, prefs, false) + + go e.runPollLoop() + return nil +} + +func (e *Extension) onSelfChange(tailcfg.NodeView) { + e.updateShouldUploadServices() +} + +func (e *Extension) onChangeProfile(_ ipn.LoginProfileView, prefs ipn.PrefsView, sameNode bool) { + e.shieldsUp.Store(prefs.ShieldsUp()) + e.updateShouldUploadServices() +} + +func (e *Extension) updateShouldUploadServices() { + v := !e.shieldsUp.Load() && e.host.NodeBackend().CollectServices() + if e.shouldUploadServicesAtomic.CompareAndSwap(!v, v) && v { + // Upon transition from false to true (enabling service reporting), try + // to wake the poller to do an immediate poll if it's sleeping. + // It's not a big deal if we miss waking it. It'll get to it soon enough. + select { + case e.wakePoller <- struct{}{}: + default: + } + } +} + +// runPollLoop is a goroutine that periodically checks the open +// ports and publishes them if they've changed. +func (e *Extension) runPollLoop() { + defer close(e.pollerDone) + + var poller portlist.Poller + + ticker, tickerChannel := e.sb.Clock().NewTicker(portlist.PollInterval()) + defer ticker.Stop() + for { + select { + case <-tickerChannel: + case <-e.wakePoller: + case <-e.ctx.Done(): + return + } + + if !e.shouldUploadServicesAtomic.Load() { + continue + } + + ports, changed, err := poller.Poll() + if err != nil { + e.logf("Poll: %v", err) + // TODO: this is kinda weird that we just return here and never try + // again. Maybe that was because all errors are assumed to be + // permission errors and thus permanent? Audit varioys OS + // implementation and check error types, and then make this check + // for permanent vs temporary errors and keep looping with a backoff + // for temporary errors? But for now we just give up, like we always + // have. + return + } + if !changed { + continue + } + sl := []tailcfg.Service{} + for _, p := range ports { + s := tailcfg.Service{ + Proto: tailcfg.ServiceProto(p.Proto), + Port: p.Port, + Description: p.Process, + } + if policy.IsInterestingService(s, version.OS()) { + sl = append(sl, s) + } + } + e.pub.Publish(ipnlocal.PortlistServices(sl)) + } +} diff --git a/feature/taildrop/ext.go b/feature/taildrop/ext.go index f8f45b53f..6bdb375cc 100644 --- a/feature/taildrop/ext.go +++ b/feature/taildrop/ext.go @@ -105,6 +105,7 @@ func (e *Extension) Init(h ipnext.Host) error { // TODO(nickkhyl): remove this after the profileManager refactoring. // See tailscale/tailscale#15974. + // This same workaround appears in feature/portlist/portlist.go. profile, prefs := h.Profiles().CurrentProfileState() e.onChangeProfile(profile, prefs, false) return nil diff --git a/ipn/ipnext/ipnext.go b/ipn/ipnext/ipnext.go index 066763ba4..4ff37dc8e 100644 --- a/ipn/ipnext/ipnext.go +++ b/ipn/ipnext/ipnext.go @@ -372,6 +372,10 @@ type Hooks struct { // SetPeerStatus is called to mutate PeerStatus. // Callers must only use NodeBackend to read data. SetPeerStatus feature.Hooks[func(*ipnstate.PeerStatus, tailcfg.NodeView, NodeBackend)] + + // ShouldUploadServices reports whether this node should include services + // in Hostinfo from the portlist extension. + ShouldUploadServices feature.Hook[func() bool] } // NodeBackend is an interface to query the current node and its peers. @@ -398,4 +402,9 @@ type NodeBackend interface { // It effectively just reports whether PeerAPIBase(node) is non-empty, but // potentially more efficiently. PeerHasPeerAPI(tailcfg.NodeView) bool + + // CollectServices reports whether the control plane is telling this + // node that the portlist service collection is desirable, should it + // choose to report them. + CollectServices() bool } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index b36f54705..62a3a2131 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -61,7 +61,6 @@ import ( "tailscale.com/ipn/ipnauth" "tailscale.com/ipn/ipnext" "tailscale.com/ipn/ipnstate" - "tailscale.com/ipn/policy" "tailscale.com/log/sockstatlog" "tailscale.com/logpolicy" "tailscale.com/net/dns" @@ -77,7 +76,6 @@ import ( "tailscale.com/net/tsaddr" "tailscale.com/net/tsdial" "tailscale.com/paths" - "tailscale.com/portlist" "tailscale.com/posture" "tailscale.com/syncs" "tailscale.com/tailcfg" @@ -211,12 +209,10 @@ type LocalBackend struct { pushDeviceToken syncs.AtomicValue[string] backendLogID logid.PublicID unregisterSysPolicyWatch func() - portpoll *portlist.Poller // may be nil - portpollOnce sync.Once // guards starting readPoller - varRoot string // or empty if SetVarRoot never called - logFlushFunc func() // or nil if SetLogFlusher wasn't called - em *expiryManager // non-nil; TODO(nickkhyl): move to nodeBackend - sshAtomicBool atomic.Bool // TODO(nickkhyl): move to nodeBackend + varRoot string // or empty if SetVarRoot never called + logFlushFunc func() // or nil if SetLogFlusher wasn't called + em *expiryManager // non-nil; TODO(nickkhyl): move to nodeBackend + sshAtomicBool atomic.Bool // TODO(nickkhyl): move to nodeBackend // webClientAtomicBool controls whether the web client is running. This should // be true unless the disable-web-client node attribute has been set. webClientAtomicBool atomic.Bool // TODO(nickkhyl): move to nodeBackend @@ -522,7 +518,6 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo pm: pm, backendLogID: logID, state: ipn.NoState, - portpoll: new(portlist.Poller), em: newExpiryManager(logf, sys.Bus.Get()), loginFlags: loginFlags, clock: clock, @@ -619,6 +614,12 @@ func (b *LocalBackend) consumeEventbusTopics(ec *eventbus.Client) func(*eventbus healthChangeSub := eventbus.Subscribe[health.Change](ec) changeDeltaSub := eventbus.Subscribe[netmon.ChangeDelta](ec) + var portlist <-chan PortlistServices + if buildfeatures.HasPortList { + portlistSub := eventbus.Subscribe[PortlistServices](ec) + portlist = portlistSub.Events() + } + return func(ec *eventbus.Client) { for { select { @@ -632,6 +633,10 @@ func (b *LocalBackend) consumeEventbusTopics(ec *eventbus.Client) func(*eventbus b.onHealthChange(change) case changeDelta := <-changeDeltaSub.Events(): b.linkChange(&changeDelta) + case pl := <-portlist: + if buildfeatures.HasPortList { // redundant, but explicit for linker deadcode and humans + b.setPortlistServices(pl) + } } } } @@ -2300,15 +2305,6 @@ func (b *LocalBackend) SetControlClientGetterForTesting(newControlClient func(co b.ccGen = newControlClient } -// DisablePortPollerForTest disables the port list poller for tests. -// It must be called before Start. -func (b *LocalBackend) DisablePortPollerForTest() { - testenv.AssertInTest() - b.mu.Lock() - defer b.mu.Unlock() - b.portpoll = nil -} - // PeersForTest returns all the current peers, sorted by Node.ID, // for integration tests in another repo. func (b *LocalBackend) PeersForTest() []tailcfg.NodeView { @@ -2457,12 +2453,6 @@ func (b *LocalBackend) Start(opts ipn.Options) error { persistv = new(persist.Persist) } - if b.portpoll != nil { - b.portpollOnce.Do(func() { - b.goTracker.Go(b.readPoller) - }) - } - discoPublic := b.MagicConn().DiscoPublicKey() var err error @@ -2906,57 +2896,6 @@ func shrinkDefaultRoute(route netip.Prefix, localInterfaceRoutes *netipx.IPSet, return b.IPSet() } -// readPoller is a goroutine that receives service lists from -// b.portpoll and propagates them into the controlclient's HostInfo. -func (b *LocalBackend) readPoller() { - if !envknob.BoolDefaultTrue("TS_PORTLIST") { - return - } - - ticker, tickerChannel := b.clock.NewTicker(portlist.PollInterval()) - defer ticker.Stop() - for { - select { - case <-tickerChannel: - case <-b.ctx.Done(): - return - } - - if !b.shouldUploadServices() { - continue - } - - ports, changed, err := b.portpoll.Poll() - if err != nil { - b.logf("error polling for open ports: %v", err) - return - } - if !changed { - continue - } - sl := []tailcfg.Service{} - for _, p := range ports { - s := tailcfg.Service{ - Proto: tailcfg.ServiceProto(p.Proto), - Port: p.Port, - Description: p.Process, - } - if policy.IsInterestingService(s, version.OS()) { - sl = append(sl, s) - } - } - - b.mu.Lock() - if b.hostinfo == nil { - b.hostinfo = new(tailcfg.Hostinfo) - } - b.hostinfo.Services = sl - b.mu.Unlock() - - b.doSetHostinfoFilterServices() - } -} - // GetPushDeviceToken returns the push notification device token. func (b *LocalBackend) GetPushDeviceToken() string { return b.pushDeviceToken.Load() @@ -3853,23 +3792,6 @@ func (b *LocalBackend) parseWgStatusLocked(s *wgengine.Status) (ret ipn.EngineSt return ret } -// shouldUploadServices reports whether this node should include services -// in Hostinfo. When the user preferences currently request "shields up" -// mode, all inbound connections are refused, so services are not reported. -// Otherwise, shouldUploadServices respects NetMap.CollectServices. -// TODO(nickkhyl): move this into [nodeBackend]? -func (b *LocalBackend) shouldUploadServices() bool { - b.mu.Lock() - defer b.mu.Unlock() - - p := b.pm.CurrentPrefs() - nm := b.currentNode().NetMap() - if !p.Valid() || nm == nil { - return false // default to safest setting - } - return !p.ShieldsUp() && nm.CollectServices -} - // SetCurrentUser is used to implement support for multi-user systems (only // Windows 2022-11-25). On such systems, the actor is used to determine which // user's state should be used. The current user is maintained by active @@ -4812,6 +4734,25 @@ func (b *LocalBackend) peerAPIServicesLocked() (ret []tailcfg.Service) { return ret } +// PortlistServices is an eventbus topic for the portlist extension +// to advertise the running services on the host. +type PortlistServices []tailcfg.Service + +func (b *LocalBackend) setPortlistServices(sl []tailcfg.Service) { + if !buildfeatures.HasPortList { // redundant, but explicit for linker deadcode and humans + return + } + + b.mu.Lock() + if b.hostinfo == nil { + b.hostinfo = new(tailcfg.Hostinfo) + } + b.hostinfo.Services = sl + b.mu.Unlock() + + b.doSetHostinfoFilterServices() +} + // doSetHostinfoFilterServices calls SetHostinfo on the controlclient, // possibly after mangling the given hostinfo. // @@ -4837,13 +4778,15 @@ func (b *LocalBackend) doSetHostinfoFilterServices() { // TODO(maisem,bradfitz): store hostinfo as a view, not as a mutable struct. hi := *b.hostinfo // shallow copy - unlock.UnlockEarly() // Make a shallow copy of hostinfo so we can mutate // at the Service field. - if !b.shouldUploadServices() { + if f, ok := b.extHost.Hooks().ShouldUploadServices.GetOk(); !ok || !f() { hi.Services = []tailcfg.Service{} } + + unlock.UnlockEarly() + // Don't mutate hi.Service's underlying array. Append to // the slice with no free capacity. c := len(hi.Services) diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 56d65767b..fd78c3418 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -5816,7 +5816,6 @@ func newLocalBackendWithSysAndTestControl(t *testing.T, enableLogging bool, sys t.Fatalf("NewLocalBackend: %v", err) } t.Cleanup(b.Shutdown) - b.DisablePortPollerForTest() b.SetControlClientGetterForTesting(func(opts controlclient.Options) (controlclient.Client, error) { return newControl(t, opts), nil diff --git a/ipn/ipnlocal/node_backend.go b/ipn/ipnlocal/node_backend.go index 4319ed372..a6e4b51f1 100644 --- a/ipn/ipnlocal/node_backend.go +++ b/ipn/ipnlocal/node_backend.go @@ -258,6 +258,12 @@ func (nb *nodeBackend) PeersForTest() []tailcfg.NodeView { return ret } +func (nb *nodeBackend) CollectServices() bool { + nb.mu.Lock() + defer nb.mu.Unlock() + return nb.netMap != nil && nb.netMap.CollectServices +} + // AppendMatchingPeers returns base with all peers that match pred appended. // // It acquires b.mu to read the netmap but releases it before calling pred. diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index 1a32f3156..9c0aa66a9 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -358,7 +358,6 @@ func TestStateMachine(t *testing.T) { t.Fatalf("NewLocalBackend: %v", err) } t.Cleanup(b.Shutdown) - b.DisablePortPollerForTest() var cc, previousCC *mockControl b.SetControlClientGetterForTesting(func(opts controlclient.Options) (controlclient.Client, error) { diff --git a/ipn/lapitest/backend.go b/ipn/lapitest/backend.go index 725ffa4de..7a1c276a7 100644 --- a/ipn/lapitest/backend.go +++ b/ipn/lapitest/backend.go @@ -45,7 +45,6 @@ func newBackend(opts *options) *ipnlocal.LocalBackend { tb.Fatalf("NewLocalBackend: %v", err) } tb.Cleanup(b.Shutdown) - b.DisablePortPollerForTest() b.SetControlClientGetterForTesting(opts.MakeControlClient) return b } diff --git a/tsnet/depaware.txt b/tsnet/depaware.txt index 795e4367f..ece4345d5 100644 --- a/tsnet/depaware.txt +++ b/tsnet/depaware.txt @@ -251,7 +251,6 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware) tailscale.com/ipn/ipnlocal from tailscale.com/ipn/localapi+ tailscale.com/ipn/ipnstate from tailscale.com/client/local+ tailscale.com/ipn/localapi from tailscale.com/tsnet - tailscale.com/ipn/policy from tailscale.com/ipn/ipnlocal tailscale.com/ipn/store from tailscale.com/ipn/ipnlocal+ L tailscale.com/ipn/store/awsstore from tailscale.com/ipn/store L tailscale.com/ipn/store/kubestore from tailscale.com/ipn/store @@ -288,7 +287,6 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware) tailscale.com/net/netknob from tailscale.com/logpolicy+ 💣 tailscale.com/net/netmon from tailscale.com/control/controlclient+ 💣 tailscale.com/net/netns from tailscale.com/derp/derphttp+ - W 💣 tailscale.com/net/netstat from tailscale.com/portlist tailscale.com/net/netutil from tailscale.com/client/local+ tailscale.com/net/netx from tailscale.com/control/controlclient+ tailscale.com/net/packet from tailscale.com/ipn/ipnlocal+ @@ -312,7 +310,6 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware) tailscale.com/net/udprelay/status from tailscale.com/client/local tailscale.com/omit from tailscale.com/ipn/conffile tailscale.com/paths from tailscale.com/client/local+ - 💣 tailscale.com/portlist from tailscale.com/ipn/ipnlocal tailscale.com/posture from tailscale.com/ipn/ipnlocal tailscale.com/proxymap from tailscale.com/tsd+ 💣 tailscale.com/safesocket from tailscale.com/client/local+ @@ -356,7 +353,7 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware) tailscale.com/util/cmpver from tailscale.com/clientupdate+ tailscale.com/util/ctxkey from tailscale.com/client/tailscale/apitype+ 💣 tailscale.com/util/deephash from tailscale.com/ipn/ipnlocal+ - LA 💣 tailscale.com/util/dirwalk from tailscale.com/metrics+ + LA 💣 tailscale.com/util/dirwalk from tailscale.com/metrics tailscale.com/util/dnsname from tailscale.com/appc+ tailscale.com/util/eventbus from tailscale.com/ipn/localapi+ tailscale.com/util/execqueue from tailscale.com/appc+ diff --git a/tsnet/tsnet_test.go b/tsnet/tsnet_test.go index d00628453..1e22681fc 100644 --- a/tsnet/tsnet_test.go +++ b/tsnet/tsnet_test.go @@ -43,6 +43,7 @@ import ( "tailscale.com/net/netns" "tailscale.com/tailcfg" "tailscale.com/tstest" + "tailscale.com/tstest/deptest" "tailscale.com/tstest/integration" "tailscale.com/tstest/integration/testcontrol" "tailscale.com/types/key" @@ -1302,3 +1303,15 @@ func mustDirect(t *testing.T, logf logger.Logf, lc1, lc2 *local.Client) { } t.Error("magicsock did not find a direct path from lc1 to lc2") } + +func TestDeps(t *testing.T) { + deptest.DepChecker{ + GOOS: "linux", + GOARCH: "amd64", + OnDep: func(dep string) { + if strings.Contains(dep, "portlist") { + t.Errorf("unexpected dep: %q", dep) + } + }, + }.Check(t) +}