Skip to content

Commit 118f955

Browse files
RaviHaridlhpasha-codefreshAlexander Matyushentsevkshamajain99
authored
feat: allow list of triggers in notification annotation (#65)
* feat: allow list of triggers in notification annotation Signed-off-by: Ravi Hari <[email protected]> * feat: add deliveryPolicy option for slack notifications (#56) The delivery policy controls how slack messages are sent. The available modes are `Post` (default), `PostAndUpdate`, and `Update`. Signed-off-by: Daniel Lee Harple <[email protected]> Signed-off-by: Ravi Hari <[email protected]> * feat: update slack version to latest (#68) feat: update slack version to latest (#68) Signed-off-by: pashavictorovich <[email protected]> Signed-off-by: Ravi Hari <[email protected]> * chore: drop reflection based slack client tests (#74) Signed-off-by: Alexander Matyushentsev <[email protected]> Signed-off-by: Ravi Hari <[email protected]> * chore: remove github.com/dgrijalva/jwt-go for CVE-2020-26160 (#73) * cve dgrijalva/jwt-go Signed-off-by: kshamajain99 <[email protected]> * fix unit tests Signed-off-by: kshamajain99 <[email protected]> Signed-off-by: Ravi Hari <[email protected]> * feat: allow list of triggers and destinaitons in notification annotation Signed-off-by: Ravi Hari <[email protected]> * fix: address lint issues Signed-off-by: Ravi Hari <[email protected]> * fix: address goimport issues Signed-off-by: Ravi Hari <[email protected]> * feat: allow list of triggers in notification annotation Signed-off-by: Ravi Hari <[email protected]> * feat: add deliveryPolicy option for slack notifications (#56) The delivery policy controls how slack messages are sent. The available modes are `Post` (default), `PostAndUpdate`, and `Update`. Signed-off-by: Daniel Lee Harple <[email protected]> Signed-off-by: Ravi Hari <[email protected]> * feat: update slack version to latest (#68) feat: update slack version to latest (#68) Signed-off-by: pashavictorovich <[email protected]> Signed-off-by: Ravi Hari <[email protected]> * chore: drop reflection based slack client tests (#74) Signed-off-by: Alexander Matyushentsev <[email protected]> Signed-off-by: Ravi Hari <[email protected]> * chore: remove github.com/dgrijalva/jwt-go for CVE-2020-26160 (#73) * cve dgrijalva/jwt-go Signed-off-by: kshamajain99 <[email protected]> * fix unit tests Signed-off-by: kshamajain99 <[email protected]> Signed-off-by: Ravi Hari <[email protected]> * feat: allow list of triggers and destinaitons in notification annotation Signed-off-by: Ravi Hari <[email protected]> * fix: rebase Signed-off-by: Ravi Hari <[email protected]> * fix: update go.mod Signed-off-by: Ravi Hari <[email protected]> * fix: rebase Signed-off-by: Ravi Hari <[email protected]> * fix: rebase Signed-off-by: Ravi Hari <[email protected]> * fix: address lint issues Signed-off-by: Ravi Hari <[email protected]> * fix: address goimport issues Signed-off-by: Ravi Hari <[email protected]> * fix: rebase Signed-off-by: Ravi Hari <[email protected]> * fix: remove unwanted module Signed-off-by: Ravi Hari <[email protected]> * fix: go sum changes Signed-off-by: Ravi Hari <[email protected]> * fix: handle cases when triggers/destinations are not passed Signed-off-by: Ravi Hari <[email protected]> * fix: update annotation name Signed-off-by: Ravi Hari <[email protected]> * fix: update documentation Signed-off-by: Ravi Hari <[email protected]> Co-authored-by: Daniel Lee Harple <[email protected]> Co-authored-by: pasha-codefresh <[email protected]> Co-authored-by: Alexander Matyushentsev <[email protected]> Co-authored-by: Kshama Jain <[email protected]>
1 parent d5892b8 commit 118f955

File tree

5 files changed

+212
-32
lines changed

5 files changed

+212
-32
lines changed

‎README.md

+15
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,21 @@ metadata:
6161
notifications.argoproj.io/subscribe.on-sync-succeeded.slack: my-channel1;my-channel2
6262
```
6363

64+
If there is more than one trigger and multiple destinations you can configure the annotation as given below.
65+
66+
```yaml
67+
notifications.argoproj.io/subscriptions: |
68+
- trigger: [on-scaling-replica-set, on-rollout-updated, on-rollout-step-completed]
69+
destinations:
70+
- service: slack
71+
recipients: [my-channel-1, my-channel-2]
72+
- service: email
73+
recipients: [recipient-1, recipient-2, recipient-3 ]
74+
- trigger: [on-rollout-aborted, on-analysis-run-failed, on-analysis-run-error]
75+
destinations:
76+
- service: slack
77+
recipients: [my-channel-21, my-channel-22]
78+
```
6479
## Getting Started
6580

6681
Ready to add notifications to your project? Check out sample notifications for [cert-manager](./examples/certmanager/README.md)

‎docs/services/slack.md

+20
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,26 @@ The Slack notification service configuration includes following settings:
5454
annotations:
5555
notifications.argoproj.io/subscribe.on-sync-succeeded.slack: my_channel
5656

57+
1. Annotation with more than one trigger multiple of destinations and recipients
58+
59+
```yaml
60+
apiVersion: argoproj.io/v1alpha1
61+
kind: Application
62+
metadata:
63+
annotations:
64+
notifications.argoproj.io/subscriptions: |
65+
- trigger: [on-scaling-replica-set, on-rollout-updated, on-rollout-step-completed]
66+
destinations:
67+
- service: slack
68+
recipients: [my-channel-1, my-channel-2]
69+
- service: email
70+
recipients: [recipient-1, recipient-2, recipient-3 ]
71+
- trigger: [on-rollout-aborted, on-analysis-run-failed, on-analysis-run-error]
72+
destinations:
73+
- service: slack
74+
recipients: [my-channel-21, my-channel-22]
75+
```
76+
5777
## Templates
5878
5979
Notification templates can be customized to leverage slack message blocks and attachments

‎pkg/controller/controller_test.go

+9-2
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,9 @@ func TestSendsNotificationIfTriggered(t *testing.T) {
114114
}), []string{"test"}, services.Destination{Service: "mock", Recipient: "recipient"}).Return(nil)
115115

116116
annotations, err := ctrl.processResource(app, logEntry)
117+
if err != nil {
118+
logEntry.Errorf("Failed to process: %v", err)
119+
}
117120

118121
assert.NoError(t, err)
119122

@@ -137,7 +140,9 @@ func TestDoesNotSendNotificationIfAnnotationPresent(t *testing.T) {
137140
api.EXPECT().RunTrigger("my-trigger", gomock.Any()).Return([]triggers.ConditionResult{{Triggered: true, Templates: []string{"test"}}}, nil)
138141

139142
_, err = ctrl.processResource(app, logEntry)
140-
143+
if err != nil {
144+
logEntry.Errorf("Failed to process: %v", err)
145+
}
141146
assert.NoError(t, err)
142147
}
143148

@@ -157,7 +162,9 @@ func TestRemovesAnnotationIfNoTrigger(t *testing.T) {
157162
api.EXPECT().RunTrigger("my-trigger", gomock.Any()).Return([]triggers.ConditionResult{{Triggered: false}}, nil)
158163

159164
annotations, err := ctrl.processResource(app, logEntry)
160-
165+
if err != nil {
166+
logEntry.Errorf("Failed to process: %v", err)
167+
}
161168
assert.NoError(t, err)
162169
state = NewState(annotations[NotifiedAnnotationKey])
163170
assert.Empty(t, state)

‎pkg/subscriptions/annotations.go

+82-18
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ import (
44
"fmt"
55
"strings"
66

7+
"github.com/ghodss/yaml"
8+
log "github.com/sirupsen/logrus"
9+
710
"github.com/argoproj/notifications-engine/pkg/services"
811
)
912

@@ -36,28 +39,89 @@ func NewAnnotations(annotations map[string]string) Annotations {
3639
return Annotations(annotations)
3740
}
3841

42+
type Subscription struct {
43+
Trigger []string
44+
Destinations []Destination
45+
}
46+
47+
// Destination holds notification destination details
48+
type Destination struct {
49+
Service string `json:"service"`
50+
Recipients []string `json:"recipients"`
51+
}
52+
3953
func (a Annotations) iterate(callback func(trigger string, service string, recipients []string, key string)) {
4054
prefix := AnnotationPrefix + "/subscribe."
55+
altPrefix := AnnotationPrefix + "/subscriptions"
56+
var recipients []string
4157
for k, v := range a {
42-
if !strings.HasPrefix(k, prefix) {
43-
continue
44-
}
45-
parts := strings.Split(k[len(prefix):], ".")
46-
trigger := parts[0]
47-
service := ""
48-
if len(parts) >= 2 {
49-
service = parts[1]
50-
} else {
51-
service = parts[0]
52-
trigger = ""
53-
}
54-
var recipients []string
55-
if v == "" {
56-
recipients = []string{""}
57-
} else {
58-
recipients = parseRecipients(v)
58+
switch {
59+
case strings.HasPrefix(k, prefix):
60+
parts := strings.Split(k[len(prefix):], ".")
61+
trigger := parts[0]
62+
service := ""
63+
if len(parts) >= 2 {
64+
service = parts[1]
65+
} else {
66+
service = parts[0]
67+
trigger = ""
68+
}
69+
if v == "" {
70+
recipients = []string{""}
71+
} else {
72+
recipients = parseRecipients(v)
73+
}
74+
callback(trigger, service, recipients, k)
75+
case strings.HasPrefix(k, altPrefix):
76+
var subscriptions []Subscription
77+
var source []byte
78+
if v != "" {
79+
source = []byte(v)
80+
} else {
81+
log.Errorf("Subscription is not defined")
82+
callback("", "", recipients, k)
83+
}
84+
err := yaml.Unmarshal(source, &subscriptions)
85+
if err != nil {
86+
log.Errorf("Notification subscription unrmashal error: %v", err)
87+
callback("", "", recipients, k)
88+
}
89+
for _, v := range subscriptions {
90+
triggers := v.Trigger
91+
destinations := v.Destinations
92+
if len(triggers) == 0 && len(destinations) == 0 {
93+
trigger := ""
94+
destination := ""
95+
recipients = []string{}
96+
log.Printf("Notification triggers and destinations are not configured")
97+
callback(trigger, destination, recipients, k)
98+
} else if len(triggers) == 0 && len(destinations) != 0 {
99+
trigger := ""
100+
log.Printf("Notification triggers are not configured")
101+
for _, destination := range destinations {
102+
log.Printf("trigger: %v, service: %v, recipient: %v \n", trigger, destination.Service, destination.Recipients)
103+
callback(trigger, destination.Service, destination.Recipients, k)
104+
}
105+
} else if len(triggers) != 0 && len(destinations) == 0 {
106+
service := ""
107+
recipients = []string{}
108+
log.Printf("Notification destinations are not configured")
109+
for _, trigger := range triggers {
110+
log.Printf("trigger: %v, service: %v, recipient: %v \n", trigger, service, recipients)
111+
callback(trigger, service, recipients, k)
112+
}
113+
} else {
114+
for _, trigger := range triggers {
115+
for _, destination := range destinations {
116+
log.Printf("Notification trigger: %v, service: %v, recipient: %v \n", trigger, destination.Service, destination.Recipients)
117+
callback(trigger, destination.Service, destination.Recipients, k)
118+
}
119+
}
120+
}
121+
}
122+
default:
123+
callback("", "", recipients, k)
59124
}
60-
callback(trigger, service, recipients, k)
61125
}
62126
}
63127

‎pkg/subscriptions/annotations_test.go

+86-12
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,27 @@ import (
88
"github.com/stretchr/testify/assert"
99
)
1010

11+
var data = `
12+
- trigger: [my-trigger1, my-trigger2, my-trigger3]
13+
destinations:
14+
- service: slack
15+
recipients:
16+
- recipient-1
17+
- recipient-2
18+
`
19+
20+
var data_without_trigger = `
21+
- trigger: []
22+
destinations:
23+
- service: slack
24+
recipients:
25+
- recipient-1
26+
- recipient-2
27+
`
28+
var data_without_destinations = `
29+
- trigger: [my-trigger1, my-trigger2, my-trigger3]
30+
`
31+
1132
func TestNewAnnotations(t *testing.T) {
1233
a := NewAnnotations(map[string]string{})
1334
assert.NotNil(t, a)
@@ -19,47 +40,100 @@ func TestNewAnnotations(t *testing.T) {
1940
func TestIterate(t *testing.T) {
2041
tests := []struct {
2142
annotations map[string]string
22-
trigger string
23-
service string
43+
triggers []string
44+
service []string
2445
recipients []string
2546
key string
2647
}{
2748
{
2849
annotations: map[string]string{
2950
"notifications.argoproj.io/subscribe.my-trigger.slack": "my-channel",
3051
},
31-
trigger: "my-trigger",
32-
service: "slack",
52+
triggers: []string{"my-trigger"},
53+
service: []string{"slack"},
3354
recipients: []string{"my-channel"},
3455
key: "notifications.argoproj.io/subscribe.my-trigger.slack",
3556
},
3657
{
3758
annotations: map[string]string{
3859
"notifications.argoproj.io/subscribe..slack": "my-channel",
3960
},
40-
trigger: "",
41-
service: "slack",
61+
triggers: []string{},
62+
service: []string{"slack"},
4263
recipients: []string{"my-channel"},
4364
key: "notifications.argoproj.io/subscribe..slack",
4465
},
4566
{
4667
annotations: map[string]string{
4768
"notifications.argoproj.io/subscribe.slack": "my-channel",
4869
},
49-
trigger: "",
50-
service: "slack",
70+
triggers: []string{},
71+
service: []string{"slack"},
5172
recipients: []string{"my-channel"},
5273
key: "notifications.argoproj.io/subscribe.slack",
5374
},
75+
{
76+
annotations: map[string]string{
77+
"notifications.argoproj.io/subscriptions": data,
78+
},
79+
triggers: []string{"my-trigger1", "my-trigger-2", "my-trigger-3"},
80+
service: []string{"slack"},
81+
recipients: []string{"recipient-1", "recipient-2"},
82+
key: "notifications.argoproj.io/subscriptions",
83+
},
84+
{
85+
annotations: map[string]string{
86+
"notifications.argoproj.io/subscriptions": data_without_trigger,
87+
},
88+
triggers: []string{},
89+
service: []string{"slack"},
90+
recipients: []string{"recipient-1", "recipient-2"},
91+
key: "notifications.argoproj.io/subscriptions",
92+
},
93+
{
94+
annotations: map[string]string{
95+
"notifications.argoproj.io/subscriptions": data_without_destinations,
96+
},
97+
triggers: []string{"my-trigger1", "my-trigger-2", "my-trigger-3"},
98+
service: []string{},
99+
recipients: []string{},
100+
key: "notifications.argoproj.io/subscriptions",
101+
},
102+
{
103+
annotations: map[string]string{
104+
"notifications.argoproj.io/random": "my-channel",
105+
},
106+
triggers: []string{},
107+
service: []string{},
108+
recipients: []string{"my-channel"},
109+
key: "notifications.argoproj.io/random",
110+
},
111+
{
112+
annotations: map[string]string{
113+
"notifications.argoproj.io/random": "",
114+
},
115+
triggers: []string{},
116+
service: []string{},
117+
recipients: []string{},
118+
key: "notifications.argoproj.io/random",
119+
},
54120
}
55121

56122
for _, tt := range tests {
57123
a := Annotations(tt.annotations)
58124
a.iterate(func(trigger, service string, recipients []string, key string) {
59-
assert.Equal(t, tt.trigger, trigger)
60-
assert.Equal(t, tt.service, service)
61-
assert.Equal(t, tt.recipients, recipients)
62-
assert.Equal(t, tt.key, key)
125+
for _, v := range tt.triggers {
126+
for _, serv := range tt.service {
127+
if trigger == v {
128+
assert.Equal(t, v, trigger)
129+
assert.Equal(t, serv, service)
130+
assert.Equal(t, tt.recipients, recipients)
131+
assert.Equal(t, tt.key, key)
132+
} else {
133+
continue
134+
}
135+
}
136+
}
63137
})
64138
}
65139
}

0 commit comments

Comments
 (0)