From 57a0b6db2ca7a7e94a606577d9a1e00d083f09aa Mon Sep 17 00:00:00 2001 From: Yuriy Tseretyan Date: Tue, 20 Sep 2022 14:37:24 -0400 Subject: [PATCH] Alerting: Refactor WeCom notifier to use encoding/json to parse settings instead of simplejson (#55423) --- .../ngalert/notifier/channels/wecom.go | 91 ++++++++++--------- .../ngalert/notifier/channels/wecom_test.go | 33 ++++++- 2 files changed, 79 insertions(+), 45 deletions(-) diff --git a/pkg/services/ngalert/notifier/channels/wecom.go b/pkg/services/ngalert/notifier/channels/wecom.go index 762634bdf00..44fbb725bc9 100644 --- a/pkg/services/ngalert/notifier/channels/wecom.go +++ b/pkg/services/ngalert/notifier/channels/wecom.go @@ -14,65 +14,72 @@ import ( "github.com/grafana/grafana/pkg/services/notifications" ) -type WeComConfig struct { - *NotificationChannelConfig - URL string - Message string - Title string +type wecomSettings struct { + URL string `json:"url" yaml:"url"` + Message string `json:"message,omitempty" yaml:"message,omitempty"` + Title string `json:"title,omitempty" yaml:"title,omitempty"` +} + +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) { - url := decryptFunc(context.Background(), config.SecureSettings, "url", config.Settings.Get("url").MustString()) - if url == "" { - return nil, errors.New("could not find webhook URL in settings") +func buildWecomNotifier(factoryConfig FactoryConfig) (*WeComNotifier, error) { + settings, err := buildWecomSettings(factoryConfig) + if err != nil { + 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, - Name: config.Name, - Type: config.Type, - DisableResolveMessage: config.DisableResolveMessage, - Settings: config.Settings, + Uid: factoryConfig.Config.UID, + Name: factoryConfig.Config.Name, + Type: factoryConfig.Config.Type, + DisableResolveMessage: factoryConfig.Config.DisableResolveMessage, + Settings: factoryConfig.Config.Settings, }), - URL: config.URL, - Message: config.Message, - Title: config.Title, - log: log.New("alerting.notifier.wecom"), - ns: ns, - tmpl: t, - } + tmpl: factoryConfig.Template, + log: log.New("alerting.notifier.wecom"), + ns: factoryConfig.NotificationService, + settings: settings, + }, nil } // WeComNotifier is responsible for sending alert notifications to WeCom. type WeComNotifier struct { *Base - URL string - Message string - Title string - tmpl *template.Template - log log.Logger - ns notifications.WebhookSender + tmpl *template.Template + log log.Logger + ns notifications.WebhookSender + settings wecomSettings } // 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.Message), + tmpl(w.settings.Title), + 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), } diff --git a/pkg/services/ngalert/notifier/channels/wecom_test.go b/pkg/services/ngalert/notifier/channels/wecom_test.go index d5343f360b6..dd497fb781a 100644 --- a/pkg/services/ngalert/notifier/channels/wecom_test.go +++ b/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)