Repository: argoproj-labs/argocd-operator (go)

Issue:
reconcile resource inclusions from Argo CD CR
**Describe the bug**
Operator currently ignores changes to resource inclusions expressed in the argo-cd CR instead of reconciling them in the config map

**To Reproduce**
Steps to reproduce the behavior:
1. Deploy operator
2. Update the argo-cd CR resource inclusions
3. wait for config map to be reconciled
4. config map does not reflect changes in resource inclusions 

**Expected behavior**
changes made to argo-cd CR concerning resource inclusions should be reflected in the `argocd-cm` config map

**Screenshots**
If applicable, add screenshots to help explain your problem.

**Additional context**
Original issue: https://issues.redhat.com/browse/GITOPS-1518
Slack thread : https://coreos.slack.com/archives/CMP95ST2N/p1635956090105300


Pull request description:
fix: reoncile resourceInclusions in argocd-cm
**What type of PR is this?**
> Uncomment only one ` /kind` line, and delete the rest.
> For example, `> /kind bug` would simply become: `/kind bug`

 /kind bug
> /kind cleanup
> /kind failing-test
> /kind enhancement
> /kind documentation
> /kind code-refactoring


**What does this PR do / why we need it**:
Adds reconciliation logic for resourceInclusions from argocd CR to argocd-cm config map

**Have you updated the necessary documentation?**

* [ ] Documentation update is required by this PR.
* [ ] Documentation has been updated.

**Which issue(s) this PR fixes**:

Fixes #471 

**How to test changes / Special notes to the reviewer**:
1. Deploy operator
2. Update the argo-cd CR resource inclusions
3. wait for config map to be reconciled
4. config map should reflect changes in resource inclusions


Patch:
diff --git a/controllers/argocd/configmap.go b/controllers/argocd/configmap.go
index f171e334..e1e989a9 100644
--- a/controllers/argocd/configmap.go
+++ b/controllers/argocd/configmap.go
@@ -471,6 +471,11 @@ func (r *ReconcileArgoCD) reconcileExistingArgoConfigMap(cm *corev1.ConfigMap, c
 		changed = true
 	}
 
+	if cm.Data[common.ArgoCDKeyResourceInclusions] != cr.Spec.ResourceInclusions {
+		cm.Data[common.ArgoCDKeyResourceInclusions] = cr.Spec.ResourceInclusions
+		changed = true
+	}
+
 	uri := r.getArgoServerURI(cr)
 	if cm.Data[common.ArgoCDKeyServerURL] != uri {
 		cm.Data[common.ArgoCDKeyServerURL] = uri
diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go
index e4c03bcb..577e69df 100644
--- a/controllers/argocd/util.go
+++ b/controllers/argocd/util.go
@@ -1100,6 +1100,31 @@ func namespaceFilterPredicate() predicate.Predicate {
 				} else {
 					log.Info(fmt.Sprintf("Successfully removed the RBACs for namespace: %s", e.ObjectOld.GetName()))
 				}
+
+				// Delete managed namespace from cluster secret
+				if err = deleteManagedNamespaceFromClusterSecret(ns, e.ObjectOld.GetName(), k8sClient); err != nil {
+					log.Error(err, fmt.Sprintf("unable to delete namespace %s from cluster secret", e.ObjectOld.GetName()))
+				} else {
+					log.Info(fmt.Sprintf("Successfully deleted namespace %s from cluster secret", e.ObjectOld.GetName()))
+				}
+
+			}
+			return false
+		},
+		DeleteFunc: func(e event.DeleteEvent) bool {
+			if ns, ok := e.Object.GetLabels()[common.ArgoCDManagedByLabel]; ok && ns != "" {
+				k8sClient, err := initK8sClient()
+
+				if err != nil {
+					return false
+				}
+				// Delete managed namespace from cluster secret
+				err = deleteManagedNamespaceFromClusterSecret(ns, e.Object.GetName(), k8sClient)
+				if err != nil {
+					log.Error(err, fmt.Sprintf("unable to delete namespace %s from cluster secret", e.Object.GetName()))
+				} else {
+					log.Info(fmt.Sprintf("Successfully deleted namespace %s from cluster secret", e.Object.GetName()))
+				}
 			}
 			return false
 		},
@@ -1141,8 +1166,13 @@ func deleteRBACsForNamespace(ownerNS, sourceNS string, k8sClient kubernetes.Inte
 		}
 	}
 
