From 0b3c60fc22acdc8d04462722e6c78f20f556ad84 Mon Sep 17 00:00:00 2001 From: Yuriy Tseretyan Date: Tue, 27 Sep 2022 16:58:18 -0400 Subject: [PATCH] Alerting: Refactor notifiers VictorOps, Threema, Telegram to use encoding/json to parse settings instead of simplejson (#55779) * update victorops * update threema * update telegram --- .../ngalert/notifier/channels/telegram.go | 89 +++++++------ .../notifier/channels/telegram_test.go | 5 +- .../ngalert/notifier/channels/threema.go | 123 +++++++++-------- .../ngalert/notifier/channels/threema_test.go | 13 +- .../ngalert/notifier/channels/victorops.go | 126 ++++++++++-------- .../notifier/channels/victorops_test.go | 11 +- 6 files changed, 194 insertions(+), 173 deletions(-) diff --git a/pkg/services/ngalert/notifier/channels/telegram.go b/pkg/services/ngalert/notifier/channels/telegram.go index feae141eff4..3e55a93faf4 100644 --- a/pkg/services/ngalert/notifier/channels/telegram.go +++ b/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 { - *NotificationChannelConfig - BotToken string - ChatID string - Message string +type telegramSettings struct { + BotToken string `json:"bottoken,omitempty" yaml:"bottoken,omitempty"` + ChatID string `json:"chatid,omitempty" yaml:"chatid,omitempty"` + Message string `json:"message,omitempty" yaml:"message,omitempty"` +} + +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 -} - -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 + return notifier, 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, - Name: config.Name, - Type: config.Type, - DisableResolveMessage: config.DisableResolveMessage, - Settings: config.Settings, + Uid: fc.Config.UID, + Name: fc.Config.Name, + Type: fc.Config.Type, + DisableResolveMessage: fc.Config.DisableResolveMessage, + Settings: fc.Config.Settings, }), - BotToken: config.BotToken, - ChatID: config.ChatID, - Message: config.Message, - tmpl: t, + tmpl: fc.Template, log: log.New("alerting.notifier.telegram"), - images: images, - ns: ns, - } + images: fc.ImageStore, + 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{ diff --git a/pkg/services/ngalert/notifier/channels/telegram_test.go b/pkg/services/ngalert/notifier/channels/telegram_test.go index f0a61692958..8a1a573c7b5 100644 --- a/pkg/services/ngalert/notifier/channels/telegram_test.go +++ b/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...) diff --git a/pkg/services/ngalert/notifier/channels/threema.go b/pkg/services/ngalert/notifier/channels/threema.go index 143fa672f5c..047689ff75c 100644 --- a/pkg/services/ngalert/notifier/channels/threema.go +++ b/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 + log log.Logger + images ImageStore + ns notifications.WebhookSender + tmpl *template.Template + settings threemaSettings } -type ThreemaConfig struct { - *NotificationChannelConfig - GatewayID string - RecipientID string - APISecret string +type threemaSettings struct { + GatewayID string `json:"gateway_id,omitempty" yaml:"gateway_id,omitempty"` + RecipientID string `json:"recipient_id,omitempty" yaml:"recipient_id,omitempty"` + APISecret string `json:"api_secret,omitempty" yaml:"api_secret,omitempty"` +} + +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) { - gatewayID := config.Settings.Get("gateway_id").MustString() - if gatewayID == "" { - return nil, errors.New("could not find Threema Gateway ID in settings") +func NewThreemaNotifier(fc FactoryConfig) (*ThreemaNotifier, error) { + settings, err := buildThreemaSettings(fc) + if err != nil { + 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, - Name: config.Name, - Type: config.Type, - DisableResolveMessage: config.DisableResolveMessage, - Settings: config.Settings, + Uid: fc.Config.UID, + Name: fc.Config.Name, + Type: fc.Config.Type, + DisableResolveMessage: fc.Config.DisableResolveMessage, + Settings: fc.Config.Settings, }), - GatewayID: config.GatewayID, - RecipientID: config.RecipientID, - APISecret: config.APISecret, - log: log.New("alerting.notifier.threema"), - images: images, - ns: ns, - tmpl: t, - } + log: log.New("alerting.notifier.threema"), + images: fc.ImageStore, + ns: fc.NotificationService, + 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("to", tn.RecipientID) - data.Set("secret", tn.APISecret) + data.Set("from", tn.settings.GatewayID) + data.Set("to", tn.settings.RecipientID) + data.Set("secret", tn.settings.APISecret) // Determine emoji stateEmoji := "\u26A0\uFE0F " // Warning sign diff --git a/pkg/services/ngalert/notifier/channels/threema_test.go b/pkg/services/ngalert/notifier/channels/threema_test.go index 7c1e1cf77db..ce905e0f806 100644 --- a/pkg/services/ngalert/notifier/channels/threema_test.go +++ b/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) diff --git a/pkg/services/ngalert/notifier/channels/victorops.go b/pkg/services/ngalert/notifier/channels/victorops.go index 56bb7355c78..927504a8f1d 100644 --- a/pkg/services/ngalert/notifier/channels/victorops.go +++ b/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 { - *NotificationChannelConfig - URL string - MessageType string +type victorOpsSettings struct { + URL string `json:"url,omitempty" yaml:"url,omitempty"` + MessageType string `json:"messageType,omitempty" yaml:"messageType,omitempty"` +} + +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 -} - -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 + return notifier, 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, - Name: config.Name, - Type: config.Type, - DisableResolveMessage: config.DisableResolveMessage, - Settings: config.Settings, + Uid: fc.Config.UID, + Name: fc.Config.Name, + Type: fc.Config.Type, + DisableResolveMessage: fc.Config.DisableResolveMessage, + Settings: fc.Config.Settings, }), - URL: config.URL, - MessageType: config.MessageType, - log: log.New("alerting.notifier.victorops"), - images: images, - ns: ns, - tmpl: t, - } + log: log.New("alerting.notifier.victorops"), + images: fc.ImageStore, + ns: fc.NotificationService, + 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 + 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.Set("message_type", messageType) - bodyJSON.Set("entity_id", groupKey.Hash()) - bodyJSON.Set("entity_display_name", tmpl(DefaultMessageTitleEmbed)) - bodyJSON.Set("timestamp", time.Now().Unix()) - bodyJSON.Set("state_message", tmpl(DefaultMessageEmbed)) - bodyJSON.Set("monitoring_tool", "Grafana v"+setting.BuildVersion) + bodyJSON := map[string]interface{}{ + "message_type": messageType, + "entity_id": groupKey.Hash(), + "entity_display_name": tmpl(DefaultMessageTitleEmbed), + "timestamp": time.Now().Unix(), + "state_message": tmpl(DefaultMessageEmbed), + "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()) - tmplErr = nil + vn.log.Info("failed to expand URL template", "err", tmplErr.Error(), "fallback", vn.settings.URL) + u = vn.settings.URL } - u := tmpl(vn.URL) - if tmplErr != nil { - vn.log.Info("failed to template VictorOps URL", "err", tmplErr.Error(), "fallback", vn.URL) - u = vn.URL - } - - b, err := bodyJSON.MarshalJSON() + b, err := json.Marshal(bodyJSON) 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 } diff --git a/pkg/services/ngalert/notifier/channels/victorops_test.go b/pkg/services/ngalert/notifier/channels/victorops_test.go index 251618bac8c..a85eb7c659a 100644 --- a/pkg/services/ngalert/notifier/channels/victorops_test.go +++ b/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)