Merge 81da2d44b3 into e8be2e540d

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)

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