Merge pull request #1356 from keleustes/droppatch

Test tracking issue patchesStrategicMerge elements can be dropped
2026-05-17 18:25:26 +00:00 · 2019-07-16 10:50:18 -07:00
parent 76a7816aeb e5ebca6604
commit 0707deae95
4 changed files with 282 additions and 26 deletions
--- a/pkg/target/chartinflatorplugin_test.go
+++ b/pkg/target/chartinflatorplugin_test.go
@@ -8,10 +8,10 @@
 package target_test

 import (
-	"sigs.k8s.io/kustomize/v3/pkg/plugins"
 	"testing"

-	"sigs.k8s.io/kustomize/v3/pkg/kusttest"
+	kusttest_test "sigs.k8s.io/kustomize/v3/pkg/kusttest"
+	plugins_test "sigs.k8s.io/kustomize/v3/pkg/plugins/test"
 )

 // This is an example of using a helm chart as a base,
--- a/pkg/target/multiplepatch_test.go
+++ b/pkg/target/multiplepatch_test.go
@@ -61,6 +61,8 @@ spec:
        volumeMounts:
        - name: nginx-persistent-storage
          mountPath: /tmp/ps
+      - name: sidecar
+        image: sidecar:latest
      volumes:
      - name: nginx-persistent-storage
        emptyDir: {}
@@ -138,8 +140,6 @@ spec:
        env:
        - name: ANOTHERENV
          value: FOO
