diff --git a/pkg/services/ngalert/accesscontrol.go b/pkg/services/ngalert/accesscontrol.go index 31fe1bdc894..753810c1099 100644 --- a/pkg/services/ngalert/accesscontrol.go +++ b/pkg/services/ngalert/accesscontrol.go @@ -179,6 +179,10 @@ var ( { Action: accesscontrol.ActionAlertingProvisioningWrite, // organization scope }, + { + Action: dashboards.ActionFoldersRead, + Scope: dashboards.ScopeFoldersAll, + }, }, }, Grants: []string{string(org.RoleAdmin)}, diff --git a/pkg/services/ngalert/api/api_provisioning.go b/pkg/services/ngalert/api/api_provisioning.go index 070e2c48318..91b6d51af0b 100644 --- a/pkg/services/ngalert/api/api_provisioning.go +++ b/pkg/services/ngalert/api/api_provisioning.go @@ -60,7 +60,7 @@ type MuteTimingService interface { type AlertRuleService interface { GetAlertRules(ctx context.Context, orgID int64) ([]*alerting_models.AlertRule, map[string]alerting_models.Provenance, error) GetAlertRule(ctx context.Context, orgID int64, ruleUID string) (alerting_models.AlertRule, alerting_models.Provenance, error) - CreateAlertRule(ctx context.Context, rule alerting_models.AlertRule, provenance alerting_models.Provenance, userID int64) (alerting_models.AlertRule, error) + CreateAlertRule(ctx context.Context, user identity.Requester, rule alerting_models.AlertRule, provenance alerting_models.Provenance) (alerting_models.AlertRule, error) UpdateAlertRule(ctx context.Context, rule alerting_models.AlertRule, provenance alerting_models.Provenance) (alerting_models.AlertRule, error) DeleteAlertRule(ctx context.Context, orgID int64, ruleUID string, provenance alerting_models.Provenance) error GetRuleGroup(ctx context.Context, orgID int64, folder, group string) (alerting_models.AlertRuleGroup, error) @@ -334,8 +334,7 @@ func (srv *ProvisioningSrv) RoutePostAlertRule(c *contextmodel.ReqContext, ar de return ErrResp(http.StatusBadRequest, err, "") } provenance := determineProvenance(c) - userID, _ := identity.UserIdentifier(c.SignedInUser.GetNamespacedID()) - createdAlertRule, err := srv.alertRules.CreateAlertRule(c.Req.Context(), upstreamModel, alerting_models.Provenance(provenance), userID) + createdAlertRule, err := srv.alertRules.CreateAlertRule(c.Req.Context(), c.SignedInUser, upstreamModel, alerting_models.Provenance(provenance)) if errors.Is(err, alerting_models.ErrAlertRuleFailedValidation) { return ErrResp(http.StatusBadRequest, err, "") } diff --git a/pkg/services/ngalert/api/api_provisioning_test.go b/pkg/services/ngalert/api/api_provisioning_test.go index c8d60ed330f..361d3a651a9 100644 --- a/pkg/services/ngalert/api/api_provisioning_test.go +++ b/pkg/services/ngalert/api/api_provisioning_test.go @@ -26,6 +26,8 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/folder" + "github.com/grafana/grafana/pkg/services/folder/foldertest" "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/notifier" @@ -271,6 +273,39 @@ func TestProvisioningApi(t *testing.T) { require.NotEmpty(t, response.Body()) require.Contains(t, string(response.Body()), "invalid alert rule") }) + + t.Run("POST returns 400 when folderUID not set", func(t *testing.T) { + sut := createProvisioningSrvSut(t) + rc := createTestRequestCtx() + rule := createTestAlertRule("rule", 1) + rule.FolderUID = "" + + response := sut.RoutePostAlertRule(&rc, rule) + + require.Equal(t, 400, response.Status()) + require.NotEmpty(t, response.Body()) + require.Contains(t, string(response.Body()), "invalid alert rule") + require.Contains(t, string(response.Body()), "folderUID must be set") + }) + + t.Run("POST returns 400 if folder does not exist", func(t *testing.T) { + testEnv := createTestEnv(t, testConfig) + // Create a fake folder service that will return an error when trying to get a folder. + folderService := foldertest.NewFakeService() + folderService.ExpectedError = dashboards.ErrFolderNotFound + testEnv.folderService = folderService + sut := createProvisioningSrvSutFromEnv(t, &testEnv) + + rc := createTestRequestCtx() + rule := createTestAlertRule("rule", 1) + + response := sut.RoutePostAlertRule(&rc, rule) + + require.Equal(t, 400, response.Status()) + require.NotEmpty(t, response.Body()) + require.Contains(t, string(response.Body()), "invalid alert rule") + require.Contains(t, string(response.Body()), "folder does not exist") + }) }) t.Run("exist in non-default orgs", func(t *testing.T) { @@ -1571,6 +1606,7 @@ type testEnvironment struct { secrets secrets.Service log log.Logger store store.DBstore + folderService folder.Service dashboardService dashboards.DashboardService configs provisioning.AMConfigStore xact provisioning.TransactionManager @@ -1602,12 +1638,18 @@ func createTestEnv(t *testing.T, testConfig string) testEnvironment { AlertmanagerConfiguration: string(raw), }) sqlStore := db.InitTestDB(t) + + // init folder service with default folder + folderService := foldertest.NewFakeService() + folderService.ExpectedFolder = &folder.Folder{} + store := store.DBstore{ Logger: log, SQLStore: sqlStore, Cfg: setting.UnifiedAlertingSettings{ BaseInterval: time.Second * 10, }, + FolderService: folderService, } quotas := &provisioning.MockQuotaChecker{} quotas.EXPECT().LimitOK() @@ -1637,6 +1679,7 @@ func createTestEnv(t *testing.T, testConfig string) testEnvironment { log: log, configs: configs, store: store, + folderService: folderService, dashboardService: dashboardService, xact: xact, prov: prov, @@ -1662,7 +1705,7 @@ func createProvisioningSrvSutFromEnv(t *testing.T, env *testEnvironment) Provisi contactPointService: provisioning.NewContactPointService(env.configs, env.secrets, env.prov, env.xact, receiverSvc, env.log, env.store), templates: provisioning.NewTemplateService(env.configs, env.prov, env.xact, env.log), muteTimings: provisioning.NewMuteTimingService(env.configs, env.prov, env.xact, env.log), - alertRules: provisioning.NewAlertRuleService(env.store, env.prov, env.dashboardService, env.quotas, env.xact, 60, 10, 100, env.log, &provisioning.NotificationSettingsValidatorProviderFake{}), + alertRules: provisioning.NewAlertRuleService(env.store, env.prov, env.folderService, env.dashboardService, env.quotas, env.xact, 60, 10, 100, env.log, &provisioning.NotificationSettingsValidatorProviderFake{}), } } diff --git a/pkg/services/ngalert/api/persist.go b/pkg/services/ngalert/api/persist.go index 59d4930d7d0..ebefb6e33ab 100644 --- a/pkg/services/ngalert/api/persist.go +++ b/pkg/services/ngalert/api/persist.go @@ -14,6 +14,7 @@ type RuleStore interface { // by returning map[string]struct{} instead of map[string]*folder.Folder GetUserVisibleNamespaces(context.Context, int64, identity.Requester) (map[string]*folder.Folder, error) GetNamespaceByUID(ctx context.Context, uid string, orgID int64, user identity.Requester) (*folder.Folder, error) + GetAlertRulesGroupByRuleUID(ctx context.Context, query *ngmodels.GetAlertRulesGroupByRuleUIDQuery) ([]*ngmodels.AlertRule, error) ListAlertRules(ctx context.Context, query *ngmodels.ListAlertRulesQuery) (ngmodels.RulesGroup, error) diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index d5ee1ff59a1..10994987927 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -329,7 +329,7 @@ func (ng *AlertNG) init() error { contactPointService := provisioning.NewContactPointService(ng.store, ng.SecretsService, ng.store, ng.store, receiverService, ng.Log, ng.store) templateService := provisioning.NewTemplateService(ng.store, ng.store, ng.store, ng.Log) muteTimingService := provisioning.NewMuteTimingService(ng.store, ng.store, ng.store, ng.Log) - alertRuleService := provisioning.NewAlertRuleService(ng.store, ng.store, ng.dashboardService, ng.QuotaService, ng.store, + alertRuleService := provisioning.NewAlertRuleService(ng.store, ng.store, ng.folderService, ng.dashboardService, ng.QuotaService, ng.store, int64(ng.Cfg.UnifiedAlerting.DefaultRuleEvaluationInterval.Seconds()), int64(ng.Cfg.UnifiedAlerting.BaseInterval.Seconds()), ng.Cfg.UnifiedAlerting.RulesPerRuleGroupLimit, ng.Log, notifier.NewNotificationSettingsValidationService(ng.store)) diff --git a/pkg/services/ngalert/provisioning/alert_rules.go b/pkg/services/ngalert/provisioning/alert_rules.go index b51e361c0d7..7ce5ac32a9f 100644 --- a/pkg/services/ngalert/provisioning/alert_rules.go +++ b/pkg/services/ngalert/provisioning/alert_rules.go @@ -7,7 +7,9 @@ import ( "time" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/notifier" "github.com/grafana/grafana/pkg/services/ngalert/store" @@ -25,6 +27,7 @@ type AlertRuleService struct { rulesPerRuleGroupLimit int64 ruleStore RuleStore provenanceStore ProvisioningStore + folderService folder.Service dashboardService dashboards.DashboardService quotas QuotaChecker xact TransactionManager @@ -34,6 +37,7 @@ type AlertRuleService struct { func NewAlertRuleService(ruleStore RuleStore, provenanceStore ProvisioningStore, + folderService folder.Service, dashboardService dashboards.DashboardService, quotas QuotaChecker, xact TransactionManager, @@ -49,6 +53,7 @@ func NewAlertRuleService(ruleStore RuleStore, rulesPerRuleGroupLimit: rulesPerRuleGroupLimit, ruleStore: ruleStore, provenanceStore: provenanceStore, + folderService: folderService, dashboardService: dashboardService, quotas: quotas, xact: xact, @@ -127,7 +132,7 @@ func (service *AlertRuleService) GetAlertRuleWithFolderTitle(ctx context.Context // CreateAlertRule creates a new alert rule. This function will ignore any // interval that is set in the rule struct and use the already existing group // interval or the default one. -func (service *AlertRuleService) CreateAlertRule(ctx context.Context, rule models.AlertRule, provenance models.Provenance, userID int64) (models.AlertRule, error) { +func (service *AlertRuleService) CreateAlertRule(ctx context.Context, user identity.Requester, rule models.AlertRule, provenance models.Provenance) (models.AlertRule, error) { if rule.UID == "" { rule.UID = util.GenerateShortUID() } else if err := util.ValidateUID(rule.UID); err != nil { @@ -145,6 +150,9 @@ func (service *AlertRuleService) CreateAlertRule(ctx context.Context, rule model if err != nil { return models.AlertRule{}, err } + if err = service.ensureRuleNamespace(ctx, user, rule); err != nil { + return models.AlertRule{}, err + } rule.Updated = time.Now() if len(rule.NotificationSettings) > 0 { validator, err := service.nsValidatorProvider.Validator(ctx, rule.OrgID) @@ -176,6 +184,11 @@ func (service *AlertRuleService) CreateAlertRule(ctx context.Context, rule model return errors.New("couldn't find newly created id") } + // default to 0 if there is no user + userID := int64(0) + if user != nil { + userID, _ = identity.UserIdentifier(user.GetNamespacedID()) + } if err = service.checkLimitsTransactionCtx(ctx, rule.OrgID, userID); err != nil { return err } @@ -639,3 +652,32 @@ func (service *AlertRuleService) checkGroupLimits(group models.AlertRuleGroup) e return nil } + +// ensureRuleNamespace ensures that the rule has a valid namespace UID. +// If the rule does not have a namespace UID or the namespace (folder) does not exist it will return an error. +func (service *AlertRuleService) ensureRuleNamespace(ctx context.Context, user identity.Requester, rule models.AlertRule) error { + if rule.NamespaceUID == "" { + return fmt.Errorf("%w: folderUID must be set", models.ErrAlertRuleFailedValidation) + } + + if user == nil { + // user is nil when this is called during file provisioning, + // which already creates the folder if it does not exist + return nil + } + + // ensure the namespace exists + _, err := service.folderService.Get(ctx, &folder.GetFolderQuery{ + OrgID: rule.OrgID, + UID: &rule.NamespaceUID, + SignedInUser: user, + }) + if err != nil { + if errors.Is(err, dashboards.ErrFolderNotFound) { + return fmt.Errorf("%w: folder does not exist", models.ErrAlertRuleFailedValidation) + } + return err + } + + return nil +} diff --git a/pkg/services/ngalert/provisioning/alert_rules_test.go b/pkg/services/ngalert/provisioning/alert_rules_test.go index be924adef91..cd063331d19 100644 --- a/pkg/services/ngalert/provisioning/alert_rules_test.go +++ b/pkg/services/ngalert/provisioning/alert_rules_test.go @@ -15,6 +15,8 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/folder" + "github.com/grafana/grafana/pkg/services/folder/foldertest" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/store" "github.com/grafana/grafana/pkg/setting" @@ -42,7 +44,7 @@ func TestAlertRuleService(t *testing.T) { t.Run("alert rule group should be updated correctly", func(t *testing.T) { rule := dummyRule("test#3", orgID) rule.RuleGroup = "a" - rule, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0) + rule, err := ruleService.CreateAlertRule(context.Background(), nil, rule, models.ProvenanceNone) require.NoError(t, err) require.Equal(t, int64(60), rule.IntervalSeconds) @@ -59,7 +61,7 @@ func TestAlertRuleService(t *testing.T) { var orgID int64 = 2 rule := dummyRule("test#1", orgID) rule.NamespaceUID = "123abc" - rule, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0) + rule, err := ruleService.CreateAlertRule(context.Background(), nil, rule, models.ProvenanceNone) require.NoError(t, err) rule.NamespaceUID = "abc123" @@ -68,6 +70,20 @@ func TestAlertRuleService(t *testing.T) { require.NoError(t, err) }) + t.Run("group update should propagate folderUID from group to rules", func(t *testing.T) { + ruleService := createAlertRuleService(t) + group := createDummyGroup("namespace-test", 1) + group.Rules[0].NamespaceUID = "" + + err := ruleService.ReplaceRuleGroup(context.Background(), 1, group, 0, models.ProvenanceAPI) + require.NoError(t, err) + + readGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "namespace-test") + require.NoError(t, err) + require.NotEmpty(t, readGroup.Rules) + require.Equal(t, "my-namespace", readGroup.Rules[0].NamespaceUID) + }) + t.Run("group creation should propagate group title correctly", func(t *testing.T) { group := createDummyGroup("group-test-3", orgID) group.Rules[0].RuleGroup = "something different" @@ -86,7 +102,7 @@ func TestAlertRuleService(t *testing.T) { t.Run("alert rule should get interval from existing rule group", func(t *testing.T) { rule := dummyRule("test#4", orgID) rule.RuleGroup = "b" - rule, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0) + rule, err := ruleService.CreateAlertRule(context.Background(), nil, rule, models.ProvenanceNone) require.NoError(t, err) var interval int64 = 120 @@ -95,7 +111,7 @@ func TestAlertRuleService(t *testing.T) { rule = dummyRule("test#4-1", orgID) rule.RuleGroup = "b" - rule, err = ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0) + rule, err = ruleService.CreateAlertRule(context.Background(), nil, rule, models.ProvenanceNone) require.NoError(t, err) require.Equal(t, interval, rule.IntervalSeconds) }) @@ -112,7 +128,7 @@ func TestAlertRuleService(t *testing.T) { rule.UID = ruleUID rule.RuleGroup = ruleGroup rule.NamespaceUID = namespaceUID - _, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0) + _, err := ruleService.CreateAlertRule(context.Background(), nil, rule, models.ProvenanceNone) require.NoError(t, err) rule, _, err = ruleService.GetAlertRule(context.Background(), orgID, ruleUID) @@ -420,7 +436,7 @@ func TestAlertRuleService(t *testing.T) { t.Run(test.name, func(t *testing.T) { var orgID int64 = 1 rule := dummyRule(t.Name(), orgID) - rule, err := ruleService.CreateAlertRule(context.Background(), rule, test.from, 0) + rule, err := ruleService.CreateAlertRule(context.Background(), nil, rule, test.from) require.NoError(t, err) _, err = ruleService.UpdateAlertRule(context.Background(), rule, test.to) @@ -501,7 +517,7 @@ func TestAlertRuleService(t *testing.T) { checker.EXPECT().LimitExceeded() ruleService.quotas = checker - _, err := ruleService.CreateAlertRule(context.Background(), dummyRule("test#1", orgID), models.ProvenanceNone, 0) + _, err := ruleService.CreateAlertRule(context.Background(), nil, dummyRule("test#1", orgID), models.ProvenanceNone) require.ErrorIs(t, err, models.ErrQuotaReached) }) @@ -524,13 +540,13 @@ func TestCreateAlertRule(t *testing.T) { var orgID int64 = 1 t.Run("should return the created id", func(t *testing.T) { - rule, err := ruleService.CreateAlertRule(context.Background(), dummyRule("test#1", orgID), models.ProvenanceNone, 0) + rule, err := ruleService.CreateAlertRule(context.Background(), nil, dummyRule("test#1", orgID), models.ProvenanceNone) require.NoError(t, err) require.NotEqual(t, 0, rule.ID, "expected to get the created id and not the zero value") }) t.Run("should set the right provenance", func(t *testing.T) { - rule, err := ruleService.CreateAlertRule(context.Background(), dummyRule("test#2", orgID), models.ProvenanceAPI, 0) + rule, err := ruleService.CreateAlertRule(context.Background(), nil, dummyRule("test#2", orgID), models.ProvenanceAPI) require.NoError(t, err) _, provenance, err := ruleService.GetAlertRule(context.Background(), orgID, rule.UID) @@ -542,14 +558,14 @@ func TestCreateAlertRule(t *testing.T) { t.Run("return error if it is not valid UID", func(t *testing.T) { rule := dummyRule("test#3", orgID) rule.UID = strings.Repeat("1", util.MaxUIDLength+1) - rule, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0) + rule, err := ruleService.CreateAlertRule(context.Background(), nil, rule, models.ProvenanceNone) require.ErrorIs(t, err, models.ErrAlertRuleFailedValidation) }) t.Run("should create a new rule with this UID", func(t *testing.T) { rule := dummyRule("test#3", orgID) uid := util.GenerateShortUID() rule.UID = uid - created, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0) + created, err := ruleService.CreateAlertRule(context.Background(), nil, rule, models.ProvenanceNone) require.NoError(t, err) require.Equal(t, uid, created.UID) _, _, err = ruleService.GetAlertRule(context.Background(), orgID, uid) @@ -561,13 +577,19 @@ func TestCreateAlertRule(t *testing.T) { func createAlertRuleService(t *testing.T) AlertRuleService { t.Helper() sqlStore := db.InitTestDB(t) + folderService := foldertest.NewFakeService() + folderService.ExpectedFolder = &folder.Folder{ + UID: "default-namespace", + } store := store.DBstore{ SQLStore: sqlStore, Cfg: setting.UnifiedAlertingSettings{ BaseInterval: time.Second * 10, }, - Logger: log.NewNopLogger(), + Logger: log.NewNopLogger(), + FolderService: folderService, } + // store := fakes.NewRuleStore(t) quotas := MockQuotaChecker{} quotas.EXPECT().LimitOK() return AlertRuleService{ @@ -578,6 +600,7 @@ func createAlertRuleService(t *testing.T) AlertRuleService { log: log.New("testing"), baseIntervalSeconds: 10, defaultIntervalSeconds: 60, + folderService: folderService, } } diff --git a/pkg/services/provisioning/alerting/rules_provisioner.go b/pkg/services/provisioning/alerting/rules_provisioner.go index 9d1ca4037e0..d4af1460ff9 100644 --- a/pkg/services/provisioning/alerting/rules_provisioner.go +++ b/pkg/services/provisioning/alerting/rules_provisioner.go @@ -85,9 +85,9 @@ func (prov *defaultAlertRuleProvisioner) provisionRule( return err } else if err != nil { prov.logger.Debug("creating rule", "uid", rule.UID, "org", rule.OrgID) - // 0 is passed as userID as then the quota logic will only check for - // the organization quota, as we don't have any user scope here. - _, err = prov.ruleService.CreateAlertRule(ctx, rule, alert_models.ProvenanceFile, 0) + // a nil user is passed in as then the quota logic will only check for + // the organization quota since we don't have any user scope here. + _, err = prov.ruleService.CreateAlertRule(ctx, nil, rule, alert_models.ProvenanceFile) } else { prov.logger.Debug("updating rule", "uid", rule.UID, "org", rule.OrgID) _, err = prov.ruleService.UpdateAlertRule(ctx, rule, alert_models.ProvenanceFile) diff --git a/pkg/services/provisioning/provisioning.go b/pkg/services/provisioning/provisioning.go index cfb02bbf9f5..59811ebe876 100644 --- a/pkg/services/provisioning/provisioning.go +++ b/pkg/services/provisioning/provisioning.go @@ -275,6 +275,7 @@ func (ps *ProvisioningServiceImpl) ProvisionAlerting(ctx context.Context) error ruleService := provisioning.NewAlertRuleService( st, st, + ps.folderService, ps.dashboardService, ps.quotaService, ps.SQLStore,