[PR #1891] [MERGED] Refactor Route IDs #14405

Closed
opened 2026-08-05 02:09:56 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/1891
Author: @lixmal
Created: 4/25/2024
Status: Merged
Merged: 5/6/2024
Merged by: @lixmal

Base: mainHead: refactor/route-ids


📝 Commits (10+)

📊 Changes

25 files changed (+320 additions, -292 deletions)

View changed files

📝 client/internal/engine.go (+7 -10)
📝 client/internal/engine_test.go (+2 -2)
📝 client/internal/routemanager/client.go (+8 -8)
📝 client/internal/routemanager/client_test.go (+28 -28)
📝 client/internal/routemanager/manager.go (+18 -18)
📝 client/internal/routemanager/mock.go (+4 -4)
📝 client/internal/routemanager/notifier.go (+1 -1)
📝 client/internal/routemanager/server.go (+1 -1)
📝 client/internal/routemanager/server_nonandroid.go (+5 -5)
📝 client/internal/routeselector/routeselector.go (+12 -16)
📝 client/internal/routeselector/routeselector_test.go (+58 -58)
📝 client/server/route.go (+15 -4)
📝 management/server/account.go (+10 -10)
📝 management/server/account_test.go (+9 -9)
📝 management/server/group.go (+1 -1)
📝 management/server/http/routes_handler.go (+8 -8)
📝 management/server/http/routes_handler_test.go (+3 -3)
📝 management/server/metrics/selfhosted_test.go (+2 -2)
📝 management/server/mock_server/account_mock.go (+75 -75)
📝 management/server/route.go (+14 -14)

...and 5 more files

📄 Description

Describe your changes

We have at least 3 distinct IDs in use for routes:

  • ID in route.Route, which is the ID in the database
  • NetID in route.Route, which is the Name/ID in the dashboard
  • HAUniqueID which is a combination of Route.NetID and Route.Network

Having so many IDs using all string it's hard to distinguish them from each other.
This PR refactors the IDs to use dedicated types to avoid mixing them up.
Conversions from/to string must be done explicitly. Conversions between the types must not happen.

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • Extended the README / documentation, if necessary

🔄 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/netbirdio/netbird/pull/1891 **Author:** [@lixmal](https://github.com/lixmal) **Created:** 4/25/2024 **Status:** ✅ Merged **Merged:** 5/6/2024 **Merged by:** [@lixmal](https://github.com/lixmal) **Base:** `main` ← **Head:** `refactor/route-ids` --- ### 📝 Commits (10+) - [`4b8c36a`](https://github.com/netbirdio/netbird/commit/4b8c36a62b5148b2f380e1e2995965700a9bbd8e) Fix route selection IDs - [`facbfb1`](https://github.com/netbirdio/netbird/commit/facbfb123b72277d295b446501db9d51621cf503) Don't panic on nil routes on comparison - [`1279637`](https://github.com/netbirdio/netbird/commit/12796377f1ad92f48849f1dfb0dd1f8ac23f913e) Check for nil - [`65105e1`](https://github.com/netbirdio/netbird/commit/65105e14b714fbcdfc74f873f38b115a63f353cc) Improve check - [`fe18f97`](https://github.com/netbirdio/netbird/commit/fe18f9752df5ccd23ddac8fc08fa692c40061bde) Use dedicated IDs for Route ID, Route NetID and HAUniqueID - [`162440c`](https://github.com/netbirdio/netbird/commit/162440c82f7d0e89b5682c1e8381a814d64325ae) Fix tests - [`b2ab301`](https://github.com/netbirdio/netbird/commit/b2ab3018d69483f8c53b7490023ae2feaa6f05de) Fix function name - [`a847d91`](https://github.com/netbirdio/netbird/commit/a847d91c774a0ecc18f4ffea8020d03514aa8bd4) Fix test - [`bd05494`](https://github.com/netbirdio/netbird/commit/bd054948f0eee14c5e27d9658a1c354de6a38acf) Merge branch 'main' into refactor/route-ids - [`eb99e1d`](https://github.com/netbirdio/netbird/commit/eb99e1d408967d31ab04dd8f3e39494ce5ba3f03) Use NetID in route selector ### 📊 Changes **25 files changed** (+320 additions, -292 deletions) <details> <summary>View changed files</summary> 📝 `client/internal/engine.go` (+7 -10) 📝 `client/internal/engine_test.go` (+2 -2) 📝 `client/internal/routemanager/client.go` (+8 -8) 📝 `client/internal/routemanager/client_test.go` (+28 -28) 📝 `client/internal/routemanager/manager.go` (+18 -18) 📝 `client/internal/routemanager/mock.go` (+4 -4) 📝 `client/internal/routemanager/notifier.go` (+1 -1) 📝 `client/internal/routemanager/server.go` (+1 -1) 📝 `client/internal/routemanager/server_nonandroid.go` (+5 -5) 📝 `client/internal/routeselector/routeselector.go` (+12 -16) 📝 `client/internal/routeselector/routeselector_test.go` (+58 -58) 📝 `client/server/route.go` (+15 -4) 📝 `management/server/account.go` (+10 -10) 📝 `management/server/account_test.go` (+9 -9) 📝 `management/server/group.go` (+1 -1) 📝 `management/server/http/routes_handler.go` (+8 -8) 📝 `management/server/http/routes_handler_test.go` (+3 -3) 📝 `management/server/metrics/selfhosted_test.go` (+2 -2) 📝 `management/server/mock_server/account_mock.go` (+75 -75) 📝 `management/server/route.go` (+14 -14) _...and 5 more files_ </details> ### 📄 Description ## Describe your changes We have at least 3 distinct IDs in use for routes: - `ID` in `route.Route`, which is the ID in the database - `NetID` in `route.Route`, which is the Name/ID in the dashboard - `HAUniqueID` which is a combination of `Route.NetID` and `Route.Network` Having so many IDs using all `string` it's hard to distinguish them from each other. This PR refactors the IDs to use dedicated types to avoid mixing them up. Conversions from/to `string` must be done explicitly. Conversions between the types must not happen. ## Issue ticket number and link ### Checklist - [ ] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [x] It is a refactor - [ ] Created tests that fail without the change (if possible) - [ ] Extended the README / documentation, if necessary --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
saavagebueno added the pull-request label 2026-08-05 02:09:56 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#14405