diff --git a/pkg/target/multiplepatch_test.go b/pkg/target/multiplepatch_test.go index 772b9f16d..47c92391f 100644 --- a/pkg/target/multiplepatch_test.go +++ b/pkg/target/multiplepatch_test.go @@ -20,7 +20,7 @@ import ( "strings" "testing" - "sigs.k8s.io/kustomize/v3/pkg/kusttest" + kusttest_test "sigs.k8s.io/kustomize/v3/pkg/kusttest" ) func makeCommonFileForMultiplePatchTest(th *kusttest_test.KustTestHarness) { @@ -294,16 +294,8 @@ spec: } } -// 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 +func TestMultiplePatchesWithOnePatchDeleteDirective(t *testing.T) { + additivePatch := `apiVersion: apps/v1beta2 kind: Deployment metadata: name: nginx @@ -315,9 +307,8 @@ spec: env: - name: SOME_NAME value: somevalue -`) - th.WriteF("/app/overlay/staging/deployment-patch2.yaml", ` -apiVersion: apps/v1beta2 +` + deletePatch := `apiVersion: apps/v1beta2 kind: Deployment metadata: name: nginx @@ -327,12 +318,36 @@ spec: containers: - $patch: delete name: sidecar -`) - m, err := th.MakeKustTarget().MakeCustomizedResMap() - if err != nil { - t.Fatalf("Err: %v", err) +` + cases := []struct { + name string + patch1 string + patch2 string + expectError bool + }{ + { + name: "Patch with delete directive first", + patch1: deletePatch, + patch2: additivePatch, + }, + { + name: "Patch with delete directive second", + patch1: additivePatch, + patch2: deletePatch, + }, } - th.AssertActualEqualsExpected(m, `apiVersion: apps/v1beta2 + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/app/overlay/staging") + + makeCommonFileForMultiplePatchTest(th) + th.WriteF("/app/overlay/staging/deployment-patch1.yaml", c.patch1) + th.WriteF("/app/overlay/staging/deployment-patch2.yaml", c.patch2) + m, err := th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + th.AssertActualEqualsExpected(m, `apiVersion: apps/v1beta2 kind: Deployment metadata: annotations: @@ -369,8 +384,6 @@ spec: volumeMounts: - mountPath: /tmp/ps name: nginx-persistent-storage - - image: sidecar:latest - name: sidecar volumes: - emptyDir: {} name: nginx-persistent-storage @@ -421,13 +434,11 @@ metadata: 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) { +func TestMultiplePatchesBothWithPatchDeleteDirective(t *testing.T) { th := kusttest_test.NewKustTestHarness(t, "/app/overlay/staging") makeCommonFileForMultiplePatchTest(th) th.WriteF("/app/overlay/staging/deployment-patch1.yaml", ` @@ -451,100 +462,16 @@ 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 + - $patch: delete 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 `) + _, err := th.MakeKustTarget().MakeCustomizedResMap() + if err == nil { + t.Fatalf("Expected error") + } + if !strings.Contains( + err.Error(), "both containing ") { + t.Fatalf("Unexpected err: %v", err) + } + return }