From 61c95f409c90728d3c3ad2627ea77fa4e1a48390 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Claus=20Lensb=C3=B8l?= Date: Wed, 15 Apr 2026 03:53:40 -0400 Subject: [PATCH] control/controlclient: accept key if last seen on exist node is absent (#19402) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On some nodes (found via natlab), the existing nodes last seen could be unset. For these cases, we would want to accept the key and write a last seen. This was breaking the cached netmap natlab tests. Updates #12639 Signed-off-by: Claus Lensbøl --- control/controlclient/map.go | 5 +++-- control/controlclient/map_test.go | 13 ++++++++++++- tstest/integration/nat/nat_test.go | 7 +++++-- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/control/controlclient/map.go b/control/controlclient/map.go index 84a7c2e8b..17c223fe3 100644 --- a/control/controlclient/map.go +++ b/control/controlclient/map.go @@ -459,8 +459,9 @@ func (ms *mapSession) removeUnwantedDiscoUpdates(resp *tailcfg.MapResponse, viaT } // Accept if: - // - lastSeen moved forward in time. - if existingLastSeen, ok := existingNode.LastSeen().GetOk(); ok && + // - if we don't have a last seen to compare against on the existing node. + // - OR lastSeen moved forward in time. + if existingLastSeen, ok := existingNode.LastSeen().GetOk(); !ok || change.LastSeen.After(existingLastSeen) { acceptedDiscoUpdates = append(acceptedDiscoUpdates, change) } diff --git a/control/controlclient/map_test.go b/control/controlclient/map_test.go index 679c48c17..8cf0156d8 100644 --- a/control/controlclient/map_test.go +++ b/control/controlclient/map_test.go @@ -717,6 +717,15 @@ func TestUpdateDiscoForNode(t *testing.T) { wantUpdate: true, wantKeyChanged: false, }, + { + name: "no_initial_last_seen", + initialOnline: false, + updateDiscoKey: true, + updateOnline: false, + updateLastSeen: time.Now(), + wantUpdate: true, + wantKeyChanged: true, + }, } for _, tt := range tests { @@ -736,7 +745,9 @@ func TestUpdateDiscoForNode(t *testing.T) { Key: key.NewNode().Public(), DiscoKey: oldKey.Public(), Online: &tt.initialOnline, - LastSeen: &tt.initialLastSeen, + } + if !tt.initialLastSeen.IsZero() { + node.LastSeen = &tt.initialLastSeen } if nm := ms.netmapForResponse(&tailcfg.MapResponse{ diff --git a/tstest/integration/nat/nat_test.go b/tstest/integration/nat/nat_test.go index 1ba8d1629..8eca5742f 100644 --- a/tstest/integration/nat/nat_test.go +++ b/tstest/integration/nat/nat_test.go @@ -28,7 +28,6 @@ import ( "golang.org/x/mod/modfile" "golang.org/x/sync/errgroup" "tailscale.com/client/tailscale" - "tailscale.com/envknob" "tailscale.com/ipn/ipnstate" "tailscale.com/syncs" "tailscale.com/tailcfg" @@ -792,8 +791,12 @@ func TestEasyEasy(t *testing.T) { nt.want(routeDirect) } +// TestTwoEasyNoControlDiscoRotate tests a situation where two nodes have been +// online and connected through control, but then loose control access and also +// rotate keys. It is not a perfect proxy for a cached node, as the node will +// still have a mapState and not use the backup method of inserting keys into +// the engine directly. func TestTwoEasyNoControlDiscoRotate(t *testing.T) { - envknob.Setenv("TS_USE_CACHED_NETMAP", "1") nt := newNatTest(t) nt.runTailscaleConnectivityTest(easyNoControlDiscoRotate, easyNoControlDiscoRotate) nt.want(routeDirect)