Merge pull request #14984 from influxdata/fix/slack-rule-details

fix(notification/rule): conditionally include url and token for slack rule

notification/endpoint/endpoint_test.go

@@ -55,13 +55,13 @@ func TestValidEndpoint(t *testing.T) {
 			},
 		},
 		{
-			name: "empty slack url",
+			name: "empty slack url and token",
 			src: &endpoint.Slack{
 				Base: goodBase,
 			},
 			err: &influxdb.Error{
 				Code: influxdb.EInvalid,
-				Msg:  "slack endpoint URL is empty",
+				Msg:  "slack endpoint URL and token are empty",
 			},
 		},
 		{

notification/endpoint/slack.go

@@ -45,16 +45,18 @@ func (s Slack) Valid() error {
 	if err := s.Base.valid(); err != nil {
 		return err
 	}
-	if s.URL == "" {
+	if s.URL == "" && s.Token.Key == "" {
 		return &influxdb.Error{
 			Code: influxdb.EInvalid,
-			Msg:  "slack endpoint URL is empty",
+			Msg:  "slack endpoint URL and token are empty",
 		}
 	}
-	if _, err := url.Parse(s.URL); err != nil {
+	if s.URL != "" {
-		return &influxdb.Error{
+		if _, err := url.Parse(s.URL); err != nil {
-			Code: influxdb.EInvalid,
+			return &influxdb.Error{
-			Msg:  fmt.Sprintf("slack endpoint URL is invalid: %s", err.Error()),
+				Code: influxdb.EInvalid,
+				Msg:  fmt.Sprintf("slack endpoint URL is invalid: %s", err.Error()),
+			}
 		}
 	}
 	// TODO(desa): this requirement seems odd

notification/rule/slack.go

@@ -43,7 +43,9 @@ func (s *Slack) GenerateFluxAST(e *endpoint.Slack) (*ast.Package, error) {
 func (s *Slack) generateFluxASTBody(e *endpoint.Slack) []ast.Statement {
 	var statements []ast.Statement
 	statements = append(statements, s.generateTaskOption())
-	statements = append(statements, s.generateFluxASTSecrets(e))
+	if e.Token.Key != "" {
+		statements = append(statements, s.generateFluxASTSecrets(e))
+	}
 	statements = append(statements, s.generateFluxASTEndpoint(e))
 	statements = append(statements, s.generateFluxASTNotificationDefinition(e))
 	statements = append(statements, s.generateFluxASTStatuses())
@@ -60,12 +62,14 @@ func (s *Slack) generateFluxASTSecrets(e *endpoint.Slack) ast.Statement {
 }
 
 func (s *Slack) generateFluxASTEndpoint(e *endpoint.Slack) ast.Statement {
-	call := flux.Call(flux.Member("slack", "endpoint"),
+	props := []*ast.Property{}
-		flux.Object(
+	if e.Token.Key != "" {
-			flux.Property("token", flux.Identifier("slack_secret")),
+		props = append(props, flux.Property("token", flux.Identifier("slack_secret")))
-			flux.Property("url", flux.String(e.URL)),
+	}
-		),
+	if e.URL != "" {
-	)
+		props = append(props, flux.Property("url", flux.String(e.URL)))
+	}
+	call := flux.Call(flux.Member("slack", "endpoint"), flux.Object(props...))
 
 	return flux.DefineVariable("slack_endpoint", call)
 }

notification/rule/slack_test.go

@@ -24,7 +24,170 @@ func statusRulePtr(r notification.CheckLevel) *notification.CheckLevel {
 }
 
 func TestSlack_GenerateFlux(t *testing.T) {
-	want := `package main
+	tests := []struct {
+		name     string
+		want     string
+		rule     *rule.Slack
+		endpoint *endpoint.Slack
+	}{
+		{
+			name: "with url",
+			want: `package main
+// foo
+import "influxdata/influxdb/monitor"
+import "slack"
+import "influxdata/influxdb/secrets"
+import "experimental"
+
+option task = {name: "foo", every: 1h}
+
+slack_endpoint = slack.endpoint(url: "http://localhost:7777")
+notification = {
+	_notification_rule_id: "0000000000000001",
+	_notification_rule_name: "foo",
+	_notification_endpoint_id: "0000000000000002",
+	_notification_endpoint_name: "foo",
+}
+statuses = monitor.from(start: -2h, fn: (r) =>
+	(r.foo == "bar" and r.baz == "bang"))
+crit = statuses
+	|> filter(fn: (r) =>
+		(r._level == "crit"))
+info_to_warn = statuses
+	|> monitor.stateChanges(fromLevel: "info", toLevel: "warn")
+all_statuses = union(tables: [crit, info_to_warn])
+	|> sort(columns: ["_time"])
+	|> filter(fn: (r) =>
+		(r._time > experimental.subDuration(from: now(), d: 1h)))
+
+all_statuses
+	|> monitor.notify(data: notification, endpoint: slack_endpoint(mapFn: (r) =>
+		({channel: "bar", text: "blah", color: if r._level == "crit" then "danger" else if r._level == "warn" then "warning" else "good"})))`,
+			rule: &rule.Slack{
+				Channel:         "bar",
+				MessageTemplate: "blah",
+				Base: rule.Base{
+					ID:         1,
+					EndpointID: 2,
+					Name:       "foo",
+					Every:      mustDuration("1h"),
+					TagRules: []notification.TagRule{
+						{
+							Tag: influxdb.Tag{
+								Key:   "foo",
+								Value: "bar",
+							},
+							Operator: notification.Equal,
+						},
+						{
+							Tag: influxdb.Tag{
+								Key:   "baz",
+								Value: "bang",
+							},
+							Operator: notification.Equal,
+						},
+					},
+					StatusRules: []notification.StatusRule{
+						{
+							CurrentLevel: notification.Critical,
+						},
+						{
+							CurrentLevel:  notification.Warn,
+							PreviousLevel: statusRulePtr(notification.Info),
+						},
+					},
+				},
+			},
+			endpoint: &endpoint.Slack{
+				Base: endpoint.Base{
+					ID:   2,
+					Name: "foo",
+				},
+				URL: "http://localhost:7777",
+			},
+		},
+		{
+			name: "with token",
+			want: `package main
+// foo
+import "influxdata/influxdb/monitor"
+import "slack"
+import "influxdata/influxdb/secrets"
+import "experimental"
+
+option task = {name: "foo", every: 1h}
+
+slack_secret = secrets.get(key: "slack_token")
+slack_endpoint = slack.endpoint(token: slack_secret)
+notification = {
+	_notification_rule_id: "0000000000000001",
+	_notification_rule_name: "foo",
+	_notification_endpoint_id: "0000000000000002",
+	_notification_endpoint_name: "foo",
+}
+statuses = monitor.from(start: -2h, fn: (r) =>
+	(r.foo == "bar" and r.baz == "bang"))
+crit = statuses
+	|> filter(fn: (r) =>
+		(r._level == "crit"))
+info_to_warn = statuses
+	|> monitor.stateChanges(fromLevel: "info", toLevel: "warn")
+all_statuses = union(tables: [crit, info_to_warn])
+	|> sort(columns: ["_time"])
+	|> filter(fn: (r) =>
+		(r._time > experimental.subDuration(from: now(), d: 1h)))
+
+all_statuses
+	|> monitor.notify(data: notification, endpoint: slack_endpoint(mapFn: (r) =>
+		({channel: "bar", text: "blah", color: if r._level == "crit" then "danger" else if r._level == "warn" then "warning" else "good"})))`,
+			rule: &rule.Slack{
+				Channel:         "bar",
+				MessageTemplate: "blah",
+				Base: rule.Base{
+					ID:         1,
+					EndpointID: 2,
+					Name:       "foo",
+					Every:      mustDuration("1h"),
+					TagRules: []notification.TagRule{
+						{
+							Tag: influxdb.Tag{
+								Key:   "foo",
+								Value: "bar",
+							},
+							Operator: notification.Equal,
+						},
+						{
+							Tag: influxdb.Tag{
+								Key:   "baz",
+								Value: "bang",
+							},
+							Operator: notification.Equal,
+						},
+					},
+					StatusRules: []notification.StatusRule{
+						{
+							CurrentLevel: notification.Critical,
+						},
+						{
+							CurrentLevel:  notification.Warn,
+							PreviousLevel: statusRulePtr(notification.Info),
+						},
+					},
+				},
+			},
+			endpoint: &endpoint.Slack{
+				Base: endpoint.Base{
+					ID:   2,
+					Name: "foo",
+				},
+				Token: influxdb.SecretField{
+					Key: "slack_token",
+				},
+			},
+		},
+		{
+			name: "with token and url",
+			want: `package main
 // foo
 import "influxdata/influxdb/monitor"
 import "slack"
@@ -55,60 +218,65 @@ all_statuses = union(tables: [crit, info_to_warn])
 
 all_statuses
 	|> monitor.notify(data: notification, endpoint: slack_endpoint(mapFn: (r) =>
-		({channel: "bar", text: "blah", color: if r._level == "crit" then "danger" else if r._level == "warn" then "warning" else "good"})))`
+		({channel: "bar", text: "blah", color: if r._level == "crit" then "danger" else if r._level == "warn" then "warning" else "good"})))`,
-
+			rule: &rule.Slack{
-	s := &rule.Slack{
+				Channel:         "bar",
-		Channel:         "bar",
+				MessageTemplate: "blah",
-		MessageTemplate: "blah",
+				Base: rule.Base{
-		Base: rule.Base{
+					ID:         1,
-			ID:         1,
+					EndpointID: 2,
-			EndpointID: 2,
+					Name:       "foo",
-			Name:       "foo",
+					Every:      mustDuration("1h"),
-			Every:      mustDuration("1h"),
+					TagRules: []notification.TagRule{
-			TagRules: []notification.TagRule{
+						{
-				{
+							Tag: influxdb.Tag{
-					Tag: influxdb.Tag{
+								Key:   "foo",
-						Key:   "foo",
+								Value: "bar",
-						Value: "bar",
+							},
+							Operator: notification.Equal,
+						},
+						{
+							Tag: influxdb.Tag{
+								Key:   "baz",
+								Value: "bang",
+							},
+							Operator: notification.Equal,
+						},
 					},
-					Operator: notification.Equal,
+					StatusRules: []notification.StatusRule{
-				},
+						{
-				{
+							CurrentLevel: notification.Critical,
-					Tag: influxdb.Tag{
+						},
-						Key:   "baz",
+						{
-						Value: "bang",
+							CurrentLevel:  notification.Warn,
+							PreviousLevel: statusRulePtr(notification.Info),
+						},
 					},
-					Operator: notification.Equal,
 				},
 			},
-			StatusRules: []notification.StatusRule{
+			endpoint: &endpoint.Slack{
-				{
+				Base: endpoint.Base{
-					CurrentLevel: notification.Critical,
+					ID:   2,
+					Name: "foo",
 				},
-				{
+				URL: "http://localhost:7777",
-					CurrentLevel:  notification.Warn,
+				Token: influxdb.SecretField{
-					PreviousLevel: statusRulePtr(notification.Info),
+					Key: "slack_token",
 				},
 			},
 		},
 	}
-	e := &endpoint.Slack{
-		Base: endpoint.Base{
-			ID:   2,
-			Name: "foo",
-		},
-		URL: "http://localhost:7777",
-		Token: influxdb.SecretField{
-			Key: "slack_token",
-		},
-	}
 
-	f, err := s.GenerateFlux(e)
+	for _, tt := range tests {
-	if err != nil {
+		t.Run(tt.name, func(t *testing.T) {
-		panic(err)
+			f, err := tt.rule.GenerateFlux(tt.endpoint)
-	}
+			if err != nil {
+				t.Fatal(err)
+			}
 
-	if f != want {
+			if f != tt.want {
-		t.Errorf("scripts did not match. want:\n%v\n\ngot:\n%v", want, f)
+				t.Errorf("scripts did not match. want:\n%v\n\ngot:\n%v", tt.want, f)
+			}
+		})
 	}
 }