From 9706c9f4ffb8637670e3d2e152607c23be621a41 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 24 Jan 2025 19:41:30 -0800 Subject: [PATCH] types/netmap,*: pass around UserProfiles as views (pointers) instead Smaller. Updates tailscale/corp#26058 (@andrew-d noticed during this) Change-Id: Id33cddd171aaf8f042073b6d3c183b0a746e9931 Signed-off-by: Brad Fitzpatrick --- control/controlclient/direct.go | 4 +++- control/controlclient/map.go | 8 ++++---- ipn/ipnlocal/local.go | 25 +++++++++++++++++++------ ipn/ipnlocal/local_test.go | 16 ++++++++-------- ipn/ipnlocal/serve_test.go | 12 ++++++------ ipn/ipnstate/ipnstate.go | 4 ++-- types/netmap/netmap.go | 11 +++++++++-- 7 files changed, 51 insertions(+), 29 deletions(-) diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index f327ecc2a..883a1a587 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -1003,7 +1003,9 @@ func (c *Direct) sendMapRequest(ctx context.Context, isStreaming bool, nu Netmap if persist == c.persist { newPersist := persist.AsStruct() newPersist.NodeID = nm.SelfNode.StableID() - newPersist.UserProfile = nm.UserProfiles[nm.User()] + if up, ok := nm.UserProfiles[nm.User()]; ok { + newPersist.UserProfile = *up.AsStruct() + } c.persist = newPersist.View() persist = c.persist diff --git a/control/controlclient/map.go b/control/controlclient/map.go index f0a11bdf1..d4283e490 100644 --- a/control/controlclient/map.go +++ b/control/controlclient/map.go @@ -77,7 +77,7 @@ type mapSession struct { peers map[tailcfg.NodeID]tailcfg.NodeView lastDNSConfig *tailcfg.DNSConfig lastDERPMap *tailcfg.DERPMap - lastUserProfile map[tailcfg.UserID]tailcfg.UserProfile + lastUserProfile map[tailcfg.UserID]tailcfg.UserProfileView lastPacketFilterRules views.Slice[tailcfg.FilterRule] // concatenation of all namedPacketFilters namedPacketFilters map[string]views.Slice[tailcfg.FilterRule] lastParsedPacketFilter []filter.Match @@ -104,7 +104,7 @@ func newMapSession(privateNodeKey key.NodePrivate, nu NetmapUpdater, controlKnob privateNodeKey: privateNodeKey, publicNodeKey: privateNodeKey.Public(), lastDNSConfig: new(tailcfg.DNSConfig), - lastUserProfile: map[tailcfg.UserID]tailcfg.UserProfile{}, + lastUserProfile: map[tailcfg.UserID]tailcfg.UserProfileView{}, // Non-nil no-op defaults, to be optionally overridden by the caller. logf: logger.Discard, @@ -294,7 +294,7 @@ func (ms *mapSession) updateStateFromResponse(resp *tailcfg.MapResponse) { } for _, up := range resp.UserProfiles { - ms.lastUserProfile[up.ID] = up + ms.lastUserProfile[up.ID] = up.View() } if dm := resp.DERPMap; dm != nil { @@ -837,7 +837,7 @@ func (ms *mapSession) netmap() *netmap.NetworkMap { PrivateKey: ms.privateNodeKey, MachineKey: ms.machinePubKey, Peers: peerViews, - UserProfiles: make(map[tailcfg.UserID]tailcfg.UserProfile), + UserProfiles: make(map[tailcfg.UserID]tailcfg.UserProfileView), Domain: ms.lastDomain, DomainAuditLogID: ms.lastDomainAuditLogID, DNS: *ms.lastDNSConfig, diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 811b978f7..c24bcbb7b 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1305,6 +1305,18 @@ func peerStatusFromNode(ps *ipnstate.PeerStatus, n tailcfg.NodeView) { } } +func profileFromView(v tailcfg.UserProfileView) tailcfg.UserProfile { + if v.Valid() { + return tailcfg.UserProfile{ + ID: v.ID(), + LoginName: v.LoginName(), + DisplayName: v.DisplayName(), + ProfilePicURL: v.ProfilePicURL(), + } + } + return tailcfg.UserProfile{} +} + // WhoIsNodeKey returns the peer info of given public key, if it exists. func (b *LocalBackend) WhoIsNodeKey(k key.NodePublic) (n tailcfg.NodeView, u tailcfg.UserProfile, ok bool) { b.mu.Lock() @@ -1314,11 +1326,12 @@ func (b *LocalBackend) WhoIsNodeKey(k key.NodePublic) (n tailcfg.NodeView, u tai return n, u, false } if self := b.netMap.SelfNode; self.Valid() && self.Key() == k { - return self, b.netMap.UserProfiles[self.User()], true + return self, profileFromView(b.netMap.UserProfiles[self.User()]), true } for _, n := range b.peers { if n.Key() == k { - u, ok = b.netMap.UserProfiles[n.User()] + up, ok := b.netMap.UserProfiles[n.User()] + u = profileFromView(up) return n, u, ok } } @@ -1388,11 +1401,11 @@ func (b *LocalBackend) WhoIs(proto string, ipp netip.AddrPort) (n tailcfg.NodeVi } n = b.netMap.SelfNode } - u, ok = b.netMap.UserProfiles[n.User()] + up, ok := b.netMap.UserProfiles[n.User()] if !ok { return failf("no userprofile for node %v", n.Key()) } - return n, u, true + return n, profileFromView(up), true } // PeerCaps returns the capabilities that remote src IP has to @@ -4193,7 +4206,7 @@ func (b *LocalBackend) setPrefsLockedOnEntry(newp *ipn.Prefs, unlock unlockOnce) } } if netMap != nil { - newProfile := netMap.UserProfiles[netMap.User()] + newProfile := profileFromView(netMap.UserProfiles[netMap.User()]) if newLoginName := newProfile.LoginName; newLoginName != "" { if !oldp.Persist().Valid() { b.logf("active login: %s", newLoginName) @@ -5803,7 +5816,7 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) { } var login string if nm != nil { - login = cmp.Or(nm.UserProfiles[nm.User()].LoginName, "") + login = cmp.Or(profileFromView(nm.UserProfiles[nm.User()]).LoginName, "") } b.netMap = nm b.updatePeersFromNetmapLocked(nm) diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index dfc2e45bd..f0c712777 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -1052,13 +1052,13 @@ func TestWhoIs(t *testing.T) { Addresses: []netip.Prefix{netip.MustParsePrefix("100.200.200.200/32")}, }).View(), }, - UserProfiles: map[tailcfg.UserID]tailcfg.UserProfile{ - 10: { + UserProfiles: map[tailcfg.UserID]tailcfg.UserProfileView{ + 10: (&tailcfg.UserProfile{ DisplayName: "Myself", - }, - 20: { + }).View(), + 20: (&tailcfg.UserProfile{ DisplayName: "Peer", - }, + }).View(), }, }) tests := []struct { @@ -2754,12 +2754,12 @@ func TestTCPHandlerForDstWithVIPService(t *testing.T) { tailcfg.NodeAttrServiceHost: []tailcfg.RawMessage{tailcfg.RawMessage(svcIPMapJSON)}, }, }).View(), - UserProfiles: map[tailcfg.UserID]tailcfg.UserProfile{ - tailcfg.UserID(1): { + UserProfiles: map[tailcfg.UserID]tailcfg.UserProfileView{ + tailcfg.UserID(1): (&tailcfg.UserProfile{ LoginName: "someone@example.com", DisplayName: "Some One", ProfilePicURL: "https://example.com/photo.jpg", - }, + }).View(), }, }, ) diff --git a/ipn/ipnlocal/serve_test.go b/ipn/ipnlocal/serve_test.go index 7f457e560..3c028c65e 100644 --- a/ipn/ipnlocal/serve_test.go +++ b/ipn/ipnlocal/serve_test.go @@ -327,12 +327,12 @@ func TestServeConfigServices(t *testing.T) { tailcfg.NodeAttrServiceHost: []tailcfg.RawMessage{tailcfg.RawMessage(svcIPMapJSON)}, }, }).View(), - UserProfiles: map[tailcfg.UserID]tailcfg.UserProfile{ - tailcfg.UserID(1): { + UserProfiles: map[tailcfg.UserID]tailcfg.UserProfileView{ + tailcfg.UserID(1): (&tailcfg.UserProfile{ LoginName: "someone@example.com", DisplayName: "Some One", ProfilePicURL: "https://example.com/photo.jpg", - }, + }).View(), }, } @@ -905,12 +905,12 @@ func newTestBackend(t *testing.T) *LocalBackend { SelfNode: (&tailcfg.Node{ Name: "example.ts.net", }).View(), - UserProfiles: map[tailcfg.UserID]tailcfg.UserProfile{ - tailcfg.UserID(1): { + UserProfiles: map[tailcfg.UserID]tailcfg.UserProfileView{ + tailcfg.UserID(1): (&tailcfg.UserProfile{ LoginName: "someone@example.com", DisplayName: "Some One", ProfilePicURL: "https://example.com/photo.jpg", - }, + }).View(), }, } b.peers = map[tailcfg.NodeID]tailcfg.NodeView{ diff --git a/ipn/ipnstate/ipnstate.go b/ipn/ipnstate/ipnstate.go index 37ab47714..5ab9b5bdf 100644 --- a/ipn/ipnstate/ipnstate.go +++ b/ipn/ipnstate/ipnstate.go @@ -367,7 +367,7 @@ func (sb *StatusBuilder) MutateSelfStatus(f func(*PeerStatus)) { } // AddUser adds a user profile to the status. -func (sb *StatusBuilder) AddUser(id tailcfg.UserID, up tailcfg.UserProfile) { +func (sb *StatusBuilder) AddUser(id tailcfg.UserID, up tailcfg.UserProfileView) { if sb.locked { log.Printf("[unexpected] ipnstate: AddUser after Locked") return @@ -377,7 +377,7 @@ func (sb *StatusBuilder) AddUser(id tailcfg.UserID, up tailcfg.UserProfile) { sb.st.User = make(map[tailcfg.UserID]tailcfg.UserProfile) } - sb.st.User[id] = up + sb.st.User[id] = *up.AsStruct() } // AddIP adds a Tailscale IP address to the status. diff --git a/types/netmap/netmap.go b/types/netmap/netmap.go index 051b0f0dc..94db7a477 100644 --- a/types/netmap/netmap.go +++ b/types/netmap/netmap.go @@ -76,7 +76,9 @@ type NetworkMap struct { // If this is empty, then data-plane audit logging is disabled. DomainAuditLogID string - UserProfiles map[tailcfg.UserID]tailcfg.UserProfile + // UserProfiles contains the profile information of UserIDs referenced + // in SelfNode and Peers. + UserProfiles map[tailcfg.UserID]tailcfg.UserProfileView // MaxKeyDuration describes the MaxKeyDuration setting for the tailnet. MaxKeyDuration time.Duration @@ -289,7 +291,12 @@ func (nm *NetworkMap) PeerWithStableID(pid tailcfg.StableNodeID) (_ tailcfg.Node func (nm *NetworkMap) printConciseHeader(buf *strings.Builder) { fmt.Fprintf(buf, "netmap: self: %v auth=%v", nm.NodeKey.ShortString(), nm.GetMachineStatus()) - login := nm.UserProfiles[nm.User()].LoginName + + var login string + up, ok := nm.UserProfiles[nm.User()] + if ok { + login = up.LoginName() + } if login == "" { if nm.User().IsZero() { login = "?"