[v11.0.x] Alerting: Fix rule storage to filter by group names using case-sensitive comparison (#89063)
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> # Conflicts: # pkg/services/ngalert/store/alert_rule.go # pkg/services/ngalert/store/alert_rule_test.go
This commit is contained in:
@@ -630,18 +630,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
|
||||
|
||||
@@ -113,7 +113,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
|
||||
@@ -407,6 +423,11 @@ 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 query.RuleGroup != "" && query.RuleGroup != rule.RuleGroup {
|
||||
continue
|
||||
}
|
||||
alertRules = append(alertRules, rule)
|
||||
}
|
||||
|
||||
@@ -522,8 +543,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)
|
||||
@@ -544,6 +570,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)
|
||||
|
||||
@@ -392,6 +392,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},
|
||||
|
||||
@@ -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, 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)
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user