diff --git a/chartify.go b/chartify.go index ec13432..b174408 100644 --- a/chartify.go +++ b/chartify.go @@ -382,13 +382,30 @@ func (r *Runner) Chartify(release, dirOrChart string, opts ...ChartifyOption) (s "This may result in outdated chart dependencies.", release) } else { // Flatten the chart by fetching dependent chart archives and merging their K8s manifests into the temporary local chart - // So that we can uniformly patch them with JSON patch, Strategic-Merge patch, or with injectors - depArgs := []string{"dependency", "up", tempDir} + // so that we can uniformly patch them with JSON patch, Strategic-Merge patch, or with injectors. + // + // Use `helm dependency build` (honors Chart.lock) when a lock exists and no adhoc deps + // were injected; otherwise fall back to `helm dependency up` (re-resolves from Chart.yaml). + hasAdhocDeps := len(u.AdhocChartDependencies) > 0 || len(u.DeprecatedAdhocChartDependencies) > 0 + useBuild := !hasAdhocDeps && hasChartLock(tempDir) + depCmd := "up" + if useBuild { + depCmd = "build" + } + depArgs := []string{"dependency", depCmd, tempDir} // Helm 4 requires --plain-http for HTTP-only OCI registries if u.OCIPlainHTTP && r.IsHelm4() { depArgs = append(depArgs, "--plain-http") } _, err := r.run(nil, r.helmBin(), depArgs...) + if err != nil && useBuild && isLockOutOfSyncErr(err) { + // `helm dependency build` errors when Chart.lock is out of sync with Chart.yaml. + // Only fall back to `up` for this specific case — other errors (network, auth, + // missing artifacts) should surface to the caller rather than silently re-resolving. + r.Logf("`helm dependency build` failed for release %s (lock out of sync), falling back to `helm dependency up`: %v", release, err) + depArgs[1] = "up" + _, err = r.run(nil, r.helmBin(), depArgs...) + } if err != nil { return "", err } @@ -680,6 +697,27 @@ func (r *Runner) RewriteChartToPreventDoubleRendering(tempDir, filesDir string) return nil } +// hasChartLock reports whether a Chart.lock or requirements.lock file exists in the +// given chart directory. Helm uses Chart.lock for apiVersion v2 charts and the legacy +// requirements.lock for v1 charts; either is sufficient for `helm dependency build`. +func hasChartLock(chartDir string) bool { + for _, name := range []string{"Chart.lock", "requirements.lock"} { + if _, err := os.Stat(filepath.Join(chartDir, name)); err == nil { + return true + } + } + return false +} + +// isLockOutOfSyncErr returns true when the error from `helm dependency build` indicates +// that the lock file is out of sync with Chart.yaml. Only this specific failure is safe +// to recover from by falling back to `helm dependency up`. +// Matches messages from Helm 3.x ("out of sync") and Helm 2.x ("lock file is out of date"). +func isLockOutOfSyncErr(err error) bool { + msg := err.Error() + return strings.Contains(msg, "out of sync") || strings.Contains(msg, "lock file is out of date") +} + func createDirForFile(f string) error { dstFileDir := filepath.Dir(f) if _, err := os.Lstat(dstFileDir); err == nil { diff --git a/chartify_test.go b/chartify_test.go index b78a9d1..1a559f2 100644 --- a/chartify_test.go +++ b/chartify_test.go @@ -2,8 +2,11 @@ package chartify import ( "context" + "fmt" + "io" "os" "os/exec" + "path/filepath" "testing" "github.com/google/go-cmp/cmp" @@ -114,6 +117,179 @@ func TestReadAdhocDependencies(t *testing.T) { }) } +func TestDepCommandSelection(t *testing.T) { + newTestRunner := func(failBuild bool, failMsg string) (*Runner, *[]helmCall) { + var calls []helmCall + r := &Runner{ + HelmBinary: "helm", + isHelm3: true, + RunCommand: func(name string, args []string, dir string, stdout, stderr io.Writer, env map[string]string) error { + calls = append(calls, helmCall{name: name, args: append([]string{}, args...)}) + if failBuild && len(args) >= 2 && args[0] == "dependency" && args[1] == "build" { + if _, err := stderr.Write([]byte(failMsg)); err != nil { + return err + } + return fmt.Errorf("%s", failMsg) + } + return nil + }, + CopyFile: CopyFile, + WriteFile: os.WriteFile, + ReadFile: os.ReadFile, + ReadDir: os.ReadDir, + Walk: filepath.Walk, + Exists: exists, + Logf: func(string, ...interface{}) {}, + MakeTempDir: func(release, chart string, opts *ChartifyOpts) string { + return "" + }, + } + return r, &calls + } + + setupChart := func(t *testing.T, withLock bool, lockName string) string { + t.Helper() + dir := t.TempDir() + chartYaml := filepath.Join(dir, "Chart.yaml") + if err := os.WriteFile(chartYaml, []byte("apiVersion: v2\nname: test\nversion: 0.1.0\n"), 0644); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(dir, "templates"), 0755); err != nil { + t.Fatal(err) + } + if withLock { + if err := os.WriteFile(filepath.Join(dir, lockName), []byte("dependencies: []\n"), 0644); err != nil { + t.Fatal(err) + } + } + return dir + } + + t.Run("uses dependency build when Chart.lock exists and no adhoc deps", func(t *testing.T) { + chartDir := setupChart(t, true, "Chart.lock") + r, calls := newTestRunner(false, "") + r.MakeTempDir = func(_, _ string, _ *ChartifyOpts) string { return chartDir } + + _, err := r.Chartify("rel", chartDir, WithChartifyOpts(&ChartifyOpts{})) + require.NoError(t, err) + + depCalls := filterDepCalls(*calls) + require.Len(t, depCalls, 1) + require.Equal(t, "build", depCalls[0].args[1]) + }) + + t.Run("uses dependency up when no lock file exists", func(t *testing.T) { + chartDir := setupChart(t, false, "") + r, calls := newTestRunner(false, "") + r.MakeTempDir = func(_, _ string, _ *ChartifyOpts) string { return chartDir } + + _, err := r.Chartify("rel", chartDir, WithChartifyOpts(&ChartifyOpts{})) + require.NoError(t, err) + + depCalls := filterDepCalls(*calls) + require.Len(t, depCalls, 1) + require.Equal(t, "up", depCalls[0].args[1]) + }) + + t.Run("uses dependency up when AdhocChartDependencies present despite lock", func(t *testing.T) { + chartDir := setupChart(t, true, "Chart.lock") + r, calls := newTestRunner(false, "") + r.MakeTempDir = func(_, _ string, _ *ChartifyOpts) string { return chartDir } + r.Exists = func(path string) (bool, error) { + if _, err := os.Stat(path); err == nil { + return true, nil + } + return false, nil + } + + _, err := r.Chartify("rel", chartDir, WithChartifyOpts(&ChartifyOpts{ + AdhocChartDependencies: []ChartDependency{{Chart: chartDir, Version: "0.1.0"}}, + })) + require.NoError(t, err) + + depCalls := filterDepCalls(*calls) + require.Len(t, depCalls, 1) + require.Equal(t, "up", depCalls[0].args[1]) + }) + + t.Run("uses dependency up when DeprecatedAdhocChartDependencies present despite lock", func(t *testing.T) { + chartDir := setupChart(t, true, "Chart.lock") + r, calls := newTestRunner(false, "") + r.MakeTempDir = func(_, _ string, _ *ChartifyOpts) string { return chartDir } + r.Exists = func(path string) (bool, error) { + if _, err := os.Stat(path); err == nil { + return true, nil + } + return false, nil + } + + _, err := r.Chartify("rel", chartDir, WithChartifyOpts(&ChartifyOpts{ + DeprecatedAdhocChartDependencies: []string{chartDir + ":0.1.0"}, + })) + require.NoError(t, err) + + depCalls := filterDepCalls(*calls) + require.Len(t, depCalls, 1) + require.Equal(t, "up", depCalls[0].args[1]) + }) + + t.Run("falls back to up when build fails with lock out of sync", func(t *testing.T) { + chartDir := setupChart(t, true, "Chart.lock") + r, calls := newTestRunner(true, "the lock file (Chart.lock) is out of sync with the dependencies listed in Chart.yaml") + r.MakeTempDir = func(_, _ string, _ *ChartifyOpts) string { return chartDir } + + _, err := r.Chartify("rel", chartDir, WithChartifyOpts(&ChartifyOpts{})) + require.NoError(t, err) + + depCalls := filterDepCalls(*calls) + require.Len(t, depCalls, 2) + require.Equal(t, "build", depCalls[0].args[1]) + require.Equal(t, "up", depCalls[1].args[1]) + }) + + t.Run("does not fall back to up when build fails with non-sync error", func(t *testing.T) { + chartDir := setupChart(t, true, "Chart.lock") + r, calls := newTestRunner(true, "network timeout fetching dependency") + r.MakeTempDir = func(_, _ string, _ *ChartifyOpts) string { return chartDir } + + _, err := r.Chartify("rel", chartDir, WithChartifyOpts(&ChartifyOpts{})) + require.Error(t, err) + require.Contains(t, err.Error(), "network timeout") + + depCalls := filterDepCalls(*calls) + require.Len(t, depCalls, 1) + require.Equal(t, "build", depCalls[0].args[1]) + }) + + t.Run("uses requirements.lock for legacy charts", func(t *testing.T) { + chartDir := setupChart(t, true, "requirements.lock") + r, calls := newTestRunner(false, "") + r.MakeTempDir = func(_, _ string, _ *ChartifyOpts) string { return chartDir } + + _, err := r.Chartify("rel", chartDir, WithChartifyOpts(&ChartifyOpts{})) + require.NoError(t, err) + + depCalls := filterDepCalls(*calls) + require.Len(t, depCalls, 1) + require.Equal(t, "build", depCalls[0].args[1]) + }) +} + +type helmCall struct { + name string + args []string +} + +func filterDepCalls(calls []helmCall) []helmCall { + var result []helmCall + for _, c := range calls { + if len(c.args) >= 2 && c.args[0] == "dependency" { + result = append(result, c) + } + } + return result +} + func TestUseHelmChartsInKustomize(t *testing.T) { repo := "myrepo" startServer(t, repo) diff --git a/util_test.go b/util_test.go index 6a944c4..8f171e8 100644 --- a/util_test.go +++ b/util_test.go @@ -1,11 +1,60 @@ package chartify import ( + "fmt" + "os" + "path/filepath" "testing" "github.com/google/go-cmp/cmp" ) +func TestHasChartLock(t *testing.T) { + tests := []struct { + name string + lockName string // empty means no lock file is created + want bool + }{ + {name: "no lock file", lockName: "", want: false}, + {name: "Chart.lock present", lockName: "Chart.lock", want: true}, + {name: "requirements.lock present", lockName: "requirements.lock", want: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + if tt.lockName != "" { + path := filepath.Join(dir, tt.lockName) + if err := os.WriteFile(path, []byte("dependencies: []\n"), 0644); err != nil { + t.Fatalf("writing fixture: %v", err) + } + } + if got := hasChartLock(dir); got != tt.want { + t.Errorf("hasChartLock() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestIsLockOutOfSyncErr(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + {name: "out of sync message", err: fmt.Errorf("the lock file (Chart.lock) is out of sync with the dependencies listed in Chart.yaml"), want: true}, + {name: "lock file is out of date", err: fmt.Errorf("lock file is out of date"), want: true}, + {name: "network error", err: fmt.Errorf("network timeout fetching dependency"), want: false}, + {name: "auth error", err: fmt.Errorf("401 unauthorized"), want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isLockOutOfSyncErr(tt.err); got != tt.want { + t.Errorf("isLockOutOfSyncErr() = %v, want %v", got, tt.want) + } + }) + } +} + func TestCreateFlagChain(t *testing.T) { testcases := []struct { flag string