From 68f1730461427dd6d3d5b077a7963897091225cf Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Tue, 4 Feb 2025 14:23:15 -0500 Subject: [PATCH] Alerting: set updated_by for system owned operations (#100068) --- pkg/services/ngalert/api/api_ruler.go | 21 +++++++++++++++---- pkg/services/ngalert/api/api_ruler_test.go | 18 ++++++++++++++++ pkg/services/ngalert/models/alert_rule.go | 5 +++++ .../ngalert/provisioning/alert_rules.go | 18 +++++++++++----- pkg/services/ngalert/store/alert_rule.go | 4 ++-- 5 files changed, 55 insertions(+), 11 deletions(-) diff --git a/pkg/services/ngalert/api/api_ruler.go b/pkg/services/ngalert/api/api_ruler.go index 215048e448c..1f122c04ad1 100644 --- a/pkg/services/ngalert/api/api_ruler.go +++ b/pkg/services/ngalert/api/api_ruler.go @@ -757,23 +757,36 @@ type userIDToUserInfoFn func(id *ngmodels.UserUID) *apimodels.UserInfo // getIdentityName returns name of either user or service account func (srv RulerSrv) resolveUserIdToNameFn(ctx context.Context) userIDToUserInfoFn { + cache := map[ngmodels.UserUID]*apimodels.UserInfo{ + ngmodels.AlertingUserUID: { + UID: string(ngmodels.AlertingUserUID), + }, + ngmodels.FileProvisioningUserUID: { + UID: string(ngmodels.FileProvisioningUserUID), + }, + } return func(id *ngmodels.UserUID) *apimodels.UserInfo { if id == nil { return nil } + if val, ok := cache[*id]; ok { + return val + } u, err := srv.userService.GetByUID(ctx, &user.GetUserByUIDQuery{ UID: string(*id), }) - var result string + var name string if err != nil { srv.log.FromContext(ctx).Warn("Failed to get user by uid. Defaulting to an empty name", "uid", id, "error", err) } if u != nil { - result = u.NameOrFallback() + name = u.NameOrFallback() } - return &apimodels.UserInfo{ + result := &apimodels.UserInfo{ UID: string(*id), - Name: result, + Name: name, } + cache[*id] = result + return result } } diff --git a/pkg/services/ngalert/api/api_ruler_test.go b/pkg/services/ngalert/api/api_ruler_test.go index dce8f9a801c..73cf2bb6300 100644 --- a/pkg/services/ngalert/api/api_ruler_test.go +++ b/pkg/services/ngalert/api/api_ruler_test.go @@ -404,6 +404,24 @@ func TestRouteGetRuleByUID(t *testing.T) { Name: "Test", }, }, + { + desc: "recognize system identifier (alerting)", + UpdatedBy: &models.AlertingUserUID, + User: nil, + UserServiceError: nil, + Expected: &apimodels.UserInfo{ + UID: string(models.AlertingUserUID), + }, + }, + { + desc: "recognize system identifier (provisioning)", + UpdatedBy: &models.FileProvisioningUserUID, + User: nil, + UserServiceError: nil, + Expected: &apimodels.UserInfo{ + UID: string(models.FileProvisioningUserUID), + }, + }, } for _, tc := range testcases { t.Run(tc.desc, func(t *testing.T) { diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index 07a5eb86280..b7da813141f 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -47,6 +47,11 @@ var ( ErrNoPanel = errors.New("no panel") ) +var ( + FileProvisioningUserUID = UserUID("__provisioning__") + AlertingUserUID = UserUID("__alerting__") +) + // swagger:enum NoDataState type NoDataState string diff --git a/pkg/services/ngalert/provisioning/alert_rules.go b/pkg/services/ngalert/provisioning/alert_rules.go index 9b8cd429653..c31413b3bdc 100644 --- a/pkg/services/ngalert/provisioning/alert_rules.go +++ b/pkg/services/ngalert/provisioning/alert_rules.go @@ -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.NewUserUID(user), []models.AlertRule{ + ids, err := service.ruleStore.InsertAlertRules(ctx, userUidOrFallback(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, models.NewUserUID(user), updateRules) + return service.ruleStore.UpdateAlertRules(ctx, userUidOrFallback(user), updateRules) }) } @@ -511,7 +511,7 @@ func (service *AlertRuleService) persistDelta(ctx context.Context, user identity New: *update.New, }) } - if err := service.ruleStore.UpdateAlertRules(ctx, models.NewUserUID(user), updates); err != nil { + if err := service.ruleStore.UpdateAlertRules(ctx, userUidOrFallback(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, models.NewUserUID(user), withoutNilAlertRules(delta.New)) + uids, err := service.ruleStore.InsertAlertRules(ctx, userUidOrFallback(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.NewUserUID(user), []models.UpdateRule{ + err := service.ruleStore.UpdateAlertRules(ctx, userUidOrFallback(user), []models.UpdateRule{ { Existing: storedRule, New: rule, @@ -871,3 +871,11 @@ func (service *AlertRuleService) ensureNamespace(ctx context.Context, user ident return nil } + +func userUidOrFallback(user identity.Requester) *models.UserUID { + userUID := models.NewUserUID(user) + if user == nil { + return &models.FileProvisioningUserUID + } + return userUID +} diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index adcfe8450fa..fbc9e061385 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -988,7 +988,7 @@ func (st DBstore) RenameReceiverInNotificationSettings(ctx context.Context, orgI } // 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) + return result, nil, st.UpdateAlertRules(ctx, &ngmodels.AlertingUserUID, updates) } // RenameTimeIntervalInNotificationSettings renames all rules that use old time interval name to the new name. @@ -1065,7 +1065,7 @@ func (st DBstore) RenameTimeIntervalInNotificationSettings( } // 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) + return result, nil, st.UpdateAlertRules(ctx, &ngmodels.AlertingUserUID, updates) } func ruleConstraintViolationToErr(sess *db.Session, rule ngmodels.AlertRule, err error, logger log.Logger) error {