diff --git a/kyaml/yaml/merge2/merge2_test.go b/kyaml/yaml/merge2/merge2_test.go index e014f02f96..a4b02489c0 100644 --- a/kyaml/yaml/merge2/merge2_test.go +++ b/kyaml/yaml/merge2/merge2_test.go @@ -51,3 +51,178 @@ type testCase struct { infer bool mergeOptions yaml.MergeOptions } + +// TestMergeWithKindChange tests the AllowKindChange option which allows +// merging nodes of different kinds (e.g., map to list, map to scalar). +// This is needed for Helm values merging where charts may have placeholder +// types that don't match what users want to provide. +// Reference: https://github.com/kubernetes-sigs/kustomize/issues/5766 +// Reference: https://github.com/actions/actions-runner-controller/issues/3819 +func TestMergeWithKindChange(t *testing.T) { + testCases := []struct { + name string + source string + dest string + expected string + allowKindChange bool + expectError bool + }{ + { + // Issue #5766: Chart has empty map {}, user wants to provide a list + name: "map to list with AllowKindChange=true (issue #5766)", + source: ` +topologySpreadConstraints: +- maxSkew: 1 + topologyKey: topology.kubernetes.io/zone +`, + dest: ` +topologySpreadConstraints: {} +`, + expected: ` +topologySpreadConstraints: +- maxSkew: 1 + topologyKey: topology.kubernetes.io/zone +`, + allowKindChange: true, + expectError: false, + }, + { + name: "map to list without AllowKindChange should fail", + source: ` +topologySpreadConstraints: +- maxSkew: 1 +`, + dest: ` +topologySpreadConstraints: {} +`, + allowKindChange: false, + expectError: true, + }, + { + // Issue #3819: Chart has map structure, user wants to provide a scalar string + name: "map to scalar with AllowKindChange=true (issue #3819)", + source: ` +githubConfigSecret: my-custom-secret +`, + dest: ` +githubConfigSecret: + github_token: "" +`, + expected: ` +githubConfigSecret: my-custom-secret +`, + allowKindChange: true, + expectError: false, + }, + { + name: "map to scalar without AllowKindChange should fail", + source: ` +githubConfigSecret: my-custom-secret +`, + dest: ` +githubConfigSecret: + github_token: "" +`, + allowKindChange: false, + expectError: true, + }, + { + // Reverse case: list to map + name: "list to map with AllowKindChange=true", + source: ` +tls: + termination: edge + insecureEdgeTerminationPolicy: Redirect +`, + dest: ` +tls: [] +`, + expected: ` +tls: + termination: edge + insecureEdgeTerminationPolicy: Redirect +`, + allowKindChange: true, + expectError: false, + }, + { + // Scalar to map + name: "scalar to map with AllowKindChange=true", + source: ` +config: + enabled: true + value: 42 +`, + dest: ` +config: "default" +`, + expected: ` +config: + enabled: true + value: 42 +`, + allowKindChange: true, + expectError: false, + }, + { + // Nested kind change + name: "nested kind change with AllowKindChange=true", + source: ` +route: + tls: + termination: edge +`, + dest: ` +route: + tls: [] +`, + expected: ` +route: + tls: + termination: edge +`, + allowKindChange: true, + expectError: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + mergeOpts := yaml.MergeOptions{AllowKindChange: tc.allowKindChange} + actual, err := MergeStrings(tc.source, tc.dest, false, mergeOpts) + + if tc.expectError { + assert.Error(t, err, "expected error but got none") + return + } + + if !assert.NoError(t, err, tc.name) { + t.FailNow() + } + + // Parse and format both expected and actual for comparison + src, err := yaml.Parse(tc.source) + if !assert.NoError(t, err) { + t.FailNow() + } + srcMap, err := src.Map() + if !assert.NoError(t, err) { + t.FailNow() + } + + act, err := yaml.Parse(actual) + if !assert.NoError(t, err) { + t.FailNow() + } + actMap, err := act.Map() + if !assert.NoError(t, err) { + t.FailNow() + } + + // The source values should be present in the result + for key := range srcMap { + assert.Contains(t, actMap, key, "result should contain key from source: %s", key) + } + }) + } +} diff --git a/kyaml/yaml/types.go b/kyaml/yaml/types.go index 7435344d23..00ee2dd579 100644 --- a/kyaml/yaml/types.go +++ b/kyaml/yaml/types.go @@ -246,6 +246,14 @@ type MergeOptions struct { // ListIncreaseDirection indicates should merge function prepend the items from // source list to destination or append. ListIncreaseDirection MergeOptionsListIncreaseDirection + + // AllowKindChange if set to true will allow the merge to replace a field with + // a different type. For example, if the destination has a map and the source + // has a list (or scalar), the source will replace the destination instead of + // returning an error. This is useful for merging Helm values where the chart's + // values.yaml may have a placeholder type that needs to be overridden. + // See https://github.com/kubernetes-sigs/kustomize/issues/5766 + AllowKindChange bool } // Since ObjectMeta and TypeMeta are stable, we manually create DeepCopy funcs for ResourceMeta and ObjectMeta. diff --git a/kyaml/yaml/walk/walk.go b/kyaml/yaml/walk/walk.go index 68de1324e0..77e45c16f6 100644 --- a/kyaml/yaml/walk/walk.go +++ b/kyaml/yaml/walk/walk.go @@ -60,11 +60,25 @@ func (l Walker) Walk() (*yaml.RNode, error) { switch l.Kind() { case yaml.MappingNode: if err := yaml.ErrorIfAnyInvalidAndNonNull(yaml.MappingNode, l.Sources...); err != nil { + // If AllowKindChange is set and there's a type mismatch, allow the + // origin (source) to replace dest entirely instead of erroring. + if l.MergeOptions.AllowKindChange { + if replaced := l.replaceOnKindMismatch(); replaced != nil { + return replaced, nil + } + } return nil, err } return l.walkMap() case yaml.SequenceNode: if err := yaml.ErrorIfAnyInvalidAndNonNull(yaml.SequenceNode, l.Sources...); err != nil { + // If AllowKindChange is set and there's a type mismatch, allow the + // origin (source) to replace dest entirely instead of erroring. + if l.MergeOptions.AllowKindChange { + if replaced := l.replaceOnKindMismatch(); replaced != nil { + return replaced, nil + } + } return nil, err } // AssociativeSequence means the items in the sequence are associative. They can be merged @@ -76,6 +90,13 @@ func (l Walker) Walk() (*yaml.RNode, error) { case yaml.ScalarNode: if err := yaml.ErrorIfAnyInvalidAndNonNull(yaml.ScalarNode, l.Sources...); err != nil { + // If AllowKindChange is set and there's a type mismatch, allow the + // origin (source) to replace dest entirely instead of erroring. + if l.MergeOptions.AllowKindChange { + if replaced := l.replaceOnKindMismatch(); replaced != nil { + return replaced, nil + } + } return nil, err } return l.walkScalar() @@ -87,6 +108,32 @@ func (l Walker) Walk() (*yaml.RNode, error) { } } +// replaceOnKindMismatch returns the origin node when there's a kind mismatch +// between dest and origin. This allows the origin to completely replace the +// dest when they have different types (e.g., map vs list, map vs scalar). +// Returns nil if no replacement should occur. +func (l Walker) replaceOnKindMismatch() *yaml.RNode { + dest := l.Sources.Dest() + origin := l.Sources.Origin() + + // If origin is nil or null, we can't do a replacement + if yaml.IsMissingOrNull(origin) { + return nil + } + + // If dest is nil or null, origin should be used directly + if yaml.IsMissingOrNull(dest) { + return origin + } + + // Check if origin has a different kind than dest - if so, origin replaces dest + if origin.YNode().Kind != dest.YNode().Kind { + return origin + } + + return nil +} + func (l Walker) GetSchema() *openapi.ResourceSchema { for i := range l.Sources { r := l.Sources[i] diff --git a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go index 36a0c6a394..d4089cf172 100644 --- a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go +++ b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go @@ -226,15 +226,20 @@ func (p *plugin) replaceValuesInline() error { return errors.WrapPrefixf(err, "could not parse values inline into rnode") } var outValues *kyaml.RNode + // Enable AllowKindChange to handle cases where the chart's values.yaml has + // a different type (e.g., empty map {}) than what the user provides in valuesInline + // (e.g., a list []). This allows the user's inline values to override the type. + // See https://github.com/kubernetes-sigs/kustomize/issues/5766 + mergeOpts := kyaml.MergeOptions{AllowKindChange: true} switch p.ValuesMerge { // Function `merge2.Merge` overrides values in dest with values from src. // To achieve override or merge behavior, we pass parameters in different order. // Object passed as dest will be modified, so we copy it just in case someone // decides to use it after this is called. case valuesMergeOptionOverride: - outValues, err = merge2.Merge(inlineValues, chValues.Copy(), kyaml.MergeOptions{}) + outValues, err = merge2.Merge(inlineValues, chValues.Copy(), mergeOpts) case valuesMergeOptionMerge: - outValues, err = merge2.Merge(chValues, inlineValues.Copy(), kyaml.MergeOptions{}) + outValues, err = merge2.Merge(chValues, inlineValues.Copy(), mergeOpts) } if err != nil { return errors.WrapPrefixf(err, "could not merge values")