fix: array merge regression - layer arrays now replace defaults by aditmeno · Pull Request #2367 · helmfile/helmfile

aditmeno · 2026-01-16T21:27:47Z

Summary

Fixes #2353 - Array merge regression introduced by PR #2288.

PR #2288 introduced element-by-element array merging to fix #2281 (where --state-values-set array[1].field=value was replacing entire arrays). However, this caused a regression: layer/environment arrays were now being merged instead of replacing base arrays entirely, breaking helmfile's documented behavior.

Root Cause

The mergeSlices() function unconditionally performed element-by-element array merging, but there are two different use cases requiring different behaviors:

Use Case	Expected Behavior	Previous Behavior
`--state-values-set array[1].x=y`	Merge element-by-element	✅ Correct
Layer/Environment YAML arrays	Replace entirely	❌ Wrong (merged)

Solution: Two-Pronged Approach

1. Automatic Sparse Array Detection (ArrayMergeStrategySparse)

The key insight is that --state-values-set creates sparse arrays with nil values at unset indices, while layer YAML creates complete arrays without nils:

// CLI: --state-values-set array[1].field=x creates:
[]any{nil, map[string]any{"field": "x"}}  // Sparse - has nil

// Layer YAML: array: [a, b] creates:
[]any{"a", "b"}  // Complete - no nils

The heuristic:

If override array contains any nil values → sparse array (CLI) → merge element-by-element
If override array contains no nil values → complete array (layer) → replace entirely

Edge cases (documented):

Empty array [] has no nils, so it replaces entirely (intentional: explicitly setting empty clears the base)
Explicit [null, value] in YAML is treated as sparse (rare but correct behavior)

2. CLIOverrides Field for Explicit Tracking

Added a new CLIOverrides field to the Environment struct to explicitly track CLI values (--state-values-set, --state-values-file) separately from layer values. This allows using different merge strategies:

Layer values (e.Values): Use ArrayMergeStrategySparse (auto-detect based on nils)
CLI overrides (e.CLIOverrides): Use ArrayMergeStrategyMerge (always element-by-element)

This follows Helm's documented behavior where arrays replace rather than merge.

Changes

Core Implementation

File	Changes
`pkg/environment/environment.go`	Added `CLIOverrides` field; updated `New()`, `DeepCopy()`, `Merge()`, `GetMergedValues()`; initialized `EmptyEnvironment` with empty maps; keep CLIOverrides separate until final merge
`pkg/maputil/maputil.go`	Added `ArrayMergeStrategy` enum (`Sparse`, `Replace`, `Merge`); added `MergeOptions` struct; updated `mergeSlices()` with auto-detection and comprehensive documentation
`pkg/state/create.go`	Fixed merge order (current part overrides previous); propagate `Defaults` and `CLIOverrides` from context environment
`pkg/state/types.go`	Updated `NewEnvironmentTemplateData()` to use merged values for `.Environment.Values`, ensuring templates see CLI overrides
`pkg/app/desired_state_file_loader.go`	Put CLI values into `CLIOverrides` instead of `Values`

Tests

File	Changes
`pkg/maputil/maputil_test.go`	Added tests for sparse vs complete array detection
`pkg/environment/environment_test.go`	Added regression tests for `GetMergedValues()`
`pkg/state/storage_test.go`	Fixed flaky test (skip in non-CI due to SSH/HTTPS git config differences)
`test/integration/test-cases/issue-2353-layer-array-replace/`	E2E test for layer array replacement
`pkg/app/testdata/**`	Updated 147 snapshot files for new Environment log format

Key Fixes

Multi-Part Helmfile Support

Fixed issues with helmfiles containing multiple --- separated parts:

Defaults propagation: Base values: from earlier parts now properly inherited via Defaults field
Merge order: Later parts correctly override earlier parts (was reversed)

Template Access to CLI Values

Templates accessing .Environment.Values now see the same merged values as .Values:

