Alerting: Update Time Interval service to support renaming of resources (#91856)

* add RenameTimeIntervalInNotificationSettings to storage
* update dependencies when the time interval is renamed
---------

Co-authored-by: William Wernert <william.wernert@grafana.com>
This commit is contained in:
Yuri Tseretyan
2024-08-16 13:55:03 -04:00
committed by GitHub
parent 3c5d799c8c
commit 135f6571a9
15 changed files with 672 additions and 80 deletions

View File

@@ -10,6 +10,10 @@ const (
ProvenanceFile Provenance = "file"
)
var (
KnownProvenances = []Provenance{ProvenanceNone, ProvenanceAPI, ProvenanceFile}
)
// Provisionable represents a resource that can be created through a provisioning mechanism, such as Terraform or config file.
type Provisionable interface {
ResourceType() string

View File

@@ -25,6 +25,7 @@ import (
type AlertRuleNotificationSettingsStore interface {
RenameReceiverInNotificationSettings(ctx context.Context, orgID int64, oldReceiver, newReceiver string) (int, error)
RenameTimeIntervalInNotificationSettings(ctx context.Context, orgID int64, oldTimeInterval, newTimeInterval string, validateProvenance func(models.Provenance) bool, dryRun bool) ([]models.AlertRuleKey, []models.AlertRuleKey, error)
ListNotificationSettings(ctx context.Context, q models.ListNotificationSettingsQuery) (map[models.AlertRuleKey][]models.NotificationSettings, error)
}

View File

@@ -13,10 +13,14 @@ var ErrNotFound = fmt.Errorf("object not found")
var (
ErrVersionConflict = errutil.Conflict("alerting.notifications.conflict")
ErrTimeIntervalNotFound = errutil.NotFound("alerting.notifications.time-intervals.notFound")
ErrTimeIntervalExists = errutil.BadRequest("alerting.notifications.time-intervals.nameExists", errutil.WithPublicMessage("Time interval with this name already exists. Use a different name or update existing one."))
ErrTimeIntervalInvalid = errutil.BadRequest("alerting.notifications.time-intervals.invalidFormat").MustTemplate("Invalid format of the submitted time interval", errutil.WithPublic("Time interval is in invalid format. Correct the payload and try again."))
ErrTimeIntervalInUse = errutil.Conflict("alerting.notifications.time-intervals.used").MustTemplate("Time interval is used")
ErrTimeIntervalNotFound = errutil.NotFound("alerting.notifications.time-intervals.notFound")
ErrTimeIntervalExists = errutil.BadRequest("alerting.notifications.time-intervals.nameExists", errutil.WithPublicMessage("Time interval with this name already exists. Use a different name or update existing one."))
ErrTimeIntervalInvalid = errutil.BadRequest("alerting.notifications.time-intervals.invalidFormat").MustTemplate("Invalid format of the submitted time interval", errutil.WithPublic("Time interval is in invalid format. Correct the payload and try again."))
ErrTimeIntervalInUse = errutil.Conflict("alerting.notifications.time-intervals.used").MustTemplate("Time interval is used")
ErrTimeIntervalDependentResourcesProvenance = errutil.Conflict("alerting.notifications.time-intervals.usedProvisioned").MustTemplate(
"Time interval cannot be renamed because it is used by provisioned {{ if .Public.UsedByRules }}alert rules{{ end }}{{ if .Public.UsedByRoutes }}{{ if .Public.UsedByRules }} and {{ end }}notification policies{{ end }}",
errutil.WithPublic(`Time interval cannot be renamed because it is used by provisioned {{ if .Public.UsedByRules }}alert rules{{ end }}{{ if .Public.UsedByRoutes }}{{ if .Public.UsedByRules }} and {{ end }}notification policies{{ end }}. You must update those resources first using the original provision method.`),
)
ErrTemplateNotFound = errutil.NotFound("alerting.notifications.templates.notFound")
ErrTemplateInvalid = errutil.BadRequest("alerting.notifications.templates.invalidFormat").MustTemplate("Invalid format of the submitted template", errutil.WithPublic("Template is in invalid format. Correct the payload and try again."))
@@ -67,3 +71,21 @@ func MakeErrTemplateInvalid(err error) error {
return ErrTemplateInvalid.Build(data)
}
func MakeErrTimeIntervalDependentResourcesProvenance(usedByRoutes bool, rules []models.AlertRuleKey) error {
uids := make([]string, 0, len(rules))
for _, key := range rules {
uids = append(uids, key.UID)
}
data := make(map[string]any, 2)
if len(uids) > 0 {
data["UsedByRules"] = uids
}
if usedByRoutes {
data["UsedByRoutes"] = true
}
return ErrTimeIntervalDependentResourcesProvenance.Build(errutil.TemplateData{
Public: data,
})
}

View File

@@ -0,0 +1,53 @@
package provisioning
import (
"errors"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/apimachinery/errutil"
"github.com/grafana/grafana/pkg/services/ngalert/models"
)
func TestMakeErrTimeIntervalDependentResourcesProvenance(t *testing.T) {
tests := []struct {
name string
usedByRoutes bool
rules []models.AlertRuleKey
expectedPrivateMessage string
expectedPublicMessage string
}{
{
name: "both specified",
usedByRoutes: true,
rules: []models.AlertRuleKey{models.GenerateRuleKey(1)},
expectedPrivateMessage: "[alerting.notifications.time-intervals.usedProvisioned] Time interval cannot be renamed because it is used by provisioned alert rules and notification policies",
expectedPublicMessage: "Time interval cannot be renamed because it is used by provisioned alert rules and notification policies. You must update those resources first using the original provision method.",
},
{
name: "rules specified",
usedByRoutes: false,
rules: []models.AlertRuleKey{models.GenerateRuleKey(1)},
expectedPrivateMessage: "[alerting.notifications.time-intervals.usedProvisioned] Time interval cannot be renamed because it is used by provisioned alert rules",
expectedPublicMessage: "Time interval cannot be renamed because it is used by provisioned alert rules. You must update those resources first using the original provision method.",
},
{
name: "routes specified",
usedByRoutes: true,
rules: nil,
expectedPrivateMessage: "[alerting.notifications.time-intervals.usedProvisioned] Time interval cannot be renamed because it is used by provisioned notification policies",
expectedPublicMessage: "Time interval cannot be renamed because it is used by provisioned notification policies. You must update those resources first using the original provision method.",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := MakeErrTimeIntervalDependentResourcesProvenance(tt.usedByRoutes, tt.rules)
assert.Equal(t, tt.expectedPrivateMessage, err.Error())
var e errutil.Error
require.True(t, errors.As(err, &e))
assert.Equal(t, tt.expectedPublicMessage, e.PublicMessage)
})
}
}

View File

@@ -3,7 +3,6 @@ package provisioning
import (
"context"
"encoding/binary"
"errors"
"fmt"
"hash/fnv"
"slices"
@@ -184,16 +183,25 @@ func (svc *MuteTimingService) UpdateMuteTiming(ctx context.Context, mt definitio
return definitions.MuteTimeInterval{}, err
}
// if the name of the time interval changed
if old.Name != mt.Name {
// TODO implement support for renaming.
return definitions.MuteTimeInterval{}, MakeErrTimeIntervalInvalid(errors.New("name change is not allowed"))
}
updateTimeInterval(revision, mt.MuteTimeInterval)
// TODO add diff and noop detection
err = svc.xact.InTransaction(ctx, func(ctx context.Context) error {
// if the name of the time interval changed
if old.Name != mt.Name {
deleteTimeInterval(revision, old)
revision.Config.AlertmanagerConfig.TimeIntervals = append(revision.Config.AlertmanagerConfig.TimeIntervals, config.TimeInterval(mt.MuteTimeInterval))
err = svc.renameTimeIntervalInDependentResources(ctx, orgID, revision.Config.AlertmanagerConfig.Route, old.Name, mt.Name, models.Provenance(mt.Provenance))
if err != nil {
return err
}
err = svc.provenanceStore.DeleteProvenance(ctx, &definitions.MuteTimeInterval{MuteTimeInterval: old}, orgID)
if err != nil {
return err
}
} else {
updateTimeInterval(revision, mt.MuteTimeInterval)
}
if err := svc.configStore.Save(ctx, revision, orgID); err != nil {
return err
}
@@ -394,3 +402,46 @@ func (svc *MuteTimingService) checkOptimisticConcurrency(current config.MuteTime
}
return nil
}
func (svc *MuteTimingService) renameTimeIntervalInDependentResources(ctx context.Context, orgID int64, route *definitions.Route, oldName, newName string, timeIntervalProvenance models.Provenance) error {
validate := validation.ValidateProvenanceOfDependentResources(timeIntervalProvenance)
// if there are no references to the old time interval, exit
updatedRoutes := replaceMuteTiming(route, oldName, newName)
canUpdate := true
if updatedRoutes > 0 {
routeProvenance, err := svc.provenanceStore.GetProvenance(ctx, route, orgID)
if err != nil {
return err
}
canUpdate = validate(routeProvenance)
}
dryRun := !canUpdate
affected, invalidProvenance, err := svc.ruleNotificationsStore.RenameTimeIntervalInNotificationSettings(ctx, orgID, oldName, newName, validate, dryRun)
if err != nil {
return err
}
if !canUpdate || len(invalidProvenance) > 0 {
return MakeErrTimeIntervalDependentResourcesProvenance(updatedRoutes > 0, invalidProvenance)
}
if len(affected) > 0 || updatedRoutes > 0 {
svc.log.FromContext(ctx).Info("Updated rules and routes that use renamed time interval", "oldName", oldName, "newName", newName, "rules", len(affected), "routes", updatedRoutes)
}
return nil
}
func replaceMuteTiming(route *definitions.Route, oldName, newName string) int {
if route == nil {
return 0
}
updated := 0
for idx := range route.MuteTimeIntervals {
if route.MuteTimeIntervals[idx] == oldName {
route.MuteTimeIntervals[idx] = newName
updated++
}
}
for _, route := range route.Routes {
updated += replaceMuteTiming(route, oldName, newName)
}
return updated
}

View File

@@ -455,6 +455,13 @@ func TestUpdateMuteTimings(t *testing.T) {
Name: "Test2",
},
},
Route: &definitions.Route{
Routes: []*definitions.Route{
{
MuteTimeIntervals: []string{original.Name},
},
},
},
},
Receivers: nil,
},
@@ -517,47 +524,6 @@ func TestUpdateMuteTimings(t *testing.T) {
require.ErrorIs(t, err, expectedErr)
})
t.Run("rejects if mute timing is renamed", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()
store.GetFn = func(ctx context.Context, orgID int64) (*legacy_storage.ConfigRevision, error) {
return &legacy_storage.ConfigRevision{Config: initialConfig()}, nil
}
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(expectedProvenance, nil)
interval := expected
interval.Name = "another-time-interval"
timing := definitions.MuteTimeInterval{
UID: expectedUID,
MuteTimeInterval: interval,
Version: originalVersion,
Provenance: definitions.Provenance(expectedProvenance),
}
_, err := sut.UpdateMuteTiming(context.Background(), timing, orgID)
require.ErrorIs(t, err, ErrTimeIntervalInvalid)
})
t.Run("rejects mute timings if provenance is not right", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()
store.GetFn = func(ctx context.Context, orgID int64) (*legacy_storage.ConfigRevision, error) {
return &legacy_storage.ConfigRevision{Config: initialConfig()}, nil
}
expectedErr := errors.New("test")
sut.validator = func(from, to models.Provenance) error {
return expectedErr
}
timing := definitions.MuteTimeInterval{
MuteTimeInterval: expected,
Provenance: definitions.Provenance(models.ProvenanceFile),
}
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(expectedProvenance, nil)
_, err := sut.UpdateMuteTiming(context.Background(), timing, orgID)
require.ErrorIs(t, err, expectedErr)
})
t.Run("returns ErrVersionConflict if storage version does not match", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()
store.GetFn = func(ctx context.Context, orgID int64) (*legacy_storage.ConfigRevision, error) {
@@ -728,6 +694,157 @@ func TestUpdateMuteTimings(t *testing.T) {
prov.AssertCalled(t, "SetProvenance", mock.Anything, &timing, orgID, expectedProvenance)
})
t.Run("renames interval and all its dependencies", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()
store.GetFn = func(ctx context.Context, orgID int64) (*legacy_storage.ConfigRevision, error) {
return &legacy_storage.ConfigRevision{Config: initialConfig()}, nil
}
store.SaveFn = func(ctx context.Context, revision *legacy_storage.ConfigRevision) error {
assertInTransaction(t, ctx)
return nil
}
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(expectedProvenance, nil)
prov.EXPECT().DeleteProvenance(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, provisionable models.Provisionable, i int64) error {
assertInTransaction(t, ctx)
return nil
})
prov.EXPECT().SetProvenance(mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn(
func(ctx context.Context, _ models.Provisionable, _ int64, _ models.Provenance) error {
assertInTransaction(t, ctx)
return nil
})
ruleStore := &fakeAlertRuleNotificationStore{
RenameTimeIntervalInNotificationSettingsFn: func(ctx context.Context, orgID int64, old, new string, validate func(models.Provenance) bool, dryRun bool) ([]models.AlertRuleKey, []models.AlertRuleKey, error) {
assertInTransaction(t, ctx)
return nil, nil, nil
},
}
sut.ruleNotificationsStore = ruleStore
interval := expected
interval.Name = "another-time-interval"
timing := definitions.MuteTimeInterval{
UID: expectedUID,
MuteTimeInterval: interval,
Version: originalVersion,
Provenance: definitions.Provenance(expectedProvenance),
}
result, err := sut.UpdateMuteTiming(context.Background(), timing, orgID)
require.NoError(t, err)
require.EqualValues(t, interval, result.MuteTimeInterval)
require.EqualValues(t, expectedProvenance, result.Provenance)
require.EqualValues(t, calculateMuteTimeIntervalFingerprint(interval), result.Version)
require.Equal(t, legacy_storage.NameToUid(result.Name), result.UID)
require.Len(t, ruleStore.Calls, 1)
assert.Equal(t, "RenameTimeIntervalInNotificationSettings", ruleStore.Calls[0].Method)
assert.Equal(t, orgID, ruleStore.Calls[0].Args[1])
assert.Equal(t, original.Name, ruleStore.Calls[0].Args[2])
assert.Equal(t, interval.Name, ruleStore.Calls[0].Args[3])
assert.NotNil(t, ruleStore.Calls[0].Args[4])
assert.False(t, ruleStore.Calls[0].Args[5].(bool))
prov.AssertCalled(t, "SetProvenance", mock.Anything, mock.MatchedBy(func(m *definitions.MuteTimeInterval) bool {
return m.Name == interval.Name
}), orgID, expectedProvenance)
prov.AssertCalled(t, "DeleteProvenance", mock.Anything, mock.MatchedBy(func(m *definitions.MuteTimeInterval) bool {
return m.Name == original.Name
}), orgID)
prov.AssertExpectations(t)
require.Len(t, store.Calls, 2)
require.Equal(t, "Get", store.Calls[0].Method)
assert.Equal(t, orgID, store.Calls[0].Args[1])
require.Equal(t, "Save", store.Calls[1].Method)
assert.Equal(t, orgID, store.Calls[1].Args[2])
revision := store.Calls[1].Args[1].(*legacy_storage.ConfigRevision)
assert.EqualValues(t, append(initialConfig().AlertmanagerConfig.TimeIntervals, config.TimeInterval(interval)), revision.Config.AlertmanagerConfig.TimeIntervals)
assert.Falsef(t, isMuteTimeInUseInRoutes(expected.Name, revision.Config.AlertmanagerConfig.Route), "There are still references to the old time interval")
assert.Truef(t, isMuteTimeInUseInRoutes(interval.Name, revision.Config.AlertmanagerConfig.Route), "There are no references to the new time interval")
})
t.Run("returns ErrTimeIntervalDependentResourcesProvenance if route has different provenance status", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()
store.GetFn = func(ctx context.Context, orgID int64) (*legacy_storage.ConfigRevision, error) {
return &legacy_storage.ConfigRevision{Config: initialConfig()}, nil
}
prov.EXPECT().GetProvenance(mock.Anything, mock.MatchedBy(func(*definitions.Route) bool { return true }), mock.Anything).Return(models.ProvenanceFile, nil)
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(expectedProvenance, nil)
ruleStore := &fakeAlertRuleNotificationStore{
RenameTimeIntervalInNotificationSettingsFn: func(ctx context.Context, orgID int64, old, new string, validate func(models.Provenance) bool, dryRun bool) ([]models.AlertRuleKey, []models.AlertRuleKey, error) {
assertInTransaction(t, ctx)
return nil, nil, nil
},
}
sut.ruleNotificationsStore = ruleStore
interval := expected
interval.Name = "another-time-interval"
timing := definitions.MuteTimeInterval{
UID: expectedUID,
MuteTimeInterval: interval,
Version: originalVersion,
Provenance: definitions.Provenance(expectedProvenance),
}
_, err := sut.UpdateMuteTiming(context.Background(), timing, orgID)
require.ErrorIs(t, err, ErrTimeIntervalDependentResourcesProvenance)
require.Len(t, ruleStore.Calls, 1)
assert.Equal(t, "RenameTimeIntervalInNotificationSettings", ruleStore.Calls[0].Method)
assert.Equal(t, orgID, ruleStore.Calls[0].Args[1])
assert.Equal(t, original.Name, ruleStore.Calls[0].Args[2])
assert.Equal(t, interval.Name, ruleStore.Calls[0].Args[3])
assert.NotNil(t, ruleStore.Calls[0].Args[4])
assert.True(t, ruleStore.Calls[0].Args[5].(bool)) // still check if there are rules that have incompatible provenance
})
t.Run("returns ErrTimeIntervalDependentResourcesProvenance if rules have different provenance status", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()
store.GetFn = func(ctx context.Context, orgID int64) (*legacy_storage.ConfigRevision, error) {
return &legacy_storage.ConfigRevision{Config: initialConfig()}, nil
}
prov.EXPECT().GetProvenance(mock.Anything, mock.MatchedBy(func(*definitions.Route) bool { return true }), mock.Anything).Return(models.ProvenanceNone, nil)
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(expectedProvenance, nil)
ruleStore := &fakeAlertRuleNotificationStore{
RenameTimeIntervalInNotificationSettingsFn: func(ctx context.Context, orgID int64, old, new string, validate func(models.Provenance) bool, dryRun bool) ([]models.AlertRuleKey, []models.AlertRuleKey, error) {
assertInTransaction(t, ctx)
return nil, []models.AlertRuleKey{models.GenerateRuleKey(orgID)}, nil
},
}
sut.ruleNotificationsStore = ruleStore
interval := expected
interval.Name = "another-time-interval"
timing := definitions.MuteTimeInterval{
UID: expectedUID,
MuteTimeInterval: interval,
Version: originalVersion,
Provenance: definitions.Provenance(expectedProvenance),
}
_, err := sut.UpdateMuteTiming(context.Background(), timing, orgID)
require.ErrorIs(t, err, ErrTimeIntervalDependentResourcesProvenance)
require.Len(t, ruleStore.Calls, 1)
assert.Equal(t, "RenameTimeIntervalInNotificationSettings", ruleStore.Calls[0].Method)
assert.Equal(t, orgID, ruleStore.Calls[0].Args[1])
assert.Equal(t, original.Name, ruleStore.Calls[0].Args[2])
assert.Equal(t, interval.Name, ruleStore.Calls[0].Args[3])
assert.NotNil(t, ruleStore.Calls[0].Args[4])
assert.False(t, ruleStore.Calls[0].Args[5].(bool))
})
t.Run("propagates errors", func(t *testing.T) {
t.Run("when unable to read config", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()
@@ -787,6 +904,36 @@ func TestUpdateMuteTimings(t *testing.T) {
require.Equal(t, "Save", store.Calls[1].Method)
})
t.Run("when RenameTimeIntervalInNotificationSettings fails", func(t *testing.T) {
sut, store, prov := createMuteTimingSvcSut()
store.GetFn = func(ctx context.Context, orgID int64) (*legacy_storage.ConfigRevision, error) {
return &legacy_storage.ConfigRevision{Config: initialConfig()}, nil
}
prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(expectedProvenance, nil)
prov.EXPECT().DeleteProvenance(mock.Anything, mock.Anything, mock.Anything).Return(nil)
prov.EXPECT().SetProvenance(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
expectedErr := errors.New("test-err")
ruleStore := &fakeAlertRuleNotificationStore{
RenameTimeIntervalInNotificationSettingsFn: func(ctx context.Context, orgID int64, old, new string, validate func(models.Provenance) bool, dryRun bool) ([]models.AlertRuleKey, []models.AlertRuleKey, error) {
return nil, nil, expectedErr
},
}
sut.ruleNotificationsStore = ruleStore
interval := expected
interval.Name = "another-time-interval"
timing := definitions.MuteTimeInterval{
UID: expectedUID,
MuteTimeInterval: interval,
Version: originalVersion,
Provenance: definitions.Provenance(expectedProvenance),
}
_, err := sut.UpdateMuteTiming(context.Background(), timing, orgID)
require.ErrorIs(t, err, expectedErr)
})
})
}

