From 95fe0a158c2ddd614d782967b5d7112222cff648 Mon Sep 17 00:00:00 2001
From: Brian Pursley <bpursley@cinlogic.com>
Date: Wed, 8 Jun 2022 19:58:10 -0400
Subject: [PATCH] Fix strategic merge patch $deleteFromPrimitiveList bug

If the $deleteFromPrimitiveList directive is used in a strategic
merge patch and the field being deleted from does not have a
patch strategy of merge, strategic merge patch would not perform
the deletion and instead would replace the contents of the field
with the values attempting to be deleted.

This commit changes strategic merge patch to always perform a
deletion when the $deleteFromPrimitiveList directive is used,
regardless of what the patch strategy of the field is.
---
 .../pkg/util/strategicpatch/patch.go          |  3 +-
 .../pkg/util/strategicpatch/patch_test.go     | 40 +++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go b/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go
index 6fb36973213..9aab2652a0a 100644
--- a/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go
+++ b/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go
@@ -1425,7 +1425,8 @@ func mergeSliceHandler(original, patch interface{}, schema LookupPatchMeta,
 		return nil, err
 	}
 
-	if fieldPatchStrategy == mergeDirective {
+	// Delete lists are handled the same way regardless of what the field's patch strategy is
+	if fieldPatchStrategy == mergeDirective || isDeleteList {
 		return mergeSlice(typedOriginal, typedPatch, schema, fieldPatchMergeKey, mergeOptions, isDeleteList)
 	} else {
 		return typedPatch, nil
diff --git a/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch_test.go b/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch_test.go
index 8ddcddc5b8d..70599e1de1c 100644
--- a/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch_test.go
+++ b/staging/src/k8s.io/apimachinery/pkg/util/strategicpatch/patch_test.go
@@ -717,6 +717,46 @@ $deleteFromPrimitiveList/mergingIntList:
 			Modified: []byte(`
 foo:
   - bar
+`),
+		},
+	},
+	{
+		Description: "$deleteFromPrimitiveList should delete item from a list with merge patch strategy",
+		StrategicMergePatchRawTestCaseData: StrategicMergePatchRawTestCaseData{
+			Original: []byte(`
+mergingIntList:
+  - 1
+  - 2
+  - 3
+`),
+			TwoWay: []byte(`
+$deleteFromPrimitiveList/mergingIntList:
+  - 2
+`),
+			Modified: []byte(`
+mergingIntList:
+  - 1
+  - 3
+`),
+		},
+	},
+	{
+		Description: "$deleteFromPrimitiveList should delete item from a list without merge patch strategy",
+		StrategicMergePatchRawTestCaseData: StrategicMergePatchRawTestCaseData{
+			Original: []byte(`
+nonMergingIntList:
+  - 1
+  - 2
+  - 3
+`),
+			TwoWay: []byte(`
+$deleteFromPrimitiveList/nonMergingIntList:
+  - 2
+`),
+			Modified: []byte(`
+nonMergingIntList:
+  - 1
+  - 3
 `),
 		},
 	},