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 <aiopsclub@163.com> * 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 <aiopsclub@163.com> * fix more test Signed-off-by: yxxhero <aiopsclub@163.com> * fix tests Signed-off-by: yxxhero <aiopsclub@163.com> * 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 <aiopsclub@163.com> * fix tests Signed-off-by: yxxhero <aiopsclub@163.com> * fix tests Signed-off-by: yxxhero <aiopsclub@163.com> * fix tests Signed-off-by: yxxhero <aiopsclub@163.com> --------- Signed-off-by: yxxhero <aiopsclub@163.com>
2026-03-19 14:25:03 +08:00 · 2026-03-19 14:25:03 +08:00 · df01afbbeb
parent 2ccd899399
commit df01afbbeb
4 changed files with 199 additions and 26 deletions
--- 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
 		}
--- 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")
+}
--- 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)
--- 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