diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index 88898de2d64..77484ba43c4 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -10,11 +10,12 @@ import ( notificationHistorian "github.com/grafana/alerting/notify/historian" "github.com/grafana/alerting/notify/historian/lokiclient" "github.com/grafana/alerting/notify/nfstatus" - "github.com/grafana/grafana/pkg/services/ngalert/lokiconfig" "github.com/prometheus/alertmanager/featurecontrol" "github.com/prometheus/alertmanager/matchers/compat" "golang.org/x/sync/errgroup" + "github.com/grafana/grafana/pkg/services/ngalert/lokiconfig" + "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/events" @@ -219,8 +220,8 @@ func (ng *AlertNG) init() error { SmtpConfig: smtpCfg, Timeout: ng.Cfg.UnifiedAlerting.RemoteAlertmanager.Timeout, } - autogenFn := func(ctx context.Context, logger log.Logger, orgID int64, cfg *definitions.PostableApiAlertingConfig, skipInvalid bool) error { - return notifier.AddAutogenConfig(ctx, logger, ng.store, orgID, cfg, skipInvalid, ng.FeatureToggles) + autogenFn := func(ctx context.Context, logger log.Logger, orgID int64, cfg *definitions.PostableApiAlertingConfig, invalidReceiverAction notifier.InvalidReceiversAction) error { + return notifier.AddAutogenConfig(ctx, logger, ng.store, orgID, cfg, invalidReceiverAction, ng.FeatureToggles) } // This function will be used by the MOA to create new Alertmanagers. diff --git a/pkg/services/ngalert/notifier/alertmanager.go b/pkg/services/ngalert/notifier/alertmanager.go index f4454d87c5f..dc2f63fb0ec 100644 --- a/pkg/services/ngalert/notifier/alertmanager.go +++ b/pkg/services/ngalert/notifier/alertmanager.go @@ -194,7 +194,7 @@ func (am *alertmanager) SaveAndApplyDefaultConfig(ctx context.Context) error { } err = am.Store.SaveAlertmanagerConfigurationWithCallback(ctx, cmd, func() error { - _, err = am.applyConfig(ctx, cfg, true) + _, err = am.applyConfig(ctx, cfg, LogInvalidReceivers) return err }) if err != nil { @@ -233,7 +233,7 @@ func (am *alertmanager) SaveAndApplyConfig(ctx context.Context, cfg *apimodels.P } err = am.Store.SaveAlertmanagerConfigurationWithCallback(ctx, cmd, func() error { - _, err = am.applyConfig(ctx, cfg, false) // fail if the autogen config is invalid + _, err = am.applyConfig(ctx, cfg, LogInvalidReceivers) // fail if the autogen config is invalid return err }) if err != nil { @@ -259,7 +259,7 @@ func (am *alertmanager) ApplyConfig(ctx context.Context, dbCfg *ngmodels.AlertCo // Since we will now update last_applied when autogen changes even if the user-created config remains the same. // To fix this however, the local alertmanager needs to be able to tell the difference between user-created and // autogen config, which may introduce cross-cutting complexity. - configChanged, err := am.applyConfig(ctx, cfg, true) + configChanged, err := am.applyConfig(ctx, cfg, ErrorOnInvalidReceivers) if err != nil { outerErr = fmt.Errorf("unable to apply configuration: %w", err) return @@ -330,7 +330,7 @@ func (am *alertmanager) aggregateInhibitMatchers(rules []config.InhibitRule, amu // applyConfig applies a new configuration by re-initializing all components using the configuration provided. // It returns a boolean indicating whether the user config was changed and an error. // It is not safe to call concurrently. -func (am *alertmanager) applyConfig(ctx context.Context, cfg *apimodels.PostableUserConfig, skipInvalid bool) (bool, error) { +func (am *alertmanager) applyConfig(ctx context.Context, cfg *apimodels.PostableUserConfig, onInvalid InvalidReceiversAction) (bool, error) { err := am.crypto.DecryptExtraConfigs(ctx, cfg) if err != nil { return false, fmt.Errorf("failed to decrypt external configurations: %w", err) @@ -347,7 +347,7 @@ func (am *alertmanager) applyConfig(ctx context.Context, cfg *apimodels.Postable templates := alertingNotify.PostableAPITemplatesToTemplateDefinitions(cfg.GetMergedTemplateDefinitions()) // Now add autogenerated config to the route. - err = AddAutogenConfig(ctx, am.logger, am.Store, am.Base.TenantID(), &amConfig, skipInvalid, am.features) + err = AddAutogenConfig(ctx, am.logger, am.Store, am.Base.TenantID(), &amConfig, onInvalid, am.features) if err != nil { return false, err } diff --git a/pkg/services/ngalert/notifier/alertmanager_config.go b/pkg/services/ngalert/notifier/alertmanager_config.go index 514da9be686..87d573124e7 100644 --- a/pkg/services/ngalert/notifier/alertmanager_config.go +++ b/pkg/services/ngalert/notifier/alertmanager_config.go @@ -133,7 +133,7 @@ func (moa *MultiOrgAlertmanager) GetAlertmanagerConfiguration(ctx context.Contex // Otherwise, broken settings (e.g. a receiver that doesn't exist) will cause the config returned here to be // different than the config currently in-use. // TODO: Preferably, we'd be getting the config directly from the in-memory AM so adding the autogen config would not be necessary. - err := AddAutogenConfig(ctx, moa.logger, moa.configStore, org, &cfg.AlertmanagerConfig, true, moa.featureManager) + err := AddAutogenConfig(ctx, moa.logger, moa.configStore, org, &cfg.AlertmanagerConfig, LogInvalidReceivers, moa.featureManager) if err != nil { return definitions.GettableUserConfig{}, err } diff --git a/pkg/services/ngalert/notifier/autogen_alertmanager.go b/pkg/services/ngalert/notifier/autogen_alertmanager.go index 7523e368ad5..f0a3ae040ee 100644 --- a/pkg/services/ngalert/notifier/autogen_alertmanager.go +++ b/pkg/services/ngalert/notifier/autogen_alertmanager.go @@ -21,10 +21,18 @@ type autogenRuleStore interface { ListNotificationSettings(ctx context.Context, q models.ListNotificationSettingsQuery) (map[models.AlertRuleKey][]models.NotificationSettings, error) } +type InvalidReceiversAction string + +const ( + ErrorOnInvalidReceivers InvalidReceiversAction = "error" + LogInvalidReceivers InvalidReceiversAction = "log" + IgnoreInvalidReceivers InvalidReceiversAction = "ignore" +) + // AddAutogenConfig creates the autogenerated configuration and adds it to the given apiAlertingConfig. // If skipInvalid is true, then invalid notification settings are skipped, otherwise an error is returned. -func AddAutogenConfig[R receiver](ctx context.Context, logger log.Logger, store autogenRuleStore, orgId int64, cfg apiAlertingConfig[R], skipInvalid bool, features featuremgmt.FeatureToggles) error { - autogenRoute, err := newAutogeneratedRoute(ctx, logger, store, orgId, cfg, skipInvalid, features) +func AddAutogenConfig[R receiver](ctx context.Context, logger log.Logger, store autogenRuleStore, orgId int64, cfg apiAlertingConfig[R], invalidReceiverAction InvalidReceiversAction, features featuremgmt.FeatureToggles) error { + autogenRoute, err := newAutogeneratedRoute(ctx, logger, store, orgId, cfg, invalidReceiverAction, features) if err != nil { return err } @@ -40,7 +48,7 @@ func AddAutogenConfig[R receiver](ctx context.Context, logger log.Logger, store // newAutogeneratedRoute creates a new autogenerated route based on the notification settings for the given org. // cfg is used to construct the settings validator and to ensure we create a dedicated route for each receiver. // skipInvalid is used to skip invalid settings instead of returning an error. -func newAutogeneratedRoute[R receiver](ctx context.Context, logger log.Logger, store autogenRuleStore, orgId int64, cfg apiAlertingConfig[R], skipInvalid bool, features featuremgmt.FeatureToggles) (autogeneratedRoute, error) { +func newAutogeneratedRoute[R receiver](ctx context.Context, logger log.Logger, store autogenRuleStore, orgId int64, cfg apiAlertingConfig[R], invalidReceiverAction InvalidReceiversAction, features featuremgmt.FeatureToggles) (autogeneratedRoute, error) { settings, err := store.ListNotificationSettings(ctx, models.ListNotificationSettingsQuery{OrgID: orgId}) if err != nil { return autogeneratedRoute{}, fmt.Errorf("failed to list alert rules: %w", err) @@ -60,11 +68,14 @@ func newAutogeneratedRoute[R receiver](ctx context.Context, logger log.Logger, s for _, setting := range ruleSettings { // TODO we should register this errors and somehow present to the users or make sure the config is always valid. if err = validator.Validate(setting); err != nil { - if skipInvalid { - logger.Error("Rule notification settings are invalid. Skipping", append(ruleKey.LogContext(), "error", err)...) - continue + switch invalidReceiverAction { + case ErrorOnInvalidReceivers: + return autogeneratedRoute{}, fmt.Errorf("invalid notification settings for rule %s: %w", ruleKey.UID, err) + case LogInvalidReceivers: + logger.Warn("Rule notification settings are invalid. Skipping", append(ruleKey.LogContext(), "error", err)...) + case IgnoreInvalidReceivers: // do nothing } - return autogeneratedRoute{}, fmt.Errorf("invalid notification settings for rule %s: %w", ruleKey.UID, err) + continue } fp := setting.Fingerprint(features) // Keep only unique settings. diff --git a/pkg/services/ngalert/notifier/autogen_alertmanager_test.go b/pkg/services/ngalert/notifier/autogen_alertmanager_test.go index bf376502caa..f0695e513bb 100644 --- a/pkg/services/ngalert/notifier/autogen_alertmanager_test.go +++ b/pkg/services/ngalert/notifier/autogen_alertmanager_test.go @@ -289,8 +289,11 @@ func TestAddAutogenConfig(t *testing.T) { for _, setting := range tt.storeSettings { store.notificationSettings[orgId][models.AlertRuleKey{OrgID: orgId, UID: util.GenerateShortUID()}] = []models.NotificationSettings{setting} } - - err := AddAutogenConfig(context.Background(), &logtest.Fake{}, store, orgId, tt.existingConfig, tt.skipInvalid, nil) + onInvalid := ErrorOnInvalidReceivers + if tt.skipInvalid { + onInvalid = IgnoreInvalidReceivers + } + err := AddAutogenConfig(context.Background(), &logtest.Fake{}, store, orgId, tt.existingConfig, onInvalid, nil) if tt.expErrorContains != "" { require.Error(t, err) require.ErrorContains(t, err, tt.expErrorContains) diff --git a/pkg/services/ngalert/remote/alertmanager.go b/pkg/services/ngalert/remote/alertmanager.go index 78fbbfe3a5b..81e3ef40181 100644 --- a/pkg/services/ngalert/remote/alertmanager.go +++ b/pkg/services/ngalert/remote/alertmanager.go @@ -47,10 +47,10 @@ type stateStore interface { } // AutogenFn is a function that adds auto-generated routes to a configuration. -type AutogenFn func(ctx context.Context, logger log.Logger, orgId int64, config *apimodels.PostableApiAlertingConfig, skipInvalid bool) error +type AutogenFn func(ctx context.Context, logger log.Logger, orgId int64, config *apimodels.PostableApiAlertingConfig, invalidReceiverAction notifier.InvalidReceiversAction) error // NoopAutogenFn is used to skip auto-generating routes. -func NoopAutogenFn(_ context.Context, _ log.Logger, _ int64, _ *apimodels.PostableApiAlertingConfig, _ bool) error { +func NoopAutogenFn(_ context.Context, _ log.Logger, _ int64, _ *apimodels.PostableApiAlertingConfig, _ notifier.InvalidReceiversAction) error { return nil } @@ -206,7 +206,7 @@ func NewAlertmanager(ctx context.Context, cfg AlertmanagerConfig, store stateSto // (grouping, group timing, time intervals etc) changes the autogenerated configuration. // The `default` flag is sent to the remote Alertmanager for informational purposes, so we can tolerate this. err = func() error { - defaultCfg, err := am.buildConfiguration(ctx, []byte(cfg.DefaultConfig), 0) + defaultCfg, err := am.buildConfiguration(ctx, []byte(cfg.DefaultConfig), 0, notifier.IgnoreInvalidReceivers) if err != nil { return fmt.Errorf("unable to build default configuration: %w", err) } @@ -271,7 +271,7 @@ func (am *Alertmanager) checkReadiness(ctx context.Context) error { // CompareAndSendConfiguration checks whether a given configuration is being used by the remote Alertmanager. // If not, it sends the configuration to the remote Alertmanager. func (am *Alertmanager) CompareAndSendConfiguration(ctx context.Context, config *models.AlertConfiguration) error { - payload, err := am.buildConfiguration(ctx, []byte(config.AlertmanagerConfiguration), config.CreatedAt) + payload, err := am.buildConfiguration(ctx, []byte(config.AlertmanagerConfiguration), config.CreatedAt, notifier.LogInvalidReceivers) if err != nil { return fmt.Errorf("unable to build configuration: %w", err) } @@ -303,14 +303,14 @@ func decrypter(ctx context.Context, crypto Crypto) models.DecryptFn { // buildConfiguration takes a raw Alertmanager configuration and returns a config that the remote Alertmanager can use. // It parses the initial configuration, adds auto-generated routes, decrypts receivers, and merges the extra configs. -func (am *Alertmanager) buildConfiguration(ctx context.Context, raw []byte, createdAtEpoch int64) (remoteClient.UserGrafanaConfig, error) { +func (am *Alertmanager) buildConfiguration(ctx context.Context, raw []byte, createdAtEpoch int64, autogenInvalidReceiverAction notifier.InvalidReceiversAction) (remoteClient.UserGrafanaConfig, error) { c, err := notifier.Load(raw) if err != nil { return remoteClient.UserGrafanaConfig{}, err } // Add auto-generated routes and decrypt before comparing. - if err := am.autogenFn(ctx, am.log, am.orgID, &c.AlertmanagerConfig, true); err != nil { + if err := am.autogenFn(ctx, am.log, am.orgID, &c.AlertmanagerConfig, autogenInvalidReceiverAction); err != nil { return remoteClient.UserGrafanaConfig{}, err } @@ -433,7 +433,7 @@ func (am *Alertmanager) SaveAndApplyConfig(ctx context.Context, cfg *apimodels.P return err } - payload, err := am.buildConfiguration(ctx, rawCopy, time.Now().Unix()) + payload, err := am.buildConfiguration(ctx, rawCopy, time.Now().Unix(), notifier.LogInvalidReceivers) if err != nil { return fmt.Errorf("unable to build configuration: %w", err) } @@ -444,7 +444,7 @@ func (am *Alertmanager) SaveAndApplyConfig(ctx context.Context, cfg *apimodels.P // SaveAndApplyDefaultConfig sends the default Grafana Alertmanager configuration to the remote Alertmanager. func (am *Alertmanager) SaveAndApplyDefaultConfig(ctx context.Context) error { am.log.Debug("Sending default configuration to a remote Alertmanager", "url", am.url) - payload, err := am.buildConfiguration(ctx, []byte(am.defaultConfig), time.Now().Unix()) + payload, err := am.buildConfiguration(ctx, []byte(am.defaultConfig), time.Now().Unix(), notifier.LogInvalidReceivers) if err != nil { return fmt.Errorf("unable to build default configuration: %w", err) } diff --git a/pkg/services/ngalert/remote/alertmanager_test.go b/pkg/services/ngalert/remote/alertmanager_test.go index 0ca0bbc950f..b183943d2ac 100644 --- a/pkg/services/ngalert/remote/alertmanager_test.go +++ b/pkg/services/ngalert/remote/alertmanager_test.go @@ -453,7 +453,7 @@ func TestCompareAndSendConfiguration(t *testing.T) { DefaultConfig: defaultGrafanaConfig, } - testAutogenFn := func(_ context.Context, _ log.Logger, _ int64, config *apimodels.PostableApiAlertingConfig, _ bool) error { + testAutogenFn := func(_ context.Context, _ log.Logger, _ int64, config *apimodels.PostableApiAlertingConfig, _ notifier.InvalidReceiversAction) error { newRoute := definition.Route{ Receiver: config.Receivers[0].Name, Match: map[string]string{"auto-gen-test": "true"}, @@ -497,7 +497,7 @@ func TestCompareAndSendConfiguration(t *testing.T) { testAutogenRoutes, err := notifier.Load([]byte(testGrafanaConfigWithSecret)) require.NoError(t, err) - require.NoError(t, testAutogenFn(nil, nil, 0, &testAutogenRoutes.AlertmanagerConfig, false)) + require.NoError(t, testAutogenFn(nil, nil, 0, &testAutogenRoutes.AlertmanagerConfig, notifier.ErrorOnInvalidReceivers)) cfgWithAutogenRoutes := client.GrafanaAlertmanagerConfig{ TemplateFiles: testAutogenRoutes.TemplateFiles, AlertmanagerConfig: testAutogenRoutes.AlertmanagerConfig, @@ -1420,7 +1420,7 @@ func genAlert(active bool, labels map[string]string) amv2.PostableAlert { } // errAutogenFn is an AutogenFn that always returns an error. -func errAutogenFn(_ context.Context, _ log.Logger, _ int64, _ *definition.PostableApiAlertingConfig, _ bool) error { +func errAutogenFn(_ context.Context, _ log.Logger, _ int64, _ *definition.PostableApiAlertingConfig, _ notifier.InvalidReceiversAction) error { return errTest }