Allow diamond structure of kustomize import

pkg/accumulator/resaccumulator.go

@@ -6,10 +6,12 @@ package accumulator
 import (
 	"fmt"
 	"log"
+	"reflect"
 	"strings"
 
 	"sigs.k8s.io/kustomize/v3/pkg/resid"
 	"sigs.k8s.io/kustomize/v3/pkg/resmap"
+	"sigs.k8s.io/kustomize/v3/pkg/resource"
 	"sigs.k8s.io/kustomize/v3/pkg/transformers"
 	"sigs.k8s.io/kustomize/v3/pkg/transformers/config"
 	"sigs.k8s.io/kustomize/v3/pkg/types"
@@ -42,6 +44,36 @@ func (ra *ResAccumulator) Vars() []types.Var {
 	return ra.varSet.AsSlice()
 }
 
+// RemoveIds removes resources of the internal resMap which are member
+// of the resId slice past in parameter.
+// The removed Resources are returned by the method.
+func (ra *ResAccumulator) RemoveConflicts(rightResources []*resource.Resource) ([]*resource.Resource, error) {
+	conflicting := []*resource.Resource{}
+	for _, rightResource := range rightResources {
+		rightId := rightResource.CurId()
+		leftResources := ra.resMap.GetMatchingResourcesByCurrentId(rightId.Equals)
+
+		if len(leftResources) == 0 {
+			// no conflict
+			continue
+		}
+
+		if len(leftResources) != 1 || !reflect.DeepEqual(leftResources[0], rightResource) {
+			// conflict detected. More than one resource or left and right are different.
+			conflicting = append(conflicting, leftResources...)
+		}
+
+		// Remove the resource from that resMap
+		err := ra.resMap.Remove(rightId)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return conflicting, nil
+}
+
+// AllIds returns all CurrentIds.
+
 func (ra *ResAccumulator) AppendAll(
 	resources resmap.ResMap) error {
 	return ra.resMap.AppendAll(resources)

pkg/resmap/resmap.go

@@ -215,8 +215,16 @@ func (m *resWrangler) Resources() []*resource.Resource {
 func (m *resWrangler) Append(res *resource.Resource) error {
 	id := res.CurId()
 	if r := m.GetMatchingResourcesByCurrentId(id.Equals); len(r) > 0 {
-		return fmt.Errorf(
-			"may not add resource with an already registered id: %s", id)
+		// This allows diamond import of resources.
+		// We can assert the same resource as been loaded because there is
+		// only one resource in the list with the same name and
+		// value of the resource we intend to append is identical to the one
+		// we already added to the list.
+		if len(r) > 1 || !res.Equals(r[0]) {
+			return fmt.Errorf(
+				"may not add resource with an already registered id: %s", id)
+		}
+		return nil
 	}
 	m.rList = append(m.rList, res)
 	return nil

pkg/resmap/resmap_test.go

@@ -45,6 +45,21 @@ func makeCm(i int) *resource.Resource {
 		})
 }
 
+// Make a resource with a predictable name and label.
+func makeCmWithLabel(i int, label string) *resource.Resource {
+	return rf.FromMap(
+		map[string]interface{}{
+			"apiVersion": "v1",
+			"kind":       "ConfigMap",
+			"metadata": map[string]interface{}{
+				"name": fmt.Sprintf("cm%03d", i),
+				"labels": map[string]interface{}{
+					"key1": label,
+				},
+			},
+		})
+}
+
 func TestAppendRemove(t *testing.T) {
 	w1 := New()
 	doAppend(t, w1, makeCm(1))
@@ -69,7 +84,7 @@ func TestAppendRemove(t *testing.T) {
 		t.Fatalf("mismatch")
 	}
 
-	err := w2.Append(makeCm(6))
+	err := w2.Append(makeCmWithLabel(6, "trigger-a-conflict"))
 	if err == nil {
 		t.Fatalf("expected error")
 	}
@@ -98,6 +113,7 @@ func TestRemove(t *testing.T) {
 
 func TestReplace(t *testing.T) {
 	cm5 := makeCm(5)
+	cm5WithLabel := makeCmWithLabel(5, "trigger-a-conflict")
 	cm700 := makeCm(700)
 	otherCm5 := makeCm(5)
 
@@ -121,7 +137,10 @@ func TestReplace(t *testing.T) {
 	if r, err := w.GetByCurrentId(cm5.OrgId()); err != nil || r != otherCm5 {
 		t.Fatalf("unexpected result r=%s, err=%v", r.CurId(), err)
 	}
-	if err := w.Append(cm5); err == nil {
+	if err := w.Append(cm5); err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if err := w.Append(cm5WithLabel); err == nil {
 		t.Fatalf("expected id already there error")
 	}
 	if err := w.Remove(cm5.OrgId()); err != nil {

pkg/target/diamondcomposition_test.go

@@ -116,13 +116,19 @@ patchesStrategicMerge:
 // of it, without using any of the `namePrefix`, `nameSuffix` or `namespace`
 // kustomization keywords.
 //
-//            composite
-//          /     |     \
-//       probe   dns  restart
-//          \     |     /
-//              base
+//               composite
+//          /       |        \
+//     1.probe    2.dns   3.restart
+//          \       |        /
+//                 base
 //
-func TestIssue1251_CompositeDiamond_Failure(t *testing.T) {
+//
+// The generation flow relies on the order of the resources in the resources field
+// of the kustomization.yaml
+// 1. Customize base (no-op), customize probe deployment (patch) and add it to composite deployment
+// 2. Customise base again (no-op), customize dns deployment (patch) and merge it into composite deployment
+// 3. Customize base again (no-op), customize restart deployment (patch) and merge it into composite deployment
+func TestIssue1251_CompositeDiamond_ProbeRestartDns_Order(t *testing.T) {
 	th := kusttest_test.NewKustTestHarness(t, "/app/composite")
 	writeDeploymentBase(th)
 	writeProbeOverlay(th)
@@ -136,14 +142,59 @@ resources:
 - ../restart
 `)
 
-	_, err := th.MakeKustTarget().MakeCustomizedResMap()
-	if err == nil {
-		t.Fatalf("Expected resource accumulation error")
+	m, err := th.MakeKustTarget().MakeCustomizedResMap()
+	if err != nil {
+		t.Fatalf("Err: %v", err)
 	}
-	if !strings.Contains(
-		err.Error(), "already registered id: apps_v1_Deployment|~X|my-deployment") {
-		t.Fatalf("Unexpected err: %v", err)
+	th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: my-deployment
+spec:
+  template:
+    spec:
+      containers:
+      - image: my-image
+        livenessProbe:
+          httpGet:
+            path: /healthz
+            port: 8080
+        name: my-deployment
+      dnsPolicy: None
+      restartPolicy: Always
+`)
+}
+
+//
+//               composite
+//          /       |        \
+//     1.probe   2.restart  3.dns
+//          \       |        /
+//                 base
+//
+// 1. Customize base (no-op), customize probe deployment (patch) and add it to composite deployment
+// 2. Customise base again (no-op), customize dns deployment (patch) and merge it into composite deployment
+// 3. Customize base again (no-op), customize restart deployment (patch) and merge it into composite deployment
+func TestIssue1251_CompositeDiamond_ProbeDnsRestart_Order(t *testing.T) {
+	th := kusttest_test.NewKustTestHarness(t, "/app/composite")
+	writeDeploymentBase(th)
+	writeProbeOverlay(th)
+	writeDNSOverlay(th)
+	writeRestartOverlay(th)
+
+	th.WriteK("/app/composite", `
+resources:
+- ../probe
+- ../restart
+- ../dns
+`)
+
+	m, err := th.MakeKustTarget().MakeCustomizedResMap()
+	if err != nil {
+		t.Fatalf("Err: %v", err)
 	}
+	th.AssertActualEqualsExpected(m, expectedPatchedDeployment)
 }
 
 const expectedPatchedDeployment = `