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()