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)