merge2: preserve explicitly set null values

Fixed bug where an explicitly set null value in yaml was
cleared, even when the patch did not operate on that field.

api/filters/patchstrategicmerge/patchstrategicmerge_test.go

@@ -732,6 +732,50 @@ spec:
           protocol: "TCP"
         - containerPort: 8301
           protocol: "UDP"
+`,
+		},
+
+		// Issue #4628
+		"should retain existing null values in targets": {
+			input: `
+apiVersion: helm.toolkit.fluxcd.io/v2beta1
+kind: HelmRelease
+metadata:
+  name: chart
+spec:
+  releaseName: helm-chart
+  timeout: 15m
+  values:
+    chart:
+      replicaCount: null
+      autoscaling: true
+`,
+			patch: yaml.MustParse(`
+apiVersion: helm.toolkit.fluxcd.io/v2beta1
+kind: HelmRelease
+metadata:
+  name: chart
+spec:
+  releaseName: helm-chart
+  timeout: 15m
+  values:
+    deepgram-api:
+      some: value
+`),
+			expected: `
+apiVersion: helm.toolkit.fluxcd.io/v2beta1
+kind: HelmRelease
+metadata:
+  name: chart
+spec:
+  releaseName: helm-chart
+  timeout: 15m
+  values:
+    chart:
+      replicaCount: null
+      autoscaling: true
+    deepgram-api:
+      some: value
 `,
 		},
 	}

kyaml/yaml/merge2/merge2.go

@@ -1,7 +1,7 @@
 // Copyright 2019 The Kubernetes Authors.
 // SPDX-License-Identifier: Apache-2.0
 
-// Package merge contains libraries for merging fields from one RNode to another
+// Package merge2 contains libraries for merging fields from one RNode to another
 // RNode
 package merge2
 
@@ -20,7 +20,7 @@ func Merge(src, dest *yaml.RNode, mergeOptions yaml.MergeOptions) (*yaml.RNode,
 	}.Walk()
 }
 
-// Merge parses the arguments, and merges fields from srcStr into destStr.
+// MergeStrings parses the arguments, and merges fields from srcStr into destStr.
 func MergeStrings(srcStr, destStr string, infer bool, mergeOptions yaml.MergeOptions) (string, error) {
 	src, err := yaml.Parse(srcStr)
 	if err != nil {
@@ -64,6 +64,12 @@ func (m Merger) VisitMap(nodes walk.Sources, s *openapi.ResourceSchema) (*yaml.R
 			return walk.ClearNode, nil
 		}
 
+		// If Origin is missing, preserve explicitly set null in Dest ("null", "~", etc)
+		if nodes.Origin().IsNil() && !nodes.Dest().IsNil() && len(nodes.Dest().YNode().Value) > 0 {
+			// Return a new node so that it won't have a "!!null" tag and therefore won't be cleared.
+			return yaml.NewScalarRNode(nodes.Dest().YNode().Value), nil
+		}
+
 		return nodes.Origin(), nil
 	}
 	if nodes.Origin().IsTaggedNull() {

kyaml/yaml/merge2/scalar_test.go

@@ -143,6 +143,66 @@ kind: Deployment
 		},
 	},
 
+	//
+	// Test Case
+	//
+	{description: `remove scalar -- null in src, empty in dest`,
+		source: `
+kind: Deployment
+field: null
+`,
+		dest: `
+kind: Deployment
+field:
+`,
+		expected: `
+kind: Deployment
+`,
+		mergeOptions: yaml.MergeOptions{
+			ListIncreaseDirection: yaml.MergeOptionsListAppend,
+		},
+	},
+
+	//
+	// Test Case
+	//
+	{description: `remove scalar -- null in src, null in dest`,
+		source: `
+kind: Deployment
+field: null
+`,
+		dest: `
+kind: Deployment
+field: null
+`,
+		expected: `
+kind: Deployment
+`,
+		mergeOptions: yaml.MergeOptions{
+			ListIncreaseDirection: yaml.MergeOptionsListAppend,
+		},
+	},
+
+	//
+	// Test Case
+	//
+	{description: `keep scalar -- missing in src, null in dest`,
+		source: `
+kind: Deployment
+`,
+		dest: `
+kind: Deployment
+field: null
+`,
+		expected: `
+kind: Deployment
+field: null
+`,
+		mergeOptions: yaml.MergeOptions{
+			ListIncreaseDirection: yaml.MergeOptionsListAppend,
+		},
+	},
+
 	//
 	// Test Case
 	//

plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go

@@ -397,6 +397,7 @@ spec:
   replicas: 1
   template:
     metadata:
+      creationTimestamp: null
       labels:
         workload.sas.com/class: stateless
     spec: