From fa460078f770733c66298a3681d4d233ee07ed9d Mon Sep 17 00:00:00 2001 From: RaviHari Date: Thu, 15 Sep 2022 01:36:02 +0530 Subject: [PATCH] fix: enable notifications without when condition (#2231) * fix: enable notifications without when condition Signed-off-by: Ravi Hari * fix: use trigger action item from the list Signed-off-by: Ravi Hari * fix: add emptycondition logic to make notifications work with/without conditions Signed-off-by: Ravi Hari * fix: linting Signed-off-by: Ravi Hari Signed-off-by: Ravi Hari --- utils/record/record.go | 47 ++++++++++++++++++++++++------------- utils/record/record_test.go | 41 ++++++++++++++++++++++++++++---- 2 files changed, 68 insertions(+), 20 deletions(-) diff --git a/utils/record/record.go b/utils/record/record.go index 3ad05671b2..f255b02c24 100644 --- a/utils/record/record.go +++ b/utils/record/record.go @@ -2,6 +2,8 @@ package record import ( "context" + "crypto/sha1" + "encoding/base64" "encoding/json" "regexp" "strings" @@ -266,29 +268,32 @@ func (e *EventRecorderAdapter) sendNotifications(object runtime.Object, opts Eve return nil } - // Creates config for notifications for built-in triggers - triggerActions, ok := cfg.Triggers[trigger] - if !ok { - logCtx.Debugf("No configured template for trigger: %s", trigger) - return nil - } - objMap, err := toObjectMap(object) if err != nil { return err } - res, err := notificationsAPI.RunTrigger(trigger, objMap) - if err != nil { - log.Errorf("Failed to execute condition of trigger %s: %v", trigger, err) - return err - } - log.Infof("Trigger %s result: %v", trigger, res) + emptyCondition := hash("") + + for _, destination := range destinations { + res, err := notificationsAPI.RunTrigger(trigger, objMap) + if err != nil { + log.Errorf("Failed to execute condition of trigger %s: %v", trigger, err) + return err + } + log.Infof("Trigger %s result: %v", trigger, res) - for _, dest := range destinations { for _, c := range res { - if c.Triggered == true { - err = notificationsAPI.Send(objMap, triggerActions[0].Send, dest) + log.Infof("Res When Condition hash: %s, Templates: %s", c.Key, c.Templates) + s := strings.Split(c.Key, ".")[1] + if s != emptyCondition && c.Triggered == true { + err = notificationsAPI.Send(objMap, c.Templates, destination) + if err != nil { + log.Errorf("notification error: %s", err.Error()) + return err + } + } else if s == emptyCondition { + err = notificationsAPI.Send(objMap, c.Templates, destination) if err != nil { log.Errorf("notification error: %s", err.Error()) return err @@ -296,9 +301,19 @@ func (e *EventRecorderAdapter) sendNotifications(object runtime.Object, opts Eve } } } + return nil } +// This function is copied over from notification engine to make sure we honour emptyCondition +// emptyConditions today are not handled well in notification engine. +// TODO: update notification engine to handle emptyConditions and remove this function and its usage +func hash(input string) string { + h := sha1.New() + _, _ = h.Write([]byte(input)) + return base64.RawURLEncoding.EncodeToString(h.Sum(nil)) +} + // toObjectMap converts an object to a map for the purposes of sending to the notification engine func toObjectMap(object interface{}) (map[string]interface{}, error) { objBytes, err := json.Marshal(object) diff --git a/utils/record/record_test.go b/utils/record/record_test.go index 931ce42c9b..c99f55985b 100644 --- a/utils/record/record_test.go +++ b/utils/record/record_test.go @@ -96,7 +96,7 @@ func TestSendNotifications(t *testing.T) { mockCtrl := gomock.NewController(t) mockAPI := mocks.NewMockAPI(mockCtrl) cr := []triggers.ConditionResult{{ - Key: "", + Key: "1." + hash(""), Triggered: true, Templates: []string{"my-template"}, }} @@ -112,6 +112,33 @@ func TestSendNotifications(t *testing.T) { assert.NoError(t, err) } +func TestSendNotificationsWhenCondition(t *testing.T) { + r := v1alpha1.Rollout{ + ObjectMeta: metav1.ObjectMeta{ + Name: "guestbook", + Namespace: "default", + Annotations: map[string]string{"notifications.argoproj.io/subscribe.on-foo-reason.console": "console"}, + }, + } + mockCtrl := gomock.NewController(t) + mockAPI := mocks.NewMockAPI(mockCtrl) + cr := []triggers.ConditionResult{{ + Key: "1." + hash(""), + Triggered: true, + Templates: []string{"my-template"}, + }} + mockAPI.EXPECT().RunTrigger(gomock.Any(), gomock.Any()).Return(cr, nil).AnyTimes() + mockAPI.EXPECT().Send(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() + mockAPI.EXPECT().GetConfig().Return(api.Config{ + Triggers: map[string][]triggers.Condition{"on-foo-reason": {triggers.Condition{When: "rollout.spec.template.spec.containers[0].image == test:blue", Send: []string{"my-template"}}}}}).AnyTimes() + apiFactory := &mocks.FakeFactory{Api: mockAPI} + rec := NewFakeEventRecorder() + rec.EventRecorderAdapter.apiFactory = apiFactory + //ch := make(chan prometheus.HistogramVec, 1) + err := rec.sendNotifications(&r, EventOptions{EventReason: "FooReason"}) + assert.NoError(t, err) +} + func TestNotificationFailedCounter(t *testing.T) { r := v1alpha1.Rollout{ ObjectMeta: metav1.ObjectMeta{ @@ -201,7 +228,7 @@ func TestSendNotificationsFails(t *testing.T) { mockCtrl := gomock.NewController(t) mockAPI := mocks.NewMockAPI(mockCtrl) cr := []triggers.ConditionResult{{ - Key: "", + Key: "1." + hash(""), Triggered: true, Templates: []string{"my-template"}, }} @@ -241,7 +268,7 @@ func TestSendNotificationsFailsWithRunTriggerError(t *testing.T) { mockCtrl := gomock.NewController(t) mockAPI := mocks.NewMockAPI(mockCtrl) cr := []triggers.ConditionResult{{ - Key: "", + Key: "1." + hash(""), Triggered: true, Templates: []string{"my-template"}, }} @@ -279,6 +306,12 @@ func TestSendNotificationsNoTrigger(t *testing.T) { mockCtrl := gomock.NewController(t) mockAPI := mocks.NewMockAPI(mockCtrl) + cr := []triggers.ConditionResult{{ + Key: "1." + hash(""), + Triggered: false, + Templates: []string{"my-template"}, + }} + mockAPI.EXPECT().RunTrigger(gomock.Any(), gomock.Any()).Return(cr, errors.New("trigger 'on-missing-reason' is not configured")).AnyTimes() mockAPI.EXPECT().GetConfig().Return(api.Config{ Triggers: map[string][]triggers.Condition{"on-foo-reason": {triggers.Condition{Send: []string{"my-template"}}}}}).AnyTimes() mockAPI.EXPECT().Send(gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("failed to send")).Times(0) @@ -287,7 +320,7 @@ func TestSendNotificationsNoTrigger(t *testing.T) { rec.EventRecorderAdapter.apiFactory = apiFactory err := rec.sendNotifications(&r, EventOptions{EventReason: "MissingReason"}) - assert.NoError(t, err) + assert.Error(t, err) } func TestNewAPIFactorySettings(t *testing.T) {