Merge pull request #3312 from monopole/conflicting

Extract conflict detection to its own interface.

api/builtins/PatchStrategicMergeTransformer.go

@@ -31,6 +31,11 @@ func (p *PatchStrategicMergeTransformerPlugin) Config(
 	}
 	if len(p.Paths) != 0 {
 		for _, onePath := range p.Paths {
+			// The following oddly attempts to interpret a path string as an
+			// actual patch (instead of as a path to a file containing a patch).
+			// All tests pass if this code is commented out.  This code should
+			// be deleted; the user should use the Patches field which
+			// exists for this purpose (inline patch declaration).
 			res, err := p.h.ResmapFactory().RF().SliceFromBytes([]byte(onePath))
 			if err == nil {
 				p.loadedPatches = append(p.loadedPatches, res...)

api/go.sum

@@ -167,6 +167,7 @@ github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7a
 github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4=
 github.com/gogo/protobuf v1.2.2-0.20190723190241-65acae22fc9d h1:3PaI8p3seN09VjbTYC/QWlUZdZ1qS1zGjy7LH2Wt07I=
 github.com/gogo/protobuf v1.2.2-0.20190723190241-65acae22fc9d/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXPKa29o=
+github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58=
 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
 github.com/golang/groupcache v0.0.0-20160516000752-02826c3e7903/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
 github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
@@ -226,6 +227,7 @@ github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+
 github.com/googleapis/gnostic v0.0.0-20170729233727-0c5108395e2d h1:7XGaL1e6bYS1yIonGp9761ExpPPV1ui0SAC59Yube9k=
 github.com/googleapis/gnostic v0.0.0-20170729233727-0c5108395e2d/go.mod h1:sJBsCZ4ayReDTBIg8b9dl28c5xFWyhBTVRp3pOg5EKY=
 github.com/gophercloud/gophercloud v0.1.0/go.mod h1:vxM41WHh5uqHVBMZHzuwNOHh8XEoIEcSTewFxm1c5g8=
+github.com/gorilla/websocket v1.4.0 h1:WDFjx/TMzVgy9VdMMQi2K2Emtwi2QcUQsztZ/zLaH/Q=
 github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
 github.com/gostaticanalysis/analysisutil v0.0.0-20190318220348-4088753ea4d3 h1:JVnpOZS+qxli+rgVl98ILOXVNbW+kb5wcxeGx8ShUIw=
 github.com/gostaticanalysis/analysisutil v0.0.0-20190318220348-4088753ea4d3/go.mod h1:eEOZF4jCKGi+aprrirO9e7WKB3beBRtWgqGunKl6pKE=
@@ -367,6 +369,7 @@ github.com/shurcooL/go v0.0.0-20180423040247-9e1955d9fb6e h1:MZM7FHLqUHYI0Y/mQAt
 github.com/shurcooL/go v0.0.0-20180423040247-9e1955d9fb6e/go.mod h1:TDJrrUr11Vxrven61rcy3hJMUqaf/CLWYhHNPmT14Lk=
 github.com/shurcooL/go-goon v0.0.0-20170922171312-37c2f522c041 h1:llrF3Fs4018ePo4+G/HV/uQUqEI1HMDjCeOf2V6puPc=
 github.com/shurcooL/go-goon v0.0.0-20170922171312-37c2f522c041/go.mod h1:N5mDOmsrJOB+vfqUK+7DmDyjhSLIIBnXo9lvZJj3MWQ=
+github.com/shurcooL/sanitized_anchor_name v1.0.0 h1:PdmoCO6wvbs+7yrJyMORt4/BmY5IYyJwS/kOiWx8mHo=
 github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc=
 github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo=
 github.com/sirupsen/logrus v1.4.2 h1:SPIRibHv4MatM3XXNO2BJeFLZwZ2LvZgfQ5+UNI2im4=

api/internal/conflict/factory.go

@@ -0,0 +1,23 @@
+// Copyright 2020 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package conflict
+
+import (
+	"sigs.k8s.io/kustomize/api/resid"
+	"sigs.k8s.io/kustomize/api/resource"
+)
+
+type cdFactory struct{}
+
+var _ resource.ConflictDetectorFactory = &cdFactory{}
+
+// NewFactory returns a new conflict detector factory.
+func NewFactory() resource.ConflictDetectorFactory {
+	return &cdFactory{}
+}
+
+// New returns an instance of smPatchMergeOnlyDetector.
+func (c cdFactory) New(_ resid.Gvk) (resource.ConflictDetector, error) {
+	return &smPatchMergeOnlyDetector{}, nil
+}

api/internal/conflict/smpatchmergeonlydetector.go

@@ -0,0 +1,28 @@
+// Copyright 2020 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package conflict
+
+import (
+	"sigs.k8s.io/kustomize/api/resource"
+)
+
+// smPatchMergeOnlyDetector ignores conflicts,
+// but does real strategic merge patching.
+// This is part of an effort to eliminate dependence on
+// apimachinery package to allow kustomize integration
+// into kubectl (#2506 and #1500)
+type smPatchMergeOnlyDetector struct{}
+
+var _ resource.ConflictDetector = &smPatchMergeOnlyDetector{}
+
+func (c *smPatchMergeOnlyDetector) HasConflict(
+	_, _ *resource.Resource) (bool, error) {
+	return false, nil
+}
+
+func (c *smPatchMergeOnlyDetector) MergePatches(
+	r, patch *resource.Resource) (*resource.Resource, error) {
+	err := r.ApplySmPatch(patch)
+	return r, err
+}

api/internal/k8sdeps/conflict/conflictdetectorjson.go

@@ -0,0 +1,43 @@
+// Copyright 2019 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package conflict
+
+import (
+	"encoding/json"
+
+	jsonpatch "github.com/evanphx/json-patch"
+	"k8s.io/apimachinery/pkg/util/mergepatch"
+	"sigs.k8s.io/kustomize/api/resource"
+)
+
+// conflictDetectorJson detects conflicts in a list of JSON patches.
+type conflictDetectorJson struct {
+	resourceFactory *resource.Factory
+}
+
+var _ resource.ConflictDetector = &conflictDetectorJson{}
+
+func (cd *conflictDetectorJson) HasConflict(
+	p1, p2 *resource.Resource) (bool, error) {
+	return mergepatch.HasConflicts(p1.Map(), p2.Map())
+}
+
+func (cd *conflictDetectorJson) MergePatches(
+	patch1, patch2 *resource.Resource) (*resource.Resource, error) {
+	baseBytes, err := json.Marshal(patch1.Map())
+	if err != nil {
+		return nil, err
+	}
+	patchBytes, err := json.Marshal(patch2.Map())
+	if err != nil {
+		return nil, err
+	}
+	mergedBytes, err := jsonpatch.MergeMergePatches(baseBytes, patchBytes)
+	if err != nil {
+		return nil, err
+	}
+	mergedMap := make(map[string]interface{})
+	err = json.Unmarshal(mergedBytes, &mergedMap)
+	return cd.resourceFactory.FromMap(mergedMap), err
+}

api/internal/k8sdeps/conflict/conflictdetectorsm.go

@@ -0,0 +1,65 @@
+// Copyright 2019 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package conflict
+
+import (
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/util/strategicpatch"
+	"sigs.k8s.io/kustomize/api/resource"
+)
+
+// conflictDetectorSm detects conflicts in a list of strategic merge patches.
+type conflictDetectorSm struct {
+	lookupPatchMeta strategicpatch.LookupPatchMeta
+	resourceFactory *resource.Factory
+}
+
+var _ resource.ConflictDetector = &conflictDetectorSm{}
+
+func (cd *conflictDetectorSm) HasConflict(
+	p1, p2 *resource.Resource) (bool, error) {
+	return strategicpatch.MergingMapsHaveConflicts(
+		p1.Map(), p2.Map(), cd.lookupPatchMeta)
+}
+
+func (cd *conflictDetectorSm) MergePatches(
+	patch1, patch2 *resource.Resource) (*resource.Resource, error) {
+	if cd.hasDeleteDirectiveMarker(patch2.Map()) {
+		if cd.hasDeleteDirectiveMarker(patch1.Map()) {
+			return nil, fmt.Errorf(
+				"cannot merge patches both containing '$patch: delete' directives")
+		}
+		patch1, patch2 = patch2, patch1
+	}
+	mergedMap, err := strategicpatch.MergeStrategicMergeMapPatchUsingLookupPatchMeta(
+		cd.lookupPatchMeta, patch1.Map(), patch2.Map())
+	return cd.resourceFactory.FromMap(mergedMap), err
+}
+
+func (cd *conflictDetectorSm) hasDeleteDirectiveMarker(
+	patch map[string]interface{}) bool {
+	if v, ok := patch["$patch"]; ok && v == "delete" {
+		return true
+	}
+	for _, v := range patch {
+		switch typedV := v.(type) {
+		case map[string]interface{}:
+			if cd.hasDeleteDirectiveMarker(typedV) {
+				return true
+			}
+		case []interface{}:
+			for _, sv := range typedV {
+				typedE, ok := sv.(map[string]interface{})
+				if !ok {
+					break
+				}
+				if cd.hasDeleteDirectiveMarker(typedE) {
+					return true
+				}
+			}
+		}
+	}
+	return false
+}

api/internal/k8sdeps/conflict/factory.go

@@ -0,0 +1,45 @@
+// Copyright 2020 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package conflict
+
+import (
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	sp "k8s.io/apimachinery/pkg/util/strategicpatch"
+	"k8s.io/client-go/kubernetes/scheme"
+	"sigs.k8s.io/kustomize/api/resid"
+	"sigs.k8s.io/kustomize/api/resource"
+)
+
+type cdFactory struct {
+	rf *resource.Factory
+}
+
+var _ resource.ConflictDetectorFactory = &cdFactory{}
+
+// NewFactory returns a conflict detector factory.
+// The detector uses a resource factory to convert resources to/from
+// json/yaml/maps representations.
+func NewFactory(rf *resource.Factory) resource.ConflictDetectorFactory {
+	return &cdFactory{rf: rf}
+}
+
+// New returns a conflict detector that's aware of the GVK type.
+func (f *cdFactory) New(gvk resid.Gvk) (resource.ConflictDetector, error) {
+	// Convert to apimachinery representation of object
+	obj, err := scheme.Scheme.New(schema.GroupVersionKind{
+		Group:   gvk.Group,
+		Version: gvk.Version,
+		Kind:    gvk.Kind,
+	})
+	if err == nil {
+		meta, err := sp.NewPatchMetaFromStruct(obj)
+		return &conflictDetectorSm{
+			lookupPatchMeta: meta, resourceFactory: f.rf}, err
+	}
+	if runtime.IsNotRegisteredError(err) {
+		return &conflictDetectorJson{resourceFactory: f.rf}, nil
+	}
+	return nil, err
+}

api/internal/k8sdeps/merge/merginator.go

@@ -1,239 +0,0 @@
-// Copyright 2019 The Kubernetes Authors.
-// SPDX-License-Identifier: Apache-2.0
-
-package merge
-
-import (
-	"encoding/json"
-	"fmt"
-
-	jsonpatch "github.com/evanphx/json-patch"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/util/mergepatch"
-	"k8s.io/apimachinery/pkg/util/strategicpatch"
-	"k8s.io/client-go/kubernetes/scheme"
-	"sigs.k8s.io/kustomize/api/resid"
-	"sigs.k8s.io/kustomize/api/resmap"
-	"sigs.k8s.io/kustomize/api/resource"
-)
-
-type conflictDetector interface {
-	hasConflict(patch1, patch2 *resource.Resource) (bool, error)
-	findConflict(
-		conflictingPatchIdx int,
-		patches []*resource.Resource) (*resource.Resource, error)
-	mergePatches(patch1, patch2 *resource.Resource) (*resource.Resource, error)
-}
-
-type jsonMergePatch struct {
-	resourceFactory *resource.Factory
-}
-
-var _ conflictDetector = &jsonMergePatch{}
-
-func newJMPConflictDetector(rf *resource.Factory) conflictDetector {
-	return &jsonMergePatch{resourceFactory: rf}
-}
-
-func (jmp *jsonMergePatch) hasConflict(
-	patch1, patch2 *resource.Resource) (bool, error) {
-	return mergepatch.HasConflicts(patch1.Map(), patch2.Map())
-}
-
-func (jmp *jsonMergePatch) findConflict(
-	conflictingPatchIdx int,
-	patches []*resource.Resource) (*resource.Resource, error) {
-	for i, patch := range patches {
-		if i == conflictingPatchIdx {
-			continue
-		}
-		if !patches[conflictingPatchIdx].OrgId().Equals(patch.OrgId()) {
-			continue
-		}
-		conflict, err := mergepatch.HasConflicts(
-			patch.Map(),
-			patches[conflictingPatchIdx].Map())
-		if err != nil {
-			return nil, err
-		}
-		if conflict {
-			return patch, nil
-		}
-	}
-	return nil, nil
-}
-
-func (jmp *jsonMergePatch) mergePatches(
-	patch1, patch2 *resource.Resource) (*resource.Resource, error) {
-	baseBytes, err := json.Marshal(patch1.Map())
-	if err != nil {
-		return nil, err
-	}
-	patchBytes, err := json.Marshal(patch2.Map())
-	if err != nil {
-		return nil, err
-	}
-	mergedBytes, err := jsonpatch.MergeMergePatches(baseBytes, patchBytes)
-	if err != nil {
-		return nil, err
-	}
-	mergedMap := make(map[string]interface{})
-	err = json.Unmarshal(mergedBytes, &mergedMap)
-	return jmp.resourceFactory.FromMap(mergedMap), err
-}
-
-type strategicMergePatch struct {
-	lookupPatchMeta strategicpatch.LookupPatchMeta
-	rf              *resource.Factory
-}
-
-var _ conflictDetector = &strategicMergePatch{}
-
-func newSMPConflictDetector(
-	versionedObj runtime.Object,
-	rf *resource.Factory) (conflictDetector, error) {
-	lookupPatchMeta, err := strategicpatch.NewPatchMetaFromStruct(versionedObj)
-	return &strategicMergePatch{lookupPatchMeta: lookupPatchMeta, rf: rf}, err
-}
-
-func (smp *strategicMergePatch) hasConflict(
-	p1, p2 *resource.Resource) (bool, error) {
-	return strategicpatch.MergingMapsHaveConflicts(
-		p1.Map(), p2.Map(), smp.lookupPatchMeta)
-}
-
-func (smp *strategicMergePatch) findConflict(
-	conflictingPatchIdx int,
-	patches []*resource.Resource) (*resource.Resource, error) {
-	for i, patch := range patches {
-		if i == conflictingPatchIdx {
-			continue
-		}
-		if !patches[conflictingPatchIdx].OrgId().Equals(patch.OrgId()) {
-			continue
-		}
-		conflict, err := strategicpatch.MergingMapsHaveConflicts(
-			patch.Map(),
-			patches[conflictingPatchIdx].Map(),
-			smp.lookupPatchMeta)
-		if err != nil {
-			return nil, err
-		}
-		if conflict {
-			return patch, nil
-		}
-	}
-	return nil, nil
-}
-
-func (smp *strategicMergePatch) mergePatches(
-	patch1, patch2 *resource.Resource) (*resource.Resource, error) {
-	if hasDeleteDirectiveMarker(patch2.Map()) {
-		if hasDeleteDirectiveMarker(patch1.Map()) {
-			return nil, fmt.Errorf(
-				"cannot merge patches both containing '$patch: delete' directives")
-		}
-		patch1, patch2 = patch2, patch1
-	}
-	mergeJSONMap, err := strategicpatch.MergeStrategicMergeMapPatchUsingLookupPatchMeta(
-		smp.lookupPatchMeta, patch1.Map(), patch2.Map())
-	return smp.rf.FromMap(mergeJSONMap), err
-}
-
-type merginatorImpl struct {
-	rf *resource.Factory
-}
-
-// NewMerginator returns a new implementation of resmap.Merginator.
-func NewMerginator(rf *resource.Factory) resmap.Merginator {
-	return &merginatorImpl{rf: rf}
-}
-
-var _ resmap.Merginator = (*merginatorImpl)(nil)
-
-// Merge merges the incoming resources into a new resmap.ResMap.
-// Returns an error on conflict.
-func (m *merginatorImpl) Merge(
-	patches []*resource.Resource) (resmap.ResMap, error) {
-	rc := resmap.New()
-	for ix, patch := range patches {
-		id := patch.OrgId()
-		existing := rc.GetMatchingResourcesByOriginalId(id.Equals)
-		if len(existing) == 0 {
-			rc.Append(patch)
-			continue
-		}
-		if len(existing) > 1 {
-			return nil, fmt.Errorf("self conflict in patches")
-		}
-
-		versionedObj, err := scheme.Scheme.New(toSchemaGvk(id.Gvk))
-		if err != nil && !runtime.IsNotRegisteredError(err) {
-			return nil, err
-		}
-		var cd conflictDetector
-		if err != nil {
-			cd = newJMPConflictDetector(m.rf)
-		} else {
-			cd, err = newSMPConflictDetector(versionedObj, m.rf)
-			if err != nil {
-				return nil, err
-			}
-		}
-
-		conflict, err := cd.hasConflict(existing[0], patch)
-		if err != nil {
-			return nil, err
-		}
-		if conflict {
-			conflictingPatch, err := cd.findConflict(ix, patches)
-			if err != nil {
-				return nil, err
-			}
-			return nil, fmt.Errorf(
-				"conflict between %#v and %#v",
-				conflictingPatch.Map(), patch.Map())
-		}
-		merged, err := cd.mergePatches(existing[0], patch)
-		if err != nil {
-			return nil, err
-		}
-		rc.Replace(merged)
-	}
-	return rc, nil
-}
-
-// toSchemaGvk converts to a schema.GroupVersionKind.
-func toSchemaGvk(x resid.Gvk) schema.GroupVersionKind {
-	return schema.GroupVersionKind{
-		Group:   x.Group,
-		Version: x.Version,
-		Kind:    x.Kind,
-	}
-}
-
-func hasDeleteDirectiveMarker(patch map[string]interface{}) bool {
-	if v, ok := patch["$patch"]; ok && v == "delete" {
-		return true
-	}
-	for _, v := range patch {
-		switch typedV := v.(type) {
-		case map[string]interface{}:
-			if hasDeleteDirectiveMarker(typedV) {
-				return true
-			}
-		case []interface{}:
-			for _, sv := range typedV {
-				typedE, ok := sv.(map[string]interface{})
-				if !ok {
-					break
-				}
-				if hasDeleteDirectiveMarker(typedE) {
-					return true
-				}
-			}
-		}
-	}
-	return false
-}

api/internal/merge/merginator.go

@@ -1,31 +0,0 @@
-// Copyright 2020 The Kubernetes Authors.
-// SPDX-License-Identifier: Apache-2.0
-
-package merge
-
-import (
-	"sigs.k8s.io/kustomize/api/resmap"
-	"sigs.k8s.io/kustomize/api/resource"
-)
-
-// Merginator implements resmap.Merginator using kyaml libs.
-type Merginator struct {
-}
-
-var _ resmap.Merginator = (*Merginator)(nil)
-
-func NewMerginator(_ *resource.Factory) *Merginator {
-	return &Merginator{}
-}
-
-// Merge implements resmap.Merginator
-// TODO: Detect conflicts, and return an error.
-// https://github.com/kubernetes-sigs/kustomize/issues/3303
-func (m Merginator) Merge(
-	resources []*resource.Resource) (resmap.ResMap, error) {
-	rm := resmap.New()
-	for i := range resources {
-		rm.Append(resources[i])
-	}
-	return rm, nil
-}

api/internal/merge/merginator_test.go

@@ -1,4 +0,0 @@
-// Copyright 2020 The Kubernetes Authors.
-// SPDX-License-Identifier: Apache-2.0
-
-package merge_test

api/internal/plugins/execplugin/execplugin_test.go

@@ -32,7 +32,8 @@ s/$BAR/bar baz/g
 		t.Fatal(err)
 	}
 	pvd := provider.NewDefaultDepProvider()
-	rf := resmap.NewFactory(pvd.GetResourceFactory(), pvd.GetMerginator())
+	rf := resmap.NewFactory(
+		pvd.GetResourceFactory(), pvd.GetConflictDetectorFactory())
 	pluginConfig := rf.RF().FromMap(
 		map[string]interface{}{
 			"apiVersion": "someteam.example.com/v1",

api/internal/plugins/loader/loader_test.go

@@ -51,7 +51,8 @@ func TestLoader(t *testing.T) {
 		BuildGoPlugin("someteam.example.com", "v1", "SomeServiceGenerator")
 	defer th.Reset()
 	p := provider.NewDefaultDepProvider()
-	rmF := resmap.NewFactory(p.GetResourceFactory(), p.GetMerginator())
+	rmF := resmap.NewFactory(
+		p.GetResourceFactory(), p.GetConflictDetectorFactory())
 	fLdr, err := loader.NewLoader(
 		loader.RestrictionRootOnly,
 		filesys.Separator, filesys.MakeFsInMemory())

api/internal/target/maker_test.go

@@ -36,7 +36,8 @@ func makeKustTargetWithRf(
 	if err != nil {
 		t.Fatal(err)
 	}
-	rf := resmap.NewFactory(pvd.GetResourceFactory(), pvd.GetMerginator())
+	rf := resmap.NewFactory(
+		pvd.GetResourceFactory(), pvd.GetConflictDetectorFactory())
 	pc := konfig.DisabledPluginConfig()
 	return target.NewKustTarget(
 		ldr,

api/krusty/kustomizer.go

@@ -53,7 +53,7 @@ func MakeKustomizer(fSys filesys.FileSystem, o *Options) *Kustomizer {
 func (b *Kustomizer) Run(path string) (resmap.ResMap, error) {
 	resmapFactory := resmap.NewFactory(
 		b.depProvider.GetResourceFactory(),
-		b.depProvider.GetMerginator())
+		b.depProvider.GetConflictDetectorFactory())
 	lr := fLdr.RestrictionNone
 	if b.options.LoadRestrictions == types.LoadRestrictionsRootOnly {
 		lr = fLdr.RestrictionRootOnly

api/provider/depprovider.go

@@ -5,14 +5,13 @@ package provider
 
 import (
 	"sigs.k8s.io/kustomize/api/ifc"
-	"sigs.k8s.io/kustomize/api/internal/k8sdeps/merge"
+	"sigs.k8s.io/kustomize/api/internal/conflict"
-	kmerge "sigs.k8s.io/kustomize/api/internal/merge"
+	k8sconflict "sigs.k8s.io/kustomize/api/internal/k8sdeps/conflict"
 	"sigs.k8s.io/kustomize/api/internal/validate"
 	"sigs.k8s.io/kustomize/api/internal/wrappy"
 	"sigs.k8s.io/kustomize/api/k8sdeps/kunstruct"
 	"sigs.k8s.io/kustomize/api/k8sdeps/validator"
 	"sigs.k8s.io/kustomize/api/konfig"
-	"sigs.k8s.io/kustomize/api/resmap"
 	"sigs.k8s.io/kustomize/api/resource"
 )
 
@@ -96,17 +95,27 @@ import (
 //            would really reduce the work
 //            (e.g. drop Vars, drop ReplacementTranformer).
 //
-//   - resmap.Merginator
+//   - resource.ConflictDetector
 //
-//       1) api/internal/k8sdeps/merge.Merginator
+//       1) api/internal/k8sdeps/conflict.conflictDetectorJson
+//          api/internal/k8sdeps/conflict.conflictDetectorSm
 //
 //            Uses k8s.io/apimachinery/pkg/util/strategicpatch,
 //            apimachinery/pkg/util/mergepatch, etc. to merge
 //            resource.Resource instances.
 //
-//       2) api/internal/merge.Merginator
+//       2) api/internal/conflict.smPatchMergeOnlyDetector
 //
-//            At time of writing, this is unimplemented.
+//           At time of writing, this doesn't report conflicts,
+//           but it does know how to merge patches. Conflict
+//           reporting isn't vital to kustomize function.  It's
+//           rare that a person would configure one transformer
+//           with many patches, much less so many that it became
+//           hard to spot conflicts.  In the case of an undetected
+//           conflict, the last patch applied wins, likely what
+//           the user wants anyway.  Regardless, the effect of this
+//           is plainly visible and usable in the output, even if
+//           a conflict happened but wasn't reported as an error.
 //
 //   - ifc.Validator
 //
@@ -117,6 +126,7 @@ import (
 //
 //       2) api/internal/validate.FieldValidator
 //
+//            See TODO inside the validator for status.
 //            At time of writing, this is a do-nothing
 //            validator as it's not critical to kustomize function.
 //
@@ -139,20 +149,20 @@ import (
 // If you're reading this, plan not done.
 //
 type DepProvider struct {
-	kFactory        ifc.KunstructuredFactory
+	kFactory                 ifc.KunstructuredFactory
-	resourceFactory *resource.Factory
+	resourceFactory          *resource.Factory
-	merginator      resmap.Merginator
+	conflictDectectorFactory resource.ConflictDetectorFactory
-	fieldValidator  ifc.Validator
+	fieldValidator           ifc.Validator
 }
 
 func makeK8sdepBasedInstances() *DepProvider {
 	kf := kunstruct.NewKunstructuredFactoryImpl()
 	rf := resource.NewFactory(kf)
 	return &DepProvider{
-		kFactory:        kf,
+		kFactory:                 kf,
-		resourceFactory: rf,
+		resourceFactory:          rf,
-		merginator:      merge.NewMerginator(rf),
+		conflictDectectorFactory: k8sconflict.NewFactory(rf),
-		fieldValidator:  validator.NewKustValidator(),
+		fieldValidator:           validator.NewKustValidator(),
 	}
 }
 
@@ -160,10 +170,10 @@ func makeKyamlBasedInstances() *DepProvider {
 	kf := &wrappy.WNodeFactory{}
 	rf := resource.NewFactory(kf)
 	return &DepProvider{
-		kFactory:        kf,
+		kFactory:                 kf,
-		resourceFactory: rf,
+		resourceFactory:          rf,
-		merginator:      kmerge.NewMerginator(rf),
+		conflictDectectorFactory: conflict.NewFactory(),
-		fieldValidator:  validate.NewFieldValidator(),
+		fieldValidator:           validate.NewFieldValidator(),
 	}
 }
 
@@ -186,8 +196,8 @@ func (dp *DepProvider) GetResourceFactory() *resource.Factory {
 	return dp.resourceFactory
 }
 
-func (dp *DepProvider) GetMerginator() resmap.Merginator {
+func (dp *DepProvider) GetConflictDetectorFactory() resource.ConflictDetectorFactory {
-	return dp.merginator
+	return dp.conflictDectectorFactory
 }
 
 func (dp *DepProvider) GetFieldValidator() ifc.Validator {

api/resmap/factory.go

@@ -12,24 +12,18 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
-// Merginator merges resources.
-type Merginator interface {
-	// Merge creates a new ResMap by merging incoming resources.
-	// Error if conflict found.
-	Merge([]*resource.Resource) (ResMap, error)
-}
-
 // Factory makes instances of ResMap.
 type Factory struct {
 	// Makes resources.
 	resF *resource.Factory
-	// Makes ResMaps via merging.
+	// Makes ConflictDetectors.
-	pm Merginator
+	cdf resource.ConflictDetectorFactory
 }
 
 // NewFactory returns a new resmap.Factory.
-func NewFactory(rf *resource.Factory, pm Merginator) *Factory {
+func NewFactory(
-	return &Factory{resF: rf, pm: pm}
+	rf *resource.Factory, cdf resource.ConflictDetectorFactory) *Factory {
+	return &Factory{resF: rf, cdf: cdf}
 }
 
 // RF returns a resource.Factory.
@@ -134,8 +128,8 @@ func (rmF *Factory) FromSecretArgs(
 
 // Merge creates a new ResMap by merging incoming resources.
 // Error if conflict found.
-func (rmF *Factory) Merge(patches []*resource.Resource) (ResMap, error) {
+func (rmF *Factory) Merge(incoming []*resource.Resource) (ResMap, error) {
-	return rmF.pm.Merge(patches)
+	return (&merginator{cdf: rmF.cdf}).Merge(incoming)
 }
 
 func newResMapFromResourceSlice(

api/resmap/merginator.go

@@ -0,0 +1,118 @@
+// Copyright 2020 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package resmap
+
+import (
+	"fmt"
+
+	"sigs.k8s.io/kustomize/api/resource"
+)
+
+// merginator coordinates merging the resources in incoming to the result.
+type merginator struct {
+	incoming []*resource.Resource
+	cdf      resource.ConflictDetectorFactory
+	result   ResMap
+}
+
+func (m *merginator) Merge(in []*resource.Resource) (ResMap, error) {
+	m.result = New()
+	m.incoming = in
+	for index := range m.incoming {
+		alreadyInResult, err := m.appendIfNoMatch(index)
+		if err != nil {
+			return nil, err
+		}
+		if alreadyInResult != nil {
+			// The resource at index has the same resId as a previously
+			// considered resource.
+			//
+			// If they conflict with each other (e.g. they both want to change
+			// the image name in a Deployment, but to different values),
+			// return an error.
+			//
+			// If they don't conflict, then merge them into a single resource,
+			// since they both target the same item, and we want cumulative
+			// behavior. E.g. say both patches modify a map.  Without a merge,
+			// the last patch wins, replacing the entire map.
+			err = m.mergeWithExisting(index, alreadyInResult)
+			if err != nil {
+				return nil, err
+			}
+		}
+	}
+	return m.result, nil
+}
+
+func (m *merginator) appendIfNoMatch(index int) (*resource.Resource, error) {
+	candidate := m.incoming[index]
+	matchedResources := m.result.GetMatchingResourcesByOriginalId(
+		candidate.OrgId().Equals)
+	if len(matchedResources) == 0 {
+		m.result.Append(candidate)
+		return nil, nil
+	}
+	if len(matchedResources) > 1 {
+		return nil, fmt.Errorf("multiple resources targeted by patch")
+	}
+	return matchedResources[0], nil
+}
+
+func (m *merginator) mergeWithExisting(
+	index int, alreadyInResult *resource.Resource) error {
+	candidate := m.incoming[index]
+	cd, err := m.cdf.New(candidate.OrgId().Gvk)
+	if err != nil {
+		return err
+	}
+	hasConflict, err := cd.HasConflict(candidate, alreadyInResult)
+	if err != nil {
+		return err
+	}
+	if hasConflict {
+		return m.makeError(cd, index)
+	}
+	merged, err := cd.MergePatches(alreadyInResult, candidate)
+	if err != nil {
+		return err
+	}
+	_, err = m.result.Replace(merged)
+	return err
+}
+
+// Make an error message describing the conflict.
+func (m *merginator) makeError(cd resource.ConflictDetector, index int) error {
+	conflict, err := m.findConflict(cd, index)
+	if err != nil {
+		return err
+	}
+	if conflict == nil {
+		return fmt.Errorf("expected conflict for %s", m.incoming[index].OrgId())
+	}
+	return fmt.Errorf(
+		"conflict between %#v at index %d and %#v",
+		m.incoming[index].Map(), index, conflict.Map())
+}
+
+// findConflict looks for a conflict in a resource slice.
+// It returns the first conflict between the resource at index
+// and some other resource.  Two resources can only conflict if
+// they have the same original ResId.
+func (m *merginator) findConflict(
+	cd resource.ConflictDetector, index int) (*resource.Resource, error) {
+	targetId := m.incoming[index].OrgId()
+	for i, p := range m.incoming {
+		if i == index || !targetId.Equals(p.OrgId()) {
+			continue
+		}
+		conflict, err := cd.HasConflict(p, m.incoming[index])
+		if err != nil {
+			return nil, err
+		}
+		if conflict {
+			return p, nil
+		}
+	}
+	return nil, nil
+}

api/resource/conflictdetector.go

@@ -0,0 +1,20 @@
+// Copyright 2019 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package resource
+
+import "sigs.k8s.io/kustomize/api/resid"
+
+// ConflictDetector detects conflicts between resources.
+type ConflictDetector interface {
+	// HasConflict returns true if the given resources have a conflict.
+	HasConflict(patch1, patch2 *Resource) (bool, error)
+	// Merge two resources into one.
+	MergePatches(patch1, patch2 *Resource) (*Resource, error)
+}
+
+// ConflictDetectorFactory makes instances of ConflictDetector that know
+// how to handle the given Group, Version, Kind tuple.
+type ConflictDetectorFactory interface {
+	New(gvk resid.Gvk) (ConflictDetector, error)
+}

api/testutils/kusttest/harnessenhanced.go

@@ -46,7 +46,8 @@ func MakeEnhancedHarness(t *testing.T) *HarnessEnhanced {
 	}
 	p := provider.NewDefaultDepProvider()
 	resourceFactory := p.GetResourceFactory()
-	resmapFactory := resmap.NewFactory(resourceFactory, p.GetMerginator())
+	resmapFactory := resmap.NewFactory(
+		resourceFactory, p.GetConflictDetectorFactory())
 
 	result := &HarnessEnhanced{
 		Harness: MakeHarness(t),

cmd/pluginator/internal/krmfunction/funcwrappersrc/main.go

@@ -16,7 +16,8 @@ import (
 func main() {
 	var plugin resmap.Configurable
 	p := provider.NewDefaultDepProvider()
-	resmapFactory := resmap.NewFactory(p.GetResourceFactory(), p.GetMerginator())
+	resmapFactory := resmap.NewFactory(
+		p.GetResourceFactory(), p.GetConflictDetectorFactory())
 	pluginHelpers := resmap.NewPluginHelpers(
 		nil, p.GetFieldValidator(), resmapFactory)
 

plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer.go

@@ -6,6 +6,7 @@ package main
 
 import (
 	"fmt"
+
 	"sigs.k8s.io/kustomize/api/resmap"
 	"sigs.k8s.io/kustomize/api/resource"
 	"sigs.k8s.io/kustomize/api/types"
@@ -34,6 +35,11 @@ func (p *plugin) Config(
 	}
 	if len(p.Paths) != 0 {
 		for _, onePath := range p.Paths {
+			// The following oddly attempts to interpret a path string as an
+			// actual patch (instead of as a path to a file containing a patch).
+			// All tests pass if this code is commented out.  This code should
+			// be deleted; the user should use the Patches field which
+			// exists for this purpose (inline patch declaration).
 			res, err := p.h.ResmapFactory().RF().SliceFromBytes([]byte(onePath))
 			if err == nil {
 				p.loadedPatches = append(p.loadedPatches, res...)

plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go

@@ -13,7 +13,6 @@ import (
 	kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
 )
 
-
 // TODO(#3304): eliminate branching on konfig.FlagEnableKyamlDefaultValue
 // Details: https://github.com/kubernetes-sigs/kustomize/issues/3304
 // All tests should pass for either true or false values
@@ -28,6 +27,9 @@ func ifApiMachineryElseKyaml(s1, s2 string) string {
 	return s2
 }
 
+// TODO(#3304)
+const skipConflictDetectionTests = konfig.FlagEnableKyamlDefaultValue
+
 func errorContains(err error, possibilities ...string) bool {
 	for _, x := range possibilities {
 		if strings.Contains(err.Error(), x) {
@@ -240,8 +242,7 @@ patches: |-
         - name: nginx
           image: nginx:latest
 `,
-		target,
+		target, `
-		ifApiMachineryElseKyaml(`
 apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -256,22 +257,7 @@ spec:
       containers:
       - image: nginx:latest
         name: nginx
-`, `
+`)
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: myDeploy
-spec:
-  replica: 3
-  template:
-    metadata:
-      labels:
-        old-label: old-value
-    spec:
-      containers:
-      - image: nginx
-        name: nginx
-`))
 }
 
 func TestPatchStrategicMergeTransformerMultiplePatches(t *testing.T) {
@@ -321,8 +307,7 @@ paths:
 - patch1.yaml
 - patch2.yaml
 `,
-		target,
+		target, `
-		ifApiMachineryElseKyaml(`
 apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -344,28 +329,13 @@ spec:
         name: nginx
       - image: busybox
         name: busybox
-`, `
+`)
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: myDeploy
-spec:
-  replica: 2
-  template:
-    metadata:
-      labels:
-        old-label: old-value
-    spec:
-      containers:
-      - env:
-        - name: SOMEENV
-          value: BAR
-        image: nginx:latest
-        name: nginx
-`))
 }
 
 func TestStrategicMergeTransformerMultiplePatchesWithConflicts(t *testing.T) {
+	if skipConflictDetectionTests {
+		t.Skip("Skipping patch merge conflict tests.")
+	}
 	th := kusttest_test.MakeEnhancedHarness(t).
 		PrepBuiltin("PatchStrategicMergeTransformer")
 	defer th.Reset()
@@ -412,13 +382,8 @@ paths:
 - patch1.yaml
 - patch2.yaml
 `, target)
-	// TODO(#3304)
+	if assert.Error(t, err) && !errorContains(err, "conflict") {
-	if konfig.FlagEnableKyamlDefaultValue {
+		t.Fatalf("expected error to contain %q but get %v", "conflict", err)
-		assert.NoError(t, err)
-	} else {
-		if assert.Error(t, err) && !errorContains(err, "conflict") {
-			t.Fatalf("expected error to contain %q but get %v", "conflict", err)
-		}
 	}
 }
 
@@ -665,11 +630,18 @@ metadata:
 spec:
   bar:
     A: X
+    B: "Y"
     C: Z
+    D: W
+  baz:
+    hello: world
 `))
 }
 
 func TestStrategicMergeTransformerNoSchemaMultiPatchesWithConflict(t *testing.T) {
+	if skipConflictDetectionTests {
+		t.Skip("Skipping patch merge conflict tests.")
+	}
 	th := kusttest_test.MakeEnhancedHarness(t).
 		PrepBuiltin("PatchStrategicMergeTransformer")
 	defer th.Reset()
@@ -702,13 +674,8 @@ paths:
 - patch1.yaml
 - patch2.yaml
 `, targetNoschema)
-	// TODO(#3304)
+	if assert.Error(t, err) && !errorContains(err, "conflict") {
-	if konfig.FlagEnableKyamlDefaultValue {
+		t.Fatalf("expected error to contain %q but get %v", "conflict", err)
-		assert.NoError(t, err)
-	} else {
-		if assert.Error(t, err) && !errorContains(err, "conflict") {
-			t.Fatalf("expected error to contain %q but get %v", "conflict", err)
-		}
 	}
 }
 
@@ -1041,26 +1008,7 @@ func TestMultiplePatches(t *testing.T) {
 				changeImagePatch(Deployment, "nginx:latest"),
 				addContainerAndEnvPatch(Deployment),
 			},
-			expected: ifApiMachineryElseKyaml(
+			expected: expectedResultMultiPatch(Deployment, false),
-				expectedResultMultiPatch(Deployment, false), `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: deploy1
-spec:
-  template:
-    metadata:
-      labels:
-        old-label: old-value
-        some-label: some-value
-    spec:
-      containers:
-      - env:
-        - name: SOMEENV
-          value: SOMEVALUE
-        image: nginx
-        name: nginx
-`),
 		},
 		"withschema-image-container-label": {
 			base: baseResource(Deployment),
@@ -1069,22 +1017,7 @@ spec:
 				addContainerAndEnvPatch(Deployment),
 				addLabelAndEnvPatch(Deployment),
 			},
-			expected: ifApiMachineryElseKyaml(
+			expected: expectedResultMultiPatch(Deployment, true),
-				expectedResultMultiPatch(Deployment, true), `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: deploy1
-spec:
-  template:
-    metadata:
-      labels:
-        old-label: old-value
-    spec:
-      containers:
-      - image: nginx:latest
-        name: nginx
-`),
 		},
 		"withschema-container-label-image": {
 			base: baseResource(Deployment),
@@ -1093,68 +1026,34 @@ spec:
 				addLabelAndEnvPatch(Deployment),
 				changeImagePatch(Deployment, "nginx:latest"),
 			},
-			expected: ifApiMachineryElseKyaml(
+			expected: expectedResultMultiPatch(Deployment, true),
-				expectedResultMultiPatch(Deployment, true), `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: deploy1
-spec:
-  template:
-    metadata:
-      labels:
-        old-label: old-value
-    spec:
-      containers:
-      - env:
-        - name: ANOTHERENV
-          value: ANOTHERVALUE
-        image: nginx
-        name: nginx
-      - image: anotherimage
-        name: anothercontainer
-`),
 		},
-		"noschema-label-image-container": {
+		"noschema-1.7.9-label-latest": {
 			base: baseResource(MyCRD),
 			patch: []string{
-				addLabelAndEnvPatch(MyCRD),
+				changeImagePatch(MyCRD, "nginx:1.7.9"),
-				changeImagePatch(MyCRD, "nginx:latest"),
-				addContainerAndEnvPatch(MyCRD),
-			},
-			// This should work
-			errorExpected: true,
-			errorMsg:      "conflict",
-		},
-		"noschema-image-container-label": {
-			base: baseResource(MyCRD),
-			patch: []string{
-				changeImagePatch(MyCRD, "nginx:latest"),
-				addContainerAndEnvPatch(MyCRD),
-				addLabelAndEnvPatch(MyCRD),
-			},
-			// This should work
-			errorExpected: true,
-			errorMsg:      "conflict",
-		},
-		"noschema-container-label-image": {
-			base: baseResource(MyCRD),
-			patch: []string{
-				addContainerAndEnvPatch(MyCRD),
 				addLabelAndEnvPatch(MyCRD),
 				changeImagePatch(MyCRD, "nginx:latest"),
 			},
-			// This should work
+			errorExpected: false,
-			errorExpected: true,
+			// There is no conflict detected. It should
-			errorMsg:      "conflict",
+			// be but the JMPConflictDector ignores it.
+			// See https://github.com/kubernetes-sigs/kustomize/issues/1370
+			expected: expectedResultJMP("nginx:latest"),
+		},
+		"noschema-latest-label-1.7.9": {
+			base: baseResource(MyCRD),
+			patch: []string{
+				changeImagePatch(MyCRD, "nginx:latest"),
+				addLabelAndEnvPatch(MyCRD),
+				changeImagePatch(MyCRD, "nginx:1.7.9"),
+			},
+			errorExpected: false,
+			// There is no conflict detected. It should
+			// be but the JMPConflictDector ignores it.
+			// See https://github.com/kubernetes-sigs/kustomize/issues/1370
+			expected: expectedResultJMP("nginx:1.7.9"),
 		},
-	}
-
-	// TODO(#3304)
-	if konfig.FlagEnableKyamlDefaultValue {
-		delete(tests, "noschema-label-image-container")
-		delete(tests, "noschema-image-container-label")
-		delete(tests, "noschema-container-label-image")
 	}
 
 	th := kusttest_test.MakeEnhancedHarness(t).
@@ -1182,6 +1081,9 @@ spec:
 // detected regardless of the order of the patches and regardless
 // of the schema availibility (SMP vs JSON)
 func TestMultiplePatchesWithConflict(t *testing.T) {
+	if skipConflictDetectionTests {
+		t.Skip("Skipping patch merge conflict tests.")
+	}
 	tests := map[string]testRecord{
 		"withschema-label-latest-1.7.9": {
 			base: baseResource(Deployment),
@@ -1234,63 +1136,6 @@ func TestMultiplePatchesWithConflict(t *testing.T) {
 			errorExpected: true,
 			errorMsg:      "conflict",
 		},
-		"noschema-latest-label-1.7.9": {
-			base: baseResource(MyCRD),
-			patch: []string{
-				changeImagePatch(MyCRD, "nginx:latest"),
-				addLabelAndEnvPatch(MyCRD),
-				changeImagePatch(MyCRD, "nginx:1.7.9"),
-			},
-			errorExpected: false,
-			// There is no conflict detected. It should
-			// be but the JMPConflictDector ignores it.
-			// See https://github.com/kubernetes-sigs/kustomize/issues/1370
-			expected: ifApiMachineryElseKyaml(
-				expectedResultJMP("nginx:1.7.9"), `
-apiVersion: apps/v1
-kind: MyCRD
-metadata:
-  name: deploy1
-spec:
-  template:
-    metadata:
-      labels:
-        old-label: old-value
-    spec:
-      containers:
-      - image: nginx:latest
-        name: nginx
-`),
-		},
-		"noschema-1.7.9-label-latest": {
-			base: baseResource(MyCRD),
-			patch: []string{
-				changeImagePatch(MyCRD, "nginx:1.7.9"),
-				addLabelAndEnvPatch(MyCRD),
-				changeImagePatch(MyCRD, "nginx:latest"),
-			},
-			errorExpected: false,
-			// There is no conflict detected. It should
-			// be but the JMPConflictDector ignores it.
-			// See https://github.com/kubernetes-sigs/kustomize/issues/1370
-
-			expected: ifApiMachineryElseKyaml(
-				expectedResultJMP("nginx:latest"), `
-apiVersion: apps/v1
-kind: MyCRD
-metadata:
-  name: deploy1
-spec:
-  template:
-    metadata:
-      labels:
-        old-label: old-value
-    spec:
-      containers:
-      - image: nginx:1.7.9
-        name: nginx
-`),
-		},
 		"noschema-1.7.9-latest-label": {
 			base: baseResource(MyCRD),
 			patch: []string{
@@ -1301,15 +1146,36 @@ spec:
 			},
 			errorExpected: true,
 		},
-	}
+		"noschema-label-image-container": {
-	// TODO(#3304)
+			base: baseResource(MyCRD),
-	if konfig.FlagEnableKyamlDefaultValue {
+			patch: []string{
-		delete(tests, "withschema-label-latest-1.7.9")
+				addLabelAndEnvPatch(MyCRD),
-		delete(tests, "withschema-1.7.9-latest-label")
+				changeImagePatch(MyCRD, "nginx:latest"),
-		delete(tests, "withschema-1.7.9-label-latest")
+				addContainerAndEnvPatch(MyCRD),
-		delete(tests, "withschema-latest-label-1.7.9-difforder")
+			},
-		delete(tests, "noschema-1.7.9-latest-label")
+			errorExpected: true,
-		delete(tests, "noschema-label-latest-1.7.9")
+			errorMsg:      "conflict",
+		},
+		"noschema-image-container-label": {
+			base: baseResource(MyCRD),
+			patch: []string{
+				changeImagePatch(MyCRD, "nginx:latest"),
+				addContainerAndEnvPatch(MyCRD),
+				addLabelAndEnvPatch(MyCRD),
+			},
+			errorExpected: true,
+			errorMsg:      "conflict",
+		},
+		"noschema-container-label-image": {
+			base: baseResource(MyCRD),
+			patch: []string{
+				addContainerAndEnvPatch(MyCRD),
+				addLabelAndEnvPatch(MyCRD),
+				changeImagePatch(MyCRD, "nginx:latest"),
+			},
+			errorExpected: true,
+			errorMsg:      "conflict",
+		},
 	}
 	th := kusttest_test.MakeEnhancedHarness(t).
 		PrepBuiltin("PatchStrategicMergeTransformer")