Alerting: Support optional matchers (#117911)

* allow empty matcher for imported configs
* add restrictions to the identifier
* change default identifier to "imported"

pkg/services/ngalert/api/api_convert_prometheus.go

@@ -17,6 +17,8 @@ import (
 	prommodel "github.com/prometheus/common/model"
 	"go.yaml.in/yaml/v3"
 
+	k8svalidation "k8s.io/apimachinery/pkg/util/validation"
+
 	"github.com/grafana/grafana/pkg/api/response"
 	"github.com/grafana/grafana/pkg/apimachinery/errutil"
 	"github.com/grafana/grafana/pkg/infra/log"
@@ -30,6 +32,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/prom"
 	"github.com/grafana/grafana/pkg/services/ngalert/provisioning"
+	"github.com/grafana/grafana/pkg/services/sqlstore/migrations/ualert"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
 )
@@ -62,7 +65,7 @@ const (
 
 	// configIdentifierHeader is the header that specifies the identifier for imported Alertmanager config.
 	configIdentifierHeader  = "X-Grafana-Alerting-Config-Identifier"
-	defaultConfigIdentifier = "default"
+	defaultConfigIdentifier = "imported"
 
 	// versionMessageHeader is the header that specifies an optional message for rule versions.
 	versionMessageHeader = "X-Grafana-Alerting-Version-Message"
@@ -592,7 +595,11 @@ func (srv *ConvertPrometheusSrv) RouteConvertPrometheusPostAlertmanagerConfig(c
 
 	logger := srv.logger.FromContext(c.Req.Context())
 
-	identifier := parseConfigIdentifierHeader(c)
+	identifier, err := parseConfigIdentifierHeader(c)
+	if err != nil {
+		logger.Error("Failed to parse config identifier header", "error", err)
+		return errorToResponse(err)
+	}
 
 	mergeMatchers, err := parseMergeMatchersHeader(c)
 	if err != nil {
@@ -631,7 +638,11 @@ func (srv *ConvertPrometheusSrv) RouteConvertPrometheusGetAlertmanagerConfig(c *
 	logger := srv.logger.FromContext(c.Req.Context())
 	ctx := c.Req.Context()
 
-	identifier := parseConfigIdentifierHeader(c)
+	identifier, err := parseConfigIdentifierHeader(c)
+	if err != nil {
+		logger.Error("Failed to parse config identifier header", "error", err)
+		return errorToResponse(err)
+	}
 
 	cfg, err := srv.am.GetAlertmanagerConfiguration(ctx, c.GetOrgID(), false, false)
 	if err != nil {
@@ -676,9 +687,13 @@ func (srv *ConvertPrometheusSrv) RouteConvertPrometheusDeleteAlertmanagerConfig(
 
 	logger := srv.logger.FromContext(c.Req.Context())
 
-	identifier := parseConfigIdentifierHeader(c)
+	identifier, err := parseConfigIdentifierHeader(c)
+	if err != nil {
+		logger.Error("Failed to parse config identifier header", "error", err)
+		return errorToResponse(err)
+	}
 
-	err := srv.am.DeleteExtraConfiguration(c.Req.Context(), c.GetOrgID(), identifier)
+	err = srv.am.DeleteExtraConfiguration(c.Req.Context(), c.GetOrgID(), identifier)
 	if err != nil {
 		logger.Error("Failed to delete alertmanager configuration", "error", err, "identifier", identifier)
 		return errorToResponse(fmt.Errorf("failed to delete alertmanager configuration: %w", err))
@@ -846,7 +861,7 @@ func parseMergeMatchersHeader(c *contextmodel.ReqContext) (amconfig.Matchers, er
 	matchersStr := strings.TrimSpace(c.Req.Header.Get(mergeMatchersHeader))
 
 	if matchersStr == "" {
-		return amconfig.Matchers{}, errInvalidHeaderValue(mergeMatchersHeader, errors.New("value cannot be empty"))
+		return nil, nil
 	}
 
 	kvPairs, err := parseKeyValuePairs(matchersStr, mergeMatchersHeader)
@@ -893,12 +908,19 @@ func formatMergeMatchers(matchers amconfig.Matchers) string {
 	return strings.Join(pairs, ",")
 }
 
-func parseConfigIdentifierHeader(c *contextmodel.ReqContext) string {
+func parseConfigIdentifierHeader(c *contextmodel.ReqContext) (string, error) {
 	identifier := strings.TrimSpace(c.Req.Header.Get(configIdentifierHeader))
 	if identifier == "" {
-		return defaultConfigIdentifier
+		return defaultConfigIdentifier, nil
 	}
-	return identifier
+	if errs := k8svalidation.IsDNS1123Subdomain(identifier); len(errs) > 0 {
+		return "", errInvalidHeaderValue(configIdentifierHeader, errors.New(strings.Join(errs, ",")))
+	}
+	if len(identifier) > ualert.UIDMaxLength {
+		return "", errInvalidHeaderValue(configIdentifierHeader,
+			fmt.Errorf("must be less than %d characters", ualert.UIDMaxLength))
+	}
+	return identifier, nil
 }
 
 // convertPrometheusResponse returns a JSON or YAML response based on the Accept header.

pkg/services/ngalert/api/api_convert_prometheus_test.go

@@ -2237,9 +2237,9 @@ func TestParseMergeMatchersHeader(t *testing.T) {
 		expectedMatchers amconfig.Matchers
 	}{
 		{
-			name:          "empty header should return error",
+			name:          "empty header should not return error",
 			headerValue:   "",
-			expectedError: true,
+			expectedError: false,
 		},
 		{
 			name:          "single matcher should parse correctly",
@@ -2335,6 +2335,11 @@ func TestParseConfigIdentifierHeader(t *testing.T) {
 			headerValue:   "   ",
 			expectedValue: defaultConfigIdentifier,
 		},
+		{
+			name:          "invalid identifier should return error",
+			headerValue:   "invalid identifier",
+			expectedError: true,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -2342,8 +2347,13 @@ func TestParseConfigIdentifierHeader(t *testing.T) {
 			rc := createRequestCtx()
 			rc.Req.Header.Set(configIdentifierHeader, tc.headerValue)
 
-			identifier := parseConfigIdentifierHeader(rc)
-			require.Equal(t, tc.expectedValue, identifier)
+			identifier, err := parseConfigIdentifierHeader(rc)
+			if tc.expectedError {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tc.expectedValue, identifier)
+			}
 		})
 	}
 }

pkg/services/ngalert/api/tooling/definitions/alertmanager.go

@@ -734,10 +734,6 @@ func (c ExtraConfiguration) Validate() error {
 		return errors.New("identifier is required")
 	}
 
-	if len(c.MergeMatchers) == 0 {
-		return errInvalidExtraConfiguration(errors.New("at least one matcher is required"))
-	}
-
 	for _, m := range c.MergeMatchers {
 		if m.Type != labels.MatchEqual {
 			return errInvalidExtraConfiguration(errors.New("only matchers with type equal are supported"))
@@ -835,10 +831,13 @@ func (c *PostableUserConfig) GetMergedAlertmanagerConfig() (MergeResult, error)
 		return MergeResult{}, fmt.Errorf("failed to merge alertmanager config: %w", err)
 	}
 
+	route := mcfg.Route
+	definition.RenameResourceUsagesInRoutes([]*definition.Route{route}, m.RenameResources)
+
 	return MergeResult{
 		MergeResult: m,
 		Identifier:  mimirCfg.Identifier,
-		ExtraRoute:  mcfg.Route,
+		ExtraRoute:  route,
 	}, nil
 }
 

pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go

@@ -7,6 +7,7 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/grafana/alerting/definition"
 	"github.com/prometheus/alertmanager/config"
@@ -15,6 +16,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.yaml.in/yaml/v3"
+
+	"github.com/grafana/grafana/pkg/util"
 )
 
 //go:embed test-data/*.*
@@ -266,12 +269,32 @@ func TestPostableUserConfig_GetMergedAlertmanagerConfig(t *testing.T) {
 		name          string
 		config        PostableUserConfig
 		expectedError string
+		expected      MergeResult
 	}{
 		{
 			name: "no extra configs",
 			config: PostableUserConfig{
 				AlertmanagerConfig: alertmanagerCfg,
 			},
+			expected: MergeResult{
+				MergeResult: definition.MergeResult{
+					Config: definition.PostableApiAlertingConfig{
+						Config: Config{
+							Route: &Route{
+								Receiver: "default",
+							},
+						},
+						Receivers: []*PostableApiReceiver{
+							{
+								Receiver: config.Receiver{
+									Name: "default",
+								},
+							},
+						},
+					},
+					RenameResources: definition.RenameResources{},
+				},
+			},
 		},
 		{
 			name: "valid mimir config",
@@ -289,14 +312,181 @@ func TestPostableUserConfig_GetMergedAlertmanagerConfig(t *testing.T) {
 						},
 						AlertmanagerConfig: `route:
   receiver: mimir-receiver
+  group_by: ['alertname']
+  routes:
+    - receiver: default
+      matchers:
+        - severity="critical"
 receivers:
-  - name: mimir-receiver`,
+  - name: mimir-receiver
+  - name: default`,
+					},
+				},
+			},
+			expected: MergeResult{
+				MergeResult: definition.MergeResult{
+					Config: definition.PostableApiAlertingConfig{
+						Config: Config{
+							Route: &Route{
+								Receiver: "default",
+								Routes: []*Route{
+									{
+										Matchers: []*labels.Matcher{
+											{
+												Type:  labels.MatchEqual,
+												Name:  "cluster",
+												Value: "prod",
+											},
+										},
+										GroupInterval:  util.Pointer(model.Duration(5 * time.Minute)),
+										GroupWait:      util.Pointer(model.Duration(30 * time.Second)),
+										RepeatInterval: util.Pointer(model.Duration(4 * time.Hour)),
+										Continue:       false,
+										Receiver:       "mimir-receiver",
+										GroupByStr:     []string{"alertname"},
+										GroupBy:        []model.LabelName{"alertname"},
+										Routes: []*Route{
+											{
+												Matchers: []*labels.Matcher{
+													{
+														Type:  labels.MatchEqual,
+														Name:  "severity",
+														Value: "critical",
+													},
+												},
+												Receiver: "defaultmimir-1",
+												Routes:   []*Route{},
+											},
+										},
+									},
+								},
+							},
+							InhibitRules:  []config.InhibitRule{},
+							TimeIntervals: []config.TimeInterval{},
+						},
+						Receivers: []*PostableApiReceiver{
+							{
+								Receiver: config.Receiver{
+									Name: "default",
+								},
+							},
+							{
+								Receiver: config.Receiver{
+									Name: "mimir-receiver",
+								},
+							},
+							{
+								Receiver: config.Receiver{
+									Name: "defaultmimir-1",
+								},
+							},
+						},
+					},
+					RenameResources: definition.RenameResources{
+						Receivers: map[string]string{
+							"default": "defaultmimir-1",
+						},
+						TimeIntervals: map[string]string{},
+					},
+				},
+				Identifier: "mimir-1",
+				ExtraRoute: &Route{
+					Receiver:   "mimir-receiver",
+					GroupByStr: []string{"alertname"},
+					GroupBy:    []model.LabelName{"alertname"},
+					Routes: []*Route{
+						{
+							Matchers: []*labels.Matcher{
+								{
+									Type:  labels.MatchEqual,
+									Name:  "severity",
+									Value: "critical",
+								},
+							},
+							Receiver: "defaultmimir-1",
+							Routes:   []*Route{},
+						},
 					},
 				},
 			},
 		},
 		{
-			name: "empty identifier",
+			name: "valid mimir config without merging matchers",
+			config: PostableUserConfig{
+				AlertmanagerConfig: alertmanagerCfg,
+				ExtraConfigs: []ExtraConfiguration{
+					{
+						Identifier: "mimir-1",
+						AlertmanagerConfig: `route:
+  receiver: mimir-receiver
+  group_by: ['alertname']
+  routes:
+    - receiver: default
+      matchers:
+        - severity="critical"
+receivers:
+  - name: mimir-receiver
+  - name: default`,
+					},
+				},
+			},
+			expected: MergeResult{
+				MergeResult: definition.MergeResult{
+					Config: definition.PostableApiAlertingConfig{
+						Config: Config{
+							Route: &Route{
+								Receiver: "default",
+							},
+							TimeIntervals: []config.TimeInterval{},
+						},
+						Receivers: []*PostableApiReceiver{
+							{
+								Receiver: config.Receiver{
+									Name: "default",
+								},
+							},
+							{
+								Receiver: config.Receiver{
+									Name: "mimir-receiver",
+								},
+							},
+							{
+								Receiver: config.Receiver{
+									Name: "defaultmimir-1",
+								},
+							},
+						},
+					},
+					RenameResources: definition.RenameResources{
+						Receivers: map[string]string{
+							"default": "defaultmimir-1",
+						},
+						TimeIntervals: map[string]string{},
+					},
+				},
+				Identifier: "mimir-1",
+				ExtraRoute: &Route{
+					Receiver:   "mimir-receiver",
+					GroupByStr: []string{"alertname"},
+					GroupBy:    []model.LabelName{"alertname"},
+					Routes: []*Route{
+						{
+							Matchers: []*labels.Matcher{
+								{
+									Type:  labels.MatchEqual,
+									Name:  "severity",
+									Value: "critical",
+								},
+							},
+							Receiver: "defaultmimir-1",
+							Routes:   []*Route{},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "empty matchers and identifier",
 			config: PostableUserConfig{
 				AlertmanagerConfig: alertmanagerCfg,
 				ExtraConfigs: []ExtraConfiguration{
@@ -343,6 +533,7 @@ receivers:
 			} else {
 				require.NoError(t, err)
 				require.NotNil(t, result.Config)
+				require.EqualValues(t, tc.expected, result)
 			}
 		})
 	}

pkg/tests/api/alerting/api_convert_prometheus_alertmanager_test.go

@@ -140,27 +140,13 @@ func TestIntegrationConvertPrometheusAlertmanagerEndpoints(t *testing.T) {
 			requireStatusCode(t, http.StatusAccepted, status, "")
 
 			getHeaders := map[string]string{
-				"X-Grafana-Alerting-Config-Identifier": "default",
+				"X-Grafana-Alerting-Config-Identifier": "imported",
 			}
 			responseConfig, status, _ := apiClient.RawConvertPrometheusGetAlertmanagerConfig(t, getHeaders)
 			requireStatusCode(t, http.StatusOK, status, "")
 			require.NotEmpty(t, responseConfig.AlertmanagerConfig)
 		})
 
-		t.Run("POST without merge matchers header should fail", func(t *testing.T) {
-			headers := map[string]string{
-				"Content-Type":                         "application/yaml",
-				"X-Grafana-Alerting-Config-Identifier": "test-config",
-			}
-
-			amConfig := apimodels.AlertmanagerUserConfig{
-				AlertmanagerConfig: string(configYaml),
-			}
-
-			_, status, _ := apiClient.RawConvertPrometheusPostAlertmanagerConfig(t, amConfig, headers)
-			requireStatusCode(t, http.StatusBadRequest, status, "")
-		})
-
 		t.Run("POST with invalid merge matchers format should fail", func(t *testing.T) {
 			headers := map[string]string{
 				"Content-Type":                         "application/yaml",

pkg/tests/apis/alerting/notifications/timeinterval/imported_test.go

@@ -41,7 +41,7 @@ func TestIntegrationImportedTimeIntervals(t *testing.T) {
 	configYaml, err := testData.ReadFile(path.Join("test-data", "imported.yaml"))
 	require.NoError(t, err)
 
-	identifier := "-test-imported-time-intervals"
+	identifier := "test-imported-time-intervals"
 	mergeMatchers := "_imported=true"
 
 	headers := map[string]string{