From 7f2232c38360dbf61c048648bd4b112ae10265c3 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 15 Mar 2026 19:56:17 +0800 Subject: [PATCH] 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 --- pkg/app/app.go | 2 - pkg/app/app_list_test.go | 93 +++++++++++++++++++++++++++++++++++ pkg/state/chart_dependency.go | 11 ++++- pkg/state/state_test.go | 2 +- 4 files changed, 103 insertions(+), 5 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index 06a41f1a..b6fbb33b 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -4,7 +4,6 @@ import ( "bytes" goContext "context" "fmt" - "maps" "os" "path/filepath" "sort" @@ -712,7 +711,6 @@ func (a *App) list(run *Run) ([]*HelmRelease, error) { if r.Labels == nil { r.Labels = map[string]string{} } - maps.Copy(r.Labels, resolvedState.CommonLabels) var keys []string for k := range r.Labels { diff --git a/pkg/app/app_list_test.go b/pkg/app/app_list_test.go index cb0beea5..5f0838c9 100644 --- a/pkg/app/app_list_test.go +++ b/pkg/app/app_list_test.go @@ -366,3 +366,96 @@ generated: "2024-01-01T00:00:00Z" 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 +dependencies: +- name: redis + repository: https://charts.bitnami.com/bitnami + version: 17.0.7 + digest: sha256:abc123 + generated: "2024-01-01T00:00:00Z" +`, + "/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 +dependencies: +- name: nginx + repository: https://charts.bitnami.com/bitnami + version: 15.0.0 + digest: sha256:def456 + generated: "2024-01-01T00:00:00Z" +`, + } + + 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..8e4b83d6 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) diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index 5de789b9..f339b624 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -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