Fix helm valuesInline merge failure when types differ

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)
+			}
+		})
+	}
+}

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.

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]

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")