diff --git a/pkg/services/ngalert/api/api_ruler.go b/pkg/services/ngalert/api/api_ruler.go index 8e368ac1e8e..268ed7b8c5d 100644 --- a/pkg/services/ngalert/api/api_ruler.go +++ b/pkg/services/ngalert/api/api_ruler.go @@ -385,14 +385,24 @@ func (srv RulerSrv) RouteGetRuleVersionsByUID(c *contextmodel.ReqContext, ruleUI } sort.Slice(rules, func(i, j int) bool { return rules[i].ID > rules[j].ID }) result := make(apimodels.GettableRuleVersions, 0, len(rules)) - userUIDmapping := srv.getUserUIDmapping(ctx, rules) + userUIDmapping := srv.getUserUIDmapping(ctx, alertRuleVersionsToAlertRules(rules)) for _, rule := range rules { // do not provide provenance status because we do not have historical changes for it - result = append(result, toGettableExtendedRuleNode(*rule, map[string]ngmodels.Provenance{}, userUIDmapping)) + ruleNode := toGettableExtendedRuleNode(rule.AlertRule, map[string]ngmodels.Provenance{}, userUIDmapping) + ruleNode.GrafanaManagedAlert.Message = rule.Message + result = append(result, ruleNode) } return response.JSON(http.StatusOK, result) } +func alertRuleVersionsToAlertRules(vs []*ngmodels.AlertRuleVersion) []*ngmodels.AlertRule { + result := make([]*ngmodels.AlertRule, len(vs)) + for i := range vs { + result[i] = &vs[i].AlertRule + } + return result +} + func (srv RulerSrv) RoutePostNameRulesConfig(c *contextmodel.ReqContext, ruleGroupConfig apimodels.PostableRuleGroupConfig, namespaceUID string) response.Response { var deletePermanently bool if c.QueryBool("deletePermanently") { diff --git a/pkg/services/ngalert/api/api_ruler_test.go b/pkg/services/ngalert/api/api_ruler_test.go index cb6a93b27b5..c6ddeee2a6c 100644 --- a/pkg/services/ngalert/api/api_ruler_test.go +++ b/pkg/services/ngalert/api/api_ruler_test.go @@ -560,11 +560,17 @@ func TestRouteGetRuleVersionsByUID(t *testing.T) { ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], f) rule := gen.GenerateRef() - history := gen.With(gen.WithUID(rule.UID)).GenerateManyRef(3) + historyRules := gen.With(gen.WithUID(rule.UID)).GenerateManyRef(3) + history := make([]*models.AlertRuleVersion, len(historyRules)) // simulate order of the history rule.ID = 100 - for i, alertRule := range history { + for i, alertRule := range historyRules { alertRule.ID = rule.ID - int64(i) - 1 + + history[i] = &models.AlertRuleVersion{ + AlertRule: *alertRule, + Message: fmt.Sprintf("revision %d", i), + } } ruleStore.PutRule(context.Background(), rule) @@ -584,11 +590,22 @@ func TestRouteGetRuleVersionsByUID(t *testing.T) { require.Len(t, result, len(history)+1) // history + current version t.Run("should be in correct order", func(t *testing.T) { - expectedHistory := append([]*models.AlertRule{rule}, history...) + expectedHistory := append([]*models.AlertRuleVersion{{AlertRule: *rule}}, history...) for i, rul := range expectedHistory { assert.Equal(t, rul.UID, result[i].GrafanaManagedAlert.UID) } }) + + t.Run("should have correct messages", func(t *testing.T) { + expectedMessages := make([]string, 0, len(history)+1) + expectedMessages = append(expectedMessages, "") + for i := range history { + expectedMessages = append(expectedMessages, history[i].Message) + } + for i := range expectedMessages { + assert.Equal(t, expectedMessages[i], result[i].GrafanaManagedAlert.Message) + } + }) }) t.Run("NotFound when rule does not exist", func(t *testing.T) { @@ -599,10 +616,17 @@ func TestRouteGetRuleVersionsByUID(t *testing.T) { UID: "test", } guid := uuid.NewString() - history := gen.With(gen.WithGUID(guid), gen.WithKey(ruleKey)).GenerateManyRef(3) + historyRules := gen.With(gen.WithGUID(guid), gen.WithKey(ruleKey)).GenerateManyRef(3) + history := make([]*models.AlertRuleVersion, len(historyRules)) + for i, alertRule := range historyRules { + history[i] = &models.AlertRuleVersion{ + AlertRule: *alertRule, + Message: fmt.Sprintf("revision %d", i), + } + } ruleStore.History[guid] = append(ruleStore.History[guid], history...) // even if history is full of records - perms := createPermissionsForRules(history, orgID) + perms := createPermissionsForRules(historyRules, orgID) req := createRequestContextWithPerms(orgID, perms, nil) response := createService(ruleStore, nil).RouteGetRuleVersionsByUID(req, ruleKey.UID) @@ -643,10 +667,17 @@ func TestRouteGetRuleVersionsByUID(t *testing.T) { guid := uuid.NewString() rule := gen.With(gen.WithGUID(guid), gen.WithKey(ruleKey), gen.WithNamespaceUID(anotherFolder.UID)).GenerateRef() ruleStore.PutRule(context.Background(), rule) - history := gen.With(gen.WithGUID(guid), gen.WithKey(ruleKey)).GenerateManyRef(3) + historyRules := gen.With(gen.WithGUID(guid), gen.WithKey(ruleKey)).GenerateManyRef(3) + history := make([]*models.AlertRuleVersion, len(historyRules)) + for i, alertRule := range historyRules { + history[i] = &models.AlertRuleVersion{ + AlertRule: *alertRule, + Message: fmt.Sprintf("revision %d", i), + } + } ruleStore.History[guid] = history - perms := createPermissionsForRules(history, orgID) // grant permissions to all records in history but not the rule itself + perms := createPermissionsForRules(historyRules, orgID) // grant permissions to all records in history but not the rule itself req := createRequestContextWithPerms(orgID, perms, nil) response := createService(ruleStore, nil).RouteGetRuleVersionsByUID(req, ruleKey.UID) diff --git a/pkg/services/ngalert/api/persist.go b/pkg/services/ngalert/api/persist.go index 52c4ca59a14..9e7df81f48e 100644 --- a/pkg/services/ngalert/api/persist.go +++ b/pkg/services/ngalert/api/persist.go @@ -35,6 +35,6 @@ type RuleStore interface { // IncreaseVersionForAllRulesInNamespaces Increases version for all rules that have specified namespace uids IncreaseVersionForAllRulesInNamespaces(ctx context.Context, orgID int64, namespaceUIDs []string) ([]ngmodels.AlertRuleKeyWithVersion, error) - GetAlertRuleVersions(ctx context.Context, orgID int64, guid string) ([]*ngmodels.AlertRule, error) + GetAlertRuleVersions(ctx context.Context, orgID int64, guid string) ([]*ngmodels.AlertRuleVersion, error) accesscontrol.RuleUIDToNamespaceStore } diff --git a/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go b/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go index 21e6b438ac2..15cdb16d185 100644 --- a/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go +++ b/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go @@ -617,6 +617,9 @@ type GettableGrafanaRule struct { Metadata *AlertRuleMetadata `json:"metadata,omitempty" yaml:"metadata,omitempty"` GUID string `json:"guid" yaml:"guid"` MissingSeriesEvalsToResolve *int64 `json:"missing_series_evals_to_resolve,omitempty" yaml:"missing_series_evals_to_resolve,omitempty"` + + // Field is only populated when listing alert rule versions. + Message string `yaml:"message,omitempty" json:"message,omitempty"` } // UserInfo represents user-related information, including a unique identifier and a name. diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index 9f899bc2af7..a4e1e8fcc04 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -369,6 +369,13 @@ type AlertRule struct { MissingSeriesEvalsToResolve *int64 } +type AlertRuleVersion struct { + AlertRule + + // Message is only stored in the alert_rule_version table. + Message string +} + type AlertRuleMetadata struct { EditorSettings EditorSettings `json:"editor_settings"` PrometheusStyleRule *PrometheusStyleRule `json:"prometheus_style_rule,omitempty"` diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index 9044bb369c5..b05ba01a562 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -192,8 +192,8 @@ func (st DBstore) GetAlertRuleByUID(ctx context.Context, query *ngmodels.GetAler return result, err } -func (st DBstore) GetAlertRuleVersions(ctx context.Context, orgID int64, guid string) ([]*ngmodels.AlertRule, error) { - alertRules := make([]*ngmodels.AlertRule, 0) +func (st DBstore) GetAlertRuleVersions(ctx context.Context, orgID int64, guid string) ([]*ngmodels.AlertRuleVersion, error) { + alertRules := make([]*ngmodels.AlertRuleVersion, 0) err := st.SQLStore.WithDbSession(ctx, func(sess *db.Session) error { rows, err := sess.Table(new(alertRuleVersion)).Where("rule_org_id = ? AND rule_guid = ?", orgID, guid).Asc("id").Rows(new(alertRuleVersion)) if err != nil { @@ -213,7 +213,7 @@ func (st DBstore) GetAlertRuleVersions(ctx context.Context, orgID int64, guid st if previousVersion != nil && previousVersion.EqualSpec(*rule) { continue } - converted, err := alertRuleToModelsAlertRule(alertRuleVersionToAlertRule(*rule), st.Logger) + converted, err := alertRuleVersionToModelsAlertRuleVersion(*rule, st.Logger) if err != nil { st.Logger.Error("Invalid rule found in DB store, cannot convert, ignoring it", "func", "GetAlertRuleVersions", "error", err, "version_id", rule.ID) continue @@ -226,7 +226,7 @@ func (st DBstore) GetAlertRuleVersions(ctx context.Context, orgID int64, guid st if err != nil { return nil, err } - slices.SortFunc(alertRules, func(a, b *ngmodels.AlertRule) int { + slices.SortFunc(alertRules, func(a, b *ngmodels.AlertRuleVersion) int { if a.ID > b.ID { return -1 } @@ -257,6 +257,7 @@ func (st DBstore) ListDeletedRules(ctx context.Context, orgID int64) ([]*ngmodel st.Logger.Error("Invalid rule version found in DB store, ignoring it", "func", "GetAlertRuleVersions", "error", err) continue } + // Note: Message is not returned as a message cannot be set when deleting rules. converted, err := alertRuleToModelsAlertRule(alertRuleVersionToAlertRule(*rule), st.Logger) if err != nil { st.Logger.Error("Invalid rule found in DB store, cannot convert, ignoring it", "func", "GetAlertRuleVersions", "error", err, "version_id", rule.ID) diff --git a/pkg/services/ngalert/store/alert_rule_test.go b/pkg/services/ngalert/store/alert_rule_test.go index 0c49528f8d6..9a491120c8c 100644 --- a/pkg/services/ngalert/store/alert_rule_test.go +++ b/pkg/services/ngalert/store/alert_rule_test.go @@ -1682,7 +1682,7 @@ func TestIntegrationGetRuleVersions(t *testing.T) { require.NoError(t, err) assert.Len(t, versions, 2) assert.IsDecreasing(t, versions[0].ID, versions[1].ID) - diff := versions[1].Diff(versions[0], AlertRuleFieldsToIgnoreInDiff[:]...) + diff := versions[1].Diff(&versions[0].AlertRule, AlertRuleFieldsToIgnoreInDiff[:]...) assert.ElementsMatch(t, []string{"Title", "RuleGroupIndex"}, diff.Paths()) }) @@ -1712,7 +1712,7 @@ func TestIntegrationGetRuleVersions(t *testing.T) { versions, err := store.GetAlertRuleVersions(context.Background(), ruleV3.OrgID, ruleV3.GUID) require.NoError(t, err) assert.Len(t, versions, 3) - diff := versions[0].Diff(versions[1], AlertRuleFieldsToIgnoreInDiff[:]...) + diff := versions[0].Diff(&versions[1].AlertRule, AlertRuleFieldsToIgnoreInDiff[:]...) assert.ElementsMatch(t, []string{"RuleGroup", "NamespaceUID"}, diff.Paths()) }) } diff --git a/pkg/services/ngalert/store/compat.go b/pkg/services/ngalert/store/compat.go index 709450d5ed8..4f4194facc1 100644 --- a/pkg/services/ngalert/store/compat.go +++ b/pkg/services/ngalert/store/compat.go @@ -210,6 +210,7 @@ func alertRuleToAlertRuleVersion(rule alertRule) alertRuleVersion { Version: rule.Version, Created: rule.Updated, // assuming the Updated time as the creation time CreatedBy: rule.UpdatedBy, + Message: "", // Message is set by caller when creating versions Title: rule.Title, Condition: rule.Condition, Data: rule.Data, @@ -261,3 +262,15 @@ func alertRuleVersionToAlertRule(version alertRuleVersion) alertRule { MissingSeriesEvalsToResolve: version.MissingSeriesEvalsToResolve, } } + +func alertRuleVersionToModelsAlertRuleVersion(version alertRuleVersion, l log.Logger) (models.AlertRuleVersion, error) { + result, err := alertRuleToModelsAlertRule(alertRuleVersionToAlertRule(version), l) + if err != nil { + return models.AlertRuleVersion{}, err + } + + return models.AlertRuleVersion{ + AlertRule: result, + Message: version.Message, + }, nil +} diff --git a/pkg/services/ngalert/store/models.go b/pkg/services/ngalert/store/models.go index 254ec8cb048..24167a225dd 100644 --- a/pkg/services/ngalert/store/models.go +++ b/pkg/services/ngalert/store/models.go @@ -69,10 +69,11 @@ type alertRuleVersion struct { NotificationSettings string `xorm:"notification_settings"` Metadata string `xorm:"metadata"` MissingSeriesEvalsToResolve *int64 `xorm:"missing_series_evals_to_resolve"` + Message string } // EqualSpec compares two alertRuleVersion objects for equality based on their specifications and returns true if they match. -// The comparison is very basic and can produce false-negative. Fields excluded: ID, ParentVersion, RestoredFrom, Version, Created, RuleGroupIndex and CreatedBy +// The comparison is very basic and can produce false-negative. Fields excluded: ID, ParentVersion, RestoredFrom, Version, Created, RuleGroupIndex, CreatedBy and Message func (a alertRuleVersion) EqualSpec(b alertRuleVersion) bool { return a.RuleOrgID == b.RuleOrgID && a.RuleGUID == b.RuleGUID && diff --git a/pkg/services/ngalert/tests/fakes/rules.go b/pkg/services/ngalert/tests/fakes/rules.go index a0dd60e32e5..09abeaffba3 100644 --- a/pkg/services/ngalert/tests/fakes/rules.go +++ b/pkg/services/ngalert/tests/fakes/rules.go @@ -23,7 +23,7 @@ type RuleStore struct { mtx sync.Mutex // OrgID -> RuleGroup -> Namespace -> Rules Rules map[int64][]*models.AlertRule - History map[string][]*models.AlertRule + History map[string][]*models.AlertRuleVersion Deleted map[int64][]*models.AlertRule Hook func(cmd any) error // use Hook if you need to intercept some query and return an error RecordedOps []any @@ -43,7 +43,7 @@ func NewRuleStore(t *testing.T) *RuleStore { return nil }, Folders: map[int64][]*folder.Folder{}, - History: map[string][]*models.AlertRule{}, + History: map[string][]*models.AlertRuleVersion{}, } } @@ -55,7 +55,10 @@ mainloop: for _, r := range rules { rgs := f.Rules[r.OrgID] cp := models.CopyRule(r) - f.History[r.GUID] = append(f.History[r.GUID], cp) + f.History[r.GUID] = append(f.History[r.GUID], &models.AlertRuleVersion{ + AlertRule: *cp, + Message: "", + }) for idx, rulePtr := range rgs { if rulePtr.UID == r.UID { rgs[idx] = r @@ -87,6 +90,18 @@ mainloop: } } +// AppendHistory appends to rules to the version history with the given change message. +func (f *RuleStore) AppendHistory(guid string, rules []*models.AlertRule, message string) { + versions := make([]*models.AlertRuleVersion, len(rules)) + for i := range rules { + versions[i] = &models.AlertRuleVersion{ + AlertRule: *rules[i], + Message: message, + } + } + f.History[guid] = append(f.History[guid], versions...) +} + // GetRecordedCommands filters recorded commands using predicate function. Returns the subset of the recorded commands that meet the predicate func (f *RuleStore) GetRecordedCommands(predicate func(cmd any) (any, bool)) []any { f.mtx.Lock() @@ -563,7 +578,7 @@ func (f *RuleStore) GetNamespacesByRuleUID(ctx context.Context, orgID int64, uid return namespacesMap, nil } -func (f *RuleStore) GetAlertRuleVersions(_ context.Context, orgID int64, guid string) ([]*models.AlertRule, error) { +func (f *RuleStore) GetAlertRuleVersions(_ context.Context, orgID int64, guid string) ([]*models.AlertRuleVersion, error) { f.mtx.Lock() defer f.mtx.Unlock() diff --git a/pkg/services/sqlstore/migrations/ualert/tables.go b/pkg/services/sqlstore/migrations/ualert/tables.go index d6d98ebfec9..706b8dcf026 100644 --- a/pkg/services/sqlstore/migrations/ualert/tables.go +++ b/pkg/services/sqlstore/migrations/ualert/tables.go @@ -24,6 +24,9 @@ func AddTablesMigrations(mg *migrator.Migrator) { mg.AddMigration("add last_applied column to alert_configuration_history", migrator.NewAddColumnMigration(migrator.Table{Name: "alert_configuration_history"}, &migrator.Column{ Name: "last_applied", Type: migrator.DB_Int, Nullable: false, Default: "0", })) + mg.AddMigration("add message column to alert_rule_version", migrator.NewAddColumnMigration(migrator.Table{Name: "alert_rule_version"}, &migrator.Column{ + Name: "message", Type: migrator.DB_Text, Nullable: true, + })) // End of migration log, add new migrations above this line. }