Repository: argoproj-labs/applicationset (go)

Issue:
ApplicationSet which generates Applications with matching names should report an error and return
As of this writing, the `generateApplications` method does not check for applications with duplicate names, and thus multiple generated Applications may use the same name.

As discussed in the comments of the design proposal, this should be reported as an error:
> Two application with the same name would be considered an error. In this case error would be saved in status as an error condition.

### To reproduce:

Apply the following YAML, and observe that it reports `generated 3 applications`, even though only 1 Application resource is created:
```yaml
# The list generator specifies a literal list of argument values to the app spec template.
apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: guestbook
spec:
  generators:
  - list:
      elements:
      - cluster: engineering-dev
        url: https://1.2.3.4
      - cluster: engineering-prod
        url: https://2.4.6.8
      - cluster: finance-preprod
        url: https://9.8.7.6
  template:
    metadata:
      name: 'matching-guestbook'
    spec:
      project: default
      source:
        repoURL: https://github.com/jgwest/argocd-example-apps.git
        targetRevision: HEAD
        path: guestbook
      destination:
        server: https://kubernetes.default.svc
        namespace: guestbook
```

In this case, 0 applications should be generated, and an error should be reported in the ApplicationSet status conditions.

Pull request description:
Raise error for duplicate application names
Signed-off-by: John Pitman <jpitman@redhat.com>

Fixes #68

Patch:
diff --git a/pkg/controllers/applicationset_controller.go b/pkg/controllers/applicationset_controller.go
index 29663a5..4f31a08 100644
--- a/pkg/controllers/applicationset_controller.go
+++ b/pkg/controllers/applicationset_controller.go
@@ -78,6 +78,21 @@ func (r *ApplicationSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err
 	if err != nil {
 		return ctrl.Result{}, err
 	}
+	if hasDuplicates, name := hasDuplicateNames(desiredApplications); hasDuplicates {
+		// The reconciler presumes that any errors that are returned are a signal
+		// that the resource should attempt to be reconciled again (causing
+		// Reconcile to be called again, which will return the same error, ad
+		// infinitum until we are exponentially backed off).
+		//
+		// In this case, since we know that what the user provided is incorrect
+		// (we successfully generated and templated their ApplicationSet, but the
+		// result of that was bad), no matter how many times we try to do so it
+		// will fail. So just log it and return that the resource was
+		// successfully reconciled (which is true... it was reconciled to an
+		// error condition).
+		log.Errorf("ApplicationSet %s contains applications with duplicate name: %s", applicationSetInfo.Name, name)
+		return ctrl.Result{}, nil
+	}
 
 	if r.Policy.Update() {
 		err = r.createOrUpdateInCluster(ctx, applicationSetInfo, desiredApplications)
@@ -190,6 +205,17 @@ func addInvalidGeneratorNames(names map[string]bool, applicationSetInfo *argopro
 	}
 }
 
+func hasDuplicateNames(applications []argov1alpha1.Application) (bool, string) {
+	nameSet := map[string]struct{}{}
+	for _, app := range applications {
+		if _, present := nameSet[app.Name]; present {
+			return true, app.Name
+		}
+		nameSet[app.Name] = struct{}{}
+	}
+	return false, ""
+}
+
 func (r *ApplicationSetReconciler) GetRelevantGenerators(requestedGenerator *argoprojiov1alpha1.ApplicationSetGenerator) []generators.Generator {
 	var res []generators.Generator
 


Test patch:
diff --git a/pkg/controllers/applicationset_controller_test.go b/pkg/controllers/applicationset_controller_test.go
index 4c07f89..8d89be3 100644
--- a/pkg/controllers/applicationset_controller_test.go
+++ b/pkg/controllers/applicationset_controller_test.go
@@ -1109,3 +1109,63 @@ func TestCheckInvalidGenerators(t *testing.T) {
 		hook.Reset()
 	}
 }
+
+func TestHasDuplicateNames(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 {
+		testName      string
+		desiredApps   []argov1alpha1.Application
+		hasDuplicates bool
+		duplicateName string
+	}{
+		{
+			testName: "has no duplicates",
+			desiredApps: []argov1alpha1.Application{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "app1",
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "app2",
+					},
+				},
+			},
+			hasDuplicates: false,
+			duplicateName: "",
+		},
+		{
+			testName: "has duplicates",
+			desiredApps: []argov1alpha1.Application{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "app1",
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "app2",
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "app1",
+					},
+				},
+			},
+			hasDuplicates: true,
+			duplicateName: "app1",
+		},
+	} {
+		hasDuplicates, name := hasDuplicateNames(c.desiredApps)
+		assert.Equal(t, c.hasDuplicates, hasDuplicates)
+		assert.Equal(t, c.duplicateName, name)
+	}
+}