Repository: argoproj-labs/applicationset (go)

Issue:
Issue #170 is not solved when using `server`, only when using `name` for destination
`ValidationDestination` is being used [here](https://github.com/argoproj-labs/applicationset/blob/v0.1.0/pkg/controllers/applicationset_controller.go#L560) to determine if we should remove the finalizer.  You can see in its implementation [here](https://github.com/argoproj/argo-cd/blob/master/util/argo/argo.go#L296), the entire function only does any validation when `dest.Name != ""`.  So when using `server` for the destination, the function passes and the finalizer is never removed.  

Pull request description:
Issue #170 is not solved when using server, only when using name for destination
This is the same fix as I made on Argo CD [here](https://github.com/argoproj/argo-cd/pull/6557) but since we support older Argo CDs in ApplicationSet controller, it needs to be made here as well.
- In addition to verifying using the name field (as before), we now also verify using the server field
- Unit test to verify the logic is correct

Fixes #220 

Patch:
diff --git a/api/v1alpha1/applicationset_types.go b/api/v1alpha1/applicationset_types.go
index 67e1b21..aff0161 100644
--- a/api/v1alpha1/applicationset_types.go
+++ b/api/v1alpha1/applicationset_types.go
@@ -65,6 +65,7 @@ type ApplicationSetTemplateMeta struct {
 	Namespace   string            `json:"namespace,omitempty"`
 	Labels      map[string]string `json:"labels,omitempty"`
 	Annotations map[string]string `json:"annotations,omitempty"`
+	Finalizers  []string          `json:"finalizers,omitempty"`
 }
 
 // ApplicationSetGenerator include list item info
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
index b0f7f3d..5eae33e 100644
--- a/api/v1alpha1/zz_generated.deepcopy.go
+++ b/api/v1alpha1/zz_generated.deepcopy.go
@@ -260,6 +260,11 @@ func (in *ApplicationSetTemplateMeta) DeepCopyInto(out *ApplicationSetTemplateMe
 			(*out)[key] = val
 		}
 	}
+	if in.Finalizers != nil {
+		in, out := &in.Finalizers, &out.Finalizers
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ApplicationSetTemplateMeta.
diff --git a/manifests/crds/argoproj.io_applicationsets.yaml b/manifests/crds/argoproj.io_applicationsets.yaml
index 0d2f3f5..46a748d 100644
--- a/manifests/crds/argoproj.io_applicationsets.yaml
+++ b/manifests/crds/argoproj.io_applicationsets.yaml
@@ -122,6 +122,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -623,6 +627,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -1099,6 +1107,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -1566,6 +1578,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -2093,6 +2109,10 @@ spec:
                                             additionalProperties:
                                               type: string
                                             type: object
+                                          finalizers:
+                                            items:
+                                              type: string
+                                            type: array
                                           labels:
                                             additionalProperties:
                                               type: string
@@ -2650,6 +2670,10 @@ spec:
                                             additionalProperties:
                                               type: string
                                             type: object
+                                          finalizers:
+                                            items:
+                                              type: string
+                                            type: array
                                           labels:
                                             additionalProperties:
                                               type: string
@@ -3176,6 +3200,10 @@ spec:
                                             additionalProperties:
                                               type: string
                                             type: object
+                                          finalizers:
+                                            items:
+                                              type: string
+                                            type: array
                                           labels:
                                             additionalProperties:
                                               type: string
@@ -3694,6 +3722,10 @@ spec:
                                             additionalProperties:
                                               type: string
                                             type: object
+                                          finalizers:
+                                            items:
+                                              type: string
+                                            type: array
                                           labels:
                                             additionalProperties:
                                               type: string
@@ -4294,6 +4326,10 @@ spec:
                                             additionalProperties:
                                               type: string
                                             type: object
+                                          finalizers:
+                                            items:
+                                              type: string
+                                            type: array
                                           labels:
                                             additionalProperties:
                                               type: string
@@ -4785,6 +4821,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -5329,6 +5369,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -5782,6 +5826,10 @@ spec:
                         additionalProperties:
                           type: string
                         type: object
+                      finalizers:
+                        items:
+                          type: string
+                        type: array
                       labels:
                         additionalProperties:
                           type: string
diff --git a/manifests/install-with-argo-cd.yaml b/manifests/install-with-argo-cd.yaml
index 4fe07ab..415ea1c 100644
--- a/manifests/install-with-argo-cd.yaml
+++ b/manifests/install-with-argo-cd.yaml
@@ -1850,6 +1850,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -2214,6 +2218,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -2572,6 +2580,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -2920,6 +2932,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -3294,6 +3310,10 @@ spec:
                                             additionalProperties:
                                               type: string
                                             type: object
+                                          finalizers:
+                                            items:
+                                              type: string
+                                            type: array
                                           labels:
                                             additionalProperties:
                                               type: string
@@ -3658,6 +3678,10 @@ spec:
                                             additionalProperties:
                                               type: string
                                             type: object
+                                          finalizers:
+                                            items:
+                                              type: string
+                                            type: array
                                           labels:
                                             additionalProperties:
                                               type: string
@@ -4016,6 +4040,10 @@ spec:
                                             additionalProperties:
                                               type: string
                                             type: object
+                                          finalizers:
+                                            items:
+                                              type: string
+                                            type: array
                                           labels:
                                             additionalProperties:
                                               type: string
@@ -4364,6 +4392,10 @@ spec:
                                             additionalProperties:
                                               type: string
                                             type: object
+                                          finalizers:
+                                            items:
+                                              type: string
+                                            type: array
                                           labels:
                                             additionalProperties:
                                               type: string
@@ -4776,6 +4808,10 @@ spec:
                                             additionalProperties:
                                               type: string
                                             type: object
+                                          finalizers:
+                                            items:
+                                              type: string
+                                            type: array
                                           labels:
                                             additionalProperties:
                                               type: string
@@ -5102,6 +5138,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -5514,6 +5554,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -5847,6 +5891,10 @@ spec:
                         additionalProperties:
                           type: string
                         type: object
+                      finalizers:
+                        items:
+                          type: string
+                        type: array
                       labels:
                         additionalProperties:
                           type: string
diff --git a/manifests/install.yaml b/manifests/install.yaml
index adfed7f..f346501 100644
--- a/manifests/install.yaml
+++ b/manifests/install.yaml
@@ -89,6 +89,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -453,6 +457,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -811,6 +819,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -1159,6 +1171,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -1533,6 +1549,10 @@ spec:
                                             additionalProperties:
                                               type: string
                                             type: object
+                                          finalizers:
+                                            items:
+                                              type: string
+                                            type: array
                                           labels:
                                             additionalProperties:
                                               type: string
@@ -1897,6 +1917,10 @@ spec:
                                             additionalProperties:
                                               type: string
                                             type: object
+                                          finalizers:
+                                            items:
+                                              type: string
+                                            type: array
                                           labels:
                                             additionalProperties:
                                               type: string
@@ -2255,6 +2279,10 @@ spec:
                                             additionalProperties:
                                               type: string
                                             type: object
+                                          finalizers:
+                                            items:
+                                              type: string
+                                            type: array
                                           labels:
                                             additionalProperties:
                                               type: string
@@ -2603,6 +2631,10 @@ spec:
                                             additionalProperties:
                                               type: string
                                             type: object
+                                          finalizers:
+                                            items:
+                                              type: string
+                                            type: array
                                           labels:
                                             additionalProperties:
                                               type: string
@@ -3015,6 +3047,10 @@ spec:
                                             additionalProperties:
                                               type: string
                                             type: object
+                                          finalizers:
+                                            items:
+                                              type: string
+                                            type: array
                                           labels:
                                             additionalProperties:
                                               type: string
@@ -3341,6 +3377,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -3753,6 +3793,10 @@ spec:
                                   additionalProperties:
                                     type: string
                                   type: object
+                                finalizers:
+                                  items:
+                                    type: string
+                                  type: array
                                 labels:
                                   additionalProperties:
                                     type: string
@@ -4086,6 +4130,10 @@ spec:
                         additionalProperties:
                           type: string
                         type: object
+                      finalizers:
+                        items:
+                          type: string
+                        type: array
                       labels:
                         additionalProperties:
                           type: string
diff --git a/pkg/controllers/applicationset_controller.go b/pkg/controllers/applicationset_controller.go
index 05a6347..044d94b 100644
--- a/pkg/controllers/applicationset_controller.go
+++ b/pkg/controllers/applicationset_controller.go
@@ -211,6 +211,7 @@ func getTempApplication(applicationSetTemplate argoprojiov1alpha1.ApplicationSet
 	tmplApplication.Namespace = applicationSetTemplate.Namespace
 	tmplApplication.Name = applicationSetTemplate.Name
 	tmplApplication.Spec = applicationSetTemplate.Spec
+	tmplApplication.Finalizers = applicationSetTemplate.Finalizers
 
 	return &tmplApplication
 }
@@ -383,10 +384,15 @@ func (r *ApplicationSetReconciler) getCurrentApplications(_ context.Context, app
 	return current.Items, nil
 }
 
-// deleteInCluster will delete application that are current in the cluster but not in appList.
+// deleteInCluster will delete Applications that are currently on the cluster, but not in appList.
 // The function must be called after all generators had been called and generated applications
 func (r *ApplicationSetReconciler) deleteInCluster(ctx context.Context, applicationSet argoprojiov1alpha1.ApplicationSet, desiredApplications []argov1alpha1.Application) error {
 
+	clusterList, err := utils.ListClusters(ctx, r.KubeClientset, applicationSet.Namespace)
+	if err != nil {
+		return err
+	}
+
 	// Save current applications to be able to delete the ones that are not in appList
 	current, err := r.getCurrentApplications(ctx, applicationSet)
 	if err != nil {
@@ -408,7 +414,7 @@ func (r *ApplicationSetReconciler) deleteInCluster(ctx context.Context, applicat
 		if !exists {
 
 			// Removes the Argo CD resources finalizer if the application contains an invalid target (eg missing cluster)
-			err := r.removeFinalizerOnInvalidDestination(ctx, applicationSet, &app, appLog)
+			err := r.removeFinalizerOnInvalidDestination(ctx, applicationSet, &app, clusterList, appLog)
 			if err != nil {
 				appLog.WithError(err).Error("failed to update Application")
 				if firstError != nil {
@@ -433,16 +439,49 @@ func (r *ApplicationSetReconciler) deleteInCluster(ctx context.Context, applicat
 }
 
 // removeFinalizerOnInvalidDestination removes the Argo CD resources finalizer if the application contains an invalid target (eg missing cluster)
-func (r *ApplicationSetReconciler) removeFinalizerOnInvalidDestination(ctx context.Context, applicationSet argoprojiov1alpha1.ApplicationSet, app *argov1alpha1.Application, appLog *log.Entry) error {
+func (r *ApplicationSetReconciler) removeFinalizerOnInvalidDestination(ctx context.Context, applicationSet argoprojiov1alpha1.ApplicationSet, app *argov1alpha1.Application, clusterList *argov1alpha1.ClusterList, appLog *log.Entry) error {
 
 	// Only check if the finalizers need to be removed IF there are finalizers to remove
 	if len(app.Finalizers) == 0 {
 		return nil
 	}
 
+	var validDestination bool
+
+	// Detect if the destination is invalid (name doesn't correspond to a matching cluster)
+	if err := utils.ValidateDestination(ctx, &app.Spec.Destination, r.KubeClientset, applicationSet.Namespace); err != nil {
+		appLog.Warnf("The destination cluster for %s couldn't be found: %v", app.Name, err)
+		validDestination = false
+	} else {
+
+		// Detect if the destination's server field does not match an existing cluster
+
+		matchingCluster := false
+		for _, cluster := range clusterList.Items {
+
+			// Server fields must match. Note that ValidateDestination ensures that the server field is set, if applicable.
+			if app.Spec.Destination.Server != cluster.Server {
+				continue
+			}
+
+			// The name must match, if it is not empty
+			if app.Spec.Destination.Name != "" && cluster.Name != app.Spec.Destination.Name {
+				continue
+			}
+
+			matchingCluster = true
+			break
+		}
+
+		if !matchingCluster {
+			appLog.Warnf("A match for the destination cluster for %s, by server url, couldn't be found.", app.Name)
+		}
+
+		validDestination = matchingCluster
+	}
 	// If the destination is invalid (for example the cluster is no longer defined), then remove
 	// the application finalizers to avoid triggering Argo CD bug #5817
-	if err := utils.ValidateDestination(ctx, &app.Spec.Destination, r.KubeClientset, applicationSet.Namespace); err != nil {
+	if !validDestination {
 
 		// Filter out the Argo CD finalizer from the finalizer list
 		var newFinalizers []string
diff --git a/pkg/utils/clusterUtils.go b/pkg/utils/clusterUtils.go
index 9cb2713..e48abb2 100644
--- a/pkg/utils/clusterUtils.go
+++ b/pkg/utils/clusterUtils.go
@@ -47,7 +47,8 @@ const (
 
 func ListClusters(ctx context.Context, clientset kubernetes.Interface, namespace string) (*appv1.ClusterList, error) {
 
-	clusterSecretsList, err := clientset.CoreV1().Secrets(namespace).List(ctx, metav1.ListOptions{LabelSelector: common.LabelKeySecretType + "=" + common.LabelValueSecretTypeCluster})
+	clusterSecretsList, err := clientset.CoreV1().Secrets(namespace).List(ctx,
+		metav1.ListOptions{LabelSelector: common.LabelKeySecretType + "=" + common.LabelValueSecretTypeCluster})
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/utils/util.go b/pkg/utils/util.go
index 99f219a..21599fd 100644
--- a/pkg/utils/util.go
+++ b/pkg/utils/util.go
@@ -49,11 +49,15 @@ func (r *Render) RenderTemplateParams(tmpl *argov1alpha1.Application, syncPolicy
 		return nil, err
 	}
 
-	if syncPolicy == nil || !syncPolicy.PreserveResourcesOnDeletion {
-		if replacedTmpl.ObjectMeta.Finalizers == nil {
-			replacedTmpl.ObjectMeta.Finalizers = []string{}
-		}
-		replacedTmpl.ObjectMeta.Finalizers = append(replacedTmpl.ObjectMeta.Finalizers, "resources-finalizer.argocd.argoproj.io")
+	// Add the 'resources-finalizer' finalizer if:
+	// The template application doesn't have any finalizers, and:
+	// a) there is no syncPolicy, or
+	// b) there IS a syncPolicy, but preserveResourcesOnDeletion is set to false
+	// See TestRenderTemplateParamsFinalizers in util_test.go for test-based definition of behaviour
+	if (syncPolicy == nil || !syncPolicy.PreserveResourcesOnDeletion) &&
+		(replacedTmpl.ObjectMeta.Finalizers == nil || len(replacedTmpl.ObjectMeta.Finalizers) == 0) {
+
+		replacedTmpl.ObjectMeta.Finalizers = []string{"resources-finalizer.argocd.argoproj.io"}
 	}
 
 	return &replacedTmpl, nil


Test patch:
diff --git a/pkg/controllers/applicationset_controller_test.go b/pkg/controllers/applicationset_controller_test.go
index 44d9de6..ace35bc 100644
--- a/pkg/controllers/applicationset_controller_test.go
+++ b/pkg/controllers/applicationset_controller_test.go
@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/argoproj-labs/applicationset/pkg/generators"
+	"github.com/argoproj-labs/applicationset/pkg/utils"
 	"github.com/argoproj/argo-cd/v2/common"
 	argov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	log "github.com/sirupsen/logrus"
@@ -840,7 +841,7 @@ func TestCreateOrUpdateInCluster(t *testing.T) {
 	}
 }
 
-func TestRemoveFinalizerOnInvalidDestination(t *testing.T) {
+func TestRemoveFinalizerOnInvalidDestination_FinalizerTypes(t *testing.T) {
 
 	scheme := runtime.NewScheme()
 	err := argoprojiov1alpha1.AddToScheme(scheme)
@@ -935,11 +936,14 @@ func TestRemoveFinalizerOnInvalidDestination(t *testing.T) {
 				KubeClientset: kubeclientset,
 			}
 
+			clusterList, err := utils.ListClusters(context.Background(), kubeclientset, "namespace")
+			assert.NoError(t, err, "Unexpected error")
+
 			appLog := log.WithFields(log.Fields{"app": app.Name, "appSet": ""})
 
 			appInputParam := app.DeepCopy()
 
-			err = r.removeFinalizerOnInvalidDestination(context.Background(), appSet, appInputParam, appLog)
+			err = r.removeFinalizerOnInvalidDestination(context.Background(), appSet, appInputParam, clusterList, appLog)
 			assert.NoError(t, err, "Unexpected error")
 
 			retrievedApp := argov1alpha1.Application{}
@@ -959,6 +963,163 @@ func TestRemoveFinalizerOnInvalidDestination(t *testing.T) {
 	}
 }
 
+func TestRemoveFinalizerOnInvalidDestination_DestinationTypes(t *testing.T) {
+
+	scheme := runtime.NewScheme()
+	err := argoprojiov1alpha1.AddToScheme(scheme)
+	assert.Nil(t, err)
+
+	err = argov1alpha1.AddToScheme(scheme)
+	assert.Nil(t, err)
+
+	for _, c := range []struct {
+		// name is human-readable test name
+		name                   string
+		destinationField       argov1alpha1.ApplicationDestination
+		expectFinalizerRemoved bool
+	}{
+		{
+			name: "invalid cluster: empty destination",
+			destinationField: argov1alpha1.ApplicationDestination{
+				Namespace: "namespace",
+			},
+			expectFinalizerRemoved: true,
+		},
+		{
+			name: "invalid cluster: invalid server url",
+			destinationField: argov1alpha1.ApplicationDestination{
+				Namespace: "namespace",
+				Server:    "https://1.2.3.4",
+			},
+			expectFinalizerRemoved: true,
+		},
+		{
+			name: "invalid cluster: invalid cluster name",
+			destinationField: argov1alpha1.ApplicationDestination{
+				Namespace: "namespace",
+				Name:      "invalid-cluster",
+			},
+			expectFinalizerRemoved: true,
+		},
+		{
+			name: "invalid cluster by both valid",
+			destinationField: argov1alpha1.ApplicationDestination{
+				Namespace: "namespace",
+				Name:      "mycluster2",
+				Server:    "https://kubernetes.default.svc",
+			},
+			expectFinalizerRemoved: true,
+		},
+		{
+			name: "invalid cluster by both invalid",
+			destinationField: argov1alpha1.ApplicationDestination{
+				Namespace: "namespace",
+				Name:      "mycluster3",
+				Server:    "https://4.5.6.7",
+			},
+			expectFinalizerRemoved: true,
+		},
+		{
+			name: "valid cluster by name",
+			destinationField: argov1alpha1.ApplicationDestination{
+				Namespace: "namespace",
+				Name:      "mycluster2",
+			},
+			expectFinalizerRemoved: false,
+		},
+		{
+			name: "valid cluster by server",
+			destinationField: argov1alpha1.ApplicationDestination{
+				Namespace: "namespace",
+				Server:    "https://kubernetes.default.svc",
+			},
+			expectFinalizerRemoved: false,
+		},
+	} {
+
+		t.Run(c.name, func(t *testing.T) {
+
+			appSet := argoprojiov1alpha1.ApplicationSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "namespace",
+				},
+				Spec: argoprojiov1alpha1.ApplicationSetSpec{
+					Template: argoprojiov1alpha1.ApplicationSetTemplate{
+						Spec: argov1alpha1.ApplicationSpec{
+							Project: "project",
+						},
+					},
+				},
+			}
+
+			app := argov1alpha1.Application{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "app1",
+					Finalizers: []string{common.ResourcesFinalizerName},
+				},
+				Spec: argov1alpha1.ApplicationSpec{
+					Project:     "project",
+					Source:      argov1alpha1.ApplicationSource{Path: "path", TargetRevision: "revision", RepoURL: "repoURL"},
+					Destination: c.destinationField,
+				},
+			}
+
+			initObjs := []crtclient.Object{&app, &appSet}
+
+			client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(initObjs...).Build()
+			secret := &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "my-secret",
+					Namespace: "namespace",
+					Labels: map[string]string{
+						generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster,
+					},
+				},
+				Data: map[string][]byte{
+					// Since this test requires the cluster to be an invalid destination, we
+					// always return a cluster named 'my-cluster2' (different from app 'my-cluster', above)
+					"name":   []byte("mycluster2"),
+					"server": []byte("https://kubernetes.default.svc"),
+					"config": []byte("{\"username\":\"foo\",\"password\":\"foo\"}"),
+				},
+			}
+
+			objects := append([]runtime.Object{}, secret)
+			kubeclientset := kubefake.NewSimpleClientset(objects...)
+
+			r := ApplicationSetReconciler{
+				Client:        client,
+				Scheme:        scheme,
+				Recorder:      record.NewFakeRecorder(10),
+				KubeClientset: kubeclientset,
+			}
+
+			clusterList, err := utils.ListClusters(context.Background(), kubeclientset, "namespace")
+			assert.NoError(t, err, "Unexpected error")
+
+			appLog := log.WithFields(log.Fields{"app": app.Name, "appSet": ""})
+
+			appInputParam := app.DeepCopy()
+
+			err = r.removeFinalizerOnInvalidDestination(context.Background(), appSet, appInputParam, clusterList, appLog)
+			assert.NoError(t, err, "Unexpected error")
+
+			retrievedApp := argov1alpha1.Application{}
+			err = client.Get(context.Background(), crtclient.ObjectKeyFromObject(&app), &retrievedApp)
+			assert.NoError(t, err, "Unexpected error")
+
+			finalizerRemoved := len(retrievedApp.Finalizers) == 0
+
+			assert.True(t, c.expectFinalizerRemoved == finalizerRemoved)
+
+			bytes, _ := json.MarshalIndent(retrievedApp, "", "  ")
+			t.Log("Contents of app after call:", string(bytes))
+
+		})
+	}
+}
+
 func TestCreateApplications(t *testing.T) {
 
 	scheme := runtime.NewScheme()
@@ -1271,9 +1432,10 @@ func TestDeleteInCluster(t *testing.T) {
 		client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(initObjs...).Build()
 
 		r := ApplicationSetReconciler{
-			Client:   client,
-			Scheme:   scheme,
-			Recorder: record.NewFakeRecorder(len(initObjs) + len(c.expected)),
+			Client:        client,
+			Scheme:        scheme,
+			Recorder:      record.NewFakeRecorder(len(initObjs) + len(c.expected)),
+			KubeClientset: kubefake.NewSimpleClientset(),
 		}
 
 		err = r.deleteInCluster(context.TODO(), c.appSet, c.desiredApps)
diff --git a/pkg/utils/util_test.go b/pkg/utils/util_test.go
index 47f52a2..a7e6ba9 100644
--- a/pkg/utils/util_test.go
+++ b/pkg/utils/util_test.go
@@ -167,6 +167,111 @@ func TestRenderTemplateParams(t *testing.T) {
 
 }
 
+func TestRenderTemplateParamsFinalizers(t *testing.T) {
+
+	emptyApplication := &argov1alpha1.Application{
+		Spec: argov1alpha1.ApplicationSpec{
+			Source: argov1alpha1.ApplicationSource{
+				Path:           "",
+				RepoURL:        "",
+				TargetRevision: "",
+				Chart:          "",
+			},
+			Destination: argov1alpha1.ApplicationDestination{
+				Server:    "",
+				Namespace: "",
+				Name:      "",
+			},
+			Project: "",
+		},
+	}
+
+	for _, c := range []struct {
+		testName           string
+		syncPolicy         *argoprojiov1alpha1.ApplicationSetSyncPolicy
+		existingFinalizers []string
+		expectedFinalizers []string
+	}{
+		{
+			testName:           "existing finalizer should be preserved",
+			existingFinalizers: []string{"existing-finalizer"},
+			syncPolicy:         nil,
+			expectedFinalizers: []string{"existing-finalizer"},
+		},
+		{
+			testName:           "background finalizer should be preserved",
+			existingFinalizers: []string{"resources-finalizer.argocd.argoproj.io/background"},
+			syncPolicy:         nil,
+			expectedFinalizers: []string{"resources-finalizer.argocd.argoproj.io/background"},
+		},
+
+		{
+			testName:           "empty finalizer and empty sync should use standard finalizer",
+			existingFinalizers: nil,
+			syncPolicy:         nil,
+			expectedFinalizers: []string{"resources-finalizer.argocd.argoproj.io"},
+		},
+
+		{
+			testName:           "standard finalizer should be preserved",
+			existingFinalizers: []string{"resources-finalizer.argocd.argoproj.io"},
+			syncPolicy:         nil,
+			expectedFinalizers: []string{"resources-finalizer.argocd.argoproj.io"},
+		},
+		{
+			testName:           "empty array finalizers should use standard finalizer",
+			existingFinalizers: []string{},
+			syncPolicy:         nil,
+			expectedFinalizers: []string{"resources-finalizer.argocd.argoproj.io"},
+		},
+		{
+			testName:           "non-nil sync policy should use standard finalizer",
+			existingFinalizers: nil,
+			syncPolicy:         &argoprojiov1alpha1.ApplicationSetSyncPolicy{},
+			expectedFinalizers: []string{"resources-finalizer.argocd.argoproj.io"},
+		},
+		{
+			testName:           "preserveResourcesOnDeletion should not have a finalizer",
+			existingFinalizers: nil,
+			syncPolicy: &argoprojiov1alpha1.ApplicationSetSyncPolicy{
+				PreserveResourcesOnDeletion: true,
+			},
+			expectedFinalizers: nil,
+		},
+		{
+			testName:           "user-specified finalizer should overwrite preserveResourcesOnDeletion",
+			existingFinalizers: []string{"resources-finalizer.argocd.argoproj.io/background"},
+			syncPolicy: &argoprojiov1alpha1.ApplicationSetSyncPolicy{
+				PreserveResourcesOnDeletion: true,
+			},
+			expectedFinalizers: []string{"resources-finalizer.argocd.argoproj.io/background"},
+		},
+	} {
+
+		t.Run(c.testName, func(t *testing.T) {
+
+			// Clone the template application
+			application := emptyApplication.DeepCopy()
+			application.Finalizers = c.existingFinalizers
+
+			params := map[string]string{
+				"one": "two",
+			}
+
+			// Render the cloned application, into a new application
+			render := Render{}
+
+			res, err := render.RenderTemplateParams(application, c.syncPolicy, params)
+			assert.Nil(t, err)
+
+			assert.ElementsMatch(t, res.Finalizers, c.expectedFinalizers)
+
+		})
+
+	}
+
+}
+
 func TestCheckInvalidGenerators(t *testing.T) {
 
 	scheme := runtime.NewScheme()
diff --git a/test/e2e/applicationset/applicationset_test.go b/test/e2e/applicationset/applicationset_test.go
index 6ea8397..d9fefdd 100644
--- a/test/e2e/applicationset/applicationset_test.go
+++ b/test/e2e/applicationset/applicationset_test.go
@@ -441,3 +441,69 @@ func TestSimpleSCMProviderGenerator(t *testing.T) {
 		},
 	}).Then().Expect(ApplicationsExist([]argov1alpha1.Application{expectedApp}))
 }
+
+func TestCustomApplicationFinalizers(t *testing.T) {
+	expectedApp := argov1alpha1.Application{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "Application",
+			APIVersion: "argoproj.io/v1alpha1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:       "my-cluster-guestbook",
+			Namespace:  utils.ArgoCDNamespace,
+			Finalizers: []string{"resources-finalizer.argocd.argoproj.io/background"},
+		},
+		Spec: argov1alpha1.ApplicationSpec{
+			Project: "default",
+			Source: argov1alpha1.ApplicationSource{
+				RepoURL:        "https://github.com/argoproj/argocd-example-apps.git",
+				TargetRevision: "HEAD",
+				Path:           "guestbook",
+			},
+			Destination: argov1alpha1.ApplicationDestination{
+				Server:    "https://kubernetes.default.svc",
+				Namespace: "guestbook",
+			},
+		},
+	}
+
+	Given(t).
+		// Create a ListGenerator-based ApplicationSet
+		When().Create(v1alpha1.ApplicationSet{ObjectMeta: metav1.ObjectMeta{
+		Name: "simple-list-generator",
+	},
+		Spec: v1alpha1.ApplicationSetSpec{
+			Template: v1alpha1.ApplicationSetTemplate{
+				ApplicationSetTemplateMeta: v1alpha1.ApplicationSetTemplateMeta{
+					Name:       "{{cluster}}-guestbook",
+					Finalizers: []string{"resources-finalizer.argocd.argoproj.io/background"},
+				},
+				Spec: argov1alpha1.ApplicationSpec{
+					Project: "default",
+					Source: argov1alpha1.ApplicationSource{
+						RepoURL:        "https://github.com/argoproj/argocd-example-apps.git",
+						TargetRevision: "HEAD",
+						Path:           "guestbook",
+					},
+					Destination: argov1alpha1.ApplicationDestination{
+						Server:    "{{url}}",
+						Namespace: "guestbook",
+					},
+				},
+			},
+			Generators: []v1alpha1.ApplicationSetGenerator{
+				{
+					List: &v1alpha1.ListGenerator{
+						Elements: []v1alpha1.ListGeneratorElement{
+							{Cluster: "my-cluster", Url: "https://kubernetes.default.svc"},
+						},
+					},
+				},
+			},
+		},
+	}).Then().Expect(ApplicationsExist([]argov1alpha1.Application{expectedApp})).
+
+		// Delete the ApplicationSet, and verify it deletes the Applications
+		When().
+		Delete().Then().Expect(ApplicationsDoNotExist([]argov1alpha1.Application{expectedApp}))
+}
diff --git a/test/e2e/applicationset/cluster_e2e_test.go b/test/e2e/applicationset/cluster_e2e_test.go
index 8e2ccfc..56c20b3 100644
--- a/test/e2e/applicationset/cluster_e2e_test.go
+++ b/test/e2e/applicationset/cluster_e2e_test.go
@@ -303,7 +303,6 @@ func TestSimpleClusterGeneratorAddingCluster(t *testing.T) {
 		Delete().Then().Expect(ApplicationsDoNotExist([]argov1alpha1.Application{expectedAppCluster1, expectedAppCluster2}))
 }
 
-// There is a bug in ArgoCD that prevents this test from passing: https://github.com/argoproj/argo-cd/issues/5817
 func TestSimpleClusterGeneratorDeletingCluster(t *testing.T) {
 
 	expectedAppTemplate := argov1alpha1.Application{