+	return nil
+}
+
+func deleteManagedNamespaceFromClusterSecret(ownerNS, sourceNS string, k8sClient kubernetes.Interface) error {
+
 	// Get the cluster secret used for configuring ArgoCD
-	labelSelector = metav1.LabelSelector{MatchLabels: map[string]string{common.ArgoCDSecretTypeLabel: "cluster"}}
+	labelSelector := metav1.LabelSelector{MatchLabels: map[string]string{common.ArgoCDSecretTypeLabel: "cluster"}}
 	secrets, err := k8sClient.CoreV1().Secrets(ownerNS).List(context.TODO(), metav1.ListOptions{LabelSelector: labels.Set(labelSelector.MatchLabels).String()})
 	if err != nil {
 		log.Error(err, fmt.Sprintf("failed to retrieve secrets for namespace: %s", ownerNS))


Test patch:
diff --git a/controllers/argocd/configmap_test.go b/controllers/argocd/configmap_test.go
index 2937fdd7..6115180a 100644
--- a/controllers/argocd/configmap_test.go
+++ b/controllers/argocd/configmap_test.go
@@ -345,6 +345,8 @@ func TestReconcileArgoCD_reconcileGPGKeysConfigMap(t *testing.T) {
 func TestReconcileArgoCD_reconcileArgoConfigMap_withResourceInclusions(t *testing.T) {
 	logf.SetLogger(ZapLogger(true))
 	customizations := "testing: testing"
+	updatedCustomizations := "updated-testing: updated-testing"
+
 	a := makeTestArgoCD(func(a *argoprojv1alpha1.ArgoCD) {
 		a.Spec.ResourceInclusions = customizations
 	})
@@ -363,6 +365,20 @@ func TestReconcileArgoCD_reconcileArgoConfigMap_withResourceInclusions(t *testin
 	if c := cm.Data["resource.inclusions"]; c != customizations {
 		t.Fatalf("reconcileArgoConfigMap failed got %q, want %q", c, customizations)
 	}
+
+	a.Spec.ResourceInclusions = updatedCustomizations
+	err = r.reconcileArgoConfigMap(a)
+	assert.NilError(t, err)
+
+	err = r.Client.Get(context.TODO(), types.NamespacedName{
+		Name:      common.ArgoCDConfigMapName,
+		Namespace: testNamespace,
+	}, cm)
+	assert.NilError(t, err)
+
+	if c := cm.Data["resource.inclusions"]; c != updatedCustomizations {
+		t.Fatalf("reconcileArgoConfigMap failed got %q, want %q", c, updatedCustomizations)
+	}
 }
 
 func TestReconcileArgoCD_reconcileArgoConfigMap_withResourceCustomizations(t *testing.T) {
diff --git a/controllers/argocd/util_test.go b/controllers/argocd/util_test.go
index b7e5b0aa..3f236098 100644
--- a/controllers/argocd/util_test.go
+++ b/controllers/argocd/util_test.go
@@ -465,17 +465,6 @@ func TestDeleteRBACsForNamespace(t *testing.T) {
 	_, err = testClient.RbacV1().RoleBindings(testNameSpace).Create(context.TODO(), roleBinding2, metav1.CreateOptions{})
 	assert.NilError(t, err)
 
-	secret := argoutil.NewSecretWithSuffix(a, "xyz")
-	secret.Labels = map[string]string{common.ArgoCDSecretTypeLabel: "cluster"}
-	secret.Data = map[string][]byte{
-		"server":     []byte(common.ArgoCDDefaultServer),
-		"namespaces": []byte(strings.Join([]string{testNameSpace, "testNamespace2"}, ",")),
-	}
-
-	// create secret with the label
-	_, err = testClient.CoreV1().Secrets(a.Namespace).Create(context.TODO(), secret, metav1.CreateOptions{})
-	assert.NilError(t, err)
-
 	// run deleteRBACsForNamespace
 	assert.NilError(t, deleteRBACsForNamespace(a.Namespace, testNameSpace, testClient))
 
@@ -492,11 +481,32 @@ func TestDeleteRBACsForNamespace(t *testing.T) {
 	// roleBinding without the label should still exists, no error
 	_, err = testClient.RbacV1().Roles(testNameSpace).Get(context.TODO(), roleBinding2.Name, metav1.GetOptions{})
 	assert.NilError(t, err)
+}
+
+func TestRemoveManagedNamespaceFromClusterSecretAfterDeletion(t *testing.T) {
+	a := makeTestArgoCD()
+	testClient := testclient.NewSimpleClientset()
+	testNameSpace := "testNameSpace"
+
+	secret := argoutil.NewSecretWithSuffix(a, "xyz")
+	secret.Labels = map[string]string{common.ArgoCDSecretTypeLabel: "cluster"}
+	secret.Data = map[string][]byte{
+		"server":     []byte(common.ArgoCDDefaultServer),
+		"namespaces": []byte(strings.Join([]string{testNameSpace, "testNamespace2"}, ",")),
+	}
+
+	// create secret with the label
+	_, err := testClient.CoreV1().Secrets(a.Namespace).Create(context.TODO(), secret, metav1.CreateOptions{})
+	assert.NilError(t, err)
+
+	// run deleteManagedNamespaceFromClusterSecret
+	assert.NilError(t, deleteManagedNamespaceFromClusterSecret(a.Namespace, testNameSpace, testClient))
 
 	// secret should still exists with updated list of namespaces
 	s, err := testClient.CoreV1().Secrets(a.Namespace).Get(context.TODO(), secret.Name, metav1.GetOptions{})
 	assert.NilError(t, err)
 	assert.DeepEqual(t, string(s.Data["namespaces"]), "testNamespace2")
+
 }
 
 func TestRemoveManagedByLabelFromNamespaces(t *testing.T) {