Alerting: Get alert rule versions by GUID (#101469)

* get alert rule versions by GUID

* protect guid field from accidental update
This commit is contained in:
Yuri Tseretyan
2025-02-28 11:27:46 -05:00
committed by GitHub
parent 87a9188bb4
commit 1d54850a68
6 changed files with 27 additions and 23 deletions

View File

@ -339,7 +339,7 @@ func (srv RulerSrv) RouteGetRuleByUID(c *contextmodel.ReqContext, ruleUID string
func (srv RulerSrv) RouteGetRuleVersionsByUID(c *contextmodel.ReqContext, ruleUID string) response.Response {
ctx := c.Req.Context()
// make sure the user has access to the current version of the rule. Also, check if it exists
_, err := srv.getAuthorizedRuleByUid(ctx, c, ruleUID)
rule, err := srv.getAuthorizedRuleByUid(ctx, c, ruleUID)
if err != nil {
if errors.Is(err, ngmodels.ErrAlertRuleNotFound) {
return response.Empty(http.StatusNotFound)
@ -347,7 +347,7 @@ func (srv RulerSrv) RouteGetRuleVersionsByUID(c *contextmodel.ReqContext, ruleUI
return response.ErrOrFallback(http.StatusInternalServerError, "failed to get rule by UID", err)
}
rules, err := srv.store.GetAlertRuleVersions(ctx, ngmodels.AlertRuleKey{OrgID: c.OrgID, UID: ruleUID})
rules, err := srv.store.GetAlertRuleVersions(ctx, rule.OrgID, rule.GUID)
if err != nil {
return response.ErrOrFallback(http.StatusInternalServerError, "failed to get rule history", err)
}

View File

@ -13,6 +13,7 @@ import (
"testing"
"time"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
@ -486,7 +487,7 @@ func TestRouteGetRuleHistoryByUID(t *testing.T) {
}
ruleStore.PutRule(context.Background(), rule)
ruleStore.History[rule.GetKey()] = append(ruleStore.History[rule.GetKey()], history...)
ruleStore.History[rule.GUID] = append(ruleStore.History[rule.GUID], history...)
perms := createPermissionsForRules([]*models.AlertRule{rule}, orgID)
req := createRequestContextWithPerms(orgID, perms, nil)
@ -516,8 +517,9 @@ func TestRouteGetRuleHistoryByUID(t *testing.T) {
OrgID: orgID,
UID: "test",
}
history := gen.With(gen.WithKey(ruleKey)).GenerateManyRef(3)
ruleStore.History[ruleKey] = append(ruleStore.History[ruleKey], history...) // even if history is full of records
guid := uuid.NewString()
history := gen.With(gen.WithGUID(guid), gen.WithKey(ruleKey)).GenerateManyRef(3)
ruleStore.History[guid] = append(ruleStore.History[guid], history...) // even if history is full of records
perms := createPermissionsForRules(history, orgID)
req := createRequestContextWithPerms(orgID, perms, nil)
@ -533,9 +535,10 @@ func TestRouteGetRuleHistoryByUID(t *testing.T) {
OrgID: orgID,
UID: "test",
}
rule := gen.With(gen.WithKey(ruleKey)).GenerateRef()
guid := uuid.NewString()
rule := gen.With(gen.WithKey(ruleKey), gen.WithGUID(guid)).GenerateRef()
ruleStore.PutRule(context.Background(), rule)
ruleStore.History[ruleKey] = nil
ruleStore.History[guid] = nil
perms := createPermissionsForRules([]*models.AlertRule{rule}, orgID)
req := createRequestContextWithPerms(orgID, perms, nil)
@ -556,10 +559,11 @@ func TestRouteGetRuleHistoryByUID(t *testing.T) {
OrgID: orgID,
UID: "test",
}
rule := gen.With(gen.WithKey(ruleKey), gen.WithNamespaceUID(anotherFolder.UID)).GenerateRef()
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.WithKey(ruleKey)).GenerateManyRef(3)
ruleStore.History[ruleKey] = history
history := gen.With(gen.WithGUID(guid), gen.WithKey(ruleKey)).GenerateManyRef(3)
ruleStore.History[guid] = history
perms := createPermissionsForRules(history, orgID) // grant permissions to all records in history but not the rule itself
req := createRequestContextWithPerms(orgID, perms, nil)

View File

@ -30,6 +30,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, key ngmodels.AlertRuleKey) ([]*ngmodels.AlertRule, error)
GetAlertRuleVersions(ctx context.Context, orgID int64, guid string) ([]*ngmodels.AlertRule, error)
accesscontrol.RuleUIDToNamespaceStore
}

View File

@ -125,10 +125,10 @@ func (st DBstore) GetAlertRuleByUID(ctx context.Context, query *ngmodels.GetAler
return result, err
}
func (st DBstore) GetAlertRuleVersions(ctx context.Context, key ngmodels.AlertRuleKey) ([]*ngmodels.AlertRule, error) {
func (st DBstore) GetAlertRuleVersions(ctx context.Context, orgID int64, guid string) ([]*ngmodels.AlertRule, error) {
alertRules := make([]*ngmodels.AlertRule, 0)
err := st.SQLStore.WithDbSession(ctx, func(sess *db.Session) error {
rows, err := sess.Table(new(alertRuleVersion)).Where("rule_org_id = ? AND rule_uid = ?", key.OrgID, key.UID).Asc("id").Rows(new(alertRuleVersion))
rows, err := sess.Table(new(alertRuleVersion)).Where("rule_org_id = ? AND rule_guid = ?", orgID, guid).Asc("id").Rows(new(alertRuleVersion))
if err != nil {
return err
}
@ -330,7 +330,7 @@ func (st DBstore) UpdateAlertRules(ctx context.Context, user *ngmodels.UserUID,
return fmt.Errorf("failed to convert alert rule %s to storage model: %w", r.New.UID, err)
}
// no way to update multiple rules at once
if updated, err := sess.ID(r.Existing.ID).AllCols().Update(converted); err != nil || updated == 0 {
if updated, err := sess.ID(r.Existing.ID).AllCols().Omit("rule_guid").Update(converted); err != nil || updated == 0 {
if err != nil {
if st.SQLStore.GetDialect().IsUniqueConstraintViolation(err) {
return ruleConstraintViolationToErr(sess, r.New, err, st.Logger)

View File

@ -1599,7 +1599,7 @@ func TestGetRuleVersions(t *testing.T) {
require.NoError(t, err)
t.Run("should return rule versions sorted in decreasing order", func(t *testing.T) {
versions, err := store.GetAlertRuleVersions(context.Background(), ruleV2.GetKey())
versions, err := store.GetAlertRuleVersions(context.Background(), ruleV2.OrgID, ruleV2.GUID)
require.NoError(t, err)
assert.Len(t, versions, 2)
assert.IsDecreasing(t, versions[0].ID, versions[1].ID)
@ -1630,7 +1630,7 @@ func TestGetRuleVersions(t *testing.T) {
},
})
versions, err := store.GetAlertRuleVersions(context.Background(), ruleV3.GetKey())
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[:]...)

View File

@ -23,7 +23,7 @@ type RuleStore struct {
mtx sync.Mutex
// OrgID -> RuleGroup -> Namespace -> Rules
Rules map[int64][]*models.AlertRule
History map[models.AlertRuleKey][]*models.AlertRule
History map[string][]*models.AlertRule
Hook func(cmd any) error // use Hook if you need to intercept some query and return an error
RecordedOps []any
Folders map[int64][]*folder.Folder
@ -42,7 +42,7 @@ func NewRuleStore(t *testing.T) *RuleStore {
return nil
},
Folders: map[int64][]*folder.Folder{},
History: map[models.AlertRuleKey][]*models.AlertRule{},
History: map[string][]*models.AlertRule{},
}
}
@ -54,7 +54,7 @@ mainloop:
for _, r := range rules {
rgs := f.Rules[r.OrgID]
cp := models.CopyRule(r)
f.History[r.GetKey()] = append(f.History[r.GetKey()], cp)
f.History[r.GUID] = append(f.History[r.GUID], cp)
for idx, rulePtr := range rgs {
if rulePtr.UID == r.UID {
rgs[idx] = r
@ -426,21 +426,21 @@ func (f *RuleStore) GetNamespacesByRuleUID(ctx context.Context, orgID int64, uid
return namespacesMap, nil
}
func (f *RuleStore) GetAlertRuleVersions(_ context.Context, key models.AlertRuleKey) ([]*models.AlertRule, error) {
func (f *RuleStore) GetAlertRuleVersions(_ context.Context, orgID int64, guid string) ([]*models.AlertRule, error) {
f.mtx.Lock()
defer f.mtx.Unlock()
q := GenericRecordedQuery{
Name: "GetAlertRuleVersions",
Params: []any{key},
Params: []any{orgID, guid},
}
defer func() {
f.RecordedOps = append(f.RecordedOps, q)
}()
if err := f.Hook(key); err != nil {
if err := f.Hook([]any{orgID, guid}); err != nil {
return nil, err
}
return f.History[key], nil
return f.History[guid], nil
}