From adedca09f21b1c0a3d9f35346cd7c64ef6cbc28e Mon Sep 17 00:00:00 2001 From: brianpursley Date: Mon, 2 Aug 2021 20:36:28 -0400 Subject: [PATCH 1/2] Add unit tests for current behavior of three way merge --- kyaml/yaml/merge3/element_test.go | 175 ++++++++++++++++++++++++++++++ 1 file changed, 175 insertions(+) diff --git a/kyaml/yaml/merge3/element_test.go b/kyaml/yaml/merge3/element_test.go index 80efe5907..c3257dddb 100644 --- a/kyaml/yaml/merge3/element_test.go +++ b/kyaml/yaml/merge3/element_test.go @@ -1476,6 +1476,181 @@ spec: protocol: TCP `}, + // + // Test Case + // + { + description: `Add single container port and add another container port after an existing container port`, + origin: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-deployment +spec: + template: + spec: + containers: + - image: test-image + name: test-deployment + ports: + - containerPort: 8001 + name: A + protocol: TCP + - containerPort: 8002 + name: B + protocol: TCP +`, + update: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-deployment +spec: + template: + spec: + containers: + - image: test-image + name: test-deployment + ports: + - containerPort: 8004 + name: D + protocol: TCP +`, + local: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-deployment +spec: + template: + spec: + containers: + - image: test-image + name: test-deployment + ports: + - containerPort: 8001 + name: A + protocol: TCP + - containerPort: 8003 + name: C + protocol: TCP +`, + // This test records current behavior, but this behavior might be undesirable. + // If so, feel free to change the test to pass with some improved algorithm. + expected: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-deployment +spec: + template: + spec: + containers: + - image: test-image + name: test-deployment + ports: + - containerPort: 8003 + name: C + protocol: TCP + - containerPort: 8004 + name: D + protocol: TCP +`}, + + // + // Test Case + // + { + description: `Add new container port after existing container ports and add another container port between existing container ports`, + origin: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-deployment +spec: + template: + spec: + containers: + - image: test-image + name: test-deployment + ports: + - containerPort: 8001 + name: A + protocol: TCP + - containerPort: 8002 + name: B + protocol: TCP +`, + update: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-deployment +spec: + template: + spec: + containers: + - image: test-image + name: test-deployment + ports: + - containerPort: 8001 + name: A + protocol: TCP + - containerPort: 8002 + name: B + protocol: TCP + - containerPort: 8004 + name: D + protocol: TCP +`, + local: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-deployment +spec: + template: + spec: + containers: + - image: test-image + name: test-deployment + ports: + - containerPort: 8001 + name: A + protocol: TCP + - containerPort: 8003 + name: C + protocol: TCP + - containerPort: 8002 + name: B + protocol: TCP +`, + expected: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-deployment +spec: + template: + spec: + containers: + - image: test-image + name: test-deployment + ports: + - containerPort: 8001 + name: A + protocol: TCP + - containerPort: 8003 + name: C + protocol: TCP + - containerPort: 8002 + name: B + protocol: TCP + - containerPort: 8004 + name: D + protocol: TCP +`}, + { description: `Retain local protocol`, origin: ` From aabbea3e78b7bf888271101d9f4fa61b7f8fdc37 Mon Sep 17 00:00:00 2001 From: brianpursley Date: Mon, 2 Aug 2021 20:36:55 -0400 Subject: [PATCH 2/2] Add unit tests for current behavior of strategic merge patch --- api/resource/resource_test.go | 260 ++++++++++++++++++++++++++++++++++ 1 file changed, 260 insertions(+) diff --git a/api/resource/resource_test.go b/api/resource/resource_test.go index a73437b9f..af833213e 100644 --- a/api/resource/resource_test.go +++ b/api/resource/resource_test.go @@ -306,6 +306,266 @@ spec: `, string(bytes)) } +func TestApplySmPatchShouldOutputListItemsInCorrectOrder(t *testing.T) { + cases := []struct { + name string + skip bool + patch string + expectedOutput string + }{ + { + name: "Order should not change when patch has foo only", + patch: `apiVersion: v1 +kind: Pod +metadata: + name: test +spec: + initContainers: + - name: foo +`, + expectedOutput: `apiVersion: v1 +kind: Pod +metadata: + name: test +spec: + initContainers: + - name: foo + - name: bar +`, + }, + { + name: "Order changes when patch has bar only", + patch: `apiVersion: v1 +kind: Pod +metadata: + name: test +spec: + initContainers: + - name: bar +`, + // This test records current behavior, but this behavior might be undesirable. + // If so, feel free to change the test to pass with some improved algorithm. + expectedOutput: `apiVersion: v1 +kind: Pod +metadata: + name: test +spec: + initContainers: + - name: bar + - name: foo +`, + }, + { + name: "Order should not change and should include a new item at the beginning when patch has a new list item", + patch: `apiVersion: v1 +kind: Pod +metadata: + name: test +spec: + initContainers: + - name: baz +`, + expectedOutput: `apiVersion: v1 +kind: Pod +metadata: + name: test +spec: + initContainers: + - name: baz + - name: foo + - name: bar +`, + }, + { + name: "Order should not change when patch has foo and bar in same order", + patch: `apiVersion: v1 +kind: Pod +metadata: + name: test +spec: + initContainers: + - name: foo + - name: bar +`, + expectedOutput: `apiVersion: v1 +kind: Pod +metadata: + name: test +spec: + initContainers: + - name: foo + - name: bar +`, + }, + { + name: "Order should change when patch has foo and bar in different order", + patch: `apiVersion: v1 +kind: Pod +metadata: + name: test +spec: + initContainers: + - name: bar + - name: foo +`, + expectedOutput: `apiVersion: v1 +kind: Pod +metadata: + name: test +spec: + initContainers: + - name: bar + - name: foo +`, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if tc.skip { + t.Skip() + } + + resource, err := factory.FromBytes([]byte(` +apiVersion: v1 +kind: Pod +metadata: + name: test +spec: + initContainers: + - name: foo + - name: bar +`)) + assert.NoError(t, err) + + patch, err := factory.FromBytes([]byte(tc.patch)) + assert.NoError(t, err) + assert.NoError(t, resource.ApplySmPatch(patch)) + bytes, err := resource.AsYAML() + assert.NoError(t, err) + assert.Equal(t, tc.expectedOutput, string(bytes)) + }) + } +} + +func TestApplySmPatchShouldOutputPrimitiveListItemsInCorrectOrder(t *testing.T) { + cases := []struct { + name string + skip bool + patch string + expectedOutput string + }{ + { + name: "Order should not change when patch has foo only", + patch: `apiVersion: v1 +kind: Pod +metadata: + name: test + finalizers: ["foo"] +`, + expectedOutput: `apiVersion: v1 +kind: Pod +metadata: + finalizers: + - foo + - bar + name: test +`, + }, + { + name: "Order should not change when patch has bar only", + skip: true, // TODO: This test should pass but fails currently. Fix the problem and unskip this test + patch: `apiVersion: v1 +kind: Pod +metadata: + name: test + finalizers: ["bar"] +`, + expectedOutput: `apiVersion: v1 +kind: Pod +metadata: + finalizers: + - foo + - bar + name: test +`, + }, + { + name: "Order should not change and should include a new item at the beginning when patch has a new list item", + patch: `apiVersion: v1 +kind: Pod +metadata: + name: test + finalizers: ["baz"] +`, + expectedOutput: `apiVersion: v1 +kind: Pod +metadata: + finalizers: + - baz + - foo + - bar + name: test +`, + }, + { + name: "Order should not change when patch has foo and bar in same order", + patch: `apiVersion: v1 +kind: Pod +metadata: + name: test + finalizers: ["foo", "bar"] +`, + expectedOutput: `apiVersion: v1 +kind: Pod +metadata: + finalizers: + - foo + - bar + name: test +`, + }, + { + name: "Order should change when patch has foo and bar in different order", + patch: `apiVersion: v1 +kind: Pod +metadata: + name: test + finalizers: ["bar", "foo"] +`, + expectedOutput: `apiVersion: v1 +kind: Pod +metadata: + finalizers: + - bar + - foo + name: test +`, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if tc.skip { + t.Skip() + } + + resource, err := factory.FromBytes([]byte(` +kind: Pod +metadata: + name: test + finalizers: ["foo", "bar"] +`)) + assert.NoError(t, err) + + patch, err := factory.FromBytes([]byte(tc.patch)) + assert.NoError(t, err) + assert.NoError(t, resource.ApplySmPatch(patch)) + bytes, err := resource.AsYAML() + assert.NoError(t, err) + assert.Equal(t, tc.expectedOutput, string(bytes)) + }) + } +} + func TestMergeDataMapFrom(t *testing.T) { resource, err := factory.FromBytes([]byte(` apiVersion: v1