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

Alerting: Time interval Delete API to check for usages in alert rules (#90500)

Browse Source 
* Check if a time interval is used in alert rules before deleting it
* Add time interval to parameters of ListAlertRulesQuery and ListNotificationSettings of DbStore

== Refacorings == 
* refactor isMuteTimeInUse to accept a single route
* update getMuteTiming to not return err
* update delete to get the mute timing from config first
This commit is contained in:
Yuri Tseretyan
2024-07-17 10:53:54 -04:00
committed by GitHub
parent 79df1b1e10
commit 970cafa20f
15 changed files with 492 additions and 130 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/ngalert/store/alert_rule.go
+40 -16
View File
@@ -399,7 +399,14 @@ func (st DBstore) ListAlertRules(ctx context.Context, query *ngmodels.ListAlertR
}
if query.ReceiverName != "" {
q, err = st.filterByReceiverName(query.ReceiverName, q)
q, err = st.filterByContentInNotificationSettings(query.ReceiverName, q)
if err != nil {
return err
}
}
if query.TimeIntervalName != "" {
q, err = st.filterByContentInNotificationSettings(query.TimeIntervalName, q)
if err != nil {
return err
}
@@ -432,6 +439,13 @@ func (st DBstore) ListAlertRules(ctx context.Context, query *ngmodels.ListAlertR
continue
}
}
if query.TimeIntervalName != "" {
if !slices.ContainsFunc(rule.NotificationSettings, func(settings ngmodels.NotificationSettings) bool {
return slices.Contains(settings.MuteTimeIntervals, query.TimeIntervalName)
}) {
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 {
@@ -720,13 +734,24 @@ func (st DBstore) ListNotificationSettings(ctx context.Context, q ngmodels.ListN
var rules []ngmodels.AlertRule
err := st.SQLStore.WithDbSession(ctx, func(sess *db.Session) error {
query := sess.Table(ngmodels.AlertRule{}).Select("uid, notification_settings").Where("org_id = ?", q.OrgID)
hasFilter := false
if q.ReceiverName != "" {
var err error
query, err = st.filterByReceiverName(q.ReceiverName, query)
query, err = st.filterByContentInNotificationSettings(q.ReceiverName, query)
if err != nil {
return err
}
} else {
hasFilter = true
}
if q.TimeIntervalName != "" {
var err error
query, err = st.filterByContentInNotificationSettings(q.TimeIntervalName, query)
if err != nil {
return err
}
hasFilter = true
}
if !hasFilter {
query = query.And("notification_settings IS NOT NULL AND notification_settings <> 'null'")
}
return query.Find(&rules)
@@ -736,16 +761,15 @@ func (st DBstore) ListNotificationSettings(ctx context.Context, q ngmodels.ListN
}
result := make(map[ngmodels.AlertRuleKey][]ngmodels.NotificationSettings, len(rules))
for _, rule := range rules {
var ns []ngmodels.NotificationSettings
if q.ReceiverName != "" { // if filter by receiver name is specified, perform fine filtering on client to avoid false-positives
for _, setting := range rule.NotificationSettings {
if q.ReceiverName == setting.Receiver { // currently, there can be only one setting. If in future there are more, we will return all settings of a rule that has a setting with receiver
ns = rule.NotificationSettings
break
}
ns := make([]ngmodels.NotificationSettings, 0, len(rule.NotificationSettings))
for _, setting := range rule.NotificationSettings {
if q.ReceiverName != "" && q.ReceiverName != setting.Receiver { // currently, there can be only one setting. If in future there are more, we will return all settings of a rule that has a setting with receiver
continue
}
} else {
ns = rule.NotificationSettings
if q.TimeIntervalName != "" && !slices.Contains(setting.MuteTimeIntervals, q.TimeIntervalName) {
continue
}
ns = append(ns, setting)
}
if len(ns) > 0 {
key := ngmodels.AlertRuleKey{
@@ -758,14 +782,14 @@ func (st DBstore) ListNotificationSettings(ctx context.Context, q ngmodels.ListN
return result, nil
}
func (st DBstore) filterByReceiverName(receiver string, sess *xorm.Session) (*xorm.Session, error) {
if receiver == "" {
func (st DBstore) filterByContentInNotificationSettings(value string, sess *xorm.Session) (*xorm.Session, error) {
if value == "" {
return sess, nil
}
// marshall string according to JSON rules so we follow escaping rules.
b, err := json.Marshal(receiver)
b, err := json.Marshal(value)
if err != nil {
return nil, fmt.Errorf("failed to marshall receiver name query: %w", err)
return nil, fmt.Errorf("failed to marshall string for notification settings content filter: %w", err)
}
var search = string(b)
if st.SQLStore.GetDialect().DriverName() != migrator.SQLite {
pkg/services/ngalert/store/alert_rule_test.go
+131 -17
View File
@@ -12,6 +12,7 @@ import (
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/exp/maps"
"golang.org/x/exp/rand"
"github.com/grafana/grafana/pkg/bus"
@@ -776,6 +777,14 @@ func TestIntegrationAlertRulesNotificationSettings(t *testing.T) {
t.Skip("skipping integration test")
}
getKeyMap := func(r []*models.AlertRule) map[models.AlertRuleKey]struct{} {
result := make(map[models.AlertRuleKey]struct{}, len(r))
for _, rule := range r {
result[rule.GetKey()] = struct{}{}
}
return result
}
sqlStore := db.InitTestReplDB(t)
cfg := setting.NewCfg()
cfg.UnifiedAlerting.BaseInterval = 1 * time.Second
@@ -787,15 +796,17 @@ func TestIntegrationAlertRulesNotificationSettings(t *testing.T) {
}
receiverName := "receiver\"-" + uuid.NewString()
timeIntervalName := "time-" + util.GenerateShortUID()
gen := models.RuleGen
gen = gen.With(gen.WithOrgID(1), gen.WithIntervalMatching(store.Cfg.BaseInterval))
rules := gen.GenerateManyRef(3)
receiveRules := gen.With(gen.WithNotificationSettingsGen(models.NotificationSettingsGen(models.NSMuts.WithReceiver(receiverName)))).GenerateManyRef(3)
noise := gen.With(gen.WithNotificationSettingsGen(models.NotificationSettingsGen(models.NSMuts.WithMuteTimeIntervals(receiverName)))).GenerateManyRef(3)
timeIntervalRules := gen.With(gen.WithNotificationSettingsGen(models.NotificationSettingsGen(models.NSMuts.WithMuteTimeIntervals(timeIntervalName)))).GenerateManyRef(3)
noise := gen.With(gen.WithNotificationSettingsGen(models.NotificationSettingsGen(models.NSMuts.WithReceiver(timeIntervalName), models.NSMuts.WithMuteTimeIntervals(receiverName)))).GenerateManyRef(3)
deref := make([]models.AlertRule, 0, len(rules)+len(receiveRules)+len(noise))
for _, rule := range append(append(rules, receiveRules...), noise...) {
deref := make([]models.AlertRule, 0, len(rules)+len(receiveRules)+len(timeIntervalRules)+len(noise))
for _, rule := range append(append(append(rules, receiveRules...), noise...), timeIntervalRules...) {
r := *rule
r.ID = 0
deref = append(deref, r)
@@ -805,18 +816,60 @@ func TestIntegrationAlertRulesNotificationSettings(t *testing.T) {
require.NoError(t, err)
t.Run("should find rules by receiver name", func(t *testing.T) {
expectedUIDs := map[string]struct{}{}
for _, rule := range receiveRules {
expectedUIDs[rule.UID] = struct{}{}
}
expected := getKeyMap(receiveRules)
actual, err := store.ListAlertRules(context.Background(), &models.ListAlertRulesQuery{
OrgID: 1,
ReceiverName: receiverName,
})
require.NoError(t, err)
assert.Len(t, actual, len(expectedUIDs))
assert.Len(t, actual, len(expected))
for _, rule := range actual {
assert.Contains(t, expectedUIDs, rule.UID)
assert.Contains(t, expected, rule.GetKey())
}
})
t.Run("should find rules by time interval name", func(t *testing.T) {
expected := getKeyMap(timeIntervalRules)
actual, err := store.ListAlertRules(context.Background(), &models.ListAlertRulesQuery{
OrgID: 1,
TimeIntervalName: timeIntervalName,
})
require.NoError(t, err)
assert.Len(t, actual, len(expected))
for _, rule := range actual {
assert.Contains(t, expected, rule.GetKey())
}
})
t.Run("should find rules by receiver and time-interval name", func(t *testing.T) {
var receiver, intervalName string
var expected []models.AlertRuleKey
rand.Shuffle(len(deref), func(i, j int) {
deref[i], deref[j] = deref[j], deref[i]
})
for _, rule := range deref {
if len(rule.NotificationSettings) == 0 || rule.NotificationSettings[0].Receiver == "" || len(rule.NotificationSettings[0].MuteTimeIntervals) == 0 {
continue
}
if len(expected) > 0 {
if rule.NotificationSettings[0].Receiver == receiver && slices.Contains(rule.NotificationSettings[0].MuteTimeIntervals, intervalName) {
expected = append(expected, rule.GetKey())
}
} else {
receiver = rule.NotificationSettings[0].Receiver
intervalName = rule.NotificationSettings[0].MuteTimeIntervals[0]
expected = append(expected, rule.GetKey())
}
}
actual, err := store.ListAlertRules(context.Background(), &models.ListAlertRulesQuery{
OrgID: 1,
ReceiverName: receiver,
TimeIntervalName: intervalName,
})
require.NoError(t, err)
assert.Len(t, actual, len(expected))
for _, rule := range actual {
assert.Contains(t, expected, rule.GetKey())
}
})
@@ -864,13 +917,17 @@ func TestIntegrationListNotificationSettings(t *testing.T) {
Cfg: cfg.UnifiedAlerting,
}
receiverName := `receiver%"-👍'test`
searchName := `name-%"-👍'test`
gen := models.RuleGen
gen = gen.With(gen.WithOrgID(1), gen.WithIntervalMatching(store.Cfg.BaseInterval))
rulesWithNotifications := gen.With(
gen.WithNotificationSettingsGen(models.NotificationSettingsGen(models.NSMuts.WithReceiver(receiverName))),
rulesWithNotificationsAndReceiver := gen.With(
gen.WithNotificationSettingsGen(models.NotificationSettingsGen(models.NSMuts.WithReceiver(searchName))),
).GenerateMany(5)
rulesWithNotificationsAndTimeInterval := gen.With(
gen.WithNotificationSettingsGen(models.NotificationSettingsGen(models.NSMuts.WithMuteTimeIntervals(searchName))),
).GenerateMany(5)
rulesInOtherOrg := gen.With(
gen.WithOrgID(2),
gen.WithNotificationSettingsGen(models.NotificationSettingsGen()),
@@ -878,15 +935,18 @@ func TestIntegrationListNotificationSettings(t *testing.T) {
rulesWithNoNotifications := gen.With(gen.WithNoNotificationSettings()).GenerateMany(5)
deref := append(append(rulesWithNotifications, rulesWithNoNotifications...), rulesInOtherOrg...)
deref := append(append(rulesWithNotificationsAndReceiver, rulesWithNoNotifications...), rulesInOtherOrg...)
deref = append(deref, rulesWithNotificationsAndTimeInterval...)
orgRules := append(rulesWithNotificationsAndReceiver, rulesWithNotificationsAndTimeInterval...)
_, err := store.InsertAlertRules(context.Background(), deref)
require.NoError(t, err)
result, err := store.ListNotificationSettings(context.Background(), models.ListNotificationSettingsQuery{OrgID: 1})
require.NoError(t, err)
require.Len(t, result, len(rulesWithNotifications))
for _, rule := range rulesWithNotifications {
require.Len(t, result, len(orgRules))
for _, rule := range rulesWithNotificationsAndReceiver {
if !assert.Contains(t, result, rule.GetKey()) {
continue
}
@@ -895,13 +955,13 @@ func TestIntegrationListNotificationSettings(t *testing.T) {
t.Run("should list notification settings by receiver name", func(t *testing.T) {
expectedUIDs := map[models.AlertRuleKey]struct{}{}
for _, rule := range rulesWithNotifications {
for _, rule := range rulesWithNotificationsAndReceiver {
expectedUIDs[rule.GetKey()] = struct{}{}
}
actual, err := store.ListNotificationSettings(context.Background(), models.ListNotificationSettingsQuery{
OrgID: 1,
ReceiverName: receiverName,
ReceiverName: searchName,
})
require.NoError(t, err)
assert.Len(t, actual, len(expectedUIDs))
@@ -909,6 +969,60 @@ func TestIntegrationListNotificationSettings(t *testing.T) {
assert.Contains(t, expectedUIDs, ruleKey)
}
})
t.Run("should filter notification settings by time interval name", func(t *testing.T) {
expectedUIDs := map[models.AlertRuleKey]struct{}{}
for _, rule := range rulesWithNotificationsAndTimeInterval {
expectedUIDs[rule.GetKey()] = struct{}{}
}
actual, err := store.ListNotificationSettings(context.Background(), models.ListNotificationSettingsQuery{
OrgID: 1,
TimeIntervalName: searchName,
})
require.NoError(t, err)
assert.Len(t, actual, len(expectedUIDs))
for ruleKey := range actual {
assert.Contains(t, expectedUIDs, ruleKey)
}
})
t.Run("should return nothing if filter does not match", func(t *testing.T) {
result, err := store.ListNotificationSettings(context.Background(), models.ListNotificationSettingsQuery{
OrgID: 1,
ReceiverName: "not-found-receiver",
TimeIntervalName: "not-found-time-interval",
})
require.NoError(t, err)
require.Empty(t, result)
})
t.Run("should filter by time interval and receiver", func(t *testing.T) {
var receiver, timeInterval string
var expected []models.AlertRuleKey
rand.Shuffle(len(orgRules), func(i, j int) {
orgRules[i], orgRules[j] = orgRules[j], orgRules[i]
})
for _, rule := range orgRules {
if len(rule.NotificationSettings) == 0 || rule.NotificationSettings[0].Receiver == "" || len(rule.NotificationSettings[0].MuteTimeIntervals) == 0 {
continue
}
if len(expected) > 0 {
if rule.NotificationSettings[0].Receiver == receiver && slices.Contains(rule.NotificationSettings[0].MuteTimeIntervals, timeInterval) {
expected = append(expected, rule.GetKey())
}
} else {
receiver = rule.NotificationSettings[0].Receiver
timeInterval = rule.NotificationSettings[0].MuteTimeIntervals[0]
expected = append(expected, rule.GetKey())
}
}
actual, err := store.ListNotificationSettings(context.Background(), models.ListNotificationSettingsQuery{
OrgID: 1,
ReceiverName: receiver,
TimeIntervalName: timeInterval,
})
require.NoError(t, err)
require.EqualValuesf(t, expected, maps.Keys(actual), "got more rules than expected: %#v", actual)
})
}
func TestIntegrationGetNamespacesByRuleUID(t *testing.T) {