[PR #5420] [MERGED] [management] Add explicit target delete on service removal #25852

Closed
opened 2026-08-05 07:06:33 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/5420
Author: @pascal-fischer
Created: 2/23/2026
Status: Merged
Merged: 3/2/2026
Merged by: @pascal-fischer

Base: mainHead: fix/target-delete-on-service-delete


📝 Commits (10+)

  • 1669b4a Add explicit target delete on service removal
  • 500d6e0 push account manager mock
  • fe20300 fix mock
  • 0632235 fix mock
  • c0d6340 delete all service targets at once
  • d0372b1 Merge branch 'main' into fix/target-delete-on-service-delete
  • ad37496 generate store mock
  • 81630f4 do not error is no rows deleted
  • cc01fee fix test imports
  • 4774f52 get service before delete

📊 Changes

7 files changed (+1932 additions, -36 deletions)

View changed files

📝 management/internals/modules/reverseproxy/manager/manager.go (+6 -3)
📝 management/internals/modules/reverseproxy/manager/manager_test.go (+67 -0)
📝 management/server/account/manager.go (+4 -2)
management/server/account/manager_mock.go (+1738 -0)
📝 management/server/store/sql_store.go (+40 -0)
📝 management/server/store/store.go (+3 -0)
📝 management/server/store/store_mock.go (+74 -31)

📄 Description

Describe your changes

Fir sqlite the cascading delete is not taking effect which is causing orphaned targets.

https://github.com/netbirdio/netbird/issues/5398

Stack

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)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Bug Fixes

    • Service deletion now also removes all associated targets, ensures target cleanup happens before final removal, and surfaces failures during target deletion to prevent orphaned targets.
  • Tests

    • Added a test that verifies deleting a service removes its targets and produces the expected events and peer updates.
  • Chores

    • Added storage and mock support to better handle target retrieval and deletion in tests and runtime.

🔄 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/5420 **Author:** [@pascal-fischer](https://github.com/pascal-fischer) **Created:** 2/23/2026 **Status:** ✅ Merged **Merged:** 3/2/2026 **Merged by:** [@pascal-fischer](https://github.com/pascal-fischer) **Base:** `main` ← **Head:** `fix/target-delete-on-service-delete` --- ### 📝 Commits (10+) - [`1669b4a`](https://github.com/netbirdio/netbird/commit/1669b4aa02b07f3b4ccfbb01c3d118f91d7ffa5e) Add explicit target delete on service removal - [`500d6e0`](https://github.com/netbirdio/netbird/commit/500d6e01bd9230a65849376ce278ba0f4f7bd124) push account manager mock - [`fe20300`](https://github.com/netbirdio/netbird/commit/fe203005eaa0bcbfc66d1cd48185642c9f3bf2f6) fix mock - [`0632235`](https://github.com/netbirdio/netbird/commit/063223589b71839485b9ade8db5e3d2060b094d4) fix mock - [`c0d6340`](https://github.com/netbirdio/netbird/commit/c0d63401a665df7900cfa35f603a90181700b4e3) delete all service targets at once - [`d0372b1`](https://github.com/netbirdio/netbird/commit/d0372b18ada5e75efa48a9e4e10b24bca3b9396c) Merge branch 'main' into fix/target-delete-on-service-delete - [`ad37496`](https://github.com/netbirdio/netbird/commit/ad374961e9fc1aee556a36456a0a3c9efd0a5e10) generate store mock - [`81630f4`](https://github.com/netbirdio/netbird/commit/81630f4f99d7eb73e4554bd612488ac4d749f98d) do not error is no rows deleted - [`cc01fee`](https://github.com/netbirdio/netbird/commit/cc01feec722f6ddab33a3da9a514a3a26752f8ec) fix test imports - [`4774f52`](https://github.com/netbirdio/netbird/commit/4774f526302e600e1386b3e1c8be4cb3fd53c7bd) get service before delete ### 📊 Changes **7 files changed** (+1932 additions, -36 deletions) <details> <summary>View changed files</summary> 📝 `management/internals/modules/reverseproxy/manager/manager.go` (+6 -3) 📝 `management/internals/modules/reverseproxy/manager/manager_test.go` (+67 -0) 📝 `management/server/account/manager.go` (+4 -2) ➕ `management/server/account/manager_mock.go` (+1738 -0) 📝 `management/server/store/sql_store.go` (+40 -0) 📝 `management/server/store/store.go` (+3 -0) 📝 `management/server/store/store_mock.go` (+74 -31) </details> ### 📄 Description ## Describe your changes Fir sqlite the cascading delete is not taking effect which is causing orphaned targets. ## Issue ticket number and link https://github.com/netbirdio/netbird/issues/5398 ## Stack <!-- branch-stack --> ### Checklist - [x] 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) > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Service deletion now also removes all associated targets, ensures target cleanup happens before final removal, and surfaces failures during target deletion to prevent orphaned targets. * **Tests** * Added a test that verifies deleting a service removes its targets and produces the expected events and peer updates. * **Chores** * Added storage and mock support to better handle target retrieval and deletion in tests and runtime. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <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 07:06:33 -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#25852