diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index 3408b7baa23..b505bf87ca6 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -692,18 +692,6 @@ type ListNamespaceAlertRulesQuery struct { NamespaceUID string } -// ListOrgRuleGroupsQuery is the query for listing unique rule groups -// for an organization -type ListOrgRuleGroupsQuery struct { - OrgID int64 - NamespaceUIDs []string - - // DashboardUID and PanelID are optional and allow filtering rules - // to return just those for a dashboard and panel. - DashboardUID string - PanelID int64 -} - type UpdateRule struct { Existing *AlertRule New AlertRule diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index 4bc1cdcf0cf..f7b81e178ea 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -114,7 +114,23 @@ func (st DBstore) GetAlertRulesGroupByRuleUID(ctx context.Context, query *ngmode if err != nil { return err } - result = rules + // MySQL by default compares strings without case-sensitivity, make sure we keep the case-sensitive comparison. + var groupKey ngmodels.AlertRuleGroupKey + // find the rule, which group we fetch + for _, rule := range rules { + if rule.UID == query.UID { + groupKey = rule.GetGroupKey() + break + } + } + result = make([]*ngmodels.AlertRule, 0, len(rules)) + // MySQL (and potentially other databases) can use case-insensitive comparison. + // This code makes sure we return groups that only exactly match the filter. + for _, rule := range rules { + if rule.GetGroupKey() == groupKey { + result = append(result, rule) + } + } return nil }) return result, err @@ -372,9 +388,14 @@ func (st DBstore) ListAlertRules(ctx context.Context, query *ngmodels.ListAlertR q = q.Where(fmt.Sprintf("uid IN (%s)", strings.Join(in, ",")), args...) } + var groupsMap map[string]struct{} if len(query.RuleGroups) > 0 { + groupsMap = make(map[string]struct{}) args, in := getINSubQueryArgs(query.RuleGroups) q = q.Where(fmt.Sprintf("rule_group IN (%s)", strings.Join(in, ",")), args...) + for _, group := range query.RuleGroups { + groupsMap[group] = struct{}{} + } } if query.ReceiverName != "" { @@ -411,6 +432,13 @@ func (st DBstore) ListAlertRules(ctx context.Context, query *ngmodels.ListAlertR continue } } + // MySQL (and potentially other databases) can use case-insensitive comparison. + // This code makes sure we return groups that only exactly match the filter. + if groupsMap != nil { + if _, ok := groupsMap[rule.RuleGroup]; !ok { + continue + } + } alertRules = append(alertRules, rule) } @@ -526,8 +554,13 @@ func (st DBstore) GetAlertRulesForScheduling(ctx context.Context, query *ngmodel alertRulesSql.NotIn("org_id", disabledOrgs) } + var groupsMap map[string]struct{} if len(query.RuleGroups) > 0 { alertRulesSql.In("rule_group", query.RuleGroups) + groupsMap = make(map[string]struct{}, len(query.RuleGroups)) + for _, group := range query.RuleGroups { + groupsMap[group] = struct{}{} + } } rule := new(ngmodels.AlertRule) @@ -548,6 +581,13 @@ func (st DBstore) GetAlertRulesForScheduling(ctx context.Context, query *ngmodel st.Logger.Error("Invalid rule found in DB store, ignoring it", "func", "GetAlertRulesForScheduling", "error", err) continue } + // MySQL (and potentially other databases) uses case-insensitive comparison. + // This code makes sure we return groups that only exactly match the filter + if groupsMap != nil { + if _, ok := groupsMap[rule.RuleGroup]; !ok { // compare groups using case-sensitive logic. + continue + } + } if st.FeatureToggles.IsEnabled(ctx, featuremgmt.FlagAlertingQueryOptimization) { if optimizations, err := OptimizeAlertQueries(rule.Data); err != nil { st.Logger.Error("Could not migrate rule from range to instant query", "rule", rule.UID, "err", err) diff --git a/pkg/services/ngalert/store/alert_rule_test.go b/pkg/services/ngalert/store/alert_rule_test.go index c8752e01627..ec701b82dd8 100644 --- a/pkg/services/ngalert/store/alert_rule_test.go +++ b/pkg/services/ngalert/store/alert_rule_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "slices" "strings" "testing" "time" @@ -422,6 +423,11 @@ func TestIntegration_GetAlertRulesForScheduling(t *testing.T) { ruleGroups: []string{rule1.RuleGroup}, rules: []string{rule1.Title}, }, + { + name: "with a rule group filter, should be case sensitive", + ruleGroups: []string{strings.ToUpper(rule1.RuleGroup)}, + rules: []string{}, + }, { name: "with a filter on orgs, it returns rules that do not belong to that org", rules: []string{rule1.Title}, @@ -958,6 +964,114 @@ func TestIntegrationGetNamespacesByRuleUID(t *testing.T) { } } +func TestIntegrationRuleGroupsCaseSensitive(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + sqlStore := db.InitTestDB(t) + cfg := setting.NewCfg() + cfg.UnifiedAlerting.BaseInterval = 1 * time.Second + store := &DBstore{ + SQLStore: sqlStore, + FolderService: setupFolderService(t, sqlStore, cfg, featuremgmt.WithFeatures()), + Logger: log.New("test-dbstore"), + Cfg: cfg.UnifiedAlerting, + FeatureToggles: featuremgmt.WithFeatures(), + } + + gen := models.RuleGen.With(models.RuleMuts.WithOrgID(1)) + misc := gen.GenerateMany(5, 10) + groupKey1 := models.GenerateGroupKey(1) + groupKey1.RuleGroup = strings.ToLower(groupKey1.RuleGroup) + groupKey2 := groupKey1 + groupKey2.RuleGroup = strings.ToUpper(groupKey2.RuleGroup) + groupKey3 := groupKey1 + groupKey3.OrgID = 2 + + group1 := gen.With(gen.WithGroupKey(groupKey1)).GenerateMany(3) + group2 := gen.With(gen.WithGroupKey(groupKey2)).GenerateMany(1, 3) + group3 := gen.With(gen.WithGroupKey(groupKey3)).GenerateMany(1, 3) + + _, err := store.InsertAlertRules(context.Background(), append(append(append(misc, group1...), group2...), group3...)) + require.NoError(t, err) + + t.Run("GetAlertRulesGroupByRuleUID", func(t *testing.T) { + t.Run("should return rules that belong to only that group", func(t *testing.T) { + result, err := store.GetAlertRulesGroupByRuleUID(context.Background(), &models.GetAlertRulesGroupByRuleUIDQuery{ + UID: group1[rand.Intn(len(group1))].UID, + OrgID: groupKey1.OrgID, + }) + require.NoError(t, err) + assert.Len(t, result, len(group1)) + for _, rule := range result { + assert.Equal(t, groupKey1, rule.GetGroupKey()) + assert.Truef(t, slices.ContainsFunc(group1, func(r models.AlertRule) bool { + return r.UID == rule.UID + }), "rule with group key [%v] should not be in group [%v]", rule.GetGroupKey(), group1) + } + if t.Failed() { + deref := make([]models.AlertRule, 0, len(result)) + for _, rule := range result { + deref = append(deref, *rule) + } + t.Logf("expected rules in group %v: %v\ngot:%v", groupKey1, group1, deref) + } + }) + }) + + t.Run("ListAlertRules", func(t *testing.T) { + t.Run("should find only group with exact case", func(t *testing.T) { + result, err := store.ListAlertRules(context.Background(), &models.ListAlertRulesQuery{ + OrgID: 1, + RuleGroups: []string{groupKey1.RuleGroup}, + }) + require.NoError(t, err) + assert.Len(t, result, len(group1)) + for _, rule := range result { + assert.Equal(t, groupKey1, rule.GetGroupKey()) + assert.Truef(t, slices.ContainsFunc(group1, func(r models.AlertRule) bool { + return r.UID == rule.UID + }), "rule with group key [%v] should not be in group [%v]", rule.GetGroupKey(), group1) + } + if t.Failed() { + deref := make([]models.AlertRule, 0, len(result)) + for _, rule := range result { + deref = append(deref, *rule) + } + t.Logf("expected rules in group %v: %v\ngot:%v", groupKey1, group1, deref) + } + }) + }) + + t.Run("GetAlertRulesForScheduling", func(t *testing.T) { + t.Run("should find only group with exact case", func(t *testing.T) { + q := &models.GetAlertRulesForSchedulingQuery{ + PopulateFolders: false, + RuleGroups: []string{groupKey1.RuleGroup}, + } + err := store.GetAlertRulesForScheduling(context.Background(), q) + require.NoError(t, err) + result := q.ResultRules + expected := append(group1, group3...) + assert.Len(t, result, len(expected)) // query fetches all orgs + for _, rule := range result { + assert.Equal(t, groupKey1.RuleGroup, rule.RuleGroup) + assert.Truef(t, slices.ContainsFunc(expected, func(r models.AlertRule) bool { + return r.UID == rule.UID + }), "rule with group key [%v] should not be in group [%v]", rule.GetGroupKey(), group1) + } + if t.Failed() { + deref := make([]models.AlertRule, 0, len(result)) + for _, rule := range result { + deref = append(deref, *rule) + } + t.Logf("expected rules in group %v: %v\ngot:%v", groupKey1.RuleGroup, expected, deref) + } + }) + }) +} + // createAlertRule creates an alert rule in the database and returns it. // If a generator is not specified, uniqueness of primary key is not guaranteed. func createRule(t *testing.T, store *DBstore, generator *models.AlertRuleGenerator) *models.AlertRule { diff --git a/pkg/tests/api/alerting/api_ruler_test.go b/pkg/tests/api/alerting/api_ruler_test.go index 61de2ac61a9..01ec065002b 100644 --- a/pkg/tests/api/alerting/api_ruler_test.go +++ b/pkg/tests/api/alerting/api_ruler_test.go @@ -2100,3 +2100,59 @@ func TestIntegrationRuleNotificationSettings(t *testing.T) { } }) } + +func TestIntegrationRuleUpdateAllDatabases(t *testing.T) { + // Setup Grafana and its Database + dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{ + DisableLegacyAlerting: true, + EnableUnifiedAlerting: true, + DisableAnonymous: true, + AppModeProduction: true, + }) + grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, path) + + // Create a user to make authenticated requests + createUser(t, env.SQLStore, env.Cfg, user.CreateUserCommand{ + DefaultOrgRole: string(org.RoleAdmin), + Password: "admin", + Login: "admin", + }) + + client := newAlertingApiClient(grafanaListedAddr, "admin", "admin") + + folderUID := util.GenerateShortUID() + client.CreateFolder(t, folderUID, "folder1") + + t.Run("group renamed followed by delete for case-only changes should not delete both groups", func(t *testing.T) { // Regression test. + group := generateAlertRuleGroup(3, alertRuleGen()) + groupName := group.Name + + _, status, body := client.PostRulesGroupWithStatus(t, folderUID, &group) + require.Equalf(t, http.StatusAccepted, status, "failed to post rule group. Response: %s", body) + getGroup := client.GetRulesGroup(t, folderUID, group.Name) + require.Lenf(t, getGroup.Rules, 3, "expected 3 rules in group") + require.Equal(t, groupName, getGroup.Rules[0].GrafanaManagedAlert.RuleGroup) + + group = convertGettableRuleGroupToPostable(getGroup.GettableRuleGroupConfig) + newGroup := strings.ToUpper(group.Name) + group.Name = newGroup + _, status, body = client.PostRulesGroupWithStatus(t, folderUID, &group) + require.Equalf(t, http.StatusAccepted, status, "failed to post rule group. Response: %s", body) + + getGroup = client.GetRulesGroup(t, folderUID, group.Name) + require.Lenf(t, getGroup.Rules, 3, "expected 3 rules in group") + require.Equal(t, newGroup, getGroup.Rules[0].GrafanaManagedAlert.RuleGroup) + + status, body = client.DeleteRulesGroup(t, folderUID, groupName) + require.Equalf(t, http.StatusAccepted, status, "failed to post noop rule group. Response: %s", body) + + // Old group is gone. + getGroup = client.GetRulesGroup(t, folderUID, groupName) + require.Lenf(t, getGroup.Rules, 0, "expected no rules") + + // New group still exists. + getGroup = client.GetRulesGroup(t, folderUID, newGroup) + require.Lenf(t, getGroup.Rules, 3, "expected 3 rules in group") + require.Equal(t, newGroup, getGroup.Rules[0].GrafanaManagedAlert.RuleGroup) + }) +}