[management] Add explicit target delete on service removal

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

@@ -385,6 +385,12 @@ func (m *managerImpl) DeleteService(ctx context.Context, accountID, userID, serv
 			return err
 		}
 
+		for _, target := range service.Targets {
+			if err := transaction.DeleteTarget(ctx, accountID, serviceID, target.ID); err != nil {
+				return fmt.Errorf("failed to delete target: %w", err)
+			}
+		}
+
 		if err = transaction.DeleteService(ctx, accountID, serviceID); err != nil {
 			return fmt.Errorf("failed to delete service: %w", err)
 		}

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

@@ -11,7 +11,14 @@
 	"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 @@
 	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/store/sql_store.go

@@ -4882,6 +4882,36 @@ func (s *SqlStore) DeleteService(ctx context.Context, accountID, serviceID strin
 	return nil
 }
 
+func (s *SqlStore) DeleteTarget(ctx context.Context, accountID string, serviceID string, targetID uint) error {
+	result := s.db.Delete(&reverseproxy.Target{}, "account_id = ? AND service_id = ? AND id = ?", accountID, serviceID, targetID)
+	if result.Error != nil {
+		log.WithContext(ctx).Errorf("failed to delete target from store: %v", result.Error)
+		return status.Errorf(status.Internal, "failed to delete target from store")
+	}
+
+	if result.RowsAffected == 0 {
+		return status.Errorf(status.NotFound, "target not found for service %s", serviceID)
+	}
+
+	return nil
+}
+
+// GetTargetsByServiceID retrieves all targets for a given service
+func (s *SqlStore) GetTargetsByServiceID(ctx context.Context, lockStrength LockingStrength, accountID string, serviceID string) ([]*reverseproxy.Target, error) {
+	var targets []*reverseproxy.Target
+	tx := s.db
+	if lockStrength != LockingStrengthNone {
+		tx = tx.Clauses(clause.Locking{Strength: string(lockStrength)})
+	}
+	result := tx.Where("account_id = ? AND service_id = ?", accountID, serviceID).Find(&targets)
+	if result.Error != nil {
+		log.WithContext(ctx).Errorf("failed to get targets from store: %v", result.Error)
+		return nil, status.Errorf(status.Internal, "failed to get targets from store")
+	}
+
+	return targets, nil
+}
+
 func (s *SqlStore) GetServiceByID(ctx context.Context, lockStrength LockingStrength, accountID, serviceID string) (*reverseproxy.Service, error) {
 	tx := s.db.Preload("Targets")
 	if lockStrength != LockingStrengthNone {

management/server/store/store.go

@@ -271,6 +271,8 @@ type Store interface {
 	GetAccountAccessLogs(ctx context.Context, lockStrength LockingStrength, accountID string, filter accesslogs.AccessLogFilter) ([]*accesslogs.AccessLogEntry, int64, error)
 	DeleteOldAccessLogs(ctx context.Context, olderThan time.Time) (int64, error)
 	GetServiceTargetByTargetID(ctx context.Context, lockStrength LockingStrength, accountID string, targetID string) (*reverseproxy.Target, error)
+	GetTargetsByServiceID(ctx context.Context, lockStrength LockingStrength, accountID string, serviceID string) ([]*reverseproxy.Target, error)
+	DeleteTarget(ctx context.Context, accountID string, serviceID string, targetID uint) error
 }
 
 const (