From 4bb6f35c1f89df94eb462640abc8aab3c691da11 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sat, 27 Jun 2026 14:07:10 +0000 Subject: [PATCH] ipn/ipnlocal: consolidate test-only LocalBackend methods behind ForTest Move all the FooForTest methods on LocalBackend to instead be methods on a new unexported forTest type which is then given out to callers in other packages via an exported ForTest method (panicking in non-test contexts) that returns that unexported type. This is unusual style (exported returning unexported) but declutters godoc and makes call sites both more explicit and easier to read without the "ForTest" suffix polluting the symbols. Now FooForTest() changes into ForTest().Foo(). This was motivated by a pending change moving a bunch of code out of LocalBackend into other packages that required adding more ForTest methods to LocalBackend to keep the tests (now in other packages) working. Instead, do this refactor now so the future change is prettier. Updates #12614 Updates #cleanup Signed-off-by: Brad Fitzpatrick Change-Id: Ib25e6d76d48dc8622ac3a955e0b1220d582e63a8 --- feature/tailnetlock/tailnetlock_test.go | 8 +- ipn/ipnlocal/c2n.go | 8 -- ipn/ipnlocal/cert.go | 14 --- ipn/ipnlocal/cert_state.go | 4 +- ipn/ipnlocal/cert_test.go | 2 +- ipn/ipnlocal/fortest.go | 121 +++++++++++++++++++++++ ipn/ipnlocal/local.go | 62 +----------- ipn/ipnlocal/local_test.go | 51 ++++------ ipn/ipnlocal/loglines_test.go | 2 +- ipn/ipnlocal/state_test.go | 4 +- ipn/ipnlocal/tailnet-lock.go | 24 ++--- ipn/ipnlocal/tailnet-lock_test.go | 2 +- ipn/ipnserver/server_test.go | 2 +- ipn/lapitest/backend.go | 2 +- ipn/lapitest/server.go | 2 +- tsnet/packet_filter_test.go | 4 +- tsnet/tsnet_test.go | 6 +- tstest/largetailnet/delta_test.go | 4 +- tstest/largetailnet/largetailnet_test.go | 4 +- wgengine/netstack/netstack_test.go | 12 +-- 20 files changed, 181 insertions(+), 157 deletions(-) create mode 100644 ipn/ipnlocal/fortest.go diff --git a/feature/tailnetlock/tailnetlock_test.go b/feature/tailnetlock/tailnetlock_test.go index 53d659ef9..03afc4fdb 100644 --- a/feature/tailnetlock/tailnetlock_test.go +++ b/feature/tailnetlock/tailnetlock_test.go @@ -68,7 +68,7 @@ func TestHandleC2NDebugTKA(t *testing.T) { req := httptest.NewRequest("GET", "/debug/tka/log", nil) rec := httptest.NewRecorder() - b.HandleC2NForTest(rec, req) + b.ForTest().HandleC2N(rec, req) if rec.Code != 400 { t.Fatalf("got status code: %v, want: 400\nBody: %s", rec.Code, rec.Body) @@ -81,7 +81,7 @@ func TestHandleC2NDebugTKA(t *testing.T) { req := httptest.NewRequest("GET", "/debug/tka/log", nil) rec := httptest.NewRecorder() - b.HandleC2NForTest(rec, req) + b.ForTest().HandleC2N(rec, req) if rec.Code != 200 { t.Fatalf("got status code: %v, want: 200\nBody: %s", rec.Code, bodyHead(rec.Body)) @@ -103,7 +103,7 @@ func TestHandleC2NDebugTKA(t *testing.T) { req := httptest.NewRequest("GET", "/debug/tka/log", nil) rec := httptest.NewRecorder() - b.HandleC2NForTest(rec, req) + b.ForTest().HandleC2N(rec, req) if rec.Code != 200 { t.Fatalf("got status code: %v, want: 200\nBody: %s", rec.Code, bodyHead(rec.Body)) @@ -125,7 +125,7 @@ func TestHandleC2NDebugTKA(t *testing.T) { req := httptest.NewRequest("GET", "/debug/tka/log?limit=60", nil) rec := httptest.NewRecorder() - b.HandleC2NForTest(rec, req) + b.ForTest().HandleC2N(rec, req) if rec.Code != 200 { t.Fatalf("got status code: %v, want: 200\nBody: %s", rec.Code, bodyHead(rec.Body)) diff --git a/ipn/ipnlocal/c2n.go b/ipn/ipnlocal/c2n.go index bf8cf2e03..8284872b9 100644 --- a/ipn/ipnlocal/c2n.go +++ b/ipn/ipnlocal/c2n.go @@ -27,7 +27,6 @@ import ( "tailscale.com/util/goroutines" "tailscale.com/util/httpm" "tailscale.com/util/set" - "tailscale.com/util/testenv" "tailscale.com/version" ) @@ -324,10 +323,3 @@ func handleC2NSetNetfilterKind(b *LocalBackend, w http.ResponseWriter, r *http.R w.WriteHeader(http.StatusNoContent) } - -// HandleC2NForTest calls [handleC2N], for use by feature/ packages that -// register C2N handlers and want to test them. -func (b *LocalBackend) HandleC2NForTest(w http.ResponseWriter, r *http.Request) { - testenv.AssertInTest() - b.handleC2N(w, r) -} diff --git a/ipn/ipnlocal/cert.go b/ipn/ipnlocal/cert.go index 87aa016a9..254bc3996 100644 --- a/ipn/ipnlocal/cert.go +++ b/ipn/ipnlocal/cert.go @@ -467,20 +467,6 @@ func (b *LocalBackend) getCertStore() (certStore, error) { return certFileStore{dir: dir, testRoots: testX509Roots}, nil } -// ConfigureCertsForTest sets a certificate retrieval function to be used by -// this local backend, skipping the usual ACME certificate registration. Should -// only be used in tests. -func (b *LocalBackend) ConfigureCertsForTest(getCert func(hostname string) (*TLSCertKeyPair, error)) { - testenv.AssertInTest() - cs := b.certState() - if cs == nil { - panic("ConfigureCertsForTest called without cert extension registered") - } - b.mu.Lock() - cs.getCertForTest = getCert - b.mu.Unlock() -} - // certFileStore implements certStore by storing the cert & key files in the named directory. type certFileStore struct { dir string diff --git a/ipn/ipnlocal/cert_state.go b/ipn/ipnlocal/cert_state.go index fe5e2fbb4..1f16685ce 100644 --- a/ipn/ipnlocal/cert_state.go +++ b/ipn/ipnlocal/cert_state.go @@ -53,8 +53,8 @@ type CertState struct { pendingCertDomains set.Set[string] // getCertForTest is used to retrieve TLS certificates in tests. - // See [LocalBackend.ConfigureCertsForTest]. Guarded by the - // containing [LocalBackend]'s mutex (b.mu). + // See [forTest.ConfigureCerts]. Guarded by the containing + // [LocalBackend]'s mutex (b.mu). getCertForTest func(hostname string) (*TLSCertKeyPair, error) // certRefreshCancel cancels the background TLS cert refresh loop diff --git a/ipn/ipnlocal/cert_test.go b/ipn/ipnlocal/cert_test.go index 6e35939c5..6b04db7f2 100644 --- a/ipn/ipnlocal/cert_test.go +++ b/ipn/ipnlocal/cert_test.go @@ -986,7 +986,7 @@ func TestRefreshApplicableCerts(t *testing.T) { b.mu.Unlock() gotCh := make(chan string, 4) - b.ConfigureCertsForTest(func(host string) (*TLSCertKeyPair, error) { + b.ForTest().ConfigureCerts(func(host string) (*TLSCertKeyPair, error) { gotCh <- host return &TLSCertKeyPair{}, nil }) diff --git a/ipn/ipnlocal/fortest.go b/ipn/ipnlocal/fortest.go new file mode 100644 index 000000000..398ea302c --- /dev/null +++ b/ipn/ipnlocal/fortest.go @@ -0,0 +1,121 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package ipnlocal + +import ( + "net/http" + + "tailscale.com/control/controlclient" + "tailscale.com/ipn" + "tailscale.com/ipn/ipnauth" + "tailscale.com/tailcfg" + "tailscale.com/types/key" + "tailscale.com/types/netmap" + "tailscale.com/util/testenv" + "tailscale.com/wgengine/filter" +) + +// forTest is an unexported type to hide all the test-only +// methods on [LocalBackend] from godoc. +type forTest struct{ b *LocalBackend } + +// ForTest returns a handle to test-only methods on b. +// The resulting type is unexported to make it very obvious +// in godoc that this is not stable API. This method panics +// if called outside of tests, which also centralizes all +// must-be-in-tests validation. +func (b *LocalBackend) ForTest() forTest { + testenv.AssertInTest() + return forTest{b} +} + +// HandleC2N calls [LocalBackend.handleC2N], for use by feature/ packages that +// register C2N handlers and want to test them. +func (f forTest) HandleC2N(w http.ResponseWriter, r *http.Request) { + f.b.handleC2N(w, r) +} + +// SetIPServiceMappings overwrites the LocalBackend's IP-to-service mappings +// and propagates them to the netstack subsystem if registered. +func (f forTest) SetIPServiceMappings(m netmap.IPServiceMappings) { + b := f.b + b.mu.Lock() + defer b.mu.Unlock() + b.ipVIPServiceMap = m + if ns, ok := b.sys.Netstack.GetOK(); ok { + ns.UpdateIPServiceMappings(m) + } +} + +// GetFilter returns the current packet filter. +func (f forTest) GetFilter() *filter.Filter { + b := f.b + // Take b.mu so the read serializes with [LocalBackend.setControlClientStatusLocked], + // which installs the netmap and the filter at separate sub-steps. Without + // this, a test thread that observes the new netmap (via [LocalBackend.NetMapWithPeers]) + // can race ahead of the filter store and read the previous filter. + b.mu.Lock() + defer b.mu.Unlock() + return b.currentNode().filterAtomic.Load() +} + +// SetControlClientGetter sets the func that creates a control plane +// client. It can be called at most once, before Start. +func (f forTest) SetControlClientGetter(newControlClient func(controlclient.Options) (controlclient.Client, error)) { + b := f.b + b.mu.Lock() + defer b.mu.Unlock() + if b.ccGen != nil { + panic("invalid use of forTest.SetControlClientGetter after Start") + } + b.ccGen = newControlClient +} + +// Peers returns all the current peers, sorted by Node.ID, for integration +// tests in another repo. +func (f forTest) Peers() []tailcfg.NodeView { + return f.b.currentNode().PeersForTest() +} + +// AwaitNodeKey returns a channel that is closed once a peer with the given +// node key first appears in the current netmap. If the peer is already +// present, the returned channel is already closed. See +// [nodeBackend.AwaitNodeKeyForTest]. +func (f forTest) AwaitNodeKey(k key.NodePublic) <-chan struct{} { + return f.b.currentNode().AwaitNodeKeyForTest(k) +} + +// CurrentUser returns the current user and the associated WindowsUserID. +// It will be removed along with the rest of the "current user" functionality +// as we progress on the multi-user improvements (tailscale/corp#18342). +func (f forTest) CurrentUser() (ipn.WindowsUserID, ipnauth.Actor) { + b := f.b + b.mu.Lock() + defer b.mu.Unlock() + return b.pm.CurrentUserID(), b.currentUser +} + +// ConfigureCerts sets a certificate retrieval function to be used by this +// local backend, skipping the usual ACME certificate registration. +func (f forTest) ConfigureCerts(getCert func(hostname string) (*TLSCertKeyPair, error)) { + b := f.b + cs := b.certState() + if cs == nil { + panic("forTest.ConfigureCerts called without cert extension registered") + } + b.mu.Lock() + cs.getCertForTest = getCert + b.mu.Unlock() +} + +// SetPrefs replaces the current prefs with newp. +func (f forTest) SetPrefs(newp *ipn.Prefs) { + if newp == nil { + panic("forTest.SetPrefs got nil prefs") + } + b := f.b + b.mu.Lock() + defer b.mu.Unlock() + b.setPrefsLocked(newp) +} diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 12b998149..0f6f2ffa6 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -539,7 +539,7 @@ type serveLabels struct { } // clientGen is a func that creates a control plane client. -// It's the type used by LocalBackend.SetControlClientGetterForTesting. +// It's the type used by forTest.SetControlClientGetter. type clientGen func(controlclient.Options) (controlclient.Client, error) // NewLocalBackend returns a new LocalBackend that is ready to run, @@ -1087,16 +1087,6 @@ func (b *LocalBackend) IPServiceMappings() netmap.IPServiceMappings { return b.ipVIPServiceMap } -func (b *LocalBackend) SetIPServiceMappingsForTest(m netmap.IPServiceMappings) { - b.mu.Lock() - defer b.mu.Unlock() - testenv.AssertInTest() - b.ipVIPServiceMap = m - if ns, ok := b.sys.Netstack.GetOK(); ok { - ns.UpdateIPServiceMappings(m) - } -} - // setConfigLocked uses the provided config to update the backend's prefs // and other state. func (b *LocalBackend) setConfigLocked(conf *conffile.Config) error { @@ -1829,18 +1819,6 @@ func (b *LocalBackend) UserProfile(id tailcfg.UserID) (u tailcfg.UserProfileView return b.currentNode().UserByID(id) } -func (b *LocalBackend) GetFilterForTest() *filter.Filter { - testenv.AssertInTest() - // Take b.mu so the read serializes with [setControlClientStatusLocked], - // which installs the netmap and the filter at separate sub-steps. Without - // this, a test thread that observes the new netmap (via [NetMapWithPeers]) - // can race ahead of the filter store and read the previous filter. - b.mu.Lock() - defer b.mu.Unlock() - nb := b.currentNode() - return nb.filterAtomic.Load() -} - // SetControlClientStatus is the callback invoked by the control client whenever it posts a new status. // Among other things, this is where we update the netmap, packet filters, DNS and DERP maps. func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st controlclient.Status) { @@ -2970,37 +2948,11 @@ func (b *LocalBackend) SetHTTPTestClient(c *http.Client) { b.httpTestClient = c } -// SetControlClientGetterForTesting sets the func that creates a -// control plane client. It can be called at most once, before Start. -func (b *LocalBackend) SetControlClientGetterForTesting(newControlClient func(controlclient.Options) (controlclient.Client, error)) { - b.mu.Lock() - defer b.mu.Unlock() - if b.ccGen != nil { - panic("invalid use of SetControlClientGetterForTesting after Start") - } - b.ccGen = newControlClient -} - -// PeersForTest returns all the current peers, sorted by Node.ID, -// for integration tests in another repo. -func (b *LocalBackend) PeersForTest() []tailcfg.NodeView { - testenv.AssertInTest() - return b.currentNode().PeersForTest() -} - -// AwaitNodeKeyForTest returns a channel that is closed once a peer with the -// given node key first appears in the current netmap. If the peer is already -// present, the returned channel is already closed. See -// [nodeBackend.AwaitNodeKeyForTest]. -func (b *LocalBackend) AwaitNodeKeyForTest(k key.NodePublic) <-chan struct{} { - return b.currentNode().AwaitNodeKeyForTest(k) -} - func (b *LocalBackend) getNewControlClientFuncLocked() clientGen { if b.ccGen == nil { // Initialize it rather than just returning the // default to make any future call to - // SetControlClientGetterForTesting panic. + // forTest.SetControlClientGetter panic. b.ccGen = func(opts controlclient.Options) (controlclient.Client, error) { return controlclient.New(opts) } @@ -4927,16 +4879,6 @@ func (b *LocalBackend) resolveBestProfileLocked() (_ ipn.LoginProfileView, isBac return b.pm.CurrentProfile(), false } -// CurrentUserForTest returns the current user and the associated WindowsUserID. -// It is used for testing only, and will be removed along with the rest of the -// "current user" functionality as we progress on the multi-user improvements (tailscale/corp#18342). -func (b *LocalBackend) CurrentUserForTest() (ipn.WindowsUserID, ipnauth.Actor) { - testenv.AssertInTest() - b.mu.Lock() - defer b.mu.Unlock() - return b.pm.CurrentUserID(), b.currentUser -} - // CheckPrefs validates the provided user modifiable settings for correctness // and returns an error if they are invalid for the current backend. func (b *LocalBackend) CheckPrefs(p *ipn.Prefs) error { diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index d98f0a76d..07a9f65a2 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -1419,7 +1419,7 @@ func TestConfigureExitNode(t *testing.T) { sys := tsd.NewSystem() sys.PolicyClient.Set(pol) lb := newTestLocalBackendWithSys(t, sys) - lb.SetPrefsForTest(tt.prefs.Clone()) + lb.ForTest().SetPrefs(tt.prefs.Clone()) // Then set the netcheck report and netmap, if any. Clone the shared // report because AddNetcheckReportForTest mutates it and subtests run @@ -1643,9 +1643,9 @@ func TestPrefsChangeDisablesExitNode(t *testing.T) { if tt.netMap != nil { lb.SetControlClientStatus(lb.cc, controlclient.Status{NetMap: tt.netMap}) } - // Set the initial prefs via SetPrefsForTest + // Set the initial prefs via the test helper. // to apply necessary adjustments. - lb.SetPrefsForTest(tt.prefs.Clone()) + lb.ForTest().SetPrefs(tt.prefs.Clone()) initialPrefs := lb.Prefs() // Check whether changeDisablesExitNodeLocked correctly identifies the change. @@ -1684,7 +1684,7 @@ func TestExitNodeNotifyOrder(t *testing.T) { lb := newTestLocalBackend(t) lb.sys.MagicSock.Get().AddNetcheckReportForTest(clientNetmap.DERPMap, report, time.Now()) - lb.SetPrefsForTest(&ipn.Prefs{ + lb.ForTest().SetPrefs(&ipn.Prefs{ ControlURL: controlURL, AutoExitNode: ipn.AnyExitNode, }) @@ -3970,11 +3970,11 @@ func TestSetExitNodeIDPolicy(t *testing.T) { t.Errorf("wanted prefs changed %v, got prefs changed %v", test.prefsChanged, changed) } - // Both [LocalBackend.SetPrefsForTest] and [LocalBackend.EditPrefs] + // Both [forTest.SetPrefs] and [LocalBackend.EditPrefs] // apply syspolicy settings to the current profile's preferences. Therefore, // we pass the current, unmodified preferences and expect the effective // preferences to change. - b.SetPrefsForTest(pm.CurrentPrefs().AsStruct()) + b.ForTest().SetPrefs(pm.CurrentPrefs().AsStruct()) if got := b.Prefs().ExitNodeID(); got != tailcfg.StableNodeID(test.exitNodeIDWant) { t.Errorf("ExitNodeID: got %q; want %q", got, test.exitNodeIDWant) @@ -4094,7 +4094,7 @@ func TestUpdateNetmapDeltaAutoExitNode(t *testing.T) { b.currentNode().SetNetMap(tt.netmap) b.lastSuggestedExitNode = tt.lastSuggestedExitNode b.sys.MagicSock.Get().AddNetcheckReportForTest(derpMap, tt.report, time.Now()) - b.SetPrefsForTest(b.pm.CurrentPrefs().AsStruct()) + b.ForTest().SetPrefs(b.pm.CurrentPrefs().AsStruct()) allDone := make(chan bool, 1) defer b.goTracker.AddDoneCallback(func() { @@ -4222,7 +4222,7 @@ func TestAutoExitNodeSetNetInfoCallback(t *testing.T) { DERPMap: defaultDERPMap, }) b.lastSuggestedExitNode = peer1.StableID() - b.SetPrefsForTest(b.pm.CurrentPrefs().AsStruct()) + b.ForTest().SetPrefs(b.pm.CurrentPrefs().AsStruct()) if eid := b.Prefs().ExitNodeID(); eid != peer1.StableID() { t.Errorf("got initial exit node %v, want %v", eid, peer1.StableID()) } @@ -4292,7 +4292,7 @@ func TestSetControlClientStatusAutoExitNode(t *testing.T) { // in terms of latency and DERP region. b.lastSuggestedExitNode = peer2.StableID() b.sys.MagicSock.Get().AddNetcheckReportForTest(derpMap, report, time.Now()) - b.SetPrefsForTest(b.pm.CurrentPrefs().AsStruct()) + b.ForTest().SetPrefs(b.pm.CurrentPrefs().AsStruct()) offlinePeer2 := makePeer(2, withCap(26), withSuggest(), withExitRoutes(), withOnline(false), withNodeKey()) updatedNetmap := &netmap.NetworkMap{ Peers: []tailcfg.NodeView{ @@ -5390,15 +5390,6 @@ func TestRoundTraffic(t *testing.T) { } } -func (b *LocalBackend) SetPrefsForTest(newp *ipn.Prefs) { - if newp == nil { - panic("SetPrefsForTest got nil prefs") - } - b.mu.Lock() - defer b.mu.Unlock() - b.setPrefsLocked(newp) -} - type peerOptFunc func(*tailcfg.Node) func makePeer(id tailcfg.NodeID, opts ...peerOptFunc) tailcfg.NodeView { @@ -7080,7 +7071,7 @@ func newLocalBackendWithSysAndTestControl(t testing.TB, enableLogging bool, sys } t.Cleanup(b.Shutdown) - b.SetControlClientGetterForTesting(func(opts controlclient.Options) (controlclient.Client, error) { + b.ForTest().SetControlClientGetter(func(opts controlclient.Options) (controlclient.Client, error) { return newControl(t, opts), nil }) return b @@ -7874,7 +7865,7 @@ func TestUpdatePrefsOnSysPolicyChange(t *testing.T) { return newClient(tb, opts) }) if tt.initialPrefs != nil { - lb.SetPrefsForTest(tt.initialPrefs) + lb.ForTest().SetPrefs(tt.initialPrefs) } if err := lb.Start(ipn.Options{}); err != nil { t.Fatalf("(*LocalBackend).Start(): %v", err) @@ -8155,7 +8146,7 @@ func TestSrcCapPacketFilter(t *testing.T) { }}, }}) - f := lb.GetFilterForTest() + f := lb.ForTest().GetFilter() res := f.Check(netip.MustParseAddr("2.2.2.2"), netip.MustParseAddr("1.1.1.1"), 22, ipproto.TCP) if res != filter.Accept { t.Errorf("Check(2.2.2.2, ...) = %s, want %s", res, filter.Accept) @@ -8317,7 +8308,7 @@ func TestDisplayMessageIPNBus(t *testing.T) { }, }}) - lb.SetPrefsForTest(&ipn.Prefs{ + lb.ForTest().SetPrefs(&ipn.Prefs{ ControlURL: "https://localhost:1/", WantRunning: true, LoggedOut: false, @@ -8387,7 +8378,7 @@ func TestOnClientVersionRespectsAutoUpdateCheck(t *testing.T) { } // With Check disabled, onClientVersion should cache but not broadcast. - lb.SetPrefsForTest(&ipn.Prefs{ + lb.ForTest().SetPrefs(&ipn.Prefs{ AutoUpdate: ipn.AutoUpdatePrefs{Check: false}, }) @@ -8405,7 +8396,7 @@ func TestOnClientVersionRespectsAutoUpdateCheck(t *testing.T) { } // With Check enabled, onClientVersion should broadcast. - lb.SetPrefsForTest(&ipn.Prefs{ + lb.ForTest().SetPrefs(&ipn.Prefs{ AutoUpdate: ipn.AutoUpdatePrefs{Check: true}, }) @@ -8425,7 +8416,7 @@ func TestWatchNotificationsInitialClientVersion(t *testing.T) { } // Set Check=true and cache a ClientVersion. - lb.SetPrefsForTest(&ipn.Prefs{ + lb.ForTest().SetPrefs(&ipn.Prefs{ AutoUpdate: ipn.AutoUpdatePrefs{Check: true}, }) lb.mu.Lock() @@ -8445,7 +8436,7 @@ func TestWatchNotificationsInitialClientVersion(t *testing.T) { nw2.check() // Watch with the flag but Check=false, should not include it. - lb.SetPrefsForTest(&ipn.Prefs{ + lb.ForTest().SetPrefs(&ipn.Prefs{ AutoUpdate: ipn.AutoUpdatePrefs{Check: false}, }) nw3 := newNotificationWatcher(t, lb, ipnauth.Self) @@ -8908,7 +8899,7 @@ func TestNoSNATWithAdvertisedExitNodeWarning(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { b := newTestLocalBackend(t) - b.SetPrefsForTest(tt.prefs) + b.ForTest().SetPrefs(tt.prefs) _, hasWarning := b.HealthTracker().CurrentState().Warnings[warnCode] if hasWarning != tt.wantWarning { t.Errorf("warning present = %v, want %v", hasWarning, tt.wantWarning) @@ -8919,11 +8910,11 @@ func TestNoSNATWithAdvertisedExitNodeWarning(t *testing.T) { // Verify that the warning clears when the conflicting combination is resolved. t.Run("warning-clears-on-fix", func(t *testing.T) { b := newTestLocalBackend(t) - b.SetPrefsForTest(&ipn.Prefs{NoSNAT: true, AdvertiseRoutes: exitRoutes}) + b.ForTest().SetPrefs(&ipn.Prefs{NoSNAT: true, AdvertiseRoutes: exitRoutes}) if _, ok := b.HealthTracker().CurrentState().Warnings[warnCode]; !ok { t.Fatal("expected warning to be set") } - b.SetPrefsForTest(&ipn.Prefs{NoSNAT: false, AdvertiseRoutes: exitRoutes}) + b.ForTest().SetPrefs(&ipn.Prefs{NoSNAT: false, AdvertiseRoutes: exitRoutes}) if _, ok := b.HealthTracker().CurrentState().Warnings[warnCode]; ok { t.Fatal("expected warning to be cleared after enabling SNAT") } @@ -8957,7 +8948,7 @@ func TestStartPreservesLoginFlags(t *testing.T) { t.Cleanup(b.Shutdown) var cc *mockControl - b.SetControlClientGetterForTesting(func(opts controlclient.Options) (controlclient.Client, error) { + b.ForTest().SetControlClientGetter(func(opts controlclient.Options) (controlclient.Client, error) { cc = newClient(t, opts) return cc, nil }) diff --git a/ipn/ipnlocal/loglines_test.go b/ipn/ipnlocal/loglines_test.go index 733c7381b..dde70e742 100644 --- a/ipn/ipnlocal/loglines_test.go +++ b/ipn/ipnlocal/loglines_test.go @@ -80,7 +80,7 @@ func TestLocalLogLines(t *testing.T) { persist := &persist.Persist{} prefs := ipn.NewPrefs() prefs.Persist = persist - lb.SetPrefsForTest(prefs) + lb.ForTest().SetPrefs(prefs) t.Run("after_prefs", testWantRemain("[v1] peer keys: %s", "[v1] v%v peers: %v")) diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index 0e6adfa1f..7c1c9a23b 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -397,7 +397,7 @@ func TestStateMachine(t *testing.T) { t.Cleanup(b.Shutdown) var cc, previousCC *mockControl - b.SetControlClientGetterForTesting(func(opts controlclient.Options) (controlclient.Client, error) { + b.ForTest().SetControlClientGetter(func(opts controlclient.Options) (controlclient.Client, error) { previousCC = cc cc = newClient(t, opts) @@ -1147,7 +1147,7 @@ func TestWGEngineStatusRace(t *testing.T) { t.Cleanup(b.Shutdown) var cc *mockControl - b.SetControlClientGetterForTesting(func(opts controlclient.Options) (controlclient.Client, error) { + b.ForTest().SetControlClientGetter(func(opts controlclient.Options) (controlclient.Client, error) { cc = newClient(t, opts) return cc, nil }) diff --git a/ipn/ipnlocal/tailnet-lock.go b/ipn/ipnlocal/tailnet-lock.go index 34a9b588b..7cd391467 100644 --- a/ipn/ipnlocal/tailnet-lock.go +++ b/ipn/ipnlocal/tailnet-lock.go @@ -776,9 +776,10 @@ func (b *LocalBackend) NetworkLockAllowed() bool { return b.TailnetLockAllowed() } -// Only use is in tests. -func (b *LocalBackend) TailnetLockVerifySignatureForTest(nks tkatype.MarshaledSignature, nodeKey key.NodePublic) error { - testenv.AssertInTest() +// TailnetLockVerifySignature verifies that nks is a valid tailnet lock +// signature for the given node key. +func (f forTest) TailnetLockVerifySignature(nks tkatype.MarshaledSignature, nodeKey key.NodePublic) error { + b := f.b b.mu.Lock() defer b.mu.Unlock() if b.tka == nil { @@ -787,14 +788,10 @@ func (b *LocalBackend) TailnetLockVerifySignatureForTest(nks tkatype.MarshaledSi return b.tka.authority.NodeKeyAuthorized(nodeKey, nks) } -// Deprecated: use [LocalBackend.TailnetLockVerifySignatureForTest] instead. -func (b *LocalBackend) NetworkLockVerifySignatureForTest(nks tkatype.MarshaledSignature, nodeKey key.NodePublic) error { - return b.TailnetLockVerifySignatureForTest(nks, nodeKey) -} - -// Only use is in tests. -func (b *LocalBackend) TailnetLockKeyTrustedForTest(keyID tkatype.KeyID) bool { - testenv.AssertInTest() +// TailnetLockKeyTrusted reports whether keyID is trusted by the tailnet lock +// authority. It panics if tailnet lock is not initialized. +func (f forTest) TailnetLockKeyTrusted(keyID tkatype.KeyID) bool { + b := f.b b.mu.Lock() defer b.mu.Unlock() if b.tka == nil { @@ -803,11 +800,6 @@ func (b *LocalBackend) TailnetLockKeyTrustedForTest(keyID tkatype.KeyID) bool { return b.tka.authority.KeyTrusted(keyID) } -// Deprecated: use [LocalBackend.TailnetLockKeyTrustedForTest] instead. -func (b *LocalBackend) NetworkLockKeyTrustedForTest(keyID tkatype.KeyID) bool { - return b.TailnetLockKeyTrustedForTest(keyID) -} - // TailnetLockForceLocalDisable shuts down TKA locally, and denylists the current // TKA from being initialized locally in future. func (b *LocalBackend) TailnetLockForceLocalDisable() error { diff --git a/ipn/ipnlocal/tailnet-lock_test.go b/ipn/ipnlocal/tailnet-lock_test.go index c28c21340..267142ffe 100644 --- a/ipn/ipnlocal/tailnet-lock_test.go +++ b/ipn/ipnlocal/tailnet-lock_test.go @@ -883,7 +883,7 @@ func TestTKAForceDisable(t *testing.T) { b := newTestLocalBackendWithSys(t, sys) b.SetVarRoot(temp) - b.SetControlClientGetterForTesting(func(controlclient.Options) (controlclient.Client, error) { + b.ForTest().SetControlClientGetter(func(controlclient.Options) (controlclient.Client, error) { return cc, nil }) b.mu.Lock() diff --git a/ipn/ipnserver/server_test.go b/ipn/ipnserver/server_test.go index 45a8d622d..409faf443 100644 --- a/ipn/ipnserver/server_test.go +++ b/ipn/ipnserver/server_test.go @@ -155,7 +155,7 @@ func TestConcurrentOSUserSwitchingOnWindows(t *testing.T) { // Get the current user from the LocalBackend's perspective // as soon as we're connected. - gotUID, gotActor := server.Backend().CurrentUserForTest() + gotUID, gotActor := server.Backend().ForTest().CurrentUser() // Wait for the first notification to arrive. // It will either be the initial state we've requested via [ipn.NotifyInitialState], diff --git a/ipn/lapitest/backend.go b/ipn/lapitest/backend.go index b622d098f..fa21d8cc3 100644 --- a/ipn/lapitest/backend.go +++ b/ipn/lapitest/backend.go @@ -45,7 +45,7 @@ func newBackend(opts *options) *ipnlocal.LocalBackend { tb.Fatalf("NewLocalBackend: %v", err) } tb.Cleanup(b.Shutdown) - b.SetControlClientGetterForTesting(opts.MakeControlClient) + b.ForTest().SetControlClientGetter(opts.MakeControlClient) return b } diff --git a/ipn/lapitest/server.go b/ipn/lapitest/server.go index 2686682af..bb5b51164 100644 --- a/ipn/lapitest/server.go +++ b/ipn/lapitest/server.go @@ -195,7 +195,7 @@ func (s *Server) CheckCurrentUser(want ipnauth.Actor) { if lb == nil { s.tb.Fatalf("Backend: nil") } - gotUID, gotActor := lb.CurrentUserForTest() + gotUID, gotActor := lb.ForTest().CurrentUser() if gotUID != wantUID { s.tb.Errorf("CurrentUser: got UID %q; want %q", gotUID, wantUID) } diff --git a/tsnet/packet_filter_test.go b/tsnet/packet_filter_test.go index 69fb7bb1e..a34a01a5c 100644 --- a/tsnet/packet_filter_test.go +++ b/tsnet/packet_filter_test.go @@ -199,7 +199,7 @@ func TestPacketFilterFromNetmap(t *testing.T) { t.Fatalf("waitFor: %s", err) } - pf := s.lb.GetFilterForTest() + pf := s.lb.ForTest().GetFilter() for _, check := range test.checks { got := pf.Check(netip.MustParseAddr(check.src), netip.MustParseAddr(check.dst), check.port, ipproto.TCP) @@ -230,7 +230,7 @@ func TestPacketFilterFromNetmap(t *testing.T) { t.Fatalf("waitFor: %s", err) } - pf := s.lb.GetFilterForTest() + pf := s.lb.ForTest().GetFilter() for _, check := range test.checks { got := pf.Check(netip.MustParseAddr(check.src), netip.MustParseAddr(check.dst), check.port, ipproto.TCP) diff --git a/tsnet/tsnet_test.go b/tsnet/tsnet_test.go index 80db06255..4fd3a7765 100644 --- a/tsnet/tsnet_test.go +++ b/tsnet/tsnet_test.go @@ -345,7 +345,7 @@ func startServer(t *testing.T, ctx context.Context, controlURL, hostname string) if err != nil { t.Fatal(err) } - s.lb.ConfigureCertsForTest(testCertRoot.getCert) + s.lb.ForTest().ConfigureCerts(testCertRoot.getCert) // Wait for the server to finish connecting to its home DERP server, // to prevent fast tests from racing the DERP handshake resulting @@ -2693,7 +2693,7 @@ func setupTwoClientTest(t *testing.T, useTUN bool) *listenTest { if err != nil { t.Fatal(err) } - s2.lb.ConfigureCertsForTest(testCertRoot.getCert) + s2.lb.ForTest().ConfigureCerts(testCertRoot.getCert) s1ip4, s1ip6 := s1.TailscaleIPs() s2ip4 := s2status.TailscaleIPs[0] @@ -3263,7 +3263,7 @@ func TestDialUDPInjectedReadRecordsFlowState(t *testing.T) { // PacketFilter-only changes don't necessarily fire peer/netmap // notifications, so poll the wgengine filter directly. if err := tstest.WaitFor(30*time.Second, func() error { - f := lt.s2.lb.GetFilterForTest() + f := lt.s2.lb.ForTest().GetFilter() if f == nil { return errors.New("no filter yet") } diff --git a/tstest/largetailnet/delta_test.go b/tstest/largetailnet/delta_test.go index 056394af2..7ca2bb503 100644 --- a/tstest/largetailnet/delta_test.go +++ b/tstest/largetailnet/delta_test.go @@ -51,7 +51,7 @@ func metricByName(t testing.TB, name string) *clientmetric.Metric { // - the corresponding side effect is observable on the [LocalBackend] // (a fresh peer resolvable via PeerByID, a UserProfile resolvable // via UserProfile, a packet filter rule reflected in -// GetFilterForTest, a per-field patch reflected in PeerByID, etc.). +// ForTest().GetFilter, a per-field patch reflected in PeerByID, etc.). // // This is the destination-side companion to // [tstest/largetailnet/BenchmarkGiantTailnet], which only measures cost @@ -201,7 +201,7 @@ func TestNetmapDeltaFastPath(t *testing.T) { if !ok || uv.LoginName() != "alice@example.com" { t.Errorf("UserProfile(%d) ok=%v login=%q", newUser, ok, uv.LoginName()) } - pf := lb.GetFilterForTest() + pf := lb.ForTest().GetFilter() if got := pf.Check(netip.MustParseAddr("100.64.0.42"), selfIP4, 22, ipproto.TCP); got != filter.Accept { t.Errorf("packet filter Check from new peer = %s; want Accept", got) } diff --git a/tstest/largetailnet/largetailnet_test.go b/tstest/largetailnet/largetailnet_test.go index c9ebb1532..b3bda6bc0 100644 --- a/tstest/largetailnet/largetailnet_test.go +++ b/tstest/largetailnet/largetailnet_test.go @@ -80,7 +80,7 @@ func BenchmarkGiantTailnetBusWatcher(b *testing.B) { // The wait mechanism differs by variant: // // - busWatcher=false: block on a channel returned by -// [ipnlocal.LocalBackend.AwaitNodeKeyForTest] (reached via +// [ipnlocal.forTest.AwaitNodeKey] (reached via // [tsnet.TestHooks]). The channel is closed by LocalBackend the moment // the just-added peer's key appears in the netmap, so the wait has zero // polling overhead. @@ -209,7 +209,7 @@ func benchGiantTailnet(b *testing.B, busWatcher bool) { // the just-added peer key has landed in the netmap. // No polling, no notify fan-out cost. select { - case <-lb.AwaitNodeKeyForTest(added.Key): + case <-lb.ForTest().AwaitNodeKey(added.Key): case <-time.After(10 * time.Second): b.Fatalf("timed out waiting for node key %v", added.Key) case <-ctx.Done(): diff --git a/wgengine/netstack/netstack_test.go b/wgengine/netstack/netstack_test.go index 7f248cd44..46f16af31 100644 --- a/wgengine/netstack/netstack_test.go +++ b/wgengine/netstack/netstack_test.go @@ -470,7 +470,7 @@ func TestShouldProcessInbound(t *testing.T) { IPServiceMap := netmap.IPServiceMappings{ serviceIP: "svc:test-service", } - i.lb.SetIPServiceMappingsForTest(IPServiceMap) + i.lb.ForTest().SetIPServiceMappings(IPServiceMap) i.atomicIsVIPServiceIPFunc.Store(func(addr netip.Addr) bool { return addr == serviceIP @@ -517,7 +517,7 @@ func TestShouldProcessInbound(t *testing.T) { IPServiceMap := netmap.IPServiceMappings{ serviceIP: "svc:test-service", } - i.lb.SetIPServiceMappingsForTest(IPServiceMap) + i.lb.ForTest().SetIPServiceMappings(IPServiceMap) i.atomicIsVIPServiceIPFunc.Store(func(addr netip.Addr) bool { return addr == serviceIP @@ -543,7 +543,7 @@ func TestShouldProcessInbound(t *testing.T) { IPServiceMap := netmap.IPServiceMappings{ serviceIPv6: "svc:test-service", } - i.lb.SetIPServiceMappingsForTest(IPServiceMap) + i.lb.ForTest().SetIPServiceMappings(IPServiceMap) i.atomicIsVIPServiceIPFunc.Store(func(addr netip.Addr) bool { return addr == serviceIPv6 @@ -587,7 +587,7 @@ func TestShouldProcessInbound(t *testing.T) { IPServiceMap := netmap.IPServiceMappings{ serviceIPv6: "svc:test-service", } - i.lb.SetIPServiceMappingsForTest(IPServiceMap) + i.lb.ForTest().SetIPServiceMappings(IPServiceMap) i.atomicIsVIPServiceIPFunc.Store(func(addr netip.Addr) bool { return addr == serviceIPv6 @@ -614,7 +614,7 @@ func TestShouldProcessInbound(t *testing.T) { IPServiceMap := netmap.IPServiceMappings{ serviceIP: "svc:test-service", } - i.lb.SetIPServiceMappingsForTest(IPServiceMap) + i.lb.ForTest().SetIPServiceMappings(IPServiceMap) i.atomicIsVIPServiceIPFunc.Store(func(addr netip.Addr) bool { return addr == serviceIP @@ -954,7 +954,7 @@ func TestHandleLocalPackets(t *testing.T) { netip.MustParseAddr("100.99.55.111"): "svc:test-service", netip.MustParseAddr("fd7a:115c:a1e0::abcd"): "svc:test-service", } - impl.lb.SetIPServiceMappingsForTest(IPServiceMap) + impl.lb.ForTest().SetIPServiceMappings(IPServiceMap) t.Run("ShouldHandleServiceIP", func(t *testing.T) { t.Parallel()