From df01afbbeb8c04845cba11a71079216518180588 Mon Sep 17 00:00:00 2001 From: yxxhero <11087727+yxxhero@users.noreply.github.com> Date: Thu, 19 Mar 2026 14:25:03 +0800 Subject: [PATCH] fix: helmfile list now reflects version from helmfile.lock (#2486) * fix: helmfile list now reflects version from helmfile.lock The list command now resolves locked dependencies before returning release information, ensuring the version field reflects the pinned version from helmfile.lock when present. Fixes #1953 Signed-off-by: yxxhero * fix: address PR review comments - Remove redundant maps.Copy in list() - labels already merged by GetReleasesWithLabels() - Fix default lockfile path to use basePath for multi-file mode - Update test to expect basePath-joined lockfile path - Add multi-file test for lockfile resolution in helmfile.d directory Signed-off-by: yxxhero * fix more test Signed-off-by: yxxhero * fix tests Signed-off-by: yxxhero * fix: propagate errors instead of panic in list() When skipCharts=false, errors from list() now properly propagate instead of causing a crash. Uses a closure variable to capture the error and propagates it after withPreparedCharts completes. Signed-off-by: yxxhero * fix tests Signed-off-by: yxxhero * fix tests Signed-off-by: yxxhero * fix tests Signed-off-by: yxxhero --------- Signed-off-by: yxxhero --- pkg/app/app.go | 37 ++++---- pkg/app/app_list_test.go | 158 ++++++++++++++++++++++++++++++++++ pkg/state/chart_dependency.go | 20 ++++- pkg/state/state_test.go | 10 +-- 4 files changed, 199 insertions(+), 26 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index 10a232b9..a93ced24 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -640,26 +640,27 @@ func (a *App) ListReleases(c ListConfigProvider) error { err := a.ForEachState(func(run *Run) (_ bool, errs []error) { var stateReleases []*HelmRelease - var err error + var listErr error if !c.SkipCharts() { - err = run.withPreparedCharts("list", state.ChartPrepareOptions{ + prepErr := run.withPreparedCharts("list", state.ChartPrepareOptions{ SkipRepos: true, SkipDeps: true, Concurrency: 2, }, func() { - rel, err := a.list(run) - if err != nil { - panic(err) - } - stateReleases = rel + stateReleases, listErr = a.list(run) }) + if prepErr != nil { + errs = append(errs, prepErr) + } + if listErr != nil { + errs = append(errs, listErr) + } } else { - stateReleases, err = a.list(run) - } - - if err != nil { - errs = append(errs, err) + stateReleases, listErr = a.list(run) + if listErr != nil { + errs = append(errs, listErr) + } } if len(stateReleases) > 0 { @@ -701,14 +702,16 @@ func (a *App) ListReleases(c ListConfigProvider) error { func (a *App) list(run *Run) ([]*HelmRelease, error) { var releases []*HelmRelease - for _, r := range run.state.Releases { + resolvedState, err := run.state.ResolveDeps() + if err != nil { + return nil, fmt.Errorf("unable to resolve dependencies for %s: %w", run.state.FilePath, err) + } + + for _, r := range resolvedState.Releases { labels := "" if r.Labels == nil { r.Labels = map[string]string{} } - for k, v := range run.state.CommonLabels { - r.Labels[k] = v - } var keys []string for k := range r.Labels { @@ -722,7 +725,7 @@ func (a *App) list(run *Run) ([]*HelmRelease, error) { } labels = strings.Trim(labels, ",") - enabled, err := state.ConditionEnabled(r, run.state.Values()) + enabled, err := state.ConditionEnabled(r, resolvedState.Values()) if err != nil { return nil, err } diff --git a/pkg/app/app_list_test.go b/pkg/app/app_list_test.go index f8084682..9fbb4644 100644 --- a/pkg/app/app_list_test.go +++ b/pkg/app/app_list_test.go @@ -2,6 +2,7 @@ package app import ( "bytes" + "encoding/json" "os" "testing" @@ -301,3 +302,160 @@ func TestListWithJSONOutput(t *testing.T) { testListWithJSONOutput(t, configImpl{skipCharts: true}) }) } + +func TestListWithLockFileVersion(t *testing.T) { + files := map[string]string{ + "/path/to/helmfile.yaml": ` +repositories: +- name: bitnami + url: https://charts.bitnami.com/bitnami + +releases: +- name: redis + namespace: default + chart: bitnami/redis + version: ">=1.0.0" +`, + "/path/to/helmfile.lock": `version: v0.0.0 +digest: sha256:abc123 +generated: "2024-01-01T00:00:00Z" +dependencies: +- name: redis + repository: https://charts.bitnami.com/bitnami + version: 17.0.7 +`, + } + + stdout := os.Stdout + defer func() { os.Stdout = stdout }() + + var buffer bytes.Buffer + syncWriter := testhelper.NewSyncWriter(&buffer) + logger := helmexec.NewLogger(syncWriter, "debug") + + valsRuntime, err := vals.New(vals.Options{CacheSize: 32}) + if err != nil { + t.Fatalf("unexpected error creating vals runtime: %v", err) + } + + app := appWithFs(&App{ + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + valsRuntime: valsRuntime, + }, files) + + expectNoCallsToHelm(app) + + out, err := testutil.CaptureStdout(func() { + err := app.ListReleases(configImpl{skipCharts: true, output: "json"}) + assert.Nil(t, err) + }) + assert.NoError(t, err) + + var releases []HelmRelease + if err := json.Unmarshal([]byte(out), &releases); err != nil { + t.Fatalf("failed to parse JSON output: %v", err) + } + + assert.Len(t, releases, 1, "expected 1 release") + assert.Equal(t, "redis", releases[0].Name) + assert.Equal(t, "bitnami/redis", releases[0].Chart) + assert.Equal(t, "17.0.7", releases[0].Version, "expected version from helmfile.lock") +} + +func TestListWithLockFileVersion_MultiFile(t *testing.T) { + files := map[string]string{ + "/path/to/helmfile.d/first.yaml": ` +repositories: +- name: bitnami + url: https://charts.bitnami.com/bitnami + +releases: +- name: redis + namespace: default + chart: bitnami/redis + version: ">=1.0.0" +`, + "/path/to/helmfile.d/first.lock": `version: v0.0.0 +digest: sha256:abc123 +generated: "2024-01-01T00:00:00Z" +dependencies: +- name: redis + repository: https://charts.bitnami.com/bitnami + version: 17.0.7 +`, + "/path/to/helmfile.d/second.yaml": ` +repositories: +- name: bitnami + url: https://charts.bitnami.com/bitnami + +releases: +- name: nginx + namespace: default + chart: bitnami/nginx + version: ">=1.0.0" +`, + "/path/to/helmfile.d/second.lock": `version: v0.0.0 +digest: sha256:def456 +generated: "2024-01-01T00:00:00Z" +dependencies: +- name: nginx + repository: https://charts.bitnami.com/bitnami + version: 15.0.0 +`, + } + + stdout := os.Stdout + defer func() { os.Stdout = stdout }() + + var buffer bytes.Buffer + syncWriter := testhelper.NewSyncWriter(&buffer) + logger := helmexec.NewLogger(syncWriter, "debug") + + valsRuntime, err := vals.New(vals.Options{CacheSize: 32}) + if err != nil { + t.Fatalf("unexpected error creating vals runtime: %v", err) + } + + app := appWithFs(&App{ + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + valsRuntime: valsRuntime, + }, files) + + expectNoCallsToHelm(app) + + out, err := testutil.CaptureStdout(func() { + err := app.ListReleases(configImpl{skipCharts: true, output: "json"}) + assert.Nil(t, err) + }) + assert.NoError(t, err) + + var releases []HelmRelease + if err := json.Unmarshal([]byte(out), &releases); err != nil { + t.Fatalf("failed to parse JSON output: %v", err) + } + + assert.Len(t, releases, 2, "expected 2 releases") + + releaseMap := make(map[string]HelmRelease) + for _, r := range releases { + releaseMap[r.Name] = r + } + + redis := releaseMap["redis"] + assert.Equal(t, "bitnami/redis", redis.Chart) + assert.Equal(t, "17.0.7", redis.Version, "expected redis version from first.lock") + + nginx := releaseMap["nginx"] + assert.Equal(t, "bitnami/nginx", nginx.Chart) + assert.Equal(t, "15.0.0", nginx.Version, "expected nginx version from second.lock") +} diff --git a/pkg/state/chart_dependency.go b/pkg/state/chart_dependency.go index f8ee0de3..60101c8a 100644 --- a/pkg/state/chart_dependency.go +++ b/pkg/state/chart_dependency.go @@ -140,8 +140,15 @@ func (st *HelmState) mergeLockedDependencies() (*HelmState, error) { // When basePath is set (e.g. when loaded with baseDir instead of os.Chdir), // resolve the lock file path relative to basePath so it can be found // without changing the working directory. - if lockFile != "" && st.basePath != "" && !filepath.IsAbs(lockFile) { - lockFile = filepath.Join(st.basePath, lockFile) + switch { + case lockFile != "": + if st.basePath != "" && !filepath.IsAbs(lockFile) { + lockFile = filepath.Join(st.basePath, lockFile) + } + case st.basePath != "": + // When no custom lockfile is specified, use the default lockfile name + // joined with basePath to ensure it's found when not changing CWD. + lockFile = filepath.Join(st.basePath, filename+".lock") } depMan := NewChartDependencyManager(filename, st.logger, lockFile) @@ -258,8 +265,13 @@ func getUnresolvedDependenciess(st *HelmState) (string, *UnresolvedDependencies) func updateDependencies(st *HelmState, shell helmexec.DependencyUpdater, unresolved *UnresolvedDependencies, filename, wd string) (*HelmState, error) { lockFile := st.LockFile - if lockFile != "" && st.basePath != "" && !filepath.IsAbs(lockFile) { - lockFile = filepath.Join(st.basePath, lockFile) + switch { + case lockFile != "": + if st.basePath != "" && !filepath.IsAbs(lockFile) { + lockFile = filepath.Join(st.basePath, lockFile) + } + case st.basePath != "": + lockFile = filepath.Join(st.basePath, filename+".lock") } depMan := NewChartDependencyManager(filename, st.logger, lockFile) diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index 5de789b9..b8e62573 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -2548,10 +2548,10 @@ generated: 2019-05-16T15:42:45.50486+09:00 } logger := helmexec.NewLogger(io.Discard, "debug") - basePath := "/src" + basePath := filepath.ToSlash(t.TempDir()) state := &HelmState{ basePath: basePath, - FilePath: "/src/helmfile.yaml", + FilePath: filepath.Join(basePath, "helmfile.yaml"), ReleaseSetSpec: ReleaseSetSpec{ Releases: []ReleaseSpec{ { @@ -2584,8 +2584,8 @@ generated: 2019-05-16T15:42:45.50486+09:00 } fs := testhelper.NewTestFs(map[string]string{ - "/example/Chart.yaml": `foo: FOO`, - "/src/example/Chart.yaml": `foo: FOO`, + "/example/Chart.yaml": `foo: FOO`, + filepath.Join(basePath, "example/Chart.yaml"): `foo: FOO`, }) fs.Cwd = basePath state = injectFs(state, fs) @@ -2648,7 +2648,7 @@ func TestHelmState_ResolveDeps_NoLockFile(t *testing.T) { logger: logger, fs: &filesystem.FileSystem{ ReadFile: func(f string) ([]byte, error) { - if f != "helmfile.lock" { + if f != filepath.Join("/src", "helmfile.lock") { return nil, fmt.Errorf("stub: unexpected file: %s", f) } return nil, os.ErrNotExist