[management] Add explicit target delete on service removal

management/internals/modules/reverseproxy/manager/manager.go

@@ -379,10 +379,8 @@ func (m *managerImpl) DeleteService(ctx context.Context, accountID, userID, serv
 
 	var service *reverseproxy.Service
 	err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
-		var err error
-		service, err = transaction.GetServiceByID(ctx, store.LockingStrengthUpdate, accountID, serviceID)
-		if err != nil {
-			return err
+		if err := transaction.DeleteServiceTargets(ctx, accountID, serviceID); err != nil {
+			return fmt.Errorf("failed to delete targets: %w", err)
 		}
 
 		if err = transaction.DeleteService(ctx, accountID, serviceID); err != nil {

management/internals/modules/reverseproxy/manager/manager_test.go

@@ -11,7 +11,14 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/netbirdio/netbird/management/internals/modules/reverseproxy"
+	nbgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc"
+	"github.com/netbirdio/netbird/management/server/account"
+	"github.com/netbirdio/netbird/management/server/activity"
+	"github.com/netbirdio/netbird/management/server/permissions"
+	"github.com/netbirdio/netbird/management/server/permissions/modules"
+	"github.com/netbirdio/netbird/management/server/permissions/operations"
 	"github.com/netbirdio/netbird/management/server/store"
+	storetypes "github.com/netbirdio/netbird/management/server/types"
 	"github.com/netbirdio/netbird/shared/management/status"
 )
 
@@ -373,3 +380,67 @@ func TestPreserveServiceMetadata(t *testing.T) {
 	assert.Equal(t, existing.SessionPrivateKey, updated.SessionPrivateKey)
 	assert.Equal(t, existing.SessionPublicKey, updated.SessionPublicKey)
 }
+
+func TestDeleteService_DeletesTargets(t *testing.T) {
+	ctx := context.Background()
+	accountID := "test-account"
+	userID := "test-user"
+
+	sqlStore, err := store.NewStore(ctx, storetypes.SqliteStoreEngine, t.TempDir(), nil, false)
+	require.NoError(t, err)
+
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	mockPerms := permissions.NewMockManager(ctrl)
+	mockAcct := account.NewMockManager(ctrl)
+	mockGRPC := &nbgrpc.ProxyServiceServer{}
+
+	mgr := &managerImpl{
+		store:              sqlStore,
+		permissionsManager: mockPerms,
+		accountManager:     mockAcct,
+		proxyGRPCServer:    mockGRPC,
+	}
+
+	service := &reverseproxy.Service{
+		ID:           "service-1",
+		AccountID:    accountID,
+		Domain:       "test.example.com",
+		ProxyCluster: "cluster1",
+		Enabled:      true,
+		Targets: []*reverseproxy.Target{
+			{AccountID: accountID, ServiceID: "service-1", TargetType: reverseproxy.TargetTypePeer, TargetId: "peer-1"},
+			{AccountID: accountID, ServiceID: "service-1", TargetType: reverseproxy.TargetTypePeer, TargetId: "peer-2"},
+			{AccountID: accountID, ServiceID: "service-1", TargetType: reverseproxy.TargetTypePeer, TargetId: "peer-3"},
+		},
+	}
+
+	err = sqlStore.CreateService(ctx, service)
+	require.NoError(t, err)
+
+	retrievedService, err := sqlStore.GetServiceByID(ctx, store.LockingStrengthNone, accountID, service.ID)
+	require.NoError(t, err)
+	require.Len(t, retrievedService.Targets, 3, "Service should have 3 targets before deletion")
+
+	mockPerms.EXPECT().
+		ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Delete).
+		Return(true, nil)
+	mockAcct.EXPECT().
+		StoreEvent(ctx, userID, service.ID, accountID, activity.ServiceDeleted, gomock.Any())
+	mockAcct.EXPECT().
+		UpdateAccountPeers(ctx, accountID)
+
+	err = mgr.DeleteService(ctx, accountID, userID, service.ID)
+	require.NoError(t, err)
+
+	_, err = sqlStore.GetServiceByID(ctx, store.LockingStrengthNone, accountID, service.ID)
+	require.Error(t, err)
+	s, ok := status.FromError(err)
+	require.True(t, ok)
+	assert.Equal(t, status.NotFound, s.Type())
+
+	targets, err := sqlStore.GetTargetsByServiceID(ctx, store.LockingStrengthNone, accountID, service.ID)
+	require.NoError(t, err)
+	assert.Len(t, targets, 0, "All targets should be deleted when service is deleted")
+}

management/server/account/manager.go

@@ -1,5 +1,7 @@
 package account
 
+//go:generate go run github.com/golang/mock/mockgen -package account -destination=manager_mock.go -source=./manager.go -build_flags=-mod=mod
+
 import (
 	"context"
 	"net"
@@ -61,11 +63,11 @@ type Manager interface {
 	GetPeers(ctx context.Context, accountID, userID, nameFilter, ipFilter string) ([]*nbpeer.Peer, error)
 	MarkPeerConnected(ctx context.Context, peerKey string, connected bool, realIP net.IP, accountID string, syncTime time.Time) error
 	DeletePeer(ctx context.Context, accountID, peerID, userID string) error
-	UpdatePeer(ctx context.Context, accountID, userID string, peer *nbpeer.Peer) (*nbpeer.Peer, error)
+	UpdatePeer(ctx context.Context, accountID, userID string, p *nbpeer.Peer) (*nbpeer.Peer, error)
 	UpdatePeerIP(ctx context.Context, accountID, userID, peerID string, newIP netip.Addr) error
 	GetNetworkMap(ctx context.Context, peerID string) (*types.NetworkMap, error)
 	GetPeerNetwork(ctx context.Context, peerID string) (*types.Network, error)
-	AddPeer(ctx context.Context, accountID, setupKey, userID string, peer *nbpeer.Peer, temporary bool) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, error)
+	AddPeer(ctx context.Context, accountID, setupKey, userID string, p *nbpeer.Peer, temporary bool) (*nbpeer.Peer, *types.NetworkMap, []*posture.Checks, error)
 	CreatePAT(ctx context.Context, accountID string, initiatorUserID string, targetUserID string, tokenName string, expiresIn int) (*types.PersonalAccessTokenGenerated, error)
 	DeletePAT(ctx context.Context, accountID string, initiatorUserID string, targetUserID string, tokenID string) error
 	GetPAT(ctx context.Context, accountID string, initiatorUserID string, targetUserID string, tokenID string) (*types.PersonalAccessToken, error)