diff --git a/controllers/argocd/secret.go b/controllers/argocd/secret.go index 06d7cfa3d..b05d6ab3e 100644 --- a/controllers/argocd/secret.go +++ b/controllers/argocd/secret.go @@ -391,23 +391,14 @@ func (r *ReconcileArgoCD) reconcileGrafanaSecret(cr *argoproj.ArgoCD) error { return nil } -// reconcileClusterPermissionsSecret ensures ArgoCD instance is namespace-scoped -func (r *ReconcileArgoCD) reconcileClusterPermissionsSecret(cr *argoproj.ArgoCD) error { - var clusterConfigInstance bool - secret := argoutil.NewSecretWithSuffix(cr, "default-cluster-config") - secret.Labels[common.ArgoCDSecretTypeLabel] = "cluster" - dataBytes, _ := json.Marshal(map[string]interface{}{ - "tlsClientConfig": map[string]interface{}{ - "insecure": false, - }, - }) - +// generateSortedManagedNamespaceListForArgoCDCR return a list of namespaces with 'managed-by' label that are managed by this 'cr', and including the namespace containing 'cr' +func generateSortedManagedNamespaceListForArgoCDCR(cr *argoproj.ArgoCD, rClient client.Client) ([]string, error) { namespaceList := corev1.NamespaceList{} listOption := client.MatchingLabels{ common.ArgoCDManagedByLabel: cr.Namespace, } - if err := r.Client.List(context.TODO(), &namespaceList, listOption); err != nil { - return err + if err := rClient.List(context.TODO(), &namespaceList, listOption); err != nil { + return nil, err } var namespaces []string @@ -419,54 +410,132 @@ func (r *ReconcileArgoCD) reconcileClusterPermissionsSecret(cr *argoproj.ArgoCD) namespaces = append(namespaces, cr.Namespace) } sort.Strings(namespaces) + return namespaces, nil +} - secret.Data = map[string][]byte{ - "config": dataBytes, - "name": []byte("in-cluster"), - "server": []byte(common.ArgoCDDefaultServer), - "namespaces": []byte(strings.Join(namespaces, ",")), +// combineClusterSecretNamespacesWithManagedNamespaces will combine the contents of clusterSecret's .data.namespaces value, with the list of namespaces in 'managedNamespaceList', sorting them and removing duplicates. +func combineClusterSecretNamespacesWithManagedNamespaces(clusterSecret corev1.Secret, managedNamespaceList []string) string { + namespacesToManageMap := map[string]string{} + + for _, managedNamespace := range managedNamespaceList { + namespacesToManageMap[managedNamespace] = managedNamespace + } + + clusterSecretNamespaces := strings.Split(string(clusterSecret.Data["namespaces"]), ",") + for _, clusterSecretNS := range clusterSecretNamespaces { + ns := strings.TrimSpace(clusterSecretNS) + namespacesToManageMap[ns] = ns + } + + namespacesToManageList := []string{} + for namespaceToManage := range namespacesToManageMap { + namespacesToManageList = append(namespacesToManageList, namespaceToManage) } + sort.Strings(namespacesToManageList) + + namespacesToManageString := strings.Join(namespacesToManageList, ",") + + return namespacesToManageString + +} + +// reconcileClusterPermissionsSecret ensures ArgoCD instance is namespace-scoped +func (r *ReconcileArgoCD) reconcileClusterPermissionsSecret(cr *argoproj.ArgoCD) error { + + managedNamespaceList, err := generateSortedManagedNamespaceListForArgoCDCR(cr, r.Client) + if err != nil { + return err + } + + // isArgoCDAClusterConfigInstance indicates whether 'cr' is a cluster config instance (mentioned in ARGOCD_CLUSTER_CONFIG_NAMESPACES) + var isArgoCDAClusterConfigInstance bool if allowedNamespace(cr.Namespace, os.Getenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES")) { - clusterConfigInstance = true + isArgoCDAClusterConfigInstance = true } + // Get all existing cluster secrets in the namespace clusterSecrets, err := r.getClusterSecrets(cr) if err != nil { return err } - for _, s := range clusterSecrets.Items { + // Find the cluster secret in the list that points to common.ArgoCDDefaultServer (default server address) + var localClusterSecret *corev1.Secret + for x, clusterSecret := range clusterSecrets.Items { + // check if cluster secret with default server address exists - if string(s.Data["server"]) == common.ArgoCDDefaultServer { - // if the cluster belongs to cluster config namespace, - // remove all namespaces from cluster secret, - // else update the list of namespaces if value differs. - var explanation string - if clusterConfigInstance { - delete(s.Data, "namespaces") + if string(clusterSecret.Data["server"]) == common.ArgoCDDefaultServer { + localClusterSecret = &clusterSecrets.Items[x] + } + } + + if localClusterSecret != nil { + + // If the default Cluster Secret already exists + + secretUpdateRequired := false + + // if the cluster belongs to cluster config namespace, + // remove all namespaces from cluster secret, + // else update the list of namespaces if value differs. + var explanation string + if isArgoCDAClusterConfigInstance { + + if _, exists := localClusterSecret.Data["namespaces"]; exists { + delete(localClusterSecret.Data, "namespaces") explanation = "removing namespaces from cluster secret" - } else { - ns := strings.Split(string(s.Data["namespaces"]), ",") - for _, n := range namespaces { - if !containsString(ns, strings.TrimSpace(n)) { - ns = append(ns, strings.TrimSpace(n)) - } - } - sort.Strings(ns) - s.Data["namespaces"] = []byte(strings.Join(ns, ",")) + secretUpdateRequired = true + } + + } else { + + namespacesToManageString := combineClusterSecretNamespacesWithManagedNamespaces(*localClusterSecret, managedNamespaceList) + + var existingNamespacesValue string + if localClusterSecret.Data["namespaces"] != nil { + existingNamespacesValue = string(localClusterSecret.Data["namespaces"]) + } + + if existingNamespacesValue != namespacesToManageString { + localClusterSecret.Data["namespaces"] = []byte(namespacesToManageString) explanation = "updating namespaces in cluster secret" + secretUpdateRequired = true } - argoutil.LogResourceUpdate(log, &s, explanation) - return r.Client.Update(context.TODO(), &s) } + + if secretUpdateRequired { + // We found the Secret, and the field needs to be updated + argoutil.LogResourceUpdate(log, localClusterSecret, explanation) + return r.Client.Update(context.TODO(), localClusterSecret) + } + + // We found the Secret, but the field hasn't changed: no update needed. + return nil } - if clusterConfigInstance { + // If ArgoCD is configured as a cluster-scoped, no need to create a Namespace containing managed namespaces + if isArgoCDAClusterConfigInstance { // do nothing return nil } + // Create the Secret, since we could not find it above + secret := argoutil.NewSecretWithSuffix(cr, "default-cluster-config") + secret.Labels[common.ArgoCDSecretTypeLabel] = "cluster" + dataBytes, _ := json.Marshal(map[string]interface{}{ + "tlsClientConfig": map[string]interface{}{ + "insecure": false, + }, + }) + + secret.Data = map[string][]byte{ + "config": dataBytes, + "name": []byte("in-cluster"), + "server": []byte(common.ArgoCDDefaultServer), + "namespaces": []byte(strings.Join(managedNamespaceList, ",")), + } + if err := controllerutil.SetControllerReference(cr, secret, r.Scheme); err != nil { return err } diff --git a/controllers/argocd/secret_test.go b/controllers/argocd/secret_test.go index 61834b98d..3218d5ab9 100644 --- a/controllers/argocd/secret_test.go +++ b/controllers/argocd/secret_test.go @@ -551,3 +551,103 @@ func Test_ReconcileArgoCD_ClusterPermissionsSecret(t *testing.T) { assert.NoError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: testSecret.Name, Namespace: testSecret.Namespace}, testSecret)) assert.Nil(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: testSecret.Name, Namespace: testSecret.Namespace}, testSecret)) } + +func TestGenerateSortedManagedNamespaceListForArgoCDCR(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + a := makeTestArgoCD() + + resObjs := []client.Object{a} + subresObjs := []client.Object{a} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + r := makeTestReconciler(cl, sch) + + assert.NoError(t, createNamespace(r, a.Namespace, "")) + + // 1) The result of call should include both 'my-managed-namespace' and a.Namespace + managedByNamespace := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-managed-namespace", + Labels: map[string]string{common.ArgoCDManagedByLabel: a.Namespace}, + }, + } + err := cl.Create(context.Background(), &managedByNamespace) + assert.NoError(t, err) + + res, err := generateSortedManagedNamespaceListForArgoCDCR(a, r.Client) + assert.NoError(t, err) + assert.Equal(t, res, []string{a.Namespace, managedByNamespace.Name}) + + // 2) Ensure that results returned by this function are sorted by namespace name + err = cl.Delete(context.Background(), &managedByNamespace) + assert.NoError(t, err) + + managedByNamespace = corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "aaaa-first-when-sorted", + Labels: map[string]string{common.ArgoCDManagedByLabel: a.Namespace}, + }, + } + err = cl.Create(context.Background(), &managedByNamespace) + assert.NoError(t, err) + + res, err = generateSortedManagedNamespaceListForArgoCDCR(a, r.Client) + assert.NoError(t, err) + assert.Equal(t, res, []string{managedByNamespace.Name, a.Namespace}) +} + +func TestCombineClusterSecretNamespacesWithManagedNamespaces(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + a := makeTestArgoCD() + + resObjs := []client.Object{a} + subresObjs := []client.Object{a} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + r := makeTestReconciler(cl, sch) + + assert.NoError(t, createNamespace(r, a.Namespace, "")) + + // 1) Namespaces already listed in the cluster secret should be preserved + res := combineClusterSecretNamespacesWithManagedNamespaces(corev1.Secret{ + Data: map[string][]byte{ + "namespaces": ([]byte)("a,b"), + }, + }, []string{}) + assert.Equal(t, "a,b", res) + + // 2) Duplicates between managed namespaces and existing namespaces should be removed + res = combineClusterSecretNamespacesWithManagedNamespaces(corev1.Secret{ + Data: map[string][]byte{ + "namespaces": ([]byte)("a,b"), + }, + }, []string{"b", "c"}) + assert.Equal(t, "a,b,c", res) + + // 3) Namespace list should be fully sorted by name + res = combineClusterSecretNamespacesWithManagedNamespaces(corev1.Secret{ + Data: map[string][]byte{ + "namespaces": ([]byte)("b,a"), + }, + }, []string{"a", "d", "c", "b"}) + assert.Equal(t, "a,b,c,d", res) + + // 4) Remove duplicates in Secret + res = combineClusterSecretNamespacesWithManagedNamespaces(corev1.Secret{ + Data: map[string][]byte{ + "namespaces": ([]byte)("b,a,b,a"), + }, + }, []string{"c", "d"}) + assert.Equal(t, "a,b,c,d", res) + + // 5) Remove duplicates in string list + res = combineClusterSecretNamespacesWithManagedNamespaces(corev1.Secret{ + Data: map[string][]byte{ + "namespaces": ([]byte)("a,b"), + }, + }, []string{"c", "d", "e", "c", "d"}) + assert.Equal(t, "a,b,c,d,e", res) + +} diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go index f7f0184dd..13cc831bb 100644 --- a/controllers/argocd/util.go +++ b/controllers/argocd/util.go @@ -1344,32 +1344,55 @@ func deleteManagedNamespaceFromClusterSecret(ownerNS, sourceNS string, k8sClient log.Error(err, message) return fmt.Errorf("%s error: %w", message, err) } - for _, secret := range secrets.Items { - if string(secret.Data["server"]) != common.ArgoCDDefaultServer { + + // Find the cluster secret in the list that points to common.ArgoCDDefaultServer (default server address) + var localClusterSecret *corev1.Secret + for x, clusterSecret := range secrets.Items { + + // check if cluster secret with default server address exists + if string(clusterSecret.Data["server"]) == common.ArgoCDDefaultServer { + localClusterSecret = &secrets.Items[x] + } + } + + if localClusterSecret == nil { + // The Secret doesn't exist: no more work to do. + return nil + } + + // If the Secret doesn't even have a 'namespaces' field, we are done. + oldNamespacesFromClusterSecret, ok := localClusterSecret.Data["namespaces"] + if !ok { + return nil + } + + oldNamespaceListFromClusterSecret := strings.Split(string(oldNamespacesFromClusterSecret), ",") + var newNamespaceList []string + + for _, n := range oldNamespaceListFromClusterSecret { + // remove the namespace from the list of namespaces + if strings.TrimSpace(n) == sourceNS { continue } - if namespaces, ok := secret.Data["namespaces"]; ok { - namespaceList := strings.Split(string(namespaces), ",") - var result []string - - for _, n := range namespaceList { - // remove the namespace from the list of namespaces - if strings.TrimSpace(n) == sourceNS { - continue - } - result = append(result, strings.TrimSpace(n)) - sort.Strings(result) - secret.Data["namespaces"] = []byte(strings.Join(result, ",")) - } - // Update the secret with the updated list of namespaces - argoutil.LogResourceUpdate(log, &secret, "removing managed namespace", sourceNS) - if _, err = k8sClient.CoreV1().Secrets(ownerNS).Update(context.TODO(), &secret, metav1.UpdateOptions{}); err != nil { - message := fmt.Sprintf("failed to update cluster permission secret for namespace: %s", ownerNS) - log.Error(err, message) - return fmt.Errorf("%s error: %w", message, err) - } + newNamespaceList = append(newNamespaceList, strings.TrimSpace(n)) + } + sort.Strings(newNamespaceList) + newNamespaceListString := strings.Join(newNamespaceList, ",") + + // If the namespace list has changed, update the cluster secret + if string(oldNamespacesFromClusterSecret) != newNamespaceListString { + + localClusterSecret.Data["namespaces"] = []byte(newNamespaceListString) + + // Update the secret with the updated list of namespaces + argoutil.LogResourceUpdate(log, localClusterSecret, "removing managed namespace", sourceNS) + if _, err = k8sClient.CoreV1().Secrets(ownerNS).Update(context.TODO(), localClusterSecret, metav1.UpdateOptions{}); err != nil { + message := fmt.Sprintf("failed to update cluster permission secret for namespace: %s", ownerNS) + log.Error(err, message) + return fmt.Errorf("%s error: %w", message, err) } } + return nil } diff --git a/controllers/argocd/util_test.go b/controllers/argocd/util_test.go index c676764b9..924b227e7 100644 --- a/controllers/argocd/util_test.go +++ b/controllers/argocd/util_test.go @@ -722,7 +722,7 @@ func TestRemoveManagedNamespaceFromClusterSecretAfterDeletion(t *testing.T) { // secret should still exists with updated list of namespaces s, err := testClient.CoreV1().Secrets(a.Namespace).Get(context.TODO(), secret.Name, metav1.GetOptions{}) assert.NoError(t, err) - assert.Equal(t, string(s.Data["namespaces"]), "testNamespace2") + assert.Equal(t, "testNamespace2", string(s.Data["namespaces"])) }