View File

@@ -169,8 +169,9 @@ func (s *fakeRuleAccessControlService) CanWriteAllRules(ctx context.Context, use
type fakeAlertRuleNotificationStore struct {
Calls []call
RenameReceiverInNotificationSettingsFn func(ctx context.Context, orgID int64, oldReceiver, newReceiver string) (int, error)
ListNotificationSettingsFn func(ctx context.Context, q models.ListNotificationSettingsQuery) (map[models.AlertRuleKey][]models.NotificationSettings, error)
RenameReceiverInNotificationSettingsFn func(ctx context.Context, orgID int64, oldReceiver, newReceiver string) (int, error)
RenameTimeIntervalInNotificationSettingsFn func(ctx context.Context, orgID int64, old, new string, validate func(models.Provenance) bool, dryRun bool) ([]models.AlertRuleKey, []models.AlertRuleKey, error)
ListNotificationSettingsFn func(ctx context.Context, q models.ListNotificationSettingsQuery) (map[models.AlertRuleKey][]models.NotificationSettings, error)
}
func (f *fakeAlertRuleNotificationStore) RenameReceiverInNotificationSettings(ctx context.Context, orgID int64, oldReceiver, newReceiver string) (int, error) {
@@ -188,6 +189,21 @@ func (f *fakeAlertRuleNotificationStore) RenameReceiverInNotificationSettings(ct
return 0, nil
}
func (f *fakeAlertRuleNotificationStore) RenameTimeIntervalInNotificationSettings(ctx context.Context, orgID int64, oldTimeInterval, newTimeInterval string, validate func(models.Provenance) bool, dryRun bool) ([]models.AlertRuleKey, []models.AlertRuleKey, error) {
call := call{
Method: "RenameTimeIntervalInNotificationSettings",
Args: []interface{}{ctx, orgID, oldTimeInterval, newTimeInterval, validate, dryRun},
}
f.Calls = append(f.Calls, call)
if f.RenameTimeIntervalInNotificationSettingsFn != nil {
return f.RenameTimeIntervalInNotificationSettingsFn(ctx, orgID, oldTimeInterval, newTimeInterval, validate, dryRun)
}
// Default values when no function hook is provided
return nil, nil, nil
}
func (f *fakeAlertRuleNotificationStore) ListNotificationSettings(ctx context.Context, q models.ListNotificationSettingsQuery) (map[models.AlertRuleKey][]models.NotificationSettings, error) {
call := call{
Method: "ListNotificationSettings",

View File

@@ -26,3 +26,11 @@ func ValidateProvenanceRelaxed(from, to models.Provenance) error {
}
return nil
}
// ValidateProvenanceOfDependentResources returns a list of allowed provenance statuses for dependent resources
// in the case when they need to be updated when the resource they depend on is changed.
func ValidateProvenanceOfDependentResources(parentProvenance models.Provenance) func(childProvenance models.Provenance) bool {
return func(childProvenance models.Provenance) bool {
return parentProvenance == childProvenance || childProvenance == models.ProvenanceNone
}
}

View File

@@ -839,6 +839,81 @@ func (st DBstore) RenameReceiverInNotificationSettings(ctx context.Context, orgI
return len(updates), st.UpdateAlertRules(ctx, updates)
}
// RenameTimeIntervalInNotificationSettings renames all rules that use old time interval name to the new name.
// Before renaming, it checks that all rules that need to be updated have allowed provenance status, and skips updating
// if at least one rule does not have allowed provenance.
// It returns a tuple:
// - a collection of models.AlertRuleKey of rules that were updated,
// - a collection of rules that have invalid provenance status,
// - database error
func (st DBstore) RenameTimeIntervalInNotificationSettings(
ctx context.Context,
orgID int64,
oldTimeInterval, newTimeInterval string,
validateProvenance func(ngmodels.Provenance) bool,
dryRun bool,
) ([]ngmodels.AlertRuleKey, []ngmodels.AlertRuleKey, error) {
// fetch entire rules because Update method requires it because it copies rules to version table
rules, err := st.ListAlertRules(ctx, &ngmodels.ListAlertRulesQuery{
OrgID: orgID,
TimeIntervalName: oldTimeInterval,
})
if err != nil {
return nil, nil, err
}
if len(rules) == 0 {
return nil, nil, nil
}
provenances, err := st.GetProvenances(ctx, orgID, (&ngmodels.AlertRule{}).ResourceType())
if err != nil {
return nil, nil, err
}
var invalidProvenance []ngmodels.AlertRuleKey
result := make([]ngmodels.AlertRuleKey, 0, len(rules))
updates := make([]ngmodels.UpdateRule, 0, len(rules))
for _, rule := range rules {
provenance, ok := provenances[rule.UID]
if !ok {
provenance = ngmodels.ProvenanceNone
}
if !validateProvenance(provenance) {
invalidProvenance = append(invalidProvenance, rule.GetKey())
}
if len(invalidProvenance) > 0 { // do not do any fixes if there is at least one rule with not allowed provenance
continue
}
result = append(result, rule.GetKey())
if dryRun {
continue
}
r := ngmodels.CopyRule(rule)
for idx := range r.NotificationSettings {
for mtIdx := range r.NotificationSettings[idx].MuteTimeIntervals {
if r.NotificationSettings[idx].MuteTimeIntervals[mtIdx] == oldTimeInterval {
r.NotificationSettings[idx].MuteTimeIntervals[mtIdx] = newTimeInterval
}
}
}
updates = append(updates, ngmodels.UpdateRule{
Existing: rule,
New: *r,
})
}
if len(invalidProvenance) > 0 {
return nil, invalidProvenance, nil
}
if dryRun {
return result, nil, nil
}
return result, nil, st.UpdateAlertRules(ctx, updates)
}
func ruleConstraintViolationToErr(rule ngmodels.AlertRule, err error) error {
msg := err.Error()
if strings.Contains(msg, "UQE_alert_rule_org_id_namespace_uid_title") || strings.Contains(msg, "alert_rule.org_id, alert_rule.namespace_uid, alert_rule.title") {

View File

@@ -811,6 +811,12 @@ func TestIntegrationAlertRulesNotificationSettings(t *testing.T) {
r.ID = 0
deref = append(deref, r)
}
provenances := make(map[models.AlertRuleKey]models.Provenance, len(receiveRules)+len(timeIntervalRules))
for idx, rule := range append(timeIntervalRules, receiveRules...) {
p := models.KnownProvenances[idx%len(models.KnownProvenances)]
provenances[rule.GetKey()] = p
require.NoError(t, store.SetProvenance(context.Background(), rule, rule.OrgID, p))
}
_, err := store.InsertAlertRules(context.Background(), deref)
require.NoError(t, err)
@@ -879,18 +885,16 @@ func TestIntegrationAlertRulesNotificationSettings(t *testing.T) {
require.NoError(t, err)
require.Equal(t, len(receiveRules), affected)
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: newName,
})
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())
}
actual, err = store.ListAlertRules(context.Background(), &models.ListAlertRulesQuery{
@@ -899,6 +903,100 @@ func TestIntegrationAlertRulesNotificationSettings(t *testing.T) {
})
require.NoError(t, err)
require.Empty(t, actual)
t.Run("should do nothing if no rules that match the filter", func(t *testing.T) {
affected, err := store.RenameReceiverInNotificationSettings(context.Background(), 1, receiverName, util.GenerateShortUID())
require.NoError(t, err)
require.Empty(t, affected)
})
})
t.Run("RenameTimeIntervalInNotificationSettings", func(t *testing.T) {
newName := "new-time-interval"
alwaysTrue := func(p models.Provenance) bool {
return true
}
t.Run("should do nothing if no rules that match the filter", func(t *testing.T) {
affected, invalidProvenance, err := store.RenameTimeIntervalInNotificationSettings(context.Background(), 1, "not-found", timeIntervalName, alwaysTrue, false)
require.NoError(t, err)
require.Empty(t, affected)
require.Empty(t, invalidProvenance)
})
t.Run("should do nothing if at least one rule has provenance that is not allowed", func(t *testing.T) {
calledTimes := 0
alwaysFalse := func(p models.Provenance) bool {
calledTimes++
return false
}
affected, invalidProvenance, err := store.RenameTimeIntervalInNotificationSettings(context.Background(), 1, timeIntervalName, newName, alwaysFalse, false)
var expected []models.AlertRuleKey
for _, rule := range timeIntervalRules {
expected = append(expected, rule.GetKey())
}
require.NoError(t, err)
require.Empty(t, affected)
require.ElementsMatch(t, expected, invalidProvenance)
assert.Equal(t, len(expected), calledTimes)
actual, err := store.ListAlertRules(context.Background(), &models.ListAlertRulesQuery{
OrgID: 1,
TimeIntervalName: timeIntervalName,
})
require.NoError(t, err)
assert.Len(t, actual, len(timeIntervalRules))
})
t.Run("should do nothing if dry run is set to true", func(t *testing.T) {
affected, invalidProvenance, err := store.RenameTimeIntervalInNotificationSettings(context.Background(), 1, timeIntervalName, newName, alwaysTrue, true)
require.NoError(t, err)
require.Empty(t, invalidProvenance)
assert.Len(t, affected, len(timeIntervalRules))
expected := getKeyMap(timeIntervalRules)
for _, key := range affected {
assert.Contains(t, expected, key)
}
actual, err := store.ListAlertRules(context.Background(), &models.ListAlertRulesQuery{
OrgID: 1,
TimeIntervalName: timeIntervalName,
})
require.NoError(t, err)
assert.Len(t, actual, len(timeIntervalRules))
})
t.Run("should update all rules that refer to the old time interval", func(t *testing.T) {
affected, invalidProvenance, err := store.RenameTimeIntervalInNotificationSettings(context.Background(), 1, timeIntervalName, newName, alwaysTrue, false)
require.NoError(t, err)
require.Empty(t, invalidProvenance)
assert.Len(t, affected, len(timeIntervalRules))
expected := getKeyMap(timeIntervalRules)
for _, key := range affected {
assert.Contains(t, expected, key)
}
actual, err := store.ListAlertRules(context.Background(), &models.ListAlertRulesQuery{
OrgID: 1,
TimeIntervalName: newName,
})
require.NoError(t, err)
assert.Len(t, actual, len(expected))
for _, rule := range actual {
assert.Contains(t, expected, rule.GetKey())
}
actual, err = store.ListAlertRules(context.Background(), &models.ListAlertRulesQuery{
OrgID: 1,
TimeIntervalName: timeIntervalName,
})
require.NoError(t, err)
require.Empty(t, actual)
})
})
}

