diff --git a/api/filters/patchstrategicmerge/patchstrategicmerge_test.go b/api/filters/patchstrategicmerge/patchstrategicmerge_test.go index 97cbee6c89..5bc15daa8b 100644 --- a/api/filters/patchstrategicmerge/patchstrategicmerge_test.go +++ b/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 `, }, } diff --git a/kyaml/yaml/merge2/merge2.go b/kyaml/yaml/merge2/merge2.go index ab0c8244c4..8b328ab9d6 100644 --- a/kyaml/yaml/merge2/merge2.go +++ b/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() { diff --git a/kyaml/yaml/merge2/scalar_test.go b/kyaml/yaml/merge2/scalar_test.go index 7d99871e88..a19394738f 100644 --- a/kyaml/yaml/merge2/scalar_test.go +++ b/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 // diff --git a/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go b/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go index bcffc03706..c16ef06ab7 100644 --- a/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go +++ b/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go @@ -397,6 +397,7 @@ spec: replicas: 1 template: metadata: + creationTimestamp: null labels: workload.sas.com/class: stateless spec: