opsgenie: Moved from deprecated, non documented teams to responders field. (#1863)

Teams config option will fail unmarshalling as it is deprecated. Fixes https://github.com/prometheus/alertmanager/issues/1818 Signed-off-by: Bartek Plotka <bwplotka@gmail.com>
2019-05-13 13:51:26 +01:00 · 2019-05-13 13:51:26 +01:00 · 9ddc5f1348
parent fba4070b2d
commit 9ddc5f1348
9 changed files with 160 additions and 54 deletions
--- 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
--- 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
--- 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())
+	}
+}
--- 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"
 )

@ -459,17 +462,46 @@ type OpsGenieConfig struct {
 	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"`
+	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.
--- a/config/testdata/conf.opsgenie-default-apikey-old-team.yml
+++ 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'
--- 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
--- 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'
--- a/notify/impl.go
+++ b/notify/impl.go
@ -873,12 +873,19 @@ type opsGenieCreateMessage struct {
 	Description string                           `json:"description,omitempty"`
 	Details     map[string]string                `json:"details"`
 	Source      string                           `json:"source"`
-	Teams       []map[string]string `json:"teams,omitempty"`
+	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 {
 	Source string `json:"source"`
 }
@ -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),
 		}
--- a/notify/impl_test.go
+++ b/notify/impl_test.go
@ -461,7 +461,16 @@ func TestOpsGenie(t *testing.T) {
 		Message:     `{{ .CommonLabels.Message }}`,
 		Description: `{{ .CommonLabels.Description }}`,
 		Source:      `{{ .CommonLabels.Source }}`,
-		Teams:       `{{ .CommonLabels.Teams }}`,
+		Responders: []config.OpsGenieConfigResponder{
+			{
+				Name: `{{ .CommonLabels.ResponderName1 }}`,
+				Type: `{{ .CommonLabels.ResponderType1 }}`,
+			},
+			{
+				Name: `{{ .CommonLabels.ResponderName2 }}`,
+				Type: `{{ .CommonLabels.ResponderType2 }}`,
+			},
+		},
 		Tags:     `{{ .CommonLabels.Tags }}`,
 		Note:     `{{ .CommonLabels.Note }}`,
 		Priority: `{{ .CommonLabels.Priority }}`,
@ -498,16 +507,19 @@ func TestOpsGenie(t *testing.T) {
 				"Message":        "message",
 				"Description":    "description",
 				"Source":         "http://prometheus",
-				"Teams":       "TeamA,TeamB,",
+				"ResponderName1": "TeamA",
+				"ResponderType1": "team",
+				"ResponderName2": "EscalationA",
+				"ResponderType2": "escalation",
 				"Tags":           "tag1,tag2",
 				"Note":           "this is a note",
-				"Priotity":    "P1",
+				"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)