diff --git a/.gitignore b/.gitignore index e3a9f210..cccce8e9 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,7 @@ !.golangci.yml !/cli/testdata/*.yml !/cli/config/testdata/*.yml +!/config/testdata/*.yml !/doc/examples/simple.yml !/circle.yml !/.travis.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 388ba1e1..8d724943 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## Next release + + * [CHANGE] Opsgenie notification now uses official responders notion instead of "teams". Teams field is deprecated now and will fail the config parsing. + ## 0.17.0 / 2019-05-02 This release includes changes to amtool which are not fully backwards diff --git a/config/config_test.go b/config/config_test.go index 3853740e..a0aec756 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -25,7 +25,7 @@ import ( commoncfg "github.com/prometheus/common/config" "github.com/prometheus/common/model" "github.com/stretchr/testify/require" - "gopkg.in/yaml.v2" + yaml "gopkg.in/yaml.v2" ) func TestLoadEmptyString(t *testing.T) { @@ -633,7 +633,7 @@ func TestEmptyFieldsAndRegex(t *testing.T) { func TestSMTPHello(t *testing.T) { c, _, err := LoadFile("testdata/conf.good.yml") if err != nil { - t.Errorf("Error parsing %s: %s", "testdata/conf.good.yml", err) + t.Fatalf("Error parsing %s: %s", "testdata/conf.good.yml", err) } const refValue = "host.example.org" @@ -646,7 +646,7 @@ func TestSMTPHello(t *testing.T) { func TestGroupByAll(t *testing.T) { c, _, err := LoadFile("testdata/conf.group-by-all.yml") if err != nil { - t.Errorf("Error parsing %s: %s", "testdata/conf.group-by-all.yml", err) + t.Fatalf("Error parsing %s: %s", "testdata/conf.group-by-all.yml", err) } if !c.Route.GroupByAll { @@ -657,12 +657,12 @@ func TestGroupByAll(t *testing.T) { func TestVictorOpsDefaultAPIKey(t *testing.T) { conf, _, err := LoadFile("testdata/conf.victorops-default-apikey.yml") if err != nil { - t.Errorf("Error parsing %s: %s", "testdata/conf.victorops-default-apikey.yml", err) + t.Fatalf("Error parsing %s: %s", "testdata/conf.victorops-default-apikey.yml", err) } var defaultKey = conf.Global.VictorOpsAPIKey if defaultKey != conf.Receivers[0].VictorOpsConfigs[0].APIKey { - t.Errorf("Invalid victorops key: %s\nExpected: %s", conf.Receivers[0].VictorOpsConfigs[0].APIKey, defaultKey) + t.Fatalf("Invalid victorops key: %s\nExpected: %s", conf.Receivers[0].VictorOpsConfigs[0].APIKey, defaultKey) } if defaultKey == conf.Receivers[1].VictorOpsConfigs[0].APIKey { t.Errorf("Invalid victorops key: %s\nExpected: %s", conf.Receivers[0].VictorOpsConfigs[0].APIKey, "qwe456") @@ -672,7 +672,7 @@ func TestVictorOpsDefaultAPIKey(t *testing.T) { func TestVictorOpsNoAPIKey(t *testing.T) { _, _, err := LoadFile("testdata/conf.victorops-no-apikey.yml") if err == nil { - t.Errorf("Expected an error parsing %s: %s", "testdata/conf.victorops-no-apikey.yml", err) + t.Fatalf("Expected an error parsing %s: %s", "testdata/conf.victorops-no-apikey.yml", err) } if err.Error() != "no global VictorOps API Key set" { t.Errorf("Expected: %s\nGot: %s", "no global VictorOps API Key set", err.Error()) @@ -682,12 +682,12 @@ func TestVictorOpsNoAPIKey(t *testing.T) { func TestOpsGenieDefaultAPIKey(t *testing.T) { conf, _, err := LoadFile("testdata/conf.opsgenie-default-apikey.yml") if err != nil { - t.Errorf("Error parsing %s: %s", "testdata/conf.opsgenie-default-apikey.yml", err) + t.Fatalf("Error parsing %s: %s", "testdata/conf.opsgenie-default-apikey.yml", err) } var defaultKey = conf.Global.OpsGenieAPIKey if defaultKey != conf.Receivers[0].OpsGenieConfigs[0].APIKey { - t.Errorf("Invalid OpsGenie key: %s\nExpected: %s", conf.Receivers[0].OpsGenieConfigs[0].APIKey, defaultKey) + t.Fatalf("Invalid OpsGenie key: %s\nExpected: %s", conf.Receivers[0].OpsGenieConfigs[0].APIKey, defaultKey) } if defaultKey == conf.Receivers[1].OpsGenieConfigs[0].APIKey { t.Errorf("Invalid OpsGenie key: %s\nExpected: %s", conf.Receivers[0].OpsGenieConfigs[0].APIKey, "qwe456") @@ -697,9 +697,22 @@ func TestOpsGenieDefaultAPIKey(t *testing.T) { func TestOpsGenieNoAPIKey(t *testing.T) { _, _, err := LoadFile("testdata/conf.opsgenie-no-apikey.yml") if err == nil { - t.Errorf("Expected an error parsing %s: %s", "testdata/conf.opsgenie-no-apikey.yml", err) + t.Fatalf("Expected an error parsing %s: %s", "testdata/conf.opsgenie-no-apikey.yml", err) } if err.Error() != "no global OpsGenie API Key set" { t.Errorf("Expected: %s\nGot: %s", "no global OpsGenie API Key set", err.Error()) } } + +func TestOpsGenieDeprecatedTeamSpecified(t *testing.T) { + _, _, err := LoadFile("testdata/conf.opsgenie-default-apikey-old-team.yml") + if err == nil { + t.Fatalf("Expected an error parsing %s: %s", "testdata/conf.opsgenie-default-apikey-old-team.yml", err) + } + + const expectedErr = `yaml: unmarshal errors: + line 18: field teams not found in type config.plain` + if err.Error() != expectedErr { + t.Errorf("Expected: %s\nGot: %s", expectedErr, err.Error()) + } +} diff --git a/config/notifiers.go b/config/notifiers.go index e937fd65..c82828d3 100644 --- a/config/notifiers.go +++ b/config/notifiers.go @@ -15,9 +15,12 @@ package config import ( "fmt" + "regexp" "strings" "time" + "github.com/pkg/errors" + commoncfg "github.com/prometheus/common/config" ) @@ -453,23 +456,52 @@ type OpsGenieConfig struct { HTTPConfig *commoncfg.HTTPClientConfig `yaml:"http_config,omitempty" json:"http_config,omitempty"` - APIKey Secret `yaml:"api_key,omitempty" json:"api_key,omitempty"` - APIURL *URL `yaml:"api_url,omitempty" json:"api_url,omitempty"` - Message string `yaml:"message,omitempty" json:"message,omitempty"` - Description string `yaml:"description,omitempty" json:"description,omitempty"` - Source string `yaml:"source,omitempty" json:"source,omitempty"` - Details map[string]string `yaml:"details,omitempty" json:"details,omitempty"` - Teams string `yaml:"teams,omitempty" json:"teams,omitempty"` - Tags string `yaml:"tags,omitempty" json:"tags,omitempty"` - Note string `yaml:"note,omitempty" json:"note,omitempty"` - Priority string `yaml:"priority,omitempty" json:"priority,omitempty"` + APIKey Secret `yaml:"api_key,omitempty" json:"api_key,omitempty"` + APIURL *URL `yaml:"api_url,omitempty" json:"api_url,omitempty"` + Message string `yaml:"message,omitempty" json:"message,omitempty"` + Description string `yaml:"description,omitempty" json:"description,omitempty"` + Source string `yaml:"source,omitempty" json:"source,omitempty"` + Details map[string]string `yaml:"details,omitempty" json:"details,omitempty"` + Responders []OpsGenieConfigResponder `yaml:"responders,omitempty" json:"responders,omitempty"` + Tags string `yaml:"tags,omitempty" json:"tags,omitempty"` + Note string `yaml:"note,omitempty" json:"note,omitempty"` + Priority string `yaml:"priority,omitempty" json:"priority,omitempty"` } +const opsgenieValidTypesRe = `^team|user|escalation|schedule$` + +var opsgenieTypeMatcher = regexp.MustCompile(opsgenieValidTypesRe) + // UnmarshalYAML implements the yaml.Unmarshaler interface. func (c *OpsGenieConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { *c = DefaultOpsGenieConfig type plain OpsGenieConfig - return unmarshal((*plain)(c)) + if err := unmarshal((*plain)(c)); err != nil { + return err + } + + for _, r := range c.Responders { + if r.ID == "" && r.Username == "" && r.Name == "" { + return errors.Errorf("OpsGenieConfig responder %v has to have at least one of id, username or name specified", r) + } + + r.Type = strings.ToLower(r.Type) + if !opsgenieTypeMatcher.MatchString(r.Type) { + return errors.Errorf("OpsGenieConfig responder %v type does not match valid options %s", r, opsgenieValidTypesRe) + } + } + + return nil +} + +type OpsGenieConfigResponder struct { + // One of those 3 should be filled. + ID string `yaml:"id,omitempty" json:"id,omitempty"` + Name string `yaml:"name,omitempty" json:"name,omitempty"` + Username string `yaml:"username,omitempty" json:"username,omitempty"` + + // team, user, escalation, schedule etc. + Type string `yaml:"type,omitempty" json:"type,omitempty"` } // VictorOpsConfig configures notifications via VictorOps. diff --git a/config/testdata/conf.opsgenie-default-apikey-old-team.yml b/config/testdata/conf.opsgenie-default-apikey-old-team.yml new file mode 100644 index 00000000..e2c7b792 --- /dev/null +++ b/config/testdata/conf.opsgenie-default-apikey-old-team.yml @@ -0,0 +1,18 @@ +global: + opsgenie_api_key: asd132 + +route: + group_by: ['alertname', 'cluster', 'service'] + group_wait: 30s + group_interval: 5m + repeat_interval: 3h + receiver: escalation-Y-opsgenie + routes: + - match: + service: foo + receiver: team-X-opsgenie + +receivers: + - name: 'team-X-opsgenie' + opsgenie_configs: + - teams: 'team-X' diff --git a/config/testdata/conf.opsgenie-default-apikey.yml b/config/testdata/conf.opsgenie-default-apikey.yml index 5b4f7365..22b9fe43 100644 --- a/config/testdata/conf.opsgenie-default-apikey.yml +++ b/config/testdata/conf.opsgenie-default-apikey.yml @@ -6,7 +6,7 @@ route: group_wait: 30s group_interval: 5m repeat_interval: 3h - receiver: team-Y-opsgenie + receiver: escalation-Y-opsgenie routes: - match: service: foo @@ -15,8 +15,12 @@ route: receivers: - name: 'team-X-opsgenie' opsgenie_configs: - - teams: 'team-X' -- name: 'team-Y-opsgenie' + - responders: + - name: 'team-X' + type: 'team' +- name: 'escalation-Y-opsgenie' opsgenie_configs: - - teams: 'team-Y' + - responders: + - name: 'escalation-Y' + type: 'escalation' api_key: qwe456 diff --git a/config/testdata/conf.opsgenie-no-apikey.yml b/config/testdata/conf.opsgenie-no-apikey.yml index 01293ce0..3e355cd4 100644 --- a/config/testdata/conf.opsgenie-no-apikey.yml +++ b/config/testdata/conf.opsgenie-no-apikey.yml @@ -12,4 +12,6 @@ route: receivers: - name: 'team-X-opsgenie' opsgenie_configs: - - teams: 'team-X' + - responders: + - name: 'team-X' + type: 'team' \ No newline at end of file diff --git a/notify/impl.go b/notify/impl.go index 0476bbab..4e73e6aa 100644 --- a/notify/impl.go +++ b/notify/impl.go @@ -868,15 +868,22 @@ func NewOpsGenie(c *config.OpsGenieConfig, t *template.Template, l log.Logger) * } type opsGenieCreateMessage struct { - Alias string `json:"alias"` - Message string `json:"message"` - Description string `json:"description,omitempty"` - Details map[string]string `json:"details"` - Source string `json:"source"` - Teams []map[string]string `json:"teams,omitempty"` - Tags []string `json:"tags,omitempty"` - Note string `json:"note,omitempty"` - Priority string `json:"priority,omitempty"` + Alias string `json:"alias"` + Message string `json:"message"` + Description string `json:"description,omitempty"` + Details map[string]string `json:"details"` + Source string `json:"source"` + Responders []opsGenieCreateMessageResponder `json:"responders,omitempty"` + Tags []string `json:"tags,omitempty"` + Note string `json:"note,omitempty"` + Priority string `json:"priority,omitempty"` +} + +type opsGenieCreateMessageResponder struct { + ID string `json:"id,omitempty"` + Name string `json:"name,omitempty"` + Username string `json:"username,omitempty"` + Type string `json:"type"` // team, user, escalation, schedule etc. } type opsGenieCloseMessage struct { @@ -955,11 +962,24 @@ func (n *OpsGenie) createRequest(ctx context.Context, as ...*types.Alert) (*http } apiURL.Path += "v2/alerts" - var teams []map[string]string - for _, t := range safeSplit(string(tmpl(n.conf.Teams)), ",") { - teams = append(teams, map[string]string{"name": t}) + + var responders []opsGenieCreateMessageResponder + for _, r := range n.conf.Responders { + responder := opsGenieCreateMessageResponder{ + ID: tmpl(r.ID), + Name: tmpl(r.Name), + Username: tmpl(r.Username), + Type: tmpl(r.Type), + } + + if responder == (opsGenieCreateMessageResponder{}) { + // Filter out empty responders. This is useful if you want to fill + // responders dynamically from alert's common labels. + continue + } + + responders = append(responders, responder) } - tags := safeSplit(string(tmpl(n.conf.Tags)), ",") msg = &opsGenieCreateMessage{ Alias: alias, @@ -967,8 +987,8 @@ func (n *OpsGenie) createRequest(ctx context.Context, as ...*types.Alert) (*http Description: tmpl(n.conf.Description), Details: details, Source: tmpl(n.conf.Source), - Teams: teams, - Tags: tags, + Responders: responders, + Tags: safeSplit(string(tmpl(n.conf.Tags)), ","), Note: tmpl(n.conf.Note), Priority: tmpl(n.conf.Priority), } diff --git a/notify/impl_test.go b/notify/impl_test.go index 113d471c..2ecc28fa 100644 --- a/notify/impl_test.go +++ b/notify/impl_test.go @@ -461,12 +461,21 @@ func TestOpsGenie(t *testing.T) { Message: `{{ .CommonLabels.Message }}`, Description: `{{ .CommonLabels.Description }}`, Source: `{{ .CommonLabels.Source }}`, - Teams: `{{ .CommonLabels.Teams }}`, - Tags: `{{ .CommonLabels.Tags }}`, - Note: `{{ .CommonLabels.Note }}`, - Priority: `{{ .CommonLabels.Priority }}`, - APIKey: `{{ .ExternalURL }}`, - APIURL: &config.URL{URL: u}, + Responders: []config.OpsGenieConfigResponder{ + { + Name: `{{ .CommonLabels.ResponderName1 }}`, + Type: `{{ .CommonLabels.ResponderType1 }}`, + }, + { + Name: `{{ .CommonLabels.ResponderName2 }}`, + Type: `{{ .CommonLabels.ResponderType2 }}`, + }, + }, + Tags: `{{ .CommonLabels.Tags }}`, + Note: `{{ .CommonLabels.Note }}`, + Priority: `{{ .CommonLabels.Priority }}`, + APIKey: `{{ .ExternalURL }}`, + APIURL: &config.URL{URL: u}, } notifier := NewOpsGenie(conf, tmpl, logger) @@ -495,19 +504,22 @@ func TestOpsGenie(t *testing.T) { alert2 := &types.Alert{ Alert: model.Alert{ Labels: model.LabelSet{ - "Message": "message", - "Description": "description", - "Source": "http://prometheus", - "Teams": "TeamA,TeamB,", - "Tags": "tag1,tag2", - "Note": "this is a note", - "Priotity": "P1", + "Message": "message", + "Description": "description", + "Source": "http://prometheus", + "ResponderName1": "TeamA", + "ResponderType1": "team", + "ResponderName2": "EscalationA", + "ResponderType2": "escalation", + "Tags": "tag1,tag2", + "Note": "this is a note", + "Priority": "P1", }, StartsAt: time.Now(), EndsAt: time.Now().Add(time.Hour), }, } - expectedBody = `{"alias":"6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b","message":"message","description":"description","details":{},"source":"http://prometheus","teams":[{"name":"TeamA"},{"name":"TeamB"}],"tags":["tag1","tag2"],"note":"this is a note"} + expectedBody = `{"alias":"6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b","message":"message","description":"description","details":{},"source":"http://prometheus","responders":[{"name":"TeamA","type":"team"},{"name":"EscalationA","type":"escalation"}],"tags":["tag1","tag2"],"note":"this is a note","priority":"P1"} ` req, retry, err = notifier.createRequest(ctx, alert2) require.NoError(t, err)