fix: keep all chart dependencies key / values by champtar · Pull Request #2501 · helmfile/helmfile

champtar · 2026-03-24T15:58:47Z

In rewriteChartDependencies we were only parsing name / repository / version, thus dropping keys like condition / import-values.

This at least fixes the use of condition.

yxxhero · 2026-03-24T22:50:17Z

@champtar does any function use those field?

Copilot

Pull request overview

This PR updates Helmfile’s rewriteChartDependencies logic to avoid dropping dependency-level fields in Chart.yaml when rewriting relative file:// dependency paths, preserving keys like condition and import-values during the unmarshal/marshal roundtrip.

Changes:

Extend the ChartDependency parsing model to retain all dependency-level YAML keys via an inline map.
Stop modeling only a fixed subset of dependency fields so additional keys are not lost when rewriting repository.

champtar · 2026-03-24T23:17:26Z

@champtar does any function use those field?

I haven't tracked down the whole logic with chartify and helm, but condition is used somewhere for sure, and this is enough to fix one of our internal charts that look like

dependencies:
  - condition: global.dbConfig.inCluster
    name: mongodb
    repository: https://internal-repo/...
    version: 18.1.20-1
  - name: data-storage
    repository: "file://charts/data-storage"
    version: ">0.0.0-0"
    import-values:
      - child: persistence
        parent: global.persistence

yxxhero · 2026-03-24T23:20:39Z

@champtar is ready for reviewing?

champtar · 2026-03-25T15:27:47Z

@yxxhero now with an integration test, ready for review

champtar · 2026-03-25T17:01:08Z

Any ideas where this failure comes from ?

TEST: chart with dependencies and condition
in test/integration/test-cases/chart-deps-condition/input/helmfile.yaml: [exit status 1

COMMAND:
  helm template --debug=false --output-dir=/tmp/chartify835999929/helmfile-tests/testchartifyinstallfalse/helmx.1.rendered testchartifyinstallfalse /tmp/chartify835999929/helmfile-tests/testchartifyinstallfalse -f /tmp/chartify835999929/helmfile-tests/testchartifyinstallfalse/values.yaml -f /tmp/helmfile120344979/helmfile-tests-testchartifyinstallfalse-values-659d88cbbb --namespace helmfile-tests

OUTPUT:
  Error: validation: chart.metadata.name is required]
FAIL: "helmfile template" shouldn't fail
make: *** [Makefile:54: integration] Error 1

The chart / subcharts I'm adding clearly have names, helmfile template -f ... --skip-deps works fine locally

champtar · 2026-03-25T17:06:35Z

Actually if I run multiple time I get the error from time to time

champtar · 2026-03-25T17:17:39Z

helmfile is patching the local ./chart/Chart.yaml in rewriteChartDependencies (it should patch a unique copy), as we are using it 6 times, there is a race condition

champtar · 2026-03-25T18:33:32Z

Updated with --concurrency 1 + added another test, can someone restart the CI ?

champtar · 2026-03-26T02:47:48Z

@yxxhero 3rd time's a charm :) (ci is now green)

Signed-off-by: Etienne Champetier <[email protected]>

In rewriteChartDependencies we were only parsing name / repository / version, thus dropping keys like condition / import-values. This at least fixes the use of condition. Signed-off-by: Etienne Champetier <[email protected]>

yxxhero · 2026-03-26T03:22:12Z

@champtar got it.

champtar requested review from aditmeno, itscaro, mumoshu, xiaomudk, yxxhero and zhaque44 as code owners March 24, 2026 15:58

yxxhero force-pushed the fixup_dependencies branch from 1618547 to 825bd37 Compare March 24, 2026 22:49

yxxhero requested a review from Copilot March 24, 2026 22:49

Copilot started reviewing on behalf of yxxhero March 24, 2026 22:51 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

Comment thread pkg/state/state.go

champtar force-pushed the fixup_dependencies branch 3 times, most recently from f825321 to a52d431 Compare March 25, 2026 15:25

champtar force-pushed the fixup_dependencies branch from a52d431 to 4c8e311 Compare March 25, 2026 18:07

champtar mentioned this pull request Mar 25, 2026

helmfile race condition when using rewriteChartDependencies #2502

Closed

champtar force-pushed the fixup_dependencies branch from 4c8e311 to 73fb0ca Compare March 25, 2026 18:31

champtar force-pushed the fixup_dependencies branch 2 times, most recently from 47c15ba to 2f95618 Compare March 26, 2026 01:07

champtar added 2 commits March 26, 2026 11:22

feat: Refactor TestRewriteChartDependencies

9b460a6

Signed-off-by: Etienne Champetier <[email protected]>

fix: keep all chart dependencies key / values

373c82e

In rewriteChartDependencies we were only parsing name / repository / version, thus dropping keys like condition / import-values. This at least fixes the use of condition. Signed-off-by: Etienne Champetier <[email protected]>

yxxhero force-pushed the fixup_dependencies branch from 2f95618 to 373c82e Compare March 26, 2026 03:22

yxxhero approved these changes Mar 26, 2026

View reviewed changes

yxxhero merged commit 4bdb6f0 into helmfile:main Mar 26, 2026
16 checks passed

champtar deleted the fixup_dependencies branch March 26, 2026 11:33

BrewTestBot mentioned this pull request Apr 8, 2026

helmfile 1.4.4 Homebrew/homebrew-core#276569

Merged

Uh oh!

Uh oh!

Conversation

champtar commented Mar 24, 2026

Uh oh!

yxxhero commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

champtar commented Mar 24, 2026

Uh oh!

yxxhero commented Mar 24, 2026

Uh oh!

champtar commented Mar 25, 2026

Uh oh!

champtar commented Mar 25, 2026

Uh oh!

champtar commented Mar 25, 2026

Uh oh!

champtar commented Mar 25, 2026

Uh oh!

champtar commented Mar 25, 2026

Uh oh!

champtar commented Mar 26, 2026

Uh oh!

yxxhero commented Mar 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants