From 894944dcb072617fea871304af030691a8701db3 Mon Sep 17 00:00:00 2001 From: Alexander Akhmetov Date: Thu, 26 Jun 2025 12:39:06 +0200 Subject: [PATCH] Alerting: Refactor remote alertmanager to use Crypto interface (#107228) --- pkg/services/ngalert/ngalert.go | 9 ++--- pkg/services/ngalert/notifier/crypto.go | 5 +++ .../multiorg_alertmanager_remote_test.go | 2 +- pkg/services/ngalert/remote/alertmanager.go | 19 ++++++----- .../ngalert/remote/alertmanager_test.go | 33 +++++++++++-------- 5 files changed, 40 insertions(+), 28 deletions(-) diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index 9939daaa653..ed2709cc995 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -188,6 +188,7 @@ func (ng *AlertNG) init() error { // If toggles for both modes are enabled, remote primary takes precedence. var overrides []notifier.Option moaLogger := log.New("ngalert.multiorg.alertmanager") + crypto := notifier.NewCrypto(ng.SecretsService, ng.store, moaLogger) remotePrimary := ng.FeatureToggles.IsEnabled(initCtx, featuremgmt.FlagAlertmanagerRemotePrimary) remoteSecondary := ng.FeatureToggles.IsEnabled(initCtx, featuremgmt.FlagAlertmanagerRemoteSecondary) if remotePrimary || remoteSecondary { @@ -238,7 +239,7 @@ func (ng *AlertNG) init() error { // Create remote Alertmanager. cfg.OrgID = orgID cfg.PromoteConfig = true - remoteAM, err := createRemoteAlertmanager(ctx, cfg, ng.KVStore, ng.SecretsService.Decrypt, autogenFn, m, ng.tracer) + remoteAM, err := createRemoteAlertmanager(ctx, cfg, ng.KVStore, crypto, autogenFn, m, ng.tracer) if err != nil { moaLogger.Error("Failed to create remote Alertmanager, falling back to using only the internal one", "err", err) return internalAM, nil @@ -263,7 +264,7 @@ func (ng *AlertNG) init() error { // Create remote Alertmanager. cfg.OrgID = orgID - remoteAM, err := createRemoteAlertmanager(ctx, cfg, ng.KVStore, ng.SecretsService.Decrypt, autogenFn, m, ng.tracer) + remoteAM, err := createRemoteAlertmanager(ctx, cfg, ng.KVStore, crypto, autogenFn, m, ng.tracer) if err != nil { moaLogger.Error("Failed to create remote Alertmanager, falling back to using only the internal one", "err", err) return internalAM, nil @@ -716,8 +717,8 @@ func configureHistorianBackend( return nil, fmt.Errorf("unrecognized state history backend: %s", backend) } -func createRemoteAlertmanager(ctx context.Context, cfg remote.AlertmanagerConfig, kvstore kvstore.KVStore, decryptFn remote.DecryptFn, autogenFn remote.AutogenFn, m *metrics.RemoteAlertmanager, tracer tracing.Tracer) (*remote.Alertmanager, error) { - return remote.NewAlertmanager(ctx, cfg, notifier.NewFileStore(cfg.OrgID, kvstore), decryptFn, autogenFn, m, tracer) +func createRemoteAlertmanager(ctx context.Context, cfg remote.AlertmanagerConfig, kvstore kvstore.KVStore, crypto remote.Crypto, autogenFn remote.AutogenFn, m *metrics.RemoteAlertmanager, tracer tracing.Tracer) (*remote.Alertmanager, error) { + return remote.NewAlertmanager(ctx, cfg, notifier.NewFileStore(cfg.OrgID, kvstore), crypto, autogenFn, m, tracer) } func createRecordingWriter(settings setting.RecordingRuleSettings, httpClientProvider httpclient.Provider, datasourceService datasources.DataSourceService, pluginContextProvider *plugincontext.Provider, clock clock.Clock, m *metrics.RemoteWriter) (schedule.RecordingWriter, error) { diff --git a/pkg/services/ngalert/notifier/crypto.go b/pkg/services/ngalert/notifier/crypto.go index 64897566853..76762dffc68 100644 --- a/pkg/services/ngalert/notifier/crypto.go +++ b/pkg/services/ngalert/notifier/crypto.go @@ -18,6 +18,7 @@ import ( type Crypto interface { LoadSecureSettings(ctx context.Context, orgId int64, receivers []*definitions.PostableApiReceiver) error Encrypt(ctx context.Context, payload []byte, opt secrets.EncryptionOptions) ([]byte, error) + Decrypt(ctx context.Context, payload []byte) ([]byte, error) EncryptExtraConfigs(ctx context.Context, config *definitions.PostableUserConfig) error DecryptExtraConfigs(ctx context.Context, config *definitions.PostableUserConfig) error @@ -236,6 +237,10 @@ func (c *alertmanagerCrypto) Encrypt(ctx context.Context, payload []byte, opt se return c.secrets.Encrypt(ctx, payload, opt) } +func (c *alertmanagerCrypto) Decrypt(ctx context.Context, payload []byte) ([]byte, error) { + return c.secrets.Decrypt(ctx, payload) +} + func (c *alertmanagerCrypto) EncryptExtraConfigs(ctx context.Context, config *definitions.PostableUserConfig) error { for i := range config.ExtraConfigs { encryptedValue, err := c.secrets.Encrypt(ctx, []byte(config.ExtraConfigs[i].AlertmanagerConfig), secrets.WithoutScope()) diff --git a/pkg/services/ngalert/notifier/multiorg_alertmanager_remote_test.go b/pkg/services/ngalert/notifier/multiorg_alertmanager_remote_test.go index 930ebe1ad6d..2a378f622d9 100644 --- a/pkg/services/ngalert/notifier/multiorg_alertmanager_remote_test.go +++ b/pkg/services/ngalert/notifier/multiorg_alertmanager_remote_test.go @@ -67,7 +67,7 @@ func TestMultiorgAlertmanager_RemoteSecondaryMode(t *testing.T) { DefaultConfig: setting.GetAlertmanagerDefaultConfiguration(), } m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - remoteAM, err := remote.NewAlertmanager(ctx, externalAMCfg, notifier.NewFileStore(orgID, kvStore), secretsService.Decrypt, remote.NoopAutogenFn, m, tracing.InitializeTracerForTest()) + remoteAM, err := remote.NewAlertmanager(ctx, externalAMCfg, notifier.NewFileStore(orgID, kvStore), notifier.NewCrypto(secretsService, configStore, log.NewNopLogger()), remote.NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) // Use both Alertmanager implementations in the forked Alertmanager. diff --git a/pkg/services/ngalert/remote/alertmanager.go b/pkg/services/ngalert/remote/alertmanager.go index 2a130470c45..006577edb55 100644 --- a/pkg/services/ngalert/remote/alertmanager.go +++ b/pkg/services/ngalert/remote/alertmanager.go @@ -48,12 +48,13 @@ func NoopAutogenFn(_ context.Context, _ log.Logger, _ int64, _ *apimodels.Postab return nil } -// DecryptFn is a function that takes in an encrypted value and returns it decrypted. -type DecryptFn func(ctx context.Context, payload []byte) ([]byte, error) +type Crypto interface { + Decrypt(ctx context.Context, payload []byte) ([]byte, error) +} type Alertmanager struct { autogenFn AutogenFn - decrypt DecryptFn + crypto Crypto defaultConfig string defaultConfigHash string log log.Logger @@ -120,7 +121,7 @@ func (cfg *AlertmanagerConfig) Validate() error { return nil } -func NewAlertmanager(ctx context.Context, cfg AlertmanagerConfig, store stateStore, decryptFn DecryptFn, autogenFn AutogenFn, metrics *metrics.RemoteAlertmanager, tracer tracing.Tracer) (*Alertmanager, error) { +func NewAlertmanager(ctx context.Context, cfg AlertmanagerConfig, store stateStore, crypto Crypto, autogenFn AutogenFn, metrics *metrics.RemoteAlertmanager, tracer tracing.Tracer) (*Alertmanager, error) { if err := cfg.Validate(); err != nil { return nil, err } @@ -204,7 +205,7 @@ func NewAlertmanager(ctx context.Context, cfg AlertmanagerConfig, store stateSto return &Alertmanager{ amClient: amc, autogenFn: autogenFn, - decrypt: decryptFn, + crypto: crypto, defaultConfig: string(rawCfg), defaultConfigHash: fmt.Sprintf("%x", md5.Sum(rawCfg)), log: logger, @@ -318,7 +319,7 @@ func (am *Alertmanager) decryptConfiguration(ctx context.Context, cfg *apimodels } // Decrypt the receivers in the configuration. - decryptedReceivers, err := legacy_storage.DecryptedReceivers(cfgCopy.AlertmanagerConfig.Receivers, decrypter(ctx, am.decrypt)) + decryptedReceivers, err := legacy_storage.DecryptedReceivers(cfgCopy.AlertmanagerConfig.Receivers, decrypter(ctx, am.crypto)) if err != nil { return nil, fmt.Errorf("unable to decrypt receivers: %w", err) } @@ -327,13 +328,13 @@ func (am *Alertmanager) decryptConfiguration(ctx context.Context, cfg *apimodels return cfgCopy, nil } -func decrypter(ctx context.Context, decryptFn DecryptFn) models.DecryptFn { +func decrypter(ctx context.Context, crypto Crypto) models.DecryptFn { return func(value string) (string, error) { decoded, err := base64.StdEncoding.DecodeString(value) if err != nil { return "", err } - decrypted, err := decryptFn(ctx, decoded) + decrypted, err := crypto.Decrypt(ctx, decoded) if err != nil { return "", err } @@ -573,7 +574,7 @@ func (am *Alertmanager) GetReceivers(ctx context.Context) ([]apimodels.Receiver, } func (am *Alertmanager) TestReceivers(ctx context.Context, c apimodels.TestReceiversConfigBodyParams) (*alertingNotify.TestReceiversResult, int, error) { - decryptedReceivers, err := legacy_storage.DecryptedReceivers(c.Receivers, decrypter(ctx, am.decrypt)) + decryptedReceivers, err := legacy_storage.DecryptedReceivers(c.Receivers, decrypter(ctx, am.crypto)) if err != nil { return nil, 0, fmt.Errorf("failed to decrypt receivers: %w", err) } diff --git a/pkg/services/ngalert/remote/alertmanager_test.go b/pkg/services/ngalert/remote/alertmanager_test.go index 2a17e1a3a44..c6a3a06b4d7 100644 --- a/pkg/services/ngalert/remote/alertmanager_test.go +++ b/pkg/services/ngalert/remote/alertmanager_test.go @@ -113,7 +113,7 @@ func TestNewAlertmanager(t *testing.T) { DefaultConfig: defaultGrafanaConfig, } m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - am, err := NewAlertmanager(context.Background(), cfg, nil, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) + am, err := NewAlertmanager(context.Background(), cfg, nil, notifier.NewCrypto(secretsService, nil, log.NewNopLogger()), NoopAutogenFn, m, tracing.InitializeTracerForTest()) if test.expErr != "" { require.EqualError(tt, err, test.expErr) return @@ -206,7 +206,7 @@ func TestApplyConfig(t *testing.T) { // An error response from the remote Alertmanager should result in the readiness check failing. m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - am, err := NewAlertmanager(ctx, cfg, fstore, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) + am, err := NewAlertmanager(ctx, cfg, fstore, notifier.NewCrypto(secretsService, nil, log.NewNopLogger()), NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) config := &ngmodels.AlertConfiguration{ @@ -252,14 +252,14 @@ func TestApplyConfig(t *testing.T) { require.Equal(t, 1, stateSyncs) // After a restart, the Alertmanager shouldn't send the configuration if it has not changed. - am, err = NewAlertmanager(context.Background(), cfg, fstore, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) + am, err = NewAlertmanager(context.Background(), cfg, fstore, notifier.NewCrypto(secretsService, nil, log.NewNopLogger()), NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) require.NoError(t, am.ApplyConfig(ctx, config)) require.Equal(t, 2, configSyncs) // Changing the "from" address should result in the configuration being updated. cfg.SmtpFrom = "new-address@test.com" - am, err = NewAlertmanager(context.Background(), cfg, fstore, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) + am, err = NewAlertmanager(context.Background(), cfg, fstore, notifier.NewCrypto(secretsService, nil, log.NewNopLogger()), NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) require.NoError(t, am.ApplyConfig(ctx, config)) require.Equal(t, 3, configSyncs) @@ -277,14 +277,14 @@ func TestApplyConfig(t *testing.T) { StaticHeaders: map[string]string{"test": "true"}, User: "Test User", } - am, err = NewAlertmanager(context.Background(), cfg, fstore, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) + am, err = NewAlertmanager(context.Background(), cfg, fstore, notifier.NewCrypto(secretsService, nil, log.NewNopLogger()), NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) require.NoError(t, am.ApplyConfig(ctx, config)) require.Equal(t, 4, configSyncs) require.Equal(t, am.smtp, configSent.SmtpConfig) // Failing to add the auto-generated routes should result in an error. - _, err = NewAlertmanager(context.Background(), cfg, fstore, secretsService.Decrypt, errAutogenFn, m, tracing.InitializeTracerForTest()) + _, err = NewAlertmanager(context.Background(), cfg, fstore, notifier.NewCrypto(secretsService, nil, log.NewNopLogger()), errAutogenFn, m, tracing.InitializeTracerForTest()) require.ErrorIs(t, err, errTest) require.Equal(t, 4, configSyncs) } @@ -293,6 +293,8 @@ func TestCompareAndSendConfiguration(t *testing.T) { const tenantID = "test" secretsService := secretsManager.SetupTestService(t, database.ProvideSecretsStore(db.InitTestDB(t))) + testCrypto := notifier.NewCrypto(secretsService, nil, log.NewNopLogger()) + var got string server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { require.Equal(t, tenantID, r.Header.Get(client.MimirTenantHeader)) @@ -420,7 +422,7 @@ func TestCompareAndSendConfiguration(t *testing.T) { am, err := NewAlertmanager(ctx, cfg, fstore, - secretsService.Decrypt, + testCrypto, NoopAutogenFn, m, tracing.InitializeTracerForTest(), @@ -452,6 +454,9 @@ func TestCompareAndSendConfiguration(t *testing.T) { func Test_TestReceiversDecryptsSecureSettings(t *testing.T) { const tenantID = "test" secretsService := secretsManager.SetupTestService(t, database.ProvideSecretsStore(db.InitTestDB(t))) + + testCrypto := notifier.NewCrypto(secretsService, nil, log.NewNopLogger()) + var got apimodels.TestReceiversConfigBodyParams server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { require.Equal(t, tenantID, r.Header.Get(client.MimirTenantHeader)) @@ -475,7 +480,7 @@ func Test_TestReceiversDecryptsSecureSettings(t *testing.T) { am, err := NewAlertmanager(context.Background(), cfg, fstore, - secretsService.Decrypt, + testCrypto, NoopAutogenFn, m, tracing.InitializeTracerForTest(), @@ -596,7 +601,7 @@ func TestIntegrationRemoteAlertmanagerConfiguration(t *testing.T) { secretsService := secretsManager.SetupTestService(t, database.ProvideSecretsStore(db.InitTestDB(t))) m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - am, err := NewAlertmanager(ctx, cfg, fstore, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) + am, err := NewAlertmanager(ctx, cfg, fstore, notifier.NewCrypto(secretsService, nil, log.NewNopLogger()), NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) encodedFullState, err := am.getFullState(ctx) @@ -764,7 +769,7 @@ func TestIntegrationRemoteAlertmanagerGetStatus(t *testing.T) { ctx := context.Background() secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - am, err := NewAlertmanager(ctx, cfg, nil, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) + am, err := NewAlertmanager(ctx, cfg, nil, notifier.NewCrypto(secretsService, nil, log.NewNopLogger()), NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) // We should get the default Cloud Alertmanager configuration. @@ -798,7 +803,7 @@ func TestIntegrationRemoteAlertmanagerSilences(t *testing.T) { ctx := context.Background() secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - am, err := NewAlertmanager(ctx, cfg, nil, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) + am, err := NewAlertmanager(ctx, cfg, nil, notifier.NewCrypto(secretsService, nil, log.NewNopLogger()), NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) // We should have no silences at first. @@ -884,7 +889,7 @@ func TestIntegrationRemoteAlertmanagerAlerts(t *testing.T) { ctx := context.Background() secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - am, err := NewAlertmanager(ctx, cfg, nil, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) + am, err := NewAlertmanager(ctx, cfg, nil, notifier.NewCrypto(secretsService, nil, log.NewNopLogger()), NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) // Wait until the Alertmanager is ready to send alerts. @@ -962,7 +967,7 @@ func TestIntegrationRemoteAlertmanagerReceivers(t *testing.T) { ctx := context.Background() secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - am, err := NewAlertmanager(ctx, cfg, nil, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) + am, err := NewAlertmanager(ctx, cfg, nil, notifier.NewCrypto(secretsService, nil, log.NewNopLogger()), NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) // We should start with the default config. @@ -1001,7 +1006,7 @@ func TestIntegrationRemoteAlertmanagerTestTemplates(t *testing.T) { ctx := context.Background() secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - am, err := NewAlertmanager(ctx, cfg, nil, secretsService.Decrypt, NoopAutogenFn, m, tracing.InitializeTracerForTest()) + am, err := NewAlertmanager(ctx, cfg, nil, notifier.NewCrypto(secretsService, nil, log.NewNopLogger()), NoopAutogenFn, m, tracing.InitializeTracerForTest()) require.NoError(t, err) // Valid template