View File

@@ -780,7 +780,7 @@ func TestMuteTimings(t *testing.T) {
},
MuteTimeIntervals: []string{anotherMuteTiming.Name},
})
status, response = apiClient.UpdateRouteWithStatus(t, route)
status, response = apiClient.UpdateRouteWithStatus(t, route, false)
requireStatusCode(t, http.StatusAccepted, status, response)
status, response = apiClient.DeleteMuteTimingWithStatus(t, anotherMuteTiming.Name)

View File

@@ -783,7 +783,15 @@ func (a apiClient) GetRouteWithStatus(t *testing.T) (apimodels.Route, int, strin
return sendRequest[apimodels.Route](t, req, http.StatusOK)
}
func (a apiClient) UpdateRouteWithStatus(t *testing.T, route apimodels.Route) (int, string) {
func (a apiClient) GetRoute(t *testing.T) apimodels.Route {
t.Helper()
route, status, data := a.GetRouteWithStatus(t)
requireStatusCode(t, http.StatusOK, status, data)
return route
}
func (a apiClient) UpdateRouteWithStatus(t *testing.T, route apimodels.Route, noProvenance bool) (int, string) {
t.Helper()
buf := bytes.Buffer{}
@@ -793,6 +801,9 @@ func (a apiClient) UpdateRouteWithStatus(t *testing.T, route apimodels.Route) (i
req, err := http.NewRequest(http.MethodPut, fmt.Sprintf("%s/api/v1/provisioning/policies", a.url), &buf)
req.Header.Add("Content-Type", "application/json")
if noProvenance {
req.Header.Add("X-Disable-Provenance", "true")
}
require.NoError(t, err)
client := &http.Client{}
@@ -807,6 +818,12 @@ func (a apiClient) UpdateRouteWithStatus(t *testing.T, route apimodels.Route) (i
return resp.StatusCode, string(body)
}
func (a apiClient) UpdateRoute(t *testing.T, route apimodels.Route, noProvenance bool) {
t.Helper()
status, data := a.UpdateRouteWithStatus(t, route, noProvenance)
requireStatusCode(t, http.StatusAccepted, status, data)
}
func (a apiClient) GetRuleHistoryWithStatus(t *testing.T, ruleUID string) (data.Frame, int, string) {
t.Helper()
u, err := url.Parse(fmt.Sprintf("%s/api/v1/rules/history", a.url))
@@ -877,6 +894,7 @@ func (a apiClient) GetActiveAlertsWithStatus(t *testing.T) (apimodels.AlertGroup
}
func sendRequest[T any](t *testing.T, req *http.Request, successStatusCode int) (T, int, string) {
t.Helper()
client := &http.Client{}
resp, err := client.Do(req)
require.NoError(t, err)
@@ -898,5 +916,6 @@ func sendRequest[T any](t *testing.T, req *http.Request, successStatusCode int)
}
func requireStatusCode(t *testing.T, expected, actual int, response string) {
t.Helper()
require.Equalf(t, expected, actual, "Unexpected status. Response: %s", response)
}

View File

@@ -14,7 +14,7 @@
]
],
"mute_time_intervals": [
"test-interval"
"test-interval", "persisted-interval"
]
}
]
@@ -28,6 +28,15 @@
"end_time": "23:59"
}
]
},
{
"name": "persisted-interval",
"time_intervals": [
{
"start_time": "06:00",
"end_time": "23:59"
}
]
}
],
"receivers": [

View File

@@ -28,7 +28,8 @@
"notification_settings": {
"receiver": "grafana-default-email",
"mute_time_intervals": [
"test-interval"
"test-interval",
"persisted-interval"
]
}
}

