Alerting: Fix rule storage to filter by group names using case-sensitive comparison (#88992)

* add test for the bug
* remove unused struct
* update db store to post process filters by group using go-lang's case-sensitive string comparison

--------

Co-authored-by: Alexander Weaver <weaver.alex.d@gmail.com>

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

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)

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 {

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)
+	})
+}