diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index e3c5b03fb..b0d6f630c 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -2579,10 +2579,7 @@ func (b *LocalBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bo // Reaching here, apply any peer changes to the netmap cache (if relevant). // Note we do this AFTER the updates are applied in the nodeBackend, so that // we can get its updated views to put back into the cache. - if buildfeatures.HasCacheNetMap && - cn.SelfHasCap(nodecap.CacheNetworkMaps) && - envknob.BoolDefaultTrue("TS_USE_CACHED_NETMAP") { - + if shouldCacheNetmap(cn) { var peersToUpdate []tailcfg.NodeView for id := range updateIDs { if n, ok := cn.NodeByID(id); ok { @@ -3191,10 +3188,10 @@ func (b *LocalBackend) startLocked(opts ipn.Options) error { // At this point we do not yet know whether we are meant to cache netmaps by // policy (as we have not yet spoken to the control plane). // - // However, since we do not create or update a netmap cache unless we observe the - // [tailcfg.NodeAttrCacheNetworkMaps] capability, we can use the presence - // of the cached netmap as a signal that we were expected to do so as of the - // last time we updated the cache. + // However, since we do not create or update a netmap cache unless we observe + // the [nodecap.CacheNetworkMaps] capability or the force-cache + // envknob, we can use the presence of the cached netmap as a signal that we + // were expected to do so as of the last time we updated the cache. // // If the policy has (since) changed, a subsequent network map from the control // plane may remove the attribute, at which point we will drop the cache. @@ -7468,9 +7465,10 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) { // now (if configured) update the cache. We do this after application to // reduce the chance we will cache a QoD netmap. // - // As of 2026-03-25 we require the envknob AND the node attribute to use - // a netmap cache, with the envknob defaulted to true so we can use it as - // a safety override during rollout. + // As of 2026-03-25 we require the use-cache envknob and either the node + // attribute or the force-cache envknob to use a netmap cache. The use-cache + // envknob defaults to true so we can use it as a safety override during + // rollout. // // We treat the envknob being false as identical to disabling the feature // by policy, and clean up the cache on that basis. That ensures we will @@ -7478,7 +7476,7 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) { // not being updated (because of the envknob) and could be read back when // the node starts up. if nm != nil { - if (b.currentNode().SelfHasCap(nodecap.CacheNetworkMaps) || envknob.Bool("TS_FORCE_CACHE_NETMAP")) && envknob.BoolDefaultTrue("TS_USE_CACHED_NETMAP") { + if shouldCacheNetmap(b.currentNode()) { if err := b.writeNetmapToDiskLockedWithPeers(nm); err != nil { b.logf("write netmap to cache: %v", err) } @@ -7488,6 +7486,14 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) { } } +// shouldCacheNetmap reports whether cn's live netmap and peer deltas should be +// persisted to the netmap cache. +func shouldCacheNetmap(cn *nodeBackend) bool { + return buildfeatures.HasCacheNetMap && + (cn.SelfHasCap(nodecap.CacheNetworkMaps) || envknob.Bool("TS_FORCE_CACHE_NETMAP")) && + envknob.BoolDefaultTrue("TS_USE_CACHED_NETMAP") +} + // HookSetRuntimeMetricsEnabled is an optional hook for the "runtimemetrics" feature. var HookSetRuntimeMetricsEnabled feature.Hook[func(enabled bool)] diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index c4379cdbc..bccdd2a11 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -730,6 +730,75 @@ func TestUpdateNetMapCache(t *testing.T) { clb.mu.Unlock() wantCacheEmpty() + + // Force caching without the node attribute and verify that incremental peer + // updates are also persisted. A later load from the cache must not recover + // the peer state from the preceding full map. + t.Setenv("TS_FORCE_CACHE_NETMAP", "1") + clb.mu.Lock() + clb.setNetMapLocked(testMap) + clb.mu.Unlock() + + loadCachedMap := func() *netmap.NetworkMap { + t.Helper() + c := netmapcache.NewCache(netmapcache.FileStore(cacheDir)) + nm, err := c.Load(t.Context()) + if err != nil { + t.Fatalf("Load cached netmap: %v", err) + } + return nm + } + peerByID := func(nm *netmap.NetworkMap, id tailcfg.NodeID) (tailcfg.NodeView, bool) { + t.Helper() + for _, p := range nm.Peers { + if p.ID() == id { + return p, true + } + } + return tailcfg.NodeView{}, false + } + + patches, ok := netmap.MutationsFromMapResponse(&tailcfg.MapResponse{ + PeersChangedPatch: []*tailcfg.PeerChange{{NodeID: 601, Online: new(false)}}, + }, time.Now()) + if !ok { + t.Fatal("MutationsFromMapResponse failed") + } + if !clb.UpdateNetmapDelta(patches) { + t.Fatal("UpdateNetmapDelta(peer patch) = false, want true") + } + if p, ok := peerByID(loadCachedMap(), 601); !ok { + t.Fatal("patched peer 601 is missing from cached netmap") + } else if online, ok := p.Online().GetOk(); !ok || online { + t.Fatalf("cached peer 601 Online = %v, %v; want false, true", online, ok) + } + + newPeer := (&tailcfg.Node{ + ID: 603, + StableID: "n603FAKE", + ComputedName: "new-peer", + User: tailcfg.UserID(1), + Key: makeNodeKeyFromID(603), + Addresses: []netip.Prefix{ + netip.MustParsePrefix("100.2.3.7/32"), + }, + }).View() + if !clb.UpdateNetmapDelta([]netmap.NodeMutation{ + netmap.NodeMutationUpsert{Node: newPeer}, + netmap.MakeNodeMutationRemove(601), + }) { + t.Fatal("UpdateNetmapDelta(peer upsert and removal) = false, want true") + } + got := loadCachedMap() + if _, ok := peerByID(got, 601); ok { + t.Error("removed peer 601 is still present in cached netmap") + } + if p, ok := peerByID(got, 603); !ok { + t.Fatal("upserted peer 603 is missing from cached netmap") + } else if diff := cmp.Diff(p.AsStruct(), newPeer.AsStruct(), + cmpopts.EquateComparable(key.NodePublic{}, key.DiscoPublic{}, netip.Addr{}, netip.Prefix{})); diff != "" { + t.Errorf("cached peer 603 differs (-got, +want):\n%s", diff) + } } func TestConfigureExitNode(t *testing.T) {