apimachinery icon indicating copy to clipboard operation
apimachinery copied to clipboard

strategicpatch utils not use Stable sort

Open Spground opened this issue 1 year ago • 2 comments

test codes as below


func TestPatch(t *testing.T) {
	// test element delete

	oldContainer := &v1.Container{
		Name: "test",
		Env: []v1.EnvVar{
			{
				Name:  "testA",
				Value: "testAa",
			},
			{
				Name:  "testB",
				Value: "testBz",
			},
			{
				Name:  "testB",
				Value: "testBa", 
			}
		},
	}

	newContainer := &v1.Container{
		Name: "test",
		Env: []v1.EnvVar{
			{
				Name:  "testA",
				Value: "testAa",
			},
			{
				Name:  "testB",
				Value: "testBa",
			},
		},
	}
	oldContainerRaw, _ := json.Marshal(oldContainer)
	newContainerRaw, _ := json.Marshal(newContainer)
	patchsRaw, _ := strategicpatch.CreateTwoWayMergePatch(oldContainerRaw, newContainerRaw, &v1.Container{})
	t.Logf("patch raw:%s", string(patchsRaw))

strategicpatch.CreateTwoWayMergePatch will sort original and modified slice before diff. However, sort is not stable, which will leads CreateTwoWayMergePatch generates unexpected patch results.

sorted slice

sorted original: [map[name:testA value:testAa] map[name:testB value:testBa] map[name:testB value:testBz]]
sorted modified: [map[name:testA value:testAa] map[name:testB value:testBa]]

Obviously, sorted original slice is not Stable compared to original slice.

sort implements https://github.com/kubernetes/apimachinery/blob/2511177aa20bc0106b0107226c301f2732b0b85f/pkg/util/strategicpatch/patch.go#L1822, in my opinion, it is better to be stable sort.

Spground avatar Feb 19 '24 08:02 Spground