From a80f04c94933e99e9d1afb2e2b3cb92c3660dea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Philippe=20Qu=C3=A9m=C3=A9ner?= Date: Wed, 23 Mar 2022 09:31:46 +0100 Subject: [PATCH] Alerting: add collision safe update function for alertmanager configurations (#46692) * Alerting: add collision safe update function for alertmanager configurations * fix typo * use bootstrap func for tests * move hash calculation to store * remove icons lol * remove removed field --- pkg/services/ngalert/models/alertmanager.go | 2 + pkg/services/ngalert/notifier/testing.go | 17 ++++ pkg/services/ngalert/store/alertmanager.go | 33 ++++++++ .../ngalert/store/alertmanager_test.go | 84 +++++++++++++++++++ pkg/services/ngalert/store/database.go | 1 + .../sqlstore/migrations/ualert/tables.go | 4 + 6 files changed, 141 insertions(+) create mode 100644 pkg/services/ngalert/store/alertmanager_test.go diff --git a/pkg/services/ngalert/models/alertmanager.go b/pkg/services/ngalert/models/alertmanager.go index b8f299750f2..89b35b6b530 100644 --- a/pkg/services/ngalert/models/alertmanager.go +++ b/pkg/services/ngalert/models/alertmanager.go @@ -7,6 +7,7 @@ type AlertConfiguration struct { ID int64 `xorm:"pk autoincr 'id'"` AlertmanagerConfiguration string + ConfigurationHash string ConfigurationVersion string CreatedAt int64 `xorm:"created"` Default bool @@ -22,6 +23,7 @@ type GetLatestAlertmanagerConfigurationQuery struct { // SaveAlertmanagerConfigurationCmd is the command to save an alertmanager configuration. type SaveAlertmanagerConfigurationCmd struct { AlertmanagerConfiguration string + FetchedConfigurationHash string ConfigurationVersion string Default bool OrgID int64 diff --git a/pkg/services/ngalert/notifier/testing.go b/pkg/services/ngalert/notifier/testing.go index e89ce32e6a3..2c11dc4a9c4 100644 --- a/pkg/services/ngalert/notifier/testing.go +++ b/pkg/services/ngalert/notifier/testing.go @@ -2,6 +2,9 @@ package notifier import ( "context" + "crypto/md5" + "errors" + "fmt" "strings" "sync" "testing" @@ -67,6 +70,20 @@ func (f *FakeConfigStore) SaveAlertmanagerConfigurationWithCallback(_ context.Co return nil } +func (f *FakeConfigStore) UpdateAlertManagerConfiguration(cmd *models.SaveAlertmanagerConfigurationCmd) error { + if config, exists := f.configs[cmd.OrgID]; exists && config.ConfigurationHash == cmd.FetchedConfigurationHash { + f.configs[cmd.OrgID] = &models.AlertConfiguration{ + AlertmanagerConfiguration: cmd.AlertmanagerConfiguration, + OrgID: cmd.OrgID, + ConfigurationHash: fmt.Sprintf("%x", md5.Sum([]byte(cmd.AlertmanagerConfiguration))), + ConfigurationVersion: "v1", + Default: cmd.Default, + } + return nil + } + return errors.New("config not found or hash not valid") +} + type FakeOrgStore struct { orgs []int64 } diff --git a/pkg/services/ngalert/store/alertmanager.go b/pkg/services/ngalert/store/alertmanager.go index 0edc09b7974..f475d3e1877 100644 --- a/pkg/services/ngalert/store/alertmanager.go +++ b/pkg/services/ngalert/store/alertmanager.go @@ -2,6 +2,7 @@ package store import ( "context" + "crypto/md5" "fmt" "xorm.io/builder" @@ -13,6 +14,9 @@ import ( var ( // ErrNoAlertmanagerConfiguration is an error for when no alertmanager configuration is found. ErrNoAlertmanagerConfiguration = fmt.Errorf("could not find an Alertmanager configuration") + // ErrVersionLockedObjectNotFound is returned when an object is not + // found using the current hash. + ErrVersionLockedObjectNotFound = fmt.Errorf("could not find object using provided id and hash") ) // GetLatestAlertmanagerConfiguration returns the lastest version of the alertmanager configuration. @@ -64,6 +68,7 @@ func (st DBstore) SaveAlertmanagerConfigurationWithCallback(ctx context.Context, return st.SQLStore.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error { config := models.AlertConfiguration{ AlertmanagerConfiguration: cmd.AlertmanagerConfiguration, + ConfigurationHash: fmt.Sprintf("%x", md5.Sum([]byte(cmd.AlertmanagerConfiguration))), ConfigurationVersion: cmd.ConfigurationVersion, Default: cmd.Default, OrgID: cmd.OrgID, @@ -79,3 +84,31 @@ func (st DBstore) SaveAlertmanagerConfigurationWithCallback(ctx context.Context, return nil }) } + +func (st *DBstore) UpdateAlertManagerConfiguration(cmd *models.SaveAlertmanagerConfigurationCmd) error { + return st.SQLStore.WithTransactionalDbSession(context.Background(), func(sess *sqlstore.DBSession) error { + config := models.AlertConfiguration{ + AlertmanagerConfiguration: cmd.AlertmanagerConfiguration, + ConfigurationHash: fmt.Sprintf("%x", md5.Sum([]byte(cmd.AlertmanagerConfiguration))), + ConfigurationVersion: cmd.ConfigurationVersion, + Default: cmd.Default, + OrgID: cmd.OrgID, + } + rows, err := sess.Table("alert_configuration").Where(` + EXISTS ( + SELECT 1 + FROM alert_configuration + WHERE + org_id = ? + AND + id = (SELECT MAX(id) FROM alert_configuration WHERE org_id = ?) + AND + configuration_hash = ? + )`, + cmd.OrgID, cmd.OrgID, cmd.FetchedConfigurationHash).Insert(config) + if rows == 0 { + return ErrVersionLockedObjectNotFound + } + return err + }) +} diff --git a/pkg/services/ngalert/store/alertmanager_test.go b/pkg/services/ngalert/store/alertmanager_test.go new file mode 100644 index 00000000000..89d02ab04e6 --- /dev/null +++ b/pkg/services/ngalert/store/alertmanager_test.go @@ -0,0 +1,84 @@ +//go:build integration +// +build integration + +package store + +import ( + "context" + "crypto/md5" + "fmt" + "testing" + + "github.com/grafana/grafana/pkg/services/ngalert/models" + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/stretchr/testify/require" +) + +func TestAlertManagerHash(t *testing.T) { + sqlStore := sqlstore.InitTestDB(t) + store := &DBstore{ + SQLStore: sqlStore, + } + setupConfig := func(t *testing.T, config string) (string, string) { + config, configMD5 := config, fmt.Sprintf("%x", md5.Sum([]byte(config))) + err := store.SaveAlertmanagerConfiguration(context.Background(), &models.SaveAlertmanagerConfigurationCmd{ + AlertmanagerConfiguration: config, + ConfigurationVersion: "v1", + Default: false, + OrgID: 1, + }) + require.NoError(t, err) + return config, configMD5 + } + t.Run("After saving the DB should return the right hash", func(t *testing.T) { + _, configMD5 := setupConfig(t, "my-config") + req := &models.GetLatestAlertmanagerConfigurationQuery{ + OrgID: 1, + } + err := store.GetLatestAlertmanagerConfiguration(context.Background(), req) + require.NoError(t, err) + require.Equal(t, configMD5, req.Result.ConfigurationHash) + }) + + t.Run("When passing the right hash the config should be updated", func(t *testing.T) { + _, configMD5 := setupConfig(t, "my-config") + req := &models.GetLatestAlertmanagerConfigurationQuery{ + OrgID: 1, + } + err := store.GetLatestAlertmanagerConfiguration(context.Background(), req) + require.NoError(t, err) + require.Equal(t, configMD5, req.Result.ConfigurationHash) + newConfig, newConfigMD5 := "my-config-new", fmt.Sprintf("%x", md5.Sum([]byte("my-config-new"))) + err = store.UpdateAlertManagerConfiguration(&models.SaveAlertmanagerConfigurationCmd{ + AlertmanagerConfiguration: newConfig, + FetchedConfigurationHash: configMD5, + ConfigurationVersion: "v1", + Default: false, + OrgID: 1, + }) + require.NoError(t, err) + err = store.GetLatestAlertmanagerConfiguration(context.Background(), req) + require.NoError(t, err) + require.Equal(t, newConfig, req.Result.AlertmanagerConfiguration) + require.Equal(t, newConfigMD5, req.Result.ConfigurationHash) + }) + + t.Run("When passing the wrong hash the update should error", func(t *testing.T) { + config, configMD5 := setupConfig(t, "my-config") + req := &models.GetLatestAlertmanagerConfigurationQuery{ + OrgID: 1, + } + err := store.GetLatestAlertmanagerConfiguration(context.Background(), req) + require.NoError(t, err) + require.Equal(t, configMD5, req.Result.ConfigurationHash) + err = store.UpdateAlertManagerConfiguration(&models.SaveAlertmanagerConfigurationCmd{ + AlertmanagerConfiguration: config, + FetchedConfigurationHash: "the-wrong-hash", + ConfigurationVersion: "v1", + Default: false, + OrgID: 1, + }) + require.Error(t, err) + require.EqualError(t, ErrVersionLockedObjectNotFound, err.Error()) + }) +} diff --git a/pkg/services/ngalert/store/database.go b/pkg/services/ngalert/store/database.go index aa0e601d632..0ff8ea92eba 100644 --- a/pkg/services/ngalert/store/database.go +++ b/pkg/services/ngalert/store/database.go @@ -22,6 +22,7 @@ type AlertingStore interface { GetAllLatestAlertmanagerConfiguration(ctx context.Context) ([]*models.AlertConfiguration, error) SaveAlertmanagerConfiguration(ctx context.Context, cmd *models.SaveAlertmanagerConfigurationCmd) error SaveAlertmanagerConfigurationWithCallback(ctx context.Context, cmd *models.SaveAlertmanagerConfigurationCmd, callback SaveCallback) error + UpdateAlertManagerConfiguration(cmd *models.SaveAlertmanagerConfigurationCmd) error } // DBstore stores the alert definitions and instances in the database. diff --git a/pkg/services/sqlstore/migrations/ualert/tables.go b/pkg/services/sqlstore/migrations/ualert/tables.go index e3f42b6057a..3adbba48d99 100644 --- a/pkg/services/sqlstore/migrations/ualert/tables.go +++ b/pkg/services/sqlstore/migrations/ualert/tables.go @@ -306,6 +306,10 @@ func AddAlertmanagerConfigMigrations(mg *migrator.Migrator) { mg.AddMigration("add index in alert_configuration table on org_id column", migrator.NewAddIndexMigration(alertConfiguration, &migrator.Index{ Cols: []string{"org_id"}, })) + + mg.AddMigration("add configuration_hash column to alert_configuration", migrator.NewAddColumnMigration(alertConfiguration, &migrator.Column{ + Name: "configuration_hash", Type: migrator.DB_Varchar, Nullable: false, Default: "'not-yet-calculated'", Length: 32, + })) } func AddAlertAdminConfigMigrations(mg *migrator.Migrator) {