[PR #366] [MERGED] Registration simplification #1408

Closed
opened 2025-12-29 02:29:57 +01:00 by adam · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/juanfont/headscale/pull/366
Author: @kradalby
Created: 2/28/2022
Status: Merged
Merged: 3/2/2022
Merged by: @kradalby

Base: mainHead: registration-simplification


📝 Commits (10+)

  • c58ce6f Generalise the registration method to DRY stuff up
  • acb9458 Generalise registration for pre auth keys
  • fd1e4a1 Generalise registration for openid
  • caffbd8 Update cli registration with new method
  • ecc2643 Fix excessive replace
  • 1caa6f5 Add todo for JSON datatype
  • 469551b Register new machines needing callback in memory
  • 402a760 Reuse machine structure for parameters, named parameters
  • 54cc3c0 Implement new machine register parameter
  • 50053e6 Ignore complexity linter

📊 Changes

22 files changed (+330 additions, -470 deletions)

View changed files

📝 .golangci.yaml (+1 -0)
📝 CHANGELOG.md (+27 -23)
📝 acls_test.go (+0 -7)
📝 api.go (+85 -96)
📝 app.go (+14 -15)
📝 app_test.go (+0 -5)
cli_test.go (+0 -41)
📝 db.go (+33 -0)
📝 dns_test.go (+0 -8)
📝 gen/go/headscale/v1/machine.pb.go (+45 -55)
📝 gen/openapiv2/headscale/v1/headscale.swagger.json (+3 -6)
📝 grpcv1.go (+8 -6)
📝 integration_cli_test.go (+0 -13)
📝 machine.go (+48 -84)
📝 machine_test.go (+0 -7)
📝 namespaces_test.go (+0 -5)
📝 oidc.go (+53 -84)
📝 preauth_keys.go (+6 -0)
📝 preauth_keys_test.go (+0 -3)
📝 proto/headscale/v1/machine.proto (+7 -7)

...and 2 more files

📄 Description

Resolves #294, #341, #365

This PR simplifies registration in a couple of ways:

  • DRY the registration logic into a function in machine.go
    We had quite a lot of duplicate logic as noted in #294, and this reduces this and centers the logic around RegisterMachine.

  • Only write machines/nodes to the database when they are registered
    In the current setup, we write the machine to the database when the registration process starts, and this means that machines that dont successfully register becomes ghost machines in the DB. They dont really have anything to do in persistent storage until successful, so this PR moves them to a cache like memory storage which is cleaned up occasionally.

  • Remove Registered column, use Expiry as main machine filter
    Since we dont have machines that isnt "unregistered" anymore, we can simplify a lot of logic and only check if a machine exist, and if it is expired or not. This makes it easier to reason about if we should care for a machine in particular scenarios (no more if registered and !expired).

  • read the CONTRIBUTING guidelines
  • raised a GitHub issue or discussed it on the projects chat beforehand
  • added unit tests
  • added integration tests
  • updated documentation if needed
  • updated CHANGELOG.md

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/juanfont/headscale/pull/366 **Author:** [@kradalby](https://github.com/kradalby) **Created:** 2/28/2022 **Status:** ✅ Merged **Merged:** 3/2/2022 **Merged by:** [@kradalby](https://github.com/kradalby) **Base:** `main` ← **Head:** `registration-simplification` --- ### 📝 Commits (10+) - [`c58ce6f`](https://github.com/juanfont/headscale/commit/c58ce6f60cd0e85750228acc30f24a3dda93b619) Generalise the registration method to DRY stuff up - [`acb9458`](https://github.com/juanfont/headscale/commit/acb945841c6878c1af6e66522f146265bd7feaa3) Generalise registration for pre auth keys - [`fd1e4a1`](https://github.com/juanfont/headscale/commit/fd1e4a1dcd4b08868f70f221eee4f2524a69ffdf) Generalise registration for openid - [`caffbd8`](https://github.com/juanfont/headscale/commit/caffbd8956776e97e54bbeb6b18ea30a4c287b40) Update cli registration with new method - [`ecc2643`](https://github.com/juanfont/headscale/commit/ecc26432fd64c19d3b314536fcfee634d53e3517) Fix excessive replace - [`1caa6f5`](https://github.com/juanfont/headscale/commit/1caa6f5d6915b762cad4c9d5b28654ae0d2efe67) Add todo for JSON datatype - [`469551b`](https://github.com/juanfont/headscale/commit/469551bc5df6dd8483af1246560f2f358a2ed943) Register new machines needing callback in memory - [`402a760`](https://github.com/juanfont/headscale/commit/402a76070f501bf490dd75391b72ef1743db66e8) Reuse machine structure for parameters, named parameters - [`54cc3c0`](https://github.com/juanfont/headscale/commit/54cc3c067ffabf2cae5339a615598ab82feb02fd) Implement new machine register parameter - [`50053e6`](https://github.com/juanfont/headscale/commit/50053e616a42109b22a1aa15cba56461c04035eb) Ignore complexity linter ### 📊 Changes **22 files changed** (+330 additions, -470 deletions) <details> <summary>View changed files</summary> 📝 `.golangci.yaml` (+1 -0) 📝 `CHANGELOG.md` (+27 -23) 📝 `acls_test.go` (+0 -7) 📝 `api.go` (+85 -96) 📝 `app.go` (+14 -15) 📝 `app_test.go` (+0 -5) ➖ `cli_test.go` (+0 -41) 📝 `db.go` (+33 -0) 📝 `dns_test.go` (+0 -8) 📝 `gen/go/headscale/v1/machine.pb.go` (+45 -55) 📝 `gen/openapiv2/headscale/v1/headscale.swagger.json` (+3 -6) 📝 `grpcv1.go` (+8 -6) 📝 `integration_cli_test.go` (+0 -13) 📝 `machine.go` (+48 -84) 📝 `machine_test.go` (+0 -7) 📝 `namespaces_test.go` (+0 -5) 📝 `oidc.go` (+53 -84) 📝 `preauth_keys.go` (+6 -0) 📝 `preauth_keys_test.go` (+0 -3) 📝 `proto/headscale/v1/machine.proto` (+7 -7) _...and 2 more files_ </details> ### 📄 Description Resolves #294, #341, #365 This PR simplifies registration in a couple of ways: - DRY the registration logic into a function in `machine.go` We had quite a lot of duplicate logic as noted in #294, and this reduces this and centers the logic around RegisterMachine. - Only write machines/nodes to the database _when they are registered_ In the current setup, we write the machine to the database when the registration process starts, and this means that machines that dont successfully register becomes ghost machines in the DB. They dont really have anything to do in persistent storage until successful, so this PR moves them to a cache like memory storage which is cleaned up occasionally. - Remove `Registered` column, use `Expiry` as main machine filter Since we dont have machines that isnt "unregistered" anymore, we can simplify a lot of logic and only check if a machine exist, and if it is expired or not. This makes it easier to reason about if we should care for a machine in particular scenarios (no more `if registered and !expired`). <!-- Please tick if the following things apply. You… --> - [x] read the [CONTRIBUTING guidelines](README.md#user-content-contributing) - [x] raised a GitHub issue or discussed it on the projects chat beforehand - [x] added unit tests - [x] added integration tests - [x] updated documentation if needed - [x] updated CHANGELOG.md <!-- If applicable, please reference the issue using `Fixes #XXX` and add tests to cover your new code. --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
adam added the pull-request label 2025-12-29 02:29:57 +01:00
adam closed this issue 2025-12-29 02:29:58 +01:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/headscale#1408