From a652ffa9b48ccddaea087fa91c134b74b22661cf Mon Sep 17 00:00:00 2001
From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com>
Date: Tue, 12 Oct 2021 20:26:00 +0100
Subject: [PATCH] [v8.2.x] Alerting: Fixes a bug when trying to sync broken
 alertmanager config (#40342)

* Alerting: Fixes a bug when trying to sync broken alertmanager config (#40338)

* Alerting: Fixes a bug when trying to sync broken alertmanager config

Broken alertmanager configuration has the potential to be introduced as part of a migration e.g. due to incompatible data between what grafana accepts and what the Alertmanager expects. When this happens, we expect an eventually consistent behaviour where we'll keep trying to apply the configuration until it works.

As part of change in https://github.com/grafana/grafana/pull/39237 we introduced a regression that modified this behaviour and instead tried to create a new Alertmanager for that organization everytime, which eventually ended up in a panic due to a duplicate metrics being registered.

This PR fixes that and introduces a test to catch further regressions.

* Remove disable orgs

(cherry picked from commit 48d73cb14857670226202db6238d81b2c29330bb)

* remove decryptFn that is not known in 8.2 branch

Co-authored-by: gotjosh <josue@grafana.com>
Co-authored-by: Yuriy Tseretyan <yuriy.tseretyan@grafana.com>
---
 .../ngalert/notifier/multiorg_alertmanager.go |  1 +
 .../notifier/multiorg_alertmanager_test.go    | 78 +++++++++++++++++++
 2 files changed, 79 insertions(+)

diff --git a/pkg/services/ngalert/notifier/multiorg_alertmanager.go b/pkg/services/ngalert/notifier/multiorg_alertmanager.go
index ca4e2b74d68..4494df31be7 100644
--- a/pkg/services/ngalert/notifier/multiorg_alertmanager.go
+++ b/pkg/services/ngalert/notifier/multiorg_alertmanager.go
@@ -169,6 +169,7 @@ func (moa *MultiOrgAlertmanager) SyncAlertmanagersForOrgs(ctx context.Context, o
 			if err != nil {
 				moa.logger.Error("unable to create Alertmanager for org", "org", orgID, "err", err)
 			}
+			moa.alertmanagers[orgID] = am
 			alertmanager = am
 		}
 
diff --git a/pkg/services/ngalert/notifier/multiorg_alertmanager_test.go b/pkg/services/ngalert/notifier/multiorg_alertmanager_test.go
index 357892e6e5d..1d2920a2862 100644
--- a/pkg/services/ngalert/notifier/multiorg_alertmanager_test.go
+++ b/pkg/services/ngalert/notifier/multiorg_alertmanager_test.go
@@ -129,6 +129,62 @@ grafana_alerting_discovered_configurations 4
 	}
 }
 
+func TestMultiOrgAlertmanager_SyncAlertmanagersForOrgsWithFailures(t *testing.T) {
+	// Include a broken configuration for organization 2.
+	configStore := &FakeConfigStore{
+		configs: map[int64]*models.AlertConfiguration{
+			2: {AlertmanagerConfiguration: brokenConfig, OrgID: 2},
+		},
+	}
+	orgStore := &FakeOrgStore{
+		orgs: []int64{1, 2, 3},
+	}
+
+	tmpDir, err := ioutil.TempDir("", "test")
+	require.NoError(t, err)
+	kvStore := newFakeKVStore(t)
+	reg := prometheus.NewPedanticRegistry()
+	m := metrics.NewNGAlert(reg)
+	cfg := &setting.Cfg{
+		DataPath: tmpDir,
+		UnifiedAlerting: setting.UnifiedAlertingSettings{
+			AlertmanagerConfigPollInterval: 10 * time.Minute,
+			DefaultConfiguration:           setting.GetAlertmanagerDefaultConfiguration(),
+		}, // do not poll in tests.
+	}
+	mam, err := NewMultiOrgAlertmanager(cfg, configStore, orgStore, kvStore, m.GetMultiOrgAlertmanagerMetrics(), log.New("testlogger"))
+	require.NoError(t, err)
+	ctx := context.Background()
+
+	// When you sync the first time, the alertmanager is created but is doesn't become ready until you have a configuration applied.
+	{
+		require.NoError(t, mam.LoadAndSyncAlertmanagersForOrgs(ctx))
+		require.Len(t, mam.alertmanagers, 3)
+		require.True(t, mam.alertmanagers[1].ready())
+		require.False(t, mam.alertmanagers[2].ready())
+		require.True(t, mam.alertmanagers[3].ready())
+	}
+
+	// On the next sync, it never panics and alertmanager is still not ready.
+	{
+		require.NoError(t, mam.LoadAndSyncAlertmanagersForOrgs(ctx))
+		require.Len(t, mam.alertmanagers, 3)
+		require.True(t, mam.alertmanagers[1].ready())
+		require.False(t, mam.alertmanagers[2].ready())
+		require.True(t, mam.alertmanagers[3].ready())
+	}
+
+	// If we fix the configuration, it becomes ready.
+	{
+		configStore.configs = map[int64]*models.AlertConfiguration{} // It'll apply the default config.
+		require.NoError(t, mam.LoadAndSyncAlertmanagersForOrgs(ctx))
+		require.Len(t, mam.alertmanagers, 3)
+		require.True(t, mam.alertmanagers[1].ready())
+		require.True(t, mam.alertmanagers[2].ready())
+		require.True(t, mam.alertmanagers[3].ready())
+	}
+}
+
 func TestMultiOrgAlertmanager_AlertmanagerFor(t *testing.T) {
 	configStore := &FakeConfigStore{
 		configs: map[int64]*models.AlertConfiguration{},
@@ -195,3 +251,25 @@ func cleanOrgDirectories(path string, t *testing.T) func() {
 		require.NoError(t, os.RemoveAll(path))
 	}
 }
+
+var brokenConfig = `
+	"alertmanager_config": {
+		"route": {
+			"receiver": "grafana-default-email"
+		},
+		"receivers": [{
+			"name": "grafana-default-email",
+			"grafana_managed_receiver_configs": [{
+				"uid": "",
+				"name": "slack receiver",
+				"type": "slack",
+				"isDefault": true,
+				"settings": {
+					"addresses": "<example@email.com>"
+					"url": "�r_��q/b�����p@ⱎȏ =��@ӹtd>Rú�H��           �;�@Uf��0�\k2*jh�}Íu�)"2�F6]�}r��R�b�d�J;��S퓧��$��",
+					"recipient": "#graphana-metrics",
+				}
+			}]
+		}]
+	}
+}`