From 7721d8c29978b65b68269bb58391fcd25a8c1085 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 30 Oct 2026 14:51:17 +0000 Subject: [PATCH] hscontrol: keep a re-keyed node from its stale ephemeral GC timer Re-registering with a permanent key left the timer armed, and the poll path only cancels for ephemeral nodes. Recheck when the timer fires. --- CHANGELOG.md | 1 + hscontrol/app.go | 8 +++++++ hscontrol/poll_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd1160e76..187f27fc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -152,6 +152,7 @@ the ACME DNS-01 challenge through a built-in DNS provider: `acmedns`, - Reject overlapping TCP listener bindings (e.g. `listen_addr` vs ACME HTTP-01 on port 80) at config load [#3236](https://github.com/juanfont/headscale/pull/3236) - Improve config and bind-failure errors: every violation reported in one pass, named YAML keys, actionable operator hints [#3236](https://github.com/juanfont/headscale/pull/3236) - Ephemeral nodes that register but never connect are removed after `node.ephemeral.inactivity_timeout` instead of staying until restart [#3545](https://github.com/juanfont/headscale/pull/3545) +- A node re-registered with a permanent key is no longer deleted by a timer armed while it was ephemeral [#3545](https://github.com/juanfont/headscale/pull/3545) ## 0.29.7 (2026-10-30) diff --git a/hscontrol/app.go b/hscontrol/app.go index 90580c8f1..6631268c4 100644 --- a/hscontrol/app.go +++ b/hscontrol/app.go @@ -169,6 +169,14 @@ func NewHeadscale(cfg *types.Config) (*Headscale, error) { return } + // Re-registering with a permanent key cancels nothing, and the poll + // path only cancels for ephemeral nodes, so the old timer still fires. + if !node.IsEphemeral() { + log.Debug().Caller().EmbedObject(node).Msg("node is no longer ephemeral, skipping garbage collection") + + return + } + // Schedule and Cancel run outside the session transition, so a // disconnecting session can arm a timer after a reconnect already // cancelled it. The session count is authoritative; a stale timer diff --git a/hscontrol/poll_test.go b/hscontrol/poll_test.go index 627188775..ac6605463 100644 --- a/hscontrol/poll_test.go +++ b/hscontrol/poll_test.go @@ -574,6 +574,54 @@ func TestEphemeralGCArmedWhenPolicyRefreshFails(t *testing.T) { "a saved ephemeral node must have a GC timer") } +// TestEphemeralGCKeepsNodeReKeyedToPermanent pins that a timer armed while a +// node was ephemeral does not delete it once a permanent key took over. +func TestEphemeralGCKeepsNodeReKeyedToPermanent(t *testing.T) { + t.Parallel() + + const timeout = time.Second + + app := createTestApp(t) + app.cfg.Node.Ephemeral.InactivityTimeout = timeout + app.StartEphemeralGCForTest(t) + + user := app.state.CreateUserForTest("eph-gc-rekey-user") + ephemeralKey, err := app.state.CreatePreAuthKey(user.TypedID(), false, true, nil, nil) + require.NoError(t, err) + permanentKey, err := app.state.CreatePreAuthKey(user.TypedID(), false, false, nil, nil) + require.NoError(t, err) + + nodeKey := key.NewNode() + machineKey := key.NewMachine() + + for _, authKey := range []string{ephemeralKey.Key, permanentKey.Key} { + _, err = app.handleRegister(context.Background(), tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{ + AuthKey: authKey, + }, + NodeKey: nodeKey.Public(), + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "eph-gc-rekey-node", + }, + Expiry: time.Now().Add(24 * time.Hour), + }, machineKey.Public()) + require.NoError(t, err) + } + + nodeView, ok := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, ok) + require.False(t, nodeView.IsEphemeral(), "test sanity: node must be permanent") + require.True(t, app.ephemeralGC.IsScheduled(nodeView.ID()), + "test sanity: the ephemeral registration's timer must still be armed") + + assert.Never(t, func() bool { + _, ok := app.state.GetNodeByID(nodeView.ID()) + + return !ok + }, 2*timeout, 20*time.Millisecond, + "a stale timer must not delete a node that is no longer ephemeral") +} + // TestGitHubIssue3129_BlockedWriteSessionLifecycle // tests the scenario reported in // https://github.com/juanfont/headscale/issues/3129.