From 81da2d44b31ce334d799dec49eb19918ec667087 Mon Sep 17 00:00:00 2001
From: manslaughter03 <manslaughter03@gmail.com>
Date: Fri, 13 Jun 2025 10:42:42 +0200
Subject: [PATCH] refactor: move check dependency conflict into
 checkSubchartConflict function called b  after loading chart.

Signed-off-by: manslaughter03 <manslaughter03@gmail.com>
---
 pkg/downloader/manager.go      | 29 ++++++++++++---
 pkg/downloader/manager_test.go | 68 +++++++++++++++++++++++++---------
 2 files changed, 73 insertions(+), 24 deletions(-)

diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go
index 094d9aff6..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)
@@ -298,12 +305,6 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
 			continue
 		}
 
-		// Check if subchart already exist before downloading tarball.
-		// https://github.com/helm/helm/issues/30710
-		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)
-		}
-
 		if strings.HasPrefix(dep.Repository, "file://") {
 			if m.Debug {
 				fmt.Fprintf(m.Out, "Archiving %s from repo %s\n", dep.Name, dep.Repository)
@@ -847,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 c565cb3ac..9fa6c9bf0 100644
--- a/pkg/downloader/manager_test.go
+++ b/pkg/downloader/manager_test.go
@@ -298,24 +298,6 @@ version: 0.1.0`
 	if err == nil {
 		t.Fatal("Expected error for bad dependency name")
 	}
-
-	remoteDep := &chart.Dependency{
-		Name:       "sub",
-		Repository: "oci://remote",
-		Version:    "0.0.1",
-	}
-
-	// create a 'tmpcharts' directory to test #30710
-	if err := os.MkdirAll(filepath.Join(chartPath, "charts", "sub"), 0755); err != nil {
-		t.Fatal(err)
-	}
-	err = m.downloadAll([]*chart.Dependency{remoteDep})
-	if err == nil {
-		t.Fatal("Expected error as subchart already exist")
-	}
-	if err.Error() != "dependency conflict detected: A subchart named 'sub' already exists in charts/ directory" {
-		t.Fatal("Download should failed with conflict issue")
-	}
 }
 
 func TestUpdateBeforeBuild(t *testing.T) {
@@ -690,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")
+	}
+}