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 +}