fix: use separate CLIOverrides field for element-by-element array merging

The previous approach using ArrayMergeStrategySparse detection didn't work
for --state-values-set array[0]=value because setting index 0 produces no
nils in the array.

This fix adds a CLIOverrides field to Environment that keeps CLI values
separate from layer values. CLI overrides are merged last using
ArrayMergeStrategyMerge (always element-by-element), while layer values
use the default strategy (arrays replace).

This ensures:
- --state-values-set array[0]=x only changes index 0, preserving other elements
- Layer/environment file arrays still replace base arrays entirely
- Issue #2281 fix is preserved (--state-values-set array[1].field=x works)

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

pkg/app/desired_state_file_loader.go

@@ -65,8 +65,8 @@ func (ld *desiredStateLoader) Load(f string, opts LoadOpts) (*state.HelmState, e
 		}
 
 		overrodeEnv = &environment.Environment{
-			Name:   ld.env,
-			Values: vals,
+			Name:         ld.env,
+			CLIOverrides: vals,
 		}
 	}
 

pkg/environment/environment.go

@@ -1,17 +1,16 @@
 package environment
 
 import (
-	"dario.cat/mergo"
-
 	"github.com/helmfile/helmfile/pkg/maputil"
 	"github.com/helmfile/helmfile/pkg/yaml"
 )
 
 type Environment struct {
-	Name        string
-	KubeContext string
-	Values      map[string]any
-	Defaults    map[string]any
+	Name         string
+	KubeContext  string
+	Values       map[string]any
+	Defaults     map[string]any
+	CLIOverrides map[string]any // CLI --state-values-set values, merged element-by-element
 }
 
 var EmptyEnvironment Environment
@@ -19,10 +18,11 @@ var EmptyEnvironment Environment
 // New return Environment with default name and values
 func New(name string) *Environment {
 	return &Environment{
-		Name:        name,
-		KubeContext: "",
-		Values:      map[string]any{},
-		Defaults:    map[string]any{},
+		Name:         name,
+		KubeContext:  "",
+		Values:       map[string]any{},
+		Defaults:     map[string]any{},
+		CLIOverrides: map[string]any{},
 	}
 }
 
@@ -53,11 +53,25 @@ func (e Environment) DeepCopy() Environment {
 		panic(err)
 	}
 
+	cliOverridesBytes, err := yaml.Marshal(e.CLIOverrides)
+	if err != nil {
+		panic(err)
+	}
+	var cliOverrides map[string]any
+	if err := yaml.Unmarshal(cliOverridesBytes, &cliOverrides); err != nil {
+		panic(err)
+	}
+	cliOverrides, err = maputil.CastKeysToStrings(cliOverrides)
+	if err != nil {
+		panic(err)
+	}
+
 	return Environment{
-		Name:        e.Name,
-		KubeContext: e.KubeContext,
-		Values:      values,
-		Defaults:    defaults,
+		Name:         e.Name,
+		KubeContext:  e.KubeContext,
+		Values:       values,
+		Defaults:     defaults,
+		CLIOverrides: cliOverrides,
 	}
 }
 
@@ -71,9 +85,19 @@ func (e *Environment) Merge(other *Environment) (*Environment, error) {
 	}
 	copy := e.DeepCopy()
 	if other != nil {
-		if err := mergo.Merge(&copy, other, mergo.WithOverride); err != nil {
-			return nil, err
+		// Merge scalar fields
+		if other.Name != "" {
+			copy.Name = other.Name
 		}
+		if other.KubeContext != "" {
+			copy.KubeContext = other.KubeContext
+		}
+		// Merge Values - layer values replace arrays
+		copy.Values = maputil.MergeMaps(copy.Values, other.Values)
+		copy.Defaults = maputil.MergeMaps(copy.Defaults, other.Defaults)
+		// Merge CLIOverrides using element-by-element array merging
+		copy.CLIOverrides = maputil.MergeMaps(copy.CLIOverrides, other.CLIOverrides,
+			maputil.MergeOptions{ArrayStrategy: maputil.ArrayMergeStrategyMerge})
 	}
 	return &copy, nil
 }
