From c8d92ee06aa50666a92d3b41f7d2da4147719084 Mon Sep 17 00:00:00 2001
From: Yuri Tseretyan <yuriy.tseretyan@grafana.com>
Date: Tue, 6 May 2025 09:36:22 -0400
Subject: [PATCH] Alerting: Refactor applyConfig in Alertmanager (#104970)

* refactor: remove applyAndMarkConfig

---------

Signed-off-by: Yuri Tseretyan <yuriy.tseretyan@grafana.com>
---
 pkg/services/ngalert/notifier/alertmanager.go | 66 +++++++------------
 1 file changed, 24 insertions(+), 42 deletions(-)

diff --git a/pkg/services/ngalert/notifier/alertmanager.go b/pkg/services/ngalert/notifier/alertmanager.go
index 01b829d6c99..2392de1df21 100644
--- a/pkg/services/ngalert/notifier/alertmanager.go
+++ b/pkg/services/ngalert/notifier/alertmanager.go
@@ -191,13 +191,7 @@ func (am *alertmanager) SaveAndApplyDefaultConfig(ctx context.Context) error {
 		}
 
 		err = am.Store.SaveAlertmanagerConfigurationWithCallback(ctx, cmd, func() error {
-			if am.withAutogen {
-				err := AddAutogenConfig(ctx, am.logger, am.Store, am.orgID, &cfg.AlertmanagerConfig, true)
-				if err != nil {
-					return err
-				}
-			}
-			_, err = am.applyConfig(cfg)
+			_, err = am.applyConfig(ctx, cfg, true)
 			return err
 		})
 		if err != nil {
@@ -230,14 +224,7 @@ func (am *alertmanager) SaveAndApplyConfig(ctx context.Context, cfg *apimodels.P
 		}
 
 		err = am.Store.SaveAlertmanagerConfigurationWithCallback(ctx, cmd, func() error {
-			if am.withAutogen {
-				err := AddAutogenConfig(ctx, am.logger, am.Store, am.orgID, &cfg.AlertmanagerConfig, false)
-				if err != nil {
-					return err
-				}
-			}
-
-			_, err = am.applyConfig(cfg)
+			_, err = am.applyConfig(ctx, cfg, false) // fail if the autogen config is invalid
 			return err
 		})
 		if err != nil {
@@ -259,21 +246,27 @@ func (am *alertmanager) ApplyConfig(ctx context.Context, dbCfg *ngmodels.AlertCo
 
 	var outerErr error
 	am.Base.WithLock(func() {
-		if am.withAutogen {
-			err := AddAutogenConfig(ctx, am.logger, am.Store, am.orgID, &cfg.AlertmanagerConfig, true)
-			if err != nil {
-				outerErr = err
-				return
-			}
-		}
 		// Note: Adding the autogen config here causes alert_configuration_history to update last_applied more often.
 		// 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.
-		if err := am.applyAndMarkConfig(ctx, dbCfg.ConfigurationHash, cfg); err != nil {
+		configChanged, err := am.applyConfig(ctx, cfg, true)
+		if err != nil {
 			outerErr = fmt.Errorf("unable to apply configuration: %w", err)
 			return
 		}
+
+		if !configChanged {
+			return
+		}
+		markConfigCmd := ngmodels.MarkConfigurationAsAppliedCmd{
+			OrgID:             am.orgID,
+			ConfigurationHash: dbCfg.ConfigurationHash,
+		}
+		err = am.Store.MarkConfigurationAsApplied(ctx, &markConfigCmd)
+		if err != nil {
+			outerErr = fmt.Errorf("unable to mark configuration as applied: %w", err)
+		}
 	})
 
 	return outerErr
@@ -328,7 +321,14 @@ 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(cfg *apimodels.PostableUserConfig) (bool, error) {
+func (am *alertmanager) applyConfig(ctx context.Context, cfg *apimodels.PostableUserConfig, skipInvalid bool) (bool, error) {
+	if am.withAutogen {
+		err := AddAutogenConfig(ctx, am.logger, am.Store, am.orgID, &cfg.AlertmanagerConfig, skipInvalid)
+		if err != nil {
+			return false, err
+		}
+	}
+
 	// First, let's make sure this config is not already loaded
 	rawConfig, err := json.Marshal(cfg)
 	if err != nil {
@@ -363,24 +363,6 @@ func (am *alertmanager) applyConfig(cfg *apimodels.PostableUserConfig) (bool, er
 	return true, nil
 }
 
-// applyAndMarkConfig applies a configuration and marks it as applied if no errors occur.
-func (am *alertmanager) applyAndMarkConfig(ctx context.Context, hash string, cfg *apimodels.PostableUserConfig) error {
-	configChanged, err := am.applyConfig(cfg)
-	if err != nil {
-		return err
-	}
-
-	if configChanged {
-		markConfigCmd := ngmodels.MarkConfigurationAsAppliedCmd{
-			OrgID:             am.orgID,
-			ConfigurationHash: hash,
-		}
-		return am.Store.MarkConfigurationAsApplied(ctx, &markConfigCmd)
-	}
-
-	return nil
-}
-
 func (am *alertmanager) AppURL() string {
 	return am.Settings.AppURL
 }