From 79a6b353e3dacd9e6588ea695ab0de54de253a9c Mon Sep 17 00:00:00 2001 From: Michael Desa Date: Thu, 5 Sep 2019 18:00:45 -0400 Subject: [PATCH] fix(notification/rule): conditionally include url and token for slack rule Previously, we expected that both token and url would be provided, but we've become aware that the user may provide either or both of them. --- notification/endpoint/endpoint_test.go | 4 +- notification/endpoint/slack.go | 14 +- notification/rule/slack.go | 18 +- notification/rule/slack_test.go | 258 ++++++++++++++++++++----- 4 files changed, 234 insertions(+), 60 deletions(-) diff --git a/notification/endpoint/endpoint_test.go b/notification/endpoint/endpoint_test.go index f41537867c..c2dafb0be0 100644 --- a/notification/endpoint/endpoint_test.go +++ b/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", }, }, { diff --git a/notification/endpoint/slack.go b/notification/endpoint/slack.go index 2d122d16c2..2dbf99b446 100644 --- a/notification/endpoint/slack.go +++ b/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 { - return &influxdb.Error{ - Code: influxdb.EInvalid, - Msg: fmt.Sprintf("slack endpoint URL is invalid: %s", err.Error()), + if s.URL != "" { + if _, err := url.Parse(s.URL); err != nil { + return &influxdb.Error{ + Code: influxdb.EInvalid, + Msg: fmt.Sprintf("slack endpoint URL is invalid: %s", err.Error()), + } } } // TODO(desa): this requirement seems odd diff --git a/notification/rule/slack.go b/notification/rule/slack.go index 252920e5aa..f0bdd2a710 100644 --- a/notification/rule/slack.go +++ b/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"), - flux.Object( - flux.Property("token", flux.Identifier("slack_secret")), - flux.Property("url", flux.String(e.URL)), - ), - ) + props := []*ast.Property{} + if e.Token.Key != "" { + props = append(props, flux.Property("token", flux.Identifier("slack_secret"))) + } + 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) } diff --git a/notification/rule/slack_test.go b/notification/rule/slack_test.go index 37eef048e4..27bc1f8cd0 100644 --- a/notification/rule/slack_test.go +++ b/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"})))` - - s := &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", + ({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, + }, }, - Operator: notification.Equal, - }, - { - Tag: influxdb.Tag{ - Key: "baz", - Value: "bang", + StatusRules: []notification.StatusRule{ + { + CurrentLevel: notification.Critical, + }, + { + CurrentLevel: notification.Warn, + PreviousLevel: statusRulePtr(notification.Info), + }, }, - Operator: notification.Equal, }, }, - StatusRules: []notification.StatusRule{ - { - CurrentLevel: notification.Critical, + endpoint: &endpoint.Slack{ + Base: endpoint.Base{ + ID: 2, + Name: "foo", }, - { - CurrentLevel: notification.Warn, - PreviousLevel: statusRulePtr(notification.Info), + URL: "http://localhost:7777", + Token: influxdb.SecretField{ + 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) - if err != nil { - panic(err) - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f, err := tt.rule.GenerateFlux(tt.endpoint) + if err != nil { + t.Fatal(err) + } - if f != want { - t.Errorf("scripts did not match. want:\n%v\n\ngot:\n%v", want, f) + if f != tt.want { + t.Errorf("scripts did not match. want:\n%v\n\ngot:\n%v", tt.want, f) + } + }) } }