Skip to content

Add key expiry disable/enable toggle#554

Open
jul13579 wants to merge 2 commits into
tale:mainfrom
jul13579:feature/disable-key-expiry
Open

Add key expiry disable/enable toggle#554
jul13579 wants to merge 2 commits into
tale:mainfrom
jul13579:feature/disable-key-expiry

Conversation

@jul13579

@jul13579 jul13579 commented May 7, 2026

Copy link
Copy Markdown

This PR adds a button to the node menu to disable (and re-enable) node key expiry, as requested by #553. Since headscale does not store the key expiry toggle state separately from the actual expiry date, re-enabling key expiry sadly currently will result in a node that expired at the time of re-enabling the key expiry. This is not the case with official tailscale, which seems to pick up the original key expiry date from before key expiry was disabled. Hence, I'm actually unsure myself if this feature adds much to the UX of headplane 🤷

@jul13579 jul13579 requested a review from tale as a code owner May 7, 2026 20:42
@github-actions github-actions Bot added the Integrations Related to Headplane integrations label May 7, 2026
@jul13579 jul13579 force-pushed the feature/disable-key-expiry branch from c377674 to c700a04 Compare May 28, 2026 19:38
@tale

tale commented May 30, 2026

Copy link
Copy Markdown
Owner

This fails integration testing and I'd also hold off as I'm changing things somewhat significantly.

@jul13579

jul13579 commented Jun 5, 2026

Copy link
Copy Markdown
Author

This fails integration testing and I'd also hold off as I'm changing things somewhat significantly.

I'm not entirely sure since I haven't looked that much into it, but I believe the tests are failing since the CI is using the latest release version of headscale. The PR adding this headscale feature however hasn't actually made it into a release version yet..

If you don't mind, I'll keep the PR update as best as I can. The changes are few enough for me to adapt them to the changes happening upstream.

@jul13579 jul13579 force-pushed the feature/disable-key-expiry branch 2 times, most recently from 5dd24f8 to 5de975d Compare July 4, 2026 09:31
@jul13579 jul13579 force-pushed the feature/disable-key-expiry branch 3 times, most recently from b2b9e99 to f8a0bd3 Compare July 4, 2026 10:05
@jul13579

jul13579 commented Jul 6, 2026

Copy link
Copy Markdown
Author

@tale This PR now passes integration tests for headscale versions 0.29.0+. Please tell me if you'd like anything changed about the implementation, such that it meets your requirements for integration 😉

@tale

tale commented Jul 13, 2026

Copy link
Copy Markdown
Owner

@pullfrog review

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The new key-expiry toggle needs two fixes before merge: the API parameter is sent in the wrong place, and the version capability is never used in the UI.

Reviewed changes — add a UI toggle for disabling/enabling node key expiry for Headscale 0.29+.

  • Add keyExpiryCanBeDisabled capability gated to Headscale >= 0.29.0 in app/server/headscale/api/capabilities.ts.
  • Add toggleExpiry API call to app/server/headscale/api/resources/nodes.ts.
  • Wire a machine-menu action in app/routes/machines/components/menu.tsx and app/routes/machines/machine-actions.ts to toggle expiry.
  • Add integration/unit tests for the new capability and API in tests/integration/api/nodes.test.ts and tests/unit/headscale/server-version.test.ts.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏

toggleExpiry: async (nodeId, disableExpiry) => {
await transport.request({
method: "POST",
path: `v1/node/${nodeId}/expire?disableExpiry=${disableExpiry}`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ disableExpiry is sent as a query parameter, but Headscale's expireNode endpoint expects it in the JSON body field disableExpiry. With the current implementation the server ignores the query string and, because the body is empty, treats the request as a normal expire (sets expiry to now). Use body: { disableExpiry } instead of embedding it in the path.

Technical details
# `disableExpiry` is sent as a query parameter instead of a body field

## Affected sites
- `app/server/headscale/api/resources/nodes.ts:111` — query parameter `disableExpiry` is appended to the path

## Required outcome
- Send `disableExpiry` as a JSON body field on `POST /api/v1/node/{nodeId}/expire`.

## Suggested approach
```ts
await transport.request({
  method: "POST",
  path: `v1/node/${nodeId}/expire`,
  apiKey,
  body: { disableExpiry },
});
```

## Source
Headscale's `expireNode` handler reads `DisableExpiry` from `ExpireNodeRequestBody.DisableExpiry` with JSON tag `disableExpiry`:
https://github.com/juanfont/headscale/blob/main/hscontrol/api/v1/nodes.go

Comment on lines +161 to +174
<MenuItem
onClick={() =>
submit(
{
action_id: "toggle_expiry",
node_id: node.id,
disableExpiry: !isNoExpiry(node.expiry),
},
{ method: "post" },
)
}
>
{isNoExpiry(node.expiry) ? "Enable" : "Disable"} key expiry
</MenuItem>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<MenuItem
onClick={() =>
submit(
{
action_id: "toggle_expiry",
node_id: node.id,
disableExpiry: !isNoExpiry(node.expiry),
},
{ method: "post" },
)
}
>
{isNoExpiry(node.expiry) ? "Enable" : "Disable"} key expiry
</MenuItem>
{supportsToggleExpiry && (
<MenuItem
onClick={() =>
submit(
{
action_id: "toggle_expiry",
node_id: node.id,
disableExpiry: !isNoExpiry(node.expiry),
},
{ method: "post" },
)
}
>
{isNoExpiry(node.expiry) ? "Enable" : "Disable"} key expiry
</MenuItem>
)}
<MenuItem onClick={() => setModal("routes")}>Edit route settings</MenuItem>
<MenuItem onClick={() => setModal("tags")}>Edit ACL tags</MenuItem>
{supportsNodeOwnerChange && (
<MenuItem onClick={() => setModal("move")}>Change owner</MenuItem>
)}
<MenuSeparator />

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Integrations Related to Headplane integrations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants