diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 348c78edb..971f06c0d 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -162,6 +162,13 @@ func (m *Manager) Update() error { return nil } + // Check if there is already subchart in charts directory before downloading tarball. + // https://github.com/helm/helm/issues/30710 + err = m.checkSubchartConflict(req) + if err != nil { + return fmt.Errorf("%s. Please remove the manually added subchart or exclude it from Chart.yaml dependencies", err.Error()) + } + // Get the names of the repositories the dependencies need that Helm is // configured to know about. repoNames, err := m.resolveRepoNames(req) @@ -271,11 +278,11 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { var saveError error churls := make(map[string]struct{}) for _, dep := range deps { + chartPath := filepath.Join(destPath, dep.Name) // No repository means the chart is in charts directory if dep.Repository == "" { fmt.Fprintf(m.Out, "Dependency %s did not declare a repository. Assuming it exists in the charts directory\n", dep.Name) // NOTE: we are only validating the local dependency conforms to the constraints. No copying to tmpPath is necessary. - chartPath := filepath.Join(destPath, dep.Name) ch, err := loader.LoadDir(chartPath) if err != nil { return fmt.Errorf("unable to load chart '%s': %v", chartPath, err) @@ -297,6 +304,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { } continue } + if strings.HasPrefix(dep.Repository, "file://") { if m.Debug { fmt.Fprintf(m.Out, "Archiving %s from repo %s\n", dep.Name, dep.Repository) @@ -840,6 +848,22 @@ func (m *Manager) loadChartRepositories() (map[string]*repo.ChartRepository, err return indices, nil } +// checkSubchartConflict Check if subchart already exist before downloading tarball. +func (m *Manager) checkSubchartConflict(req []*chart.Dependency) error { + for _, dep := range req { + // No repository means the chart is in charts directory + if dep.Repository == "" { + continue + } + chartPath := filepath.Join(m.ChartPath, "charts", dep.Name) + if _, err := os.Stat(chartPath); !os.IsNotExist(err) { + return fmt.Errorf("dependency conflict detected: A subchart named '%s' already exists in charts/ directory", dep.Name) + } + } + + return nil +} + // writeLock writes a lockfile to disk func writeLock(chartpath string, lock *chart.Lock, legacyLockfile bool) error { data, err := yaml.Marshal(lock) diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index 53955c45b..9fa6c9bf0 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -672,3 +672,53 @@ func TestDedupeRepos(t *testing.T) { }) } } + +func TestCheckSubchartConflict(t *testing.T) { + chartPath := t.TempDir() + m := &Manager{ + Out: new(bytes.Buffer), + RepositoryConfig: repoConfig, + RepositoryCache: repoCache, + ChartPath: chartPath, + } + // create a 'tmpcharts' directory to test #30710 + if err := os.MkdirAll(filepath.Join(chartPath, "charts", "existing"), 0755); err != nil { + t.Fatal(err) + } + + // Conflict use case, there is already a 'existing' directory in charts/. + existingDep := &chart.Dependency{ + Name: "existing", + Repository: "oci://remote", + Version: "0.0.1", + } + err := m.checkSubchartConflict([]*chart.Dependency{existingDep}) + if err == nil { + t.Fatal("Expected error as subchart already exist") + } + if err.Error() != "dependency conflict detected: A subchart named 'existing' already exists in charts/ directory" { + t.Fatal("checkSubchartConflict should failed with conflict issue") + } + + // There is no exiting subchart with 'inexisting' name. + inexistingDep := &chart.Dependency{ + Name: "inexisting", + Repository: "oci://remote", + Version: "0.0.1", + } + err = m.checkSubchartConflict([]*chart.Dependency{inexistingDep}) + if err != nil { + t.Fatal("checkSubchartConflict should failed not failed") + } + + // No repository use case, means the chart is already in charts directory. + existingDep = &chart.Dependency{ + Name: "existing", + Repository: "", + Version: "0.0.1", + } + err = m.checkSubchartConflict([]*chart.Dependency{existingDep}) + if err != nil { + t.Fatal("checkSubchartConflict should failed not failed") + } +}