diff --git a/pkg/services/ngalert/api/api_convert_prometheus.go b/pkg/services/ngalert/api/api_convert_prometheus.go index 40159e8da8a..8107269b33c 100644 --- a/pkg/services/ngalert/api/api_convert_prometheus.go +++ b/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. diff --git a/pkg/services/ngalert/api/api_convert_prometheus_test.go b/pkg/services/ngalert/api/api_convert_prometheus_test.go index 4780b6b508d..ab47f9b528f 100644 --- a/pkg/services/ngalert/api/api_convert_prometheus_test.go +++ b/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) + } }) } } diff --git a/pkg/services/ngalert/api/tooling/definitions/alertmanager.go b/pkg/services/ngalert/api/tooling/definitions/alertmanager.go index 626398d4402..57eed0cca7d 100644 --- a/pkg/services/ngalert/api/tooling/definitions/alertmanager.go +++ b/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 } diff --git a/pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go b/pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go index 08f48e74159..e5dceb82801 100644 --- a/pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go +++ b/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) } }) } diff --git a/pkg/tests/api/alerting/api_convert_prometheus_alertmanager_test.go b/pkg/tests/api/alerting/api_convert_prometheus_alertmanager_test.go index 427131bb90c..9192a21a0bd 100644 --- a/pkg/tests/api/alerting/api_convert_prometheus_alertmanager_test.go +++ b/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", diff --git a/pkg/tests/apis/alerting/notifications/timeinterval/imported_test.go b/pkg/tests/apis/alerting/notifications/timeinterval/imported_test.go index fb14065982b..b1ef1bbfc23 100644 --- a/pkg/tests/apis/alerting/notifications/timeinterval/imported_test.go +++ b/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{