Alerting: Refactor alertmanager notifier to use encoding/json to parse settings instead of simplejson (#55507)

* replace basic auth header with method call

Co-authored-by: Yuri Tseretyan <yuriy.tseretyan@grafana.com>

pkg/services/ngalert/notifier/channels/alertmanager.go

@@ -9,11 +9,8 @@ import (
 	"strings"
 
 	"github.com/grafana/alerting/alerting/notifier/channels"
-	"github.com/prometheus/alertmanager/template"
 	"github.com/prometheus/alertmanager/types"
 	"github.com/prometheus/common/model"
-
-	"github.com/grafana/grafana/pkg/components/simplejson"
 )
 
 type AlertmanagerConfig struct {
@@ -23,20 +20,36 @@ type AlertmanagerConfig struct {
 	BasicAuthPassword string
 }
 
-func NewAlertmanagerConfig(config *channels.NotificationChannelConfig, fn channels.GetDecryptedValueFn) (*AlertmanagerConfig, error) {
+type alertmanagerSettings struct {
-	simpleConfig, err := simplejson.NewJson(config.Settings)
+	URLs     []*url.URL
+	User     string
+	Password string
+}
+
+func AlertmanagerFactory(fc channels.FactoryConfig) (channels.NotificationChannel, error) {
+	ch, err := buildAlertmanagerNotifier(fc)
 	if err != nil {
-		return nil, err
+		return nil, receiverInitError{
+			Reason: err.Error(),
+			Cfg:    *fc.Config,
+		}
 	}
-	urlStr := simpleConfig.Get("url").MustString()
+	return ch, nil
-	if urlStr == "" {
+}
-		return nil, errors.New("could not find url property in settings")
+
+func buildAlertmanagerNotifier(fc channels.FactoryConfig) (*AlertmanagerNotifier, error) {
+	var settings struct {
+		URL      channels.CommaSeparatedStrings `json:"url,omitempty" yaml:"url,omitempty"`
+		User     string                         `json:"basicAuthUser,omitempty" yaml:"basicAuthUser,omitempty"`
+		Password string                         `json:"basicAuthPassword,omitempty" yaml:"basicAuthPassword,omitempty"`
+	}
+	err := json.Unmarshal(fc.Config.Settings, &settings)
+	if err != nil {
+		return nil, fmt.Errorf("failed to unmarshal settings: %w", err)
 	}
 
-	urlParts := strings.Split(urlStr, ",")
+	urls := make([]*url.URL, 0, len(settings.URL))
-	urls := make([]*url.URL, 0, len(urlParts))
+	for _, uS := range settings.URL {
-
-	for _, uS := range urlParts {
 		uS = strings.TrimSpace(uS)
 		if uS == "" {
 			continue
@@ -48,46 +61,29 @@ func NewAlertmanagerConfig(config *channels.NotificationChannelConfig, fn channe
 		}
 		urls = append(urls, u)
 	}
-	return &AlertmanagerConfig{
+	if len(settings.URL) == 0 || len(urls) == 0 {
-		NotificationChannelConfig: config,
+		return nil, errors.New("could not find url property in settings")
-		URLs:                      urls,
-		BasicAuthUser:             simpleConfig.Get("basicAuthUser").MustString(),
-		BasicAuthPassword:         fn(context.Background(), config.SecureSettings, "basicAuthPassword", simpleConfig.Get("basicAuthPassword").MustString()),
-	}, nil
-}
-
-func AlertmanagerFactory(fc channels.FactoryConfig) (channels.NotificationChannel, error) {
-	config, err := NewAlertmanagerConfig(fc.Config, fc.DecryptFunc)
-	if err != nil {
-		return nil, receiverInitError{
-			Reason: err.Error(),
-			Cfg:    *fc.Config,
-		}
 	}
-	return NewAlertmanagerNotifier(config, fc.Logger, fc.ImageStore, nil, fc.DecryptFunc), nil
+	settings.Password = fc.DecryptFunc(context.Background(), fc.Config.SecureSettings, "basicAuthPassword", settings.Password)
-}
 
-// NewAlertmanagerNotifier returns a new Alertmanager notifier.
-func NewAlertmanagerNotifier(config *AlertmanagerConfig, l channels.Logger, images channels.ImageStore, _ *template.Template, fn channels.GetDecryptedValueFn) *AlertmanagerNotifier {
 	return &AlertmanagerNotifier{
-		Base:              channels.NewBase(config.NotificationChannelConfig),
+		Base:   channels.NewBase(fc.Config),
-		images:            images,
+		images: fc.ImageStore,
-		urls:              config.URLs,
+		settings: alertmanagerSettings{
-		basicAuthUser:     config.BasicAuthUser,
+			URLs:     urls,
-		basicAuthPassword: config.BasicAuthPassword,
+			User:     settings.User,
-		logger:            l,
+			Password: settings.Password,
-	}
+		},
+		logger: fc.Logger,
+	}, nil
 }
 
 // AlertmanagerNotifier sends alert notifications to the alert manager
 type AlertmanagerNotifier struct {
 	*channels.Base
-	images channels.ImageStore
+	images   channels.ImageStore
-
+	settings alertmanagerSettings
-	urls              []*url.URL
+	logger   channels.Logger
-	basicAuthUser     string
-	basicAuthPassword string
-	logger            channels.Logger
 }
 
 // Notify sends alert notifications to Alertmanager.
@@ -116,10 +112,10 @@ func (n *AlertmanagerNotifier) Notify(ctx context.Context, as ...*types.Alert) (
 		lastErr error
 		numErrs int
 	)
-	for _, u := range n.urls {
+	for _, u := range n.settings.URLs {
 		if _, err := sendHTTPRequest(ctx, u, httpCfg{
-			user:     n.basicAuthUser,
+			user:     n.settings.User,
-			password: n.basicAuthPassword,
+			password: n.settings.Password,
 			body:     body,
 		}, n.logger); err != nil {
 			n.logger.Warn("failed to send to Alertmanager", "error", err, "alertmanager", n.Name, "url", u.String())
@@ -128,7 +124,7 @@ func (n *AlertmanagerNotifier) Notify(ctx context.Context, as ...*types.Alert) (
 		}
 	}
 
-	if numErrs == len(n.urls) {
+	if numErrs == len(n.settings.URLs) {
 		// All attempts to send alerts have failed
 		n.logger.Warn("all attempts to send to Alertmanager failed", "alertmanager", n.Name)
 		return false, fmt.Errorf("failed to send alert to Alertmanager: %w", lastErr)

pkg/services/ngalert/notifier/channels/alertmanager_test.go

@@ -40,6 +40,30 @@ func TestNewAlertmanagerNotifier(t *testing.T) {
 			expectedInitError: `invalid url property in settings: parse "://alertmanager.com/api/v1/alerts": missing protocol scheme`,
 			receiverName:      "Alertmanager",
 		},
+		{
+			name: "Error in initing: empty URL",
+			settings: `{
+				"url": ""
+			}`,
+			expectedInitError: `could not find url property in settings`,
+			receiverName:      "Alertmanager",
+		},
+		{
+			name: "Error in initing: null URL",
+			settings: `{
+				"url": null
+			}`,
+			expectedInitError: `could not find url property in settings`,
+			receiverName:      "Alertmanager",
+		},
+		{
+			name: "Error in initing: one of multiple URLs is invalid",
+			settings: `{
+				"url": "https://alertmanager-01.com,://url"
+			}`,
+			expectedInitError: "invalid url property in settings: parse \"://url/api/v1/alerts\": missing protocol scheme",
+			receiverName:      "Alertmanager",
+		},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
@@ -55,14 +79,20 @@ func TestNewAlertmanagerNotifier(t *testing.T) {
 			decryptFn := func(ctx context.Context, sjd map[string][]byte, key string, fallback string) string {
 				return fallback
 			}
-			cfg, err := NewAlertmanagerConfig(m, decryptFn)
+
-			if c.expectedInitError != "" {
+			fc := channels.FactoryConfig{
-				require.Equal(t, c.expectedInitError, err.Error())
+				Config:      m,
-				return
+				DecryptFunc: decryptFn,
+				ImageStore:  &channels.UnavailableImageStore{},
+				Template:    tmpl,
+				Logger:      &channels.FakeLogger{},
+			}
+			sn, err := buildAlertmanagerNotifier(fc)
+			if c.expectedInitError != "" {
+				require.ErrorContains(t, err, c.expectedInitError)
+			} else {
+				require.NotNil(t, sn)
 			}
-			require.NoError(t, err)
-			sn := NewAlertmanagerNotifier(cfg, &channels.FakeLogger{}, &channels.UnavailableImageStore{}, tmpl, decryptFn)
-			require.NotNil(t, sn)
 		})
 	}
 }
@@ -153,9 +183,16 @@ func TestAlertmanagerNotifier_Notify(t *testing.T) {
 			decryptFn := func(ctx context.Context, sjd map[string][]byte, key string, fallback string) string {
 				return fallback
 			}
-			cfg, err := NewAlertmanagerConfig(m, decryptFn)
+			fc := channels.FactoryConfig{
+				Config:      m,
+				DecryptFunc: decryptFn,
+				ImageStore:  images,
+				Template:    tmpl,
+				Logger:      &channels.FakeLogger{},
+			}
+			sn, err := buildAlertmanagerNotifier(fc)
 			require.NoError(t, err)
-			sn := NewAlertmanagerNotifier(cfg, &channels.FakeLogger{}, images, tmpl, decryptFn)
+
 			var body []byte
 			origSendHTTPRequest := sendHTTPRequest
 			t.Cleanup(func() {

pkg/services/ngalert/notifier/channels/util.go

@@ -20,7 +20,6 @@ import (
 	"github.com/prometheus/common/model"
 
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
-	"github.com/grafana/grafana/pkg/util"
 )
 
 var (
@@ -151,7 +150,7 @@ var sendHTTPRequest = func(ctx context.Context, url *url.URL, cfg httpCfg, logge
 		return nil, fmt.Errorf("failed to create HTTP request: %w", err)
 	}
 	if cfg.user != "" && cfg.password != "" {
-		request.Header.Set("Authorization", util.GetBasicAuthHeader(cfg.user, cfg.password))
+		request.SetBasicAuth(cfg.user, cfg.password)
 	}
 
 	request.Header.Set("Content-Type", "application/json")