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

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

@@ -14,65 +14,72 @@ import (
 	"github.com/grafana/grafana/pkg/services/notifications"
 )
 
-type WeComConfig struct {
+type wecomSettings struct {
-	*NotificationChannelConfig
+	URL     string `json:"url" yaml:"url"`
-	URL     string
+	Message string `json:"message,omitempty" yaml:"message,omitempty"`
-	Message string
+	Title   string `json:"title,omitempty" yaml:"title,omitempty"`
-	Title   string
+}
+
+func buildWecomSettings(factoryConfig FactoryConfig) (wecomSettings, error) {
+	var settings = wecomSettings{}
+
+	err := factoryConfig.Config.unmarshalSettings(&settings)
+	if err != nil {
+		return settings, fmt.Errorf("failed to unmarshal settings: %w", err)
+	}
+
+	if settings.Message == "" {
+		settings.Message = DefaultMessageEmbed
+	}
+	if settings.Title == "" {
+		settings.Title = DefaultMessageTitleEmbed
+	}
+
+	settings.URL = factoryConfig.DecryptFunc(context.Background(), factoryConfig.Config.SecureSettings, "url", settings.URL)
+	if settings.URL == "" {
+		return settings, errors.New("could not find webhook URL in settings")
+	}
+	return settings, nil
 }
 
 func WeComFactory(fc FactoryConfig) (NotificationChannel, error) {
-	cfg, err := NewWeComConfig(fc.Config, fc.DecryptFunc)
+	ch, err := buildWecomNotifier(fc)
 	if err != nil {
 		return nil, receiverInitError{
 			Reason: err.Error(),
 			Cfg:    *fc.Config,
 		}
 	}
-	return NewWeComNotifier(cfg, fc.NotificationService, fc.Template), nil
+	return ch, nil
 }
 
-func NewWeComConfig(config *NotificationChannelConfig, decryptFunc GetDecryptedValueFn) (*WeComConfig, error) {
+func buildWecomNotifier(factoryConfig FactoryConfig) (*WeComNotifier, error) {
-	url := decryptFunc(context.Background(), config.SecureSettings, "url", config.Settings.Get("url").MustString())
+	settings, err := buildWecomSettings(factoryConfig)
-	if url == "" {
+	if err != nil {
-		return nil, errors.New("could not find webhook URL in settings")
+		return nil, err
 	}
-	return &WeComConfig{
-		NotificationChannelConfig: config,
-		URL:                       url,
-		Message:                   config.Settings.Get("message").MustString(DefaultMessageEmbed),
-		Title:                     config.Settings.Get("title").MustString(DefaultMessageTitleEmbed),
-	}, nil
-}
-
-// NewWeComNotifier is the constructor for WeCom notifier.
-func NewWeComNotifier(config *WeComConfig, ns notifications.WebhookSender, t *template.Template) *WeComNotifier {
 	return &WeComNotifier{
 		Base: NewBase(&models.AlertNotification{
-			Uid:                   config.UID,
+			Uid:                   factoryConfig.Config.UID,
-			Name:                  config.Name,
+			Name:                  factoryConfig.Config.Name,
-			Type:                  config.Type,
+			Type:                  factoryConfig.Config.Type,
-			DisableResolveMessage: config.DisableResolveMessage,
+			DisableResolveMessage: factoryConfig.Config.DisableResolveMessage,
-			Settings:              config.Settings,
+			Settings:              factoryConfig.Config.Settings,
 		}),
-		URL:     config.URL,
+		tmpl:     factoryConfig.Template,
-		Message: config.Message,
+		log:      log.New("alerting.notifier.wecom"),
-		Title:   config.Title,
+		ns:       factoryConfig.NotificationService,
-		log:     log.New("alerting.notifier.wecom"),
+		settings: settings,
-		ns:      ns,
+	}, nil
-		tmpl:    t,
-	}
 }
 
 // WeComNotifier is responsible for sending alert notifications to WeCom.
 type WeComNotifier struct {
 	*Base
-	URL     string
+	tmpl     *template.Template
-	Message string
+	log      log.Logger
-	Title   string
+	ns       notifications.WebhookSender
-	tmpl    *template.Template
+	settings wecomSettings
-	log     log.Logger
-	ns      notifications.WebhookSender
 }
 
 // Notify send an alert notification to WeCom.
@@ -86,8 +93,8 @@ func (w *WeComNotifier) Notify(ctx context.Context, as ...*types.Alert) (bool, e
 		"msgtype": "markdown",
 	}
 	content := fmt.Sprintf("# %s\n%s\n",
-		tmpl(w.Title),
+		tmpl(w.settings.Title),
-		tmpl(w.Message),
+		tmpl(w.settings.Message),
 	)
 
 	bodyMsg["markdown"] = map[string]interface{}{
@@ -100,7 +107,7 @@ func (w *WeComNotifier) Notify(ctx context.Context, as ...*types.Alert) (bool, e
 	}
 
 	cmd := &models.SendWebhookSync{
-		Url:  w.URL,
+		Url:  w.settings.URL,
 		Body: string(body),
 	}
 

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

@@ -108,6 +108,25 @@ func TestWeComNotifier(t *testing.T) {
 			settings:     `{}`,
 			expInitError: `could not find webhook URL in settings`,
 		},
+		{
+			name:     "Use default if optional fields are explicitly empty",
+			settings: `{"url": "http://localhost", "message": "", "title": ""}`,
+			alerts: []*types.Alert{
+				{
+					Alert: model.Alert{
+						Labels:      model.LabelSet{"alertname": "alert1", "lbl1": "val1"},
+						Annotations: model.LabelSet{"ann1": "annv1", "__dashboardUid__": "abcd", "__panelId__": "efgh"},
+					},
+				},
+			},
+			expMsg: map[string]interface{}{
+				"markdown": map[string]interface{}{
+					"content": "# [FIRING:1]  (val1)\n**Firing**\n\nValue: [no value]\nLabels:\n - alertname = alert1\n - lbl1 = val1\nAnnotations:\n - ann1 = annv1\nSilence: http://localhost/alerting/silence/new?alertmanager=grafana&matcher=alertname%3Dalert1&matcher=lbl1%3Dval1\nDashboard: http://localhost/d/abcd\nPanel: http://localhost/d/abcd?viewPanel=efgh\n\n",
+				},
+				"msgtype": "markdown",
+			},
+			expMsgError: nil,
+		},
 	}
 
 	for _, c := range cases {
@@ -123,8 +142,16 @@ func TestWeComNotifier(t *testing.T) {
 
 			webhookSender := mockNotificationService()
 			secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
-			decryptFn := secretsService.GetDecryptedValue
+
-			cfg, err := NewWeComConfig(m, decryptFn)
+			fc := FactoryConfig{
+				Config:              m,
+				NotificationService: webhookSender,
+				DecryptFunc:         secretsService.GetDecryptedValue,
+				ImageStore:          nil,
+				Template:            tmpl,
+			}
+
+			pn, err := buildWecomNotifier(fc)
 			if c.expInitError != "" {
 				require.Equal(t, c.expInitError, err.Error())
 				return
@@ -133,7 +160,7 @@ func TestWeComNotifier(t *testing.T) {
 
 			ctx := notify.WithGroupKey(context.Background(), "alertname")
 			ctx = notify.WithGroupLabels(ctx, model.LabelSet{"alertname": ""})
-			pn := NewWeComNotifier(cfg, webhookSender, tmpl)
+
 			ok, err := pn.Notify(ctx, c.alerts...)
 			if c.expMsgError != nil {
 				require.False(t, ok)