From 56cd797d6584fa262faa1b21c6abee28c619f7ed Mon Sep 17 00:00:00 2001 From: "grafana-delivery-bot[bot]" <132647405+grafana-delivery-bot[bot]@users.noreply.github.com> Date: Fri, 19 Apr 2024 03:25:13 +0200 Subject: [PATCH] [v11.0.x] Alerting: Fix simplified routing group by override (#86563) 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) Co-authored-by: Matthew Jacobson --- .../ngalert/notifier/autogen_alertmanager.go | 26 +++++++++++++++++-- .../notifier/autogen_alertmanager_test.go | 3 +++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/pkg/services/ngalert/notifier/autogen_alertmanager.go b/pkg/services/ngalert/notifier/autogen_alertmanager.go index 003ff617e33..80df43c9a0f 100644 --- a/pkg/services/ngalert/notifier/autogen_alertmanager.go +++ b/pkg/services/ngalert/notifier/autogen_alertmanager.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/prometheus/alertmanager/pkg/labels" + "github.com/prometheus/common/model" "golang.org/x/exp/maps" "github.com/grafana/grafana/pkg/infra/log" @@ -115,12 +116,16 @@ func generateRouteFromSettings(defaultReceiver string, settings map[data.Fingerp if err != nil { return autogeneratedRoute{}, err } + groupByStr := append([]string{}, models.DefaultNotificationSettingsGroupBy...) + 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: append([]string{}, models.DefaultNotificationSettingsGroupBy...), + GroupByStr: groupByStr, + GroupBy: groupBy, + GroupByAll: groupByAll, } receiverRoutes[s.Receiver] = receiverRoute autoGenRoot.Routes = append(autoGenRoot.Routes, receiverRoute) @@ -134,12 +139,16 @@ func generateRouteFromSettings(defaultReceiver string, settings map[data.Fingerp if err != nil { return autogeneratedRoute{}, err } + normalized := s.NormalizedGroupBy() + groupByAll, groupBy := toGroupBy(normalized...) 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.NormalizedGroupBy(), + GroupByStr: normalized, + GroupBy: groupBy, + GroupByAll: groupByAll, MuteTimeIntervals: s.MuteTimeIntervals, GroupWait: s.GroupWait, GroupInterval: s.GroupInterval, @@ -152,6 +161,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 14e50e844ff..bc962728eac 100644 --- a/pkg/services/ngalert/notifier/autogen_alertmanager_test.go +++ b/pkg/services/ngalert/notifier/autogen_alertmanager_test.go @@ -292,6 +292,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{}), }