More merge tests, clearer method names.

api/builtins/PatchStrategicMergeTransformer.go

@@ -13,7 +13,6 @@ import (
 )
 
 type PatchStrategicMergeTransformerPlugin struct {
-	h             *resmap.PluginHelpers
 	loadedPatches []*resource.Resource
 	Paths         []types.PatchStrategicMerge `json:"paths,omitempty" yaml:"paths,omitempty"`
 	Patches       string                      `json:"patches,omitempty" yaml:"patches,omitempty"`
@@ -21,7 +20,6 @@ type PatchStrategicMergeTransformerPlugin struct {
 
 func (p *PatchStrategicMergeTransformerPlugin) Config(
 	h *resmap.PluginHelpers, c []byte) (err error) {
-	p.h = h
 	err = yaml.Unmarshal(c, p)
 	if err != nil {
 		return err
@@ -36,13 +34,13 @@ func (p *PatchStrategicMergeTransformerPlugin) Config(
 			// 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))
+			res, err := h.ResmapFactory().RF().SliceFromBytes([]byte(onePath))
 			if err == nil {
 				p.loadedPatches = append(p.loadedPatches, res...)
 				continue
 			}
-			res, err = p.h.ResmapFactory().RF().SliceFromPatches(
+			res, err = h.ResmapFactory().RF().SliceFromPatches(
-				p.h.Loader(), []types.PatchStrategicMerge{onePath})
+				h.Loader(), []types.PatchStrategicMerge{onePath})
 			if err != nil {
 				return err
 			}
@@ -50,7 +48,7 @@ func (p *PatchStrategicMergeTransformerPlugin) Config(
 		}
 	}
 	if p.Patches != "" {
-		res, err := p.h.ResmapFactory().RF().SliceFromBytes([]byte(p.Patches))
+		res, err := h.ResmapFactory().RF().SliceFromBytes([]byte(p.Patches))
 		if err != nil {
 			return err
 		}
@@ -61,15 +59,17 @@ func (p *PatchStrategicMergeTransformerPlugin) Config(
 		return fmt.Errorf(
 			"patch appears to be empty; files=%v, Patch=%s", p.Paths, p.Patches)
 	}
-	return err
+	// Merge the patches, looking for conflicts.
-}
+	m, err := h.ResmapFactory().ConflatePatches(p.loadedPatches)
-
-func (p *PatchStrategicMergeTransformerPlugin) Transform(m resmap.ResMap) error {
-	patches, err := p.h.ResmapFactory().Merge(p.loadedPatches)
 	if err != nil {
 		return err
 	}
-	for _, patch := range patches.Resources() {
+	p.loadedPatches = m.Resources()
+	return nil
+}
+
+func (p *PatchStrategicMergeTransformerPlugin) Transform(m resmap.ResMap) error {
+	for _, patch := range p.loadedPatches {
 		target, err := m.GetById(patch.OrgId())
 		if err != nil {
 			return err

api/internal/conflict/smpatchmergeonlydetector.go

@@ -21,6 +21,11 @@ func (c *smPatchMergeOnlyDetector) HasConflict(
 	return false, nil
 }
 
+// There's at least one case that doesn't work.  Suppose one has a
+// Deployment with a volume with the bizarre "emptyDir: {}" entry.
+// If you want to get rid of this entry via a patch containing
+// the entry "emptyDir: null", then the following won't work,
+// because null entries are eliminated.
 func (c *smPatchMergeOnlyDetector) MergePatches(
 	r, patch *resource.Resource) (*resource.Resource, error) {
 	err := r.ApplySmPatch(patch)

api/krusty/multiplepatch_test.go

@@ -10,6 +10,441 @@ import (
 	kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
 )
 
+func TestVolumePatch1(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK(".", `
+resources:
+- deployment.yaml
+patchesStrategicMerge:
+- patch.yaml
+`)
+	th.WriteF("deployment.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      volumes:
+      - name: fancyDisk
+        emptyDir: {}
+`)
+	th.WriteF("patch.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      volumes:
+      - name: fancyDisk
+        emptyDir: null
+        gcePersistentDisk:
+          pdName: fancyDisk
+`)
+	m := th.Run(".", th.MakeDefaultOptions())
+	th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      volumes:
+      - gcePersistentDisk:
+          pdName: fancyDisk
+        name: fancyDisk
+`)
+}
+
+func TestVolumePatch2(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK("base", `
+resources:
+- deployment.yaml
+configMapGenerator:
+- name: baseCm
+  literals:
+  - foo=bar
+`)
+	th.WriteF("base/deployment.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: nginx
+        volumeMounts:
+        - name: fancyDisk
+          mountPath: /tmp/ps
+      volumes:
+      - name: fancyDisk
+        emptyDir: {}
+      - configMap:
+          name: baseCm
+        name: baseCm
+`)
+	m := th.Run("base", th.MakeDefaultOptions())
+	th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx
+        name: nginx
+        volumeMounts:
+        - mountPath: /tmp/ps
+          name: fancyDisk
+      volumes:
+      - emptyDir: {}
+        name: fancyDisk
+      - configMap:
+          name: baseCm-798k5k7g9f
+        name: baseCm
+---
+apiVersion: v1
+data:
+  foo: bar
+kind: ConfigMap
+metadata:
+  name: baseCm-798k5k7g9f
+`)
+
+	th.WriteK("overlay", `
+patchesStrategicMerge:
+- patch.yaml
+resources:
+- ../base
+configMapGenerator:
+- name: overlayCm
+  literals:
+  - hello=world
+`)
+	th.WriteF("overlay/patch.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      volumes:
+      - name: fancyDisk
+        emptyDir: null
+        gcePersistentDisk:
+          pdName: fancyDisk
+      - configMap:
+          name: overlayCm
+        name: overlayCm
+`)
+	m = th.Run("overlay", th.MakeDefaultOptions())
+	th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx
+        name: nginx
+        volumeMounts:
+        - mountPath: /tmp/ps
+          name: fancyDisk
+      volumes:
+      - gcePersistentDisk:
+          pdName: fancyDisk
+        name: fancyDisk
+      - configMap:
+          name: overlayCm-dc6fm46dhm
+        name: overlayCm
+      - configMap:
+          name: baseCm-798k5k7g9f
+        name: baseCm
+---
+apiVersion: v1
+data:
+  foo: bar
+kind: ConfigMap
+metadata:
+  name: baseCm-798k5k7g9f
+---
+apiVersion: v1
+data:
+  hello: world
+kind: ConfigMap
+metadata:
+  name: overlayCm-dc6fm46dhm
+`)
+}
+
+func TestVolumePatch3(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK("base", `
+commonLabels:
+  team: foo
+resources:
+- deployment.yaml
+configMapGenerator:
+- name: configmap-in-base
+  literals:
+  - foo=bar
+`)
+	th.WriteF("base/deployment.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: nginx
+      - name: sidecar
+        image: sidecar:latest
+      volumes:
+      - name: nginx-persistent-storage
+        emptyDir: {}
+      - configMap:
+          name: configmap-in-base
+        name: configmap-in-base
+`)
+	th.WriteK("overlay", `
+commonLabels:
+  env: staging
+patchesStrategicMerge:
+- deployment-patch1.yaml
+- deployment-patch2.yaml
+resources:
+- ../base
+`)
+	th.WriteF("overlay/deployment-patch1.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      volumes:
+      - name: nginx-persistent-storage
+        emptyDir: null
+        gcePersistentDisk:
+          pdName: nginx-persistent-storage
+`)
+	th.WriteF("overlay/deployment-patch2.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx
+        name: nginx
+        env:
+        - name: ANOTHERENV
+          value: FOO
+      volumes:
+      - name: nginx-persistent-storage
+`)
+	m := th.Run("overlay", th.MakeDefaultOptions())
+	th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  labels:
+    env: staging
+    team: foo
+  name: nginx
+spec:
+  selector:
+    matchLabels:
+      env: staging
+      team: foo
+  template:
+    metadata:
+      labels:
+        env: staging
+        team: foo
+    spec:
+      containers:
+      - env:
+        - name: ANOTHERENV
+          value: FOO
+        image: nginx
+        name: nginx
+      - image: sidecar:latest
+        name: sidecar
+      volumes:
+      - gcePersistentDisk:
+          pdName: nginx-persistent-storage
+        name: nginx-persistent-storage
+      - configMap:
+          name: configmap-in-base-798k5k7g9f
+        name: configmap-in-base
+---
+apiVersion: v1
+data:
+  foo: bar
+kind: ConfigMap
+metadata:
+  labels:
+    env: staging
+    team: foo
+  name: configmap-in-base-798k5k7g9f
+`)
+}
+
+func TestEmptyDirOverrideMultiplePatches(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK("base", `
+namePrefix: b-
+commonLabels:
+  team: foo
+resources:
+- deployment.yaml
+configMapGenerator:
+- name: configmap-in-base
+  literals:
+  - foo=bar
+`)
+	th.WriteF("base/deployment.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: nginx
+        volumeMounts:
+        - name: nginx-persistent-storage
+          mountPath: /tmp/ps
+      - name: sidecar
+        image: sidecar:latest
+      volumes:
+      - name: nginx-persistent-storage
+        emptyDir: {}
+      - configMap:
+          name: configmap-in-base
+        name: configmap-in-base
+`)
+	th.WriteK("overlay", `
+namePrefix: a-
+commonLabels:
+  env: staging
+patchesStrategicMerge:
+- deployment-patch1.yaml
+- deployment-patch2.yaml
+resources:
+- ../base
+`)
+	th.WriteF("overlay/deployment-patch1.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: nginx:latest
+        env:
+        - name: ENVKEY
+          value: ENVVALUE
+      volumes:
+      - name: nginx-persistent-storage
+        emptyDir: null
+        gcePersistentDisk:
+          pdName: nginx-persistent-storage
+`)
+	th.WriteF("overlay/deployment-patch2.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      containers:
+      - name: nginx
+        env:
+        - name: ANOTHERENV
+          value: FOO
+      volumes:
+      - name: nginx-persistent-storage
+`)
+	m := th.Run("overlay", th.MakeDefaultOptions())
+	th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  labels:
+    env: staging
+    team: foo
+  name: a-b-nginx
+spec:
+  selector:
+    matchLabels:
+      env: staging
+      team: foo
+  template:
+    metadata:
+      labels:
+        env: staging
+        team: foo
+    spec:
+      containers:
+      - env:
+        - name: ANOTHERENV
+          value: FOO
+        - name: ENVKEY
+          value: ENVVALUE
+        image: nginx:latest
+        name: nginx
+        volumeMounts:
+        - mountPath: /tmp/ps
+          name: nginx-persistent-storage
+      - image: sidecar:latest
+        name: sidecar
+      volumes:
+      - gcePersistentDisk:
+          pdName: nginx-persistent-storage
+        name: nginx-persistent-storage
+      - configMap:
+          name: a-b-configmap-in-base-798k5k7g9f
+        name: configmap-in-base
+---
+apiVersion: v1
+data:
+  foo: bar
+kind: ConfigMap
+metadata:
+  labels:
+    env: staging
+    team: foo
+  name: a-b-configmap-in-base-798k5k7g9f
+`)
+}
+
 func TestSimpleMultiplePatches(t *testing.T) {
 	th := kusttest_test.MakeHarness(t)
 	th.WriteK("base", `
@@ -405,6 +840,10 @@ metadata:
 
 func TestMultiplePatchesWithConflict(t *testing.T) {
 	th := kusttest_test.MakeHarness(t)
+	opts := th.MakeDefaultOptions()
+	if opts.UseKyaml {
+		t.Skip("kyaml merging doesn't look for conflicts")
+	}
 	makeCommonFileForMultiplePatchTest(th)
 	th.WriteF("/app/overlay/staging/deployment-patch1.yaml", `
 apiVersion: apps/v1
@@ -442,7 +881,7 @@ spec:
         - name: ENABLE_FEATURE_FOO
           value: FALSE
 `)
-	err := th.RunWithErr("/app/overlay/staging", th.MakeDefaultOptions())
+	err := th.RunWithErr("/app/overlay/staging", opts)
 	if err == nil {
 		t.Fatalf("expected conflict")
 	}

api/resmap/factory.go

@@ -126,10 +126,11 @@ func (rmF *Factory) FromSecretArgs(
 	return rmF.FromResource(res), nil
 }
 
-// Merge creates a new ResMap by merging incoming resources.
+// ConflatePatches creates a new ResMap containing a merger of the
+// incoming patches.
 // Error if conflict found.
-func (rmF *Factory) Merge(incoming []*resource.Resource) (ResMap, error) {
+func (rmF *Factory) ConflatePatches(patches []*resource.Resource) (ResMap, error) {
-	return (&merginator{cdf: rmF.cdf}).Merge(incoming)
+	return (&merginator{cdf: rmF.cdf}).ConflatePatches(patches)
 }
 
 func newResMapFromResourceSlice(

api/resmap/factory_test.go

@@ -351,13 +351,13 @@ metadata:
 	}
 }
 
-func TestMerge_Empty(t *testing.T) {
+func TestConflatePatches_Empty(t *testing.T) {
-	rm, err := rmF.Merge([]*resource.Resource{})
+	rm, err := rmF.ConflatePatches([]*resource.Resource{})
 	assert.NoError(t, err)
 	assert.Equal(t, 0, rm.Size())
 }
 
-func TestMerge(t *testing.T) {
+func TestConflatePatches(t *testing.T) {
 	var (
 		err    error
 		yml    []byte
@@ -387,7 +387,7 @@ spec:
 `))
 	assert.NoError(t, err)
 
-	rm, err := rmF.Merge([]*resource.Resource{r1, r2})
+	rm, err := rmF.ConflatePatches([]*resource.Resource{r1, r2})
 	assert.NoError(t, err)
 
 	yml, err = rm.AsYaml()

api/resmap/merginator.go

@@ -16,7 +16,7 @@ type merginator struct {
 	result   ResMap
 }
 
-func (m *merginator) Merge(in []*resource.Resource) (ResMap, error) {
+func (m *merginator) ConflatePatches(in []*resource.Resource) (ResMap, error) {
 	m.result = New()
 	m.incoming = in
 	for index := range m.incoming {

plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer.go

@@ -14,7 +14,6 @@ import (
 )
 
 type plugin struct {
-	h             *resmap.PluginHelpers
 	loadedPatches []*resource.Resource
 	Paths         []types.PatchStrategicMerge `json:"paths,omitempty" yaml:"paths,omitempty"`
 	Patches       string                      `json:"patches,omitempty" yaml:"patches,omitempty"`
@@ -25,7 +24,6 @@ var KustomizePlugin plugin
 
 func (p *plugin) Config(
 	h *resmap.PluginHelpers, c []byte) (err error) {
-	p.h = h
 	err = yaml.Unmarshal(c, p)
 	if err != nil {
 		return err
@@ -40,13 +38,13 @@ func (p *plugin) Config(
 			// 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))
+			res, err := h.ResmapFactory().RF().SliceFromBytes([]byte(onePath))
 			if err == nil {
 				p.loadedPatches = append(p.loadedPatches, res...)
 				continue
 			}
-			res, err = p.h.ResmapFactory().RF().SliceFromPatches(
+			res, err = h.ResmapFactory().RF().SliceFromPatches(
-				p.h.Loader(), []types.PatchStrategicMerge{onePath})
+				h.Loader(), []types.PatchStrategicMerge{onePath})
 			if err != nil {
 				return err
 			}
@@ -54,7 +52,7 @@ func (p *plugin) Config(
 		}
 	}
 	if p.Patches != "" {
-		res, err := p.h.ResmapFactory().RF().SliceFromBytes([]byte(p.Patches))
+		res, err := h.ResmapFactory().RF().SliceFromBytes([]byte(p.Patches))
 		if err != nil {
 			return err
 		}
@@ -65,15 +63,17 @@ func (p *plugin) Config(
 		return fmt.Errorf(
 			"patch appears to be empty; files=%v, Patch=%s", p.Paths, p.Patches)
 	}
-	return err
+	// Merge the patches, looking for conflicts.
-}
+	m, err := h.ResmapFactory().ConflatePatches(p.loadedPatches)
-
-func (p *plugin) Transform(m resmap.ResMap) error {
-	patches, err := p.h.ResmapFactory().Merge(p.loadedPatches)
 	if err != nil {
 		return err
 	}
-	for _, patch := range patches.Resources() {
+	p.loadedPatches = m.Resources()
+	return nil
+}
+
+func (p *plugin) Transform(m resmap.ResMap) error {
+	for _, patch := range p.loadedPatches {
 		target, err := m.GetById(patch.OrgId())
 		if err != nil {
 			return err