-      - name: sidecar
-        image: sidecar
      volumes:
      - name: nginx-persistent-storage
 `)
@@ -187,7 +187,7 @@ spec:
        volumeMounts:
        - mountPath: /tmp/ps
          name: nginx-persistent-storage
-      - image: sidecar
+      - image: sidecar:latest
        name: sidecar
      volumes:
      - gcePersistentDisk:
@@ -293,3 +293,258 @@ spec:
 		t.Fatalf("Unexpected err: %v", err)
 	}
 }
+
+// TestMultiplePatchesWithPatchDeleteIgnored demonstrates that if the
+// patch containing the $patch:delete directive is second in the list,
+// the behavior of kustomize is incorrect, and the sidecar container is
+// not removed from the final output. No conflict, nor error is reported
+// even so the patch is ignored. See issue #1354
+func TestMultiplePatchesWithPatchDeleteIgnored(t *testing.T) {
+	th := kusttest_test.NewKustTestHarness(t, "/app/overlay/staging")
+	makeCommonFileForMultiplePatchTest(th)
+	th.WriteF("/app/overlay/staging/deployment-patch1.yaml", `
+apiVersion: apps/v1beta2
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      containers:
+      - name: nginx
+        env:
+        - name: SOME_NAME
+          value: somevalue
+`)
+	th.WriteF("/app/overlay/staging/deployment-patch2.yaml", `
+apiVersion: apps/v1beta2
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      containers:
+      - $patch: delete
+        name: sidecar
+`)
+	m, err := th.MakeKustTarget().MakeCustomizedResMap()
+	if err != nil {
+		t.Fatalf("Err: %v", err)
+	}
+	th.AssertActualEqualsExpected(m, `apiVersion: apps/v1beta2
+kind: Deployment
+metadata:
+  annotations:
+    note: This is a test annotation
+  labels:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+  name: staging-team-foo-nginx
+spec:
+  selector:
+    matchLabels:
+      app: mynginx
+      env: staging
+      org: example.com
+      team: foo
+  template:
+    metadata:
+      annotations:
+        note: This is a test annotation
+      labels:
+        app: mynginx
+        env: staging
+        org: example.com
+        team: foo
+    spec:
+      containers:
+      - env:
+        - name: SOME_NAME
+          value: somevalue
+        image: nginx
+        name: nginx
+        volumeMounts:
+        - mountPath: /tmp/ps
+          name: nginx-persistent-storage
+      - image: sidecar:latest
+        name: sidecar
+      volumes:
+      - emptyDir: {}
+        name: nginx-persistent-storage
+      - configMap:
+          name: staging-team-foo-configmap-in-base-g7k6gt2889
+        name: configmap-in-base
+---
+apiVersion: v1
+kind: Service
+metadata:
+  annotations:
+    note: This is a test annotation
+  labels:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+  name: staging-team-foo-nginx
+spec:
+  ports:
+  - port: 80
+  selector:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+---
+apiVersion: v1
+data:
+  foo: bar
+kind: ConfigMap
+metadata:
+  annotations:
+    note: This is a test annotation
+  labels:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+  name: staging-team-foo-configmap-in-base-g7k6gt2889
+---
+apiVersion: v1
+data:
+  hello: world
+kind: ConfigMap
+metadata:
+  labels:
+    env: staging
+  name: staging-configmap-in-overlay-k7cbc75tg8
+`)
+}
+
+// TestMultiplePatchesWithPatchDeleteApplied demonstrates that if the
+// patch containing the $patch:delete directive is first in the list,
+// the behavior of kustomize is correct, and the sidecar container
+// is removed from the final output. See issue #1354
+func TestMultiplePatchesWithPatchDeleteApplied(t *testing.T) {
+	th := kusttest_test.NewKustTestHarness(t, "/app/overlay/staging")
+	makeCommonFileForMultiplePatchTest(th)
+	th.WriteF("/app/overlay/staging/deployment-patch1.yaml", `
+apiVersion: apps/v1beta2
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      containers:
+      - $patch: delete
+        name: sidecar
+`)
+	th.WriteF("/app/overlay/staging/deployment-patch2.yaml", `
+apiVersion: apps/v1beta2
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  template:
+    spec:
+      containers:
+      - name: nginx
+        env:
+        - name: SOME_NAME
+          value: somevalue
+`)
+	m, err := th.MakeKustTarget().MakeCustomizedResMap()
+	if err != nil {
+		t.Fatalf("Err: %v", err)
+	}
+	th.AssertActualEqualsExpected(m, `apiVersion: apps/v1beta2
+kind: Deployment
+metadata:
+  annotations:
+    note: This is a test annotation
+  labels:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+  name: staging-team-foo-nginx
+spec:
+  selector:
+    matchLabels:
+      app: mynginx
+      env: staging
+      org: example.com
+      team: foo
+  template:
+    metadata:
+      annotations:
+        note: This is a test annotation
+      labels:
+        app: mynginx
+        env: staging
+        org: example.com
+        team: foo
+    spec:
+      containers:
+      - env:
+        - name: SOME_NAME
+          value: somevalue
+        image: nginx
+        name: nginx
+        volumeMounts:
+        - mountPath: /tmp/ps
+          name: nginx-persistent-storage
+      volumes:
+      - emptyDir: {}
+        name: nginx-persistent-storage
+      - configMap:
+          name: staging-team-foo-configmap-in-base-g7k6gt2889
+        name: configmap-in-base
+---
+apiVersion: v1
+kind: Service
+metadata:
+  annotations:
+    note: This is a test annotation
+  labels:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+  name: staging-team-foo-nginx
+spec:
+  ports:
+  - port: 80
+  selector:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+---
+apiVersion: v1
+data:
+  foo: bar
+kind: ConfigMap
+metadata:
+  annotations:
+    note: This is a test annotation
+  labels:
+    app: mynginx
+    env: staging
+    org: example.com
+    team: foo
+  name: staging-team-foo-configmap-in-base-g7k6gt2889
+---
+apiVersion: v1
+data:
+  hello: world
+kind: ConfigMap
+metadata:
+  labels:
+    env: staging
+  name: staging-configmap-in-overlay-k7cbc75tg8
+`)
+}
--- a/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go
+++ b/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go
@@ -8,7 +8,7 @@ import (
 	"testing"

 	"sigs.k8s.io/kustomize/v3/pkg/kusttest"
-	"sigs.k8s.io/kustomize/v3/pkg/plugins"
+	plugins_test "sigs.k8s.io/kustomize/v3/pkg/plugins/test"
 )

 const (
@@ -58,7 +58,7 @@ spec:
 )

 func TestPatchStrategicMergeTransformerMissingFile(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -83,7 +83,7 @@ paths:
 }

 func TestBadPatchStrategicMergeTransformer(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -108,7 +108,7 @@ patches: 'thisIsNotAPatch'
 }

 func TestBothEmptyPatchStrategicMergeTransformer(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -131,7 +131,7 @@ metadata:
 }

 func TestPatchStrategicMergeTransformerFromFiles(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -181,7 +181,7 @@ spec:
 }

 func TestPatchStrategicMergeTransformerWithInline(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -216,7 +216,7 @@ spec:
 }

 func TestPatchStrategicMergeTransformerMultiplePatches(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -293,7 +293,7 @@ spec:
 }

 func TestStrategicMergeTransformerMultiplePatchesWithConflicts(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -354,7 +354,7 @@ paths:
 }

 func TestStrategicMergeTransformerWrongNamespace(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -397,7 +397,7 @@ paths:
 }

 func TestStrategicMergeTransformerNoSchema(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -437,7 +437,7 @@ spec:
 }

 func TestStrategicMergeTransformerNoSchemaMultiPatches(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -493,7 +493,7 @@ spec:
 }

 func TestStrategicMergeTransformerNoSchemaMultiPatchesWithConflict(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
--- a/plugin/builtin/patchtransformer/PatchTransformer_test.go
+++ b/plugin/builtin/patchtransformer/PatchTransformer_test.go
@@ -4,10 +4,11 @@
 package main_test

 import (
-	kusttest_test "sigs.k8s.io/kustomize/v3/pkg/kusttest"
-	"sigs.k8s.io/kustomize/v3/pkg/plugins"
 	"strings"
 	"testing"
+
+	kusttest_test "sigs.k8s.io/kustomize/v3/pkg/kusttest"
+	plugins_test "sigs.k8s.io/kustomize/v3/pkg/plugins/test"
 )

 const (
@@ -65,7 +66,7 @@ spec:
 )

 func TestPatchTransformerMissingFile(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -89,7 +90,7 @@ path: patch.yaml
 }

 func TestPatchTransformerBadPatch(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -113,7 +114,7 @@ patch: "thisIsNotAPatch"
 }

 func TestPatchTransformerMissingSelector(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -137,7 +138,7 @@ patch: '[{"op": "add", "path": "/spec/template/spec/dnsPolicy", "value": "Cluste
 }

 func TestPatchTransformerBothEmptyPathAndPatch(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -160,7 +161,7 @@ metadata:
 }

 func TestPatchTransformerBothNonEmptyPathAndPatch(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -185,7 +186,7 @@ Patch: "something"
 }

 func TestPatchTransformerFromFiles(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(
@@ -267,7 +268,7 @@ spec:
 }

 func TestPatchTransformerWithInline(t *testing.T) {
-	tc := plugins.NewEnvForTest(t).Set()
+	tc := plugins_test.NewEnvForTest(t).Set()
 	defer tc.Reset()

 	tc.BuildGoPlugin(