diff --git a/CHANGELOG.md b/CHANGELOG.md index ddc7511aa..bd1160e76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -151,6 +151,7 @@ the ACME DNS-01 challenge through a built-in DNS provider: `acmedns`, - Add the v2 `acl/validate` endpoint, which Terraform provider 0.29.3 and newer call to check `tailscale_acl` at plan time; before it, those releases failed with `invalid character 'p' after top-level value` - 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) ## 0.29.7 (2026-10-30) diff --git a/hscontrol/auth.go b/hscontrol/auth.go index 8f91f4af6..beb22e4b6 100644 --- a/hscontrol/auth.go +++ b/hscontrol/auth.go @@ -466,6 +466,13 @@ func (h *Headscale) handleRegisterWithAuthKey( req, machineKey, ) + + // A client that leaves before its first poll ends no session to arm this. + // Before the error check: a failed policy refresh returns the saved node. + if node.IsEphemeral() { + h.ephemeralGC.Schedule(node.ID(), h.cfg.Node.Ephemeral.InactivityTimeout) + } + if err != nil { h.Change(changed) diff --git a/hscontrol/db/node.go b/hscontrol/db/node.go index 94a61176a..38f85bb68 100644 --- a/hscontrol/db/node.go +++ b/hscontrol/db/node.go @@ -366,8 +366,8 @@ func NodeSetMachineKey( // EphemeralGarbageCollector is a garbage collector that will delete nodes after // a certain amount of time. -// It is used to delete ephemeral nodes ([types.Node.IsEphemeral]) that have disconnected and should be -// cleaned up. +// It is used to delete ephemeral nodes ([types.Node.IsEphemeral]) that have disconnected, or never +// connected, and should be cleaned up. type EphemeralGarbageCollector struct { mu sync.Mutex diff --git a/hscontrol/poll_test.go b/hscontrol/poll_test.go index 9dc16c46c..627188775 100644 --- a/hscontrol/poll_test.go +++ b/hscontrol/poll_test.go @@ -2,6 +2,7 @@ package hscontrol import ( "context" + "errors" "fmt" "io" "net/http" @@ -14,6 +15,7 @@ import ( "github.com/juanfont/headscale/hscontrol/db" "github.com/juanfont/headscale/hscontrol/mapper" + "github.com/juanfont/headscale/hscontrol/policy" "github.com/juanfont/headscale/hscontrol/state" "github.com/juanfont/headscale/hscontrol/types" "github.com/juanfont/headscale/hscontrol/types/change" @@ -305,6 +307,7 @@ func TestFailedReconnectDoesNotCancelEphemeralGC(t *testing.T) { t.Parallel() app := createTestApp(t) + app.cfg.Node.Ephemeral.InactivityTimeout = 5 * time.Second app.StartEphemeralGCForTest(t) user := app.state.CreateUserForTest("eph-gc-cancel-user") @@ -391,6 +394,11 @@ func TestEphemeralGCDoesNotDeleteReconnectedNode(t *testing.T) { nodeView, ok := app.state.GetNodeByNodeKey(nodeKey.Public()) require.True(t, ok) require.True(t, nodeView.IsEphemeral(), "test sanity: node must be ephemeral") + require.True(t, app.ephemeralGC.IsScheduled(nodeView.ID()), + "test sanity: registration must arm the GC timer") + // Stands in for the first poll's Cancel; the sessions below are replayed + // by hand and must not race the registration's timer. + app.ephemeralGC.Cancel(nodeView.ID()) node := nodeView.AsStruct() session := app.newMapSession(context.Background(), tailcfg.MapRequest{ @@ -447,6 +455,7 @@ func TestDeleteNodeCancelsEphemeralGC(t *testing.T) { t.Parallel() app := createTestApp(t) + app.cfg.Node.Ephemeral.InactivityTimeout = 5 * time.Second app.StartEphemeralGCForTest(t) user := app.state.CreateUserForTest("eph-gc-delete-user") @@ -478,6 +487,93 @@ func TestDeleteNodeCancelsEphemeralGC(t *testing.T) { "deleting a node must cancel its ephemeral GC timer") } +func TestEphemeralGCDeletesNodeThatNeverPolled(t *testing.T) { + t.Parallel() + + app := createTestApp(t) + app.cfg.Node.Ephemeral.InactivityTimeout = time.Second + app.StartEphemeralGCForTest(t) + + user := app.state.CreateUserForTest("eph-gc-never-polled-user") + pak, err := app.state.CreatePreAuthKey(user.TypedID(), false, true, nil, nil) + require.NoError(t, err) + + nodeKey := key.NewNode() + + _, err = app.handleRegister(context.Background(), tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{ + AuthKey: pak.Key, + }, + NodeKey: nodeKey.Public(), + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "eph-gc-never-polled-node", + }, + Expiry: time.Now().Add(24 * time.Hour), + }, key.NewMachine().Public()) + require.NoError(t, err) + + nodeView, ok := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, ok) + require.True(t, nodeView.IsEphemeral(), "test sanity: node must be ephemeral") + + assert.Eventually(t, func() bool { + _, ok := app.state.GetNodeByID(nodeView.ID()) + + return !ok + }, 5*time.Second, 10*time.Millisecond, + "an ephemeral node that never long-polled must be garbage collected") +} + +var errInjectedSetUsers = errors.New("injected SetUsers failure") + +// failingSetUsersPolicyManager fails the user refresh that follows saving a +// registered node. +type failingSetUsersPolicyManager struct { + policy.PolicyManager +} + +func (failingSetUsersPolicyManager) SetUsers([]types.User) (bool, bool, error) { + return false, false, errInjectedSetUsers +} + +// TestEphemeralGCArmedWhenPolicyRefreshFails pins that a node saved by a +// registration that then returns an error still gets its timer: the client +// may never retry, and no session would ever arm one. +func TestEphemeralGCArmedWhenPolicyRefreshFails(t *testing.T) { + t.Parallel() + + app := createTestApp(t) + app.cfg.Node.Ephemeral.InactivityTimeout = time.Hour + app.StartEphemeralGCForTest(t) + + user := app.state.CreateUserForTest("eph-gc-policy-fail-user") + pak, err := app.state.CreatePreAuthKey(user.TypedID(), false, true, nil, nil) + require.NoError(t, err) + + app.state.WrapPolicyManagerForTest(func(pm policy.PolicyManager) policy.PolicyManager { + return failingSetUsersPolicyManager{PolicyManager: pm} + }) + + nodeKey := key.NewNode() + + _, err = app.handleRegister(context.Background(), tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{ + AuthKey: pak.Key, + }, + NodeKey: nodeKey.Public(), + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "eph-gc-policy-fail-node", + }, + Expiry: time.Now().Add(24 * time.Hour), + }, key.NewMachine().Public()) + require.ErrorIs(t, err, errInjectedSetUsers) + + nodeView, ok := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, ok, "test sanity: the node must be saved despite the error") + assert.True(t, app.ephemeralGC.IsScheduled(nodeView.ID()), + "a saved ephemeral node must have a GC timer") +} + // TestGitHubIssue3129_BlockedWriteSessionLifecycle // tests the scenario reported in // https://github.com/juanfont/headscale/issues/3129. diff --git a/hscontrol/servertest/ephemeral_test.go b/hscontrol/servertest/ephemeral_test.go index 0edb6630a..bd2afd182 100644 --- a/hscontrol/servertest/ephemeral_test.go +++ b/hscontrol/servertest/ephemeral_test.go @@ -73,8 +73,8 @@ func TestEphemeralNodes(t *testing.T) { // Ensure the ephemeral node's long-poll session is fully // established on the server before disconnecting. Without // this, the [TestClient.Disconnect] may cancel a [controlclient.Direct.PollNetMap] that hasn't - // yet reached serveLongPoll, so no grace period or ephemeral - // GC would ever be scheduled. + // yet reached serveLongPoll, so no grace period would ever + // start. ephemeral.WaitForPeers(t, 1, 10*time.Second) // Disconnect the ephemeral node. @@ -122,6 +122,30 @@ func TestEphemeralNodes(t *testing.T) { []*servertest.TestClient{r1, r2, e1}) }) + t.Run("ephemeral_outlives_timeout_while_polling", func(t *testing.T) { + t.Parallel() + + srv := servertest.NewServer(t, + servertest.WithEphemeralTimeout(3*time.Second)) + user := srv.CreateUser(t, "eph-polling-user") + + regular := servertest.NewClient(t, srv, "eph-polling-regular", + servertest.WithUser(user)) + ephemeral := servertest.NewClient(t, srv, "eph-polling-ephemeral", + servertest.WithUser(user), servertest.WithEphemeral()) + + regular.WaitForPeers(t, 1, 10*time.Second) + ephemeral.WaitForPeers(t, 1, 10*time.Second) + + // Registration armed a timer; the session must outlast it. + assert.Never(t, func() bool { + _, found := regular.PeerByName("eph-polling-ephemeral") + + return !found + }, 6*time.Second, 100*time.Millisecond, + "a polling ephemeral node must not be deleted") + }) + t.Run("ephemeral_reconnect_prevents_cleanup", func(t *testing.T) { t.Parallel() diff --git a/hscontrol/servertest/issues_test.go b/hscontrol/servertest/issues_test.go index 618789234..6445003de 100644 --- a/hscontrol/servertest/issues_test.go +++ b/hscontrol/servertest/issues_test.go @@ -861,8 +861,8 @@ func TestIssuesGracePeriod(t *testing.T) { // Ensure the ephemeral node's long-poll session is fully // established on the server before disconnecting. Without // this, the [TestClient.Disconnect] may cancel a [controlclient.Direct.PollNetMap] that hasn't - // yet reached serveLongPoll, so no grace period or ephemeral - // GC would ever be scheduled. + // yet reached serveLongPoll, so no grace period would ever + // start. ephemeral.WaitForPeers(t, 1, 10*time.Second) ephemeral.Disconnect(t)