Alerting: Update GetRuleGroupAlertRules to accept optional rule group (#46889)

* rename GetRuleGroupAlertRules to GetAlertRules
* make rule group optional in GetAlertRulesQuery
* simplify FakeStore. the current structure did not support optional rule group

pkg/services/ngalert/api/api_ruler.go

@@ -139,12 +139,12 @@ func (srv RulerSrv) RouteGetRulegGroupConfig(c *models.ReqContext) response.Resp
 	}
 
 	ruleGroup := web.Params(c.Req)[":Groupname"]
-	q := ngmodels.ListRuleGroupAlertRulesQuery{
+	q := ngmodels.GetAlertRulesQuery{
 		OrgID:        c.SignedInUser.OrgId,
 		NamespaceUID: namespace.Uid,
-		RuleGroup:    ruleGroup,
+		RuleGroup:    &ruleGroup,
 	}
-	if err := srv.store.GetRuleGroupAlertRules(c.Req.Context(), &q); err != nil {
+	if err := srv.store.GetAlertRules(c.Req.Context(), &q); err != nil {
 		return ErrResp(http.StatusInternalServerError, err, "failed to get group alert rules")
 	}
 
@@ -419,12 +419,12 @@ func (c *changes) isEmpty() bool {
 // calculateChanges calculates the difference between rules in the group in the database and the submitted rules. If a submitted rule has UID it tries to find it in the database (in other groups).
 // returns a list of rules that need to be added, updated and deleted. Deleted considered rules in the database that belong to the group but do not exist in the list of submitted rules.
 func calculateChanges(ctx context.Context, ruleStore store.RuleStore, orgId int64, namespace *models.Folder, ruleGroupName string, submittedRules []*ngmodels.AlertRule) (*changes, error) {
-	q := &ngmodels.ListRuleGroupAlertRulesQuery{
+	q := &ngmodels.GetAlertRulesQuery{
 		OrgID:        orgId,
 		NamespaceUID: namespace.Uid,
-		RuleGroup:    ruleGroupName,
+		RuleGroup:    &ruleGroupName,
 	}
-	if err := ruleStore.GetRuleGroupAlertRules(ctx, q); err != nil {
+	if err := ruleStore.GetAlertRules(ctx, q); err != nil {
 		return nil, fmt.Errorf("failed to query database for rules in the group %s: %w", ruleGroupName, err)
 	}
 	existingGroupRules := q.Result

pkg/services/ngalert/api/api_ruler_test.go

@@ -221,7 +221,7 @@ func TestCalculateChanges(t *testing.T) {
 		expectedErr := errors.New("TEST ERROR")
 		fakeStore.Hook = func(cmd interface{}) error {
 			switch cmd.(type) {
-			case models.ListRuleGroupAlertRulesQuery:
+			case models.GetAlertRulesQuery:
 				return expectedErr
 			}
 			return nil

pkg/services/ngalert/models/alert_rule.go

@@ -250,12 +250,12 @@ type ListNamespaceAlertRulesQuery struct {
 	Result []*AlertRule
 }
 
-// ListRuleGroupAlertRulesQuery is the query for listing rule group alert rules
+// GetAlertRulesQuery is the query for listing rule group alert rules
-type ListRuleGroupAlertRulesQuery struct {
+type GetAlertRulesQuery struct {
 	OrgID int64
 	// Namespace is the folder slug
 	NamespaceUID string
-	RuleGroup    string
+	RuleGroup    *string
 
 	// DashboardUID and PanelID are optional and allow filtering rules
 	// to return just those for a dashboard and panel.

pkg/services/ngalert/schedule/schedule_unit_test.go

@@ -1117,12 +1117,12 @@ func CreateTestAlertRule(t *testing.T, dbstore *store.FakeRuleStore, intervalSec
 	})
 	require.NoError(t, err)
 
-	q := models.ListRuleGroupAlertRulesQuery{
+	q := models.GetAlertRulesQuery{
 		OrgID:        orgID,
 		NamespaceUID: "namespace",
-		RuleGroup:    ruleGroup,
+		RuleGroup:    &ruleGroup,
 	}
-	err = dbstore.GetRuleGroupAlertRules(ctx, &q)
+	err = dbstore.GetAlertRules(ctx, &q)
 	require.NoError(t, err)
 	require.NotEmpty(t, q.Result)
 

pkg/services/ngalert/store/alert_rule.go

@@ -43,7 +43,7 @@ type RuleStore interface {
 	GetAlertRulesForScheduling(ctx context.Context, query *ngmodels.ListAlertRulesQuery) error
 	GetOrgAlertRules(ctx context.Context, query *ngmodels.ListAlertRulesQuery) error
 	GetNamespaceAlertRules(ctx context.Context, query *ngmodels.ListNamespaceAlertRulesQuery) error
-	GetRuleGroupAlertRules(ctx context.Context, query *ngmodels.ListRuleGroupAlertRulesQuery) error
+	GetAlertRules(ctx context.Context, query *ngmodels.GetAlertRulesQuery) error
 	GetNamespaces(context.Context, int64, *models.SignedInUser) (map[string]*models.Folder, error)
 	GetNamespaceByTitle(context.Context, string, int64, *models.SignedInUser, bool) (*models.Folder, error)
 	GetOrgRuleGroups(ctx context.Context, query *ngmodels.ListOrgRuleGroupsQuery) error
@@ -315,23 +315,22 @@ func (st DBstore) GetNamespaceAlertRules(ctx context.Context, query *ngmodels.Li
 	})
 }
 
-// GetRuleGroupAlertRules is a handler for retrieving rule group alert rules of specific organisation.
+// GetAlertRules is a handler for retrieving rule group alert rules of specific organisation.
-func (st DBstore) GetRuleGroupAlertRules(ctx context.Context, query *ngmodels.ListRuleGroupAlertRulesQuery) error {
+func (st DBstore) GetAlertRules(ctx context.Context, query *ngmodels.GetAlertRulesQuery) error {
 	return st.SQLStore.WithDbSession(ctx, func(sess *sqlstore.DBSession) error {
-		q := "SELECT * FROM alert_rule WHERE org_id = ? and namespace_uid = ? and rule_group = ?"
+		q := sess.Table("alert_rule").Where("org_id = ? AND namespace_uid = ?", query.OrgID, query.NamespaceUID)
-		args := []interface{}{query.OrgID, query.NamespaceUID, query.RuleGroup}
+		if query.RuleGroup != nil {
-
+			q = q.Where("rule_group = ?", *query.RuleGroup)
+		}
 		if query.DashboardUID != "" {
-			q = fmt.Sprintf("%s and dashboard_uid = ?", q)
+			q = q.Where("dashboard_uid = ?", query.DashboardUID)
-			args = append(args, query.DashboardUID)
 			if query.PanelID != 0 {
-				q = fmt.Sprintf("%s and panel_id = ?", q)
+				q = q.Where("panel_id = ?", query.PanelID)
-				args = append(args, query.PanelID)
 			}
 		}
 
 		alertRules := make([]*ngmodels.AlertRule, 0)
-		if err := sess.SQL(q, args...).Find(&alertRules); err != nil {
+		if err := q.Find(&alertRules); err != nil {
 			return err
 		}
 

pkg/services/ngalert/store/testing.go

@@ -25,7 +25,7 @@ import (
 func NewFakeRuleStore(t *testing.T) *FakeRuleStore {
 	return &FakeRuleStore{
 		t:     t,
-		Rules: map[int64]map[string]map[string][]*models.AlertRule{},
+		Rules: map[int64][]*models.AlertRule{},
 		Hook: func(interface{}) error {
 			return nil
 		},
@@ -37,7 +37,7 @@ type FakeRuleStore struct {
 	t   *testing.T
 	mtx sync.Mutex
 	// OrgID -> RuleGroup -> Namespace -> Rules
-	Rules       map[int64]map[string]map[string][]*models.AlertRule
+	Rules       map[int64][]*models.AlertRule
 	Hook        func(cmd interface{}) error // use Hook if you need to intercept some query and return an error
 	RecordedOps []interface{}
 }
@@ -48,27 +48,15 @@ func (f *FakeRuleStore) PutRule(_ context.Context, rules ...*models.AlertRule) {
 	defer f.mtx.Unlock()
 mainloop:
 	for _, r := range rules {
-		rgs, ok := f.Rules[r.OrgID]
+		rgs := f.Rules[r.OrgID]
-		if !ok {
+		for idx, rulePtr := range rgs {
-			f.Rules[r.OrgID] = map[string]map[string][]*models.AlertRule{}
-		}
-
-		rg, ok := rgs[r.RuleGroup]
-		if !ok {
-			f.Rules[r.OrgID][r.RuleGroup] = map[string][]*models.AlertRule{}
-		}
-
-		_, ok = rg[r.NamespaceUID]
-		if !ok {
-			f.Rules[r.OrgID][r.RuleGroup][r.NamespaceUID] = []*models.AlertRule{}
-		}
-		for idx, rulePtr := range f.Rules[r.OrgID][r.RuleGroup][r.NamespaceUID] {
 			if rulePtr.UID == r.UID {
-				f.Rules[r.OrgID][r.RuleGroup][r.NamespaceUID][idx] = r
+				rgs[idx] = r
 				continue mainloop
 			}
 		}
-		f.Rules[r.OrgID][r.RuleGroup][r.NamespaceUID] = append(f.Rules[r.OrgID][r.RuleGroup][r.NamespaceUID], r)
+		rgs = append(rgs, r)
+		f.Rules[r.OrgID] = rgs
 	}
 }
 
@@ -105,22 +93,17 @@ func (f *FakeRuleStore) GetAlertRuleByUID(_ context.Context, q *models.GetAlertR
 	if err := f.Hook(*q); err != nil {
 		return err
 	}
-	rgs, ok := f.Rules[q.OrgID]
+	rules, ok := f.Rules[q.OrgID]
 	if !ok {
 		return nil
 	}
 
-	for _, rg := range rgs {
+	for _, rule := range rules {
-		for _, rules := range rg {
+		if rule.UID == q.UID {
-			for _, r := range rules {
+			q.Result = rule
-				if r.UID == q.UID {
-					q.Result = r
 			break
 		}
 	}
-		}
-	}
-
 	return nil
 }
 
@@ -132,14 +115,9 @@ func (f *FakeRuleStore) GetAlertRulesForScheduling(_ context.Context, q *models.
 	if err := f.Hook(*q); err != nil {
 		return err
 	}
-	for _, rg := range f.Rules {
+	for _, rules := range f.Rules {
-		for _, n := range rg {
+		q.Result = append(q.Result, rules...)
-			for _, r := range n {
-				q.Result = append(q.Result, r...)
 	}
-		}
-	}
-
 	return nil
 }
 
@@ -148,19 +126,11 @@ func (f *FakeRuleStore) GetOrgAlertRules(_ context.Context, q *models.ListAlertR
 	defer f.mtx.Unlock()
 	f.RecordedOps = append(f.RecordedOps, *q)
 
-	if _, ok := f.Rules[q.OrgID]; !ok {
+	rules, ok := f.Rules[q.OrgID]
+	if !ok {
 		return nil
 	}
-
-	var rules []*models.AlertRule
-	for ruleGroup := range f.Rules[q.OrgID] {
-		for _, storedRules := range f.Rules[q.OrgID][ruleGroup] {
-			rules = append(rules, storedRules...)
-		}
-	}
-
 	q.Result = rules
-
 	return nil
 }
 func (f *FakeRuleStore) GetNamespaceAlertRules(_ context.Context, q *models.ListNamespaceAlertRulesQuery) error {
@@ -169,36 +139,28 @@ func (f *FakeRuleStore) GetNamespaceAlertRules(_ context.Context, q *models.List
 	f.RecordedOps = append(f.RecordedOps, *q)
 	return nil
 }
-func (f *FakeRuleStore) GetRuleGroupAlertRules(_ context.Context, q *models.ListRuleGroupAlertRulesQuery) error {
+func (f *FakeRuleStore) GetAlertRules(_ context.Context, q *models.GetAlertRulesQuery) error {
 	f.mtx.Lock()
 	defer f.mtx.Unlock()
 	f.RecordedOps = append(f.RecordedOps, *q)
 	if err := f.Hook(*q); err != nil {
 		return err
 	}
-	rgs, ok := f.Rules[q.OrgID]
+	rules, ok := f.Rules[q.OrgID]
 	if !ok {
 		return nil
 	}
-
+	var result []*models.AlertRule
-	rg, ok := rgs[q.RuleGroup]
+	for _, rule := range rules {
-	if !ok {
+		if q.NamespaceUID != rule.NamespaceUID {
-		return nil
+			continue
 		}
-
+		if q.RuleGroup != nil && *q.RuleGroup != rule.RuleGroup {
-	if q.NamespaceUID != "" {
+			continue
-		r, ok := rg[q.NamespaceUID]
-		if !ok {
-			return nil
 		}
-		q.Result = r
+		result = append(result, rule)
-		return nil
 	}
-
+	q.Result = result
-	for _, r := range rg {
-		q.Result = append(q.Result, r...)
-	}
-
 	return nil
 }
 func (f *FakeRuleStore) GetNamespaces(_ context.Context, orgID int64, _ *models2.SignedInUser) (map[string]*models2.Folder, error) {
@@ -212,12 +174,9 @@ func (f *FakeRuleStore) GetNamespaces(_ context.Context, orgID int64, _ *models2
 		return namespacesMap, nil
 	}
 
-	for rg := range f.Rules[orgID] {
+	for _, rule := range f.Rules[orgID] {
-		for namespace := range f.Rules[orgID][rg] {
+		namespacesMap[rule.NamespaceUID] = &models2.Folder{}
-			namespacesMap[namespace] = &models2.Folder{}
 	}
-	}
-
 	return namespacesMap, nil
 }
 func (f *FakeRuleStore) GetNamespaceByTitle(_ context.Context, _ string, _ int64, _ *models2.SignedInUser, _ bool) (*models2.Folder, error) {
@@ -233,18 +192,16 @@ func (f *FakeRuleStore) GetOrgRuleGroups(_ context.Context, q *models.ListOrgRul
 
 	// If we have namespaces, we want to try and retrieve the list of rules stored.
 	if len(q.NamespaceUIDs) != 0 {
-		_, ok := f.Rules[q.OrgID]
+		rules, ok := f.Rules[q.OrgID]
 		if !ok {
 			return nil
 		}
 
 		var ruleGroups [][]string
-		for rg := range f.Rules[q.OrgID] {
+		for _, rule := range rules {
-			for storedNamespace := range f.Rules[q.OrgID][rg] {
 			for _, namespace := range q.NamespaceUIDs {
-					if storedNamespace == namespace { // if they match, they should go in.
+				if rule.NamespaceUID == namespace { // if they match, they should go in.
-						ruleGroups = append(ruleGroups, []string{rg, storedNamespace, storedNamespace})
+					ruleGroups = append(ruleGroups, []string{rule.RuleGroup, rule.NamespaceUID, rule.NamespaceUID})
-					}
 				}
 			}
 		}
@@ -263,6 +220,7 @@ func (f *FakeRuleStore) UpsertAlertRules(_ context.Context, q []UpsertRule) erro
 	}
 	return nil
 }
+
 func (f *FakeRuleStore) UpdateRuleGroup(_ context.Context, cmd UpdateRuleGroupCmd) error {
 	f.mtx.Lock()
 	defer f.mtx.Unlock()
@@ -270,29 +228,15 @@ func (f *FakeRuleStore) UpdateRuleGroup(_ context.Context, cmd UpdateRuleGroupCm
 	if err := f.Hook(cmd); err != nil {
 		return err
 	}
-	rgs, ok := f.Rules[cmd.OrgID]
+	existingRules := f.Rules[cmd.OrgID]
-	if !ok {
-		f.Rules[cmd.OrgID] = map[string]map[string][]*models.AlertRule{}
-	}
 
-	rg, ok := rgs[cmd.RuleGroupConfig.Name]
-	if !ok {
-		f.Rules[cmd.OrgID][cmd.RuleGroupConfig.Name] = map[string][]*models.AlertRule{}
-	}
-
-	_, ok = rg[cmd.NamespaceUID]
-	if !ok {
-		f.Rules[cmd.OrgID][cmd.RuleGroupConfig.Name][cmd.NamespaceUID] = []*models.AlertRule{}
-	}
-
-	rules := []*models.AlertRule{}
 	for _, r := range cmd.RuleGroupConfig.Rules {
 		// TODO: Not sure why this is not being set properly, where is the code that sets this?
 		for i := range r.GrafanaManagedAlert.Data {
 			r.GrafanaManagedAlert.Data[i].DatasourceUID = "-100"
 		}
 
-		new := &models.AlertRule{
+		newRule := &models.AlertRule{
 			OrgID:           cmd.OrgID,
 			Title:           r.GrafanaManagedAlert.Title,
 			Condition:       r.GrafanaManagedAlert.Condition,
@@ -307,26 +251,26 @@ func (f *FakeRuleStore) UpdateRuleGroup(_ context.Context, cmd UpdateRuleGroupCm
 		}
 
 		if r.ApiRuleNode != nil {
-			new.For = time.Duration(r.ApiRuleNode.For)
+			newRule.For = time.Duration(r.ApiRuleNode.For)
-			new.Annotations = r.ApiRuleNode.Annotations
+			newRule.Annotations = r.ApiRuleNode.Annotations
-			new.Labels = r.ApiRuleNode.Labels
+			newRule.Labels = r.ApiRuleNode.Labels
 		}
 
-		if new.NoDataState == "" {
+		if newRule.NoDataState == "" {
-			new.NoDataState = models.NoData
+			newRule.NoDataState = models.NoData
 		}
 
-		if new.ExecErrState == "" {
+		if newRule.ExecErrState == "" {
-			new.ExecErrState = models.AlertingErrState
+			newRule.ExecErrState = models.AlertingErrState
 		}
 
-		err := new.PreSave(time.Now)
+		err := newRule.PreSave(time.Now)
 		require.NoError(f.t, err)
 
-		rules = append(rules, new)
+		existingRules = append(existingRules, newRule)
 	}
 
-	f.Rules[cmd.OrgID][cmd.RuleGroupConfig.Name][cmd.NamespaceUID] = rules
+	f.Rules[cmd.OrgID] = existingRules
 	return nil
 }
 

pkg/services/ngalert/tests/util.go

@@ -109,12 +109,12 @@ func CreateTestAlertRuleWithLabels(t *testing.T, ctx context.Context, dbstore *s
 	})
 	require.NoError(t, err)
 
-	q := models.ListRuleGroupAlertRulesQuery{
+	q := models.GetAlertRulesQuery{
 		OrgID:        orgID,
 		NamespaceUID: "namespace",
-		RuleGroup:    ruleGroup,
+		RuleGroup:    &ruleGroup,
 	}
-	err = dbstore.GetRuleGroupAlertRules(ctx, &q)
+	err = dbstore.GetAlertRules(ctx, &q)
 	require.NoError(t, err)
 	require.NotEmpty(t, q.Result)