Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge2: preserve explicitly set null values #4890

Merged
merged 1 commit into from
Nov 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions api/filters/patchstrategicmerge/patchstrategicmerge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
`,
},
}
Expand Down
10 changes: 8 additions & 2 deletions kyaml/yaml/merge2/merge2.go
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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 {
Expand Down Expand Up @@ -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() {
Expand Down
60 changes: 60 additions & 0 deletions kyaml/yaml/merge2/scalar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ spec:
replicas: 1
template:
metadata:
creationTimestamp: null
labels:
workload.sas.com/class: stateless
spec:
Expand Down