NewEnvironmentTemplateData() creates an environment copy with pre-merged values
Both .Values and .Environment.Values return Defaults + Values + CLIOverrides

Environment Struct

type Environment struct {
    Name         string
    KubeContext  string
    Values       map[string]any      // Layer values (arrays replace via auto-detect)
    Defaults     map[string]any      // Base values from top-level values:
    CLIOverrides map[string]any      // CLI values (arrays merge element-by-element)
}

Value Merge Priority (in GetMergedValues)

Defaults (base, lowest priority)
Values (layer/environment values)
CLIOverrides (CLI flags, highest priority, element-by-element merge)

Test Plan

Unit tests pass for pkg/maputil (sparse vs complete array detection)
Unit tests pass for pkg/environment (GetMergedValues with both scenarios)
Unit tests pass for pkg/app (StateValueOverrides, EnvironmentValueOverrides, multi-part templates)
Unit tests pass for pkg/state
Integration test added for issue Issue 2281 introduced unexpected array merging in helmfile v1.2.1 #2353 scenario
Existing Unexpected behaviour when setting complex array values with --state-values-set #2281 fix preserved (sparse arrays still merge element-by-element)
All 147 snapshot tests updated and passing
Templates can access CLI values via both .Values and .Environment.Values

Backward Compatibility

✅ Fixes Issue 2281 introduced unexpected array merging in helmfile v1.2.1 #2353 (layer arrays now replace correctly)
✅ Preserves Unexpected behaviour when setting complex array values with --state-values-set #2281 fix (CLI sparse arrays still merge element-by-element)
✅ Multi-part helmfiles work correctly (Defaults propagation, merge order)
✅ Templates can access CLI values via .Environment.Values
✅ No user configuration needed - works automatically

Copilot Review Feedback Addressed

✅ Documented empty array [] edge case behavior
✅ Documented recursive strategy propagation for nested maps
✅ Added comprehensive function documentation for mergeSlices()

Copilot

Pull request overview

This PR fixes a regression introduced by #2288 where layer/environment arrays were incorrectly merged element-by-element instead of being replaced entirely, breaking Helmfile's documented behavior. The fix introduces automatic sparse array detection: arrays with nil values (from --state-values-set CLI) merge element-by-element, while complete arrays without nils (from YAML layers) replace entirely.

Changes:

Added auto-detection heuristic to distinguish sparse CLI arrays from complete layer arrays
Modified MergeMaps to accept optional merge strategy parameter (backward compatible)
Removed misleading comments that no longer accurately describe the behavior

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`pkg/maputil/maputil.go`	Added sparse array auto-detection in `mergeSlices()`, introduced `MergeOptions` with `ArrayMergeStrategy`
`pkg/maputil/maputil_test.go`	Added comprehensive tests for both sparse and complete array behaviors
`pkg/environment/environment.go`	Removed outdated comment in `GetMergedValues()`
`pkg/environment/environment_test.go`	Added regression tests for both #2353 (layer replace) and #2281 (sparse merge)
`pkg/state/create.go`	Removed outdated comment in `loadEnvValues()`
`pkg/state/storage_test.go`	Added CI-only skip for flaky git-dependent test
`test/integration/test-cases/issue-2353-layer-array-replace/`	Added E2E integration test for layer array replacement
`test/integration/run.sh`	Added new integration test to test suite

…file#2353) PR helmfile#2288 introduced element-by-element array merging to fix helmfile#2281, but this caused a regression where layer/environment arrays were merged instead of replacing base arrays entirely. This fix uses automatic sparse array detection: - Arrays with nil values (from --state-values-set) merge element-by-element - Arrays without nils (from layer YAML) replace entirely This follows Helm's documented behavior where arrays replace rather than merge. Signed-off-by: Aditya Menon <[email protected]>

Copilot

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.

…ging 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 helmfile#2281 fix is preserved (--state-values-set array[1].field=x works) Signed-off-by: Aditya Menon <[email protected]>

