From e280b949e3526f549928dcbf1f9e7f1794792671 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Thu, 24 Jul 2025 12:00:38 -0400 Subject: [PATCH] Alerting: Refactor remote Alertmanager and Mimir client (#107808) * deduplicate config preparation logic squashes config preparation in 3 places into a single method buildConfiguration * remove copying config from decrypt because we already use copy * move logic from decryptConfiguration to buildConfiguration * move logic from mergeExtraConfigs to buildConfiguration * load default config with buildConfiguration method and skip if fails --------- Co-authored-by: Santiago --- pkg/services/ngalert/remote/alertmanager.go | 173 +++++++----------- .../ngalert/remote/alertmanager_test.go | 5 +- 2 files changed, 69 insertions(+), 109 deletions(-) diff --git a/pkg/services/ngalert/remote/alertmanager.go b/pkg/services/ngalert/remote/alertmanager.go index e1d1621f770..e577f7334e1 100644 --- a/pkg/services/ngalert/remote/alertmanager.go +++ b/pkg/services/ngalert/remote/alertmanager.go @@ -12,17 +12,16 @@ import ( "time" "github.com/go-openapi/strfmt" + alertingClusterPB "github.com/grafana/alerting/cluster/clusterpb" "github.com/grafana/alerting/definition" + alertingModels "github.com/grafana/alerting/models" + alertingNotify "github.com/grafana/alerting/notify" amalert "github.com/prometheus/alertmanager/api/v2/client/alert" amalertgroup "github.com/prometheus/alertmanager/api/v2/client/alertgroup" amgeneral "github.com/prometheus/alertmanager/api/v2/client/general" amsilence "github.com/prometheus/alertmanager/api/v2/client/silence" "github.com/prometheus/client_golang/prometheus" - alertingClusterPB "github.com/grafana/alerting/cluster/clusterpb" - alertingModels "github.com/grafana/alerting/models" - alertingNotify "github.com/grafana/alerting/notify" - "gopkg.in/yaml.v3" "github.com/grafana/grafana/pkg/infra/log" @@ -186,30 +185,12 @@ func NewAlertmanager(ctx context.Context, cfg AlertmanagerConfig, store stateSto return nil, err } - // Parse the default configuration into a postable config. - pCfg, err := notifier.Load([]byte(cfg.DefaultConfig)) - if err != nil { - return nil, err - } - - if err := autogenFn(ctx, logger, cfg.OrgID, &pCfg.AlertmanagerConfig, true); err != nil { - return nil, err - } - - rawCfg, err := json.Marshal(pCfg) - if err != nil { - return nil, err - } - - // Initialize LastReadinessCheck so it's present even if the check fails. - metrics.LastReadinessCheck.Set(0) - - return &Alertmanager{ + am := &Alertmanager{ amClient: amc, autogenFn: autogenFn, crypto: crypto, - defaultConfig: string(rawCfg), - defaultConfigHash: fmt.Sprintf("%x", md5.Sum(rawCfg)), + defaultConfig: cfg.DefaultConfig, + defaultConfigHash: "", // calculated below log: logger, metrics: metrics, mimirClient: mc, @@ -223,7 +204,31 @@ func NewAlertmanager(ctx context.Context, cfg AlertmanagerConfig, store stateSto // TODO: Remove once it can be sent only in the 'smtp_config' field. smtpFrom: cfg.SmtpFrom, - }, nil + } + + // Parse the default configuration once and remember its hash so we can compare it later. + // Known edge case: assigning a default contact point to a rule and setting route overrides + // (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)) + if err != nil { + return fmt.Errorf("unable to build default configuration: %w", err) + } + rawDefaultCfg, err := json.Marshal(defaultCfg) + if err != nil { + return fmt.Errorf("unable to marshal default configuration: %w", err) + } + am.defaultConfigHash = fmt.Sprintf("%x", md5.Sum(rawDefaultCfg)) + return nil + }() + if err != nil { + logger.Error("Unable to calculate hash of the default configuration. Remote Alertmanager will always get isDefault=false", "error", err) + } + // Initialize LastReadinessCheck so it's present even if the check fails. + metrics.LastReadinessCheck.Set(0) + + return am, nil } // ApplyConfig is called by the multi-org Alertmanager on startup and on every sync loop iteration (1m default). @@ -275,31 +280,14 @@ 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 { - c, err := notifier.Load([]byte(config.AlertmanagerConfiguration)) + payload, err := am.buildConfiguration(ctx, []byte(config.AlertmanagerConfiguration)) if err != nil { - return err - } - - // Add auto-generated routes and decrypt before comparing. - if err := am.autogenFn(ctx, am.log, am.orgID, &c.AlertmanagerConfig, true); err != nil { - return err - } - - decryptedCfg, err := am.decryptConfiguration(ctx, c) - if err != nil { - return err - } - - // Decrypt and merge extra configs - payload, err := am.mergeExtraConfigs(ctx, decryptedCfg) - if err != nil { - return fmt.Errorf("unable to merge extra configurations: %w", err) + return fmt.Errorf("unable to build configuration: %w", err) } rawPayload, err := json.Marshal(payload) if err != nil { return fmt.Errorf("unable to marshal decrypted configuration: %w", err) } - configHash := fmt.Sprintf("%x", md5.Sum(rawPayload)) // Send the configuration only if we need to. @@ -314,30 +302,6 @@ func (am *Alertmanager) isDefaultConfiguration(configHash string) bool { return configHash == am.defaultConfigHash } -// decryptConfiguration creates a copy of the configuration, decrypts it, and returns the decrypted configuration alongside its hash. -// Should not be used outside of this package and the specific use case of decrypting the configuration before sending -// it to the remote Alertmanager. -func (am *Alertmanager) decryptConfiguration(ctx context.Context, cfg *apimodels.PostableUserConfig) (*apimodels.PostableUserConfig, error) { - // Create a copy of the configuration to avoid modifying the original - cfgCopy := &apimodels.PostableUserConfig{} - rawCfg, err := json.Marshal(cfg) - if err != nil { - return nil, fmt.Errorf("unable to marshal original configuration: %w", err) - } - if err := json.Unmarshal(rawCfg, cfgCopy); err != nil { - return nil, fmt.Errorf("unable to unmarshal original configuration: %w", err) - } - - // Decrypt the receivers in the configuration. - decryptedReceivers, err := legacy_storage.DecryptedReceivers(cfgCopy.AlertmanagerConfig.Receivers, decrypter(ctx, am.crypto)) - if err != nil { - return nil, fmt.Errorf("unable to decrypt receivers: %w", err) - } - cfgCopy.AlertmanagerConfig.Receivers = decryptedReceivers - - return cfgCopy, nil -} - func decrypter(ctx context.Context, crypto Crypto) models.DecryptFn { return func(value string) (string, error) { decoded, err := base64.StdEncoding.DecodeString(value) @@ -352,31 +316,42 @@ func decrypter(ctx context.Context, crypto Crypto) models.DecryptFn { } } -// mergeExtraConfigs decrypts and applies merged configuration if extra configs exist. -func (am *Alertmanager) mergeExtraConfigs(ctx context.Context, config *apimodels.PostableUserConfig) (remoteClient.GrafanaAlertmanagerConfig, error) { - if len(config.ExtraConfigs) == 0 { - return remoteClient.GrafanaAlertmanagerConfig{ - TemplateFiles: config.TemplateFiles, - AlertmanagerConfig: config.AlertmanagerConfig, - Templates: nil, - }, nil +// 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) (remoteClient.GrafanaAlertmanagerConfig, error) { + c, err := notifier.Load(raw) + if err != nil { + return remoteClient.GrafanaAlertmanagerConfig{}, err } - if err := am.crypto.DecryptExtraConfigs(ctx, config); err != nil { + // Add auto-generated routes and decrypt before comparing. + if err := am.autogenFn(ctx, am.log, am.orgID, &c.AlertmanagerConfig, true); err != nil { + return remoteClient.GrafanaAlertmanagerConfig{}, err + } + + // Decrypt the receivers in the configuration. + decryptedReceivers, err := legacy_storage.DecryptedReceivers(c.AlertmanagerConfig.Receivers, decrypter(ctx, am.crypto)) + if err != nil { + return remoteClient.GrafanaAlertmanagerConfig{}, fmt.Errorf("unable to decrypt receivers: %w", err) + } + c.AlertmanagerConfig.Receivers = decryptedReceivers + + if err := am.crypto.DecryptExtraConfigs(ctx, c); err != nil { return remoteClient.GrafanaAlertmanagerConfig{}, fmt.Errorf("unable to decrypt extra configs: %w", err) } - mergeResult, err := config.GetMergedAlertmanagerConfig() + mergeResult, err := c.GetMergedAlertmanagerConfig() if err != nil { return remoteClient.GrafanaAlertmanagerConfig{}, fmt.Errorf("unable to get merged Alertmanager configuration: %w", err) } - if logctx := mergeResult.LogContext(); len(logctx) > 0 { - am.log.Debug("Configurations merged successfully but some resources were renamed", logctx...) + + var templates []definition.PostableApiTemplate + if len(c.ExtraConfigs) > 0 && len(c.ExtraConfigs[0].TemplateFiles) > 0 { + templates = definition.TemplatesMapToPostableAPITemplates(c.ExtraConfigs[0].TemplateFiles, definition.MimirTemplateKind) } - templates := definition.TemplatesMapToPostableAPITemplates(config.ExtraConfigs[0].TemplateFiles, definition.MimirTemplateKind) + return remoteClient.GrafanaAlertmanagerConfig{ - // TODO keep sending Grafana templates as a map to not break old Mimir - TemplateFiles: config.TemplateFiles, + TemplateFiles: c.TemplateFiles, AlertmanagerConfig: mergeResult.Config, Templates: templates, }, nil @@ -456,19 +431,17 @@ func (am *Alertmanager) SendState(ctx context.Context) error { // SaveAndApplyConfig decrypts and sends a configuration to the remote Alertmanager. func (am *Alertmanager) SaveAndApplyConfig(ctx context.Context, cfg *apimodels.PostableUserConfig) error { - // Add auto-generated routes and decrypt before sending. - if err := am.autogenFn(ctx, am.log, am.orgID, &cfg.AlertmanagerConfig, false); err != nil { - return err - } - decryptedCfg, err := am.decryptConfiguration(ctx, cfg) + // Copy the configuration by marshalling to avoid any mutations to the provided configuration. + rawCopy, err := json.Marshal(cfg) if err != nil { return err } - payload, err := am.mergeExtraConfigs(ctx, decryptedCfg) + payload, err := am.buildConfiguration(ctx, rawCopy) if err != nil { - return fmt.Errorf("unable to merge extra configurations: %w", err) + return fmt.Errorf("unable to build configuration: %w", err) } + rawCfg, err := json.Marshal(payload) if err != nil { return err @@ -480,24 +453,12 @@ 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 { - c, err := notifier.Load([]byte(am.defaultConfig)) + am.log.Debug("Sending default configuration to a remote Alertmanager", "url", am.url) + payload, err := am.buildConfiguration(ctx, []byte(am.defaultConfig)) if err != nil { - return fmt.Errorf("unable to parse the default configuration: %w", err) + return fmt.Errorf("unable to build default configuration: %w", err) } - // Add auto-generated routes and decrypt before sending. - if err := am.autogenFn(ctx, am.log, am.orgID, &c.AlertmanagerConfig, true); err != nil { - return err - } - decryptedCfg, err := am.decryptConfiguration(ctx, c) - if err != nil { - return err - } - - payload := remoteClient.GrafanaAlertmanagerConfig{ - TemplateFiles: c.TemplateFiles, - AlertmanagerConfig: decryptedCfg.AlertmanagerConfig, - } rawCfg, err := json.Marshal(payload) if err != nil { return err diff --git a/pkg/services/ngalert/remote/alertmanager_test.go b/pkg/services/ngalert/remote/alertmanager_test.go index c8d379089cd..bf1d9f0bfbd 100644 --- a/pkg/services/ngalert/remote/alertmanager_test.go +++ b/pkg/services/ngalert/remote/alertmanager_test.go @@ -411,10 +411,9 @@ func TestIntegrationApplyConfig(t *testing.T) { require.Equal(t, 4, configSyncs) require.Equal(t, am.smtp, configSent.SmtpConfig) - // Failing to add the auto-generated routes should result in an error. + // Failing to add the auto-generated routes should not result in an error. _, err = NewAlertmanager(context.Background(), cfg, fstore, notifier.NewCrypto(secretsService, nil, log.NewNopLogger()), errAutogenFn, m, tracing.InitializeTracerForTest()) - require.ErrorIs(t, err, errTest) - require.Equal(t, 4, configSyncs) + require.NoError(t, err, errTest) } func TestCompareAndSendConfiguration(t *testing.T) {