Fix helm valuesInline merge failure when types differ

When using valuesInline with helm charts, kustomize would fail with
"wrong node kind" errors when the chart's values.yaml had a different
type than what the user provided. For example:
- Chart has `topologySpreadConstraints: {}` but user provides a list
- Chart has `githubConfigSecret: {github_token: ""}` but user provides
  a string

This adds an AllowKindChange option to MergeOptions that allows the
source value to completely replace the destination when their YAML
node kinds differ (e.g., map vs list, map vs scalar).

The HelmChartInflationGenerator now enables this option when merging
values, fixing the regression introduced in kustomize 5.4.0.

Fixes #5766
Fixes actions/actions-runner-controller#3819

Author: claude

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