IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Alerting: Store information about user that created\updated alert rule (#99395)

Browse Source 
* introduce new fields created_by in rule tables
* update domain model and compat layer to support UpdatedBy
* add alert rule generator mutators for UpdatedBy
* ignore UpdatedBy in diff and hash calculation
* Add user context to alert rule insert/update operations
  Updated InsertAlertRules and UpdateAlertRules methods to accept a user context parameter. This change ensures auditability and better tracking of user actions when creating or updating alert rules. Adjusted all relevant calls and interfaces to pass the user context accordingly.

* set UpdatedBy in PreSave because this is where Updated is set
* Use nil userID for system-initiated updates
This ensures differentiation between system and user-initiated changes for better traceability and clarity in update origins.

---------

Signed-off-by: Yuri Tseretyan <yuriy.tseretyan@grafana.com>
This commit is contained in:
Yuri Tseretyan 2025-01-24 12:09:17 -05:00 committed by GitHub
parent b3209b1e01
commit 92d6762a3a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
21 changed files with 224 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
pkg/services/annotations/annotationsimpl/loki/historian_store_test.go
View File

@ -636,7 +636,7 @@ func createAlertRule(t *testing.T, sql *sqlstore.SQLStore, title string, generat
rule.PanelID = nil
ruleStore := store.SetupStoreForTesting(t, sql)
ids, err := ruleStore.InsertAlertRules(context.Background(), []ngmodels.AlertRule{rule})
ids, err := ruleStore.InsertAlertRules(context.Background(), nil, []ngmodels.AlertRule{rule})
require.NoError(t, err)
result, err := ruleStore.GetAlertRuleByUID(context.Background(), &ngmodels.GetAlertRuleByUIDQuery{OrgID: rule.OrgID, UID: ids[0].UID})
require.NoError(t, err)
@ -668,7 +668,7 @@ func createAlertRuleFromDashboard(t *testing.T, sql *sqlstore.SQLStore, title st
rule.PanelID = panelID
}
ruleStore := store.SetupStoreForTesting(t, sql)
ids, err := ruleStore.InsertAlertRules(context.Background(), []ngmodels.AlertRule{rule})
ids, err := ruleStore.InsertAlertRules(context.Background(), nil, []ngmodels.AlertRule{rule})
require.NoError(t, err)
result, err := ruleStore.GetAlertRuleByUID(context.Background(), &ngmodels.GetAlertRuleByUIDQuery{OrgID: rule.OrgID, UID: ids[0].UID})
require.NoError(t, err)

2
pkg/services/folder/folderimpl/folder_test.go
View File

@ -2581,7 +2581,7 @@ func createRule(t *testing.T, store *ngstore.DBstore, folderUID, title string) *
gen.WithNamespaceUID(folderUID),
gen.WithIntervalSeconds(10),
).Generate()
ids, err := store.InsertAlertRules(context.Background(), []models.AlertRule{rule})
ids, err := store.InsertAlertRules(context.Background(), nil, []models.AlertRule{rule})
require.NoError(t, err)
result, err := store.GetAlertRuleByUID(context.Background(), &models.GetAlertRuleByUIDQuery{OrgID: orgID, UID: ids[0].UID})

4
pkg/services/ngalert/api/api_ruler.go
View File

@ -443,7 +443,7 @@ func (srv RulerSrv) updateAlertRulesInGroup(c *contextmodel.ReqContext, groupKey
New: *update.New,
})
}
err = srv.store.UpdateAlertRules(tranCtx, updates)
err = srv.store.UpdateAlertRules(tranCtx, ngmodels.NewUserUID(c.SignedInUser), updates)
if err != nil {
return fmt.Errorf("failed to update rules: %w", err)
}
@ -454,7 +454,7 @@ func (srv RulerSrv) updateAlertRulesInGroup(c *contextmodel.ReqContext, groupKey
for _, rule := range finalChanges.New {
inserts = append(inserts, *rule)
}
added, err := srv.store.InsertAlertRules(tranCtx, inserts)
added, err := srv.store.InsertAlertRules(tranCtx, ngmodels.NewUserUID(c.SignedInUser), inserts)
if err != nil {
return fmt.Errorf("failed to add rules: %w", err)
}

4
pkg/services/ngalert/api/persist.go
View File

@ -22,8 +22,8 @@ type RuleStore interface {
// InsertAlertRules will insert all alert rules passed into the function
// and return the map of uuid to id.
InsertAlertRules(ctx context.Context, rule []ngmodels.AlertRule) ([]ngmodels.AlertRuleKeyWithId, error)
UpdateAlertRules(ctx context.Context, rule []ngmodels.UpdateRule) error
InsertAlertRules(ctx context.Context, user *ngmodels.UserUID, rules []ngmodels.AlertRule) ([]ngmodels.AlertRuleKeyWithId, error)
UpdateAlertRules(ctx context.Context, user *ngmodels.UserUID, rules []ngmodels.UpdateRule) error
DeleteAlertRulesByUID(ctx context.Context, orgID int64, ruleUID ...string) error
// IncreaseVersionForAllRulesInNamespaces Increases version for all rules that have specified namespace uids

19
pkg/services/ngalert/models/alert_rule.go
View File

@ -243,6 +243,21 @@ func SortAlertRuleGroupWithFolderTitle(g []AlertRuleGroupWithFolderFullpath) {
})
}
type UserUID string
func NewUserUID(requester interface{ GetIdentifier() string }) *UserUID {
// use anonymous interface to abstract from identity package, which is part of apimachinery
if requester == nil {
return nil
}
identifier := requester.GetIdentifier()
if identifier == "" {
return nil
}
userUID := UserUID(identifier)
return &userUID
}
// AlertRule is the model for alert rules in unified alerting.
type AlertRule struct {
ID int64
@ -251,6 +266,7 @@ type AlertRule struct {
Condition string
Data []AlertQuery
Updated time.Time
UpdatedBy *UserUID
IntervalSeconds int64
Version int64
UID string
@ -535,7 +551,7 @@ func (alertRule *AlertRule) GetGroupKey() AlertRuleGroupKey {
}
// PreSave sets default values and loads the updated model for each alert query.
func (alertRule *AlertRule) PreSave(timeNow func() time.Time) error {
func (alertRule *AlertRule) PreSave(timeNow func() time.Time, userUID *UserUID) error {
for i, q := range alertRule.Data {
err := q.PreSave()
if err != nil {
@ -544,6 +560,7 @@ func (alertRule *AlertRule) PreSave(timeNow func() time.Time) error {
alertRule.Data[i] = q
}
alertRule.Updated = timeNow()
alertRule.UpdatedBy = userUID
return nil
}

5
pkg/services/ngalert/models/alert_rule_test.go
View File

@ -445,6 +445,11 @@ func TestDiff(t *testing.T) {
assert.Equal(t, rule2.Updated, diff[0].Right.Interface())
difCnt++
}
if rule1.UpdatedBy != rule2.UpdatedBy {
diff := diffs.GetDiffsForField("UpdatedBy")
assert.Len(t, diff, 1)
difCnt++
}
if rule1.IntervalSeconds != rule2.IntervalSeconds {
diff := diffs.GetDiffsForField("IntervalSeconds")
assert.Len(t, diff, 1)

13
pkg/services/ngalert/models/testing.go
View File

@ -96,6 +96,11 @@ func (g *AlertRuleGenerator) Generate() AlertRule {
ns = append(ns, NotificationSettingsGen()())
}
var updatedBy *UserUID
if rand.Int63()%2 == 0 {
updatedBy = util.Pointer(UserUID(util.GenerateShortUID()))
}
rule := AlertRule{
ID: 0,
OrgID: rand.Int63n(1500) + 1, // Prevent OrgID=0 as this does not pass alert rule validation.
@ -103,6 +108,7 @@ func (g *AlertRuleGenerator) Generate() AlertRule {
Condition: "A",
Data: []AlertQuery{g.GenerateQuery()},
Updated: time.Now().Add(-time.Duration(rand.Intn(100) + 1)),
UpdatedBy: updatedBy,
IntervalSeconds: rand.Int63n(60) + 1,
Version: rand.Int63n(1500), // Don't generate a rule ID too big for postgres
UID: util.GenerateShortUID(),
@ -538,6 +544,12 @@ func (a *AlertRuleMutators) WithRecordFrom(from string) AlertRuleMutator {
}
}
func (a *AlertRuleMutators) WithUpdatedBy(uid *UserUID) AlertRuleMutator {
return func(r *AlertRule) {
r.UpdatedBy = uid
}
}
func (g *AlertRuleGenerator) GenerateLabels(min, max int, prefix string) data.Labels {
count := max
if min > max {
@ -618,6 +630,7 @@ func CopyRule(r *AlertRule, mutators ...AlertRuleMutator) *AlertRule {
Title: r.Title,
Condition: r.Condition,
Updated: r.Updated,
UpdatedBy: r.UpdatedBy,
IntervalSeconds: r.IntervalSeconds,
Version: r.Version,
UID: r.UID,

10
pkg/services/ngalert/provisioning/alert_rules.go
View File

@ -232,7 +232,7 @@ func (service *AlertRuleService) CreateAlertRule(ctx context.Context, user ident
}
}
err = service.xact.InTransaction(ctx, func(ctx context.Context) error {
ids, err := service.ruleStore.InsertAlertRules(ctx, []models.AlertRule{
ids, err := service.ruleStore.InsertAlertRules(ctx, models.NewUserUID(user), []models.AlertRule{
rule,
})
if err != nil {
@ -359,7 +359,7 @@ func (service *AlertRuleService) UpdateRuleGroup(ctx context.Context, user ident
}
}
return service.ruleStore.UpdateAlertRules(ctx, updateRules)
return service.ruleStore.UpdateAlertRules(ctx, models.NewUserUID(user), updateRules)
})
}
@ -511,7 +511,7 @@ func (service *AlertRuleService) persistDelta(ctx context.Context, user identity
New: *update.New,
})
}
if err := service.ruleStore.UpdateAlertRules(ctx, updates); err != nil {
if err := service.ruleStore.UpdateAlertRules(ctx, models.NewUserUID(user), updates); err != nil {
return fmt.Errorf("failed to update alert rules: %w", err)
}
for _, update := range delta.Update {
@ -522,7 +522,7 @@ func (service *AlertRuleService) persistDelta(ctx context.Context, user identity
}
if len(delta.New) > 0 {
uids, err := service.ruleStore.InsertAlertRules(ctx, withoutNilAlertRules(delta.New))
uids, err := service.ruleStore.InsertAlertRules(ctx, models.NewUserUID(user), withoutNilAlertRules(delta.New))
if err != nil {
return fmt.Errorf("failed to insert alert rules: %w", err)
}
@ -618,7 +618,7 @@ func (service *AlertRuleService) UpdateAlertRule(ctx context.Context, user ident
return models.AlertRule{}, err
}
err = service.xact.InTransaction(ctx, func(ctx context.Context) error {
err := service.ruleStore.UpdateAlertRules(ctx, []models.UpdateRule{
err := service.ruleStore.UpdateAlertRules(ctx, models.NewUserUID(user), []models.UpdateRule{
{
Existing: storedRule,
New: rule,

17
pkg/services/ngalert/provisioning/alert_rules_test.go
View File

@ -41,8 +41,9 @@ func TestAlertRuleService(t *testing.T) {
ruleService := createAlertRuleService(t, nil)
var orgID int64 = 1
u := &user.SignedInUser{
UserID: 1,
OrgID: orgID,
UserUID: util.GenerateShortUID(),
UserID: 1,
OrgID: orgID,
}
t.Run("group creation should set the right provenance", func(t *testing.T) {
@ -196,7 +197,7 @@ func TestAlertRuleService(t *testing.T) {
},
}
rule.Metadata = ruleMetadata
r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), []models.AlertRule{rule})
r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.AlertRule{rule})
require.NoError(t, err)
require.Len(t, r, 1)
@ -233,7 +234,7 @@ func TestAlertRuleService(t *testing.T) {
},
}
rule.Metadata = ruleMetadata
r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), []models.AlertRule{rule})
r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.AlertRule{rule})
require.NoError(t, err)
require.Len(t, r, 1)
@ -624,7 +625,7 @@ func TestAlertRuleService(t *testing.T) {
func TestCreateAlertRule(t *testing.T) {
orgID := rand.Int63()
u := &user.SignedInUser{OrgID: orgID}
u := &user.SignedInUser{OrgID: orgID, UserUID: util.GenerateShortUID()}
groupKey := models.GenerateGroupKey(orgID)
groupIntervalSeconds := int64(30)
gen := models.RuleGen
@ -1008,7 +1009,7 @@ func TestUpdateAlertRule(t *testing.T) {
rule := models.CopyRule(rules[0])
_, err := service.ruleStore.InsertAlertRules(context.Background(), []models.AlertRule{*rule})
_, err := service.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.AlertRule{*rule})
require.NoError(t, err)
ac.CanWriteAllRulesFunc = func(ctx context.Context, user identity.Requester) (bool, error) {
@ -1626,7 +1627,7 @@ func TestProvisiongWithFullpath(t *testing.T) {
require.NoError(t, err)
t.Run("for a rule under a root folder should set the right fullpath", func(t *testing.T) {
r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), []models.AlertRule{
r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(&signedInUser), []models.AlertRule{
createTestRule("my-cool-group", "my-cool-group", orgID, namespaceUID),
})
require.NoError(t, err)
@ -1657,7 +1658,7 @@ func TestProvisiongWithFullpath(t *testing.T) {
})
require.NoError(t, err)
r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), []models.AlertRule{
r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(&signedInUser), []models.AlertRule{
createTestRule("my-cool-group-2", "my-cool-group-2", orgID, otherNamespaceUID),
})
require.NoError(t, err)

4
pkg/services/ngalert/provisioning/persist.go
View File

@ -33,8 +33,8 @@ type RuleStore interface {
GetAlertRuleByUID(ctx context.Context, query *models.GetAlertRuleByUIDQuery) (*models.AlertRule, error)
ListAlertRules(ctx context.Context, query *models.ListAlertRulesQuery) (models.RulesGroup, error)
GetRuleGroupInterval(ctx context.Context, orgID int64, namespaceUID string, ruleGroup string) (int64, error)
InsertAlertRules(ctx context.Context, rule []models.AlertRule) ([]models.AlertRuleKeyWithId, error)
UpdateAlertRules(ctx context.Context, rule []models.UpdateRule) error
InsertAlertRules(ctx context.Context, user *models.UserUID, rule []models.AlertRule) ([]models.AlertRuleKeyWithId, error)
UpdateAlertRules(ctx context.Context, user *models.UserUID, rule []models.UpdateRule) error
DeleteAlertRulesByUID(ctx context.Context, orgID int64, ruleUID ...string) error
GetAlertRulesGroupByRuleUID(ctx context.Context, query *models.GetAlertRulesGroupByRuleUIDQuery) ([]*models.AlertRule, error)
}

1
pkg/services/ngalert/schedule/registry_test.go
View File

@ -259,6 +259,7 @@ func TestRuleWithFolderFingerprint(t *testing.T) {
excludedFields := map[string]struct{}{
"Version": {},
"Updated": {},
"UpdatedBy": {},
"IntervalSeconds": {},
"Annotations": {},
}

16
pkg/services/ngalert/store/alert_rule.go
View File

@ -181,7 +181,7 @@ func (st DBstore) GetAlertRulesGroupByRuleUID(ctx context.Context, query *ngmode
// InsertAlertRules is a handler for creating/updating alert rules.
// Returns the UID and ID of rules that were created in the same order as the input rules.
func (st DBstore) InsertAlertRules(ctx context.Context, rules []ngmodels.AlertRule) ([]ngmodels.AlertRuleKeyWithId, error) {
func (st DBstore) InsertAlertRules(ctx context.Context, user *ngmodels.UserUID, rules []ngmodels.AlertRule) ([]ngmodels.AlertRuleKeyWithId, error) {
ids := make([]ngmodels.AlertRuleKeyWithId, 0, len(rules))
keys := make([]ngmodels.AlertRuleKey, 0, len(rules))
return ids, st.SQLStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
@ -200,7 +200,7 @@ func (st DBstore) InsertAlertRules(ctx context.Context, rules []ngmodels.AlertRu
if err := st.validateAlertRule(r); err != nil {
return err
}
if err := (&r).PreSave(TimeNow); err != nil {
if err := (&r).PreSave(TimeNow, user); err != nil {
return err
}
@ -245,7 +245,7 @@ func (st DBstore) InsertAlertRules(ctx context.Context, rules []ngmodels.AlertRu
}
// UpdateAlertRules is a handler for updating alert rules.
func (st DBstore) UpdateAlertRules(ctx context.Context, rules []ngmodels.UpdateRule) error {
func (st DBstore) UpdateAlertRules(ctx context.Context, user *ngmodels.UserUID, rules []ngmodels.UpdateRule) error {
return st.SQLStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
err := st.preventIntermediateUniqueConstraintViolations(sess, rules)
if err != nil {
@ -263,7 +263,7 @@ func (st DBstore) UpdateAlertRules(ctx context.Context, rules []ngmodels.UpdateR
if err := st.validateAlertRule(r.New); err != nil {
return err
}
if err := (&r.New).PreSave(TimeNow); err != nil {
if err := (&r.New).PreSave(TimeNow, user); err != nil {
return err
}
converted, err := alertRuleFromModelsAlertRule(r.New)
@ -956,7 +956,9 @@ func (st DBstore) RenameReceiverInNotificationSettings(ctx context.Context, orgI
if dryRun {
return result, nil, nil
}
return result, nil, st.UpdateAlertRules(ctx, updates)
// Provide empty user identifier to ensure it's clear that the rule update was made by the system
// and not by the user who changed the receiver's title.
return result, nil, st.UpdateAlertRules(ctx, nil, updates)
}
// RenameTimeIntervalInNotificationSettings renames all rules that use old time interval name to the new name.
@ -1031,7 +1033,9 @@ func (st DBstore) RenameTimeIntervalInNotificationSettings(
if dryRun {
return result, nil, nil
}
return result, nil, st.UpdateAlertRules(ctx, updates)
// Provide empty user identifier to ensure it's clear that the rule update was made by the system
// and not by the user who changed the receiver's title.
return result, nil, st.UpdateAlertRules(ctx, nil, updates)
}
func ruleConstraintViolationToErr(sess *db.Session, rule ngmodels.AlertRule, err error, logger log.Logger) error {

143
pkg/services/ngalert/store/alert_rule_test.go
View File

@ -47,6 +47,7 @@ func TestIntegrationUpdateAlertRules(t *testing.T) {
folderService := setupFolderService(t, sqlStore, cfg, featuremgmt.WithFeatures())
b := &fakeBus{}
store := createTestStore(sqlStore, folderService, logger, cfg.UnifiedAlerting, b)
usr := models.UserUID("1234")
gen := models.RuleGen
gen = gen.With(gen.WithIntervalMatching(store.Cfg.BaseInterval))
@ -56,7 +57,7 @@ func TestIntegrationUpdateAlertRules(t *testing.T) {
rule := createRule(t, store, gen)
newRule := models.CopyRule(rule)
newRule.Title = util.GenerateShortUID()
err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
err := store.UpdateAlertRules(context.Background(), &usr, []models.UpdateRule{{
Existing: rule,
New: *newRule,
},
@ -79,7 +80,7 @@ func TestIntegrationUpdateAlertRules(t *testing.T) {
newRule := models.CopyRule(rule)
newRule.Record.Metric = "new_metric"
err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
err := store.UpdateAlertRules(context.Background(), &usr, []models.UpdateRule{{
Existing: rule,
New: *newRule,
},
@ -104,7 +105,7 @@ func TestIntegrationUpdateAlertRules(t *testing.T) {
newRule := models.CopyRule(rule)
newRule.Title = util.GenerateShortUID()
err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
err := store.UpdateAlertRules(context.Background(), &usr, []models.UpdateRule{{
Existing: rule,
New: *newRule,
},
@ -125,22 +126,78 @@ func TestIntegrationUpdateAlertRules(t *testing.T) {
called = true
return nil
}
t.Cleanup(func() {
b.publishFn = nil
})
newRule := models.CopyRule(rule)
newRule.Title = util.GenerateShortUID()
err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
err := store.UpdateAlertRules(context.Background(), &usr, []models.UpdateRule{{
Existing: rule,
New: *newRule,
}})
require.NoError(t, err)
require.True(t, called)
})
t.Run("should set UpdatedBy", func(t *testing.T) {
rule := createRule(t, store, gen)
newRule := models.CopyRule(rule)
newRule.Title = util.GenerateShortUID()
err := store.UpdateAlertRules(context.Background(), &usr, []models.UpdateRule{{
Existing: rule,
New: *newRule,
},
})
require.NoError(t, err)
dbrule := &alertRule{}
err = sqlStore.WithDbSession(context.Background(), func(sess *db.Session) error {
exist, err := sess.Table(alertRule{}).ID(rule.ID).Get(dbrule)
require.Truef(t, exist, fmt.Sprintf("rule with ID %d does not exist", rule.ID))
return err
})
require.NoError(t, err)
require.NotNil(t, dbrule.UpdatedBy)
require.Equal(t, string(usr), *dbrule.UpdatedBy)
t.Run("should set CreatedBy in rule version table", func(t *testing.T) {
dbVersion := &alertRuleVersion{}
err = sqlStore.WithDbSession(context.Background(), func(sess *db.Session) error {
exist, err := sess.Table(alertRuleVersion{}).Where("rule_uid = ? AND version = ?", dbrule.UID, dbrule.Version).Get(dbVersion)
require.Truef(t, exist, "new version of the rule does not exist in version table")
return err
})
require.NoError(t, err)
require.NotNil(t, dbVersion.CreatedBy)
require.Equal(t, *dbrule.UpdatedBy, *dbVersion.CreatedBy)
})
t.Run("nil identity should be handled correctly", func(t *testing.T) {
rule.Version++
newRule.Title = util.GenerateShortUID()
err = store.UpdateAlertRules(context.Background(), nil, []models.UpdateRule{{
Existing: rule,
New: *newRule,
},
})
dbrule := &alertRule{}
err = sqlStore.WithDbSession(context.Background(), func(sess *db.Session) error {
exist, err := sess.Table(alertRule{}).ID(rule.ID).Get(dbrule)
require.Truef(t, exist, fmt.Sprintf("rule with ID %d does not exist", rule.ID))
return err
})
assert.Nil(t, dbrule.UpdatedBy)
})
})
}
func TestIntegrationUpdateAlertRulesWithUniqueConstraintViolation(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test")
}
usr := models.UserUID("test")
cfg := setting.NewCfg()
cfg.UnifiedAlerting = setting.UnifiedAlertingSettings{BaseInterval: time.Duration(rand.Int63n(100)+1) * time.Second}
sqlStore := db.InitTestDB(t)
@ -167,7 +224,7 @@ func TestIntegrationUpdateAlertRulesWithUniqueConstraintViolation(t *testing.T)
newRule1.Title = rule2.Title
newRule2.Title = util.GenerateShortUID()
err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
err := store.UpdateAlertRules(context.Background(), &usr, []models.UpdateRule{{
Existing: rule1,
New: *newRule1,
}, {
@ -211,7 +268,7 @@ func TestIntegrationUpdateAlertRulesWithUniqueConstraintViolation(t *testing.T)
newRule2.Title = rule3.Title
newRule3.Title = rule1.Title
err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
err := store.UpdateAlertRules(context.Background(), &usr, []models.UpdateRule{{
Existing: rule1,
New: *newRule1,
}, {
@ -263,7 +320,7 @@ func TestIntegrationUpdateAlertRulesWithUniqueConstraintViolation(t *testing.T)
newRule1.Title = strings.ToUpper(rule2.Title)
newRule2.Title = strings.ToUpper(rule1.Title)
err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
err := store.UpdateAlertRules(context.Background(), &usr, []models.UpdateRule{{
Existing: rule1,
New: *newRule1,
}, {
@ -310,7 +367,7 @@ func TestIntegrationUpdateAlertRulesWithUniqueConstraintViolation(t *testing.T)
newRule3.Title = rule4.Title
newRule4.Title = rule3.Title
err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
err := store.UpdateAlertRules(context.Background(), &usr, []models.UpdateRule{{
Existing: rule1,
New: *newRule1,
}, {
@ -372,7 +429,7 @@ func TestIntegrationUpdateAlertRulesWithUniqueConstraintViolation(t *testing.T)
newRule2 := models.CopyRule(rule2)
newRule2.Title = newRule1.Title
err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
err := store.UpdateAlertRules(context.Background(), &usr, []models.UpdateRule{{
Existing: rule2,
New: *newRule2,
},
@ -704,6 +761,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) {
}
orgID := int64(1)
usr := models.UserUID("test")
sqlStore := db.InitTestDB(t)
cfg := setting.NewCfg()
cfg.UnifiedAlerting.BaseInterval = 1 * time.Second
@ -724,7 +782,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) {
rules := append(gen.GenerateMany(5), recordingRulesGen.GenerateMany(5)...)
ids, err := store.InsertAlertRules(context.Background(), rules)
ids, err := store.InsertAlertRules(context.Background(), &usr, rules)
require.NoError(t, err)
require.Len(t, ids, len(rules))
@ -776,12 +834,20 @@ func TestIntegrationInsertAlertRules(t *testing.T) {
}
})
t.Run("inserted rules should have UpdatedBy set", func(t *testing.T) {
for _, rule := range dbRules {
if assert.NotNil(t, rule.UpdatedBy) {
assert.Equal(t, usr, *rule.UpdatedBy)
}
}
})
t.Run("inserted recording rules fail validation if metric name is invalid", func(t *testing.T) {
t.Run("invalid UTF-8", func(t *testing.T) {
invalidMetric := "my_metric\x80"
invalidRule := recordingRulesGen.Generate()
invalidRule.Record.Metric = invalidMetric
_, err := store.InsertAlertRules(context.Background(), []models.AlertRule{invalidRule})
_, err := store.InsertAlertRules(context.Background(), &usr, []models.AlertRule{invalidRule})
require.ErrorIs(t, err, models.ErrAlertRuleFailedValidation)
require.ErrorContains(t, err, "metric name for recording rule must be a valid utf8 string")
})
@ -789,7 +855,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) {
t.Run("clears fields that should not exist on recording rules", func(t *testing.T) {
rule := recordingRulesGen.Generate()
rules, err := store.InsertAlertRules(context.Background(), []models.AlertRule{rule})
rules, err := store.InsertAlertRules(context.Background(), &usr, []models.AlertRule{rule})
require.NoError(t, err)
require.Len(t, rules, 1)
ruleUID := rules[0].UID
@ -806,13 +872,13 @@ func TestIntegrationInsertAlertRules(t *testing.T) {
})
t.Run("fail to insert rules with same ID", func(t *testing.T) {
_, err = store.InsertAlertRules(context.Background(), []models.AlertRule{rules[0]})
_, err = store.InsertAlertRules(context.Background(), &usr, []models.AlertRule{rules[0]})
require.ErrorIs(t, err, models.ErrAlertRuleConflictBase)
})
t.Run("fail insert rules with the same title in a folder", func(t *testing.T) {
cp := models.CopyRule(&rules[0])
cp.UID = cp.UID + "-new"
_, err = store.InsertAlertRules(context.Background(), []models.AlertRule{*cp})
_, err = store.InsertAlertRules(context.Background(), &usr, []models.AlertRule{*cp})
require.ErrorIs(t, err, models.ErrAlertRuleConflictBase)
require.ErrorIs(t, err, models.ErrAlertRuleUniqueConstraintViolation)
require.NotEqual(t, rules[0].UID, "")
@ -825,7 +891,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) {
t.Run("should not let insert rules with the same UID", func(t *testing.T) {
cp := models.CopyRule(&rules[0])
cp.Title = "unique-test-title"
_, err = store.InsertAlertRules(context.Background(), []models.AlertRule{*cp})
_, err = store.InsertAlertRules(context.Background(), &usr, []models.AlertRule{*cp})
require.ErrorIs(t, err, models.ErrAlertRuleConflictBase)
require.ErrorContains(t, err, "rule UID under the same organisation should be unique")
})
@ -833,6 +899,9 @@ func TestIntegrationInsertAlertRules(t *testing.T) {
t.Run("should emit event when rules are inserted", func(t *testing.T) {
rule := gen.Generate()
called := false
t.Cleanup(func() {
b.publishFn = nil
})
b.publishFn = func(ctx context.Context, msg bus.Msg) error {
event, ok := msg.(*RuleChangeEvent)
require.True(t, ok)
@ -843,11 +912,23 @@ func TestIntegrationInsertAlertRules(t *testing.T) {
return nil
}
rules, err := store.InsertAlertRules(context.Background(), []models.AlertRule{rule})
rules, err := store.InsertAlertRules(context.Background(), &usr, []models.AlertRule{rule})
require.NoError(t, err)
require.Len(t, rules, 1)
require.True(t, called)
})
t.Run("nil identity should be handled correctly", func(t *testing.T) {
rule := gen.Generate()
ids, err = store.InsertAlertRules(context.Background(), nil, []models.AlertRule{rule})
require.NoError(t, err)
insertedRule, err := store.GetRuleByID(context.Background(), models.GetAlertRuleByIDQuery{
ID: ids[0].ID,
OrgID: rule.OrgID,
})
require.NoError(t, err)
require.Nil(t, insertedRule.UpdatedBy)
})
}
func TestIntegrationAlertRulesNotificationSettings(t *testing.T) {
@ -855,6 +936,8 @@ func TestIntegrationAlertRulesNotificationSettings(t *testing.T) {
t.Skip("skipping integration test")
}
usr := models.UserUID("test")
getKeyMap := func(r []*models.AlertRule) map[models.AlertRuleKey]struct{} {
result := make(map[models.AlertRuleKey]struct{}, len(r))
for _, rule := range r {
@ -894,7 +977,7 @@ func TestIntegrationAlertRulesNotificationSettings(t *testing.T) {
require.NoError(t, store.SetProvenance(context.Background(), rule, rule.OrgID, p))
}
_, err := store.InsertAlertRules(context.Background(), deref)
_, err := store.InsertAlertRules(context.Background(), &usr, deref)
require.NoError(t, err)
t.Run("should find rules by receiver name", func(t *testing.T) {
@ -1136,7 +1219,7 @@ func TestIntegrationListNotificationSettings(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test")
}
usr := models.UserUID("test")
sqlStore := db.InitTestDB(t)
folderService := setupFolderService(t, sqlStore, setting.NewCfg(), featuremgmt.WithFeatures())
logger := log.New("test-dbstore")
@ -1168,7 +1251,7 @@ func TestIntegrationListNotificationSettings(t *testing.T) {
orgRules := append(rulesWithNotificationsAndReceiver, rulesWithNotificationsAndTimeInterval...)
_, err := store.InsertAlertRules(context.Background(), deref)
_, err := store.InsertAlertRules(context.Background(), &usr, deref)
require.NoError(t, err)
result, err := store.ListNotificationSettings(context.Background(), models.ListNotificationSettingsQuery{OrgID: 1})
@ -1258,6 +1341,8 @@ func TestIntegrationGetNamespacesByRuleUID(t *testing.T) {
t.Skip("skipping integration test")
}
usr := models.UserUID("test")
sqlStore := db.InitTestDB(t)
cfg := setting.NewCfg()
cfg.UnifiedAlerting.BaseInterval = 1 * time.Second
@ -1267,7 +1352,7 @@ func TestIntegrationGetNamespacesByRuleUID(t *testing.T) {
store := createTestStore(sqlStore, folderService, logger, cfg.UnifiedAlerting, b)
rules := models.RuleGen.With(models.RuleMuts.WithOrgID(1), models.RuleMuts.WithRandomRecordingRules()).GenerateMany(5)
_, err := store.InsertAlertRules(context.Background(), rules)
_, err := store.InsertAlertRules(context.Background(), &usr, rules)
require.NoError(t, err)
uids := make([]string, 0, len(rules))
@ -1306,6 +1391,7 @@ func TestIntegrationRuleGroupsCaseSensitive(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test")
}
usr := models.UserUID("test")
sqlStore := db.InitTestDB(t)
cfg := setting.NewCfg()
@ -1329,7 +1415,7 @@ func TestIntegrationRuleGroupsCaseSensitive(t *testing.T) {
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...))
_, err := store.InsertAlertRules(context.Background(), &usr, append(append(append(misc, group1...), group2...), group3...))
require.NoError(t, err)
t.Run("GetAlertRulesGroupByRuleUID", func(t *testing.T) {
@ -1524,6 +1610,7 @@ func TestIntegration_AlertRuleVersionsCleanup(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test")
}
usr := models.UserUID("test")
cfg := setting.NewCfg()
cfg.UnifiedAlerting = setting.UnifiedAlertingSettings{
BaseInterval: time.Duration(rand.Int63n(100)+1) * time.Second,
@ -1544,7 +1631,7 @@ func TestIntegration_AlertRuleVersionsCleanup(t *testing.T) {
rule := createRule(t, store, generator)
firstNewRule := models.CopyRule(rule)
firstNewRule.Title = util.GenerateShortUID()
err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
err := store.UpdateAlertRules(context.Background(), &usr, []models.UpdateRule{{
Existing: rule,
New: *firstNewRule,
},
@ -1553,7 +1640,7 @@ func TestIntegration_AlertRuleVersionsCleanup(t *testing.T) {
firstNewRule.Version = firstNewRule.Version + 1
secondNewRule := models.CopyRule(firstNewRule)
secondNewRule.Title = util.GenerateShortUID()
err = store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
err = store.UpdateAlertRules(context.Background(), &usr, []models.UpdateRule{{
Existing: firstNewRule,
New: *secondNewRule,
},
@ -1592,7 +1679,7 @@ func TestIntegration_AlertRuleVersionsCleanup(t *testing.T) {
rule := createRule(t, store, generator)
oldRule := models.CopyRule(rule)
oldRule.Title = "old-record"
err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
err := store.UpdateAlertRules(context.Background(), &usr, []models.UpdateRule{{
Existing: rule,
New: *oldRule,
}}) // first entry in `rule_version_history` table happens here
@ -1601,19 +1688,19 @@ func TestIntegration_AlertRuleVersionsCleanup(t *testing.T) {
rule.Version = rule.Version + 1
middleRule := models.CopyRule(rule)
middleRule.Title = "middle-record"
err = store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
err = store.UpdateAlertRules(context.Background(), &usr, []models.UpdateRule{{
Existing: rule,
New: *middleRule,
}}) //second entry in `rule_version_history` table happens here
}}) // second entry in `rule_version_history` table happens here
require.NoError(t, err)
rule.Version = rule.Version + 1
newerRule := models.CopyRule(rule)
newerRule.Title = "newer-record"
err = store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
err = store.UpdateAlertRules(context.Background(), &usr, []models.UpdateRule{{
Existing: rule,
New: *newerRule,
}}) //second entry in `rule_version_history` table happens here
}}) // second entry in `rule_version_history` table happens here
require.NoError(t, err)
// only the `old-record` should be deleted since limit is set to 1 and there are total 2 records

10
pkg/services/ngalert/store/compat.go
View File

@ -5,6 +5,7 @@ import (
"fmt"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/services/ngalert/models"
)
@ -35,6 +36,10 @@ func alertRuleToModelsAlertRule(ar alertRule, l log.Logger) (models.AlertRule, e
IsPaused: ar.IsPaused,
}
if ar.UpdatedBy != nil {
result.UpdatedBy = util.Pointer(models.UserUID(*ar.UpdatedBy))
}
if ar.NoDataState != "" {
result.NoDataState, err = models.NoDataStateFromString(ar.NoDataState)
if err != nil {
@ -120,6 +125,10 @@ func alertRuleFromModelsAlertRule(ar models.AlertRule) (alertRule, error) {
IsPaused: ar.IsPaused,
}
if ar.UpdatedBy != nil {
result.UpdatedBy = util.Pointer(string(*ar.UpdatedBy))
}
// Serialize complex types to JSON strings
data, err := json.Marshal(ar.Data)
if err != nil {
@ -179,6 +188,7 @@ func alertRuleToAlertRuleVersion(rule alertRule) alertRuleVersion {
RestoredFrom: 0,
Version: rule.Version,
Created: rule.Updated, // assuming the Updated time as the creation time
CreatedBy: rule.UpdatedBy,
Title: rule.Title,
Condition: rule.Condition,
Data: rule.Data,

2
pkg/services/ngalert/store/deltas.go
View File

@ -9,7 +9,7 @@ import (
)
// AlertRuleFieldsToIgnoreInDiff contains fields that are ignored when calculating the RuleDelta.Diff.
var AlertRuleFieldsToIgnoreInDiff = [...]string{"ID", "Version", "Updated"}
var AlertRuleFieldsToIgnoreInDiff = [...]string{"ID", "Version", "Updated", "UpdatedBy"}
type RuleDelta struct {
Existing *models.AlertRule

2
pkg/services/ngalert/store/models.go
View File

@ -10,6 +10,7 @@ type alertRule struct {
Condition string
Data string
Updated time.Time
UpdatedBy *string `xorm:"updated_by"`
IntervalSeconds int64
Version int64 `xorm:"version"` // this tag makes xorm add optimistic lock (see https://xorm.io/docs/chapter-06/1.lock/)
UID string `xorm:"uid"`
@ -46,6 +47,7 @@ type alertRuleVersion struct {
Version int64
Created time.Time
CreatedBy *string `xorm:"created_by"`
Title string
Condition string
Data string

4
pkg/services/ngalert/tests/fakes/rules.go
View File

@ -258,7 +258,7 @@ func (f *RuleStore) GetNamespaceByUID(_ context.Context, uid string, orgID int64
return nil, fmt.Errorf("not found")
}
func (f *RuleStore) UpdateAlertRules(_ context.Context, q []models.UpdateRule) error {
func (f *RuleStore) UpdateAlertRules(_ context.Context, _ *models.UserUID, q []models.UpdateRule) error {
f.mtx.Lock()
defer f.mtx.Unlock()
f.RecordedOps = append(f.RecordedOps, q)
@ -268,7 +268,7 @@ func (f *RuleStore) UpdateAlertRules(_ context.Context, q []models.UpdateRule) e
return nil
}
func (f *RuleStore) InsertAlertRules(_ context.Context, q []models.AlertRule) ([]models.AlertRuleKeyWithId, error) {
func (f *RuleStore) InsertAlertRules(_ context.Context, _ *models.UserUID, q []models.AlertRule) ([]models.AlertRuleKeyWithId, error) {
f.mtx.Lock()
defer f.mtx.Unlock()
f.RecordedOps = append(f.RecordedOps, q)

2
pkg/services/ngalert/tests/util.go
View File

@ -113,7 +113,7 @@ func CreateTestAlertRuleWithLabels(t testing.TB, ctx context.Context, dbstore *s
}
require.NoError(t, err)
_, err = dbstore.InsertAlertRules(ctx, []models.AlertRule{
_, err = dbstore.InsertAlertRules(ctx, models.NewUserUID(user), []models.AlertRule{
{
ID: 0,

2
pkg/services/sqlstore/migrations/migrations.go
View File

@ -141,4 +141,6 @@ func (oss *OSSMigrations) AddMigration(mg *Migrator) {
externalsession.AddMigration(mg)
accesscontrol.AddReceiverCreateScopeMigration(mg)
ualert.AddAlertRuleUpdatedByMigration(mg)
}

20
pkg/services/sqlstore/migrations/ualert/rule_creator_mig.go Normal file
View File

@ -0,0 +1,20 @@
package ualert
import "github.com/grafana/grafana/pkg/services/sqlstore/migrator"
// AddRuleNotificationSettingsColumns creates a column for notification settings in the alert_rule and alert_rule_version tables.
func AddAlertRuleUpdatedByMigration(mg *migrator.Migrator) {
mg.AddMigration("add created_by column to alert_rule_version table", migrator.NewAddColumnMigration(migrator.Table{Name: "alert_rule_version"}, &migrator.Column{
Name: "created_by",
Type: migrator.DB_Varchar,
Length: 40,
Nullable: true,
}))
mg.AddMigration("add updated_by column to alert_rule table", migrator.NewAddColumnMigration(migrator.Table{Name: "alert_rule"}, &migrator.Column{
Name: "updated_by",
Type: migrator.DB_Varchar,
Length: 40,
Nullable: true,
}))
}

2
pkg/storage/unified/federated/federatedtests/stats_test.go
View File

@ -69,7 +69,7 @@ func TestDirectSQLStats(t *testing.T) {
// create an alert rule inside of folder test2
ruleStore := ngalertstore.SetupStoreForTesting(t, db)
_, err = ruleStore.InsertAlertRules(context.Background(), []ngmodels.AlertRule{
_, err = ruleStore.InsertAlertRules(context.Background(), ngmodels.NewUserUID(tempUser), []ngmodels.AlertRule{
{
DashboardUID: &folder2UID,
UID: "test",