Issue #1070 - Handle duplicated resource definitions (#1284)

controller/appcontroller.go

@@ -717,11 +717,12 @@ func (ctrl *ApplicationController) refreshAppConditions(app *appv1.Application)
 
 	// List of condition types which have to be reevaluated by controller; all remaining conditions should stay as is.
 	reevaluateTypes := map[appv1.ApplicationConditionType]bool{
-		appv1.ApplicationConditionInvalidSpecError:      true,
-		appv1.ApplicationConditionUnknownError:          true,
-		appv1.ApplicationConditionComparisonError:       true,
-		appv1.ApplicationConditionSharedResourceWarning: true,
-		appv1.ApplicationConditionSyncError:             true,
+		appv1.ApplicationConditionInvalidSpecError:        true,
+		appv1.ApplicationConditionUnknownError:            true,
+		appv1.ApplicationConditionComparisonError:         true,
+		appv1.ApplicationConditionSharedResourceWarning:   true,
+		appv1.ApplicationConditionSyncError:               true,
+		appv1.ApplicationConditionRepeatedResourceWarning: true,
 	}
 	appConditions := make([]appv1.ApplicationCondition, 0)
 	for i := 0; i < len(app.Status.Conditions); i++ {

controller/state.go

@@ -131,6 +131,41 @@ func (m *appStateManager) getRepoObjs(app *v1alpha1.Application, source v1alpha1
 	return targetObjs, hooks, manifestInfo, nil
 }
 
+func (m *appStateManager) deduplicateTargetObjects(
+	server string,
+	namespace string,
+	objs []*unstructured.Unstructured,
+) ([]*unstructured.Unstructured, []v1alpha1.ApplicationCondition, error) {
+	targetByKey := make(map[kubeutil.ResourceKey][]*unstructured.Unstructured)
+	for i := range objs {
+		obj := objs[i]
+		isNamespaced, err := m.liveStateCache.IsNamespaced(server, obj)
+		if err != nil {
+			return objs, nil, err
+		}
+		if !isNamespaced {
+			obj.SetNamespace("")
+		} else if obj.GetNamespace() == "" {
+			obj.SetNamespace(namespace)
+		}
+		key := kubeutil.GetResourceKey(obj)
+		targetByKey[key] = append(targetByKey[key], obj)
+	}
+	conditions := make([]v1alpha1.ApplicationCondition, 0)
+	result := make([]*unstructured.Unstructured, 0)
+	for key, targets := range targetByKey {
+		if len(targets) > 1 {
+			conditions = append(conditions, appv1.ApplicationCondition{
+				Type:    appv1.ApplicationConditionRepeatedResourceWarning,
+				Message: fmt.Sprintf("Resource %s appeared %d times among application resources.", key.String(), len(targets)),
+			})
+		}
+		result = append(result, targets[len(targets)-1])
+	}
+
+	return result, conditions, nil
+}
+
 // CompareAppState compares application git state to the live app state, using the specified
 // revision and supplied source. If revision or overrides are empty, then compares against
 // revision and overrides in the app spec.
@@ -151,6 +186,12 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, revision st
 		conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error()})
 		failedToLoadObjs = true
 	}
+	targetObjs, dedupConditions, err := m.deduplicateTargetObjects(app.Spec.Destination.Server, app.Spec.Destination.Namespace, targetObjs)
+	if err != nil {
+		conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error()})
+	}
+	conditions = append(conditions, dedupConditions...)
+
 	logCtx.Debugf("Generated config manifests")
 	liveObjByKey, err := m.liveStateCache.GetManagedLiveObjs(app, targetObjs)
 	if err != nil {

controller/state_test.go

@@ -141,3 +141,40 @@ func TestCompareAppStateExtraHook(t *testing.T) {
 	assert.Equal(t, 1, len(compRes.managedResources))
 	assert.Equal(t, 0, len(compRes.conditions))
 }
+
+func toJSON(t *testing.T, obj *unstructured.Unstructured) string {
+	data, err := json.Marshal(obj)
+	assert.NoError(t, err)
+	return string(data)
+}
+
+func TestCompareAppStateDuplicatedNamespacedResources(t *testing.T) {
+	obj1 := test.NewPod()
+	obj1.SetNamespace(test.FakeDestNamespace)
+	obj2 := test.NewPod()
+	obj3 := test.NewPod()
+	obj3.SetNamespace("kube-system")
+
+	app := newFakeApp()
+	data := fakeData{
+		manifestResponse: &repository.ManifestResponse{
+			Manifests: []string{toJSON(t, obj1), toJSON(t, obj2), toJSON(t, obj3)},
+			Namespace: test.FakeDestNamespace,
+			Server:    test.FakeClusterURL,
+			Revision:  "abc123",
+		},
+		managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{
+			kube.GetResourceKey(obj1): obj1,
+			kube.GetResourceKey(obj3): obj3,
+		},
+	}
+	ctrl := newFakeController(&data)
+	compRes, err := ctrl.appStateManager.CompareAppState(app, "", app.Spec.Source, false)
+	assert.NoError(t, err)
+	assert.NotNil(t, compRes)
+	assert.Contains(t, compRes.conditions, argoappv1.ApplicationCondition{
+		Message: "Resource /Pod/fake-dest-ns/my-pod appeared 2 times among application resources.",
+		Type:    argoappv1.ApplicationConditionRepeatedResourceWarning,
+	})
+	assert.Equal(t, 2, len(compRes.resources))
+}

pkg/apis/application/v1alpha1/types.go

@@ -469,6 +469,8 @@ const (
 	ApplicationConditionUnknownError = "UnknownError"
 	// ApplicationConditionSharedResourceWarning indicates that controller detected resources which belongs to more than one application
 	ApplicationConditionSharedResourceWarning = "SharedResourceWarning"
+	// ApplicationConditionRepeatedResourceWarning indicates that application source has resource with same Group, Kind, Name, Namespace multiple times
+	ApplicationConditionRepeatedResourceWarning = "RepeatedResourceWarning"
 )
 
 // ApplicationCondition contains details about current application condition

test/e2e/functional/duplicated-resources/duplicates-cluster.yaml

@@ -0,0 +1,33 @@
+---
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: clusterdummies.argoproj.io
+spec:
+  group: argoproj.io
+  version: v1alpha1
+  scope: Cluster
+  names:
+    kind: ClusterDummy
+    plural: clusterdummies
+
+
+---
+apiVersion: argoproj.io/v1alpha1
+kind: ClusterDummy
+metadata:
+  name: cluster-dummy-crd-instance
+
+---
+apiVersion: argoproj.io/v1alpha1
+kind: ClusterDummy
+metadata:
+  name: cluster-dummy-crd-instance
+  namespace: default
+
+---
+apiVersion: argoproj.io/v1alpha1
+kind: ClusterDummy
+metadata:
+  name: cluster-dummy-crd-instance
+  namespace: kube-system

test/e2e/functional/duplicated-resources/duplicates-namespaced.yaml

@@ -0,0 +1,32 @@
+---
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: dummies.argoproj.io
+spec:
+  group: argoproj.io
+  version: v1alpha1
+  scope: Namespaced
+  names:
+    kind: Dummy
+    plural: dummies
+
+---
+apiVersion: argoproj.io/v1alpha1
+kind: Dummy
+metadata:
+  name: dummy-crd-instance
+  namespace: default
+
+---
+apiVersion: argoproj.io/v1alpha1
+kind: Dummy
+metadata:
+  name: dummy-crd-instance
+  namespace: kube-system
+
+---
+apiVersion: argoproj.io/v1alpha1
+kind: Dummy
+metadata:
+  name: dummy-crd-instance