@@ -81,9 +105,11 @@ func (e *Environment) Merge(other *Environment) (*Environment, error) {
 func (e *Environment) GetMergedValues() (map[string]any, error) {
 	vals := map[string]any{}
 	vals = maputil.MergeMaps(vals, e.Defaults)
-	// Arrays in e.Values without nils (from YAML layers) replace e.Defaults arrays entirely.
-	// Sparse arrays with nils (from CLI --state-values-set) merge element-by-element.
 	vals = maputil.MergeMaps(vals, e.Values)
+	// CLI overrides are merged last using element-by-element array merging.
+	// This ensures --state-values-set array[0]=x only changes that index.
+	vals = maputil.MergeMaps(vals, e.CLIOverrides,
+		maputil.MergeOptions{ArrayStrategy: maputil.ArrayMergeStrategyMerge})
 
 	vals, err := maputil.CastKeysToStrings(vals)
 	if err != nil {

pkg/maputil/maputil.go

@@ -237,6 +237,11 @@ func MergeMaps(a, b map[string]interface{}, opts ...MergeOptions) map[string]int
 	}
 	for k, v := range b {
 		if v == nil {
+			// If key doesn't exist in base, add nil (issue #1154).
+			// If key exists in base, don't overwrite with nil.
+			if _, exists := out[k]; !exists {
+				out[k] = nil
+			}
 			continue
 		}
 		if v, ok := v.(map[string]interface{}); ok {

pkg/state/create.go

@@ -397,24 +397,28 @@ func (c *StateCreator) loadEnvValues(st *HelmState, name string, failOnMissingEn
 		return nil, &UndefinedEnvError{Env: name}
 	}
 
-	newEnv := &environment.Environment{Name: name, Values: valuesVals, KubeContext: envSpec.KubeContext}
+	newEnv := &environment.Environment{Name: name, Values: valuesVals, KubeContext: envSpec.KubeContext, CLIOverrides: map[string]any{}}
 
 	if ctxEnv != nil {
-		intCtxEnv := *ctxEnv
-
-		if err := mergo.Merge(&intCtxEnv, newEnv, mergo.WithOverride); err != nil {
-			return nil, fmt.Errorf("error while merging environment values for \"%s\": %v", name, err)
+		// Merge layer values (arrays replace)
+		newEnv.Values = maputil.MergeMaps(newEnv.Values, ctxEnv.Values)
+		// Copy CLI overrides to be merged at GetMergedValues time
+		newEnv.CLIOverrides = maputil.MergeMaps(newEnv.CLIOverrides, ctxEnv.CLIOverrides,
+			maputil.MergeOptions{ArrayStrategy: maputil.ArrayMergeStrategyMerge})
+		if ctxEnv.Name != "" {
+			newEnv.Name = ctxEnv.Name
+		}
+		if ctxEnv.KubeContext != "" {
+			newEnv.KubeContext = ctxEnv.KubeContext
 		}
-
-		newEnv = &intCtxEnv
 	}
 
 	if overrode != nil {
-		intOverrodeEnv := *newEnv
-		// CLI overrides always merge arrays element-by-element (ArrayMergeStrategyMerge)
-		intOverrodeEnv.Values = maputil.MergeMaps(intOverrodeEnv.Values, overrode.Values,
+		// Merge layer values from overrode (arrays replace)
+		newEnv.Values = maputil.MergeMaps(newEnv.Values, overrode.Values)
+		// Merge CLI overrides (arrays merge element-by-element)
+		newEnv.CLIOverrides = maputil.MergeMaps(newEnv.CLIOverrides, overrode.CLIOverrides,
 			maputil.MergeOptions{ArrayStrategy: maputil.ArrayMergeStrategyMerge})
-		newEnv = &intOverrodeEnv
 	}
 
 	return newEnv, nil