From 879edcbc32a7ddb4328bce736427a7be0915b617 Mon Sep 17 00:00:00 2001
From: Matthew Jacobson <matthew.jacobson@grafana.com>
Date: Fri, 19 Apr 2024 11:23:16 -0400
Subject: [PATCH] [v10.4.x] Alerting: Fix simplified routing group by override
 (#86620)

Alerting: Fix simplified routing group by override (#86552)

* Alerting: Fix simplified routing custom group by override

Custom group by overrides for simplified routing were missing required fields
GroupBy and GroupByAll normally set during upstream Route validation.

This fix ensures those missing fields are applied to the generated routes.

* Inline GroupBy and GroupByAll initialization instead of normalize after

(cherry picked from commit 71445002b7061244cd08b4c180481248e262c309)
---
 pkg/services/ngalert/models/notifications.go  |  6 ++---
 .../ngalert/notifier/autogen_alertmanager.go  | 22 ++++++++++++++++++-
 .../notifier/autogen_alertmanager_test.go     |  3 +++
 3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/pkg/services/ngalert/models/notifications.go b/pkg/services/ngalert/models/notifications.go
index 3271eb6d0ba..4f2ff59aa87 100644
--- a/pkg/services/ngalert/models/notifications.go
+++ b/pkg/services/ngalert/models/notifications.go
@@ -12,8 +12,8 @@ import (
 	"github.com/prometheus/common/model"
 )
 
-// groupByAll is a special value defined by alertmanager that can be used in a Route's GroupBy field to aggregate by all possible labels.
-const groupByAll = "..."
+// GroupByAll is a special value defined by alertmanager that can be used in a Route's GroupBy field to aggregate by all possible labels.
+const GroupByAll = "..."
 
 type ListNotificationSettingsQuery struct {
 	OrgID        int64
@@ -44,7 +44,7 @@ func (s *NotificationSettings) Validate() error {
 	if len(s.GroupBy) > 0 {
 		alertName, folderTitle := false, false
 		for _, lbl := range s.GroupBy {
-			if lbl == groupByAll {
+			if lbl == GroupByAll {
 				alertName, folderTitle = true, true
 				break
 			}
diff --git a/pkg/services/ngalert/notifier/autogen_alertmanager.go b/pkg/services/ngalert/notifier/autogen_alertmanager.go
index af5de7947a5..ca2a145951a 100644
--- a/pkg/services/ngalert/notifier/autogen_alertmanager.go
+++ b/pkg/services/ngalert/notifier/autogen_alertmanager.go
@@ -116,12 +116,16 @@ func generateRouteFromSettings(defaultReceiver string, settings map[data.Fingerp
 			if err != nil {
 				return autogeneratedRoute{}, err
 			}
+			groupByStr := []string{models.FolderTitleLabel, model.AlertNameLabel}
+			groupByAll, groupBy := toGroupBy(groupByStr...)
 			receiverRoute = &definitions.Route{
 				Receiver:       s.Receiver,
 				ObjectMatchers: definitions.ObjectMatchers{contactMatcher},
 				Continue:       false,
 				// Since we'll have many rules from different folders using this policy, we ensure it has these necessary groupings.
-				GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel},
+				GroupByStr: groupByStr,
+				GroupBy:    groupBy,
+				GroupByAll: groupByAll,
 			}
 			receiverRoutes[s.Receiver] = receiverRoute
 			autoGenRoot.Routes = append(autoGenRoot.Routes, receiverRoute)
@@ -135,12 +139,15 @@ func generateRouteFromSettings(defaultReceiver string, settings map[data.Fingerp
 		if err != nil {
 			return autogeneratedRoute{}, err
 		}
+		groupByAll, groupBy := toGroupBy(s.GroupBy...)
 		receiverRoute.Routes = append(receiverRoute.Routes, &definitions.Route{
 			Receiver:       s.Receiver,
 			ObjectMatchers: definitions.ObjectMatchers{settingMatcher},
 			Continue:       false, // Only a single setting-specific route should match.
 
 			GroupByStr:        s.GroupBy, // Note: in order to pass validation at least FolderTitleLabel and AlertNameLabel are always included.
+			GroupBy:           groupBy,
+			GroupByAll:        groupByAll,
 			MuteTimeIntervals: s.MuteTimeIntervals,
 			GroupWait:         s.GroupWait,
 			GroupInterval:     s.GroupInterval,
@@ -153,6 +160,19 @@ func generateRouteFromSettings(defaultReceiver string, settings map[data.Fingerp
 	}, nil
 }
 
+// toGroupBy converts the given label strings to (groupByAll, []model.LabelName) where groupByAll is true if the input
+// contains models.GroupByAll. This logic is in accordance with upstream Route.ValidateChild().
+func toGroupBy(groupByStr ...string) (groupByAll bool, groupBy []model.LabelName) {
+	for _, l := range groupByStr {
+		if l == models.GroupByAll {
+			return true, nil
+		} else {
+			groupBy = append(groupBy, model.LabelName(l))
+		}
+	}
+	return false, groupBy
+}
+
 // addToRoute adds this autogenerated route to the given route as the first top-level route under the root.
 func (ar *autogeneratedRoute) addToRoute(route *definitions.Route) error {
 	if route == nil {
diff --git a/pkg/services/ngalert/notifier/autogen_alertmanager_test.go b/pkg/services/ngalert/notifier/autogen_alertmanager_test.go
index 6ac486dbeb3..268a805336a 100644
--- a/pkg/services/ngalert/notifier/autogen_alertmanager_test.go
+++ b/pkg/services/ngalert/notifier/autogen_alertmanager_test.go
@@ -227,6 +227,9 @@ func TestAddAutogenConfig(t *testing.T) {
 				require.NoError(t, err)
 			}
 
+			// We compare against the upstream normalized route.
+			require.NoError(t, tt.expRoute.Validate())
+
 			cOpt := []cmp.Option{
 				cmpopts.IgnoreUnexported(definitions.Route{}, labels.Matcher{}),
 			}