Alerting: Fix contact point testing with secure settings (#72235)

* Alerting: Fix contact point testing with secure settings

Fixes double encryption of secure settings during contact point testing and removes code duplication
that helped cause the drift between alertmanager and test endpoint. Also adds integration tests to cover
the regression.

Note: provisioningStore is created to remove cycle and the unnecessary dependency.

pkg/services/ngalert/api/api_alertmanager.go

@@ -19,7 +19,6 @@ import (
 	apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
 	"github.com/grafana/grafana/pkg/services/ngalert/notifier"
 	"github.com/grafana/grafana/pkg/services/ngalert/store"
-	"github.com/grafana/grafana/pkg/services/secrets"
 	"github.com/grafana/grafana/pkg/util"
 )
 
@@ -302,17 +301,11 @@ func (srv AlertmanagerSrv) RouteGetReceivers(c *contextmodel.ReqContext) respons
 }
 
 func (srv AlertmanagerSrv) RoutePostTestReceivers(c *contextmodel.ReqContext, body apimodels.TestReceiversConfigBodyParams) response.Response {
-	if err := srv.crypto.LoadSecureSettings(c.Req.Context(), c.OrgID, body.Receivers); err != nil {
+	if err := srv.crypto.ProcessSecureSettings(c.Req.Context(), c.OrgID, body.Receivers); err != nil {
 		var unknownReceiverError UnknownReceiverError
 		if errors.As(err, &unknownReceiverError) {
 			return ErrResp(http.StatusBadRequest, err, "")
 		}
-		return ErrResp(http.StatusInternalServerError, err, "")
-	}
-
-	if err := body.ProcessConfig(func(ctx context.Context, payload []byte) ([]byte, error) {
-		return srv.crypto.Encrypt(ctx, payload, secrets.WithoutScope())
-	}); err != nil {
 		return ErrResp(http.StatusInternalServerError, err, "failed to post process Alertmanager configuration")
 	}
 

pkg/services/ngalert/api/api_provisioning_test.go

@@ -1170,7 +1170,7 @@ func createTestEnv(t *testing.T, testConfig string) testEnvironment {
 	// Encrypt secure settings.
 	c, err := notifier.Load([]byte(testConfig))
 	require.NoError(t, err)
-	err = c.EncryptConfig(func(ctx context.Context, payload []byte) ([]byte, error) {
+	err = notifier.EncryptReceiverConfigs(c.AlertmanagerConfig.Receivers, func(ctx context.Context, payload []byte) ([]byte, error) {
 		return secretsService.Encrypt(ctx, payload, secrets.WithoutScope())
 	})
 	require.NoError(t, err)

pkg/services/ngalert/api/tooling/definitions/alertmanager.go

@@ -2,7 +2,6 @@ package definitions
 
 import (
 	"context"
-	"encoding/base64"
 	"encoding/json"
 	"fmt"
 	"reflect"
@@ -16,8 +15,6 @@ import (
 	"github.com/prometheus/alertmanager/pkg/labels"
 	"github.com/prometheus/common/model"
 	"gopkg.in/yaml.v3"
-
-	"github.com/grafana/grafana/pkg/util"
 )
 
 // swagger:route POST /api/alertmanager/grafana/config/api/v1/alerts alertmanager RoutePostGrafanaAlertingConfig
@@ -276,14 +273,6 @@ type TestReceiversConfigBodyParams struct {
 	Receivers []*PostableApiReceiver          `yaml:"receivers,omitempty" json:"receivers,omitempty"`
 }
 
-func (c *TestReceiversConfigBodyParams) ProcessConfig(encrypt EncryptFn) error {
-	err := encryptReceiverConfigs(c.Receivers, encrypt)
-	if err != nil {
-		return err
-	}
-	return assignReceiverConfigsUIDs(c.Receivers)
-}
-
 type TestReceiversConfigAlertParams struct {
 	Annotations model.LabelSet `yaml:"annotations,omitempty" json:"annotations,omitempty"`
 	Labels      model.LabelSet `yaml:"labels,omitempty" json:"labels,omitempty"`
@@ -619,16 +608,6 @@ func (c *PostableUserConfig) GetGrafanaReceiverMap() map[string]*PostableGrafana
 	return UIDs
 }
 
-// EncryptConfig parses grafana receivers and encrypts secrets.
-func (c *PostableUserConfig) EncryptConfig(encrypt EncryptFn) error {
-	return encryptReceiverConfigs(c.AlertmanagerConfig.Receivers, encrypt)
-}
-
-// AssignMissingConfigUIDs assigns missing UUIDs to receiver configs.
-func (c *PostableUserConfig) AssignMissingConfigUIDs() error {
-	return assignReceiverConfigsUIDs(c.AlertmanagerConfig.Receivers)
-}
-
 // MarshalYAML implements yaml.Marshaller.
 func (c *PostableUserConfig) MarshalYAML() (interface{}, error) {
 	yml, err := yaml.Marshal(c.amSimple)
@@ -1294,55 +1273,6 @@ type PostableGrafanaReceivers struct {
 
 type EncryptFn func(ctx context.Context, payload []byte) ([]byte, error)
 
-func encryptReceiverConfigs(c []*PostableApiReceiver, encrypt EncryptFn) error {
-	// encrypt secure settings for storing them in DB
-	for _, r := range c {
-		switch r.Type() {
-		case GrafanaReceiverType:
-			for _, gr := range r.PostableGrafanaReceivers.GrafanaManagedReceivers {
-				for k, v := range gr.SecureSettings {
-					encryptedData, err := encrypt(context.Background(), []byte(v))
-					if err != nil {
-						return fmt.Errorf("failed to encrypt secure settings: %w", err)
-					}
-					gr.SecureSettings[k] = base64.StdEncoding.EncodeToString(encryptedData)
-				}
-			}
-		default:
-		}
-	}
-	return nil
-}
-
-func assignReceiverConfigsUIDs(c []*PostableApiReceiver) error {
-	seenUIDs := make(map[string]struct{})
-	// encrypt secure settings for storing them in DB
-	for _, r := range c {
-		switch r.Type() {
-		case GrafanaReceiverType:
-			for _, gr := range r.PostableGrafanaReceivers.GrafanaManagedReceivers {
-				if gr.UID == "" {
-					retries := 5
-					for i := 0; i < retries; i++ {
-						gen := util.GenerateShortUID()
-						_, ok := seenUIDs[gen]
-						if !ok {
-							gr.UID = gen
-							break
-						}
-					}
-					if gr.UID == "" {
-						return fmt.Errorf("all %d attempts to generate UID for receiver have failed; please retry", retries)
-					}
-				}
-				seenUIDs[gr.UID] = struct{}{}
-			}
-		default:
-		}
-	}
-	return nil
-}
-
 // ObjectMatchers is Matchers with a different Unmarshal and Marshal methods that accept matchers as objects
 // that have already been parsed.
 type ObjectMatchers labels.Matchers

pkg/services/ngalert/notifier/alertmanager_config.go

@@ -7,10 +7,11 @@ import (
 	"time"
 
 	"github.com/go-openapi/strfmt"
+
 	"github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/store"
-	"github.com/grafana/grafana/pkg/services/secrets"
+	"github.com/grafana/grafana/pkg/util"
 )
 
 type UnknownReceiverError struct {
@@ -166,22 +167,12 @@ func (moa *MultiOrgAlertmanager) ApplyAlertmanagerConfiguration(ctx context.Cont
 		}
 	}
 
-	// First, we encrypt the new or updated secure settings. Then, we load the existing secure settings from the database
-	// and add back any that weren't updated.
-	// We perform these steps in this order to ensure the hash of the secure settings remains stable when no secure
-	// settings were modified.
-	if err := config.EncryptConfig(func(ctx context.Context, payload []byte) ([]byte, error) {
-		return moa.Crypto.Encrypt(ctx, payload, secrets.WithoutScope())
-	}); err != nil {
+	if err := moa.Crypto.ProcessSecureSettings(ctx, org, config.AlertmanagerConfig.Receivers); err != nil {
 		return fmt.Errorf("failed to post process Alertmanager configuration: %w", err)
 	}
 
-	if err := moa.Crypto.LoadSecureSettings(ctx, org, config.AlertmanagerConfig.Receivers); err != nil {
-		return err
-	}
-
-	if err := config.AssignMissingConfigUIDs(); err != nil {
-		return fmt.Errorf("failed to post process Alertmanager configuration: %w", err)
+	if err := assignReceiverConfigsUIDs(config.AlertmanagerConfig.Receivers); err != nil {
+		return fmt.Errorf("failed to assign missing uids: %w", err)
 	}
 
 	am, err := moa.AlertmanagerFor(org)
@@ -200,6 +191,43 @@ func (moa *MultiOrgAlertmanager) ApplyAlertmanagerConfiguration(ctx context.Cont
 	return nil
 }
 
+// assignReceiverConfigsUIDs assigns missing UUIDs to receiver configs.
+func assignReceiverConfigsUIDs(c []*definitions.PostableApiReceiver) error {
+	seenUIDs := make(map[string]struct{})
+	// encrypt secure settings for storing them in DB
+	for _, r := range c {
+		switch r.Type() {
+		case definitions.GrafanaReceiverType:
+			for _, gr := range r.PostableGrafanaReceivers.GrafanaManagedReceivers {
+				if gr.UID == "" {
+					retries := 5
+					for i := 0; i < retries; i++ {
+						gen := util.GenerateShortUID()
+						_, ok := seenUIDs[gen]
+						if !ok {
+							gr.UID = gen
+							break
+						}
+					}
+					if gr.UID == "" {
+						return fmt.Errorf("all %d attempts to generate UID for receiver have failed; please retry", retries)
+					}
+				}
+				seenUIDs[gr.UID] = struct{}{}
+			}
+		default:
+		}
+	}
+	return nil
+}
+
+type provisioningStore interface {
+	GetProvenance(ctx context.Context, o models.Provisionable, org int64) (models.Provenance, error)
+	GetProvenances(ctx context.Context, org int64, resourceType string) (map[string]models.Provenance, error)
+	SetProvenance(ctx context.Context, o models.Provisionable, org int64, p models.Provenance) error
+	DeleteProvenance(ctx context.Context, o models.Provisionable, org int64) error
+}
+
 func (moa *MultiOrgAlertmanager) mergeProvenance(ctx context.Context, config definitions.GettableUserConfig, org int64) (definitions.GettableUserConfig, error) {
 	if config.AlertmanagerConfig.Route != nil {
 		provenance, err := moa.ProvStore.GetProvenance(ctx, config.AlertmanagerConfig.Route, org)

pkg/services/ngalert/notifier/crypto.go

@@ -19,6 +19,7 @@ type Crypto interface {
 	Encrypt(ctx context.Context, payload []byte, opt secrets.EncryptionOptions) ([]byte, error)
 
 	getDecryptedSecret(r *definitions.PostableGrafanaReceiver, key string) (string, error)
+	ProcessSecureSettings(ctx context.Context, orgId int64, recvs []*definitions.PostableApiReceiver) error
 }
 
 // alertmanagerCrypto implements decryption of Alertmanager configuration and encryption of arbitrary payloads based on Grafana's encryptions.
@@ -36,6 +37,46 @@ func NewCrypto(secrets secrets.Service, configs configurationStore, log log.Logg
 	}
 }
 
+// ProcessSecureSettings encrypts new secure settings and loads existing secure settings from the database.
+func (c *alertmanagerCrypto) ProcessSecureSettings(ctx context.Context, orgId int64, recvs []*definitions.PostableApiReceiver) error {
+	// First, we encrypt the new or updated secure settings. Then, we load the existing secure settings from the database
+	// and add back any that weren't updated.
+	// We perform these steps in this order to ensure the hash of the secure settings remains stable when no secure
+	// settings were modified.
+	if err := EncryptReceiverConfigs(recvs, func(ctx context.Context, payload []byte) ([]byte, error) {
+		return c.Encrypt(ctx, payload, secrets.WithoutScope())
+	}); err != nil {
+		return fmt.Errorf("failed to encrypt receivers: %w", err)
+	}
+
+	if err := c.LoadSecureSettings(ctx, orgId, recvs); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// EncryptReceiverConfigs encrypts all SecureSettings in the given receivers.
+func EncryptReceiverConfigs(c []*definitions.PostableApiReceiver, encrypt definitions.EncryptFn) error {
+	// encrypt secure settings for storing them in DB
+	for _, r := range c {
+		switch r.Type() {
+		case definitions.GrafanaReceiverType:
+			for _, gr := range r.PostableGrafanaReceivers.GrafanaManagedReceivers {
+				for k, v := range gr.SecureSettings {
+					encryptedData, err := encrypt(context.Background(), []byte(v))
+					if err != nil {
+						return fmt.Errorf("failed to encrypt secure settings: %w", err)
+					}
+					gr.SecureSettings[k] = base64.StdEncoding.EncodeToString(encryptedData)
+				}
+			}
+		default:
+		}
+	}
+	return nil
+}
+
 // LoadSecureSettings adds the corresponding unencrypted secrets stored to the list of input receivers.
 func (c *alertmanagerCrypto) LoadSecureSettings(ctx context.Context, orgId int64, receivers []*definitions.PostableApiReceiver) error {
 	// Get the last known working configuration.

pkg/services/ngalert/notifier/multiorg_alertmanager.go

@@ -18,7 +18,6 @@ import (
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/ngalert/metrics"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
-	"github.com/grafana/grafana/pkg/services/ngalert/provisioning"
 	"github.com/grafana/grafana/pkg/services/ngalert/store"
 	"github.com/grafana/grafana/pkg/services/notifications"
 	"github.com/grafana/grafana/pkg/services/secrets"
@@ -32,7 +31,7 @@ var (
 
 type MultiOrgAlertmanager struct {
 	Crypto    Crypto
-	ProvStore provisioning.ProvisioningStore
+	ProvStore provisioningStore
 
 	alertmanagersMtx sync.RWMutex
 	alertmanagers    map[int64]*Alertmanager
@@ -55,7 +54,7 @@ type MultiOrgAlertmanager struct {
 }
 
 func NewMultiOrgAlertmanager(cfg *setting.Cfg, configStore AlertingStore, orgStore store.OrgStore,
-	kvStore kvstore.KVStore, provStore provisioning.ProvisioningStore, decryptFn alertingNotify.GetDecryptedValueFn,
+	kvStore kvstore.KVStore, provStore provisioningStore, decryptFn alertingNotify.GetDecryptedValueFn,
 	m *metrics.MultiOrgAlertmanager, ns notifications.Service, l log.Logger, s secrets.Service,
 ) (*MultiOrgAlertmanager, error) {
 	moa := &MultiOrgAlertmanager{

pkg/services/ngalert/provisioning/contactpoints_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
+	"github.com/grafana/grafana/pkg/services/ngalert/notifier"
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/secrets"
 	"github.com/grafana/grafana/pkg/services/secrets/database"
@@ -310,7 +311,7 @@ func createContactPointServiceSut(t *testing.T, secretService secrets.Service) *
 	c := &definitions.PostableUserConfig{}
 	err := json.Unmarshal([]byte(defaultAlertmanagerConfigJSON), c)
 	require.NoError(t, err)
-	err = c.EncryptConfig(func(ctx context.Context, payload []byte) ([]byte, error) {
+	err = notifier.EncryptReceiverConfigs(c.AlertmanagerConfig.Receivers, func(ctx context.Context, payload []byte) ([]byte, error) {
 		return secretService.Encrypt(ctx, payload, secrets.WithoutScope())
 	})
 	require.NoError(t, err)