diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 168f76268..c8367d14d 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -6145,7 +6145,7 @@ func TestLoginNotifications(t *testing.T) { t.Fatal(err) } - lb.cc.(*mockControl).send(nil, loginURL, false, nil) + lb.cc.(*mockControl).send(sendOpt{url: loginURL}) var wg sync.WaitGroup wg.Add(len(sessions)) @@ -6810,7 +6810,7 @@ func TestSrcCapPacketFilter(t *testing.T) { must.Do(k.UnmarshalText([]byte("nodekey:5c8f86d5fc70d924e55f02446165a5dae8f822994ad26bcf4b08fd841f9bf261"))) controlClient := lb.cc.(*mockControl) - controlClient.send(nil, "", false, &netmap.NetworkMap{ + controlClient.send(sendOpt{nm: &netmap.NetworkMap{ SelfNode: (&tailcfg.Node{ Addresses: []netip.Prefix{netip.MustParsePrefix("1.1.1.1/32")}, }).View(), @@ -6839,7 +6839,7 @@ func TestSrcCapPacketFilter(t *testing.T) { }, }}, }}, - }) + }}) f := lb.GetFilterForTest() res := f.Check(netip.MustParseAddr("2.2.2.2"), netip.MustParseAddr("1.1.1.1"), 22, ipproto.TCP) @@ -7015,10 +7015,10 @@ func TestDisplayMessageIPNBus(t *testing.T) { cc := lb.cc.(*mockControl) // Assert that we are logged in and authorized, and also send our DisplayMessages - cc.send(nil, "", true, &netmap.NetworkMap{ + cc.send(sendOpt{loginFinished: true, nm: &netmap.NetworkMap{ SelfNode: (&tailcfg.Node{MachineAuthorized: true}).View(), DisplayMessages: msgs, - }) + }}) // Tell the health tracker that we are in a map poll because // mockControl doesn't tell it diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index a4b9ba1f4..fca01f105 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -182,9 +182,17 @@ func (cc *mockControl) populateKeys() (newKeys bool) { return newKeys } +type sendOpt struct { + err error + url string + loginFinished bool + nm *netmap.NetworkMap +} + // send publishes a controlclient.Status notification upstream. // (In our tests here, upstream is the ipnlocal.Local instance.) -func (cc *mockControl) send(err error, url string, loginFinished bool, nm *netmap.NetworkMap) { +func (cc *mockControl) send(opts sendOpt) { + err, url, loginFinished, nm := opts.err, opts.url, opts.loginFinished, opts.nm if loginFinished { cc.mu.Lock() cc.authBlocked = false @@ -211,7 +219,7 @@ func (cc *mockControl) authenticated(nm *netmap.NetworkMap) { cc.persist.UserProfile = *selfUser.AsStruct() } cc.persist.NodeID = nm.SelfNode.StableID() - cc.send(nil, "", true, nm) + cc.send(sendOpt{loginFinished: true, nm: nm}) } func (cc *mockControl) sendAuthURL(nm *netmap.NetworkMap) { @@ -480,7 +488,7 @@ func runTestStateMachine(t *testing.T, seamless bool) { }, }) url1 := "https://localhost:1/1" - cc.send(nil, url1, false, nil) + cc.send(sendOpt{url: url1}) { cc.assertCalls() @@ -533,7 +541,7 @@ func runTestStateMachine(t *testing.T, seamless bool) { t.Logf("\n\nLogin2 (url response)") notifies.expect(1) url2 := "https://localhost:1/2" - cc.send(nil, url2, false, nil) + cc.send(sendOpt{url: url2}) { cc.assertCalls() @@ -560,7 +568,7 @@ func runTestStateMachine(t *testing.T, seamless bool) { sys.ControlKnobs().SeamlessKeyRenewal.Store(true) } - cc.send(nil, "", true, &netmap.NetworkMap{}) + cc.send(sendOpt{loginFinished: true, nm: &netmap.NetworkMap{}}) { nn := notifies.drain(3) // Arguably it makes sense to unpause now, since the machine @@ -589,9 +597,9 @@ func runTestStateMachine(t *testing.T, seamless bool) { // but the current code is brittle. // (ie. I suspect it would be better to change false->true in send() // below, and do the same in the real controlclient.) - cc.send(nil, "", false, &netmap.NetworkMap{ + cc.send(sendOpt{nm: &netmap.NetworkMap{ SelfNode: (&tailcfg.Node{MachineAuthorized: true}).View(), - }) + }}) { nn := notifies.drain(1) cc.assertCalls() @@ -752,7 +760,7 @@ func runTestStateMachine(t *testing.T, seamless bool) { // an interactive login URL to visit. notifies.expect(2) url3 := "https://localhost:1/3" - cc.send(nil, url3, false, nil) + cc.send(sendOpt{url: url3}) { nn := notifies.drain(2) cc.assertCalls("Login") @@ -763,9 +771,9 @@ func runTestStateMachine(t *testing.T, seamless bool) { notifies.expect(3) cc.persist.UserProfile.LoginName = "user2" cc.persist.NodeID = "node2" - cc.send(nil, "", true, &netmap.NetworkMap{ + cc.send(sendOpt{loginFinished: true, nm: &netmap.NetworkMap{ SelfNode: (&tailcfg.Node{MachineAuthorized: true}).View(), - }) + }}) t.Logf("\n\nLoginFinished3") { nn := notifies.drain(3) @@ -833,9 +841,9 @@ func runTestStateMachine(t *testing.T, seamless bool) { // the control server at all when stopped). t.Logf("\n\nStart4 -> netmap") notifies.expect(0) - cc.send(nil, "", true, &netmap.NetworkMap{ + cc.send(sendOpt{loginFinished: true, nm: &netmap.NetworkMap{ SelfNode: (&tailcfg.Node{MachineAuthorized: true}).View(), - }) + }}) { notifies.drain(0) cc.assertCalls("pause") @@ -880,7 +888,7 @@ func runTestStateMachine(t *testing.T, seamless bool) { notifies.expect(1) b.StartLoginInteractive(context.Background()) url4 := "https://localhost:1/4" - cc.send(nil, url4, false, nil) + cc.send(sendOpt{url: url4}) { nn := notifies.drain(1) // It might seem like WantRunning should switch to true here, @@ -902,9 +910,9 @@ func runTestStateMachine(t *testing.T, seamless bool) { notifies.expect(3) cc.persist.UserProfile.LoginName = "user3" cc.persist.NodeID = "node3" - cc.send(nil, "", true, &netmap.NetworkMap{ + cc.send(sendOpt{loginFinished: true, nm: &netmap.NetworkMap{ SelfNode: (&tailcfg.Node{MachineAuthorized: true}).View(), - }) + }}) { nn := notifies.drain(3) // BUG: pause() being called here is a bad sign. @@ -950,9 +958,9 @@ func runTestStateMachine(t *testing.T, seamless bool) { // Control server accepts our valid key from before. t.Logf("\n\nLoginFinished5") notifies.expect(0) - cc.send(nil, "", true, &netmap.NetworkMap{ + cc.send(sendOpt{loginFinished: true, nm: &netmap.NetworkMap{ SelfNode: (&tailcfg.Node{MachineAuthorized: true}).View(), - }) + }}) { notifies.drain(0) cc.assertCalls() @@ -965,10 +973,10 @@ func runTestStateMachine(t *testing.T, seamless bool) { } t.Logf("\n\nExpireKey") notifies.expect(1) - cc.send(nil, "", false, &netmap.NetworkMap{ + cc.send(sendOpt{nm: &netmap.NetworkMap{ Expiry: time.Now().Add(-time.Minute), SelfNode: (&tailcfg.Node{MachineAuthorized: true}).View(), - }) + }}) { nn := notifies.drain(1) cc.assertCalls() @@ -980,10 +988,10 @@ func runTestStateMachine(t *testing.T, seamless bool) { t.Logf("\n\nExtendKey") notifies.expect(1) - cc.send(nil, "", false, &netmap.NetworkMap{ + cc.send(sendOpt{nm: &netmap.NetworkMap{ Expiry: time.Now().Add(time.Minute), SelfNode: (&tailcfg.Node{MachineAuthorized: true}).View(), - }) + }}) { nn := notifies.drain(1) cc.assertCalls() @@ -1118,9 +1126,9 @@ func TestWGEngineStatusRace(t *testing.T) { wantState(ipn.NeedsLogin) // Assert that we are logged in and authorized. - cc.send(nil, "", true, &netmap.NetworkMap{ + cc.send(sendOpt{loginFinished: true, nm: &netmap.NetworkMap{ SelfNode: (&tailcfg.Node{MachineAuthorized: true}).View(), - }) + }}) wantState(ipn.Starting) // Simulate multiple concurrent callbacks from wgengine. @@ -1397,9 +1405,9 @@ func TestEngineReconfigOnStateChange(t *testing.T) { mustDo(t)(lb.Start(ipn.Options{})) mustDo2(t)(lb.EditPrefs(connect)) cc().authenticated(node1) - cc().send(nil, "", false, &netmap.NetworkMap{ + cc().send(sendOpt{nm: &netmap.NetworkMap{ Expiry: time.Now().Add(-time.Minute), - }) + }}) }, wantState: ipn.NeedsLogin, wantCfg: &wgcfg.Config{}, @@ -1526,9 +1534,9 @@ func TestEngineReconfigOnStateChange(t *testing.T) { mustDo(t)(lb.Start(ipn.Options{})) mustDo2(t)(lb.EditPrefs(connect)) cc().authenticated(node1) - cc().send(nil, "", false, &netmap.NetworkMap{ + cc().send(sendOpt{nm: &netmap.NetworkMap{ Expiry: time.Now().Add(-time.Minute), - }) + }}) }, // Even with seamless, if the key we are using expires, we want to disconnect: wantState: ipn.NeedsLogin, @@ -1616,9 +1624,9 @@ func runTestStateMachineURLRace(t *testing.T, seamless bool) { nw.watch(0, []wantedNotification{ wantStateNotify(ipn.Starting)}) - cc.send(nil, "", true, &netmap.NetworkMap{ + cc.send(sendOpt{loginFinished: true, nm: &netmap.NetworkMap{ SelfNode: (&tailcfg.Node{MachineAuthorized: true}).View(), - }) + }}) nw.check() t.Logf("Running") @@ -1682,7 +1690,7 @@ func runTestStateMachineURLRace(t *testing.T, seamless bool) { t.Logf("Re-auth (receive URL)") url1 := "https://localhost:1/1" - cc.send(nil, url1, false, nil) + cc.send(sendOpt{url: url1}) // Don't need to wait on anything else - once .send completes, authURL should // be set, and once .send has completed, any opportunities for a WG engine @@ -1718,9 +1726,9 @@ func TestWGEngineDownThenUpRace(t *testing.T) { nw.watch(0, []wantedNotification{ wantStateNotify(ipn.Starting)}) - cc.send(nil, "", true, &netmap.NetworkMap{ + cc.send(sendOpt{loginFinished: true, nm: &netmap.NetworkMap{ SelfNode: (&tailcfg.Node{MachineAuthorized: true}).View(), - }) + }}) nw.check() nw.watch(0, []wantedNotification{ @@ -1762,7 +1770,7 @@ func TestWGEngineDownThenUpRace(t *testing.T) { wg.Go(func() { t.Log("cc.send starting") - cc.send(nil, url1, false, nil) // will block until engine stops + cc.send(sendOpt{url: url1}) // will block until engine stops t.Log("cc.send returned") })