diff --git a/hscontrol/app.go b/hscontrol/app.go index ac724db38..e3e6c9d2c 100644 --- a/hscontrol/app.go +++ b/hscontrol/app.go @@ -183,6 +183,15 @@ func NewHeadscale(cfg *types.Config) (*Headscale, error) { return } + // Ephemerality can change after the timer is armed: re-registration + // with a permanent key, or a user claiming a tagged node. + if !node.IsEphemeral() { + log.Debug().Caller().EmbedObject(node). + Msg("node is no longer ephemeral, skipping garbage collection") + + return + } + changes, err := app.state.DeleteNode(node) app.Change(changes...) diff --git a/hscontrol/poll_test.go b/hscontrol/poll_test.go index 30dd85d7f..dc3d80399 100644 --- a/hscontrol/poll_test.go +++ b/hscontrol/poll_test.go @@ -359,6 +359,63 @@ func TestFailedReconnectDoesNotCancelEphemeralGC(t *testing.T) { "failed reconnect must not cancel the ephemeral GC timer (issue #3382)") } +func TestStaleGCTimerSparesPermanentNode(t *testing.T) { + t.Parallel() + app := createTestApp(t) + app.cfg.Node.Ephemeral.InactivityTimeout = time.Hour + app.StartEphemeralGCForTest(t) + user := app.state.CreateUserForTest("gc-permanent-user") + ephemeral, err := app.state.CreatePreAuthKey(user.TypedID(), true, true, nil, nil) + require.NoError(t, err) + permanent, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, nil) + require.NoError(t, err) + + machineKey := key.NewMachine().Public() + req := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: ephemeral.Key}, + NodeKey: key.NewNode().Public(), + Hostinfo: &tailcfg.Hostinfo{Hostname: "gc-permanent-node"}, + } + _, err = app.handleRegisterWithAuthKey(req, machineKey) + require.NoError(t, err) + + node, ok := app.state.GetNodeByNodeKey(req.NodeKey) + require.True(t, ok) + require.True(t, node.IsEphemeral()) + session := app.newMapSession(t.Context(), tailcfg.MapRequest{}, + &recordingResponseWriter{}, node) + _, epoch := app.state.Connect(node.ID()) + app.ephemeralGC.Cancel(node.ID()) + + req.Auth.AuthKey = permanent.Key + _, err = app.handleRegisterWithAuthKey(req, machineKey) + require.NoError(t, err) + + fresh, ok := app.state.GetNodeByID(node.ID()) + require.True(t, ok) + require.False(t, fresh.IsEphemeral()) + + _, err = app.state.Disconnect(node.ID(), epoch) + require.NoError(t, err) + + app.cfg.Node.Ephemeral.InactivityTimeout = 25 * time.Millisecond + + session.afterServeLongPoll() + require.True(t, app.ephemeralGC.IsScheduled(node.ID())) + + require.Eventually(t, func() bool { + return !app.ephemeralGC.IsScheduled(node.ID()) + }, time.Second, 5*time.Millisecond) + assert.Never(t, func() bool { + _, ok := app.state.GetNodeByID(node.ID()) + return !ok + }, 2*time.Second, 5*time.Millisecond, + "a stale session must not collect a node that became permanent") + + _, err = app.state.DB().GetNodeByID(node.ID()) + require.NoError(t, err) +} + // TestEphemeralGCDoesNotDeleteReconnectedNode replays, in order, the // interleaving of two sessions from // https://github.com/juanfont/headscale/issues/3535: the old session takes the