ipn, ipn/ipnlocal: remove legacy runtime Notify.NetMap emission, rate limiting

The WinUI GUI was the last consumer of the legacy Notify.NetMap field
on runtime (non-initial) IPN bus messages. It was converted to peer
deltas in tailscale/corp#47958, and the GUI and tailscaled ship
together as a unit on Windows, so tailscaled no longer needs to build
and emit full netmaps on the bus on any platform. Remove
goosGetsLegacyNetmapNotify and the code it gated. The initial netmap
(NotifyInitialNetMap) is unaffected; NotifyNoNetMap is now a no-op but
remains accepted for compatibility.

With no runtime netmaps left to rate limit, the NotifyRateLimit
subscription bit is meaningless, so remove the rateLimitingBusSender
machinery too. The bit value stays reserved under the name
ObsoleteNotifyRateLimit and ValidateNotifyWatchOpt now rejects
subscriptions that set it; previously it was only rejected in
combination with new-style delta bits.

Updates #12542

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: Ic6184ea549726de9d9d56f133aae21e53f62cbc5

ipn/backend.go

@@ -4,6 +4,7 @@
 package ipn
 
 import (
+	"errors"
 	"fmt"
 	"slices"
 	"strconv"
@@ -95,7 +96,14 @@ const (
 
 	NotifyInitialHealthState NotifyWatchOpt = 1 << 7 // if set, the first Notify message (sent immediately) will contain the current health.State of the client
 
-	NotifyRateLimit NotifyWatchOpt = 1 << 8 // if set, rate limit spammy netmap updates to every few seconds
+	// ObsoleteNotifyRateLimit is a formerly valid bit (previously named
+	// NotifyRateLimit) that asked tailscaled to rate limit spammy netmap
+	// updates to every few seconds. It became meaningless once tailscaled
+	// stopped emitting [Notify.NetMap] on runtime (non-initial) bus
+	// messages, so subscription requests that set it are now rejected by
+	// [ValidateNotifyWatchOpt]. The bit value remains reserved so it is
+	// never reused with a different meaning.
+	ObsoleteNotifyRateLimit NotifyWatchOpt = 1 << 8
 
 	NotifyHealthActions NotifyWatchOpt = 1 << 9 // if set, include PrimaryActions in health.State. Otherwise append the action URL to the text
 
@@ -109,7 +117,7 @@ const (
 	//
 	// 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]).
+	// demand).
 	//
 	// Watchers that want narrower per-field updates as well (Online,
 	// LastSeen, DERPHome, Endpoints) should additionally set
@@ -120,19 +128,18 @@ const (
 	// observes every per-peer mutation; it just receives them as full
 	// Nodes rather than narrow patches. The cost is bus bandwidth.
 	//
-	// On platforms where the legacy [Notify.NetMap] is still emitted
-	// (Windows, macOS, iOS, Android), it is permitted to combine this
-	// with [NotifyInitialNetMap] for backwards compatibility. New code
-	// should pair this with [NotifyInitialStatus] instead.
+	// It is permitted to combine this with [NotifyInitialNetMap] for
+	// backwards compatibility. New code 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 historically suppressed the legacy [Notify.NetMap]
+	// field on runtime (non-initial) Notify messages on platforms where
+	// tailscaled still emitted it by default. tailscaled no longer emits
+	// NetMap on runtime messages on any platform, so this bit is now a
+	// no-op. It remains accepted for compatibility with clients that set
+	// it. The initial-state NetMap (sent when [NotifyInitialNetMap] is
+	// set) was never affected by this bit.
 	NotifyNoNetMap NotifyWatchOpt = 1 << 13
 
 	// NotifyInitialStatus, if set, causes the first Notify message (sent
@@ -224,7 +231,7 @@ func (o NotifyWatchOpt) String() string {
 	try(NotifyInitialDriveShares, "NotifyInitialDriveShares")
 	try(NotifyInitialOutgoingFiles, "NotifyInitialOutgoingFiles")
 	try(NotifyInitialHealthState, "NotifyInitialHealthState")
-	try(NotifyRateLimit, "NotifyRateLimit")
+	try(ObsoleteNotifyRateLimit, "ObsoleteNotifyRateLimit")
 	try(NotifyHealthActions, "NotifyHealthActions")
 	try(NotifyInitialSuggestedExitNode, "NotifyInitialSuggestedExitNode")
 	try(NotifyInitialClientVersion, "NotifyInitialClientVersion")
@@ -270,21 +277,11 @@ func (o *NotifyWatchOpt) UnmarshalText(text []byte) error {
 	return nil
 }
 
-// NotifyRateLimitIncompatibleBits is the set of new-style IPN bus
-// subscription bits that cannot be combined with [NotifyRateLimit].
-//
-// Those bits describe stateful delta streams. Randomly delaying or merging
-// messages in those streams would break the consumer's ability to maintain a
-// coherent local view.
-const NotifyRateLimitIncompatibleBits = NotifyPeerChanges | NotifyNoNetMap | NotifyInitialStatus | NotifyPeerPatches
-
 // ValidateNotifyWatchOpt reports whether mask is a valid WatchIPNBus
 // subscription mask.
 func ValidateNotifyWatchOpt(mask NotifyWatchOpt) error {
-	if mask&NotifyRateLimit != 0 {
-		if bad := mask & NotifyRateLimitIncompatibleBits; bad != 0 {
-			return fmt.Errorf("NotifyRateLimit is incompatible with new-style IPN bus subscription bits %v", bad)
-		}
+	if mask&ObsoleteNotifyRateLimit != 0 {
+		return errors.New("the NotifyRateLimit IPN bus subscription bit is no longer supported")
 	}
 	return nil
 }
@@ -333,22 +330,16 @@ type Notify struct {
 	// 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:
+	// NetMap, if non-nil, is the full network map. It is only delivered
+	// on the initial Notify of a session, and only when the watcher
+	// requested [NotifyInitialNetMap]; it is always nil on subsequent
+	// messages. New consumers should prefer [LocalClient.NetMap] for
+	// one-shot fetches and [Notify.SelfChange] / [Notify.PeerChanges]
+	// for incremental reactive updates.
 	//
-	//   - On the initial Notify if the watcher requested
-	//     [NotifyInitialNetMap] (any platform).
-	//   - On subsequent Notify messages, only when tailscaled is running
-	//     on Windows. On all other platforms it is always nil after the
-	//     initial notify.
-	//
-	// Deprecated: this field is only populated on Windows and
-	// is slated for removal in favor of [Notify.InitialStatus] +
-	// [Notify.SelfChange] / [Notify.PeerChanges], etc, as this field
-	// doesn't scale.
+	// Deprecated: this field 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

ipn/backend_test.go

@@ -93,11 +93,7 @@ func TestValidateNotifyWatchOpt(t *testing.T) {
 		wantErr bool
 	}{
 		{
-			name: "legacy-rate-limit-only",
-			mask: NotifyRateLimit,
-		},
-		{
-			name: "peer-changes-without-rate-limit",
+			name: "peer-changes",
 			mask: NotifyPeerChanges | NotifyPeerPatches | NotifyNoNetMap | NotifyInitialStatus,
 		},
 		{
@@ -105,23 +101,13 @@ func TestValidateNotifyWatchOpt(t *testing.T) {
 			mask: NotifyInProcessNoDisconnect | NotifyPeerChanges,
 		},
 		{
-			name:    "rate-limit-with-peer-changes",
-			mask:    NotifyRateLimit | NotifyPeerChanges,
+			name:    "obsolete-rate-limit-only",
+			mask:    ObsoleteNotifyRateLimit,
 			wantErr: true,
 		},
 		{
-			name:    "rate-limit-with-peer-patches",
-			mask:    NotifyRateLimit | NotifyPeerPatches,
-			wantErr: true,
-		},
-		{
-			name:    "rate-limit-with-no-netmap",
-			mask:    NotifyRateLimit | NotifyNoNetMap,
-			wantErr: true,
-		},
-		{
-			name:    "rate-limit-with-initial-status",
-			mask:    NotifyRateLimit | NotifyInitialStatus,
+			name:    "obsolete-rate-limit-with-peer-changes",
+			mask:    ObsoleteNotifyRateLimit | NotifyPeerChanges,
 			wantErr: true,
 		},
 	}

ipn/ipnlocal/bus.go

@@ -1,249 +0,0 @@
-// Copyright (c) Tailscale Inc & contributors
-// SPDX-License-Identifier: BSD-3-Clause
-
-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"
-
-type rateLimitingBusSender struct {
-	fn              func(*ipn.Notify) (keepGoing bool)
-	lastFlush       time.Time           // last call to fn, or zero value if none
-	interval        time.Duration       // 0 to flush immediately; non-zero to rate limit sends
-	clock           tstime.DefaultClock // non-nil for testing
-	didSendTestHook func()              // non-nil for testing
-
-	// pending, if non-nil, is the pending notification that we
-	// haven't sent yet. We own this memory to mutate.
-	pending *ipn.Notify
-
-	// flushTimer is non-nil if the timer is armed.
-	flushTimer  tstime.TimerController // effectively a *time.Timer
-	flushTimerC <-chan time.Time       // ... said ~Timer's C chan
-}
-
-func (s *rateLimitingBusSender) close() {
-	if s.flushTimer != nil {
-		s.flushTimer.Stop()
-	}
-}
-
-func (s *rateLimitingBusSender) flushChan() <-chan time.Time {
-	return s.flushTimerC
-}
-
-func (s *rateLimitingBusSender) flush() (keepGoing bool) {
-	if n := s.pending; n != nil {
-		s.pending = nil
-		return s.flushNotify(n)
-	}
-	return true
-}
-
-func (s *rateLimitingBusSender) flushNotify(n *ipn.Notify) (keepGoing bool) {
-	s.lastFlush = s.clock.Now()
-	return s.fn(n)
-}
-
-// send conditionally sends n to the underlying fn, possibly rate
-// limiting it, depending on whether s.interval is set, and whether
-// n is a notable notification that the client (typically a GUI) would
-// want to act on (render) immediately.
-//
-// It returns whether the caller should keep looping.
-//
-// The passed-in memory 'n' is owned by the caller and should
-// not be mutated.
-func (s *rateLimitingBusSender) send(n *ipn.Notify) (keepGoing bool) {
-	if s.interval <= 0 {
-		// No rate limiting case.
-		return s.fn(n)
-	}
-	if isNotableNotify(n) {
-		// Notable notifications are always sent immediately.
-		// But first send any boring one that was pending.
-		// TODO(bradfitz): there might be a boring one pending
-		// with a NetMap or Engine field that is redundant
-		// with the new one (n) with NetMap or Engine populated.
-		// We should clear the pending one's NetMap/Engine in
-		// that case. Or really, merge the two, but mergeBoringNotifies
-		// only handles the case of both sides being boring.
-		// So for now, flush both.
-		if !s.flush() {
-			return false
-		}
-		return s.flushNotify(n)
-	}
-	s.pending = mergeBoringNotifies(s.pending, n)
-	d := s.clock.Now().Sub(s.lastFlush)
-	if d > s.interval {
-		return s.flush()
-	}
-	nextFlushIn := s.interval - d
-	if s.flushTimer == nil {
-		s.flushTimer, s.flushTimerC = s.clock.NewTimer(nextFlushIn)
-	} else {
-		s.flushTimer.Reset(nextFlushIn)
-	}
-	return true
-}
-
-func (s *rateLimitingBusSender) Run(ctx context.Context, ch <-chan *ipn.Notify) {
-	for {
-		select {
-		case <-ctx.Done():
-			return
-		case n, ok := <-ch:
-			if !ok {
-				return
-			}
-			if !s.send(n) {
-				return
-			}
-			if f := s.didSendTestHook; f != nil {
-				f()
-			}
-		case <-s.flushChan():
-			if !s.flush() {
-				return
-			}
-		}
-	}
-}
-
-// mergeBoringNotify merges new notify src into possibly-nil dst,
-// either mutating dst or allocating a new one if dst is nil,
-// returning the merged result.
-//
-// dst and src must both be "boring" (i.e. not notable per isNotifiableNotify).
-func mergeBoringNotifies(dst, src *ipn.Notify) *ipn.Notify {
-	if dst == nil {
-		dst = &ipn.Notify{Version: src.Version}
-	}
-	if goosGetsLegacyNetmapNotify && src.NetMap != nil {
-		// Full netmap supersedes any accumulated peer-change deltas.
-		dst.NetMap = src.NetMap
-		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
-	}
-	return dst
-}
-
-// 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
-	}
-
-	for _, nd := range src {
-		if oi, ok := idxByNode[nd.NodeID]; ok {
-			dst[oi] = mergePeerChangeForIpnBus(dst[oi], nd)
-			continue
-		}
-		idxByNode[nd.NodeID] = len(dst)
-		dst = append(dst, nd)
-	}
-	return dst
-}
-
-// mergePeerChangeForIpnBus merges new with old, returning the result.
-// Fields set in new override those in old; fields only set in old are preserved.
-func mergePeerChangeForIpnBus(old, new *tailcfg.PeerChange) *tailcfg.PeerChange {
-	merged := *old
-
-	// This is a subset of PeerChange that reflects only the fields that can
-	// be changed via a NodeMutation.  If future fields can be updated via
-	// NodeMutations from map responses (and they are relevant to the ipn bus), then
-	// they should be added here and merged in the same way.
-	if new.DERPRegion != 0 {
-		// netmap.NodeMutationDerpHome
-		merged.DERPRegion = new.DERPRegion
-	}
-	if new.Online != nil {
-		// netmap.NodeMutationOnline
-		merged.Online = new.Online
-	}
-	if new.LastSeen != nil {
-		// netmap.NodeMutationLastSeen
-		merged.LastSeen = new.LastSeen
-	}
-	if new.Endpoints != nil {
-		// netmap.NodeMutationEndpoints
-		merged.Endpoints = new.Endpoints
-	}
-
-	return &merged
-}
-
-// isNotableNotify reports whether n is a "notable" notification that
-// should be sent on the IPN bus immediately (e.g. to GUIs) without
-// rate limiting it for a few seconds.
-//
-// This is only used for legacy [ipn.NotifyRateLimit] subscribers. New-style
-// subscriptions that receive delta streams are rejected by
-// [ipn.ValidateNotifyWatchOpt] when combined with NotifyRateLimit.
-//
-// Legacy NetMap and Engine are the only "boring" (rate-limitable) fields.
-func isNotableNotify(n *ipn.Notify) bool {
-	if n == nil {
-		return false
-	}
-	return n.State != nil ||
-		n.SessionID != "" ||
-		n.BrowseToURL != nil ||
-		n.LocalTCPPort != nil ||
-		n.ClientVersion != nil ||
-		n.Prefs != nil ||
-		n.ErrMessage != nil ||
-		n.LoginFinished != nil ||
-		n.SelfChange != nil ||
-		n.InitialStatus != nil ||
-		len(n.PeerChangedPatch) > 0 ||
-		len(n.PeersChanged) > 0 ||
-		len(n.PeersRemoved) > 0 ||
-		len(n.UserProfiles) > 0 ||
-		len(n.PeerState) > 0 ||
-		!n.DriveShares.IsNil() ||
-		n.Health != nil ||
-		len(n.IncomingFiles) > 0 ||
-		len(n.OutgoingFiles) > 0 ||
-		n.FilesWaiting != nil ||
-		n.SuggestedExitNode != nil ||
-		n.Policy != nil
-}

ipn/ipnlocal/bus_test.go

@@ -1,329 +0,0 @@
-// Copyright (c) Tailscale Inc & contributors
-// SPDX-License-Identifier: BSD-3-Clause
-
-package ipnlocal
-
-import (
-	"context"
-	"reflect"
-	"slices"
-	"testing"
-	"time"
-
-	"tailscale.com/drive"
-	"tailscale.com/ipn"
-	"tailscale.com/tailcfg"
-	"tailscale.com/tstest"
-	"tailscale.com/tstime"
-	"tailscale.com/types/logger"
-	"tailscale.com/types/netmap"
-	"tailscale.com/types/views"
-)
-
-func TestIsNotableNotify(t *testing.T) {
-	tests := []struct {
-		name   string
-		notify *ipn.Notify
-		want   bool
-	}{
-		{"nil", nil, false},
-		{"empty", &ipn.Notify{}, false},
-		{"version", &ipn.Notify{Version: "foo"}, false},
-		{"netmap", &ipn.Notify{NetMap: new(netmap.NetworkMap)}, false},
-		{"peerchanges", &ipn.Notify{PeerChangedPatch: []*tailcfg.PeerChange{{}}}, true},
-		{"peerschanged", &ipn.Notify{PeersChanged: []*tailcfg.Node{{}}}, true},
-		{"peersremoved", &ipn.Notify{PeersRemoved: []tailcfg.NodeID{1}}, true},
-		{"userprofiles", &ipn.Notify{UserProfiles: map[tailcfg.UserID]tailcfg.UserProfileView{1: (&tailcfg.UserProfile{}).View()}}, true},
-		{"engine", &ipn.Notify{Engine: new(ipn.EngineStatus)}, false},
-		{"peerstate", &ipn.Notify{PeerState: map[tailcfg.StableNodeID]ipn.PeerState{"a": {PeerWireGuardState: ipn.PeerWireGuardStateHandshake}}}, true},
-		{"selfchange", &ipn.Notify{SelfChange: &tailcfg.Node{}}, true},
-	}
-
-	// Then for all other fields, assume they're notable.
-	// We use reflect to catch fields that might be added in the future without
-	// remembering to update the [isNotableNotify] function.
-	rt := reflect.TypeFor[ipn.Notify]()
-	for sf := range rt.Fields() {
-		n := &ipn.Notify{}
-		switch sf.Name {
-		case "_", "NetMap", "PeerChangedPatch", "SelfChange", "PeersChanged", "PeersRemoved", "UserProfiles", "Engine", "PeerState", "Version":
-			// Already covered above or not applicable.
-			continue
-		case "DriveShares":
-			n.DriveShares = views.SliceOfViews[*drive.Share, drive.ShareView](make([]*drive.Share, 1))
-		default:
-			rf := reflect.ValueOf(n).Elem().FieldByIndex(sf.Index)
-			switch rf.Kind() {
-			case reflect.Pointer:
-				rf.Set(reflect.New(rf.Type().Elem()))
-			case reflect.String:
-				rf.SetString("foo")
-			case reflect.Slice:
-				rf.Set(reflect.MakeSlice(rf.Type(), 1, 1))
-			default:
-				t.Errorf("unhandled field kind %v for %q", rf.Kind(), sf.Name)
-			}
-		}
-
-		tests = append(tests, struct {
-			name   string
-			notify *ipn.Notify
-			want   bool
-		}{
-			name:   "field-" + sf.Name,
-			notify: n,
-			want:   true,
-		})
-	}
-
-	for _, tt := range tests {
-		if got := isNotableNotify(tt.notify); got != tt.want {
-			t.Errorf("%v: got %v; want %v", tt.name, got, tt.want)
-		}
-	}
-}
-
-type rateLimitingBusSenderTester struct {
-	tb    testing.TB
-	got   []*ipn.Notify
-	clock *tstest.Clock
-	s     *rateLimitingBusSender
-}
-
-func (st *rateLimitingBusSenderTester) init() {
-	if st.s != nil {
-		return
-	}
-	st.clock = tstest.NewClock(tstest.ClockOpts{
-		Start: time.Unix(1731777537, 0), // time I wrote this test :)
-	})
-	st.s = &rateLimitingBusSender{
-		clock: tstime.DefaultClock{Clock: st.clock},
-		fn: func(n *ipn.Notify) bool {
-			st.got = append(st.got, n)
-			return true
-		},
-	}
-}
-
-func (st *rateLimitingBusSenderTester) send(n *ipn.Notify) {
-	st.tb.Helper()
-	st.init()
-	if !st.s.send(n) {
-		st.tb.Fatal("unexpected send failed")
-	}
-}
-
-func (st *rateLimitingBusSenderTester) advance(d time.Duration) {
-	st.tb.Helper()
-	st.clock.Advance(d)
-	select {
-	case <-st.s.flushChan():
-		if !st.s.flush() {
-			st.tb.Fatal("unexpected flush failed")
-		}
-	default:
-	}
-}
-
-func TestRateLimitingBusSender(t *testing.T) {
-	ver1 := &ipn.Notify{Version: "1"}
-	ver2 := &ipn.Notify{Version: "2"}
-	eng1 := &ipn.Notify{Engine: new(ipn.EngineStatus)}
-	eng2 := &ipn.Notify{Engine: new(ipn.EngineStatus)}
-
-	t.Run("unbuffered", func(t *testing.T) {
-		st := &rateLimitingBusSenderTester{tb: t}
-		st.send(ver1)
-		st.send(ver2)
-		st.send(eng1)
-		st.send(eng2)
-		if !slices.Equal(st.got, []*ipn.Notify{ver1, ver2, eng1, eng2}) {
-			t.Errorf("got %d items; want 4 specific ones, unmodified", len(st.got))
-		}
-	})
-
-	t.Run("buffered", func(t *testing.T) {
-		st := &rateLimitingBusSenderTester{tb: t}
-		st.init()
-		st.s.interval = 1 * time.Second
-		st.send(&ipn.Notify{Version: "initial"})
-		if len(st.got) != 1 {
-			t.Fatalf("got %d items; expected 1 (first to flush immediately)", len(st.got))
-		}
-		st.send(ver1)
-		st.send(ver2)
-		st.send(eng1)
-		st.send(eng2)
-		if len(st.got) != 1 {
-			if len(st.got) != 1 {
-				t.Fatalf("got %d items; expected still just that first 1", len(st.got))
-			}
-		}
-
-		// But moving the clock should flush the rest, collasced into one new one.
-		st.advance(5 * time.Second)
-		if len(st.got) != 2 {
-			t.Fatalf("got %d items; want 2", len(st.got))
-		}
-		gotn := st.got[1]
-		if gotn.Version != ver1.Version {
-			t.Errorf("got wrong Version; got %q want %q", gotn.Version, ver1.Version)
-		}
-		if gotn.Engine != eng2.Engine {
-			t.Errorf("got wrong Engine; got %p", gotn.Engine)
-		}
-		if t.Failed() {
-			t.Logf("failed Notify was: %v", logger.AsJSON(gotn))
-		}
-	})
-
-	// Test the Run method
-	t.Run("run", func(t *testing.T) {
-		st := &rateLimitingBusSenderTester{tb: t}
-		st.init()
-		st.s.interval = 1 * time.Second
-		st.s.lastFlush = st.clock.Now() // pretend we just flushed
-
-		flushc := make(chan *ipn.Notify, 1)
-		st.s.fn = func(n *ipn.Notify) bool {
-			flushc <- n
-			return true
-		}
-		didSend := make(chan bool, 2)
-		st.s.didSendTestHook = func() { didSend <- true }
-		waitSend := func() {
-			select {
-			case <-didSend:
-			case <-time.After(5 * time.Second):
-				t.Error("timeout waiting for call to send")
-			}
-		}
-
-		ctx, cancel := context.WithCancel(context.Background())
-		defer cancel()
-
-		incoming := make(chan *ipn.Notify, 2)
-		go func() {
-			incoming <- ver1
-			waitSend()
-			incoming <- eng2
-			waitSend()
-			st.advance(5 * time.Second)
-			select {
-			case n := <-flushc:
-				if n.Version != ver1.Version {
-					t.Errorf("got wrong Version; got %q want %q", n.Version, ver1.Version)
-				}
-				if n.Engine != eng2.Engine {
-					t.Errorf("got wrong Engine; got %p", n.Engine)
-				}
-			case <-time.After(10 * time.Second):
-				t.Error("timeout")
-			}
-			cancel()
-		}()
-
-		st.s.Run(ctx, incoming)
-	})
-}
-
-func TestMergePeerChangedPatch(t *testing.T) {
-	online := true
-	offline := false
-
-	t.Run("no_overlap_appends", func(t *testing.T) {
-		old := []*tailcfg.PeerChange{
-			{NodeID: 1, DERPRegion: 1},
-		}
-		new := []*tailcfg.PeerChange{
-			{NodeID: 2, DERPRegion: 2},
-		}
-		got := mergePeerChangedPatch(old, new)
-		if len(got) != 2 {
-			t.Fatalf("len = %d; want 2", len(got))
-		}
-		if got[0].NodeID != 1 || got[1].NodeID != 2 {
-			t.Errorf("got NodeIDs %d, %d; want 1, 2", got[0].NodeID, got[1].NodeID)
-		}
-	})
-
-	t.Run("overlap_merges", func(t *testing.T) {
-		old := []*tailcfg.PeerChange{
-			{NodeID: 1, DERPRegion: 1, Online: &online},
-			{NodeID: 2, DERPRegion: 10},
-		}
-		new := []*tailcfg.PeerChange{
-			{NodeID: 1, DERPRegion: 5, Online: &offline},
-		}
-		got := mergePeerChangedPatch(old, new)
-		if len(got) != 2 {
-			t.Fatalf("len = %d; want 2 (merged, not appended)", len(got))
-		}
-		if got[0].DERPRegion != 5 {
-			t.Errorf("DERPRegion = %d; want 5 (from new)", got[0].DERPRegion)
-		}
-		if *got[0].Online != false {
-			t.Errorf("Online = %v; want false (from new)", *got[0].Online)
-		}
-		// Node 2 should be untouched.
-		if got[1].NodeID != 2 || got[1].DERPRegion != 10 {
-			t.Errorf("node 2 was modified unexpectedly")
-		}
-	})
-
-	t.Run("partial_overlap_merges_and_appends", func(t *testing.T) {
-		old := []*tailcfg.PeerChange{
-			{NodeID: 1, DERPRegion: 1},
-		}
-		new := []*tailcfg.PeerChange{
-			{NodeID: 1, DERPRegion: 2},
-			{NodeID: 3, DERPRegion: 30},
-		}
-		got := mergePeerChangedPatch(old, new)
-		if len(got) != 2 {
-			t.Fatalf("len = %d; want 2", len(got))
-		}
-		if got[0].NodeID != 1 || got[0].DERPRegion != 2 {
-			t.Errorf("node 1: DERPRegion = %d; want 2", got[0].DERPRegion)
-		}
-		if got[1].NodeID != 3 || got[1].DERPRegion != 30 {
-			t.Errorf("node 3: DERPRegion = %d; want 30", got[1].DERPRegion)
-		}
-	})
-
-	t.Run("preserves_old_fields_on_merge", func(t *testing.T) {
-		old := []*tailcfg.PeerChange{
-			{NodeID: 1, DERPRegion: 1, Online: &online, Cap: 10},
-		}
-		new := []*tailcfg.PeerChange{
-			{NodeID: 1, Online: &offline},
-		}
-		got := mergePeerChangedPatch(old, new)
-		if len(got) != 1 {
-			t.Fatalf("len = %d; want 1", len(got))
-		}
-		if got[0].DERPRegion != 1 {
-			t.Errorf("DERPRegion = %d; want 1 (preserved from old)", got[0].DERPRegion)
-		}
-		if got[0].Cap != 10 {
-			t.Errorf("Cap = %d; want 10 (preserved from old)", got[0].Cap)
-		}
-		if *got[0].Online != false {
-			t.Errorf("Online = %v; want false (from new)", *got[0].Online)
-		}
-	})
-
-	t.Run("nil_old", func(t *testing.T) {
-		new := []*tailcfg.PeerChange{
-			{NodeID: 1, DERPRegion: 1},
-		}
-		got := mergePeerChangedPatch(nil, new)
-		if len(got) != 1 {
-			t.Fatalf("len = %d; want 1", len(got))
-		}
-		if got[0].NodeID != 1 {
-			t.Errorf("NodeID = %d; want 1", got[0].NodeID)
-		}
-	})
-}

ipn/ipnlocal/local.go

@@ -2085,9 +2085,6 @@ func (b *LocalBackend) setControlClientStatusLocked(c controlclient.Client, st c
 				notify.PeersChanged = append(notify.PeersChanged, p.AsStruct())
 			}
 		}
-		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
@@ -2595,9 +2592,6 @@ func (b *LocalBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bo
 		} 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
@@ -3886,14 +3880,16 @@ func (b *LocalBackend) WatchNotificationsAs(ctx context.Context, actor ipnauth.A
 	// TODO(marwan-at-work): streaming background logs?
 	defer b.DeleteForegroundSession(sessionID)
 
-	sender := &rateLimitingBusSender{fn: fn}
-	defer sender.close()
-
-	if mask&ipn.NotifyRateLimit != 0 {
-		sender.interval = 3 * time.Second
+	for {
+		select {
+		case <-ctx.Done():
+			return
+		case n, ok := <-ch:
+			if !ok || !fn(n) {
+				return
+			}
+		}
 	}
-
-	sender.Run(ctx, ch)
 }
 
 // appendHealthActions returns an IPN listener func that wraps the supplied IPN
@@ -4150,7 +4146,6 @@ func (b *LocalBackend) notifyForSessionLocked(sess *watchSession, n *ipn.Notify)
 	// 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
@@ -4185,13 +4180,10 @@ func (b *LocalBackend) notifyForSessionLocked(sess *watchSession, n *ipn.Notify)
 	}
 	replaceUserProfiles := !stripUserProfiles && len(sessUserProfiles) != len(n.UserProfiles)
 
-	if !stripNetMap && !stripPeersChanged && !stripPeersRemoved && !stripPatches && !stripPeerState && !stripUserProfiles && !replaceUserProfiles && !promotePatches {
+	if !stripPeersChanged && !stripPeersRemoved && !stripPatches && !stripPeerState && !stripUserProfiles && !replaceUserProfiles && !promotePatches {
 		return n
 	}
 	nCopy := *n
-	if stripNetMap {
-		nCopy.NetMap = nil
-	}
 	if stripPeersChanged {
 		nCopy.PeersChanged = nil
 	}

ipn/ipnlocal/node_backend.go

@@ -154,13 +154,14 @@ type nodeBackend struct {
 	// by mergeUserProfiles as deltas arrive. It parallels the peers map:
 	// netMap.UserProfiles is the frozen snapshot from the last full install,
 	// while this field reflects incremental updates. Readers that need a
-	// snapshot (e.g. the legacy Notify.NetMap path) must clone this map.
+	// snapshot (e.g. the initial-Notify NetMap or [LocalClient.NetMap]
+	// paths) must clone this map.
 	userProfiles map[tailcfg.UserID]tailcfg.UserProfileView
 
 	// packetFilterRules and packetFilter are the live packet filter state,
 	// updated by setPacketFilter as deltas arrive. Like userProfiles, they
 	// exist separately from netMap's frozen fields so that concurrent
-	// JSON-encoding of a Notify.NetMap snapshot doesn't race with writes.
+	// JSON-encoding of a NetMap snapshot doesn't race with writes.
 	packetFilterRules views.Slice[tailcfg.FilterRule]
 	packetFilter      []filter.Match
 

ipn/localapi/localapi.go

@@ -912,10 +912,8 @@ func (h *Handler) serveWatchIPNBus(w http.ResponseWriter, r *http.Request) {
 	}
 	// NotifyInitialNetMap is permitted alongside NotifyPeerChanges /
 	// NotifyPeerPatches for backwards compatibility with clients that
-	// set both (e.g. the Apple client). On platforms where
-	// goosGetsLegacyNetmapNotify is true, the initial netmap is
-	// delivered regardless; peer-change subscribers simply receive
-	// deltas after that point.
+	// set both (e.g. the Apple client). The initial netmap is a one-shot;
+	// peer-change subscribers simply receive deltas after that point.
 
 	w.Header().Set("Content-Type", "application/json")
 	ctx := r.Context()

ipn/localapi/localapi_test.go

@@ -657,9 +657,9 @@ func TestServeWatchIPNBus(t *testing.T) {
 			wantStatus:  http.StatusOK,
 		},
 		{
-			desc:       "invalid-rate-limit-mask",
+			desc:       "obsolete-rate-limit-mask",
 			permitRead: true,
-			mask:       ipn.NotifyRateLimit | ipn.NotifyPeerChanges,
+			mask:       ipn.ObsoleteNotifyRateLimit,
 			wantStatus: http.StatusBadRequest,
 		},
 		{