From a98afcf22ab6f2a830b3129f15c0403d2007b5e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Knecht?= Date: Thu, 24 Sep 2020 13:42:41 +0200 Subject: [PATCH] notify/pagerduty: Filter out empty images and links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PagerDuty Event API v2 [1] requires images to have an `src` property, and links to have an `href` property. This commit filters out images and links that don't satisfy those conditions, to avoid getting an HTTP 400 error in response. This also adds flexibilty when using templates to configure images and links, as it's now possible to omit images or links by letting the template return an empty string for the `src` or `href` property, respectively. [1]: https://developer.pagerduty.com/docs/events-api-v2/trigger-events/#context-properties Signed-off-by: Benoît Knecht --- notify/pagerduty/pagerduty.go | 30 ++++--- notify/pagerduty/pagerduty_test.go | 126 +++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 9 deletions(-) diff --git a/notify/pagerduty/pagerduty.go b/notify/pagerduty/pagerduty.go index 7d85392bb1..a145135bba 100644 --- a/notify/pagerduty/pagerduty.go +++ b/notify/pagerduty/pagerduty.go @@ -215,8 +215,8 @@ func (n *Notifier) notifyV2( RoutingKey: tmpl(string(n.conf.RoutingKey)), EventAction: eventType, DedupKey: key.Hash(), - Images: make([]pagerDutyImage, len(n.conf.Images)), - Links: make([]pagerDutyLink, len(n.conf.Links)), + Images: make([]pagerDutyImage, 0, len(n.conf.Images)), + Links: make([]pagerDutyLink, 0, len(n.conf.Links)), Payload: &pagerDutyPayload{ Summary: summary, Source: tmpl(n.conf.Client), @@ -228,15 +228,27 @@ func (n *Notifier) notifyV2( }, } - for index, item := range n.conf.Images { - msg.Images[index].Src = tmpl(item.Src) - msg.Images[index].Alt = tmpl(item.Alt) - msg.Images[index].Href = tmpl(item.Href) + for _, item := range n.conf.Images { + image := pagerDutyImage{ + Src: tmpl(item.Src), + Alt: tmpl(item.Alt), + Href: tmpl(item.Href), + } + + if image.Src != "" { + msg.Images = append(msg.Images, image) + } } - for index, item := range n.conf.Links { - msg.Links[index].HRef = tmpl(item.Href) - msg.Links[index].Text = tmpl(item.Text) + for _, item := range n.conf.Links { + link := pagerDutyLink{ + HRef: tmpl(item.Href), + Text: tmpl(item.Text), + } + + if link.HRef != "" { + msg.Links = append(msg.Links, link) + } } if tmplErr != nil { diff --git a/notify/pagerduty/pagerduty_test.go b/notify/pagerduty/pagerduty_test.go index 64fac902a4..0830923628 100644 --- a/notify/pagerduty/pagerduty_test.go +++ b/notify/pagerduty/pagerduty_test.go @@ -322,3 +322,129 @@ func TestEventSizeEnforcement(t *testing.T) { require.NoError(t, err) require.Contains(t, encodedV2.String(), `"custom_details":{"error":"Custom details have been removed because the original event exceeds the maximum size of 512KB"}`) } + +func TestPagerDutyEmptySrcHref(t *testing.T) { + type pagerDutyEvent struct { + RoutingKey string `json:"routing_key"` + EventAction string `json:"event_action"` + DedupKey string `json:"dedup_key"` + Payload pagerDutyPayload `json:"payload"` + Images []pagerDutyImage + Links []pagerDutyLink + } + + images := []config.PagerdutyImage{ + { + Src: "", + Alt: "Empty src", + Href: "https://example.com/", + }, + { + Src: "https://example.com/cat.jpg", + Alt: "Empty href", + Href: "", + }, + { + Src: "https://example.com/cat.jpg", + Alt: "", + Href: "https://example.com/", + }, + } + + links := []config.PagerdutyLink{ + { + Href: "", + Text: "Empty href", + }, + { + Href: "https://example.com/", + Text: "", + }, + } + + expectedImages := make([]pagerDutyImage, 0, len(images)) + for _, image := range images { + if image.Src == "" { + continue + } + expectedImages = append(expectedImages, pagerDutyImage{ + Src: image.Src, + Alt: image.Alt, + Href: image.Href, + }) + } + + expectedLinks := make([]pagerDutyLink, 0, len(links)) + for _, link := range links { + if link.Href == "" { + continue + } + expectedLinks = append(expectedLinks, pagerDutyLink{ + HRef: link.Href, + Text: link.Text, + }) + } + + server := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + decoder := json.NewDecoder(r.Body) + var event pagerDutyEvent + if err := decoder.Decode(&event); err != nil { + panic(err) + } + + if event.RoutingKey == "" || event.EventAction == "" { + http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest) + return + } + + for _, image := range event.Images { + if image.Src == "" { + http.Error(w, "Event object is invalid: 'image src' is missing or blank", http.StatusBadRequest) + return + } + } + + for _, link := range event.Links { + if link.HRef == "" { + http.Error(w, "Event object is invalid: 'link href' is missing or blank", http.StatusBadRequest) + return + } + } + + require.Equal(t, expectedImages, event.Images) + require.Equal(t, expectedLinks, event.Links) + }, + )) + defer server.Close() + + url, err := url.Parse(server.URL) + require.NoError(t, err) + + pagerDutyConfig := config.PagerdutyConfig{ + HTTPConfig: &commoncfg.HTTPClientConfig{}, + RoutingKey: config.Secret("01234567890123456789012345678901"), + URL: &config.URL{URL: url}, + Images: images, + Links: links, + } + + pagerDuty, err := New(&pagerDutyConfig, test.CreateTmpl(t), log.NewNopLogger()) + require.NoError(t, err) + + ctx := context.Background() + ctx = notify.WithGroupKey(ctx, "1") + + _, err = pagerDuty.Notify(ctx, []*types.Alert{ + { + Alert: model.Alert{ + Labels: model.LabelSet{ + "lbl1": "val1", + }, + StartsAt: time.Now(), + EndsAt: time.Now().Add(time.Hour), + }, + }, + }...) + require.NoError(t, err) +}