control/controlclient: accept key if last seen on exist node is absent (#19402)

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 <claus@tailscale.com>
This commit is contained in:
Claus Lensbøl 2026-04-15 03:53:40 -04:00 committed by GitHub
parent effbe67fe3
commit 61c95f409c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 20 additions and 5 deletions

View File

@ -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)
}

View File

@ -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{

View File

@ -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)