View File

@@ -7,9 +7,11 @@ import (
"fmt"
"net/http"
"path"
"slices"
"testing"
"github.com/prometheus/alertmanager/config"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/api/errors"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -25,6 +27,7 @@ import (
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/folder/foldertest"
"github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
"github.com/grafana/grafana/pkg/services/ngalert/store"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/tests/api/alerting"
@@ -95,14 +98,21 @@ func TestIntegrationResourceIdentifier(t *testing.T) {
existingInterval = actual
})
t.Run("update should fail if name in the specification changes", func(t *testing.T) {
t.Run("update should rename interval if name in the specification changes", func(t *testing.T) {
if existingInterval == nil {
t.Skip()
}
updated := existingInterval.DeepCopy()
updated.Spec.Name = "another-newInterval"
_, err := client.Update(ctx, updated, v1.UpdateOptions{})
require.Truef(t, errors.IsBadRequest(err), "Expected BadRequest but got %s", err)
actual, err := client.Update(ctx, updated, v1.UpdateOptions{})
require.NoError(t, err)
require.Equal(t, updated.Spec, actual.Spec)
require.NotEqualf(t, updated.Name, actual.Name, "Update should change the resource name but it didn't")
require.NotEqualf(t, updated.ResourceVersion, actual.ResourceVersion, "Update should change the resource version but it didn't")
resource, err := client.Get(ctx, actual.Name, v1.GetOptions{})
require.NoError(t, err)
require.Equal(t, actual, resource)
})
}
@@ -641,6 +651,11 @@ func TestIntegrationTimeIntervalReferentialIntegrity(t *testing.T) {
ctx := context.Background()
helper := getTestHelper(t)
env := helper.GetEnv()
ac := acimpl.ProvideAccessControl(env.FeatureToggles, zanzana.NewNoopClient())
db, err := store.ProvideDBStore(env.Cfg, env.FeatureToggles, env.SQLStore, &foldertest.FakeService{}, &dashboards.FakeDashboardService{}, ac)
require.NoError(t, err)
orgID := helper.Org1.Admin.Identity.GetOrgID()
cliCfg := helper.Org1.Admin.NewRestConfig()
legacyCli := alerting.NewAlertingLegacyAPIClient(helper.GetEnv().Server.HTTPServer.Listener.Addr().String(), cliCfg.Username, cliCfg.Password)
@@ -664,27 +679,100 @@ func TestIntegrationTimeIntervalReferentialIntegrity(t *testing.T) {
_, status, data := legacyCli.PostRulesGroupWithStatus(t, folderUID, &ruleGroup)
require.Equalf(t, http.StatusAccepted, status, "Failed to post Rule: %s", data)
currentRoute := legacyCli.GetRoute(t)
currentRuleGroup := legacyCli.GetRulesGroup(t, folderUID, ruleGroup.Name)
adminK8sClient, err := versioned.NewForConfig(cliCfg)
require.NoError(t, err)
adminClient := adminK8sClient.NotificationsV0alpha1().TimeIntervals("default")
intervals, err := adminClient.List(ctx, v1.ListOptions{})
require.NoError(t, err)
require.Len(t, intervals.Items, 1)
require.Len(t, intervals.Items, 2)
intervalIdx := slices.IndexFunc(intervals.Items, func(interval v0alpha1.TimeInterval) bool {
return interval.Spec.Name == "test-interval"
})
interval := intervals.Items[intervalIdx]
intervalToDelete := intervals.Items[0]
replace := func(input []string, oldName, newName string) []string {
result := make([]string, 0, len(input))
for _, s := range input {
if s == oldName {
result = append(result, newName)
continue
}
result = append(result, s)
}
return result
}
t.Run("should fail to delete if time interval is used in rule and routes", func(t *testing.T) {
err := adminClient.Delete(ctx, intervalToDelete.Name, v1.DeleteOptions{})
require.Truef(t, errors.IsConflict(err), "Expected Conflict, got: %s", err)
t.Run("Update", func(t *testing.T) {
t.Run("should rename all references if name changes", func(t *testing.T) {
renamed := interval.DeepCopy()
renamed.Spec.Name += "-new"
actual, err := adminClient.Update(ctx, renamed, v1.UpdateOptions{})
require.NoError(t, err)
updatedRuleGroup := legacyCli.GetRulesGroup(t, folderUID, ruleGroup.Name)
for idx, rule := range updatedRuleGroup.Rules {
expectedTimeIntervals := currentRuleGroup.Rules[idx].GrafanaManagedAlert.NotificationSettings.MuteTimeIntervals
expectedTimeIntervals = replace(expectedTimeIntervals, interval.Spec.Name, actual.Spec.Name)
assert.Equalf(t, expectedTimeIntervals, rule.GrafanaManagedAlert.NotificationSettings.MuteTimeIntervals, "time interval in rules should have been renamed but it did not")
}
updatedRoute := legacyCli.GetRoute(t)
for idx, route := range updatedRoute.Routes {
expectedTimeIntervals := replace(currentRoute.Routes[idx].MuteTimeIntervals, interval.Spec.Name, actual.Spec.Name)
assert.Equalf(t, expectedTimeIntervals, route.MuteTimeIntervals, "time interval in routes should have been renamed but it did not")
}
interval = *actual
})
t.Run("should fail if at least one resource is provisioned", func(t *testing.T) {
require.NoError(t, err)
renamed := interval.DeepCopy()
renamed.Spec.Name += util.GenerateShortUID()
t.Run("provisioned route", func(t *testing.T) {
require.NoError(t, db.SetProvenance(ctx, &currentRoute, orgID, "API"))
t.Cleanup(func() {
require.NoError(t, db.DeleteProvenance(ctx, &currentRoute, orgID))
})
actual, err := adminClient.Update(ctx, renamed, v1.UpdateOptions{})
require.Errorf(t, err, "Expected error but got successful result: %v", actual)
require.Truef(t, errors.IsConflict(err), "Expected Conflict, got: %s", err)
})
t.Run("provisioned rules", func(t *testing.T) {
ruleUid := currentRuleGroup.Rules[0].GrafanaManagedAlert.UID
resource := &ngmodels.AlertRule{UID: ruleUid}
require.NoError(t, db.SetProvenance(ctx, resource, orgID, "API"))
t.Cleanup(func() {
require.NoError(t, db.DeleteProvenance(ctx, resource, orgID))
})
actual, err := adminClient.Update(ctx, renamed, v1.UpdateOptions{})
require.Errorf(t, err, "Expected error but got successful result: %v", actual)
require.Truef(t, errors.IsConflict(err), "Expected Conflict, got: %s", err)
})
})
})
t.Run("should fail to delete if time interval is used in only rule", func(t *testing.T) {
amConfig.AlertmanagerConfig.Route.Routes[0].MuteTimeIntervals = nil
success, err := legacyCli.PostConfiguration(t, amConfig)
require.Truef(t, success, "Failed to post Alertmanager configuration: %s", err)
t.Run("Delete", func(t *testing.T) {
t.Run("should fail to delete if time interval is used in rule and routes", func(t *testing.T) {
err := adminClient.Delete(ctx, interval.Name, v1.DeleteOptions{})
require.Truef(t, errors.IsConflict(err), "Expected Conflict, got: %s", err)
})
err = adminClient.Delete(ctx, intervalToDelete.Name, v1.DeleteOptions{})
require.Truef(t, errors.IsConflict(err), "Expected Conflict, got: %s", err)
t.Run("should fail to delete if time interval is used in only rule", func(t *testing.T) {
route := legacyCli.GetRoute(t)
route.Routes[0].MuteTimeIntervals = nil
legacyCli.UpdateRoute(t, route, true)
err = adminClient.Delete(ctx, interval.Name, v1.DeleteOptions{})
require.Truef(t, errors.IsConflict(err), "Expected Conflict, got: %s", err)
})
})
}