From 3b06f52bab976b71293f3483721c82e9c7b48a16 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Wed, 12 May 2021 07:58:16 -0400 Subject: [PATCH] Alerting/allow empty receiver (#33962) * simplifies yaml unmarshaling: PostableApiReceiver * allow empty receiver type * allows name only receivers (blackhole) * better receiver type parsing * linting --- pkg/services/ngalert/api/forked_am.go | 14 +- .../api/tooling/definitions/alertmanager.go | 100 +++++++---- .../tooling/definitions/alertmanager_test.go | 158 ++++++++++++++++++ 3 files changed, 229 insertions(+), 43 deletions(-) diff --git a/pkg/services/ngalert/api/forked_am.go b/pkg/services/ngalert/api/forked_am.go index 5eec484ba6f..463ded6cf85 100644 --- a/pkg/services/ngalert/api/forked_am.go +++ b/pkg/services/ngalert/api/forked_am.go @@ -117,22 +117,16 @@ func (am *ForkedAMSvc) RoutePostAlertingConfig(ctx *models.ReqContext, body apim return response.Error(400, err.Error(), nil) } - backendType, err := backendType(ctx, am.DatasourceCache) + b, err := backendType(ctx, am.DatasourceCache) if err != nil { return response.Error(400, err.Error(), nil) } - payloadType := body.AlertmanagerConfig.Type() - - if backendType != payloadType { + if err := body.AlertmanagerConfig.ReceiverType().MatchesBackend(b); err != nil { return response.Error( 400, - fmt.Sprintf( - "unexpected backend type (%v) vs payload type (%v)", - backendType, - payloadType, - ), - nil, + "bad match", + err, ) } diff --git a/pkg/services/ngalert/api/tooling/definitions/alertmanager.go b/pkg/services/ngalert/api/tooling/definitions/alertmanager.go index fdc9792900d..b4429602f12 100644 --- a/pkg/services/ngalert/api/tooling/definitions/alertmanager.go +++ b/pkg/services/ngalert/api/tooling/definitions/alertmanager.go @@ -4,6 +4,7 @@ import ( "encoding/base64" "encoding/json" "fmt" + "reflect" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/models" @@ -381,6 +382,8 @@ func (c *GettableApiAlertingConfig) validate() error { hasGrafReceivers = true case AlertmanagerReceiverType: hasAMReceivers = true + default: + continue } } @@ -398,19 +401,6 @@ func (c *GettableApiAlertingConfig) validate() error { return nil } -// Type requires validate has been called and just checks the first receiver type -func (c *GettableApiAlertingConfig) Type() (backend Backend) { - for _, r := range c.Receivers { - switch r.Type() { - case GrafanaReceiverType: - return GrafanaBackend - case AlertmanagerReceiverType: - return AlertmanagerBackend - } - } - return -} - // Config is the top-level configuration for Alertmanager's config files. type Config struct { Global *config.GlobalConfig `yaml:"global,omitempty" json:"global,omitempty"` @@ -448,6 +438,8 @@ func (c *PostableApiAlertingConfig) validate() error { hasGrafReceivers = true case AlertmanagerReceiverType: hasAMReceivers = true + default: + continue } } @@ -466,22 +458,26 @@ func (c *PostableApiAlertingConfig) validate() error { } // Type requires validate has been called and just checks the first receiver type -func (c *PostableApiAlertingConfig) Type() (backend Backend) { +func (c *PostableApiAlertingConfig) ReceiverType() ReceiverType { for _, r := range c.Receivers { switch r.Type() { case GrafanaReceiverType: - return GrafanaBackend + return GrafanaReceiverType case AlertmanagerReceiverType: - return AlertmanagerBackend + return AlertmanagerReceiverType + default: + continue } } - return + return EmptyReceiverType } // AllReceivers will recursively walk a routing tree and return a list of all the // referenced receiver names. func AllReceivers(route *config.Route) (res []string) { - res = append(res, route.Receiver) + if route.Receiver != "" { + res = append(res, route.Receiver) + } for _, subRoute := range route.Routes { res = append(res, AllReceivers(subRoute)...) } @@ -494,10 +490,52 @@ type PostableGrafanaReceiver models.CreateAlertNotificationCommand type ReceiverType int const ( - GrafanaReceiverType ReceiverType = iota + GrafanaReceiverType ReceiverType = 1 << iota AlertmanagerReceiverType + EmptyReceiverType = GrafanaReceiverType | AlertmanagerReceiverType ) +func (r ReceiverType) String() string { + switch r { + case GrafanaReceiverType: + return "grafana" + case AlertmanagerReceiverType: + return "alertmanager" + case EmptyReceiverType: + return "empty" + default: + return "unknown" + } +} + +// Can determines whether a receiver type can implement another receiver type. +// This is useful as receivers with just names but no contact points +// are valid in all backends. +func (r ReceiverType) Can(other ReceiverType) bool { return r&other != 0 } + +// MatchesBackend determines if a config payload can be sent to a particular backend type +func (r ReceiverType) MatchesBackend(backend Backend) error { + msg := func(backend Backend, receiver ReceiverType) error { + return fmt.Errorf( + "unexpected backend type (%s) for receiver type (%s)", + backend.String(), + receiver.String(), + ) + } + var ok bool + switch backend { + case GrafanaBackend: + ok = r.Can(GrafanaReceiverType) + case AlertmanagerBackend: + ok = r.Can(AlertmanagerReceiverType) + default: + } + if !ok { + return msg(backend, r) + } + return nil +} + type GettableApiReceiver struct { config.Receiver `yaml:",inline"` GettableGrafanaReceivers `yaml:",inline"` @@ -554,25 +592,14 @@ type PostableApiReceiver struct { } func (r *PostableApiReceiver) UnmarshalYAML(unmarshal func(interface{}) error) error { - var grafanaReceivers PostableGrafanaReceivers - if err := unmarshal(&grafanaReceivers); err != nil { + if err := unmarshal(&r.PostableGrafanaReceivers); err != nil { return err } - r.PostableGrafanaReceivers = grafanaReceivers - var cfg config.Receiver - if err := unmarshal(&cfg); err != nil { + if err := unmarshal(&r.Receiver); err != nil { return err } - r.Name = cfg.Name - r.EmailConfigs = cfg.EmailConfigs - r.PagerdutyConfigs = cfg.PagerdutyConfigs - r.SlackConfigs = cfg.SlackConfigs - r.WebhookConfigs = cfg.WebhookConfigs - r.OpsGenieConfigs = cfg.OpsGenieConfigs - r.WechatConfigs = cfg.WechatConfigs - r.PushoverConfigs = cfg.PushoverConfigs - r.VictorOpsConfigs = cfg.VictorOpsConfigs + return nil } @@ -617,6 +644,13 @@ func (r *PostableApiReceiver) Type() ReceiverType { if len(r.PostableGrafanaReceivers.GrafanaManagedReceivers) > 0 { return GrafanaReceiverType } + + cpy := r.Receiver + cpy.Name = "" + if reflect.ValueOf(cpy).IsZero() { + return EmptyReceiverType + } + return AlertmanagerReceiverType } diff --git a/pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go b/pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go index 0e0bca32008..529e69c56b2 100644 --- a/pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go +++ b/pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go @@ -69,6 +69,50 @@ func Test_ApiReceiver_Marshaling(t *testing.T) { } } +func Test_APIReceiverType(t *testing.T) { + for _, tc := range []struct { + desc string + input PostableApiReceiver + expected ReceiverType + }{ + { + desc: "empty", + input: PostableApiReceiver{ + Receiver: config.Receiver{ + Name: "foo", + }, + }, + expected: EmptyReceiverType, + }, + { + desc: "am", + input: PostableApiReceiver{ + Receiver: config.Receiver{ + Name: "foo", + EmailConfigs: []*config.EmailConfig{{}}, + }, + }, + expected: AlertmanagerReceiverType, + }, + { + desc: "graf", + input: PostableApiReceiver{ + Receiver: config.Receiver{ + Name: "foo", + }, + PostableGrafanaReceivers: PostableGrafanaReceivers{ + GrafanaManagedReceivers: []*PostableGrafanaReceiver{{}}, + }, + }, + expected: GrafanaReceiverType, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + require.Equal(t, tc.expected, tc.input.Type()) + }) + } +} + func Test_AllReceivers(t *testing.T) { input := &config.Route{ Receiver: "foo", @@ -88,6 +132,10 @@ func Test_AllReceivers(t *testing.T) { } require.Equal(t, []string{"foo", "bar", "bazz", "buzz"}, AllReceivers(input)) + + // test empty + var empty []string + require.Equal(t, empty, AllReceivers(&config.Route{})) } func Test_ApiAlertingConfig_Marshaling(t *testing.T) { @@ -405,3 +453,113 @@ func Test_GettableUserConfigRoundtrip(t *testing.T) { require.Nil(t, err) require.Equal(t, string(yamlEncoded), string(out)) } + +func Test_ReceiverCompatibility(t *testing.T) { + for _, tc := range []struct { + desc string + a, b ReceiverType + expected bool + }{ + { + desc: "grafana=grafana", + a: GrafanaReceiverType, + b: GrafanaReceiverType, + expected: true, + }, + { + desc: "am=am", + a: AlertmanagerReceiverType, + b: AlertmanagerReceiverType, + expected: true, + }, + { + desc: "empty=grafana", + a: EmptyReceiverType, + b: AlertmanagerReceiverType, + expected: true, + }, + { + desc: "empty=am", + a: EmptyReceiverType, + b: AlertmanagerReceiverType, + expected: true, + }, + { + desc: "empty=empty", + a: EmptyReceiverType, + b: EmptyReceiverType, + expected: true, + }, + { + desc: "graf!=am", + a: GrafanaReceiverType, + b: AlertmanagerReceiverType, + expected: false, + }, + { + desc: "am!=graf", + a: AlertmanagerReceiverType, + b: GrafanaReceiverType, + expected: false, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + require.Equal(t, tc.expected, tc.a.Can(tc.b)) + }) + } +} + +func Test_ReceiverMatchesBackend(t *testing.T) { + for _, tc := range []struct { + desc string + rec ReceiverType + b Backend + err bool + }{ + { + desc: "graf=graf", + rec: GrafanaReceiverType, + b: GrafanaBackend, + err: false, + }, + { + desc: "empty=graf", + rec: EmptyReceiverType, + b: GrafanaBackend, + err: false, + }, + { + desc: "am=am", + rec: AlertmanagerReceiverType, + b: AlertmanagerBackend, + err: false, + }, + { + desc: "empty=am", + rec: EmptyReceiverType, + b: AlertmanagerBackend, + err: false, + }, + { + desc: "graf!=am", + rec: GrafanaReceiverType, + b: AlertmanagerBackend, + err: true, + }, + { + desc: "am!=ruler", + rec: GrafanaReceiverType, + b: LoTexRulerBackend, + err: true, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + err := tc.rec.MatchesBackend(tc.b) + if tc.err { + require.NotNil(t, err) + } else { + require.Nil(t, err) + } + }) + } +}