fix: nested helmfile values should replace arrays, not merge element-by-element by aditmeno · Pull Request #2458 · helmfile/helmfile

aditmeno · 2026-03-05T09:52:24Z

Problem

PR #2367 introduced CLIOverrides on the Environment struct to give --state-values-set element-by-element array merge semantics (so --state-values-set array[1].x=y only touches index 1). However, nested helmfile values (helmfiles[].values:) were also routed into CLIOverrides, causing their arrays to merge element-by-element instead of replacing entirely. This broke the pre-v1.3.0 behavior.

Reproducer (from #2451)

Parent passes a partial array to a child helmfile:

# parent helmfile.yaml
helmfiles:
  - path: child.yaml.gotmpl
    values:
      - list:
          - name: override1

Child has defaults with a two-element array:

# defaults.yaml
list:
  - name: default1
  - name: default2

Expected: The override fully replaces the array → only override1 appears.
Actual (bug): Element-by-element merge produces both override1 and default2.

Root Cause

In pkg/app/desired_state_file_loader.go:69, all OverrideValues are unconditionally placed into CLIOverrides:

overrodeEnv = &environment.Environment{
    Name:         ld.env,
    CLIOverrides: vals,  // ← wrong for nested helmfile values
}

Both CLI flags and nested helmfile values arrive through opts.Environment.OverrideValues, but need different merge semantics:

Source	Merge Strategy	Behavior
`--state-values-set` (CLI)	`CLIOverrides` → `ArrayMergeStrategyMerge`	Element-by-element
`helmfiles[].values:` (YAML)	`Values` → Sparse (default)	Full array replacement

Additionally, LoadOpts.DeepCopy() uses yaml.Marshal/yaml.Unmarshal, which drops the OverrideValuesAreCLI flag (tagged yaml:"-"), losing the discriminator during nested helmfile processing.

Fix

4 files changed, minimal diff:

1. `pkg/state/state.go` — Add discriminator flag

type SubhelmfileEnvironmentSpec struct {
    OverrideValues       []any `yaml:"values,omitempty"`
    OverrideValuesAreCLI bool  `yaml:"-"`  // never serialized
}

2. `pkg/app/app.go` — Mark CLI path

When override values originate from CLI flags (--state-values-file / --state-values-set), set the flag:

if len(envvals) > 0 {
    opts.Environment.OverrideValues = envvals
    opts.Environment.OverrideValuesAreCLI = true  // ← new
}

Nested helmfiles populate m.Environment from YAML parsing — the flag defaults to false.

3. `pkg/app/desired_state_file_loader.go` — Branch on flag

if opts.Environment.OverrideValuesAreCLI {
    overrodeEnv = &environment.Environment{
        Name:         ld.env,
        CLIOverrides: vals,       // element-by-element merge
    }
} else {
    overrodeEnv = &environment.Environment{
        Name:   ld.env,
        Values: vals,             // full array replacement
    }
}

4. `pkg/app/load_opts.go` — Preserve flag in `DeepCopy()`

DeepCopy() uses yaml.Marshal/yaml.Unmarshal which drops yaml:"-" fields. Manually copy the flag after unmarshal:

new.Environment.OverrideValuesAreCLI = o.Environment.OverrideValuesAreCLI

Why this works

In pkg/state/create.go:

Line 432: overrode.Values merges with default Sparse strategy → arrays replace
Line 434: overrode.CLIOverrides merges with ArrayMergeStrategyMerge → arrays merge element-by-element

By routing nested helmfile values into Values instead of CLIOverrides, they get array-replacement semantics. CLI values stay in CLIOverrides unchanged.

Integration Test

New test case modeled after the issue reporter's scenario. A parent helmfile passes a 1-element array override to a child whose defaults define a 2-element array. Verifies that the override fully replaces the default array (only override1 appears, not default2).

Testing

go build ./... — compiles cleanly
go test ./pkg/app/... ./pkg/environment/... ./pkg/state/... — all pass
golangci-lint run ./pkg/app/... — 0 issues

Copilot

Pull request overview

This PR fixes a regression introduced in PR #2367 where nested helmfile values: arrays were being merged element-by-element instead of fully replaced. The fix adds a discriminator flag OverrideValuesAreCLI to distinguish between CLI-originated overrides (which need element-by-element merge semantics) and nested helmfile values (which need full array replacement semantics).

Changes:

Added OverrideValuesAreCLI bool discriminator flag to SubhelmfileEnvironmentSpec and branched merge strategy accordingly in the loader
Preserved the flag through DeepCopy() and marked it at the CLI call site
Added an integration test reproducing the issue scenario

Reviewed changes

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

Show a summary per file

File	Description
`pkg/state/state.go`	Adds `OverrideValuesAreCLI` field (never serialized) to `SubhelmfileEnvironmentSpec`
`pkg/app/app.go`	Sets the new flag when override values originate from CLI arguments
`pkg/app/desired_state_file_loader.go`	Branches on the flag to route values into `CLIOverrides` vs `Values`
`pkg/app/load_opts.go`	Manually preserves the flag after `yaml.Marshal`/`yaml.Unmarshal` in `DeepCopy()`
`test/integration/run.sh`	Registers the new integration test case
`test/integration/test-cases/issue-2451-nested-helmfile-array-replace.sh`	Shell script running the new integration test
`test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/helmfile.yaml`	Parent helmfile for the integration test
`test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/child.yaml.gotmpl`	Child helmfile template for the integration test
`test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/defaults.yaml`	Default values file with a 2-element array
`test/integration/test-cases/issue-2451-nested-helmfile-array-replace/output/output.yaml`	Expected output for the integration test

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 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 Signed-off-by: Aditya Menon <[email protected]>

yxxhero · 2026-03-09T05:54:38Z

@aditmeno is ready for reviewing?

aditmeno · 2026-03-09T07:49:43Z

@aditmeno is ready for reviewing?

Almost, checking if copilot has any other suggestions

Copilot

Pull request overview

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

aditmeno requested review from itscaro, mumoshu, xiaomudk, yxxhero and zhaque44 as code owners March 5, 2026 09:52

aditmeno force-pushed the fix/issue-2451-nested-helmfile-array-replace branch 3 times, most recently from 2588a7b to 0bdd2cd Compare March 8, 2026 12:26

aditmeno requested a review from Copilot March 8, 2026 13:09

Copilot started reviewing on behalf of aditmeno March 8, 2026 13:10 View session

Copilot AI reviewed Mar 8, 2026

View reviewed changes

Comment thread test/integration/test-cases/issue-2451-nested-helmfile-array-replace.sh Outdated

aditmeno force-pushed the fix/issue-2451-nested-helmfile-array-replace branch from 0bdd2cd to 53c8592 Compare March 8, 2026 13:16

aditmeno requested a review from Copilot March 8, 2026 13:17

Copilot started reviewing on behalf of aditmeno March 8, 2026 13:17 View session

Copilot AI reviewed Mar 8, 2026

View reviewed changes

Comment thread pkg/app/load_opts.go

Comment thread test/integration/test-cases/issue-2451-nested-helmfile-array-replace.sh Outdated

aditmeno force-pushed the fix/issue-2451-nested-helmfile-array-replace branch from 53c8592 to 5b6b5d6 Compare March 8, 2026 16:01

aditmeno requested a review from Copilot March 9, 2026 07:49

Copilot started reviewing on behalf of aditmeno March 9, 2026 07:49 View session

Merge branch 'main' into fix/issue-2451-nested-helmfile-array-replace

6a5a358

Copilot AI reviewed Mar 9, 2026

View reviewed changes

yxxhero approved these changes Mar 9, 2026

View reviewed changes

yxxhero merged commit c375b48 into helmfile:main Mar 9, 2026
16 checks passed

aditmeno deleted the fix/issue-2451-nested-helmfile-array-replace branch March 9, 2026 14:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: nested helmfile values should replace arrays, not merge element-by-element#2458

fix: nested helmfile values should replace arrays, not merge element-by-element#2458
yxxhero merged 2 commits into
helmfile:mainfrom
aditmeno:fix/issue-2451-nested-helmfile-array-replace

aditmeno commented Mar 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

yxxhero commented Mar 9, 2026

Uh oh!

aditmeno commented Mar 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

aditmeno commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Reproducer (from #2451)

Root Cause

Fix

1. pkg/state/state.go — Add discriminator flag

2. pkg/app/app.go — Mark CLI path

3. pkg/app/desired_state_file_loader.go — Branch on flag

4. pkg/app/load_opts.go — Preserve flag in DeepCopy()

Why this works

Integration Test

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

yxxhero commented Mar 9, 2026

Uh oh!

aditmeno commented Mar 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aditmeno commented Mar 5, 2026 •

edited

Loading

1. `pkg/state/state.go` — Add discriminator flag

2. `pkg/app/app.go` — Mark CLI path

3. `pkg/app/desired_state_file_loader.go` — Branch on flag

4. `pkg/app/load_opts.go` — Preserve flag in `DeepCopy()`