From 0282264e540324f0c3417634f847429535655247 Mon Sep 17 00:00:00 2001 From: Oktarian Tilney-Bassett Date: Mon, 10 Oct 2022 13:50:42 +0100 Subject: [PATCH 01/10] Add new source field to PD config, set if defined, else default to client field value --- config/notifiers.go | 1 + notify/pagerduty/pagerduty.go | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/config/notifiers.go b/config/notifiers.go index 7b6f719720..fa52a58808 100644 --- a/config/notifiers.go +++ b/config/notifiers.go @@ -217,6 +217,7 @@ type PagerdutyConfig struct { Details map[string]string `yaml:"details,omitempty" json:"details,omitempty"` Images []PagerdutyImage `yaml:"images,omitempty" json:"images,omitempty"` Links []PagerdutyLink `yaml:"links,omitempty" json:"links,omitempty"` + Source string `yaml:"source,omitempty" json:"source,omitempty"` Severity string `yaml:"severity,omitempty" json:"severity,omitempty"` Class string `yaml:"class,omitempty" json:"class,omitempty"` Component string `yaml:"component,omitempty" json:"component,omitempty"` diff --git a/notify/pagerduty/pagerduty.go b/notify/pagerduty/pagerduty.go index 6c6465f544..ef1513ec8a 100644 --- a/notify/pagerduty/pagerduty.go +++ b/notify/pagerduty/pagerduty.go @@ -209,6 +209,11 @@ func (n *Notifier) notifyV2( level.Debug(n.logger).Log("msg", "Truncated summary", "summary", summary, "key", key) } + payloadSource := n.conf.Client + if n.conf.Source != "" { + payloadSource = n.conf.Source + } + msg := &pagerDutyMessage{ Client: tmpl(n.conf.Client), ClientURL: tmpl(n.conf.ClientURL), @@ -219,7 +224,7 @@ func (n *Notifier) notifyV2( Links: make([]pagerDutyLink, 0, len(n.conf.Links)), Payload: &pagerDutyPayload{ Summary: summary, - Source: tmpl(n.conf.Client), + Source: tmpl(payloadSource), Severity: tmpl(n.conf.Severity), CustomDetails: details, Class: tmpl(n.conf.Class), From 71124d3fb496ff78a9571523ab8eb732a7c59892 Mon Sep 17 00:00:00 2001 From: Oktarian Tilney-Bassett Date: Mon, 10 Oct 2022 13:52:26 +0100 Subject: [PATCH 02/10] Add tests to check behavior and backward compatability is as expected --- notify/pagerduty/pagerduty_test.go | 60 ++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/notify/pagerduty/pagerduty_test.go b/notify/pagerduty/pagerduty_test.go index bb84327203..a2bdaf3636 100644 --- a/notify/pagerduty/pagerduty_test.go +++ b/notify/pagerduty/pagerduty_test.go @@ -448,3 +448,63 @@ func TestPagerDutyEmptySrcHref(t *testing.T) { }...) require.NoError(t, err) } + +func TestPagerDutySourceField(t *testing.T) { + for _, tc := range []struct { + title string + cfg *config.PagerdutyConfig + + expectedSource string + }{ + { + title: "check source field is backward compatible", + cfg: &config.PagerdutyConfig{ + RoutingKey: config.Secret("01234567890123456789012345678901"), + Client: "alert-manager-client", + }, + expectedSource: "alert-manager-client", + }, + { + title: "check source field is set", + cfg: &config.PagerdutyConfig{ + RoutingKey: config.Secret("01234567890123456789012345678901"), + Client: "alert-manager-client", + Source: "alert-manager-source", + }, + expectedSource: "alert-manager-source", + }, + } { + t.Run(tc.title, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + dec := json.NewDecoder(r.Body) + out := make(map[string]interface{}) + err := dec.Decode(&out) + if err != nil { + panic(err) + } + + payloadJSON, err := json.Marshal(out["payload"]) + require.NoError(t, err) + payload := pagerDutyPayload{} + err = json.Unmarshal(payloadJSON, &payload) + require.NoError(t, err) + + require.Equal(t, tc.expectedSource, payload.Source) + })) + defer srv.Close() + u, _ := url.Parse(srv.URL) + + tc.cfg.URL = &config.URL{URL: u} + tc.cfg.HTTPConfig = &commoncfg.HTTPClientConfig{} + pd, err := New(tc.cfg, test.CreateTmpl(t), log.NewNopLogger()) + require.NoError(t, err) + + ctx := context.Background() + ctx = notify.WithGroupKey(ctx, "1") + + retry, err := pd.Notify(ctx) + require.NoError(t, err) + require.Equal(t, false, retry) + }) + } +} From 5488b74ba3b2a364798027eb22e53205facee69c Mon Sep 17 00:00:00 2001 From: Oktarian Tilney-Bassett Date: Mon, 10 Oct 2022 13:53:02 +0100 Subject: [PATCH 03/10] Update docs with new field --- docs/configuration.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/configuration.md b/docs/configuration.md index c1871ef2b2..c76f33a1aa 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -660,6 +660,9 @@ service_key: # Severity of the incident. [ severity: | default = 'error' ] +# Unique location of the affected system. +[ source: | default = client ] + # A set of arbitrary key/value pairs that provide further detail # about the incident. [ details: { : , ... } | default = { From 8c79fb9ab91aa4a2c9ef8810a6eae4e4f58f609f Mon Sep 17 00:00:00 2001 From: Oktarian Tilney-Bassett Date: Wed, 12 Oct 2022 11:11:25 +0100 Subject: [PATCH 04/10] Handle default source config value on unmarshaling Signed-off-by: Oktarian Tilney-Bassett --- config/notifiers.go | 3 ++ config/notifiers_test.go | 35 +++++++++++++++++ notify/pagerduty/pagerduty.go | 7 +--- notify/pagerduty/pagerduty_test.go | 60 ------------------------------ 4 files changed, 39 insertions(+), 66 deletions(-) diff --git a/config/notifiers.go b/config/notifiers.go index fa52a58808..da561730af 100644 --- a/config/notifiers.go +++ b/config/notifiers.go @@ -250,6 +250,9 @@ func (c *PagerdutyConfig) UnmarshalYAML(unmarshal func(interface{}) error) error if c.Details == nil { c.Details = make(map[string]string) } + if c.Source == "" { + c.Source = c.Client + } for k, v := range DefaultPagerdutyDetails { if _, ok := c.Details[k]; !ok { c.Details[k] = v diff --git a/config/notifiers_test.go b/config/notifiers_test.go index 889abab627..de9fa8eea3 100644 --- a/config/notifiers_test.go +++ b/config/notifiers_test.go @@ -14,6 +14,7 @@ package config import ( + "github.com/stretchr/testify/require" "strings" "testing" @@ -146,6 +147,40 @@ details: } } +func TestPagerDutySource(t *testing.T) { + for _, tc := range []struct { + title string + in string + + expectedSource string + }{ + { + title: "check source field is backward compatible", + in: ` +routing_key: 'xyz' +client: 'alert-manager-client' +`, + expectedSource: "alert-manager-client", + }, + { + title: "check source field is set", + in: ` +routing_key: 'xyz' +client: 'alert-manager-client' +source: 'alert-manager-source' +`, + expectedSource: "alert-manager-source", + }, + } { + t.Run(tc.title, func(t *testing.T) { + var cfg PagerdutyConfig + err := yaml.UnmarshalStrict([]byte(tc.in), &cfg) + require.NoError(t, err) + require.Equal(t, tc.expectedSource, cfg.Source) + }) + } +} + func TestWebhookURLIsPresent(t *testing.T) { in := `{}` var cfg WebhookConfig diff --git a/notify/pagerduty/pagerduty.go b/notify/pagerduty/pagerduty.go index ef1513ec8a..32cd6b418f 100644 --- a/notify/pagerduty/pagerduty.go +++ b/notify/pagerduty/pagerduty.go @@ -209,11 +209,6 @@ func (n *Notifier) notifyV2( level.Debug(n.logger).Log("msg", "Truncated summary", "summary", summary, "key", key) } - payloadSource := n.conf.Client - if n.conf.Source != "" { - payloadSource = n.conf.Source - } - msg := &pagerDutyMessage{ Client: tmpl(n.conf.Client), ClientURL: tmpl(n.conf.ClientURL), @@ -224,7 +219,7 @@ func (n *Notifier) notifyV2( Links: make([]pagerDutyLink, 0, len(n.conf.Links)), Payload: &pagerDutyPayload{ Summary: summary, - Source: tmpl(payloadSource), + Source: tmpl(n.conf.Source), Severity: tmpl(n.conf.Severity), CustomDetails: details, Class: tmpl(n.conf.Class), diff --git a/notify/pagerduty/pagerduty_test.go b/notify/pagerduty/pagerduty_test.go index a2bdaf3636..bb84327203 100644 --- a/notify/pagerduty/pagerduty_test.go +++ b/notify/pagerduty/pagerduty_test.go @@ -448,63 +448,3 @@ func TestPagerDutyEmptySrcHref(t *testing.T) { }...) require.NoError(t, err) } - -func TestPagerDutySourceField(t *testing.T) { - for _, tc := range []struct { - title string - cfg *config.PagerdutyConfig - - expectedSource string - }{ - { - title: "check source field is backward compatible", - cfg: &config.PagerdutyConfig{ - RoutingKey: config.Secret("01234567890123456789012345678901"), - Client: "alert-manager-client", - }, - expectedSource: "alert-manager-client", - }, - { - title: "check source field is set", - cfg: &config.PagerdutyConfig{ - RoutingKey: config.Secret("01234567890123456789012345678901"), - Client: "alert-manager-client", - Source: "alert-manager-source", - }, - expectedSource: "alert-manager-source", - }, - } { - t.Run(tc.title, func(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - dec := json.NewDecoder(r.Body) - out := make(map[string]interface{}) - err := dec.Decode(&out) - if err != nil { - panic(err) - } - - payloadJSON, err := json.Marshal(out["payload"]) - require.NoError(t, err) - payload := pagerDutyPayload{} - err = json.Unmarshal(payloadJSON, &payload) - require.NoError(t, err) - - require.Equal(t, tc.expectedSource, payload.Source) - })) - defer srv.Close() - u, _ := url.Parse(srv.URL) - - tc.cfg.URL = &config.URL{URL: u} - tc.cfg.HTTPConfig = &commoncfg.HTTPClientConfig{} - pd, err := New(tc.cfg, test.CreateTmpl(t), log.NewNopLogger()) - require.NoError(t, err) - - ctx := context.Background() - ctx = notify.WithGroupKey(ctx, "1") - - retry, err := pd.Notify(ctx) - require.NoError(t, err) - require.Equal(t, false, retry) - }) - } -} From e3773cc0e0b7f772808c9f8a87244ab74bdbc07e Mon Sep 17 00:00:00 2001 From: Oktarian Tilney-Bassett Date: Wed, 12 Oct 2022 11:20:24 +0100 Subject: [PATCH 05/10] Fix goimports Signed-off-by: Oktarian Tilney-Bassett --- config/notifiers_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/notifiers_test.go b/config/notifiers_test.go index de9fa8eea3..0d38623163 100644 --- a/config/notifiers_test.go +++ b/config/notifiers_test.go @@ -14,10 +14,11 @@ package config import ( - "github.com/stretchr/testify/require" "strings" "testing" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v2" ) From 7cee5aa4b1d7c8b7b1227cfdd3fecdf792eb81a9 Mon Sep 17 00:00:00 2001 From: Oktarian Tilney-Bassett Date: Wed, 12 Oct 2022 11:20:24 +0100 Subject: [PATCH 06/10] Sign commits Signed-off-by: Oktarian Tilney-Bassett --- config/notifiers_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/notifiers_test.go b/config/notifiers_test.go index de9fa8eea3..0d38623163 100644 --- a/config/notifiers_test.go +++ b/config/notifiers_test.go @@ -14,10 +14,11 @@ package config import ( - "github.com/stretchr/testify/require" "strings" "testing" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v2" ) From 3fe17f54db197f8545e34545002dd771c86be3a8 Mon Sep 17 00:00:00 2001 From: Oktarian Tilney-Bassett Date: Mon, 10 Oct 2022 13:52:26 +0100 Subject: [PATCH 07/10] Add tests to check behavior and backward compatability is as expected --- notify/pagerduty/pagerduty_test.go | 60 ++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/notify/pagerduty/pagerduty_test.go b/notify/pagerduty/pagerduty_test.go index bb84327203..a2bdaf3636 100644 --- a/notify/pagerduty/pagerduty_test.go +++ b/notify/pagerduty/pagerduty_test.go @@ -448,3 +448,63 @@ func TestPagerDutyEmptySrcHref(t *testing.T) { }...) require.NoError(t, err) } + +func TestPagerDutySourceField(t *testing.T) { + for _, tc := range []struct { + title string + cfg *config.PagerdutyConfig + + expectedSource string + }{ + { + title: "check source field is backward compatible", + cfg: &config.PagerdutyConfig{ + RoutingKey: config.Secret("01234567890123456789012345678901"), + Client: "alert-manager-client", + }, + expectedSource: "alert-manager-client", + }, + { + title: "check source field is set", + cfg: &config.PagerdutyConfig{ + RoutingKey: config.Secret("01234567890123456789012345678901"), + Client: "alert-manager-client", + Source: "alert-manager-source", + }, + expectedSource: "alert-manager-source", + }, + } { + t.Run(tc.title, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + dec := json.NewDecoder(r.Body) + out := make(map[string]interface{}) + err := dec.Decode(&out) + if err != nil { + panic(err) + } + + payloadJSON, err := json.Marshal(out["payload"]) + require.NoError(t, err) + payload := pagerDutyPayload{} + err = json.Unmarshal(payloadJSON, &payload) + require.NoError(t, err) + + require.Equal(t, tc.expectedSource, payload.Source) + })) + defer srv.Close() + u, _ := url.Parse(srv.URL) + + tc.cfg.URL = &config.URL{URL: u} + tc.cfg.HTTPConfig = &commoncfg.HTTPClientConfig{} + pd, err := New(tc.cfg, test.CreateTmpl(t), log.NewNopLogger()) + require.NoError(t, err) + + ctx := context.Background() + ctx = notify.WithGroupKey(ctx, "1") + + retry, err := pd.Notify(ctx) + require.NoError(t, err) + require.Equal(t, false, retry) + }) + } +} From 9afe0be7c1d7b968954bbf69ceb3895738e0df43 Mon Sep 17 00:00:00 2001 From: Oktarian Tilney-Bassett Date: Mon, 10 Oct 2022 13:53:02 +0100 Subject: [PATCH 08/10] Update docs with new field --- docs/configuration.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/configuration.md b/docs/configuration.md index c1871ef2b2..c76f33a1aa 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -660,6 +660,9 @@ service_key: # Severity of the incident. [ severity: | default = 'error' ] +# Unique location of the affected system. +[ source: | default = client ] + # A set of arbitrary key/value pairs that provide further detail # about the incident. [ details: { : , ... } | default = { From dd92a4a9f0c1d6c562d4770fa3a87d809c775eb1 Mon Sep 17 00:00:00 2001 From: Oktarian Tilney-Bassett Date: Wed, 12 Oct 2022 11:11:25 +0100 Subject: [PATCH 09/10] Handle default source config value on unmarshaling Signed-off-by: Oktarian Tilney-Bassett --- config/notifiers.go | 3 ++ config/notifiers_test.go | 35 +++++++++++++++++ notify/pagerduty/pagerduty.go | 7 +--- notify/pagerduty/pagerduty_test.go | 60 ------------------------------ 4 files changed, 39 insertions(+), 66 deletions(-) diff --git a/config/notifiers.go b/config/notifiers.go index fa52a58808..da561730af 100644 --- a/config/notifiers.go +++ b/config/notifiers.go @@ -250,6 +250,9 @@ func (c *PagerdutyConfig) UnmarshalYAML(unmarshal func(interface{}) error) error if c.Details == nil { c.Details = make(map[string]string) } + if c.Source == "" { + c.Source = c.Client + } for k, v := range DefaultPagerdutyDetails { if _, ok := c.Details[k]; !ok { c.Details[k] = v diff --git a/config/notifiers_test.go b/config/notifiers_test.go index 889abab627..de9fa8eea3 100644 --- a/config/notifiers_test.go +++ b/config/notifiers_test.go @@ -14,6 +14,7 @@ package config import ( + "github.com/stretchr/testify/require" "strings" "testing" @@ -146,6 +147,40 @@ details: } } +func TestPagerDutySource(t *testing.T) { + for _, tc := range []struct { + title string + in string + + expectedSource string + }{ + { + title: "check source field is backward compatible", + in: ` +routing_key: 'xyz' +client: 'alert-manager-client' +`, + expectedSource: "alert-manager-client", + }, + { + title: "check source field is set", + in: ` +routing_key: 'xyz' +client: 'alert-manager-client' +source: 'alert-manager-source' +`, + expectedSource: "alert-manager-source", + }, + } { + t.Run(tc.title, func(t *testing.T) { + var cfg PagerdutyConfig + err := yaml.UnmarshalStrict([]byte(tc.in), &cfg) + require.NoError(t, err) + require.Equal(t, tc.expectedSource, cfg.Source) + }) + } +} + func TestWebhookURLIsPresent(t *testing.T) { in := `{}` var cfg WebhookConfig diff --git a/notify/pagerduty/pagerduty.go b/notify/pagerduty/pagerduty.go index ef1513ec8a..32cd6b418f 100644 --- a/notify/pagerduty/pagerduty.go +++ b/notify/pagerduty/pagerduty.go @@ -209,11 +209,6 @@ func (n *Notifier) notifyV2( level.Debug(n.logger).Log("msg", "Truncated summary", "summary", summary, "key", key) } - payloadSource := n.conf.Client - if n.conf.Source != "" { - payloadSource = n.conf.Source - } - msg := &pagerDutyMessage{ Client: tmpl(n.conf.Client), ClientURL: tmpl(n.conf.ClientURL), @@ -224,7 +219,7 @@ func (n *Notifier) notifyV2( Links: make([]pagerDutyLink, 0, len(n.conf.Links)), Payload: &pagerDutyPayload{ Summary: summary, - Source: tmpl(payloadSource), + Source: tmpl(n.conf.Source), Severity: tmpl(n.conf.Severity), CustomDetails: details, Class: tmpl(n.conf.Class), diff --git a/notify/pagerduty/pagerduty_test.go b/notify/pagerduty/pagerduty_test.go index a2bdaf3636..bb84327203 100644 --- a/notify/pagerduty/pagerduty_test.go +++ b/notify/pagerduty/pagerduty_test.go @@ -448,63 +448,3 @@ func TestPagerDutyEmptySrcHref(t *testing.T) { }...) require.NoError(t, err) } - -func TestPagerDutySourceField(t *testing.T) { - for _, tc := range []struct { - title string - cfg *config.PagerdutyConfig - - expectedSource string - }{ - { - title: "check source field is backward compatible", - cfg: &config.PagerdutyConfig{ - RoutingKey: config.Secret("01234567890123456789012345678901"), - Client: "alert-manager-client", - }, - expectedSource: "alert-manager-client", - }, - { - title: "check source field is set", - cfg: &config.PagerdutyConfig{ - RoutingKey: config.Secret("01234567890123456789012345678901"), - Client: "alert-manager-client", - Source: "alert-manager-source", - }, - expectedSource: "alert-manager-source", - }, - } { - t.Run(tc.title, func(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - dec := json.NewDecoder(r.Body) - out := make(map[string]interface{}) - err := dec.Decode(&out) - if err != nil { - panic(err) - } - - payloadJSON, err := json.Marshal(out["payload"]) - require.NoError(t, err) - payload := pagerDutyPayload{} - err = json.Unmarshal(payloadJSON, &payload) - require.NoError(t, err) - - require.Equal(t, tc.expectedSource, payload.Source) - })) - defer srv.Close() - u, _ := url.Parse(srv.URL) - - tc.cfg.URL = &config.URL{URL: u} - tc.cfg.HTTPConfig = &commoncfg.HTTPClientConfig{} - pd, err := New(tc.cfg, test.CreateTmpl(t), log.NewNopLogger()) - require.NoError(t, err) - - ctx := context.Background() - ctx = notify.WithGroupKey(ctx, "1") - - retry, err := pd.Notify(ctx) - require.NoError(t, err) - require.Equal(t, false, retry) - }) - } -} From dcd88e0392ab3dfed9f13092ad10620a1b536c61 Mon Sep 17 00:00:00 2001 From: Oktarian Tilney-Bassett Date: Wed, 12 Oct 2022 11:20:24 +0100 Subject: [PATCH 10/10] Sign commits Signed-off-by: Oktarian Tilney-Bassett --- config/notifiers_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/notifiers_test.go b/config/notifiers_test.go index de9fa8eea3..0d38623163 100644 --- a/config/notifiers_test.go +++ b/config/notifiers_test.go @@ -14,10 +14,11 @@ package config import ( - "github.com/stretchr/testify/require" "strings" "testing" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v2" )