chore(notifications): Refactor: invert function to match to applicati…

…on controller's implementation Signed-off-by: motoki317 <motoki317@gmail.com>
argoproj · Oct 30, 2023 · 8bd9b51 · 8bd9b51
1 parent c2f1df0
commit 8bd9b51
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 9 deletions.
diff --git a/notification_controller/controller/controller.go b/notification_controller/controller/controller.go
@@ -89,7 +89,7 @@ func NewController(
 			if !ok {
 				return false, ""
 			}
-			if checkAppNotInAdditionalNamespaces(app, namespace, applicationNamespaces) {
+			if !isAppNamespaceAllowed(app, namespace, applicationNamespaces) {
 				return true, "app is not in one of the application-namespaces, nor the notification controller namespace"
 			}
 			return !isAppSyncStatusRefreshed(app, log.WithField("app", obj.GetName())), "sync status out of date"
@@ -99,9 +99,10 @@ func NewController(
 	return res
 }
 
-// Check if app is not in the namespace where the controller is in, and also app is not in one of the applicationNamespaces
-func checkAppNotInAdditionalNamespaces(app *unstructured.Unstructured, namespace string, applicationNamespaces []string) bool {
-	return namespace != app.GetNamespace() && !glob.MatchStringInList(applicationNamespaces, app.GetNamespace(), false)
+// isAppNamespaceAllowed returns whether app is in the namespace where the controller is in,
+// or app is in one of the applicationNamespaces.
+func isAppNamespaceAllowed(app *unstructured.Unstructured, namespace string, applicationNamespaces []string) bool {
+	return namespace == app.GetNamespace() || glob.MatchStringInList(applicationNamespaces, app.GetNamespace(), false)
 }
 
 func (c *notificationController) alterDestinations(obj v1.Object, destinations services.Destinations, cfg api.Config) services.Destinations {
@@ -130,7 +131,7 @@ func newInformer(resClient dynamic.ResourceInterface, controllerNamespace string
 				}
 				newItems := []unstructured.Unstructured{}
 				for _, res := range appList.Items {
-					if controllerNamespace == res.GetNamespace() || glob.MatchStringInList(applicationNamespaces, res.GetNamespace(), false) {
+					if isAppNamespaceAllowed(&res, controllerNamespace, applicationNamespaces) {
 						newItems = append(newItems, res)
 					}
 				}

diff --git a/notification_controller/controller/controller_test.go b/notification_controller/controller/controller_test.go
@@ -167,7 +167,7 @@ func TestInitTimeout(t *testing.T) {
 	assert.Equal(t, "Timed out waiting for caches to sync", err.Error())
 }
 
-func TestCheckAppNotInAdditionalNamespaces(t *testing.T) {
+func TestIsAppNamespaceAllowed(t *testing.T) {
 	app := &unstructured.Unstructured{
 		Object: map[string]interface{}{
 			"spec": map[string]interface{}{},
@@ -180,13 +180,13 @@ func TestCheckAppNotInAdditionalNamespaces(t *testing.T) {
 
 	// app is in same namespace as controller's namespace
 	app.SetNamespace(namespace)
-	assert.False(t, checkAppNotInAdditionalNamespaces(app, namespace, applicationNamespaces))
+	assert.True(t, isAppNamespaceAllowed(app, namespace, applicationNamespaces))
 
 	// app is not in the namespace as controller's namespace, but it is in one of the applicationNamespaces
 	app.SetNamespace("namespace2")
-	assert.False(t, checkAppNotInAdditionalNamespaces(app, "", applicationNamespaces))
+	assert.True(t, isAppNamespaceAllowed(app, "", applicationNamespaces))
 
 	// app is not in the namespace as controller's namespace, and it is not in any of the applicationNamespaces
 	app.SetNamespace("namespace3")
-	assert.True(t, checkAppNotInAdditionalNamespaces(app, "", applicationNamespaces))
+	assert.False(t, isAppNamespaceAllowed(app, "", applicationNamespaces))
 }