Orgs: Remove dependency on dashboard table for deletion (#98501)

pkg/api/http_server.go

@@ -205,6 +205,7 @@ type HTTPServer struct {
 	tempUserService      tempUser.Service
 	loginAttemptService  loginAttempt.Service
 	orgService           org.Service
+	orgDeletionService   org.DeletionService
 	TeamService          team.Service
 	accesscontrolService accesscontrol.Service
 	annotationsRepo      annotations.Repository
@@ -264,7 +265,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
 	secretsMigrator secrets.Migrator, secretsPluginManager plugins.SecretsPluginManager, secretsService secrets.Service,
 	secretsPluginMigrator spm.SecretMigrationProvider, secretsStore secretsKV.SecretsKVStore,
 	publicDashboardsApi *publicdashboardsApi.Api, userService user.Service, tempUserService tempUser.Service,
-	loginAttemptService loginAttempt.Service, orgService org.Service, teamService team.Service,
+	loginAttemptService loginAttempt.Service, orgService org.Service, orgDeletionService org.DeletionService, teamService team.Service,
 	accesscontrolService accesscontrol.Service, navTreeService navtree.Service,
 	annotationRepo annotations.Repository, tagService tag.Service, searchv2HTTPService searchV2.SearchHTTPService, oauthTokenService oauthtoken.OAuthTokenService,
 	statsService stats.Service, authnService authn.Service, pluginsCDNService *pluginscdn.Service, promGatherer prometheus.Gatherer,
@@ -356,6 +357,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
 		tempUserService:              tempUserService,
 		loginAttemptService:          loginAttemptService,
 		orgService:                   orgService,
+		orgDeletionService:           orgDeletionService,
 		TeamService:                  teamService,
 		navTreeService:               navTreeService,
 		accesscontrolService:         accesscontrolService,

pkg/api/org.go

@@ -297,7 +297,7 @@ func (hs *HTTPServer) DeleteOrgByID(c *contextmodel.ReqContext) response.Respons
 		return response.Error(http.StatusBadRequest, "Can not delete org for current user", nil)
 	}
 
-	if err := hs.orgService.Delete(c.Req.Context(), &org.DeleteOrgCommand{ID: orgID}); err != nil {
+	if err := hs.orgDeletionService.Delete(c.Req.Context(), &org.DeleteOrgCommand{ID: orgID}); err != nil {
 		if errors.Is(err, org.ErrOrgNotFound) {
 			return response.Error(http.StatusNotFound, "Failed to delete organization. ID not found", nil)
 		}

pkg/api/org_test.go

@@ -228,6 +228,7 @@ func TestAPIEndpoint_DeleteOrgs(t *testing.T) {
 			server := SetupAPITestServer(t, func(hs *HTTPServer) {
 				hs.Cfg = setting.NewCfg()
 				hs.orgService = &orgtest.FakeOrgService{ExpectedOrg: &org.Org{}}
+				hs.orgDeletionService = &orgtest.FakeOrgDeletionService{}
 				hs.userService = &usertest.FakeUserService{ExpectedSignedInUser: &user.SignedInUser{OrgID: 1}}
 				hs.accesscontrolService = actest.FakeService{ExpectedPermissions: tt.permission}
 				hs.authnService = &authntest.FakeService{}

pkg/server/wire.go

@@ -337,6 +337,7 @@ var wireBasicSet = wire.NewSet(
 	starApi.ProvideApi,
 	userimpl.ProvideService,
 	orgimpl.ProvideService,
+	orgimpl.ProvideDeletionService,
 	statsimpl.ProvideService,
 	grpccontext.ProvideContextHandler,
 	grpcserver.ProvideService,

pkg/services/dashboards/dashboard.go

@@ -16,6 +16,7 @@ import (
 type DashboardService interface {
 	BuildSaveDashboardCommand(ctx context.Context, dto *SaveDashboardDTO, validateProvisionedDashboard bool) (*SaveDashboardCommand, error)
 	DeleteDashboard(ctx context.Context, dashboardId int64, dashboardUID string, orgId int64) error
+	DeleteAllDashboards(ctx context.Context, orgID int64) error
 	FindDashboards(ctx context.Context, query *FindPersistedDashboardsQuery) ([]DashboardSearchProjection, error)
 	// GetDashboard fetches a dashboard.
 	// To fetch a dashboard under root by title should set the folder UID to point to an empty string
@@ -60,6 +61,7 @@ type DashboardProvisioningService interface {
 //go:generate mockery --name Store --structname FakeDashboardStore --inpackage --filename store_mock.go
 type Store interface {
 	DeleteDashboard(ctx context.Context, cmd *DeleteDashboardCommand) error
+	DeleteAllDashboards(ctx context.Context, orgID int64) error
 	DeleteOrphanedProvisionedDashboards(ctx context.Context, cmd *DeleteOrphanedProvisionedDashboardsCommand) error
 	FindDashboards(ctx context.Context, query *FindPersistedDashboardsQuery) ([]DashboardSearchProjection, error)
 	GetDashboard(ctx context.Context, query *GetDashboardQuery) (*Dashboard, error)

pkg/services/dashboards/dashboard_service_mock.go

@@ -120,6 +120,24 @@ func (_m *FakeDashboardService) DeleteDashboard(ctx context.Context, dashboardId
 	return r0
 }
 
+// DeleteAllDashboards provides a mock function with given fields: ctx, orgID
+func (_m *FakeDashboardService) DeleteAllDashboards(ctx context.Context,  orgID int64) error {
+	ret := _m.Called(ctx, orgID)
+
+	if len(ret) == 0 {
+		panic("no return value specified for DeleteDashboard")
+	}
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, int64) error); ok {
+		r0 = rf(ctx, orgID)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
 // FindDashboards provides a mock function with given fields: ctx, query
 func (_m *FakeDashboardService) FindDashboards(ctx context.Context, query *FindPersistedDashboardsQuery) ([]DashboardSearchProjection, error) {
 	ret := _m.Called(ctx, query)

pkg/services/dashboards/database/database.go

@@ -667,6 +667,16 @@ func (d *dashboardStore) deleteDashboard(cmd *dashboards.DeleteDashboardCommand,
 	return nil
 }
 
+func (d *dashboardStore) DeleteAllDashboards(ctx context.Context, orgID int64) error {
+	ctx, span := tracer.Start(ctx, "dashboards.database.DeleteAllDashboards")
+	defer span.End()
+
+	return d.store.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
+		_, err := sess.Where("org_id = ?", orgID).Delete(&dashboards.Dashboard{})
+		return err
+	})
+}
+
 // FIXME: Remove me and handle nested deletions in the service with the DashboardPermissionsService
 func (d *dashboardStore) deleteResourcePermissions(sess *db.Session, orgID int64, resourceScope string) error {
 	// retrieve all permissions for the resource scope and org id

pkg/services/dashboards/database/database_test.go

@@ -240,6 +240,32 @@ func TestIntegrationDashboardDataAccess(t *testing.T) {
 		require.NotContains(t, terms, "delete this")
 	})
 
+	t.Run("Should be able to delete all dashboards for an org", func(t *testing.T) {
+		setup()
+		dash1 := insertTestDashboard(t, dashboardStore, "delete me", 1, 0, "", false, "delete this")
+		dash2 := insertTestDashboard(t, dashboardStore, "delete me2", 1, 0, "", false, "delete this2")
+		dash3 := insertTestDashboard(t, dashboardStore, "dont delete me", 2, 0, "", false, "dont delete me")
+
+		err := dashboardStore.DeleteAllDashboards(context.Background(), 1)
+		require.NoError(t, err)
+
+		// no dashboards should exist for org 1
+		queryResult, err := dashboardStore.GetDashboards(context.Background(), &dashboards.GetDashboardsQuery{
+			OrgID:         1,
+			DashboardUIDs: []string{dash1.UID, dash2.UID},
+		})
+		require.NoError(t, err)
+		assert.Equal(t, len(queryResult), 0)
+
+		// but we should still have one for org 2
+		queryResult, err = dashboardStore.GetDashboards(context.Background(), &dashboards.GetDashboardsQuery{
+			OrgID:         2,
+			DashboardUIDs: []string{dash3.UID},
+		})
+		require.NoError(t, err)
+		assert.Equal(t, len(queryResult), 1)
+	})
+
 	t.Run("Should be able to create dashboard", func(t *testing.T) {
 		setup()
 		cmd := dashboards.SaveDashboardCommand{

pkg/services/dashboards/service/dashboard_service.go

@@ -511,6 +511,15 @@ func (dr *DashboardServiceImpl) DeleteDashboard(ctx context.Context, dashboardId
 	return dr.deleteDashboard(ctx, dashboardId, dashboardUID, orgId, true)
 }
 
+// DeleteAllDashboards will delete all dashboards within a given org.
+func (dr *DashboardServiceImpl) DeleteAllDashboards(ctx context.Context, orgId int64) error {
+	if dr.features.IsEnabledGlobally(featuremgmt.FlagKubernetesCliDashboards) {
+		return dr.deleteAllDashboardThroughK8s(ctx, orgId)
+	}
+
+	return dr.dashboardStore.DeleteAllDashboards(ctx, orgId)
+}
+
 func (dr *DashboardServiceImpl) GetDashboardByPublicUid(ctx context.Context, dashboardPublicUid string) (*dashboards.Dashboard, error) {
 	return nil, nil
 }
@@ -1178,6 +1187,29 @@ func (dr *DashboardServiceImpl) saveDashboardThroughK8s(ctx context.Context, cmd
 	return finalDash, nil
 }
 
+func (dr *DashboardServiceImpl) deleteAllDashboardThroughK8s(ctx context.Context, orgID int64) error {
+	// create a new context - prevents issues when the request stems from the k8s api itself
+	// otherwise the context goes through the handlers twice and causes issues
+	newCtx, cancel, err := dr.getK8sContext(ctx)
+	if err != nil {
+		return err
+	} else if cancel != nil {
+		defer cancel()
+	}
+
+	client, ok := dr.k8sclient.getClient(newCtx, orgID)
+	if !ok {
+		return fmt.Errorf("could not get k8s client")
+	}
+
+	err = client.DeleteCollection(newCtx, v1.DeleteOptions{}, v1.ListOptions{})
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func (dr *DashboardServiceImpl) deleteDashboardThroughK8s(ctx context.Context, cmd *dashboards.DeleteDashboardCommand) error {
 	// create a new context - prevents issues when the request stems from the k8s api itself
 	// otherwise the context goes through the handlers twice and causes issues

pkg/services/dashboards/service/dashboard_service_test.go

@@ -305,6 +305,11 @@ func (m *mockResourceInterface) Delete(ctx context.Context, name string, options
 	return args.Error(0)
 }
 
+func (m *mockResourceInterface) DeleteCollection(ctx context.Context, options metav1.DeleteOptions, listOptions metav1.ListOptions) error {
+	args := m.Called(ctx, options, listOptions)
+	return args.Error(0)
+}
+
 func setupK8sDashboardTests(service *DashboardServiceImpl) (context.Context, *mockDashK8sCli, *mockResourceInterface) {
 	k8sClientMock := new(mockDashK8sCli)
 	k8sResourceMock := new(mockResourceInterface)
@@ -644,6 +649,33 @@ func TestDeleteDashboard(t *testing.T) {
 	})
 }
 
+func TestDeleteAllDashboards(t *testing.T) {
+	fakeStore := dashboards.FakeDashboardStore{}
+	defer fakeStore.AssertExpectations(t)
+	service := &DashboardServiceImpl{
+		cfg:            setting.NewCfg(),
+		dashboardStore: &fakeStore,
+	}
+
+	t.Run("Should fallback to dashboard store if Kubernetes feature flags are not enabled", func(t *testing.T) {
+		service.features = featuremgmt.WithFeatures()
+		fakeStore.On("DeleteAllDashboards", mock.Anything, mock.Anything).Return(nil).Once()
+		err := service.DeleteAllDashboards(context.Background(), 1)
+		require.NoError(t, err)
+		fakeStore.AssertExpectations(t)
+	})
+
+	t.Run("Should use Kubernetes client if feature flags are enabled", func(t *testing.T) {
+		ctx, k8sClientMock, k8sResourceMock := setupK8sDashboardTests(service)
+		k8sClientMock.On("getClient", mock.Anything, int64(1)).Return(k8sResourceMock, true).Once()
+		k8sResourceMock.On("DeleteCollection", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
+
+		err := service.DeleteAllDashboards(ctx, 1)
+		require.NoError(t, err)
+		k8sClientMock.AssertExpectations(t)
+	})
+}
+
 func TestSearchDashboards(t *testing.T) {
 	fakeStore := dashboards.FakeDashboardStore{}
 	defer fakeStore.AssertExpectations(t)

pkg/services/dashboards/store_mock.go

@@ -94,6 +94,24 @@ func (_m *FakeDashboardStore) DeleteDashboard(ctx context.Context, cmd *DeleteDa
 	return r0
 }
 
+// DeleteAllDashboards provides a mock function with given fields: ctx, orgID
+func (_m *FakeDashboardStore) DeleteAllDashboards(ctx context.Context, orgID int64) error {
+	ret := _m.Called(ctx, orgID)
+
+	if len(ret) == 0 {
+		panic("no return value specified for DeleteDashboard")
+	}
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, int64) error); ok {
+		r0 = rf(ctx, orgID)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
 // DeleteDashboardsInFolders provides a mock function with given fields: ctx, request
 func (_m *FakeDashboardStore) DeleteDashboardsInFolders(ctx context.Context, request *DeleteDashboardsInFolderRequest) error {
 	ret := _m.Called(ctx, request)

pkg/services/folder/folderimpl/sqlstore_test.go

@@ -11,10 +11,12 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/folder"
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/org/orgimpl"
@@ -975,10 +977,14 @@ func CreateOrg(t *testing.T, db db.DB, cfg *setting.Cfg) int64 {
 
 	orgService, err := orgimpl.ProvideService(db, cfg, quotatest.New(false, nil))
 	require.NoError(t, err)
+	dashSvc := &dashboards.FakeDashboardService{}
+	dashSvc.On("DeleteAllDashboards", mock.Anything, mock.Anything).Return(nil)
+	deleteOrgService, err := orgimpl.ProvideDeletionService(db, cfg, dashSvc)
+	require.NoError(t, err)
 	orgID, err := orgService.GetOrCreate(context.Background(), "test-org")
 	require.NoError(t, err)
 	t.Cleanup(func() {
-		err = orgService.Delete(context.Background(), &org.DeleteOrgCommand{ID: orgID})
+		err = deleteOrgService.Delete(context.Background(), &org.DeleteOrgCommand{ID: orgID})
 		require.NoError(t, err)
 	})
 

pkg/services/org/org.go

@@ -16,7 +16,6 @@ type Service interface {
 	GetByName(context.Context, *GetOrgByNameQuery) (*Org, error)
 	CreateWithMember(context.Context, *CreateOrgCommand) (*Org, error)
 	UpdateAddress(context.Context, *UpdateOrgAddressCommand) error
-	Delete(context.Context, *DeleteOrgCommand) error
 	GetOrCreate(context.Context, string) (int64, error)
 	AddOrgUser(context.Context, *AddOrgUserCommand) error
 	UpdateOrgUser(context.Context, *UpdateOrgUserCommand) error
@@ -25,3 +24,7 @@ type Service interface {
 	SearchOrgUsers(context.Context, *SearchOrgUsersQuery) (*SearchOrgUsersQueryResult, error)
 	RegisterDelete(query string)
 }
+
+type DeletionService interface {
+	Delete(context.Context, *DeleteOrgCommand) error
+}

pkg/services/org/orgimpl/org.go

@@ -138,11 +138,6 @@ func (s *Service) UpdateAddress(ctx context.Context, cmd *org.UpdateOrgAddressCo
 	return s.store.UpdateAddress(ctx, cmd)
 }
 
-// TODO: refactor service to call store CRUD method
-func (s *Service) Delete(ctx context.Context, cmd *org.DeleteOrgCommand) error {
-	return s.store.Delete(ctx, cmd)
-}
-
 func (s *Service) GetOrCreate(ctx context.Context, orgName string) (int64, error) {
 	var orga = &org.Org{}
 	var err error

pkg/services/org/orgimpl/org_delete_svc.go

@@ -0,0 +1,43 @@
+package orgimpl
+
+import (
+	"context"
+
+	"github.com/grafana/grafana/pkg/infra/db"
+	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/services/dashboards"
+	"github.com/grafana/grafana/pkg/services/org"
+	"github.com/grafana/grafana/pkg/setting"
+)
+
+type DeletionService struct {
+	store   store
+	cfg     *setting.Cfg
+	log     log.Logger
+	dashSvc dashboards.DashboardService
+}
+
+func ProvideDeletionService(db db.DB, cfg *setting.Cfg, dashboardService dashboards.DashboardService) (org.DeletionService, error) {
+	log := log.New("org deletion service")
+	s := &DeletionService{
+		store: &sqlStore{
+			db:      db,
+			dialect: db.GetDialect(),
+			log:     log,
+		},
+		cfg:     cfg,
+		dashSvc: dashboardService,
+		log:     log,
+	}
+
+	return s, nil
+}
+
+func (s *DeletionService) Delete(ctx context.Context, cmd *org.DeleteOrgCommand) error {
+	err := s.dashSvc.DeleteAllDashboards(ctx, cmd.ID)
+	if err != nil {
+		return err
+	}
+
+	return s.store.Delete(ctx, cmd)
+}

pkg/services/org/orgimpl/store.go

@@ -220,9 +220,8 @@ func (ss *sqlStore) Delete(ctx context.Context, cmd *org.DeleteOrgCommand) error
 		}
 
 		deletes := []string{
-			"DELETE FROM star WHERE EXISTS (SELECT 1 FROM dashboard WHERE org_id = ? AND star.dashboard_uid = dashboard.uid)",
-			"DELETE FROM dashboard_tag WHERE EXISTS (SELECT 1 FROM dashboard WHERE org_id = ? AND dashboard_tag.dashboard_id = dashboard.id)",
-			"DELETE FROM dashboard WHERE org_id = ?",
+			"DELETE FROM star WHERE org_id = ?",
+			"DELETE FROM dashboard_tag WHERE org_id = ?",
 			"DELETE FROM api_key WHERE org_id = ?",
 			"DELETE FROM data_source WHERE org_id = ?",
 			"DELETE FROM org_user WHERE org_id = ?",

pkg/services/org/orgtest/fake.go

@@ -75,10 +75,6 @@ func (f *FakeOrgService) UpdateAddress(ctx context.Context, cmd *org.UpdateOrgAd
 	return f.ExpectedError
 }
 
-func (f *FakeOrgService) Delete(ctx context.Context, cmd *org.DeleteOrgCommand) error {
-	return f.ExpectedError
-}
-
 func (f *FakeOrgService) GetOrCreate(ctx context.Context, orgName string) (int64, error) {
 	return f.ExpectedOrg.ID, f.ExpectedError
 }
@@ -107,3 +103,15 @@ func (f *FakeOrgService) SearchOrgUsers(ctx context.Context, query *org.SearchOr
 
 func (f *FakeOrgService) RegisterDelete(query string) {
 }
+
+type FakeOrgDeletionService struct {
+	ExpectedError error
+}
+
+func NewOrgDeletionServiceFake() *FakeOrgDeletionService {
+	return &FakeOrgDeletionService{}
+}
+
+func (f *FakeOrgDeletionService) Delete(ctx context.Context, cmd *org.DeleteOrgCommand) error {
+	return f.ExpectedError
+}