Signed-off-by: Aditya Menon <[email protected]>

Copilot

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.

- Add Defaults field merging from ctxEnv to preserve base values across helmfile parts separated by --- - Fix merge order: current part values now correctly override previous parts (was reversed, causing older values to win) - Update 147 snapshot test files for new Environment log format with CLIOverrides field This completes the fix for issue helmfile#2353 by ensuring: 1. Layer arrays replace entirely (not element-by-element merge) 2. CLI --state-values-set sparse arrays still merge element-by-element 3. Multi-part helmfiles properly inherit and override values Signed-off-by: Aditya Menon <[email protected]>

- Initialize EmptyEnvironment with empty maps to match New() constructor - Update test comment to accurately describe ArrayMergeStrategySparse Signed-off-by: Aditya Menon <[email protected]>

Copilot

Pull request overview

Copilot reviewed 160 out of 161 changed files in this pull request and generated 4 comments.

This commit fixes a regression in the CLIOverrides integration where templates accessing .Environment.Values couldn't see CLI override values. Changes: - Remove CLIOverrides-into-Values merge from Merge() to keep proper layering order (Defaults → Values → CLIOverrides) in GetMergedValues() - Update NewEnvironmentTemplateData to set envCopy.Values to the merged values, ensuring templates see the same values via both .Values and .Environment.Values This ensures: - Issue helmfile#2353: Layer arrays still replace entirely (Sparse strategy) - Issue helmfile#2281: CLI sparse arrays still merge element-by-element - Templates can access CLI overrides via .Environment.Values Signed-off-by: Aditya Menon <[email protected]>

Address Copilot review comments on PR helmfile#2367: - Document empty array edge case: explicitly setting [] clears base array - Document recursive strategy propagation for nested map merging - Add comprehensive behavior description for all array merge strategies Signed-off-by: Aditya Menon <[email protected]>

Copilot

Pull request overview

Copilot reviewed 161 out of 162 changed files in this pull request and generated no new comments.

Environment value files (*.yaml.gotmpl) can reference CLI values via .Values. Previously, only env.Values was passed to template rendering, which didn't include CLIOverrides. Now we call env.GetMergedValues() to get Defaults + Values + CLIOverrides before rendering, so templates can access CLI values like: --state-values-set foo=bar This fixes the state-values-set-cli-args-in-environments integration test. Signed-off-by: Aditya Menon <[email protected]>

Copilot

Pull request overview

Copilot reviewed 162 out of 163 changed files in this pull request and generated no new comments.

…by-element PR helmfile#2367 introduced CLIOverrides to give --state-values-set element-by-element array merge semantics. However, nested helmfile values (helmfiles[].values:) were also routed into CLIOverrides, causing their arrays to merge instead of replace. This broke the pre-v1.3.0 behavior where passing an array via helmfiles[].values: would fully replace the child's default array. Add OverrideValuesAreCLI flag to SubhelmfileEnvironmentSpec so the loader can distinguish CLI flags from nested helmfile values. CLI values continue using CLIOverrides (element-by-element merge); nested helmfile values now use Values (Sparse merge strategy → full array replacement). Fixes helmfile#2451

…by-element PR helmfile#2367 introduced CLIOverrides to give --state-values-set element-by-element array merge semantics. However, nested helmfile values (helmfiles[].values:) were also routed into CLIOverrides, causing their arrays to merge instead of replace. This broke the pre-v1.3.0 behavior where passing an array via helmfiles[].values: would fully replace the child's default array. Add OverrideValuesAreCLI flag to SubhelmfileEnvironmentSpec so the loader can distinguish CLI flags from nested helmfile values. CLI values continue using CLIOverrides (element-by-element merge); nested helmfile values now use Values (Sparse merge strategy → full array replacement). Fixes helmfile#2451 Signed-off-by: Aditya Menon <[email protected]>

