fix: broken check for context in notificationController (#1626) (#1657)

* fix: broken check for context in notificationController (#1626)

* fix: broken check for context in notificationController

Signed-off-by: Anand Kumar Singh <[email protected]>

* add unit tests

Signed-off-by: Anand Kumar Singh <[email protected]>

* update test

Signed-off-by: Anand Kumar Singh <[email protected]>

---------

Signed-off-by: Anand Kumar Singh <[email protected]>
(cherry picked from commit eace21c)

* update logic to simplify the code

Signed-off-by: Anand Kumar Singh <[email protected]>

---------

Signed-off-by: Anand Kumar Singh <[email protected]>

controllers/notificationsconfiguration/configmap.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -65,7 +66,16 @@ func (r *NotificationsConfigurationReconciler) reconcileNotificationsConfigmap(c
 		expectedConfiguration["context"] = mapToString(cr.Spec.Context)
 	}
 
-	if !reflect.DeepEqual(expectedConfiguration, NotificationsConfigMap.Data) {
+	// check context separately as converting context map to string produce different string due to random serialization of map value
+	changed := checkIfContextChanged(cr, NotificationsConfigMap)
+
+	for k, _ := range expectedConfiguration {
+		if !reflect.DeepEqual(expectedConfiguration[k], NotificationsConfigMap.Data[k]) && k != "context" {
+			changed = true
+		}
+	}
+
+	if changed {
 		NotificationsConfigMap.Data = expectedConfiguration
 		err := r.Client.Update(context.TODO(), NotificationsConfigMap)
 		if err != nil {
@@ -76,10 +86,34 @@ func (r *NotificationsConfigurationReconciler) reconcileNotificationsConfigmap(c
 	// Do nothing
 	return nil
 }
+
 func mapToString(m map[string]string) string {
 	result := ""
 	for key, value := range m {
 		result += fmt.Sprintf("%s: %s\n", key, value)
 	}
 	return result
 }
+
+// checkIfContextChanged checks if context value in NotificationConfiguration and notificationConfigMap context have same value
+// return true if there is difference, and false if no changes observed
+func checkIfContextChanged(cr *v1alpha1.NotificationsConfiguration, notificationConfigMap *corev1.ConfigMap) bool {
+	cmContext := strings.Split(strings.TrimSuffix(notificationConfigMap.Data["context"], "\n"), "\n")
+	if len(cmContext) != len(cr.Spec.Context) {
+		return true
+	} else {
+		// Create a map for quick lookups
+		stringMap := make(map[string]bool)
+		for _, item := range cmContext {
+			stringMap[item] = true
+		}
+
+		// Check for each item in array1
+		for key, value := range cr.Spec.Context {
+			if !stringMap[fmt.Sprintf("%s: %s", key, value)] {
+				return true
+			}
+		}
+	}
+	return false
+}

controllers/notificationsconfiguration/configmap_test.go

@@ -226,3 +226,38 @@ func TestReconcileNotifications_DeleteConfigMap(t *testing.T) {
 	assert.Equal(t, testCM.Data["trigger.on-sync-status-test"],
 		"- when: app.status.sync.status == 'Unknown' \n send: [my-custom-template]")
 }
+
+func Test_checkIfContextEquals(t *testing.T) {
+	a := makeTestNotificationsConfiguration(func(a *v1alpha1.NotificationsConfiguration) {})
+	a.Spec = v1alpha1.NotificationsConfigurationSpec{
+		Context: map[string]string{"key1": "value1",
+			"key2": "value2",
+			"key4": "value4",
+			"key3": "value3",
+			"key6": "value6"},
+	}
+
+	var testmap = []struct {
+		testcase string
+		cm       corev1.ConfigMap
+		result   bool
+	}{
+		{"equal context",
+			corev1.ConfigMap{Data: map[string]string{"context": "key4: value4\nkey2: value2\nkey6: value6\nkey1: value1\nkey3: value3\n"}},
+			false,
+		},
+		{"context is not equal",
+			corev1.ConfigMap{Data: map[string]string{"context": "key1: value1\nkey4: value4\nkey9: value9\nkey6: value6\n"}},
+			true,
+		},
+		{"context of same length but not equal",
+			corev1.ConfigMap{Data: map[string]string{"context": "key2: value2\nkey1: value1\nkey4: value4\nkey9: value9\nkey6: value6\n"}},
+			true,
+		},
+	}
+	for _, tt := range testmap {
+		t.Run(tt.testcase, func(t *testing.T) {
+			assert.Equal(t, tt.result, checkIfContextChanged(a, &tt.cm))
+		})
+	}
+}