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 <santiagohernandez.1997@gmail.com>

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

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) {