…by-element (#2458) PR #2367 introduced CLIOverrides to give --state-values-set element-by-element array merge semantics. However, nested helmfile values (helmfiles[].values:) were also routed into CLIOverrides, causing their arrays to merge instead of replace. This broke the pre-v1.3.0 behavior where passing an array via helmfiles[].values: would fully replace the child's default array. Add OverrideValuesAreCLI flag to SubhelmfileEnvironmentSpec so the loader can distinguish CLI flags from nested helmfile values. CLI values continue using CLIOverrides (element-by-element merge); nested helmfile values now use Values (Sparse merge strategy → full array replacement). Fixes #2451 Signed-off-by: Aditya Menon <[email protected]>

…be dropped (#2527) PR #2367 introduced envCopy.Values = values in NewEnvironmentTemplateData, where values = GetMergedValues() = Defaults + Values + CLIOverrides. This caused .Environment.Values to include Defaults, so when multi-part helmfiles re-assigned environment values via {{ toYaml .Environment.Values }}, Defaults values (e.g. helmDefaults.atomic: true) were written into the environment Values field. Later, GetMergedValues() applied Values over Defaults, causing the stale atomic: true to win over the correct atomic: false override. Fix: set .Environment.Values to Values + CLIOverrides only (excluding Defaults), so re-assignment patterns don't pollute the Values layer with Defaults. Signed-off-by: yxx <[email protected]> Signed-off-by: yxxhero <[email protected]>

…) (#2532) * fix: environment values pollution causing boolean false overrides to be dropped (#2527) PR #2367 introduced envCopy.Values = values in NewEnvironmentTemplateData, where values = GetMergedValues() = Defaults + Values + CLIOverrides. This caused .Environment.Values to include Defaults, so when multi-part helmfiles re-assigned environment values via {{ toYaml .Environment.Values }}, Defaults values (e.g. helmDefaults.atomic: true) were written into the environment Values field. Later, GetMergedValues() applied Values over Defaults, causing the stale atomic: true to win over the correct atomic: false override. Fix: set .Environment.Values to Values + CLIOverrides only (excluding Defaults), so re-assignment patterns don't pollute the Values layer with Defaults. Signed-off-by: yxx <[email protected]> Signed-off-by: yxxhero <[email protected]> * fix: rename test to correctly reflect Values override Defaults precedence Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/1b251877-7050-404b-8cc7-abd6aa3ec36b Co-authored-by: yxxhero <[email protected]> * test: flip regression test fixture to exercise false override (issue #2527) Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/c428fd46-b698-4e88-bff2-4c9ac72d2deb Co-authored-by: yxxhero <[email protected]> --------- Signed-off-by: yxx <[email protected]> Signed-off-by: yxxhero <[email protected]> Co-authored-by: copilot-swe-agent[bot] <[email protected]>

aditmeno requested review from itscaro, mumoshu, xiaomudk, yxxhero and zhaque44 as code owners January 16, 2026 21:27

aditmeno requested a review from Copilot January 16, 2026 21:28

aditmeno self-assigned this Jan 16, 2026

Copilot started reviewing on behalf of aditmeno January 16, 2026 21:28 View session

aditmeno force-pushed the fix/issue-2353-array-merge-regression branch from f6aee45 to d115008 Compare January 16, 2026 21:32

Copilot AI reviewed Jan 16, 2026

View reviewed changes

Comment thread pkg/environment/environment.go

Comment thread pkg/maputil/maputil.go Outdated

Comment thread pkg/state/storage_test.go Outdated

aditmeno force-pushed the fix/issue-2353-array-merge-regression branch from d115008 to 0b25494 Compare January 17, 2026 01:56

aditmeno force-pushed the fix/issue-2353-array-merge-regression branch from 0b25494 to 5355ee4 Compare January 17, 2026 01:58

aditmeno requested a review from Copilot January 17, 2026 01:58

Copilot started reviewing on behalf of aditmeno January 17, 2026 01:58 View session