Alerting: Fix Alertmanager configuration updates (#99610)

* Alerting: Fix Alertmanager configuration updates

Alertmanager configuration updates would behave inconsistently when performing no-op updates with `mysql` as the store.

In particular this bug manifested as a failure to reload the provisioned alertmanager configuration components with no changes to the configuration itself. This would result in a 500 error with mysql store only.

The core issue is that we were relying on the number of rows affected by the update query to determine if the configuration was found in the db or not.
While this behavior works for certain sql dialects, mysql does not return the number of rows matched by the update query but rather the number of rows actually updated.

Also discovered and fixed the mismatched `xorm` tag for the `CreatedAt` field to match the actual column name in the db.

References: https://dev.mysql.com/doc/refman/8.4/en/update.html

pkg/services/ngalert/models/alertmanager.go

@@ -9,7 +9,7 @@ type AlertConfiguration struct {
 	AlertmanagerConfiguration string
 	ConfigurationHash         string
 	ConfigurationVersion      string
-	CreatedAt                 int64 `xorm:"created"`
+	CreatedAt                 int64 `xorm:"created_at"`
 	Default                   bool
 	OrgID                     int64 `xorm:"org_id"`
 }

pkg/services/ngalert/store/alertmanager.go

@@ -110,9 +110,24 @@ func (st DBstore) SaveAlertmanagerConfigurationWithCallback(ctx context.Context,
 // UpdateAlertmanagerConfiguration replaces an alertmanager configuration with optimistic locking. It assumes that an existing revision of the configuration exists in the store, and will return an error otherwise.
 func (st *DBstore) UpdateAlertmanagerConfiguration(ctx context.Context, cmd *models.SaveAlertmanagerConfigurationCmd) error {
 	return st.SQLStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
+		newConfigHash := fmt.Sprintf("%x", md5.Sum([]byte(cmd.AlertmanagerConfiguration)))
+		// check for no-op update
+		if newConfigHash == cmd.FetchedConfigurationHash {
+			// double check that the configuration with this hash is in the db
+			ok, err := sess.Table("alert_configuration").
+				Where("org_id = ? AND configuration_hash = ?", cmd.OrgID, cmd.FetchedConfigurationHash).
+				Exist()
+			if err != nil {
+				return err
+			}
+			if !ok {
+				return ErrVersionLockedObjectNotFound
+			}
+			return nil
+		}
 		config := models.AlertConfiguration{
 			AlertmanagerConfiguration: cmd.AlertmanagerConfiguration,
-			ConfigurationHash:         fmt.Sprintf("%x", md5.Sum([]byte(cmd.AlertmanagerConfiguration))),
+			ConfigurationHash:         newConfigHash,
 			ConfigurationVersion:      cmd.ConfigurationVersion,
 			Default:                   cmd.Default,
 			OrgID:                     cmd.OrgID,

pkg/services/ngalert/store/alertmanager_test.go

@@ -114,6 +114,32 @@ func TestIntegrationAlertmanagerStore(t *testing.T) {
 
 		require.ErrorIs(t, err, ErrVersionLockedObjectNotFound)
 	})
+
+	t.Run("UpdateAlertmanagerConfiguration doesn't update the db if the update is a no-op", func(t *testing.T) {
+		_, configMD5 := setupConfig(t, "my-config", store)
+
+		originalConfig, err := store.GetLatestAlertmanagerConfiguration(context.Background(), 1)
+		require.NoError(t, err)
+		cmd := buildSaveConfigCmd(t, "my-config", 1)
+		cmd.FetchedConfigurationHash = configMD5
+		err = store.UpdateAlertmanagerConfiguration(context.Background(), &cmd)
+		require.NoError(t, err)
+		config, err := store.GetLatestAlertmanagerConfiguration(context.Background(), 1)
+		require.NoError(t, err)
+		require.Equal(t, "my-config", config.AlertmanagerConfiguration)
+		require.Equal(t, configMD5, config.ConfigurationHash)
+		// CreatedAt should not have changed as we didn't touch the config in the DB
+		require.Equal(t, originalConfig.CreatedAt, config.CreatedAt)
+	})
+	t.Run("UpdateAlertmanagerConfiguration fails if the config doesn't exist and the hashes in the cmd match", func(t *testing.T) {
+		configRaw := "my-non-existent-config"
+		configHash := fmt.Sprintf("%x", md5.Sum([]byte(configRaw)))
+		cmd := buildSaveConfigCmd(t, configRaw, 1)
+		cmd.FetchedConfigurationHash = configHash
+		err := store.UpdateAlertmanagerConfiguration(context.Background(), &cmd)
+		require.Error(t, err)
+		require.EqualError(t, err, ErrVersionLockedObjectNotFound.Error())
+	})
 }
 
 func TestIntegrationAlertmanagerHash(t *testing.T) {

pkg/tests/api/alerting/api_provisioning_test.go

@@ -903,6 +903,9 @@ func TestIntegrationExportFileProvision(t *testing.T) {
 			require.Len(t, export.MuteTimings, 1)
 			require.YAMLEq(t, expectedYaml, exportRaw)
 		})
+		t.Run("reloading provisioning should not fail", func(t *testing.T) {
+			apiClient.ReloadAlertingFileProvisioning(t)
+		})
 	})
 }
 
@@ -1004,6 +1007,17 @@ func TestIntegrationExportFileProvisionContactPoints(t *testing.T) {
 			var export definitions.AlertingFileExport
 			require.NoError(t, yaml.Unmarshal([]byte(exportRaw), &export))
 
+			// verify the file exported matches the file provisioned thing
+			require.Len(t, export.ContactPoints, 1)
+			require.YAMLEq(t, string(expectedYaml), exportRaw)
+		})
+		t.Run("reloading provisioning should not change things", func(t *testing.T) {
+			apiClient.ReloadAlertingFileProvisioning(t)
+
+			exportRaw := apiClient.ExportReceiver(t, "cp_1_$escaped", "yaml", true)
+			var export definitions.AlertingFileExport
+			require.NoError(t, yaml.Unmarshal([]byte(exportRaw), &export))
+
 			// verify the file exported matches the file provisioned thing
 			require.Len(t, export.ContactPoints, 1)
 			require.YAMLEq(t, string(expectedYaml), exportRaw)