diff --git a/pkg/services/ngalert/models/alertmanager.go b/pkg/services/ngalert/models/alertmanager.go index 0b69f682c8a..98e86587625 100644 --- a/pkg/services/ngalert/models/alertmanager.go +++ b/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"` } diff --git a/pkg/services/ngalert/store/alertmanager.go b/pkg/services/ngalert/store/alertmanager.go index fc07e7a5f35..33e62f46fb9 100644 --- a/pkg/services/ngalert/store/alertmanager.go +++ b/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, diff --git a/pkg/services/ngalert/store/alertmanager_test.go b/pkg/services/ngalert/store/alertmanager_test.go index 4433760c8e1..4f27e2b6e83 100644 --- a/pkg/services/ngalert/store/alertmanager_test.go +++ b/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) { diff --git a/pkg/tests/api/alerting/api_provisioning_test.go b/pkg/tests/api/alerting/api_provisioning_test.go index ec9cda827b8..f4bc4dab890 100644 --- a/pkg/tests/api/alerting/api_provisioning_test.go +++ b/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)