PublicDashboards: Delete on folder deletion (#99040)

This commit is contained in:
Ezequiel Victorero
2025-01-23 17:23:59 -03:00
committed by GitHub
parent fdbac6fb54
commit 3a94057ec8
37 changed files with 550 additions and 312 deletions
+46 -20
View File
@@ -33,6 +33,7 @@ import (
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/guardian"
"github.com/grafana/grafana/pkg/services/publicdashboards"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/grafana/grafana/pkg/services/store/entity"
@@ -46,15 +47,16 @@ import (
const FULLPATH_SEPARATOR = "/"
type Service struct {
store folder.Store
unifiedStore folder.Store
db db.DB
log *slog.Logger
dashboardStore dashboards.Store
dashboardFolderStore folder.FolderStore
features featuremgmt.FeatureToggles
accessControl accesscontrol.AccessControl
k8sclient folderK8sHandler
store folder.Store
unifiedStore folder.Store
db db.DB
log *slog.Logger
dashboardStore dashboards.Store
dashboardFolderStore folder.FolderStore
features featuremgmt.FeatureToggles
accessControl accesscontrol.AccessControl
k8sclient folderK8sHandler
publicDashboardService publicdashboards.ServiceWrapper
// bus is currently used to publish event in case of folder full path change.
// For example when a folder is moved to another folder or when a folder is renamed.
bus bus.Bus
@@ -74,22 +76,24 @@ func ProvideService(
db db.DB, // DB for the (new) nested folder store
features featuremgmt.FeatureToggles,
supportBundles supportbundles.Service,
publicDashboardService publicdashboards.ServiceWrapper,
cfg *setting.Cfg,
r prometheus.Registerer,
tracer tracing.Tracer,
) *Service {
srv := &Service{
log: slog.Default().With("logger", "folder-service"),
dashboardStore: dashboardStore,
dashboardFolderStore: folderStore,
store: store,
features: features,
accessControl: ac,
bus: bus,
db: db,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(r),
tracer: tracer,
log: slog.Default().With("logger", "folder-service"),
dashboardStore: dashboardStore,
dashboardFolderStore: folderStore,
store: store,
features: features,
accessControl: ac,
bus: bus,
db: db,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(r),
tracer: tracer,
publicDashboardService: publicDashboardService,
}
srv.DBMigration(db)
@@ -1008,7 +1012,28 @@ func (s *Service) deleteChildrenInFolder(ctx context.Context, orgID int64, folde
}
func (s *Service) legacyDelete(ctx context.Context, cmd *folder.DeleteFolderCommand, folderUIDs []string) error {
// if dashboard restore is on we don't delete public dashboards, the hard delete will take care of it later
if !s.features.IsEnabledGlobally(featuremgmt.FlagDashboardRestore) {
// We need a list of dashboard uids inside the folder to delete related public dashboards
dashes, err := s.dashboardStore.FindDashboards(ctx, &dashboards.FindPersistedDashboardsQuery{SignedInUser: cmd.SignedInUser, FolderUIDs: folderUIDs, OrgId: cmd.OrgID})
if err != nil {
return folder.ErrInternal.Errorf("failed to fetch dashboards: %w", err)
}
dashboardUIDs := make([]string, 0, len(dashes))
for _, dashboard := range dashes {
dashboardUIDs = append(dashboardUIDs, dashboard.UID)
}
// Delete all public dashboards in the folders
err = s.publicDashboardService.DeleteByDashboardUIDs(ctx, cmd.OrgID, dashboardUIDs)
if err != nil {
return folder.ErrInternal.Errorf("failed to delete public dashboards: %w", err)
}
}
// TODO use bulk delete
// Delete all dashboards in the folders
for _, folderUID := range folderUIDs {
// nolint:staticcheck
deleteCmd := dashboards.DeleteDashboardCommand{OrgID: cmd.OrgID, UID: folderUID, ForceDeleteFolderRules: cmd.ForceDeleteRules}
@@ -1016,6 +1041,7 @@ func (s *Service) legacyDelete(ctx context.Context, cmd *folder.DeleteFolderComm
return toFolderError(err)
}
}
return nil
}
+58 -45
View File
@@ -41,6 +41,7 @@ import (
"github.com/grafana/grafana/pkg/services/ngalert/models"
ngstore "github.com/grafana/grafana/pkg/services/ngalert/store"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/publicdashboards"
"github.com/grafana/grafana/pkg/services/quota/quotatest"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/store/entity"
@@ -64,7 +65,7 @@ func TestIntegrationProvideFolderService(t *testing.T) {
db, cfg := db.InitTestDBWithCfg(t)
store := ProvideStore(db)
ProvideService(store, ac, bus.ProvideBus(tracing.InitializeTracerForTest()), nil, nil, db,
featuremgmt.WithFeatures(), supportbundlestest.NewFakeBundleService(), cfg, nil, tracing.InitializeTracerForTest())
featuremgmt.WithFeatures(), supportbundlestest.NewFakeBundleService(), nil, cfg, nil, tracing.InitializeTracerForTest())
require.Len(t, ac.Calls.RegisterAttributeScopeResolver, 2)
})
@@ -80,7 +81,7 @@ func TestIntegrationFolderService(t *testing.T) {
nestedFolderStore := ProvideStore(db)
folderStore := foldertest.NewFakeFolderStore(t)
publicDashboardService := publicdashboards.NewFakePublicDashboardServiceWrapper(t)
features := featuremgmt.WithFeatures()
alertingStore := ngstore.DBstore{
@@ -91,17 +92,18 @@ func TestIntegrationFolderService(t *testing.T) {
}
service := &Service{
log: slog.New(logtest.NewTestHandler(t)).With("logger", "test-folder-service"),
dashboardStore: dashStore,
dashboardFolderStore: folderStore,
store: nestedFolderStore,
features: features,
bus: bus.ProvideBus(tracing.InitializeTracerForTest()),
db: db,
accessControl: acimpl.ProvideAccessControl(features),
metrics: newFoldersMetrics(nil),
registry: make(map[string]folder.RegistryService),
tracer: tracing.InitializeTracerForTest(),
log: slog.New(logtest.NewTestHandler(t)).With("logger", "test-folder-service"),
dashboardStore: dashStore,
dashboardFolderStore: folderStore,
store: nestedFolderStore,
publicDashboardService: publicDashboardService,
features: features,
bus: bus.ProvideBus(tracing.InitializeTracerForTest()),
db: db,
accessControl: acimpl.ProvideAccessControl(features),
metrics: newFoldersMetrics(nil),
registry: make(map[string]folder.RegistryService),
tracer: tracing.InitializeTracerForTest(),
}
require.NoError(t, service.RegisterService(alertingStore))
@@ -263,11 +265,13 @@ func TestIntegrationFolderService(t *testing.T) {
f := folder.NewFolder(util.GenerateShortUID(), "")
f.UID = util.GenerateShortUID()
folderStore.On("GetFolderByUID", mock.Anything, orgID, f.UID).Return(f, nil)
publicDashboardService.On("DeleteByDashboardUIDs", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
var actualCmd *dashboards.DeleteDashboardCommand
dashStore.On("DeleteDashboard", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
actualCmd = args.Get(1).(*dashboards.DeleteDashboardCommand)
}).Return(nil).Once()
dashStore.On("FindDashboards", mock.Anything, mock.Anything).Return([]dashboards.DashboardSearchProjection{}, nil).Once()
expectedForceDeleteRules := rand.Int63()%2 == 0
err := service.Delete(context.Background(), &folder.DeleteFolderCommand{
@@ -427,22 +431,24 @@ func TestIntegrationNestedFolderService(t *testing.T) {
dashStore, err := database.ProvideDashboardStore(db, cfg, featuresFlagOn, tagimpl.ProvideService(db))
require.NoError(t, err)
nestedFolderStore := ProvideStore(db)
publicDashboardFakeService := publicdashboards.NewFakePublicDashboardServiceWrapper(t)
b := bus.ProvideBus(tracing.InitializeTracerForTest())
ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures())
serviceWithFlagOn := &Service{
log: slog.New(logtest.NewTestHandler(t)).With("logger", "test-folder-service"),
dashboardStore: dashStore,
dashboardFolderStore: folderStore,
store: nestedFolderStore,
features: featuresFlagOn,
bus: b,
db: db,
accessControl: ac,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
log: slog.New(logtest.NewTestHandler(t)).With("logger", "test-folder-service"),
dashboardStore: dashStore,
dashboardFolderStore: folderStore,
store: nestedFolderStore,
features: featuresFlagOn,
bus: b,
db: db,
accessControl: ac,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
publicDashboardService: publicDashboardFakeService,
}
signedInUser := user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{
@@ -485,8 +491,9 @@ func TestIntegrationNestedFolderService(t *testing.T) {
// CanEditValue is required to create library elements
CanEditValue: true,
})
publicDashboardFakeService.On("DeleteByDashboardUIDs", mock.Anything, mock.Anything, mock.Anything).Return(nil)
dashSrv, err := dashboardservice.ProvideDashboardServiceImpl(cfg, dashStore, folderStore, featuresFlagOn, folderPermissions, ac, serviceWithFlagOn, nestedFolderStore, nil, nil, nil, nil, quotaService, nil)
dashSrv, err := dashboardservice.ProvideDashboardServiceImpl(cfg, dashStore, folderStore, featuresFlagOn, folderPermissions, ac, serviceWithFlagOn, nestedFolderStore, nil, nil, nil, nil, quotaService, nil, publicDashboardFakeService)
require.NoError(t, err)
dashSrv.RegisterDashboardPermissions(dashboardPermissions)
@@ -548,16 +555,17 @@ func TestIntegrationNestedFolderService(t *testing.T) {
nestedFolderStore := ProvideStore(db)
serviceWithFlagOff := &Service{
log: slog.New(logtest.NewTestHandler(t)).With("logger", "test-folder-service"),
dashboardStore: dashStore,
dashboardFolderStore: folderStore,
store: nestedFolderStore,
features: featuresFlagOff,
bus: b,
db: db,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
log: slog.New(logtest.NewTestHandler(t)).With("logger", "test-folder-service"),
dashboardStore: dashStore,
dashboardFolderStore: folderStore,
store: nestedFolderStore,
features: featuresFlagOff,
bus: b,
db: db,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
publicDashboardService: publicDashboardFakeService,
}
origNewGuardian := guardian.New
@@ -568,8 +576,10 @@ func TestIntegrationNestedFolderService(t *testing.T) {
CanEditValue: true,
})
publicDashboardFakeService.On("DeleteByDashboardUIDs", mock.Anything, mock.Anything, mock.Anything).Return(nil)
dashSrv, err := dashboardservice.ProvideDashboardServiceImpl(cfg, dashStore, folderStore, featuresFlagOff,
folderPermissions, ac, serviceWithFlagOff, nestedFolderStore, nil, nil, nil, nil, quotaService, nil)
folderPermissions, ac, serviceWithFlagOff, nestedFolderStore, nil, nil, nil, nil, quotaService, nil, publicDashboardFakeService)
require.NoError(t, err)
dashSrv.RegisterDashboardPermissions(dashboardPermissions)
alertStore, err := ngstore.ProvideDBStore(cfg, featuresFlagOff, db, serviceWithFlagOff, dashSrv, ac, b)
@@ -628,14 +638,15 @@ func TestIntegrationNestedFolderService(t *testing.T) {
t.Run("Should delete folders", func(t *testing.T) {
featuresFlagOff := featuremgmt.WithFeatures()
serviceWithFlagOff := &Service{
log: slog.New(logtest.NewTestHandler(t)).With("logger", "test-folder-service"),
dashboardFolderStore: folderStore,
features: featuresFlagOff,
bus: b,
db: db,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
log: slog.New(logtest.NewTestHandler(t)).With("logger", "test-folder-service"),
dashboardFolderStore: folderStore,
features: featuresFlagOff,
bus: b,
db: db,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
publicDashboardService: publicDashboardFakeService,
}
testCases := []struct {
@@ -713,8 +724,9 @@ func TestIntegrationNestedFolderService(t *testing.T) {
nestedFolderStore := ProvideStore(db)
tc.service.dashboardStore = dashStore
tc.service.store = nestedFolderStore
publicDashboardFakeService.On("DeleteByDashboardUIDs", mock.Anything, mock.Anything, mock.Anything).Return(nil)
dashSrv, err := dashboardservice.ProvideDashboardServiceImpl(cfg, dashStore, folderStore, tc.featuresFlag, folderPermissions, ac, tc.service, tc.service.store, nil, nil, nil, nil, quotaService, nil)
dashSrv, err := dashboardservice.ProvideDashboardServiceImpl(cfg, dashStore, folderStore, tc.featuresFlag, folderPermissions, ac, tc.service, tc.service.store, nil, nil, nil, nil, quotaService, nil, publicDashboardFakeService)
require.NoError(t, err)
dashSrv.RegisterDashboardPermissions(dashboardPermissions)
@@ -1502,6 +1514,7 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) {
nil,
quotaService,
nil,
nil,
)
require.NoError(t, err)
dashboardService.RegisterDashboardPermissions(dashboardPermissions)
@@ -586,6 +586,26 @@ func (s *Service) deleteFromApiServer(ctx context.Context, cmd *folder.DeleteFol
if alertRulesInFolder > 0 {
return folder.ErrFolderNotEmpty.Errorf("folder contains %d alert rules", alertRulesInFolder)
}
// if dashboard restore is on we don't delete public dashboards, the hard delete will take care of it later
if !s.features.IsEnabledGlobally(featuremgmt.FlagDashboardRestore) {
// We need a list of dashboard uids inside the folder to delete related public dashboards
dashes, err := s.dashboardStore.FindDashboards(ctx, &dashboards.FindPersistedDashboardsQuery{SignedInUser: cmd.SignedInUser, FolderUIDs: folders, OrgId: cmd.OrgID})
if err != nil {
return folder.ErrInternal.Errorf("failed to fetch dashboards: %w", err)
}
dashboardUIDs := make([]string, 0, len(dashes))
for _, dashboard := range dashes {
dashboardUIDs = append(dashboardUIDs, dashboard.UID)
}
// Delete all public dashboards in the folders
err = s.publicDashboardService.DeleteByDashboardUIDs(ctx, cmd.OrgID, dashboardUIDs)
if err != nil {
return folder.ErrInternal.Errorf("failed to delete public dashboards: %w", err)
}
}
}
err = s.unifiedStore.Delete(ctx, folders, cmd.OrgID)
@@ -10,6 +10,7 @@ import (
"net/http/httptest"
"testing"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"google.golang.org/grpc"
clientrest "k8s.io/client-go/rest"
@@ -31,6 +32,7 @@ import (
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/guardian"
ngstore "github.com/grafana/grafana/pkg/services/ngalert/store"
"github.com/grafana/grafana/pkg/services/publicdashboards"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/storage/unified/resource"
@@ -202,18 +204,21 @@ func TestIntegrationFolderServiceViaUnifiedStorage(t *testing.T) {
featuresArr := []any{
featuremgmt.FlagKubernetesFoldersServiceV2}
features := featuremgmt.WithFeatures(featuresArr...)
dashboardStore := dashboards.NewFakeDashboardStore(t)
publicDashboardService := publicdashboards.NewFakePublicDashboardServiceWrapper(t)
folderService := &Service{
log: slog.New(logtest.NewTestHandler(t)).With("logger", "test-folder-service"),
unifiedStore: unifiedStore,
features: features,
bus: bus.ProvideBus(tracing.InitializeTracerForTest()),
// db: db,
accessControl: acimpl.ProvideAccessControl(features),
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
k8sclient: k8sHandler,
log: slog.New(logtest.NewTestHandler(t)).With("logger", "test-folder-service"),
unifiedStore: unifiedStore,
features: features,
bus: bus.ProvideBus(tracing.InitializeTracerForTest()),
accessControl: acimpl.ProvideAccessControl(features),
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
k8sclient: k8sHandler,
dashboardStore: dashboardStore,
publicDashboardService: publicDashboardService,
}
require.NoError(t, folderService.RegisterService(alertingStore))
@@ -345,6 +350,9 @@ func TestIntegrationFolderServiceViaUnifiedStorage(t *testing.T) {
})
t.Run("When deleting folder by uid should not return access denied error - ForceDeleteRules false", func(t *testing.T) {
dashboardStore.On("FindDashboards", mock.Anything, mock.Anything).Return([]dashboards.DashboardSearchProjection{}, nil)
publicDashboardService.On("DeleteByDashboardUIDs", mock.Anything, mock.Anything, mock.Anything).Return(nil)
err := folderService.Delete(ctx, &folder.DeleteFolderCommand{
UID: "deletefolder",
OrgID: orgID,