From da754f1969342d1c1c4538e2b0673566dad8b346 Mon Sep 17 00:00:00 2001 From: "alexandr.danilin" Date: Mon, 17 Nov 2025 13:59:38 +0300 Subject: [PATCH 1/6] Fixes #31496 Signed-off-by: Alexandr Danilin --- pkg/cmd/dependency_update_test.go | 86 +++++++++++++++++++++++++++++++ pkg/downloader/manager.go | 26 +++++++++- 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/dependency_update_test.go b/pkg/cmd/dependency_update_test.go index 3eaa51df1..eed251b32 100644 --- a/pkg/cmd/dependency_update_test.go +++ b/pkg/cmd/dependency_update_test.go @@ -310,3 +310,89 @@ func createTestingChart(t *testing.T, dest, name, baseURL string) { t.Fatal(err) } } + +func TestDependencyUpdateCmd_NestedLocalDependencies(t *testing.T) { + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testcharts/*.tgz"), + ) + defer srv.Stop() + + if err := srv.LinkIndices(); err != nil { + t.Fatal(err) + } + + dir := func(p ...string) string { + return filepath.Join(append([]string{srv.Root()}, p...)...) + } + + // Create leaf dependency (has remote dependency) + leafChart := &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: chart.APIVersionV2, + Name: "leaf-chart", + Version: "0.1.0", + Dependencies: []*chart.Dependency{ + {Name: "reqtest", Version: "0.1.0", Repository: srv.URL()}, + }, + }, + } + if err := chartutil.SaveDir(leafChart, dir()); err != nil { + t.Fatal(err) + } + + // Create middle dependency (has local dependency on leaf) + middleChart := &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: chart.APIVersionV2, + Name: "middle-chart", + Version: "0.1.0", + Dependencies: []*chart.Dependency{ + {Name: "leaf-chart", Version: "0.1.0", Repository: "file://../leaf-chart"}, + }, + }, + } + if err := chartutil.SaveDir(middleChart, dir()); err != nil { + t.Fatal(err) + } + + rootChart := &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: chart.APIVersionV2, + Name: "root-chart", + Version: "0.1.0", + Dependencies: []*chart.Dependency{ + {Name: "middle-chart", Version: "0.1.0", Repository: "file://../middle-chart"}, + }, + }, + } + if err := chartutil.SaveDir(rootChart, dir()); err != nil { + t.Fatal(err) + } + + contentCache := t.TempDir() + + _, out, err := executeActionCommand( + fmt.Sprintf("dependency update '%s' --repository-config %s --repository-cache %s --content-cache %s --plain-http", + dir("root-chart"), dir("repositories.yaml"), dir(), contentCache), + ) + if err != nil { + t.Logf("Output: %s", out) + t.Fatal(err) + } + + expectMiddle := dir("root-chart", "charts/middle-chart-0.1.0.tgz") + if _, err := os.Stat(expectMiddle); err != nil { + t.Fatalf("Expected middle-chart dependency: %s", err) + } + + expectLeaf := dir("middle-chart", "charts/leaf-chart-0.1.0.tgz") + if _, err := os.Stat(expectLeaf); err != nil { + t.Fatalf("Expected leaf-chart dependency: %s", err) + } + + expectReqtest := dir("leaf-chart", "charts/reqtest-0.1.0.tgz") + if _, err := os.Stat(expectReqtest); err != nil { + t.Fatalf("Expected reqtest dependency: %s", err) + } +} diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 6043fbaaa..522c57053 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -192,6 +192,26 @@ func (m *Manager) Update() error { } } + // do resolution for each local dependency first + // it is required when local dependencies may have their own dependencies which must be resolved + for _, dep := range req { + if !isLocalDependency(dep.Repository) { + continue + } + man := *m + // no need to update repositories, it is already done in main chart + man.SkipUpdate = true + chartpath, err := resolver.GetLocalPath(dep.Repository, man.ChartPath) + if err != nil { + return err + } + man.ChartPath = chartpath + err = man.Update() + if err != nil { + return err + } + } + // Now we need to find out which version of a chart best satisfies the // dependencies in the Chart.yaml lock, err := m.resolve(req, repoNames) @@ -300,7 +320,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { } continue } - if strings.HasPrefix(dep.Repository, "file://") { + if isLocalDependency(dep.Repository) { if m.Debug { fmt.Fprintf(m.Out, "Archiving %s from repo %s\n", dep.Name, dep.Repository) } @@ -920,3 +940,7 @@ func key(name string) (string, error) { } return hex.EncodeToString(hash.Sum(nil)), nil } + +func isLocalDependency(repositoryPath string) bool { + return strings.HasPrefix(repositoryPath, "file://") +} From 6da5ae2a0fa258afc2ed891fb9fe0d5ee5a79334 Mon Sep 17 00:00:00 2001 From: Alexandr Danilin Date: Mon, 17 Nov 2025 14:02:53 +0300 Subject: [PATCH 2/6] Fixes #31496 Signed-off-by: Alexandr Danilin --- pkg/downloader/manager.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 522c57053..4ac6c2e41 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -198,13 +198,13 @@ func (m *Manager) Update() error { if !isLocalDependency(dep.Repository) { continue } - man := *m - // no need to update repositories, it is already done in main chart - man.SkipUpdate = true - chartpath, err := resolver.GetLocalPath(dep.Repository, man.ChartPath) + chartpath, err := resolver.GetLocalPath(dep.Repository, m.ChartPath) if err != nil { return err } + man := *m + // no need to update repositories, it is already done in main chart + man.SkipUpdate = true man.ChartPath = chartpath err = man.Update() if err != nil { From 29e3487933018de53a428693e0e6158f80406ef6 Mon Sep 17 00:00:00 2001 From: Alexandr Danilin Date: Mon, 17 Nov 2025 16:04:42 +0300 Subject: [PATCH 3/6] Reduce duplication code Signed-off-by: Alexandr Danilin --- internal/resolver/resolver.go | 6 +++++- pkg/downloader/manager.go | 14 +++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 3efe94f10..d8c1f16cb 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -77,7 +77,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string } continue } - if strings.HasPrefix(d.Repository, "file://") { + if IsLocalDependency(d.Repository) { chartpath, err := GetLocalPath(d.Repository, r.chartpath) if err != nil { return nil, err @@ -261,3 +261,7 @@ func GetLocalPath(repo, chartpath string) (string, error) { return depPath, nil } + +func IsLocalDependency(repositoryPath string) bool { + return strings.HasPrefix(repositoryPath, "file://") +} diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 4ac6c2e41..fdb151c9d 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -195,7 +195,7 @@ func (m *Manager) Update() error { // do resolution for each local dependency first // it is required when local dependencies may have their own dependencies which must be resolved for _, dep := range req { - if !isLocalDependency(dep.Repository) { + if !resolver.IsLocalDependency(dep.Repository) { continue } chartpath, err := resolver.GetLocalPath(dep.Repository, m.ChartPath) @@ -320,7 +320,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { } continue } - if isLocalDependency(dep.Repository) { + if resolver.IsLocalDependency(dep.Repository) { if m.Debug { fmt.Fprintf(m.Out, "Archiving %s from repo %s\n", dep.Name, dep.Repository) } @@ -496,7 +496,7 @@ func (m *Manager) hasAllRepos(deps []*chart.Dependency) error { Loop: for _, dd := range deps { // If repo is from local path or OCI, continue - if strings.HasPrefix(dd.Repository, "file://") || registry.IsOCI(dd.Repository) { + if resolver.IsLocalDependency(dd.Repository) || registry.IsOCI(dd.Repository) { continue } @@ -601,7 +601,7 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string, continue } // if dep chart is from local path, verify the path is valid - if strings.HasPrefix(dd.Repository, "file://") { + if resolver.IsLocalDependency(dd.Repository) { if _, err := resolver.GetLocalPath(dd.Repository, m.ChartPath); err != nil { return nil, err } @@ -894,7 +894,7 @@ func writeLock(chartpath string, lock *chart.Lock, legacyLockfile bool) error { // archive a dep chart from local directory and save it into destPath func tarFromLocalDir(chartpath, name, repo, version, destPath string) (string, error) { - if !strings.HasPrefix(repo, "file://") { + if !resolver.IsLocalDependency(repo) { return "", fmt.Errorf("wrong format: chart %s repository %s", name, repo) } @@ -940,7 +940,3 @@ func key(name string) (string, error) { } return hex.EncodeToString(hash.Sum(nil)), nil } - -func isLocalDependency(repositoryPath string) bool { - return strings.HasPrefix(repositoryPath, "file://") -} From d99661d626e7d817640a879bc98a16777db02cc7 Mon Sep 17 00:00:00 2001 From: Alexandr Danilin Date: Tue, 18 Nov 2025 20:56:20 +0300 Subject: [PATCH 4/6] Add suggested change Signed-off-by: Alexandr Danilin --- pkg/downloader/manager.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index fdb151c9d..be21a612b 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -192,8 +192,8 @@ func (m *Manager) Update() error { } } - // do resolution for each local dependency first - // it is required when local dependencies may have their own dependencies which must be resolved + // Do resolution for each local dependency first. Local dependencies may + // have their own dependencies which must be resolved. for _, dep := range req { if !resolver.IsLocalDependency(dep.Repository) { continue From bc6b6199ba799b2800eff286153a31e448dc3bdb Mon Sep 17 00:00:00 2001 From: Alexandr Danilin Date: Tue, 3 Mar 2026 15:36:38 +0300 Subject: [PATCH 5/6] Detect and handle circular dependencies in local chart dependencies Signed-off-by: Alexandr Danilin --- pkg/cmd/dependency_update_test.go | 49 +++++++++++++++++++++++++++++++ pkg/downloader/manager.go | 47 +++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/pkg/cmd/dependency_update_test.go b/pkg/cmd/dependency_update_test.go index eed251b32..0f6a223b7 100644 --- a/pkg/cmd/dependency_update_test.go +++ b/pkg/cmd/dependency_update_test.go @@ -396,3 +396,52 @@ func TestDependencyUpdateCmd_NestedLocalDependencies(t *testing.T) { t.Fatalf("Expected reqtest dependency: %s", err) } } + +func TestDependencyUpdateCmd_CircularDependency(t *testing.T) { + srv := repotest.NewTempServer(t) + defer srv.Stop() + + dir := func(p ...string) string { + return filepath.Join(append([]string{srv.Root()}, p...)...) + } + + // Create chart A that depends on B + chartA := &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: chart.APIVersionV2, + Name: "chart-a", + Version: "1.0.0", + Dependencies: []*chart.Dependency{ + {Name: "chart-b", Version: "1.0.0", Repository: "file://../chart-b"}, + }, + }, + } + if err := chartutil.SaveDir(chartA, dir()); err != nil { + t.Fatal(err) + } + + // Create chart B that depends on A (circular) + chartB := &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: chart.APIVersionV2, + Name: "chart-b", + Version: "1.0.0", + Dependencies: []*chart.Dependency{ + {Name: "chart-a", Version: "1.0.0", Repository: "file://../chart-a"}, + }, + }, + } + if err := chartutil.SaveDir(chartB, dir()); err != nil { + t.Fatal(err) + } + + _, out, err := executeActionCommand( + fmt.Sprintf("dependency update '%s'", dir("chart-a")), + ) + if err == nil { + t.Fatal("Expected circular dependency error") + } + if !strings.Contains(out, "circular dependency detected") { + t.Fatalf("Expected 'circular dependency detected' in output, got: %s", out) + } +} diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index be21a612b..5f175fcda 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -192,6 +192,11 @@ func (m *Manager) Update() error { } } + // Check for circular dependencies in local dependencies + if err := m.checkCircularDeps(req, nil); err != nil { + return err + } + // Do resolution for each local dependency first. Local dependencies may // have their own dependencies which must be resolved. for _, dep := range req { @@ -940,3 +945,45 @@ func key(name string) (string, error) { } return hex.EncodeToString(hash.Sum(nil)), nil } + +// checkCircularDeps checks local dependencies for circular dependency issue. +// When local charts depend on each other, helm will quit at the very beginning with the clear message. +func (m *Manager) checkCircularDeps(deps []*chart.Dependency, chain []string) error { + absPath, err := filepath.Abs(m.ChartPath) + if err != nil { + return err + } + + for i, visited := range chain { + if visited == absPath { + cycle := append(chain[i:], absPath) + return fmt.Errorf("circular dependency detected:\n%s", strings.Join(cycle, "\n -> ")) + } + } + + // Create a new chain with the current path to avoid modifying the caller's slice + newChain := make([]string, len(chain)+1) + copy(newChain, chain) + newChain[len(chain)] = absPath + + for _, dep := range deps { + if !resolver.IsLocalDependency(dep.Repository) { + continue + } + chartpath, err := resolver.GetLocalPath(dep.Repository, m.ChartPath) + if err != nil { + return err + } + c, err := loader.LoadDir(chartpath) + if err != nil { + return err + } + man := *m + man.ChartPath = chartpath + if err := man.checkCircularDeps(c.Metadata.Dependencies, newChain); err != nil { + return err + } + } + + return nil +} From 862a5dfb4fa360ea13d6310fdadadfdea94c904c Mon Sep 17 00:00:00 2001 From: Alexandr Danilin Date: Tue, 10 Mar 2026 12:22:45 +0300 Subject: [PATCH 6/6] Address comments from the MR Signed-off-by: Alexandr Danilin --- internal/resolver/resolver.go | 2 ++ pkg/downloader/manager.go | 46 +++++++++++++---------------------- 2 files changed, 19 insertions(+), 29 deletions(-) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index d8c1f16cb..5f1e372ad 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -262,6 +262,8 @@ func GetLocalPath(repo, chartpath string) (string, error) { return depPath, nil } +// IsLocalDependency checks if the repository path is a local file dependency. +// Local dependencies use the "file://" protocol prefix. func IsLocalDependency(repositoryPath string) bool { return strings.HasPrefix(repositoryPath, "file://") } diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 5f175fcda..4b15f5923 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -23,6 +23,7 @@ import ( "io" stdfs "io/fs" "log" + "log/slog" "net/url" "os" "path/filepath" @@ -192,29 +193,10 @@ func (m *Manager) Update() error { } } - // Check for circular dependencies in local dependencies - if err := m.checkCircularDeps(req, nil); err != nil { - return err - } - // Do resolution for each local dependency first. Local dependencies may // have their own dependencies which must be resolved. - for _, dep := range req { - if !resolver.IsLocalDependency(dep.Repository) { - continue - } - chartpath, err := resolver.GetLocalPath(dep.Repository, m.ChartPath) - if err != nil { - return err - } - man := *m - // no need to update repositories, it is already done in main chart - man.SkipUpdate = true - man.ChartPath = chartpath - err = man.Update() - if err != nil { - return err - } + if err := m.updateLocalDeps(req, nil); err != nil { + return err } // Now we need to find out which version of a chart best satisfies the @@ -946,14 +928,14 @@ func key(name string) (string, error) { return hex.EncodeToString(hash.Sum(nil)), nil } -// checkCircularDeps checks local dependencies for circular dependency issue. -// When local charts depend on each other, helm will quit at the very beginning with the clear message. -func (m *Manager) checkCircularDeps(deps []*chart.Dependency, chain []string) error { +// updateLocalDeps recursively updates local dependencies and detects circular dependencies. +func (m *Manager) updateLocalDeps(deps []*chart.Dependency, chain []string) error { absPath, err := filepath.Abs(m.ChartPath) if err != nil { return err } + // Check for circular dependency for i, visited := range chain { if visited == absPath { cycle := append(chain[i:], absPath) @@ -961,10 +943,7 @@ func (m *Manager) checkCircularDeps(deps []*chart.Dependency, chain []string) er } } - // Create a new chain with the current path to avoid modifying the caller's slice - newChain := make([]string, len(chain)+1) - copy(newChain, chain) - newChain[len(chain)] = absPath + chain = append(chain, absPath) for _, dep := range deps { if !resolver.IsLocalDependency(dep.Repository) { @@ -974,13 +953,22 @@ func (m *Manager) checkCircularDeps(deps []*chart.Dependency, chain []string) er if err != nil { return err } + c, err := loader.LoadDir(chartpath) if err != nil { return err } + man := *m + man.SkipUpdate = true man.ChartPath = chartpath - if err := man.checkCircularDeps(c.Metadata.Dependencies, newChain); err != nil { + slog.Debug("Recursively updating dependencies for local chart", "chart", dep.Name, "path", chartpath) + + if err := man.updateLocalDeps(c.Metadata.Dependencies, chain); err != nil { + return err + } + + if err := man.Update(); err != nil { return err } }