Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cherry pick] release-0.12 fix: broken check for context in notificationController (#1626) #1656

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion controllers/notificationsconfiguration/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"reflect"
"strings"

"github.com/argoproj-labs/argocd-operator/api/v1alpha1"
"github.com/argoproj-labs/argocd-operator/controllers/argoutil"
Expand Down Expand Up @@ -64,7 +65,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 {
Expand All @@ -75,10 +85,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) {
anandrkskd marked this conversation as resolved.
Show resolved Hide resolved
// 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
}
}
} else {
return true
}
return false
}
35 changes: 35 additions & 0 deletions controllers/notificationsconfiguration/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
}
}
Loading