From 4ad80cec1a1789fb49c69f421bbd312ed2e541e3 Mon Sep 17 00:00:00 2001 From: Alexander Akhmetov Date: Mon, 28 Jul 2025 10:09:26 +0200 Subject: [PATCH] Alerting: Validate extra configuration with PostableApiAlertingConfig (#108706) --- .../api/api_convert_prometheus_test.go | 33 ++++++- .../api/tooling/definitions/alertmanager.go | 8 +- .../tooling/definitions/alertmanager_test.go | 90 +++++++++++++++++++ 3 files changed, 127 insertions(+), 4 deletions(-) diff --git a/pkg/services/ngalert/api/api_convert_prometheus_test.go b/pkg/services/ngalert/api/api_convert_prometheus_test.go index 7478783da62..e5833280319 100644 --- a/pkg/services/ngalert/api/api_convert_prometheus_test.go +++ b/pkg/services/ngalert/api/api_convert_prometheus_test.go @@ -1567,7 +1567,18 @@ func TestRouteConvertPrometheusPostAlertmanagerConfig(t *testing.T) { ft := featuremgmt.WithFeatures(featuremgmt.FlagAlertingImportAlertmanagerAPI) srv, _, _ := createConvertPrometheusSrv(t, withAlertmanager(mockAM), withFeatureToggles(ft)) - amCfg := apimodels.AlertmanagerUserConfig{} + amCfg := apimodels.AlertmanagerUserConfig{ + AlertmanagerConfig: `{ + "route": { + "receiver": "default" + }, + "receivers": [ + { + "name": "default" + } + ] + }`, + } response := srv.RouteConvertPrometheusPostAlertmanagerConfig(rc, amCfg) require.Equal(t, http.StatusAccepted, response.Status()) @@ -1585,6 +1596,26 @@ func TestRouteConvertPrometheusPostAlertmanagerConfig(t *testing.T) { require.Equal(t, http.StatusBadRequest, response.Status()) require.Contains(t, string(response.Body()), "format should be 'key=value,key2=value2'") }) + + t.Run("should return error when alertmanager config has empty route", func(t *testing.T) { + rc := createRequestCtx() + rc.Req.Header.Set(configIdentifierHeader, identifier) + rc.Req.Header.Set(mergeMatchersHeader, "env=prod") + + amCfg := apimodels.AlertmanagerUserConfig{ + AlertmanagerConfig: `{ + "receivers": [ + { + "name": "default" + } + ] + }`, + } + response := srv.RouteConvertPrometheusPostAlertmanagerConfig(rc, amCfg) + + require.Equal(t, http.StatusBadRequest, response.Status()) + require.Contains(t, string(response.Body()), "failed to parse alertmanager config") + }) } func TestRouteConvertPrometheusGetAlertmanagerConfig(t *testing.T) { diff --git a/pkg/services/ngalert/api/tooling/definitions/alertmanager.go b/pkg/services/ngalert/api/tooling/definitions/alertmanager.go index c73486d277a..7cf30d3b702 100644 --- a/pkg/services/ngalert/api/tooling/definitions/alertmanager.go +++ b/pkg/services/ngalert/api/tooling/definitions/alertmanager.go @@ -745,12 +745,14 @@ func (c ExtraConfiguration) Validate() error { } } - // Alertmanager configuration is validated during YAML unmarshalling. - am := config.Config{} - err := yaml.Unmarshal([]byte(c.AlertmanagerConfig), &am) + cfg, err := c.GetAlertmanagerConfig() if err != nil { return errInvalidExtraConfiguration(fmt.Errorf("failed to parse alertmanager config: %w", err)) } + err = cfg.Validate() + if err != nil { + return errInvalidExtraConfiguration(fmt.Errorf("invalid alertmanager config: %w", err)) + } return nil } diff --git a/pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go b/pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go index 7c18af905e4..e6f074a7938 100644 --- a/pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go +++ b/pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go @@ -403,3 +403,93 @@ func TestPostableUserConfig_GetMergedTemplateDefinitions(t *testing.T) { }) } } + +func TestExtraConfiguration_Validate(t *testing.T) { + testCases := []struct { + name string + config ExtraConfiguration + expectedError string + }{ + { + name: "valid configuration", + config: ExtraConfiguration{ + Identifier: "test-config", + MergeMatchers: config.Matchers{{Type: labels.MatchEqual, Name: "env", Value: "prod"}}, + AlertmanagerConfig: `route: + receiver: default +receivers: + - name: default`, + }, + }, + { + name: "empty identifier", + config: ExtraConfiguration{ + Identifier: "", + MergeMatchers: config.Matchers{{Type: labels.MatchEqual, Name: "env", Value: "prod"}}, + AlertmanagerConfig: `route: {receiver: default}`, + }, + expectedError: "identifier is required", + }, + { + name: "invalid matcher type", + config: ExtraConfiguration{ + Identifier: "test-config", + MergeMatchers: config.Matchers{{Type: labels.MatchNotEqual, Name: "env", Value: "prod"}}, + AlertmanagerConfig: `route: + receiver: default +receivers: + - name: default`, + }, + expectedError: "only matchers with type equal are supported", + }, + { + name: "invalid YAML alertmanager config", + config: ExtraConfiguration{ + Identifier: "test-config", + MergeMatchers: config.Matchers{{Type: labels.MatchEqual, Name: "env", Value: "prod"}}, + AlertmanagerConfig: `invalid: yaml: content: [`, + }, + expectedError: "failed to parse alertmanager config", + }, + { + name: "missing route in alertmanager config", + config: ExtraConfiguration{ + Identifier: "test-config", + MergeMatchers: config.Matchers{{Type: labels.MatchEqual, Name: "env", Value: "prod"}}, + AlertmanagerConfig: `receivers: + - name: default`, + }, + expectedError: "no routes provided", + }, + { + name: "missing receivers in alertmanager config", + config: ExtraConfiguration{ + Identifier: "test-config", + MergeMatchers: config.Matchers{{Type: labels.MatchEqual, Name: "env", Value: "prod"}}, + AlertmanagerConfig: `route: + receiver: default`, + }, + expectedError: "undefined receiver", + }, + { + name: "empty alertmanager config", + config: ExtraConfiguration{ + Identifier: "test-config", + MergeMatchers: config.Matchers{{Type: labels.MatchEqual, Name: "env", Value: "prod"}}, + AlertmanagerConfig: "", + }, + expectedError: "failed to parse alertmanager config", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.config.Validate() + if tc.expectedError == "" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, tc.expectedError) + } + }) + } +}