mirror of
https://github.com/juanfont/headscale.git
synced 2026-11-02 03:34:06 +09:00
hscontrol: arm ephemeral GC when a node registers with an auth key
The ephemeral garbage collector was only armed when a long-poll session ended, or for every ephemeral node at startup. A client that registered and went away before its first long-poll reached Connect, for example a pod killed right after login, never ended a session, so no deletion timer was armed. The node then stayed until headscale restarted, well past node.ephemeral.inactivity_timeout. Schedule the GC right after the auth-key registration, before the error check: a failed policy refresh still returns the saved node. The existing Cancel in serveLongPoll stops the timer once the node connects. A timer that fires while a session is live is still skipped by the ActiveSessions check. TestEphemeralGCDoesNotDeleteReconnectedNode now checks that registration arms the timer and cancels it before replaying its sessions. Otherwise the 50ms timer could fire while the test's Connect was still waiting for a NodeStore flush. TestFailedReconnectDoesNotCancelEphemeralGC and TestDeleteNodeCancelsEphemeralGC ran with a zero inactivity timeout, so the registration timer deleted the node at once, and the delete cancelled the timer the tests armed themselves. They now use 5s, which outlasts the tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
704f1cafd3
commit
1ff8e368b6
6 files changed
+134
-6
No files matched your search
@@ -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`
|
- 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)
|
- 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)
|
- 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)
|
## 0.29.7 (2026-10-30)
|
||||||
|
|
||||||
|
|||||||
@@ -466,6 +466,13 @@ func (h *Headscale) handleRegisterWithAuthKey(
|
|||||||
req,
|
req,
|
||||||
machineKey,
|
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 {
|
if err != nil {
|
||||||
h.Change(changed)
|
h.Change(changed)
|
||||||
|
|
||||||
|
|||||||
@@ -366,8 +366,8 @@ func NodeSetMachineKey(
|
|||||||
|
|
||||||
// EphemeralGarbageCollector is a garbage collector that will delete nodes after
|
// EphemeralGarbageCollector is a garbage collector that will delete nodes after
|
||||||
// a certain amount of time.
|
// a certain amount of time.
|
||||||
// It is used to delete ephemeral nodes ([types.Node.IsEphemeral]) that have disconnected and should be
|
// It is used to delete ephemeral nodes ([types.Node.IsEphemeral]) that have disconnected, or never
|
||||||
// cleaned up.
|
// connected, and should be cleaned up.
|
||||||
type EphemeralGarbageCollector struct {
|
type EphemeralGarbageCollector struct {
|
||||||
mu sync.Mutex
|
mu sync.Mutex
|
||||||
|
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package hscontrol
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
@@ -14,6 +15,7 @@ import (
|
|||||||
|
|
||||||
"github.com/juanfont/headscale/hscontrol/db"
|
"github.com/juanfont/headscale/hscontrol/db"
|
||||||
"github.com/juanfont/headscale/hscontrol/mapper"
|
"github.com/juanfont/headscale/hscontrol/mapper"
|
||||||
|
"github.com/juanfont/headscale/hscontrol/policy"
|
||||||
"github.com/juanfont/headscale/hscontrol/state"
|
"github.com/juanfont/headscale/hscontrol/state"
|
||||||
"github.com/juanfont/headscale/hscontrol/types"
|
"github.com/juanfont/headscale/hscontrol/types"
|
||||||
"github.com/juanfont/headscale/hscontrol/types/change"
|
"github.com/juanfont/headscale/hscontrol/types/change"
|
||||||
@@ -305,6 +307,7 @@ func TestFailedReconnectDoesNotCancelEphemeralGC(t *testing.T) {
|
|||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
app := createTestApp(t)
|
app := createTestApp(t)
|
||||||
|
app.cfg.Node.Ephemeral.InactivityTimeout = 5 * time.Second
|
||||||
app.StartEphemeralGCForTest(t)
|
app.StartEphemeralGCForTest(t)
|
||||||
|
|
||||||
user := app.state.CreateUserForTest("eph-gc-cancel-user")
|
user := app.state.CreateUserForTest("eph-gc-cancel-user")
|
||||||
@@ -391,6 +394,11 @@ func TestEphemeralGCDoesNotDeleteReconnectedNode(t *testing.T) {
|
|||||||
nodeView, ok := app.state.GetNodeByNodeKey(nodeKey.Public())
|
nodeView, ok := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||||
require.True(t, ok)
|
require.True(t, ok)
|
||||||
require.True(t, nodeView.IsEphemeral(), "test sanity: node must be ephemeral")
|
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()
|
node := nodeView.AsStruct()
|
||||||
session := app.newMapSession(context.Background(), tailcfg.MapRequest{
|
session := app.newMapSession(context.Background(), tailcfg.MapRequest{
|
||||||
@@ -447,6 +455,7 @@ func TestDeleteNodeCancelsEphemeralGC(t *testing.T) {
|
|||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
app := createTestApp(t)
|
app := createTestApp(t)
|
||||||
|
app.cfg.Node.Ephemeral.InactivityTimeout = 5 * time.Second
|
||||||
app.StartEphemeralGCForTest(t)
|
app.StartEphemeralGCForTest(t)
|
||||||
|
|
||||||
user := app.state.CreateUserForTest("eph-gc-delete-user")
|
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")
|
"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
|
// TestGitHubIssue3129_BlockedWriteSessionLifecycle
|
||||||
// tests the scenario reported in
|
// tests the scenario reported in
|
||||||
// https://github.com/juanfont/headscale/issues/3129.
|
// https://github.com/juanfont/headscale/issues/3129.
|
||||||
|
|||||||
@@ -73,8 +73,8 @@ func TestEphemeralNodes(t *testing.T) {
|
|||||||
// Ensure the ephemeral node's long-poll session is fully
|
// Ensure the ephemeral node's long-poll session is fully
|
||||||
// established on the server before disconnecting. Without
|
// established on the server before disconnecting. Without
|
||||||
// this, the [TestClient.Disconnect] may cancel a [controlclient.Direct.PollNetMap] that hasn't
|
// this, the [TestClient.Disconnect] may cancel a [controlclient.Direct.PollNetMap] that hasn't
|
||||||
// yet reached serveLongPoll, so no grace period or ephemeral
|
// yet reached serveLongPoll, so no grace period would ever
|
||||||
// GC would ever be scheduled.
|
// start.
|
||||||
ephemeral.WaitForPeers(t, 1, 10*time.Second)
|
ephemeral.WaitForPeers(t, 1, 10*time.Second)
|
||||||
|
|
||||||
// Disconnect the ephemeral node.
|
// Disconnect the ephemeral node.
|
||||||
@@ -122,6 +122,30 @@ func TestEphemeralNodes(t *testing.T) {
|
|||||||
[]*servertest.TestClient{r1, r2, e1})
|
[]*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.Run("ephemeral_reconnect_prevents_cleanup", func(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
@@ -861,8 +861,8 @@ func TestIssuesGracePeriod(t *testing.T) {
|
|||||||
// Ensure the ephemeral node's long-poll session is fully
|
// Ensure the ephemeral node's long-poll session is fully
|
||||||
// established on the server before disconnecting. Without
|
// established on the server before disconnecting. Without
|
||||||
// this, the [TestClient.Disconnect] may cancel a [controlclient.Direct.PollNetMap] that hasn't
|
// this, the [TestClient.Disconnect] may cancel a [controlclient.Direct.PollNetMap] that hasn't
|
||||||
// yet reached serveLongPoll, so no grace period or ephemeral
|
// yet reached serveLongPoll, so no grace period would ever
|
||||||
// GC would ever be scheduled.
|
// start.
|
||||||
ephemeral.WaitForPeers(t, 1, 10*time.Second)
|
ephemeral.WaitForPeers(t, 1, 10*time.Second)
|
||||||
|
|
||||||
ephemeral.Disconnect(t)
|
ephemeral.Disconnect(t)
|
||||||
|
|||||||
Reference in new issue
Block a user