Alerting: Refactor notifiers VictorOps, Threema, Telegram to use encoding/json to parse settings instead of simplejson (#55779)

* update victorops
* update threema
* update telegram

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

@@ -27,68 +27,69 @@ var (
 // alert notifications to Telegram.
 type TelegramNotifier struct {
 	*Base
-	BotToken string
-	ChatID   string
-	Message  string
 	log      log.Logger
 	images   ImageStore
 	ns       notifications.WebhookSender
 	tmpl     *template.Template
+	settings telegramSettings
 }
 
-type TelegramConfig struct {
+type telegramSettings struct {
-	*NotificationChannelConfig
+	BotToken string `json:"bottoken,omitempty" yaml:"bottoken,omitempty"`
-	BotToken string
+	ChatID   string `json:"chatid,omitempty" yaml:"chatid,omitempty"`
-	ChatID   string
+	Message  string `json:"message,omitempty" yaml:"message,omitempty"`
-	Message  string
+}
+
+func buildTelegramSettings(fc FactoryConfig) (telegramSettings, error) {
+	settings := telegramSettings{}
+	err := fc.Config.unmarshalSettings(&settings)
+	if err != nil {
+		return settings, fmt.Errorf("failed to unmarshal settings: %w", err)
+	}
+	settings.BotToken = fc.DecryptFunc(context.Background(), fc.Config.SecureSettings, "bottoken", settings.BotToken)
+	if settings.BotToken == "" {
+		return settings, errors.New("could not find Bot Token in settings")
+	}
+	if settings.ChatID == "" {
+		return settings, errors.New("could not find Chat Id in settings")
+	}
+	if settings.Message == "" {
+		settings.Message = DefaultMessageEmbed
+	}
+	return settings, nil
 }
 
 func TelegramFactory(fc FactoryConfig) (NotificationChannel, error) {
-	config, err := NewTelegramConfig(fc.Config, fc.DecryptFunc)
+	notifier, err := NewTelegramNotifier(fc)
 	if err != nil {
 		return nil, receiverInitError{
 			Reason: err.Error(),
 			Cfg:    *fc.Config,
 		}
 	}
-	return NewTelegramNotifier(config, fc.ImageStore, fc.NotificationService, fc.Template), nil
+	return notifier, nil
-}
-
-func NewTelegramConfig(config *NotificationChannelConfig, fn GetDecryptedValueFn) (*TelegramConfig, error) {
-	botToken := fn(context.Background(), config.SecureSettings, "bottoken", config.Settings.Get("bottoken").MustString())
-	if botToken == "" {
-		return &TelegramConfig{}, errors.New("could not find Bot Token in settings")
-	}
-	chatID := config.Settings.Get("chatid").MustString()
-	if chatID == "" {
-		return &TelegramConfig{}, errors.New("could not find Chat Id in settings")
-	}
-	return &TelegramConfig{
-		NotificationChannelConfig: config,
-		BotToken:                  botToken,
-		ChatID:                    chatID,
-		Message:                   config.Settings.Get("message").MustString(DefaultMessageEmbed),
-	}, nil
 }
 
 // NewTelegramNotifier is the constructor for the Telegram notifier
-func NewTelegramNotifier(config *TelegramConfig, images ImageStore, ns notifications.WebhookSender, t *template.Template) *TelegramNotifier {
+func NewTelegramNotifier(fc FactoryConfig) (*TelegramNotifier, error) {
+	settings, err := buildTelegramSettings(fc)
+	if err != nil {
+		return nil, err
+	}
 	return &TelegramNotifier{
 		Base: NewBase(&models.AlertNotification{
-			Uid:                   config.UID,
+			Uid:                   fc.Config.UID,
-			Name:                  config.Name,
+			Name:                  fc.Config.Name,
-			Type:                  config.Type,
+			Type:                  fc.Config.Type,
-			DisableResolveMessage: config.DisableResolveMessage,
+			DisableResolveMessage: fc.Config.DisableResolveMessage,
-			Settings:              config.Settings,
+			Settings:              fc.Config.Settings,
 		}),
-		BotToken: config.BotToken,
+		tmpl:     fc.Template,
-		ChatID:   config.ChatID,
-		Message:  config.Message,
-		tmpl:     t,
 		log:      log.New("alerting.notifier.telegram"),
-		images:   images,
+		images:   fc.ImageStore,
-		ns:       ns,
+		ns:       fc.NotificationService,
-	}
+		settings: settings,
+	}, nil
 }
 
 // Notify send an alert notification to Telegram.
@@ -160,9 +161,9 @@ func (tn *TelegramNotifier) buildTelegramMessage(ctx context.Context, as []*type
 
 	tmpl, _ := TmplText(ctx, tn.tmpl, as, tn.log, &tmplErr)
 	// Telegram supports 4096 chars max
-	messageText, truncated := notify.Truncate(tmpl(tn.Message), 4096)
+	messageText, truncated := notify.Truncate(tmpl(tn.settings.Message), 4096)
 	if truncated {
-		tn.log.Warn("Telegram message too long, truncate message", "original_message", tn.Message)
+		tn.log.Warn("Telegram message too long, truncate message", "original_message", tn.settings.Message)
 	}
 
 	m := make(map[string]string)
@@ -186,7 +187,7 @@ func (tn *TelegramNotifier) newWebhookSyncCmd(action string, fn func(writer *mul
 	if err != nil {
 		return nil, err
 	}
-	if _, err := fw.Write([]byte(tn.ChatID)); err != nil {
+	if _, err := fw.Write([]byte(tn.settings.ChatID)); err != nil {
 		return nil, err
 	}
 
@@ -199,7 +200,7 @@ func (tn *TelegramNotifier) newWebhookSyncCmd(action string, fn func(writer *mul
 	}
 
 	cmd := &models.SendWebhookSync{
-		Url:        fmt.Sprintf(TelegramAPIURL, tn.BotToken, action),
+		Url:        fmt.Sprintf(TelegramAPIURL, tn.settings.BotToken, action),
 		Body:       b.String(),
 		HttpMethod: "POST",
 		HttpHeader: map[string]string{

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

@@ -123,9 +123,10 @@ func TestTelegramNotifier(t *testing.T) {
 				ImageStore:          images,
 				NotificationService: notificationService,
 				DecryptFunc:         decryptFn,
+				Template:            tmpl,
 			}
 
-			cfg, err := NewTelegramConfig(fc.Config, decryptFn)
+			n, err := NewTelegramNotifier(fc)
 			if c.expInitError != "" {
 				require.Error(t, err)
 				require.Equal(t, c.expInitError, err.Error())
@@ -133,8 +134,6 @@ func TestTelegramNotifier(t *testing.T) {
 			}
 			require.NoError(t, err)
 
-			n := NewTelegramNotifier(cfg, images, notificationService, tmpl)
-
 			ctx := notify.WithGroupKey(context.Background(), "alertname")
 			ctx = notify.WithGroupLabels(ctx, model.LabelSet{"alertname": ""})
 			ok, err := n.Notify(ctx, c.alerts...)

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

@@ -26,95 +26,94 @@ var (
 // alert notifications to Threema.
 type ThreemaNotifier struct {
 	*Base
-	GatewayID   string
-	RecipientID string
-	APISecret   string
 	log      log.Logger
 	images   ImageStore
 	ns       notifications.WebhookSender
 	tmpl     *template.Template
+	settings threemaSettings
 }
 
-type ThreemaConfig struct {
+type threemaSettings struct {
-	*NotificationChannelConfig
+	GatewayID   string `json:"gateway_id,omitempty" yaml:"gateway_id,omitempty"`
-	GatewayID   string
+	RecipientID string `json:"recipient_id,omitempty" yaml:"recipient_id,omitempty"`
-	RecipientID string
+	APISecret   string `json:"api_secret,omitempty" yaml:"api_secret,omitempty"`
-	APISecret   string
+}
+
+func buildThreemaSettings(fc FactoryConfig) (threemaSettings, error) {
+	settings := threemaSettings{}
+	err := fc.Config.unmarshalSettings(&settings)
+	if err != nil {
+		return settings, fmt.Errorf("failed to unmarshal settings: %w", err)
+	}
+	// GatewayID validaiton
+	if settings.GatewayID == "" {
+		return settings, errors.New("could not find Threema Gateway ID in settings")
+	}
+	if !strings.HasPrefix(settings.GatewayID, "*") {
+		return settings, errors.New("invalid Threema Gateway ID: Must start with a *")
+	}
+	if len(settings.GatewayID) != 8 {
+		return settings, errors.New("invalid Threema Gateway ID: Must be 8 characters long")
+	}
+
+	// RecipientID validation
+	if settings.RecipientID == "" {
+		return settings, errors.New("could not find Threema Recipient ID in settings")
+	}
+	if len(settings.RecipientID) != 8 {
+		return settings, errors.New("invalid Threema Recipient ID: Must be 8 characters long")
+	}
+	settings.APISecret = fc.DecryptFunc(context.Background(), fc.Config.SecureSettings, "api_secret", settings.APISecret)
+	if settings.APISecret == "" {
+		return settings, errors.New("could not find Threema API secret in settings")
+	}
+	return settings, nil
 }
 
 func ThreemaFactory(fc FactoryConfig) (NotificationChannel, error) {
-	cfg, err := NewThreemaConfig(fc.Config, fc.DecryptFunc)
+	notifier, err := NewThreemaNotifier(fc)
 	if err != nil {
 		return nil, receiverInitError{
 			Reason: err.Error(),
 			Cfg:    *fc.Config,
 		}
 	}
-	return NewThreemaNotifier(cfg, fc.ImageStore, fc.NotificationService, fc.Template), nil
+	return notifier, nil
 }
 
-func NewThreemaConfig(config *NotificationChannelConfig, decryptFunc GetDecryptedValueFn) (*ThreemaConfig, error) {
+func NewThreemaNotifier(fc FactoryConfig) (*ThreemaNotifier, error) {
-	gatewayID := config.Settings.Get("gateway_id").MustString()
+	settings, err := buildThreemaSettings(fc)
-	if gatewayID == "" {
+	if err != nil {
-		return nil, errors.New("could not find Threema Gateway ID in settings")
+		return nil, err
 	}
-	if !strings.HasPrefix(gatewayID, "*") {
-		return nil, errors.New("invalid Threema Gateway ID: Must start with a *")
-	}
-	if len(gatewayID) != 8 {
-		return nil, errors.New("invalid Threema Gateway ID: Must be 8 characters long")
-	}
-	recipientID := config.Settings.Get("recipient_id").MustString()
-	if recipientID == "" {
-		return nil, errors.New("could not find Threema Recipient ID in settings")
-	}
-	if len(recipientID) != 8 {
-		return nil, errors.New("invalid Threema Recipient ID: Must be 8 characters long")
-	}
-	apiSecret := decryptFunc(context.Background(), config.SecureSettings, "api_secret", config.Settings.Get("api_secret").MustString())
-	if apiSecret == "" {
-		return nil, errors.New("could not find Threema API secret in settings")
-	}
-	return &ThreemaConfig{
-		NotificationChannelConfig: config,
-		GatewayID:                 gatewayID,
-		RecipientID:               recipientID,
-		APISecret:                 apiSecret,
-	}, nil
-}
-
-// NewThreemaNotifier is the constructor for the Threema notifier
-func NewThreemaNotifier(config *ThreemaConfig, images ImageStore, ns notifications.WebhookSender, t *template.Template) *ThreemaNotifier {
 	return &ThreemaNotifier{
 		Base: NewBase(&models.AlertNotification{
-			Uid:                   config.UID,
+			Uid:                   fc.Config.UID,
-			Name:                  config.Name,
+			Name:                  fc.Config.Name,
-			Type:                  config.Type,
+			Type:                  fc.Config.Type,
-			DisableResolveMessage: config.DisableResolveMessage,
+			DisableResolveMessage: fc.Config.DisableResolveMessage,
-			Settings:              config.Settings,
+			Settings:              fc.Config.Settings,
 		}),
-		GatewayID:   config.GatewayID,
-		RecipientID: config.RecipientID,
-		APISecret:   config.APISecret,
 		log:      log.New("alerting.notifier.threema"),
-		images:      images,
+		images:   fc.ImageStore,
-		ns:          ns,
+		ns:       fc.NotificationService,
-		tmpl:        t,
+		tmpl:     fc.Template,
-	}
+		settings: settings,
+	}, nil
 }
 
 // Notify send an alert notification to Threema
 func (tn *ThreemaNotifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) {
-	tn.log.Debug("sending threema alert notification", "from", tn.GatewayID, "to", tn.RecipientID)
+	tn.log.Debug("sending threema alert notification", "from", tn.settings.GatewayID, "to", tn.settings.RecipientID)
 
 	var tmplErr error
 	tmpl, _ := TmplText(ctx, tn.tmpl, as, tn.log, &tmplErr)
 
 	// Set up basic API request data
 	data := url.Values{}
-	data.Set("from", tn.GatewayID)
+	data.Set("from", tn.settings.GatewayID)
-	data.Set("to", tn.RecipientID)
+	data.Set("to", tn.settings.RecipientID)
-	data.Set("secret", tn.APISecret)
+	data.Set("secret", tn.settings.APISecret)
 
 	// Determine emoji
 	stateEmoji := "\u26A0\uFE0F " // Warning sign

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

@@ -112,8 +112,16 @@ func TestThreemaNotifier(t *testing.T) {
 
 			webhookSender := mockNotificationService()
 			secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
-			decryptFn := secretsService.GetDecryptedValue
+
-			cfg, err := NewThreemaConfig(m, decryptFn)
+			fc := FactoryConfig{
+				Config:              m,
+				NotificationService: webhookSender,
+				ImageStore:          images,
+				Template:            tmpl,
+				DecryptFunc:         secretsService.GetDecryptedValue,
+			}
+
+			pn, err := NewThreemaNotifier(fc)
 			if c.expInitError != "" {
 				require.Error(t, err)
 				require.Equal(t, c.expInitError, err.Error())
@@ -123,7 +131,6 @@ func TestThreemaNotifier(t *testing.T) {
 
 			ctx := notify.WithGroupKey(context.Background(), "alertname")
 			ctx = notify.WithGroupLabels(ctx, model.LabelSet{"alertname": ""})
-			pn := NewThreemaNotifier(cfg, images, webhookSender, tmpl)
 			ok, err := pn.Notify(ctx, c.alerts...)
 			if c.expMsgError != nil {
 				require.False(t, ok)

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

@@ -2,7 +2,9 @@ package channels
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
+	"fmt"
 	"strings"
 	"time"
 
@@ -11,7 +13,6 @@ import (
 	"github.com/prometheus/alertmanager/types"
 	"github.com/prometheus/common/model"
 
-	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
 	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
@@ -27,53 +28,58 @@ const (
 	victoropsAlertStateRecovery = "RECOVERY"
 )
 
-type VictorOpsConfig struct {
+type victorOpsSettings struct {
-	*NotificationChannelConfig
+	URL         string `json:"url,omitempty" yaml:"url,omitempty"`
-	URL         string
+	MessageType string `json:"messageType,omitempty" yaml:"messageType,omitempty"`
-	MessageType string
+}
+
+func buildVictorOpsSettings(fc FactoryConfig) (victorOpsSettings, error) {
+	settings := victorOpsSettings{}
+	err := fc.Config.unmarshalSettings(&settings)
+	if err != nil {
+		return settings, fmt.Errorf("failed to unmarshal settings: %w", err)
+	}
+	if settings.URL == "" {
+		return settings, errors.New("could not find victorops url property in settings")
+	}
+	if settings.MessageType == "" {
+		settings.MessageType = victoropsAlertStateCritical
+	}
+	return settings, nil
 }
 
 func VictorOpsFactory(fc FactoryConfig) (NotificationChannel, error) {
-	cfg, err := NewVictorOpsConfig(fc.Config)
+	notifier, err := NewVictoropsNotifier(fc)
 	if err != nil {
 		return nil, receiverInitError{
 			Reason: err.Error(),
 			Cfg:    *fc.Config,
 		}
 	}
-	return NewVictoropsNotifier(cfg, fc.ImageStore, fc.NotificationService, fc.Template), nil
+	return notifier, nil
-}
-
-func NewVictorOpsConfig(config *NotificationChannelConfig) (*VictorOpsConfig, error) {
-	url := config.Settings.Get("url").MustString()
-	if url == "" {
-		return nil, errors.New("could not find victorops url property in settings")
-	}
-	return &VictorOpsConfig{
-		NotificationChannelConfig: config,
-		URL:                       url,
-		MessageType:               config.Settings.Get("messageType").MustString(),
-	}, nil
 }
 
 // NewVictoropsNotifier creates an instance of VictoropsNotifier that
 // handles posting notifications to Victorops REST API
-func NewVictoropsNotifier(config *VictorOpsConfig, images ImageStore, ns notifications.WebhookSender, t *template.Template) *VictoropsNotifier {
+func NewVictoropsNotifier(fc FactoryConfig) (*VictoropsNotifier, error) {
+	settings, err := buildVictorOpsSettings(fc)
+	if err != nil {
+		return nil, err
+	}
 	return &VictoropsNotifier{
 		Base: NewBase(&models.AlertNotification{
-			Uid:                   config.UID,
+			Uid:                   fc.Config.UID,
-			Name:                  config.Name,
+			Name:                  fc.Config.Name,
-			Type:                  config.Type,
+			Type:                  fc.Config.Type,
-			DisableResolveMessage: config.DisableResolveMessage,
+			DisableResolveMessage: fc.Config.DisableResolveMessage,
-			Settings:              config.Settings,
+			Settings:              fc.Config.Settings,
 		}),
-		URL:         config.URL,
-		MessageType: config.MessageType,
 		log:      log.New("alerting.notifier.victorops"),
-		images:      images,
+		images:   fc.ImageStore,
-		ns:          ns,
+		ns:       fc.NotificationService,
-		tmpl:        t,
+		tmpl:     fc.Template,
-	}
+		settings: settings,
+	}, nil
 }
 
 // VictoropsNotifier defines URL property for Victorops REST API
@@ -81,23 +87,23 @@ func NewVictoropsNotifier(config *VictorOpsConfig, images ImageStore, ns notific
 // Victorops specifications (http://victorops.force.com/knowledgebase/articles/Integration/Alert-Ingestion-API-Documentation/)
 type VictoropsNotifier struct {
 	*Base
-	URL         string
-	MessageType string
 	log      log.Logger
 	images   ImageStore
 	ns       notifications.WebhookSender
 	tmpl     *template.Template
+	settings victorOpsSettings
 }
 
 // Notify sends notification to Victorops via POST to URL endpoint
 func (vn *VictoropsNotifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) {
-	vn.log.Debug("executing victorops notification", "notification", vn.Name)
+	vn.log.Debug("sending notification", "notification", vn.Name)
 
 	var tmplErr error
 	tmpl, _ := TmplText(ctx, vn.tmpl, as, vn.log, &tmplErr)
 
-	messageType := strings.ToUpper(tmpl(vn.MessageType))
+	messageType := strings.ToUpper(tmpl(vn.settings.MessageType))
 	if messageType == "" {
+		vn.log.Warn("expansion of message type template resulted in an empty string. Using fallback", "fallback", victoropsAlertStateCritical, "template", vn.settings.MessageType)
 		messageType = victoropsAlertStateCritical
 	}
 	alerts := types.Alerts(as...)
@@ -110,38 +116,40 @@ func (vn *VictoropsNotifier) Notify(ctx context.Context, as ...*types.Alert) (bo
 		return false, err
 	}
 
-	bodyJSON := simplejson.New()
+	bodyJSON := map[string]interface{}{
-	bodyJSON.Set("message_type", messageType)
+		"message_type":        messageType,
-	bodyJSON.Set("entity_id", groupKey.Hash())
+		"entity_id":           groupKey.Hash(),
-	bodyJSON.Set("entity_display_name", tmpl(DefaultMessageTitleEmbed))
+		"entity_display_name": tmpl(DefaultMessageTitleEmbed),
-	bodyJSON.Set("timestamp", time.Now().Unix())
+		"timestamp":           time.Now().Unix(),
-	bodyJSON.Set("state_message", tmpl(DefaultMessageEmbed))
+		"state_message":       tmpl(DefaultMessageEmbed),
-	bodyJSON.Set("monitoring_tool", "Grafana v"+setting.BuildVersion)
+		"monitoring_tool":     "Grafana v" + setting.BuildVersion,
+	}
+
+	if tmplErr != nil {
+		vn.log.Warn("failed to expand message template. "+
+			"", "err", tmplErr.Error())
+		tmplErr = nil
+	}
 
 	_ = withStoredImages(ctx, vn.log, vn.images,
 		func(index int, image ngmodels.Image) error {
 			if image.URL != "" {
-				bodyJSON.Set("image_url", image.URL)
+				bodyJSON["image_url"] = image.URL
 				return ErrImagesDone
 			}
 			return nil
 		}, as...)
 
 	ruleURL := joinUrlPath(vn.tmpl.ExternalURL.String(), "/alerting/list", vn.log)
-	bodyJSON.Set("alert_url", ruleURL)
+	bodyJSON["alert_url"] = ruleURL
 
+	u := tmpl(vn.settings.URL)
 	if tmplErr != nil {
-		vn.log.Warn("failed to template VictorOps message", "err", tmplErr.Error())
+		vn.log.Info("failed to expand URL template", "err", tmplErr.Error(), "fallback", vn.settings.URL)
-		tmplErr = nil
+		u = vn.settings.URL
 	}
 
-	u := tmpl(vn.URL)
+	b, err := json.Marshal(bodyJSON)
-	if tmplErr != nil {
-		vn.log.Info("failed to template VictorOps URL", "err", tmplErr.Error(), "fallback", vn.URL)
-		u = vn.URL
-	}
-
-	b, err := bodyJSON.MarshalJSON()
 	if err != nil {
 		return false, err
 	}
@@ -151,7 +159,7 @@ func (vn *VictoropsNotifier) Notify(ctx context.Context, as ...*types.Alert) (bo
 	}
 
 	if err := vn.ns.SendWebhookSync(ctx, cmd); err != nil {
-		vn.log.Error("Failed to send Victorops notification", "err", err, "webhook", vn.Name)
+		vn.log.Error("failed to send notification", "err", err, "webhook", vn.Name)
 		return false, err
 	}
 

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

@@ -173,7 +173,15 @@ func TestVictoropsNotifier(t *testing.T) {
 			}
 
 			webhookSender := mockNotificationService()
-			cfg, err := NewVictorOpsConfig(m)
+
+			fc := FactoryConfig{
+				Config:              m,
+				NotificationService: webhookSender,
+				ImageStore:          images,
+				Template:            tmpl,
+			}
+
+			pn, err := NewVictoropsNotifier(fc)
 			if c.expInitError != "" {
 				require.Error(t, err)
 				require.Equal(t, c.expInitError, err.Error())
@@ -183,7 +191,6 @@ func TestVictoropsNotifier(t *testing.T) {
 
 			ctx := notify.WithGroupKey(context.Background(), "alertname")
 			ctx = notify.WithGroupLabels(ctx, model.LabelSet{"alertname": ""})
-			pn := NewVictoropsNotifier(cfg, images, webhookSender, tmpl)
 			ok, err := pn.Notify(ctx, c.alerts...)
 			if c.expMsgError != nil {
 				require.False(t, ok)