public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Alerting: Fix deleting rules in a folder with matching UID in another organization (#78258)

Browse Source 
* Remove usage of obsolete function for deleting alert rules under folder

* Apply suggestion from code review

* Update tests
This commit is contained in:
Sofia Papagiannaki
2023-12-04 11:34:38 +02:00
committed by GitHub
parent 4e2201ffeb
commit 6d4625ad52
15 changed files with 93 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/folder/folderimpl/folder.go
+13
View File
@@ -642,6 +642,19 @@ func (s *Service) Delete(ctx context.Context, cmd *folder.DeleteFolderCommand) e
if err := s.deleteChildrenInFolder(ctx, dashFolder.OrgID, dashFolder.UID, cmd.SignedInUser); err != nil {
return err
}
} else {
alertRuleSrv, ok := s.registry[entity.StandardKindAlertRule]
if !ok {
return folder.ErrInternal.Errorf("no alert rule service found in registry")
}
alertRulesInFolder, err := alertRuleSrv.CountInFolder(ctx, dashFolder.OrgID, dashFolder.UID, cmd.SignedInUser)
if err != nil {
s.log.Error("failed to count alert rules in folder", "error", err)
return err
}
if alertRulesInFolder > 0 {
return folder.ErrFolderNotEmpty.Errorf("folder contains %d alert rules", alertRulesInFolder)
}
}
if err = s.legacyDelete(ctx, cmd, dashFolder); err != nil {
pkg/services/folder/folderimpl/folder_test.go
+22 -7
View File
@@ -74,6 +74,16 @@ func TestIntegrationFolderService(t *testing.T) {
cfg := setting.NewCfg()
features := featuremgmt.WithFeatures()
ac := acmock.New().WithPermissions([]accesscontrol.Permission{
{Action: accesscontrol.ActionAlertingRuleDelete, Scope: dashboards.ScopeFoldersAll},
})
alertingStore := ngstore.DBstore{
SQLStore: db,
Cfg: cfg.UnifiedAlerting,
Logger: log.New("test-alerting-store"),
AccessControl: ac,
}
service := &Service{
cfg: cfg,
log: log.New("test-folder-service"),
@@ -84,8 +94,11 @@ func TestIntegrationFolderService(t *testing.T) {
bus: bus.ProvideBus(tracing.InitializeTracerForTest()),
db: db,
accessControl: acimpl.ProvideAccessControl(cfg),
registry: make(map[string]folder.RegistryService),
}
require.NoError(t, service.RegisterService(alertingStore))
t.Run("Given user has no permissions", func(t *testing.T) {
origNewGuardian := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{})
@@ -382,8 +395,10 @@ func TestIntegrationNestedFolderService(t *testing.T) {
signedInUser := user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{
orgID: {
dashboards.ActionFoldersCreate: {},
dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersAll}},
dashboards.ActionFoldersCreate: {},
dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersAll},
accesscontrol.ActionAlertingRuleDelete: {dashboards.ScopeFoldersAll},
},
}}
createCmd := folder.CreateFolderCommand{
OrgID: orgID,
@@ -422,7 +437,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
dashSrv, err := service.ProvideDashboardServiceImpl(cfg, dashStore, folderStore, nil, featuresFlagOn, folderPermissions, dashboardPermissions, ac, serviceWithFlagOn)
require.NoError(t, err)
alertStore, err := ngstore.ProvideDBStore(cfg, featuresFlagOn, db, serviceWithFlagOn, dashSrv)
alertStore, err := ngstore.ProvideDBStore(cfg, featuresFlagOn, db, serviceWithFlagOn, dashSrv, ac)
require.NoError(t, err)
elementService := libraryelements.ProvideService(cfg, db, routeRegister, serviceWithFlagOn, featuresFlagOn, ac)
@@ -501,7 +516,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
folderPermissions, dashboardPermissions, ac, serviceWithFlagOff)
require.NoError(t, err)
alertStore, err := ngstore.ProvideDBStore(cfg, featuresFlagOff, db, serviceWithFlagOff, dashSrv)
alertStore, err := ngstore.ProvideDBStore(cfg, featuresFlagOff, db, serviceWithFlagOff, dashSrv, ac)
require.NoError(t, err)
elementService := libraryelements.ProvideService(cfg, db, routeRegister, serviceWithFlagOff, featuresFlagOff, ac)
@@ -595,7 +610,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
prefix: "flagon-noforce",
depth: 3,
forceDelete: false,
deletionErr: dashboards.ErrFolderContainsAlertRules,
deletionErr: folder.ErrFolderNotEmpty,
desc: "With nested folder feature flag on and no force deletion of rules",
},
{
@@ -615,7 +630,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
prefix: "flagoff-noforce",
depth: 1,
forceDelete: false,
deletionErr: dashboards.ErrFolderContainsAlertRules,
deletionErr: folder.ErrFolderNotEmpty,
desc: "With nested folder feature flag off and no force deletion of rules",
},
}
@@ -642,7 +657,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
dashSrv, err := service.ProvideDashboardServiceImpl(cfg, dashStore, folderStore, nil, tc.featuresFlag, folderPermissions, dashboardPermissions, ac, tc.service)
require.NoError(t, err)
alertStore, err := ngstore.ProvideDBStore(cfg, tc.featuresFlag, db, tc.service, dashSrv)
alertStore, err := ngstore.ProvideDBStore(cfg, tc.featuresFlag, db, tc.service, dashSrv, ac)
require.NoError(t, err)
ancestorUIDs := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, tc.depth, tc.prefix, createCmd)