diff --git a/client/local/local.go b/client/local/local.go index 5c75c0487..faeee1496 100644 --- a/client/local/local.go +++ b/client/local/local.go @@ -1065,11 +1065,13 @@ func (lc *Client) DNSConfig(ctx context.Context) (*tailcfg.DNSConfig, error) { } // PeerByID returns a peer's current full [tailcfg.Node] looked up by its -// [tailcfg.NodeID], in O(1) time on the daemon side. It returns an error -// if no peer with that NodeID is in the current netmap. +// [tailcfg.NodeID]. It returns an error if no peer with that NodeID is in the +// current netmap. // -// It is intended for callers that need the latest state of a single peer -// without fetching the entire netmap. +// It is intended for callers that observed a peer-mutation signal (e.g. +// [ipn.Notify.PeerChangedPatch] or [ipn.Notify.PeersChanged]) and want the +// latest state of the affected node without having to apply the patch +// themselves. func (lc *Client) PeerByID(ctx context.Context, id tailcfg.NodeID) (*tailcfg.Node, error) { body, err := lc.get200(ctx, "/localapi/v0/peer-by-id?id="+strconv.FormatInt(int64(id), 10)) if err != nil { @@ -1078,6 +1080,22 @@ func (lc *Client) PeerByID(ctx context.Context, id tailcfg.NodeID) (*tailcfg.Nod return decodeJSON[*tailcfg.Node](body) } +// UserProfile returns the current [tailcfg.UserProfile] for the given +// [tailcfg.UserID]. It returns an error if no user with that UserID is in the +// current netmap. +// +// It is the LocalAPI fallback for IPN-bus consumers that see a UserID +// referenced by a peer Node and want to resolve it to a UserProfile. Sessions +// opted in to [ipn.NotifyPeerChanges] / [ipn.NotifyPeerPatches] also receive +// UserProfiles automatically via [ipn.Notify.UserProfiles]. +func (lc *Client) UserProfile(ctx context.Context, id tailcfg.UserID) (*tailcfg.UserProfile, error) { + body, err := lc.get200(ctx, "/localapi/v0/user-profile?id="+strconv.FormatInt(int64(id), 10)) + if err != nil { + return nil, err + } + return decodeJSON[*tailcfg.UserProfile](body) +} + // PingOpts contains options for the ping request. // // The zero value is valid, which means to use defaults. diff --git a/cmd/tailscale/cli/debug.go b/cmd/tailscale/cli/debug.go index 3531172bb..f6972f004 100644 --- a/cmd/tailscale/cli/debug.go +++ b/cmd/tailscale/cli/debug.go @@ -268,8 +268,7 @@ func debugCmd() *ffcli.Command { ShortHelp: "Subscribe to IPN message bus", FlagSet: (func() *flag.FlagSet { fs := newFlagSet("watch-ipn") - fs.BoolVar(&watchIPNArgs.netmap, "netmap", true, "include netmap in messages") - fs.BoolVar(&watchIPNArgs.initial, "initial", false, "include initial status") + fs.BoolVar(&watchIPNArgs.initial, "initial", false, "include the initial backend State and Prefs in the first message") fs.BoolVar(&watchIPNArgs.rateLimit, "rate-limit", true, "rate limit messages") fs.IntVar(&watchIPNArgs.count, "count", 0, "exit after printing this many statuses, or 0 to keep going forever") return fs @@ -632,16 +631,15 @@ func runPrefs(ctx context.Context, args []string) error { } var watchIPNArgs struct { - netmap bool initial bool rateLimit bool count int } func runWatchIPN(ctx context.Context, args []string) error { - var mask ipn.NotifyWatchOpt + mask := ipn.NotifyPeerChanges | ipn.NotifyPeerPatches if watchIPNArgs.initial { - mask = ipn.NotifyInitialState | ipn.NotifyInitialPrefs | ipn.NotifyInitialNetMap + mask |= ipn.NotifyInitialState | ipn.NotifyInitialPrefs } if watchIPNArgs.rateLimit { mask |= ipn.NotifyRateLimit @@ -657,9 +655,6 @@ func runWatchIPN(ctx context.Context, args []string) error { if err != nil { return err } - if !watchIPNArgs.netmap { - n.NetMap = nil - } j, _ := json.MarshalIndent(n, "", "\t") fmt.Printf("%s\n", j) } diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 032999cb9..859df747d 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -56,6 +56,7 @@ import ( "tailscale.com/types/netmap" "tailscale.com/types/persist" "tailscale.com/types/tkatype" + "tailscale.com/types/views" "tailscale.com/util/clientmetric" "tailscale.com/util/eventbus" "tailscale.com/util/singleflight" @@ -64,6 +65,7 @@ import ( "tailscale.com/util/testenv" "tailscale.com/util/vizerror" "tailscale.com/util/zstdframe" + "tailscale.com/wgengine/filter" ) // Direct is the client that connects to a tailcontrol server for a node. @@ -226,6 +228,9 @@ type NetmapUpdater interface { // rather than just full updates. type NetmapDeltaUpdater interface { // UpdateNetmapDelta is called with discrete changes to the network map. + // The mutation slice may contain [netmap.NodeMutationAdd] and + // [netmap.NodeMutationRemove] entries when peers were added or removed, + // alongside per-field patches. // // The ok result is whether the implementation was able to apply the // mutations. It might return false if its internal state doesn't @@ -234,6 +239,40 @@ type NetmapDeltaUpdater interface { UpdateNetmapDelta([]netmap.NodeMutation) (ok bool) } +// PacketFilterUpdater is an optional interface that can be implemented by +// NetmapUpdater implementations to receive incremental packet-filter updates +// without a full netmap rebuild. +// +// It exists because the packet filter currently changes on every peer +// addition, so a MapResponse carrying PeersChanged almost always also carries +// PacketFilter (or PacketFilters). Handling the filter narrowly keeps peer +// churn O(1) on the controlclient side. +type PacketFilterUpdater interface { + // UpdatePacketFilter is called when a MapResponse's PacketFilter (or + // PacketFilters) changed. rules is the already-merged concatenation of + // the session's named packet filter chunks; parsed is the parsed form. + UpdatePacketFilter(rules views.Slice[tailcfg.FilterRule], parsed []filter.Match) +} + +// UserProfileUpdater is an optional interface that can be implemented by +// NetmapUpdater implementations to receive incremental UserProfile updates +// without a full netmap rebuild. +// +// It exists so consumers of [ipn.Notify.UserProfiles] can be told about +// new or updated UserProfiles before (or with) the [ipn.Notify.PeersChanged] +// or [ipn.Notify.PeerChangedPatch] entry that references the corresponding +// UserID. +type UserProfileUpdater interface { + // UpdateUserProfiles is called when a MapResponse carries UserProfiles + // entries. profiles is the new/updated subset (NOT the full map); + // implementations should merge with whatever they already know. + // + // The values are [tailcfg.UserProfileView]s sharing backing memory + // with the caller's tracking map; implementations may store them + // directly without copying. + UpdateUserProfiles(profiles map[tailcfg.UserID]tailcfg.UserProfileView) +} + // patchDiscoKeyer is an optional interface that can be implemented by an [Observer] to be // notified about node disco keys received out-of-band from control, via // existing connection state. diff --git a/control/controlclient/map.go b/control/controlclient/map.go index 34b5ecc55..1d50c655d 100644 --- a/control/controlclient/map.go +++ b/control/controlclient/map.go @@ -393,11 +393,45 @@ func (ms *mapSession) tryHandleIncrementally(res *tailcfg.MapResponse) bool { if !ok { return false } + // If the response carries a new packet filter, the updater must + // support pushing it narrowly; otherwise fall back to a full netmap + // rebuild. PacketFilter/PacketFilters are no longer in + // mapResponseContainsNonPatchFields, so MutationsFromMapResponse will + // happily return mutations alongside a filter change — we need to + // deliver the filter separately before those mutations land. + if res.PacketFilter != nil || res.PacketFilters != nil { + pfu, ok := ms.netmapUpdater.(PacketFilterUpdater) + if !ok { + return false + } + pfu.UpdatePacketFilter(ms.lastPacketFilterRules, ms.lastParsedPacketFilter) + } + // Same shape for UserProfiles: deliver any new/updated profiles before + // the peer mutations that may reference them, so bus consumers never + // see a UserID for which a profile hasn't been published. The values + // are read from ms.lastUserProfile (just populated by + // updateStateFromResponse) so views are shared with mapSession's + // store; downstream consumers can use [UserProfileView.Equal] for + // dedup without copying. + if len(res.UserProfiles) > 0 { + upu, ok := ms.netmapUpdater.(UserProfileUpdater) + if !ok { + return false + } + profiles := make(map[tailcfg.UserID]tailcfg.UserProfileView, len(res.UserProfiles)) + for _, up := range res.UserProfiles { + profiles[up.ID] = ms.lastUserProfile[up.ID] + } + upu.UpdateUserProfiles(profiles) + } mutations, ok := netmap.MutationsFromMapResponse(res, time.Now()) - if ok && len(mutations) > 0 { + if !ok { + return false + } + if len(mutations) > 0 { return nud.UpdateNetmapDelta(mutations) } - return ok + return true } // updateStats are some stats from updateStateFromResponse, primarily for diff --git a/ipn/backend.go b/ipn/backend.go index 51617e08e..8c1fb57bf 100644 --- a/ipn/backend.go +++ b/ipn/backend.go @@ -88,10 +88,60 @@ const ( NotifyInitialClientVersion NotifyWatchOpt = 1 << 11 // if set, the first Notify message (sent immediately) will contain the current ClientVersion if available and if update checks are enabled - // NotifyPeerChanges, if set, causes netmap delta updates to be sent as [tailcfg.PeerChange] rather than a full NetMap. - // Full netmap responses from the control plane are still sent as a full NetMap. PeerChanges are only sent to sessions - // that have opted in to this mode. + // NotifyPeerChanges, if set, opts the watcher into peer-set delta + // notifications: [Notify.PeersChanged] (peer added or full-Node + // replaced) and [Notify.PeersRemoved] (peer removed by NodeID). + // + // Without this bit, peer adds/removes/replacements are not delivered + // over the bus at all (consumers fall back to fetching the netmap on + // demand or, on legacy-emit platforms, to watching [Notify.NetMap]). + // + // Watchers that want narrower per-field updates as well (Online, + // LastSeen, DERPHome, Endpoints) should additionally set + // [NotifyPeerPatches]. Without [NotifyPeerPatches], any per-field + // patch tailscaled would have emitted as a [tailcfg.PeerChange] is + // promoted into a full-Node entry in [Notify.PeersChanged] for this + // watcher, so a watcher that opts only into [NotifyPeerChanges] still + // observes every per-peer mutation; it just receives them as full + // Nodes rather than narrow patches. The cost is bus bandwidth. + // + // It is mutually exclusive with [NotifyInitialNetMap]: callers that + // want a continuous stitch of initial state plus peer changes should + // pair this with [NotifyInitialStatus] instead. NotifyPeerChanges NotifyWatchOpt = 1 << 12 + + // NotifyNoNetMap, if set, suppresses the legacy [Notify.NetMap] field on + // runtime (non-initial) Notify messages delivered to this watcher. It + // only matters on platforms where tailscaled still emits NetMap on the + // bus by default — Windows, macOS, and iOS — and is intended for GUI + // clients on those platforms that have migrated to read peers via + // [Notify.PeersChanged] / [LocalClient.NetMap]. The initial-state NetMap + // (sent when [NotifyInitialNetMap] is set) is unaffected. + NotifyNoNetMap NotifyWatchOpt = 1 << 13 + + // NotifyInitialStatus, if set, causes the first Notify message (sent + // immediately) to contain the current [ipnstate.Status] in + // [Notify.InitialStatus]. Together with [Notify.SelfChange] and + // [Notify.PeersChanged] on subsequent messages, it lets a watcher + // stitch together a continuous view of the local node's state without + // fetching the netmap directly. Prefer this over [LocalClient.NetMap] + // for new code that wants a stable, client-facing snapshot type. + NotifyInitialStatus NotifyWatchOpt = 1 << 14 + + // NotifyPeerPatches, if set, opts the watcher into narrow per-field + // peer patches via [Notify.PeerChangedPatch]. It implies + // [NotifyPeerChanges]: a watcher with [NotifyPeerPatches] also + // receives [Notify.PeersChanged] and [Notify.PeersRemoved]. + // + // This is the lower-bandwidth mode: changes to fields that fit in a + // [tailcfg.PeerChange] (currently Online, LastSeen, DERPHome, + // Endpoints) ride as patches; only changes that don't fit ride as + // full Nodes in [Notify.PeersChanged]. + // + // Without this bit but with [NotifyPeerChanges], the producer + // promotes any patch into a full-Node entry in [Notify.PeersChanged] + // for this session, at the cost of bandwidth. + NotifyPeerPatches NotifyWatchOpt = 1 << 15 ) // Notify is a communication from a backend (e.g. tailscaled) to a frontend @@ -113,10 +163,9 @@ type Notify struct { // For State InUseOtherUser, ErrMessage is not critical and just contains the details. ErrMessage *string - LoginFinished *empty.Message // non-nil when/if the login process succeeded - State *State // if non-nil, the new or current IPN state - Prefs *PrefsView // if non-nil && Valid, the new or current preferences - NetMap *netmap.NetworkMap // if non-nil, the new or current netmap + LoginFinished *empty.Message // non-nil when/if the login process succeeded + State *State // if non-nil, the new or current IPN state + Prefs *PrefsView // if non-nil && Valid, the new or current preferences // SelfChange, if non-nil, indicates that this node's own [tailcfg.Node] // has changed: addresses, name, key expiry, capabilities, etc. It carries @@ -125,15 +174,98 @@ type Notify struct { // full netmap. // // Consumers that need additional state (peers, DNS config, packet - // filter) should react to SelfChange by fetching the relevant bits on - // demand via [LocalClient]. + // filter) should react to SelfChange by fetching the full netmap on + // demand via [LocalClient.NetMap]. SelfChange *tailcfg.Node `json:",omitzero"` - // PeerChanges, if non-nil, is a list of [tailcfg.PeerChange] that have occurred since the last - // full netmap update. This is sent in lieu of a full NetMap when [NotifyPeerChanges] is set in - // the session's mask and a netmap update is derived from an incremental MapResponse. - // Full MapResponse updates from the control plane are sent as a full NetMap. - PeerChanges []*tailcfg.PeerChange `json:",omitzero"` + // InitialStatus, if non-nil, is the current [ipnstate.Status]. It is + // only set in the first Notify of a session when the watcher requested + // [NotifyInitialStatus]. Together with subsequent [Notify.SelfChange] + // and [Notify.PeerChanges] messages, it lets a watcher stitch together + // a continuous view of node state without fetching the netmap. + InitialStatus *ipnstate.Status `json:",omitzero"` + + // NetMap, if non-nil, is the full network map. New consumers should prefer + // [LocalClient.NetMap] for one-shot fetches and [Notify.SelfChange] / + // [Notify.PeerChanges] for incremental reactive updates; NetMap on the bus + // is the legacy path retained for hosts whose GUIs have not yet finished + // migrating. It is delivered: + // + // - On the initial Notify if the watcher requested + // [NotifyInitialNetMap] (any platform). + // - On subsequent Notify messages, only when tailscaled is running + // on Windows, macOS, or iOS. On Linux and other platforms it is + // always nil after the initial notify. + // + // Deprecated: this field is only populated on Windows, macOS, and iOS and + // is slated for removal in favor of [Notify.InitialStatus] + + // [Notify.SelfChange] / [Notify.PeerChanges], etc, as this field + // doesn't scale. + NetMap *netmap.NetworkMap + + // PeerChangedPatch, if non-empty, lists narrow per-field peer patches + // since the last Notify (currently Online, LastSeen, DERPHome, + // Endpoints). It mirrors [tailcfg.MapResponse.PeersChangedPatch]. + // + // Peer additions and any peer change that can't be expressed as a + // [tailcfg.PeerChange] travel in [Notify.PeersChanged]; peer removals + // in [Notify.PeersRemoved]. + // + // Watchers must opt in to receive this field by setting + // [NotifyPeerPatches]; without that bit (but with [NotifyPeerChanges]) + // the producer promotes each patch into a full-Node entry in + // [Notify.PeersChanged] instead. + // + // The [tailcfg.PeerChange] type may grow more fields over time; + // consumers that see a [tailcfg.PeerChange] with a field they don't + // recognize should re-fetch the affected node by NodeID via + // [LocalClient.PeerByID] (an O(1) lookup) to learn its current value + // rather than ignoring the change. + PeerChangedPatch []*tailcfg.PeerChange `json:",omitzero"` + + // PeersChanged, if non-empty, lists peers whose full [tailcfg.Node] + // has been added or replaced since the last Notify. A node ID may + // appear here either because it is a brand-new peer or because the + // control plane sent a fresh full Node for an existing peer when the + // change wasn't expressible as a [tailcfg.PeerChange] patch (e.g. a + // CapMap, Addresses, Hostinfo, or Tags change). Consumers should + // upsert by NodeID. + // + // This mirrors [tailcfg.MapResponse.PeersChanged] semantics; peer + // removals travel in [Notify.PeersRemoved] and narrow per-field + // patches in [Notify.PeerChanges]. + PeersChanged []*tailcfg.Node `json:",omitzero"` + + // PeersRemoved, if non-empty, lists [tailcfg.NodeID]s that have been + // removed from the netmap since the last Notify. See + // [Notify.PeersChanged]. This mirrors + // [tailcfg.MapResponse.PeersRemoved]. + PeersRemoved []tailcfg.NodeID `json:",omitzero"` + + // UserProfiles, if non-empty, carries [tailcfg.UserProfileView] + // entries that have been added or updated since the last Notify on + // this session. Watchers must opt in via [NotifyPeerChanges] or + // [NotifyPeerPatches]; this field is gated on the same bits as + // [Notify.PeersChanged] / [Notify.PeerChangedPatch] because its + // only purpose is to let those consumers resolve the [tailcfg.UserID] + // referenced by a peer Node. + // + // The producer guarantees that any UserID referenced by a peer in + // a [Notify.PeersChanged] / [Notify.PeerChangedPatch] entry will + // have its profile delivered either earlier on this same session + // (e.g. via the initial NetMap or via an earlier Notify carrying + // UserProfiles) or in this same Notify. A consumer that sees a + // UserID it doesn't recognize on a session that opted in to + // peer-change notifications can treat it as a bug; the + // [LocalClient.UserProfile] LocalAPI fallback exists for sessions + // that didn't subscribe with the peer-change bits or that need to + // look up a UserID for any other reason. + // + // The values are [tailcfg.UserProfileView] so they share backing + // memory with the producer's tracking maps; consumers should treat + // them as read-only and use [tailcfg.UserProfileView.AsStruct] or + // the per-field accessors to read them. + UserProfiles map[tailcfg.UserID]tailcfg.UserProfileView `json:",omitzero"` Engine *EngineStatus // if non-nil, the new or current wireguard stats BrowseToURL *string // if non-nil, UI should open a browser right now @@ -204,14 +336,11 @@ func (n Notify) String() string { if n.Prefs != nil && n.Prefs.Valid() { fmt.Fprintf(&sb, "%v ", n.Prefs.Pretty()) } - if n.NetMap != nil { - sb.WriteString("NetMap{...} ") - } if n.SelfChange != nil { fmt.Fprintf(&sb, "SelfChange(%v) ", n.SelfChange.StableID) } - if n.PeerChanges != nil { - fmt.Fprintf(&sb, "PeerChanges(%d) ", len(n.PeerChanges)) + if n.PeerChangedPatch != nil { + fmt.Fprintf(&sb, "PeerChangedPatch(%d) ", len(n.PeerChangedPatch)) } if n.Engine != nil { fmt.Fprintf(&sb, "wg=%v ", *n.Engine) diff --git a/ipn/ipnlocal/bus.go b/ipn/ipnlocal/bus.go index 8be508010..cad7f0576 100644 --- a/ipn/ipnlocal/bus.go +++ b/ipn/ipnlocal/bus.go @@ -5,13 +5,29 @@ package ipnlocal import ( "context" + "runtime" "time" "tailscale.com/ipn" "tailscale.com/tailcfg" "tailscale.com/tstime" + "tailscale.com/util/mak" ) +// goosGetsLegacyNetmapNotify reports whether tailscaled, when running on the +// current GOOS, still emits the legacy [ipn.Notify.NetMap] field on runtime +// (non-initial) bus messages. It is true on platforms whose host GUIs have +// not yet finished migrating to the narrower bus signals +// ([ipn.Notify.SelfChange] / [ipn.Notify.PeerChanges]) and the on-demand +// [LocalClient.NetMap] fetch. +// +// runtime.GOOS is a compile-time constant, so the producer-side code that +// builds and ships NetMap on the bus is dead-code-eliminated on Linux and +// other geese where this is false. +const goosGetsLegacyNetmapNotify = runtime.GOOS == "windows" || + runtime.GOOS == "darwin" || + runtime.GOOS == "ios" + type rateLimitingBusSender struct { fn func(*ipn.Notify) (keepGoing bool) lastFlush time.Time // last call to fn, or zero value if none @@ -126,11 +142,21 @@ func mergeBoringNotifies(dst, src *ipn.Notify) *ipn.Notify { if dst == nil { dst = &ipn.Notify{Version: src.Version} } - if src.NetMap != nil { + if goosGetsLegacyNetmapNotify && src.NetMap != nil { + // Full netmap supersedes any accumulated peer-change deltas. dst.NetMap = src.NetMap - dst.PeerChanges = nil // full netmap supersedes any accumulated deltas - } else if src.PeerChanges != nil { - dst.PeerChanges = mergePeerChanges(dst.PeerChanges, src.PeerChanges) + dst.PeerChangedPatch = nil + } else if src.PeerChangedPatch != nil { + dst.PeerChangedPatch = mergePeerChangedPatch(dst.PeerChangedPatch, src.PeerChangedPatch) + } + if len(src.PeersChanged) > 0 { + dst.PeersChanged = append(dst.PeersChanged, src.PeersChanged...) + } + if len(src.PeersRemoved) > 0 { + dst.PeersRemoved = append(dst.PeersRemoved, src.PeersRemoved...) + } + for id, up := range src.UserProfiles { + mak.Set(&dst.UserProfiles, id, up) } if src.Engine != nil { dst.Engine = src.Engine @@ -138,10 +164,10 @@ func mergeBoringNotifies(dst, src *ipn.Notify) *ipn.Notify { return dst } -// mergePeerChanges merges new peer changes from src into dst, either -// mutating dst or allocating a new slice if dst is nil, returning the merged result. -// Values in src override those in dst for the same NodeID. -func mergePeerChanges(dst, src []*tailcfg.PeerChange) []*tailcfg.PeerChange { +// mergePeerChangedPatch merges new peer-changed patches from src into dst, +// either mutating dst or allocating a new slice if dst is nil, returning the +// merged result. Values in src override those in dst for the same NodeID. +func mergePeerChangedPatch(dst, src []*tailcfg.PeerChange) []*tailcfg.PeerChange { idxByNode := make(map[tailcfg.NodeID]int, len(dst)) for i, d := range dst { idxByNode[d.NodeID] = i @@ -191,8 +217,7 @@ func mergePeerChangeForIpnBus(old, new *tailcfg.PeerChange) *tailcfg.PeerChange // should be sent on the IPN bus immediately (e.g. to GUIs) without // rate limiting it for a few seconds. // -// It effectively reports whether n contains any field set that's -// not NetMap or Engine. +// PeerChanges and Engine are the only "boring" (rate-limitable) fields. func isNotableNotify(n *ipn.Notify) bool { if n == nil { return false @@ -206,6 +231,7 @@ func isNotableNotify(n *ipn.Notify) bool { n.ErrMessage != nil || n.LoginFinished != nil || n.SelfChange != nil || + n.InitialStatus != nil || !n.DriveShares.IsNil() || n.Health != nil || len(n.IncomingFiles) > 0 || diff --git a/ipn/ipnlocal/bus_test.go b/ipn/ipnlocal/bus_test.go index 048e5bff4..68551b695 100644 --- a/ipn/ipnlocal/bus_test.go +++ b/ipn/ipnlocal/bus_test.go @@ -30,7 +30,10 @@ func TestIsNotableNotify(t *testing.T) { {"empty", &ipn.Notify{}, false}, {"version", &ipn.Notify{Version: "foo"}, false}, {"netmap", &ipn.Notify{NetMap: new(netmap.NetworkMap)}, false}, - {"peerchanges", &ipn.Notify{PeerChanges: []*tailcfg.PeerChange{{}}}, false}, + {"peerchanges", &ipn.Notify{PeerChangedPatch: []*tailcfg.PeerChange{{}}}, false}, + {"peerschanged", &ipn.Notify{PeersChanged: []*tailcfg.Node{{}}}, false}, + {"peersremoved", &ipn.Notify{PeersRemoved: []tailcfg.NodeID{1}}, false}, + {"userprofiles", &ipn.Notify{UserProfiles: map[tailcfg.UserID]tailcfg.UserProfileView{1: (&tailcfg.UserProfile{}).View()}}, false}, {"engine", &ipn.Notify{Engine: new(ipn.EngineStatus)}, false}, {"selfchange", &ipn.Notify{SelfChange: &tailcfg.Node{}}, true}, } @@ -42,7 +45,7 @@ func TestIsNotableNotify(t *testing.T) { for sf := range rt.Fields() { n := &ipn.Notify{} switch sf.Name { - case "_", "NetMap", "PeerChanges", "SelfChange", "Engine", "Version": + case "_", "NetMap", "PeerChangedPatch", "SelfChange", "PeersChanged", "PeersRemoved", "UserProfiles", "Engine", "Version": // Already covered above or not applicable. continue case "DriveShares": @@ -123,8 +126,10 @@ func (st *rateLimitingBusSenderTester) advance(d time.Duration) { } func TestRateLimitingBusSender(t *testing.T) { - nm1 := &ipn.Notify{NetMap: new(netmap.NetworkMap)} - nm2 := &ipn.Notify{NetMap: new(netmap.NetworkMap)} + // Both share NodeID 1 so merge collapses to a single PeerChange and + // the later one (nm2) wins. + nm1 := &ipn.Notify{PeerChangedPatch: []*tailcfg.PeerChange{{NodeID: 1, DERPRegion: 1}}} + nm2 := &ipn.Notify{PeerChangedPatch: []*tailcfg.PeerChange{{NodeID: 1, DERPRegion: 2}}} eng1 := &ipn.Notify{Engine: new(ipn.EngineStatus)} eng2 := &ipn.Notify{Engine: new(ipn.EngineStatus)} @@ -163,8 +168,8 @@ func TestRateLimitingBusSender(t *testing.T) { t.Fatalf("got %d items; want 2", len(st.got)) } gotn := st.got[1] - if gotn.NetMap != nm2.NetMap { - t.Errorf("got wrong NetMap; got %p", gotn.NetMap) + if !reflect.DeepEqual(gotn.PeerChangedPatch, nm2.PeerChangedPatch) { + t.Errorf("got wrong PeerChangedPatch; got %v want %v", gotn.PeerChangedPatch, nm2.PeerChangedPatch) } if gotn.Engine != eng2.Engine { t.Errorf("got wrong Engine; got %p", gotn.Engine) @@ -208,8 +213,8 @@ func TestRateLimitingBusSender(t *testing.T) { st.advance(5 * time.Second) select { case n := <-flushc: - if n.NetMap != nm2.NetMap { - t.Errorf("got wrong NetMap; got %p", n.NetMap) + if !reflect.DeepEqual(n.PeerChangedPatch, nm2.PeerChangedPatch) { + t.Errorf("got wrong PeerChangedPatch; got %v want %v", n.PeerChangedPatch, nm2.PeerChangedPatch) } case <-time.After(10 * time.Second): t.Error("timeout") @@ -221,7 +226,7 @@ func TestRateLimitingBusSender(t *testing.T) { }) } -func TestMergePeerChanges(t *testing.T) { +func TestMergePeerChangedPatch(t *testing.T) { online := true offline := false @@ -232,7 +237,7 @@ func TestMergePeerChanges(t *testing.T) { new := []*tailcfg.PeerChange{ {NodeID: 2, DERPRegion: 2}, } - got := mergePeerChanges(old, new) + got := mergePeerChangedPatch(old, new) if len(got) != 2 { t.Fatalf("len = %d; want 2", len(got)) } @@ -249,7 +254,7 @@ func TestMergePeerChanges(t *testing.T) { new := []*tailcfg.PeerChange{ {NodeID: 1, DERPRegion: 5, Online: &offline}, } - got := mergePeerChanges(old, new) + got := mergePeerChangedPatch(old, new) if len(got) != 2 { t.Fatalf("len = %d; want 2 (merged, not appended)", len(got)) } @@ -273,7 +278,7 @@ func TestMergePeerChanges(t *testing.T) { {NodeID: 1, DERPRegion: 2}, {NodeID: 3, DERPRegion: 30}, } - got := mergePeerChanges(old, new) + got := mergePeerChangedPatch(old, new) if len(got) != 2 { t.Fatalf("len = %d; want 2", len(got)) } @@ -292,7 +297,7 @@ func TestMergePeerChanges(t *testing.T) { new := []*tailcfg.PeerChange{ {NodeID: 1, Online: &offline}, } - got := mergePeerChanges(old, new) + got := mergePeerChangedPatch(old, new) if len(got) != 1 { t.Fatalf("len = %d; want 1", len(got)) } @@ -311,7 +316,7 @@ func TestMergePeerChanges(t *testing.T) { new := []*tailcfg.PeerChange{ {NodeID: 1, DERPRegion: 1}, } - got := mergePeerChanges(nil, new) + got := mergePeerChangedPatch(nil, new) if len(got) != 1 { t.Fatalf("len = %d; want 1", len(got)) } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index b5a0a353c..261882018 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -153,6 +153,18 @@ type watchSession struct { sessionID string cancel context.CancelFunc // to shut down the session mask ipn.NotifyWatchOpt // watch options for this session + + // lastSentUserProfile is the per-UserID [tailcfg.UserProfileView] + // most recently delivered to this session via [Notify.UserProfiles]. + // On a subsequent send, an incoming entry whose + // [tailcfg.UserProfileView.Equal] reports identity-or-equal-fields + // to the stored view for that UserID is dropped from the + // per-session copy of [Notify.UserProfiles], so the session only + // sees genuinely new or changed profiles. The views share backing + // memory with the producer's tracking maps, so the common + // "control re-announces the same profile" case is a pointer-cheap + // equality check. + lastSentUserProfile map[tailcfg.UserID]tailcfg.UserProfileView } var ( @@ -1337,7 +1349,13 @@ func (b *LocalBackend) UpdateStatus(sb *ipnstate.StatusBuilder) { b.mu.Lock() defer b.mu.Unlock() + b.updateStatusLocked(sb) +} +// updateStatusLocked is the b.mu-holding portion of [LocalBackend.UpdateStatus]. +// +// b.mu must be held. +func (b *LocalBackend) updateStatusLocked(sb *ipnstate.StatusBuilder) { cn := b.currentNode() nm := cn.NetMap() sb.MutateStatus(func(s *ipnstate.Status) { @@ -1625,14 +1643,32 @@ func (b *LocalBackend) PeerCaps(src netip.Addr) tailcfg.PeerCapMap { // given NodeID, in O(1) time. It returns ok=false if no such peer is in // the current netmap. // -// It is intended for callers that need the latest state of a single peer -// without fetching the entire netmap. +// It is intended for callers that observed a peer-mutation signal (e.g. +// [ipn.Notify.PeerChangedPatch] or [ipn.Notify.PeersChanged]) and want +// the latest state of the affected node without having to apply the patch +// themselves — useful for older clients that don't recognize a new +// [tailcfg.PeerChange] field, or that just don't want to bother. func (b *LocalBackend) PeerByID(id tailcfg.NodeID) (n tailcfg.NodeView, ok bool) { return b.currentNode().NodeByID(id) } +// UserProfile returns the current [tailcfg.UserProfile] for the given UserID, +// in O(1) time. It returns ok=false if no such User is in the current netmap. +// +// It is the LocalAPI/LocalBackend fallback for IPN-bus consumers that see a +// UserID they don't recognize and want to resolve it. +func (b *LocalBackend) UserProfile(id tailcfg.UserID) (u tailcfg.UserProfileView, ok bool) { + 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() } @@ -1909,13 +1945,17 @@ func (b *LocalBackend) setControlClientStatusLocked(c controlclient.Client, st c // Notify watchers that the self node may have changed. Reactive // consumers (containerboot, kube agents, sniproxy, etc.) listen on - // this signal and re-fetch peers/DNS via the LocalAPI if they need - // more than self info. + // this signal and re-fetch peers/DNS via [LocalClient.NetMap] if + // they need more than self info. var selfChange *tailcfg.Node if st.NetMap.SelfNode.Valid() { selfChange = st.NetMap.SelfNode.AsStruct() } - b.sendLocked(ipn.Notify{NetMap: st.NetMap, SelfChange: selfChange}) + notify := ipn.Notify{SelfChange: selfChange} + if goosGetsLegacyNetmapNotify { + notify.NetMap = st.NetMap + } + b.sendLocked(notify) // The error here is unimportant as is the result. This will recalculate the suggested exit node // cache the value and push any changes to the IPN bus. @@ -2206,6 +2246,7 @@ func (b *LocalBackend) sysPolicyChanged(policy policyclient.PolicyChange) { } var _ controlclient.NetmapDeltaUpdater = (*LocalBackend)(nil) +var _ controlclient.PacketFilterUpdater = (*LocalBackend)(nil) // UpdateNetmapDelta implements controlclient.NetmapDeltaUpdater. func (b *LocalBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bool) { @@ -2222,9 +2263,23 @@ func (b *LocalBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bo cn := b.currentNode() cn.UpdateNetmapDelta(muts) - if ms, ok := b.sys.MagicSock.GetOK(); ok { - ms.UpdateNetmapDelta(muts) + // Dispatch Add/Remove per-peer to magicsock, and any per-field + // patches via the existing UpdateNetmapDelta path. The per-peer + // methods take c.mu themselves, so we can't call them from inside + // magicsock.UpdateNetmapDelta which already holds c.mu. + peersAddedOrRemoved := false + ms := b.MagicConn() + for _, m := range muts { + switch m := m.(type) { + case netmap.NodeMutationAdd: + ms.UpsertPeer(m.Node) + peersAddedOrRemoved = true + case netmap.NodeMutationRemove: + ms.RemovePeer(m.NodeIDBeingMutated()) + peersAddedOrRemoved = true + } } + ms.UpdateNetmapDelta(muts) // If auto exit nodes are enabled and our exit node went offline, // we need to schedule picking a new one. @@ -2255,15 +2310,30 @@ func (b *LocalBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bo return true } - if mutationsAreWorthyOfTellingIPNBus(muts) { - // The notifier will strip the netmap based on the watchOpts mask if the watcher - // has indicated it can handle PeerChanges. - notify = &ipn.Notify{NetMap: cn.netMapWithPeers()} - if peerChanges, ok := ipnBusPeerChangesFromNodeMutations(muts); ok { - notify.PeerChanges = peerChanges - } else { + // A single MapResponse can carry adds/removes (full Nodes) AND + // per-field patches in the same delta. Build one Notify that + // reflects all of them; per-session stripping in [sendToLocked] + // hides fields the watcher didn't opt in to (and promotes patches + // into full Nodes for watchers that asked for PeerChanges but not + // PeerPatches). + if peersAddedOrRemoved || mutationsAreWorthyOfTellingIPNBus(muts) { + notify = &ipn.Notify{} + for _, m := range muts { + switch m := m.(type) { + case netmap.NodeMutationAdd: + notify.PeersChanged = append(notify.PeersChanged, m.Node.AsStruct()) + case netmap.NodeMutationRemove: + notify.PeersRemoved = append(notify.PeersRemoved, m.NodeIDBeingMutated()) + } + } + if patches, ok := ipnBusPeerChangedPatchFromNodeMutations(muts); ok && len(patches) > 0 { + notify.PeerChangedPatch = patches + } else if !ok { b.logf("[unexpected] got mutations worthy of telling IPN bus but failed to convert to peer changes") } + if goosGetsLegacyNetmapNotify { + notify.NetMap = cn.netMapWithPeers() + } } else if testenv.InTest() { // In tests, send an empty Notify as a wake-up so end-to-end // integration tests in another repo can check on the status of @@ -2273,6 +2343,54 @@ func (b *LocalBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bo return true } +// UpdatePacketFilter implements [controlclient.PacketFilterUpdater]. +// +// It is called by the controlclient when a MapResponse carries a new packet +// filter. Avoiding a full netmap rebuild matters here because the packet +// filter currently changes on every peer add on large tailnets. +func (b *LocalBackend) UpdatePacketFilter(rules views.Slice[tailcfg.FilterRule], parsed []filter.Match) { + b.mu.Lock() + defer b.mu.Unlock() + cn := b.currentNode() + if cn.NetMap() == nil { + // No netmap installed yet; the initial full-netmap path will + // take care of installing the filter. + return + } + cn.setPacketFilter(rules, parsed) + b.updateFilterLocked(b.pm.CurrentPrefs()) +} + +// UpdateUserProfiles implements [controlclient.UserProfileUpdater]. +// +// It is called by the controlclient when a MapResponse carries new or +// updated [tailcfg.UserProfileView] entries. It merges them into the +// current netmap's UserProfiles so [LocalBackend.UserProfile] can +// resolve them, and emits an [ipn.Notify] with [Notify.UserProfiles] +// populated so IPN-bus consumers (sessions opted in to +// NotifyPeerChanges / NotifyPeerPatches) get the new profiles before +// any subsequent PeersChanged / PeerChangedPatch entries that reference +// these UserIDs. +// +// The views in profiles share backing memory with the controlclient +// caller's tracking map; nodeBackend stores them as-is, and per-bus +// sessions can dedup via [UserProfileView.Equal] without copying. +func (b *LocalBackend) UpdateUserProfiles(profiles map[tailcfg.UserID]tailcfg.UserProfileView) { + if len(profiles) == 0 { + return + } + b.mu.Lock() + defer b.mu.Unlock() + cn := b.currentNode() + if cn.NetMap() == nil { + // No netmap installed yet; the initial full-netmap path will + // take care of installing UserProfiles. + return + } + cn.mergeUserProfiles(profiles) + b.sendLocked(ipn.Notify{UserProfiles: profiles}) +} + // mustationsAreWorthyOfRecalculatingSuggestedExitNode reports whether any mutation type in muts is // worthy of recalculating the suggested exit node. func mutationsAreWorthyOfRecalculatingSuggestedExitNode(muts []netmap.NodeMutation, cn *nodeBackend, sid tailcfg.StableNodeID) bool { @@ -2311,32 +2429,40 @@ func mutationsAreWorthyOfRecalculatingSuggestedExitNode(muts []netmap.NodeMutati return false } -// ipnBusPeerChangesFromNodeMutations converts a slice of NodeMutations to a slice of -// *tailcfg.PeerChange for use in ipn.Notify.PeerChanges. -// Multiple mutations to the same node are merged into a single PeerChange. -// If we encounter any mutations that we cannot convert to a PeerChange, we return (nil, false) -// to indicate that the caller should send a Notify with the full netmap instead of -// trying to send granular peer changes. -func ipnBusPeerChangesFromNodeMutations(muts []netmap.NodeMutation) ([]*tailcfg.PeerChange, bool) { +// ipnBusPeerChangedPatchFromNodeMutations converts the patch-shaped subset of +// muts (per-field updates that fit in a [tailcfg.PeerChange]) into a slice of +// [tailcfg.PeerChange] for use in [ipn.Notify.PeerChangedPatch]. Multiple +// mutations against the same node are merged into a single PeerChange. +// +// Add/Remove mutations are skipped (they ride +// [ipn.Notify.PeersChanged]/[ipn.Notify.PeersRemoved]). Any other mutation +// type that doesn't fit a [tailcfg.PeerChange] causes ok=false; the caller +// should fall back to a full netmap rebuild. +func ipnBusPeerChangedPatchFromNodeMutations(muts []netmap.NodeMutation) ([]*tailcfg.PeerChange, bool) { byID := map[tailcfg.NodeID]*tailcfg.PeerChange{} var ordered []*tailcfg.PeerChange - for _, m := range muts { - nid := m.NodeIDBeingMutated() + getOrAdd := func(nid tailcfg.NodeID) *tailcfg.PeerChange { pc := byID[nid] if pc == nil { pc = &tailcfg.PeerChange{NodeID: nid} byID[nid] = pc ordered = append(ordered, pc) } + return pc + } + for _, m := range muts { switch v := m.(type) { + case netmap.NodeMutationAdd, netmap.NodeMutationRemove: + // These go in PeersChanged / PeersRemoved, not as patches. + continue case netmap.NodeMutationOnline: - pc.Online = &v.Online + getOrAdd(v.NodeIDBeingMutated()).Online = &v.Online case netmap.NodeMutationLastSeen: - pc.LastSeen = &v.LastSeen + getOrAdd(v.NodeIDBeingMutated()).LastSeen = &v.LastSeen case netmap.NodeMutationDERPHome: - pc.DERPRegion = v.DERPRegion + getOrAdd(v.NodeIDBeingMutated()).DERPRegion = v.DERPRegion case netmap.NodeMutationEndpoints: - pc.Endpoints = v.Endpoints + getOrAdd(v.NodeIDBeingMutated()).Endpoints = v.Endpoints default: return nil, false } @@ -3304,9 +3430,21 @@ func (b *LocalBackend) WatchNotificationsAs(ctx context.Context, actor ipnauth.A var ini *ipn.Notify + // Build the engine half of the InitialStatus before taking b.mu, since + // b.e.UpdateStatus has its own locking and shouldn't be called under + // b.mu (lock-ordering: outer-to-inner is b.mu -> engine, not the other + // way). The backend half is then populated under b.mu below, atomically + // with watcher registration so no events arrive on the watcher's + // channel before InitialStatus is delivered. + var statusSB *ipnstate.StatusBuilder + if mask&ipn.NotifyInitialStatus != 0 { + statusSB = &ipnstate.StatusBuilder{WantPeers: true} + b.e.UpdateStatus(statusSB) + } + b.mu.Lock() - const initialBits = ipn.NotifyInitialState | ipn.NotifyInitialPrefs | ipn.NotifyInitialNetMap | ipn.NotifyInitialDriveShares | ipn.NotifyInitialSuggestedExitNode | ipn.NotifyInitialClientVersion + const initialBits = ipn.NotifyInitialState | ipn.NotifyInitialPrefs | ipn.NotifyInitialNetMap | ipn.NotifyInitialStatus | ipn.NotifyInitialDriveShares | ipn.NotifyInitialSuggestedExitNode | ipn.NotifyInitialClientVersion if mask&initialBits != 0 { cn := b.currentNode() ini = &ipn.Notify{Version: version.Long()} @@ -3321,7 +3459,18 @@ func (b *LocalBackend) WatchNotificationsAs(ctx context.Context, actor ipnauth.A ini.Prefs = new(b.sanitizedPrefsLocked()) } if mask&ipn.NotifyInitialNetMap != 0 { - ini.NetMap = cn.NetMap() + if nm := cn.NetMap(); nm != nil && nm.SelfNode.Valid() { + ini.SelfChange = nm.SelfNode.AsStruct() + } + // The legacy initial NetMap is delivered cross-platform: it + // is what watchers asked for by setting NotifyInitialNetMap + // and is always a one-shot, so the cost of building it is + // paid once per bus subscription. + ini.NetMap = cn.netMapWithPeers() + } + if statusSB != nil { + b.updateStatusLocked(statusSB) + ini.InitialStatus = statusSB.Status() } if mask&ipn.NotifyInitialDriveShares != 0 && b.DriveSharingEnabled() { ini.DriveShares = b.pm.prefs.DriveShares() @@ -3450,16 +3599,6 @@ func (b *LocalBackend) DebugNotify(n ipn.Notify) { b.send(n) } -// DebugNotifyLastNetMap injects a fake notify message to clients, -// repeating whatever the last netmap was. -// -// It should only be used via the LocalAPI's debug handler. -func (b *LocalBackend) DebugNotifyLastNetMap() { - if nm := b.currentNode().NetMap(); nm != nil { - b.send(ipn.Notify{NetMap: nm}) - } -} - // DebugForceNetmapUpdate forces a full no-op netmap update of the current // netmap in all the various subsystems (wireguard, magicsock, LocalBackend). // @@ -3590,27 +3729,107 @@ func (b *LocalBackend) sendToLocked(n ipn.Notify, recipient notificationTarget) if !recipient.match(sess.owner) { continue } - nOut := &n - if n.PeerChanges != nil { - // Take a shallow copy of n so we can elide the PeerChanges or the Netmap - // based on the session's mask. - nOut = new(n) - if sess.mask&ipn.NotifyPeerChanges != 0 { - // Skip the full Netmap - nOut.NetMap = nil - } else { - // Skip the PeerChanges - nOut.PeerChanges = nil - } - } + nForSess := b.notifyForSessionLocked(sess, &n) select { - case sess.ch <- nOut: + case sess.ch <- nForSess: default: // Drop the notification if the channel is full. } } } +// notifyForSessionLocked returns the [ipn.Notify] to deliver to sess, +// applying per-session field gating to n: stripping fields the session +// didn't opt in to receive, promoting [Notify.PeerChangedPatch] entries +// into full-Node [Notify.PeersChanged] entries for sessions that asked +// for peer changes but not patches, and tracking on sess which +// [tailcfg.UserProfileView]s have already been delivered so subsequent +// sends only carry new/changed profiles. +// +// The returned pointer is either n itself (no adjustments needed for +// this session) or a fresh *ipn.Notify with the adjusted fields. The +// caller's *ipn.Notify is not mutated. +// +// b.mu must be held. +func (b *LocalBackend) notifyForSessionLocked(sess *watchSession, n *ipn.Notify) *ipn.Notify { + // Visibility of peer-set fields is governed by the watcher's mask: + // + // - NotifyPeerChanges: PeersChanged + PeersRemoved + // - NotifyPeerPatches (implies): + PeerChangedPatch + // + // A watcher with NotifyPeerChanges but not NotifyPeerPatches still + // observes every per-peer mutation; we just promote each + // PeerChangedPatch entry into a full-Node entry in PeersChanged so + // the watcher doesn't have to handle the patch shape. + wantsPeerChanges := sess.mask&(ipn.NotifyPeerChanges|ipn.NotifyPeerPatches) != 0 + wantsPeerPatches := sess.mask&ipn.NotifyPeerPatches != 0 + stripNetMap := goosGetsLegacyNetmapNotify && n.NetMap != nil && sess.mask&ipn.NotifyNoNetMap != 0 + stripPeersChanged := len(n.PeersChanged) > 0 && !wantsPeerChanges + stripPeersRemoved := len(n.PeersRemoved) > 0 && !wantsPeerChanges + stripPatches := len(n.PeerChangedPatch) > 0 && !wantsPeerPatches + promotePatches := len(n.PeerChangedPatch) > 0 && wantsPeerChanges && !wantsPeerPatches + + // UserProfiles ride alongside peer changes and are gated on the + // same opt-in. Sessions that didn't ask for peer changes get the + // field stripped entirely; opted-in sessions get a per-session + // subset containing only profiles that differ from what was last + // delivered to that session, compared via + // [tailcfg.UserProfileView.Equal] (pointer-cheap when the view + // shares backing memory with the previous send). + stripUserProfiles := len(n.UserProfiles) > 0 && !wantsPeerChanges + var sessUserProfiles map[tailcfg.UserID]tailcfg.UserProfileView + if !stripUserProfiles && len(n.UserProfiles) > 0 { + for id, up := range n.UserProfiles { + if up.Equal(sess.lastSentUserProfile[id]) { + continue // already has this exact profile + } + mak.Set(&sessUserProfiles, id, up) + mak.Set(&sess.lastSentUserProfile, id, up) + } + if len(sessUserProfiles) == 0 { + // All entries deduped. + stripUserProfiles = true + } + } + replaceUserProfiles := !stripUserProfiles && len(sessUserProfiles) != len(n.UserProfiles) + + if !stripNetMap && !stripPeersChanged && !stripPeersRemoved && !stripPatches && !stripUserProfiles && !replaceUserProfiles && !promotePatches { + return n + } + nCopy := *n + if stripNetMap { + nCopy.NetMap = nil + } + if stripPeersChanged { + nCopy.PeersChanged = nil + } + if stripPeersRemoved { + nCopy.PeersRemoved = nil + } + if promotePatches { + // Look up each patched peer's current Node and append it to + // PeersChanged. Watchers in this mode receive only full-Node + // updates; they never see PeerChangedPatch. + cn := b.currentNode() + for _, pc := range n.PeerChangedPatch { + nv, ok := cn.NodeByID(pc.NodeID) + if !ok { + continue + } + nCopy.PeersChanged = append(nCopy.PeersChanged, nv.AsStruct()) + } + } + if stripPatches { + nCopy.PeerChangedPatch = nil + } + if stripUserProfiles { + nCopy.UserProfiles = nil + } else if replaceUserProfiles { + nCopy.UserProfiles = sessUserProfiles + } + return &nCopy +} + // setAuthURLLocked sets the authURL and triggers [LocalBackend.popBrowserAuthNow] if the URL has changed. // This method is called when a new authURL is received from the control plane, meaning that either a user // has started a new interactive login (e.g., by running `tailscale login` or clicking Login in the GUI), @@ -5160,24 +5379,20 @@ func (b *LocalBackend) NetMap() *netmap.NetworkMap { // current. Use this for any caller that does not need to iterate Peers, // since it's O(1) regardless of tailnet size. // -// Returns nil if no network map has been received yet. +// It returns nil if no network map has been received yet. func (b *LocalBackend) NetMapNoPeers() *netmap.NetworkMap { return b.currentNode().NetMap() } -// NetMapWithPeers returns the latest network map with the Peers slice -// populated. +// NetMapWithPeers returns a copy of the latest cached network map with +// its Peers slice populated from the live per-node-backend peers map +// (i.e. reflecting any incremental delta updates applied since the last +// full netmap install). It is O(N) in the size of the peer set; prefer +// [LocalBackend.NetMapNoPeers] when only non-Peers fields are needed. // -// Currently this is the same as [LocalBackend.NetMapNoPeers]: the cached -// netmap's Peers slice may be stale relative to the live per-node-backend -// peers map. A follow-up change will switch this method to return a -// freshly-built netmap with up-to-date Peers, at O(N) cost per call. -// Callers that genuinely need the up-to-date peer set should use this -// method (and document why) so the upcoming change reaches them. -// -// Returns nil if no network map has been received yet. +// It returns nil if no netmap is yet available. func (b *LocalBackend) NetMapWithPeers() *netmap.NetworkMap { - return b.currentNode().NetMap() + return b.currentNode().netMapWithPeers() } // lookupPeerByIP returns the node public key for the peer that owns the diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 70cbc8991..8fded3755 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -1692,18 +1692,18 @@ func TestExitNodeNotifyOrder(t *testing.T) { // and an exit node ID notification (since an exit node is selected). // The netmap notification should be sent first. nw.watch(0, []wantedNotification{ - wantNetmapNotify(clientNetmap), + wantSelfChangeNotify(selfNode), wantExitNodeIDNotify(exitNode1.StableID()), }) lb.SetControlClientStatus(lb.cc, controlclient.Status{NetMap: clientNetmap}) nw.check() } -func wantNetmapNotify(want *netmap.NetworkMap) wantedNotification { +func wantSelfChangeNotify(want tailcfg.NodeView) wantedNotification { return wantedNotification{ - name: "Netmap", + name: "SelfChange", cond: func(t testing.TB, _ ipnauth.Actor, n *ipn.Notify) bool { - return n.NetMap == want + return n.SelfChange != nil && want.Valid() && n.SelfChange.StableID == want.StableID() }, } } @@ -2077,6 +2077,198 @@ func TestWatchNotificationsCallbacks(t *testing.T) { } } +// TestNotifyForSessionPeerVisibility verifies the per-session masking +// logic in [LocalBackend.notifyForSessionLocked] for the +// NotifyPeerChanges / NotifyPeerPatches flag pair: +// +// - A watcher with no peer-change bits should not see PeersChanged, +// PeersRemoved, or PeerChangedPatch. +// - A watcher with NotifyPeerChanges (but not NotifyPeerPatches) should +// see PeersChanged and PeersRemoved, AND any incoming +// PeerChangedPatch entries should be promoted to full Nodes in +// PeersChanged. PeerChangedPatch itself must be cleared. +// - A watcher with NotifyPeerPatches should see all three fields. +func TestNotifyForSessionPeerVisibility(t *testing.T) { + b := newTestLocalBackend(t) + + // Install a netmap with two peers so the patch-promotion path can + // resolve PeerChangedPatch entries to full Nodes. + nm := &netmap.NetworkMap{} + for _, id := range []tailcfg.NodeID{10, 20} { + nm.Peers = append(nm.Peers, (&tailcfg.Node{ + ID: id, + Key: makeNodeKeyFromID(id), + Addresses: []netip.Prefix{netip.MustParsePrefix(fmt.Sprintf("100.64.0.%d/32", id))}, + }).View()) + } + b.currentNode().SetNetMap(nm) + + // Build a Notify carrying every peer-change kind: an added peer + // (PeersChanged), a removed peer (PeersRemoved), and a patch for an + // existing peer (PeerChangedPatch). + addedPeer := &tailcfg.Node{ID: 30, Key: makeNodeKeyFromID(30)} + online := true + notify := ipn.Notify{ + PeersChanged: []*tailcfg.Node{addedPeer}, + PeersRemoved: []tailcfg.NodeID{99}, + PeerChangedPatch: []*tailcfg.PeerChange{{NodeID: 10, Online: &online}}, + } + + deliver := func(mask ipn.NotifyWatchOpt) *ipn.Notify { + sess := &watchSession{mask: mask} + b.mu.Lock() + defer b.mu.Unlock() + return b.notifyForSessionLocked(sess, ¬ify) + } + + t.Run("no_peer_bits", func(t *testing.T) { + n := deliver(0) + if len(n.PeersChanged) != 0 { + t.Errorf("PeersChanged = %v; want empty", n.PeersChanged) + } + if len(n.PeersRemoved) != 0 { + t.Errorf("PeersRemoved = %v; want empty", n.PeersRemoved) + } + if len(n.PeerChangedPatch) != 0 { + t.Errorf("PeerChangedPatch = %v; want empty", n.PeerChangedPatch) + } + }) + + t.Run("peer_changes_only_promotes_patches", func(t *testing.T) { + n := deliver(ipn.NotifyPeerChanges) + if len(n.PeerChangedPatch) != 0 { + t.Errorf("PeerChangedPatch should be stripped; got %v", n.PeerChangedPatch) + } + if len(n.PeersRemoved) != 1 || n.PeersRemoved[0] != 99 { + t.Errorf("PeersRemoved = %v; want [99]", n.PeersRemoved) + } + // PeersChanged should contain the originally-added peer (30) AND + // a promoted full-Node entry for the patched peer (10). + ids := make(map[tailcfg.NodeID]bool, len(n.PeersChanged)) + for _, p := range n.PeersChanged { + ids[p.ID] = true + } + if !ids[30] { + t.Errorf("PeersChanged missing added peer 30; got %+v", n.PeersChanged) + } + if !ids[10] { + t.Errorf("PeersChanged missing promoted peer 10; got %+v", n.PeersChanged) + } + }) + + t.Run("peer_patches_keeps_patch_field", func(t *testing.T) { + n := deliver(ipn.NotifyPeerPatches) + if len(n.PeerChangedPatch) != 1 || n.PeerChangedPatch[0].NodeID != 10 { + t.Errorf("PeerChangedPatch = %v; want [{NodeID:10,...}]", n.PeerChangedPatch) + } + if len(n.PeersChanged) != 1 || n.PeersChanged[0].ID != 30 { + t.Errorf("PeersChanged = %v; want [{ID:30}]", n.PeersChanged) + } + if len(n.PeersRemoved) != 1 || n.PeersRemoved[0] != 99 { + t.Errorf("PeersRemoved = %v; want [99]", n.PeersRemoved) + } + }) + + t.Run("both_bits_unchanged", func(t *testing.T) { + n := deliver(ipn.NotifyPeerChanges | ipn.NotifyPeerPatches) + if len(n.PeerChangedPatch) != 1 { + t.Errorf("PeerChangedPatch len = %d; want 1", len(n.PeerChangedPatch)) + } + if len(n.PeersChanged) != 1 { + t.Errorf("PeersChanged len = %d; want 1", len(n.PeersChanged)) + } + if len(n.PeersRemoved) != 1 { + t.Errorf("PeersRemoved len = %d; want 1", len(n.PeersRemoved)) + } + }) +} + +// TestNotifyForSessionUserProfilesGating verifies that +// [Notify.UserProfiles] is only delivered to sessions opted in to +// NotifyPeerChanges/NotifyPeerPatches, and is deduped per-UserID +// against [watchSession.lastSentUserProfile] across successive sends. +func TestNotifyForSessionUserProfilesGating(t *testing.T) { + b := newTestLocalBackend(t) + + deliver := func(sess *watchSession, profiles map[tailcfg.UserID]tailcfg.UserProfileView) *ipn.Notify { + b.mu.Lock() + defer b.mu.Unlock() + return b.notifyForSessionLocked(sess, &ipn.Notify{UserProfiles: profiles}) + } + + profiles := map[tailcfg.UserID]tailcfg.UserProfileView{ + 7: (&tailcfg.UserProfile{ID: 7, LoginName: "alice@example.com", DisplayName: "Alice"}).View(), + } + + t.Run("no_bits_strips", func(t *testing.T) { + n := deliver(&watchSession{}, profiles) + if len(n.UserProfiles) != 0 { + t.Errorf("UserProfiles = %v; want empty", n.UserProfiles) + } + }) + t.Run("peer_changes_delivers", func(t *testing.T) { + n := deliver(&watchSession{mask: ipn.NotifyPeerChanges}, profiles) + if got, want := len(n.UserProfiles), 1; got != want { + t.Fatalf("UserProfiles len = %d; want %d", got, want) + } + if n.UserProfiles[7].LoginName() != "alice@example.com" { + t.Errorf("got %+v; want alice", n.UserProfiles) + } + }) + t.Run("peer_patches_delivers", func(t *testing.T) { + n := deliver(&watchSession{mask: ipn.NotifyPeerPatches}, profiles) + if got, want := len(n.UserProfiles), 1; got != want { + t.Fatalf("UserProfiles len = %d; want %d", got, want) + } + }) + + // The remaining cases share a single session so the dedup state on + // [watchSession.lastSentUserProfile] persists across deliveries. + sess := &watchSession{mask: ipn.NotifyPeerChanges} + + t.Run("first_send", func(t *testing.T) { + n := deliver(sess, profiles) + if got, want := len(n.UserProfiles), 1; got != want { + t.Fatalf("UserProfiles len = %d; want %d", got, want) + } + }) + t.Run("dedup_repeat_same_map", func(t *testing.T) { + // Resending the exact same map should deliver nothing. + n := deliver(sess, profiles) + if len(n.UserProfiles) != 0 { + t.Errorf("got UserProfiles=%v on repeat; want empty (deduped)", n.UserProfiles) + } + }) + t.Run("per_user_dedup", func(t *testing.T) { + // A Notify with two profiles where only one changed should + // deliver only the changed one. + mixed := map[tailcfg.UserID]tailcfg.UserProfileView{ + 7: (&tailcfg.UserProfile{ID: 7, LoginName: "alice@example.com", DisplayName: "Alice"}).View(), // unchanged + 8: (&tailcfg.UserProfile{ID: 8, LoginName: "bob@example.com", DisplayName: "Bob the New"}).View(), // new + } + n := deliver(sess, mixed) + if got, want := len(n.UserProfiles), 1; got != want { + t.Fatalf("UserProfiles len = %d; want %d (only the new user)", got, want) + } + if _, ok := n.UserProfiles[7]; ok { + t.Errorf("UserProfiles still includes user 7 (should have been deduped)") + } + if got := n.UserProfiles[8].LoginName(); got != "bob@example.com" { + t.Errorf("UserProfiles[8].LoginName = %q; want bob", got) + } + }) + t.Run("changed_user_delivers", func(t *testing.T) { + // Updating an existing UserID re-sends just that one. + updated := map[tailcfg.UserID]tailcfg.UserProfileView{ + 7: (&tailcfg.UserProfile{ID: 7, LoginName: "alice@example.com", DisplayName: "Alice 2.0"}).View(), + } + n := deliver(sess, updated) + if n.UserProfiles[7].DisplayName() != "Alice 2.0" { + t.Errorf("got %+v; want updated alice", n.UserProfiles) + } + }) +} + // tests LocalBackend.updateNetmapDeltaLocked func TestUpdateNetmapDelta(t *testing.T) { b := newTestLocalBackend(t) diff --git a/ipn/ipnlocal/node_backend.go b/ipn/ipnlocal/node_backend.go index f8579900d..dc6a432d1 100644 --- a/ipn/ipnlocal/node_backend.go +++ b/ipn/ipnlocal/node_backend.go @@ -572,10 +572,37 @@ func (nb *nodeBackend) updatePeersLocked() { } } +// setPacketFilter updates the netmap's packet filter rules and parsed +// form in place. nb.mu is acquired by this method. +func (nb *nodeBackend) setPacketFilter(rules views.Slice[tailcfg.FilterRule], parsed []filter.Match) { + nb.mu.Lock() + defer nb.mu.Unlock() + if nb.netMap == nil { + return + } + nb.netMap.PacketFilterRules = rules + nb.netMap.PacketFilter = parsed +} + +// mergeUserProfiles merges new/updated [tailcfg.UserProfileView] +// entries into the current netmap's UserProfiles map. Callers must hold +// [LocalBackend.mu]. nb.mu is acquired by this method. The views share backing +// memory with the caller; they are stored as-is. +func (nb *nodeBackend) mergeUserProfiles(profiles map[tailcfg.UserID]tailcfg.UserProfileView) { + nb.mu.Lock() + defer nb.mu.Unlock() + if nb.netMap == nil { + return + } + for id, up := range profiles { + mak.Set(&nb.netMap.UserProfiles, id, up) + } +} + func (nb *nodeBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bool) { nb.mu.Lock() defer nb.mu.Unlock() - if nb.netMap == nil || len(nb.peers) == 0 { + if nb.netMap == nil { return false } @@ -585,9 +612,35 @@ func (nb *nodeBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bo var mutableNodes map[tailcfg.NodeID]*tailcfg.Node for _, m := range muts { - n, ok := mutableNodes[m.NodeIDBeingMutated()] + switch m := m.(type) { + case netmap.NodeMutationAdd: + nid := m.Node.ID() + mak.Set(&nb.peers, nid, m.Node) + for _, ipp := range m.Node.Addresses().All() { + if ipp.IsSingleIP() { + mak.Set(&nb.nodeByAddr, ipp.Addr(), nid) + } + } + mak.Set(&nb.nodeByKey, m.Node.Key(), nid) + continue + case netmap.NodeMutationRemove: + nid := m.NodeIDBeingMutated() + if old, ok := nb.peers[nid]; ok { + for _, ipp := range old.Addresses().All() { + if ipp.IsSingleIP() { + delete(nb.nodeByAddr, ipp.Addr()) + } + } + delete(nb.nodeByKey, old.Key()) + delete(nb.peers, nid) + } + continue + } + // Per-field mutation. + nid := m.NodeIDBeingMutated() + n, ok := mutableNodes[nid] if !ok { - nv, ok := nb.peers[m.NodeIDBeingMutated()] + nv, ok := nb.peers[nid] if !ok { // TODO(bradfitz): unexpected metric? return false @@ -600,6 +653,7 @@ func (nb *nodeBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bo for nid, n := range mutableNodes { nb.peers[nid] = n.View() } + nb.signalKeyWaitersForTestLocked() return true } diff --git a/ipn/localapi/debug.go b/ipn/localapi/debug.go index 6f222bef0..f6b0d631b 100644 --- a/ipn/localapi/debug.go +++ b/ipn/localapi/debug.go @@ -200,8 +200,6 @@ func (h *Handler) serveDebug(w http.ResponseWriter, r *http.Request) { break } h.b.DebugNotify(n) - case "notify-last-netmap": - h.b.DebugNotifyLastNetMap() case "break-tcp-conns": err = h.b.DebugBreakTCPConns() case "break-derp-conns": diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index 6375f440d..af0ae11c5 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -90,6 +90,7 @@ var handler = map[string]LocalAPIHandler{ "shutdown": (*Handler).serveShutdown, "start": (*Handler).serveStart, "status": (*Handler).serveStatus, + "user-profile": (*Handler).serveUserProfile, "whois": (*Handler).serveWhoIs, } @@ -880,6 +881,10 @@ func (h *Handler) serveWatchIPNBus(w http.ResponseWriter, r *http.Request) { } mask = ipn.NotifyWatchOpt(v) } + if mask&(ipn.NotifyPeerChanges|ipn.NotifyPeerPatches) != 0 && mask&ipn.NotifyInitialNetMap != 0 { + http.Error(w, "NotifyPeerChanges/NotifyPeerPatches are mutually exclusive with NotifyInitialNetMap", http.StatusBadRequest) + return + } w.Header().Set("Content-Type", "application/json") ctx := r.Context() @@ -1118,12 +1123,14 @@ type peerByIDBackend interface { PeerByID(tailcfg.NodeID) (tailcfg.NodeView, bool) } -// servePeerByID returns the current full [tailcfg.Node] for the peer with -// the NodeID given in the "id" query parameter, in O(1) time. It returns -// 404 if no such peer is in the current netmap. +// servePeerByID returns the current full [tailcfg.Node] for the peer with the +// NodeID given in the "id" query parameter. It returns 404 if no such peer is +// in the current netmap. // -// It is intended for clients that need the latest state of a single peer -// without fetching the entire netmap. +// It is intended for clients that observed a peer-mutation signal (e.g. +// [ipn.Notify.PeerChangedPatch] or [ipn.Notify.PeersChanged]) and want the +// latest state of the affected node without having to apply the patch +// themselves. func (h *Handler) servePeerByID(w http.ResponseWriter, r *http.Request) { h.servePeerByIDWithBackend(w, r, h.b) } @@ -1150,6 +1157,45 @@ func (h *Handler) servePeerByIDWithBackend(w http.ResponseWriter, r *http.Reques e.Encode(nv.AsStruct()) } +// userProfileBackend is the subset of [ipnlocal.LocalBackend] used by +// [Handler.serveUserProfile]. It exists so the handler can be tested +// with a trivial mock without spinning up a full LocalBackend. +type userProfileBackend interface { + UserProfile(tailcfg.UserID) (tailcfg.UserProfileView, bool) +} + +// serveUserProfile returns the current [tailcfg.UserProfile] for the User +// with the UserID given in the "id" query parameter, in O(1) time. It +// returns 404 if no such user is in the current netmap. +// +// It is the LocalAPI fallback for IPN-bus consumers that see a UserID +// referenced by a peer Node and want to resolve it to a UserProfile. +func (h *Handler) serveUserProfile(w http.ResponseWriter, r *http.Request) { + h.serveUserProfileWithBackend(w, r, h.b) +} + +func (h *Handler) serveUserProfileWithBackend(w http.ResponseWriter, r *http.Request, b userProfileBackend) { + if !h.PermitRead { + http.Error(w, "user-profile access denied", http.StatusForbidden) + return + } + idStr := r.FormValue("id") + id, err := strconv.ParseInt(idStr, 10, 64) + if err != nil || id <= 0 { + http.Error(w, "invalid 'id' parameter", http.StatusBadRequest) + return + } + uv, ok := b.UserProfile(tailcfg.UserID(id)) + if !ok { + http.Error(w, "no user with that UserID", http.StatusNotFound) + return + } + w.Header().Set("Content-Type", "application/json") + e := json.NewEncoder(w) + e.SetIndent("", "\t") + e.Encode(uv.AsStruct()) +} + // serveSetExpirySooner sets the expiry date on the current machine, specified // by an `expiry` unix timestamp as POST or query param. func (h *Handler) serveSetExpirySooner(w http.ResponseWriter, r *http.Request) { diff --git a/ipn/localapi/localapi_test.go b/ipn/localapi/localapi_test.go index 352f71e00..df9409a9a 100644 --- a/ipn/localapi/localapi_test.go +++ b/ipn/localapi/localapi_test.go @@ -268,6 +268,66 @@ func TestServePeerByID(t *testing.T) { }) } +type fakeUserProfileBackend map[tailcfg.UserID]*tailcfg.UserProfile + +func (f fakeUserProfileBackend) UserProfile(id tailcfg.UserID) (tailcfg.UserProfileView, bool) { + u, ok := f[id] + if !ok { + return tailcfg.UserProfileView{}, false + } + return u.View(), true +} + +func TestServeUserProfile(t *testing.T) { + h := handlerForTest(t, &Handler{PermitRead: true}) + b := fakeUserProfileBackend{ + 7: {ID: 7, LoginName: "alice@example.com", DisplayName: "Alice"}, + } + + tests := []struct { + name string + query string + wantCode int + wantLogin string + }{ + {"hit", "id=7", 200, "alice@example.com"}, + {"miss", "id=99", 404, ""}, + {"bad_id", "id=garbage", 400, ""}, + {"missing_id", "", 400, ""}, + {"zero_id", "id=0", 400, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rec := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/v0/user-profile?"+tt.query, nil) + h.serveUserProfileWithBackend(rec, req, b) + if rec.Code != tt.wantCode { + t.Fatalf("status = %d, want %d; body=%q", rec.Code, tt.wantCode, rec.Body.String()) + } + if tt.wantCode != 200 { + return + } + var got tailcfg.UserProfile + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("unmarshal body %q: %v", rec.Body.Bytes(), err) + } + if got.LoginName != tt.wantLogin { + t.Errorf("LoginName = %q, want %q", got.LoginName, tt.wantLogin) + } + }) + } + + t.Run("forbidden", func(t *testing.T) { + hh := handlerForTest(t, &Handler{PermitRead: false}) + rec := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/v0/user-profile?id=7", nil) + hh.serveUserProfileWithBackend(rec, req, b) + if rec.Code != http.StatusForbidden { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusForbidden) + } + }) +} + func TestShouldDenyServeConfigForGOOSAndUserContext(t *testing.T) { newHandler := func(connIsLocalAdmin bool) *Handler { return handlerForTest(t, &Handler{ diff --git a/ipn/serve.go b/ipn/serve.go index 21d15ab81..5a65658a5 100644 --- a/ipn/serve.go +++ b/ipn/serve.go @@ -191,7 +191,7 @@ func (sc *ServeConfig) WebHandlerExists(svcName tailcfg.ServiceName, hp HostPort } // GetWebHandler returns the HTTPHandler for the given host:port and mount point. -// Returns nil if the handler does not exist. +// It returns nil if the handler does not exist. func (sc *ServeConfig) GetWebHandler(svcName tailcfg.ServiceName, hp HostPort, mount string) *HTTPHandler { if sc == nil { return nil diff --git a/tsnet/packet_filter_test.go b/tsnet/packet_filter_test.go index ca776436e..342210f4b 100644 --- a/tsnet/packet_filter_test.go +++ b/tsnet/packet_filter_test.go @@ -20,26 +20,23 @@ import ( "tailscale.com/wgengine/filter" ) -// waitFor blocks until a NetMap is seen on the IPN bus that satisfies the given -// function f. Note: has no timeout, should be called with a ctx that has an -// appropriate timeout set. +// waitFor blocks until the LocalBackend's current netmap satisfies the given +// function f. It uses a bus subscription to wake up on netmap and peer-mutation +// events rather than polling. Note: it has no timeout and should be called with +// a ctx that has an appropriate timeout set. func waitFor(t testing.TB, ctx context.Context, s *Server, f func(*netmap.NetworkMap) bool) error { t.Helper() - watcher, err := s.localClient.WatchIPNBus(ctx, ipn.NotifyInitialNetMap) + w, err := s.localClient.WatchIPNBus(ctx, ipn.NotifyInitialState|ipn.NotifyPeerChanges) if err != nil { - t.Fatalf("error watching IPN bus: %s", err) + return fmt.Errorf("watching IPN bus: %w", err) } - defer watcher.Close() - + defer w.Close() for { - n, err := watcher.Next() - if err != nil { - return fmt.Errorf("getting next ipn.Notify from IPN bus: %w", err) + if nm := s.lb.NetMapWithPeers(); nm != nil && f(nm) { + return nil } - if n.NetMap != nil { - if f(n.NetMap) { - return nil - } + if _, err := w.Next(); err != nil { + return fmt.Errorf("waiting for netmap: %w", err) } } } @@ -192,7 +189,7 @@ func TestPacketFilterFromNetmap(t *testing.T) { controlURL, c := startControl(t) s, _, pubKey := startServer(t, ctx, controlURL, "node") - if test.waitTest(s.lb.NetMap()) { + if test.waitTest(s.lb.NetMapWithPeers()) { t.Fatal("waitTest already passes before sending initial netmap: this will be flaky") } @@ -223,7 +220,7 @@ func TestPacketFilterFromNetmap(t *testing.T) { t.Fatal("incrementalWaitTest must be set if incrementalMapResponse is set") } - if test.incrementalWaitTest(s.lb.NetMap()) { + if test.incrementalWaitTest(s.lb.NetMapWithPeers()) { t.Fatal("incrementalWaitTest already passes before sending incremental netmap: this will be flaky") } diff --git a/tsnet/tsnet_test.go b/tsnet/tsnet_test.go index 4ed20fb2a..05598ca85 100644 --- a/tsnet/tsnet_test.go +++ b/tsnet/tsnet_test.go @@ -1022,7 +1022,11 @@ func setUpServiceState(t *testing.T, name, ip string, host, client *Server, t.Helper() w := must.Get(s.localClient.WatchIPNBus(t.Context(), ipn.NotifyInitialNetMap)) defer w.Close() - for n := must.Get(w.Next()); !netmapUpToDate(n.NetMap); n = must.Get(w.Next()) { + for { + must.Get(w.Next()) + if nm := s.lb.NetMapWithPeers(); nm != nil && netmapUpToDate(nm) { + return + } } } waitForLatestNetmap(t, client) diff --git a/tstest/integration/integration_test.go b/tstest/integration/integration_test.go index 3064d6a26..b14a19c95 100644 --- a/tstest/integration/integration_test.go +++ b/tstest/integration/integration_test.go @@ -34,7 +34,6 @@ import ( "github.com/miekg/dns" "go4.org/mem" "tailscale.com/client/local" - "tailscale.com/client/tailscale" "tailscale.com/cmd/testwrapper/flakytest" "tailscale.com/feature" _ "tailscale.com/feature/clientupdate" @@ -69,6 +68,14 @@ func TestMain(m *testing.M) { os.Exit(0) } +// fetchNetMapForTest fetches the current netmap from tailscaled via the +// "current-netmap" debug action. The debug action's payload shape is +// intentionally not part of any stable API; tests use it to inspect +// internal state. +func fetchNetMapForTest(ctx context.Context, lc *local.Client) (*netmap.NetworkMap, error) { + return local.GetDebugResultJSON[*netmap.NetworkMap](ctx, lc, "current-netmap") +} + // Tests that tailscaled starts up in TUN mode, and also without data races: // https://github.com/tailscale/tailscale/issues/7894 func TestTUNMode(t *testing.T) { @@ -1189,20 +1196,18 @@ func TestClientSideJailing(t *testing.T) { if err != nil { t.Fatal(err) } - waitPeerIsJailed := func(t *testing.T, b *tailscale.IPNBusWatcher, jailed bool) { + waitPeerIsJailed := func(t *testing.T, b *local.IPNBusWatcher, lc *local.Client, jailed bool) { t.Helper() for { - n, err := b.Next() + _, err := b.Next() if err != nil { t.Fatal(err) } - if n.NetMap == nil { + nm, err := fetchNetMapForTest(context.Background(), lc) + if err != nil || nm == nil || len(nm.Peers) == 0 { continue } - if len(n.NetMap.Peers) == 0 { - continue - } - if j := n.NetMap.Peers[0].IsJailed(); j == jailed { + if j := nm.Peers[0].IsJailed(); j == jailed { break } } @@ -1213,8 +1218,8 @@ func TestClientSideJailing(t *testing.T) { env.Control.SetJailed(k2, k1, tc.n1JailedForN2) // Wait for the jailed status to propagate. - waitPeerIsJailed(t, b1, tc.n2JailedForN1) - waitPeerIsJailed(t, b2, tc.n1JailedForN2) + waitPeerIsJailed(t, b1, lc1, tc.n2JailedForN1) + waitPeerIsJailed(t, b2, lc2, tc.n1JailedForN2) testDial(t, lc1, ip2, port, tc.n1JailedForN2) testDial(t, lc2, ip1, port, tc.n2JailedForN1) diff --git a/tstest/largetailnet/largetailnet_test.go b/tstest/largetailnet/largetailnet_test.go index 07f67df82..c9ebb1532 100644 --- a/tstest/largetailnet/largetailnet_test.go +++ b/tstest/largetailnet/largetailnet_test.go @@ -40,7 +40,7 @@ var ( // processing peer-add/peer-remove deltas in steady state, with no IPN bus // subscribers attached. This represents the headless-tailscaled workload // (Linux subnet routers, container sidecars, ...) where the LocalBackend -// does not pay for fanning Notify.NetMap out to GUI watchers. +// does not pay for fanning Notify events out to GUI watchers. // // Use [BenchmarkGiantTailnetBusWatcher] for the GUI-client workload. // @@ -54,9 +54,9 @@ func BenchmarkGiantTailnet(b *testing.B) { // BenchmarkGiantTailnetBusWatcher is like [BenchmarkGiantTailnet] but // attaches one [local.Client.WatchIPNBus] subscriber for the duration of the -// benchmark. The Notify-fan-out cost (notably Notify.NetMap encoding to -// every watcher on every full-rebuild path) is therefore included in the -// per-delta measurement, which approximates the GUI-client workload. +// benchmark. The Notify-fan-out cost (per-watcher encoding done on every +// full-rebuild path) is therefore included in the per-delta measurement, +// which approximates the GUI-client workload. // // The benchmark is opt-in via --actually-test-giant-tailnet. func BenchmarkGiantTailnetBusWatcher(b *testing.B) { @@ -160,15 +160,17 @@ func benchGiantTailnet(b *testing.B, busWatcher bool) { notifyCh = make(chan struct{}, 1024) go func() { for { - n, err := bw.Next() - if err != nil { + if _, err := bw.Next(); err != nil { return } - if n.NetMap != nil || len(n.PeerChanges) > 0 { - select { - case notifyCh <- struct{}{}: - default: - } + // Any notify counts as a per-delta ack: peer add/remove + // in the delta path emits Notify.PeersChanged / + // Notify.PeersRemoved, peer patches emit + // Notify.PeerChanges, and self-node updates emit + // Notify.SelfChange. + select { + case notifyCh <- struct{}{}: + default: } } }() diff --git a/types/netmap/nodemut.go b/types/netmap/nodemut.go index 901296b1f..aa91d03ea 100644 --- a/types/netmap/nodemut.go +++ b/types/netmap/nodemut.go @@ -68,6 +68,25 @@ func (m NodeMutationLastSeen) Apply(n *tailcfg.Node) { n.LastSeen = new(m.LastSeen) } +// NodeMutationAdd is a NodeMutation that says a new peer has been added. +// Apply is a no-op: consumers of NodeMutationAdd must type-switch to handle +// adds by inserting Node into their peer map. +type NodeMutationAdd struct { + Node tailcfg.NodeView +} + +func (m NodeMutationAdd) NodeIDBeingMutated() tailcfg.NodeID { return m.Node.ID() } +func (m NodeMutationAdd) Apply(*tailcfg.Node) {} + +// NodeMutationRemove is a NodeMutation that says a peer has been removed. +// Apply is a no-op: consumers of NodeMutationRemove must type-switch to handle +// removes by deleting the node from their peer map. +type NodeMutationRemove struct { + mutatingNodeID +} + +func (m NodeMutationRemove) Apply(*tailcfg.Node) {} + var peerChangeFields = sync.OnceValue(func() []reflect.StructField { var fields []reflect.StructField rt := reflect.TypeFor[tailcfg.PeerChange]() @@ -110,8 +129,12 @@ func NodeMutationsFromPatch(p *tailcfg.PeerChange) (_ []NodeMutation, ok bool) { } // MutationsFromMapResponse returns all the discrete node mutations described -// by res. It returns ok=false if res contains any non-patch field as defined +// by res. It returns ok=false if res contains any non-delta field as defined // by mapResponseContainsNonPatchFields. +// +// Adds and removes (from res.PeersChanged / res.PeersRemoved) are emitted as +// NodeMutationAdd / NodeMutationRemove entries. Callers must type-switch to +// handle those alongside field mutations. func MutationsFromMapResponse(res *tailcfg.MapResponse, now time.Time) (ret []NodeMutation, ok bool) { if now.IsZero() { now = time.Now() @@ -119,8 +142,15 @@ func MutationsFromMapResponse(res *tailcfg.MapResponse, now time.Time) (ret []No if mapResponseContainsNonPatchFields(res) { return nil, false } - // All that remains is PeersChangedPatch, OnlineChange, and LastSeenChange. + for _, id := range res.PeersRemoved { + ret = append(ret, NodeMutationRemove{mutatingNodeID(id)}) + } + for _, n := range res.PeersChanged { + // Any n still in PeersChanged after patchifyPeersChanged is a + // truly-new (or replaced) peer. + ret = append(ret, NodeMutationAdd{Node: n.View()}) + } for _, p := range res.PeersChangedPatch { deltas, ok := NodeMutationsFromPatch(p) if !ok { @@ -142,25 +172,26 @@ func MutationsFromMapResponse(res *tailcfg.MapResponse, now time.Time) (ret []No return ret, true } -// mapResponseContainsNonPatchFields reports whether res contains only "patch" -// fields set (PeersChangedPatch primarily, but also including the legacy -// PeerSeenChange and OnlineChange fields). +// mapResponseContainsNonPatchFields reports whether res contains any field +// that can't be expressed as a per-peer NodeMutation (including the new +// NodeMutationAdd / NodeMutationRemove variants) or via the sibling narrow +// setter methods on the map-session backend (e.g. UpdatePacketFilter). // -// It ignores any of the meta fields that are handled by PollNetMap before the -// peer change handling gets involved. +// When this returns true, the caller must fall back to rebuilding and +// dispatching a full NetworkMap. When it returns false, the response can be +// handled incrementally. // -// The purpose of this function is to ask whether this is a tricky enough -// MapResponse to warrant a full netmap update. When this returns false, it -// means the response can be handled incrementally, patching up the local state. +// PeersChanged, PeersRemoved, and PacketFilter(s) are intentionally not in +// this list: new/removed peers ride NodeMutationAdd/Remove, packet +// filter updates are delivered via the backend's UpdatePacketFilter +// method, and UserProfile updates ride the backend's UpdateUserProfiles +// method. func mapResponseContainsNonPatchFields(res *tailcfg.MapResponse) bool { return res.Node != nil || res.DERPMap != nil || res.DNSConfig != nil || res.Domain != "" || res.CollectServices != "" || - res.PacketFilter != nil || - res.PacketFilters != nil || - res.UserProfiles != nil || res.Health != nil || res.DisplayMessages != nil || res.SSHPolicy != nil || @@ -170,11 +201,5 @@ func mapResponseContainsNonPatchFields(res *tailcfg.MapResponse) bool { res.ControlDialPlan != nil || res.ClientVersion != nil || res.Peers != nil || - res.PeersRemoved != nil || - // PeersChanged is too coarse to be considered a patch. Also, we convert - // PeersChanged to PeersChangedPatch in patchifyPeersChanged before this - // function is called, so it should never be set anyway. But for - // completedness, and for tests, check it too: - res.PeersChanged != nil || res.DeprecatedDefaultAutoUpdate != "" } diff --git a/types/netmap/nodemut_test.go b/types/netmap/nodemut_test.go index 1ae2ab1f9..965e9a367 100644 --- a/types/netmap/nodemut_test.go +++ b/types/netmap/nodemut_test.go @@ -52,7 +52,16 @@ func TestMapResponseContainsNonPatchFields(t *testing.T) { // They should be ignored. want = false case "PeersChangedPatch", "PeerSeenChange", "OnlineChange": - // The actual three delta fields we care about handling. + // The three legacy delta fields handled via NodeMutation patches. + want = false + case "PeersChanged", "PeersRemoved": + // Now carried as NodeMutationAdd / NodeMutationRemove entries. + want = false + case "PacketFilter", "PacketFilters": + // Now delivered separately via PacketFilterUpdater. + want = false + case "UserProfiles": + // Now delivered separately via UserProfileUpdater. want = false default: // Everything else should be conseratively handled as a @@ -175,6 +184,36 @@ func TestMutationsFromMapResponse(t *testing.T) { }, want: nil, }, + { + name: "peer-removed", + mr: &tailcfg.MapResponse{ + PeersRemoved: []tailcfg.NodeID{5}, + }, + want: muts(NodeMutationRemove{5}), + }, + { + name: "peer-added", + mr: &tailcfg.MapResponse{ + PeersChanged: []*tailcfg.Node{{ID: 7}}, + }, + want: muts(NodeMutationAdd{Node: (&tailcfg.Node{ID: 7}).View()}), + }, + { + name: "add-and-remove-mixed-with-patch", + mr: &tailcfg.MapResponse{ + PeersRemoved: []tailcfg.NodeID{3}, + PeersChanged: []*tailcfg.Node{{ID: 7}}, + PeersChangedPatch: []*tailcfg.PeerChange{{ + NodeID: 5, + DERPRegion: 2, + }}, + }, + want: muts( + NodeMutationRemove{3}, + NodeMutationDERPHome{5, 2}, + NodeMutationAdd{Node: (&tailcfg.Node{ID: 7}).View()}, + ), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -188,11 +227,13 @@ func TestMutationsFromMapResponse(t *testing.T) { if diff := cmp.Diff(tt.want, got, cmp.Comparer(func(a, b netip.Addr) bool { return a == b }), cmp.Comparer(func(a, b netip.AddrPort) bool { return a == b }), + cmp.Comparer(func(a, b tailcfg.NodeView) bool { return a.ID() == b.ID() }), cmp.AllowUnexported( NodeMutationEndpoints{}, NodeMutationDERPHome{}, NodeMutationOnline{}, NodeMutationLastSeen{}, + NodeMutationRemove{}, )); diff != "" { t.Errorf("wrong result (-want +got):\n%s", diff) } diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 9720f57cd..026f744ae 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -3204,7 +3204,7 @@ func (c *Conn) UpsertPeer(n tailcfg.NodeView) { return } flags := c.debugFlagsLocked() - c.peersByID[n.ID()] = n + mak.Set(&c.peersByID, n.ID(), n) c.upsertPeerLocked(n, flags, debugRingBufferSize(len(c.peersByID))) var relayUpsert candidatePeerRelay