ipn/ipnlocal: fix StatusWithoutPeers not populating parts of Status

Fixes #4311

Change-Id: Iaae0615148fa7154f4ef8f66b455e3a6c2fa9df3
Co-authored-by: Claire Wang <claire@tailscale.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

ipn/ipnlocal/local.go

@@ -545,7 +545,7 @@ func (b *LocalBackend) sanitizedPrefsLocked() ipn.PrefsView {
 // Status returns the latest status of the backend and its
 // sub-components.
 func (b *LocalBackend) Status() *ipnstate.Status {
-	sb := new(ipnstate.StatusBuilder)
+	sb := &ipnstate.StatusBuilder{WantPeers: true}
 	b.UpdateStatus(sb)
 	return sb.Status()
 }
@@ -553,15 +553,19 @@ func (b *LocalBackend) Status() *ipnstate.Status {
 // StatusWithoutPeers is like Status but omits any details
 // of peers.
 func (b *LocalBackend) StatusWithoutPeers() *ipnstate.Status {
-	sb := new(ipnstate.StatusBuilder)
+	sb := &ipnstate.StatusBuilder{WantPeers: false}
-	b.updateStatus(sb, nil)
+	b.UpdateStatus(sb)
 	return sb.Status()
 }
 
 // UpdateStatus implements ipnstate.StatusUpdater.
 func (b *LocalBackend) UpdateStatus(sb *ipnstate.StatusBuilder) {
 	b.e.UpdateStatus(sb)
-	b.updateStatus(sb, b.populatePeerStatusLocked)
+	var extraLocked func(*ipnstate.StatusBuilder)
+	if sb.WantPeers {
+		extraLocked = b.populatePeerStatusLocked
+	}
+	b.updateStatus(sb, extraLocked)
 }
 
 // updateStatus populates sb with status.

ipn/ipnlocal/local_test.go

@@ -14,11 +14,13 @@ import (
 	"time"
 
 	"go4.org/netipx"
+	"tailscale.com/control/controlclient"
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/store/mem"
 	"tailscale.com/net/interfaces"
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/tailcfg"
+	"tailscale.com/tstest"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/netmap"
 	"tailscale.com/wgengine"
@@ -744,6 +746,45 @@ func TestPacketFilterPermitsUnlockedNodes(t *testing.T) {
 
 }
 
+func TestStatusWithoutPeers(t *testing.T) {
+	logf := tstest.WhileTestRunningLogger(t)
+	store := new(testStateStorage)
+	e, err := wgengine.NewFakeUserspaceEngine(logf, 0)
+	if err != nil {
+		t.Fatalf("NewFakeUserspaceEngine: %v", err)
+	}
+	t.Cleanup(e.Close)
+
+	b, err := NewLocalBackend(logf, "logid", store, "", nil, e, 0)
+	if err != nil {
+		t.Fatalf("NewLocalBackend: %v", err)
+	}
+	var cc *mockControl
+	b.SetControlClientGetterForTesting(func(opts controlclient.Options) (controlclient.Client, error) {
+		cc = newClient(t, opts)
+
+		t.Logf("ccGen: new mockControl.")
+		cc.called("New")
+		return cc, nil
+	})
+	b.Start(ipn.Options{})
+	b.Login(nil)
+	cc.send(nil, "", false, &netmap.NetworkMap{
+		MachineStatus: tailcfg.MachineAuthorized,
+		Addresses:     ipps("100.101.101.101"),
+		SelfNode: &tailcfg.Node{
+			Addresses: ipps("100.101.101.101"),
+		},
+	})
+	got := b.StatusWithoutPeers()
+	if got.TailscaleIPs == nil {
+		t.Errorf("got nil, expected TailscaleIPs value to not be nil")
+	}
+	if !reflect.DeepEqual(got.TailscaleIPs, got.Self.TailscaleIPs) {
+		t.Errorf("got %v, expected %v", got.TailscaleIPs, got.Self.TailscaleIPs)
+	}
+}
+
 // legacyBackend was the interface between Tailscale frontends
 // (e.g. cmd/tailscale, iOS/MacOS/Windows GUIs) and the tailscale
 // backend (e.g. cmd/tailscaled) running on the same machine.

ipn/ipnstate/ipnstate.go

@@ -245,6 +245,8 @@ type PeerStatus struct {
 }
 
 type StatusBuilder struct {
+	WantPeers bool // whether caller wants peers
+
 	mu     sync.Mutex
 	locked bool
 	st     Status

wgengine/magicsock/magicsock.go

@@ -3545,12 +3545,14 @@ func (c *Conn) UpdateStatus(sb *ipnstate.StatusBuilder) {
 		ss.TailscaleIPs = tailscaleIPs
 	})
 
-	c.peerMap.forEachEndpoint(func(ep *endpoint) {
+	if sb.WantPeers {
-		ps := &ipnstate.PeerStatus{InMagicSock: true}
+		c.peerMap.forEachEndpoint(func(ep *endpoint) {
-		//ps.Addrs = append(ps.Addrs, n.Endpoints...)
+			ps := &ipnstate.PeerStatus{InMagicSock: true}
-		ep.populatePeerStatus(ps)
+			//ps.Addrs = append(ps.Addrs, n.Endpoints...)
-		sb.AddPeer(ep.publicKey, ps)
+			ep.populatePeerStatus(ps)
-	})
+			sb.AddPeer(ep.publicKey, ps)
+		})
+	}
 
 	c.foreachActiveDerpSortedLocked(func(node int, ad activeDerp) {
 		// TODO(bradfitz): add to ipnstate.StatusBuilder

wgengine/magicsock/magicsock_test.go

@@ -219,6 +219,7 @@ func (s *magicStack) Public() key.NodePublic {
 
 func (s *magicStack) Status() *ipnstate.Status {
 	var sb ipnstate.StatusBuilder
+	sb.WantPeers = true
 	s.conn.UpdateStatus(&sb)
 	return sb.Status()
 }

wgengine/userspace.go

@@ -1265,13 +1265,15 @@ func (e *userspaceEngine) UpdateStatus(sb *ipnstate.StatusBuilder) {
 		e.logf("wgengine: getStatus: %v", err)
 		return
 	}
-	for _, ps := range st.Peers {
+	if sb.WantPeers {
-		sb.AddPeer(ps.NodeKey, &ipnstate.PeerStatus{
+		for _, ps := range st.Peers {
-			RxBytes:       int64(ps.RxBytes),
+			sb.AddPeer(ps.NodeKey, &ipnstate.PeerStatus{
-			TxBytes:       int64(ps.TxBytes),
+				RxBytes:       int64(ps.RxBytes),
-			LastHandshake: ps.LastHandshake,
+				TxBytes:       int64(ps.TxBytes),
-			InEngine:      true,
+				LastHandshake: ps.LastHandshake,
-		})
+				InEngine:      true,
+			})
+		}
 	}
 
 	e.magicConn.UpdateStatus(sb)