Raise error for duplicate application names (#69)

* Raise error for duplicate application names

Signed-off-by: John Pitman <jpitman@redhat.com>

* Updates based on review comments

Signed-off-by: John Pitman <jpitman@redhat.com>

* Update pkg/controllers/applicationset_controller.go

Co-authored-by: William Tam <wtam@redhat.com>

Co-authored-by: William Tam <wtam@redhat.com>

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
 

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)
+	}
+}