Repository: helm/helm (go)

Issue:
Upgrading to Helm 3.16 broke dependencies references
running helm update with pre 3.16 depenencies list starting to fail due to malformed download URLs.

Example command
```
helm dep update /path/to/local/chart/
```

Example Chart.yaml snippet
```
dependencies:
  - name: docker-registry
    version: 2.2.2
    repository: https://helm.twun.io/
    condition: ...
```

```
Saving 5 charts
Downloading docker-registry from repo https://helm.twun.io/
Save error occurred:  could not download docker-registry-2.2.2.tgz: non-absolute URLs should be in form of repo_name/path_to_chart, got: docker-registry-2.2.2.tgz
Error: could not download docker-registry-2.2.2.tgz: non-absolute URLs should be in form of repo_name/path_to_chart, got: docker-registry-2.2.2.tgz
make: *** [helm] Error 1
```

Output of `helm version`:
```
helm version
version.BuildInfo{Version:"v3.16.0", GitCommit:"0d439e1a09683f21a0ab9401eb661401f185b00b", GitTreeState:"clean", GoVersion:"go1.22.6"}
```

Output of `kubectl version`:
```
❯ kubectl version
Client Version: v1.29.4
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.30.3-eks-a18cd3a
```

Cloud Provider/Platform (AKS, GKE, Minikube etc.): 
Helm template locally


Pull request:
Revert "Improve helm dependency update performance"
The change in #11726 caused a regression where `helm dependency udpate` stopped working. The format of the internal representation of the data changed causing errors of "non-absolute URLs should be in form of repo_name/path_to_chart". See #13324 for more details.

Since this change is in released Helm and it's a regression, reverting the original change was the fastest and safest route to deliver a fix as quickly as possible.

Closes #13324

<!--  Thanks for sending a pull request!  Here are some tips for you:
1. Make sure to read the Contributing Guide before submitting your PR: https://github.com/helm/helm/blob/main/CONTRIBUTING.md
2. If this PR closes another issue, add 'closes #<issue number>' somewhere in the PR summary. GitHub will automatically close that issue when this PR gets merged. Alternatively, adding 'refs #<issue number>' will not close the issue, but help provide the reviewer more context.-->

**What this PR does / why we need it**:

The latest version of Helm has a regression causing functionality to be broken.

**Special notes for your reviewer**:

**If applicable**:
- [ ] this PR contains user facing changes (the `docs needed` label should be applied if so)
- [ ] this PR contains unit tests
- [ ] this PR has been tested for backwards compatibility


Patch:
diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go
index 3d2878ea9..c5fc63643 100644
--- a/internal/resolver/resolver.go
+++ b/internal/resolver/resolver.go
@@ -52,23 +52,21 @@ func New(chartpath, cachepath string, registryClient *registry.Client) *Resolver
 }
 
 // Resolve resolves dependencies and returns a lock file with the resolution.
-func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string) (*chart.Lock, map[string]string, error) {
+func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string) (*chart.Lock, error) {
 
 	// Now we clone the dependencies, locking as we go.
 	locked := make([]*chart.Dependency, len(reqs))
 	missing := []string{}
-	loadedIndexFiles := make(map[string]*repo.IndexFile)
-	urls := make(map[string]string)
 	for i, d := range reqs {
 		constraint, err := semver.NewConstraint(d.Version)
 		if err != nil {
-			return nil, nil, errors.Wrapf(err, "dependency %q has an invalid version/constraint format", d.Name)
+			return nil, errors.Wrapf(err, "dependency %q has an invalid version/constraint format", d.Name)
 		}
 
 		if d.Repository == "" {
 			// Local chart subfolder
 			if _, err := GetLocalPath(filepath.Join("charts", d.Name), r.chartpath); err != nil {
-				return nil, nil, err
+				return nil, err
 			}
 
 			locked[i] = &chart.Dependency{
@@ -81,12 +79,12 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 		if strings.HasPrefix(d.Repository, "file://") {
 			chartpath, err := GetLocalPath(d.Repository, r.chartpath)
 			if err != nil {
-				return nil, nil, err
+				return nil, err
 			}
 
 			ch, err := loader.LoadDir(chartpath)
 			if err != nil {
-				return nil, nil, err
+				return nil, err
 			}
 
 			v, err := semver.NewVersion(ch.Metadata.Version)
@@ -124,26 +122,14 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 		var ok bool
 		found := true
 		if !registry.IsOCI(d.Repository) {
-			filepath := filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName))
-			var repoIndex *repo.IndexFile
-
-			// Store previously loaded index files in a map. If repositories share the
-			// same index file there is no need to reload the same file again. This
-			// improves performance.
-			if indexFile, loaded := loadedIndexFiles[filepath]; !loaded {
-				var err error
-				repoIndex, err = repo.LoadIndexFile(filepath)
-				loadedIndexFiles[filepath] = repoIndex
-				if err != nil {
-					return nil, nil, errors.Wrapf(err, "no cached repository for %s found. (try 'helm repo update')", repoName)
-				}
-			} else {
-				repoIndex = indexFile
+			repoIndex, err := repo.LoadIndexFile(filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName)))
+			if err != nil {
+				return nil, errors.Wrapf(err, "no cached repository for %s found. (try 'helm repo update')", repoName)
 			}
 
 			vs, ok = repoIndex.Entries[d.Name]
 			if !ok {
-				return nil, nil, errors.Errorf("%s chart not found in repo %s", d.Name, d.Repository)
+				return nil, errors.Errorf("%s chart not found in repo %s", d.Name, d.Repository)
 			}
 			found = false
 		} else {
@@ -165,7 +151,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 				ref := fmt.Sprintf("%s/%s", strings.TrimPrefix(d.Repository, fmt.Sprintf("%s://", registry.OCIScheme)), d.Name)
 				tags, err := r.registryClient.Tags(ref)
 				if err != nil {
-					return nil, nil, errors.Wrapf(err, "could not retrieve list of tags for repository %s", d.Repository)
+					return nil, errors.Wrapf(err, "could not retrieve list of tags for repository %s", d.Repository)
 				}
 
 				vs = make(repo.ChartVersions, len(tags))
@@ -186,7 +172,6 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 			Repository: d.Repository,
 			Version:    version,
 		}
-
 		// The version are already sorted and hence the first one to satisfy the constraint is used
 		for _, ver := range vs {
 			v, err := semver.NewVersion(ver.Version)
@@ -197,9 +182,6 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 			}
 			if constraint.Check(v) {
 				found = true
-				if len(ver.URLs) > 0 {
-					urls[d.Repository+ver.Name+ver.Version] = ver.URLs[0]
-				}
 				locked[i].Version = v.Original()
 				break
 			}
@@ -210,19 +192,19 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 		}
 	}
 	if len(missing) > 0 {
-		return nil, nil, errors.Errorf("can't get a valid version for %d subchart(s): %s. Make sure a matching chart version exists in the repo, or change the version constraint in Chart.yaml", len(missing), strings.Join(missing, ", "))
+		return nil, errors.Errorf("can't get a valid version for %d subchart(s): %s. Make sure a matching chart version exists in the repo, or change the version constraint in Chart.yaml", len(missing), strings.Join(missing, ", "))
 	}
 
 	digest, err := HashReq(reqs, locked)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	return &chart.Lock{
 		Generated:    time.Now(),
 		Digest:       digest,
 		Dependencies: locked,
-	}, urls, nil
+	}, nil
 }
 
 // HashReq generates a hash of the dependencies.
diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go
index 03c13bcf1..d5340575d 100644
--- a/pkg/downloader/manager.go
+++ b/pkg/downloader/manager.go
@@ -141,7 +141,7 @@ func (m *Manager) Build() error {
 	}
 
 	// Now we need to fetch every package here into charts/
-	return m.downloadAll(lock.Dependencies, nil)
+	return m.downloadAll(lock.Dependencies)
 }
 
 // Update updates a local charts directory.
@@ -191,13 +191,13 @@ func (m *Manager) Update() error {
 
 	// Now we need to find out which version of a chart best satisfies the
 	// dependencies in the Chart.yaml
-	lock, urls, err := m.resolve(req, repoNames)
+	lock, err := m.resolve(req, repoNames)
 	if err != nil {
 		return err
 	}
 
 	// Now we need to fetch every package here into charts/
-	if err := m.downloadAll(lock.Dependencies, urls); err != nil {
+	if err := m.downloadAll(lock.Dependencies); err != nil {
 		return err
 	}
 
@@ -230,7 +230,7 @@ func (m *Manager) loadChartDir() (*chart.Chart, error) {
 // resolve takes a list of dependencies and translates them into an exact version to download.
 //
 // This returns a lock file, which has all of the dependencies normalized to a specific version.
-func (m *Manager) resolve(req []*chart.Dependency, repoNames map[string]string) (*chart.Lock, map[string]string, error) {
+func (m *Manager) resolve(req []*chart.Dependency, repoNames map[string]string) (*chart.Lock, error) {
 	res := resolver.New(m.ChartPath, m.RepositoryCache, m.RegistryClient)
 	return res.Resolve(req, repoNames)
 }
@@ -239,7 +239,7 @@ func (m *Manager) resolve(req []*chart.Dependency, repoNames map[string]string)
 //
 // It will delete versions of the chart that exist on disk and might cause
 // a conflict.
-func (m *Manager) downloadAll(deps []*chart.Dependency, urls map[string]string) error {
+func (m *Manager) downloadAll(deps []*chart.Dependency) error {
 	repos, err := m.loadChartRepositories()
 	if err != nil {
 		return err
@@ -312,7 +312,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency, urls map[string]string)
 
 		// Any failure to resolve/download a chart should fail:
 		// https://github.com/helm/helm/issues/1439
-		churl, username, password, insecureskiptlsverify, passcredentialsall, caFile, certFile, keyFile, err := m.findChartURL(dep.Name, dep.Version, dep.Repository, repos, urls)
+		churl, username, password, insecureskiptlsverify, passcredentialsall, caFile, certFile, keyFile, err := m.findChartURL(dep.Name, dep.Version, dep.Repository, repos)
 		if err != nil {
 			saveError = errors.Wrapf(err, "could not find %s", churl)
 			break
@@ -501,7 +501,6 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart.
 
 	var ru []*repo.Entry
 
-Outer:
 	for _, dd := range deps {
 
 		// If the chart is in the local charts directory no repository needs
@@ -529,14 +528,6 @@ Outer:
 
 		repoNames[dd.Name] = rn
 
-		// If repository is already present don't add to array. This will skip
-		// unnecessary index file downloading improving performance.
-		for _, item := range ru {
-			if item.URL == dd.Repository {
-				continue Outer
-			}
-		}
-
 		// Assuming the repository is generally available. For Helm managed
 		// access controls the repository needs to be added through the user
 		// managed system. This path will work for public charts, like those
@@ -712,7 +703,7 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error {
 // repoURL is the repository to search
 //
 // If it finds a URL that is "relative", it will prepend the repoURL.
-func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*repo.ChartRepository, urls map[string]string) (url, username, password string, insecureskiptlsverify, passcredentialsall bool, caFile, certFile, keyFile string, err error) {
+func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*repo.ChartRepository) (url, username, password string, insecureskiptlsverify, passcredentialsall bool, caFile, certFile, keyFile string, err error) {
 	if registry.IsOCI(repoURL) {
 		return fmt.Sprintf("%s/%s:%s", repoURL, name, version), "", "", false, false, "", "", "", nil
 	}
@@ -751,14 +742,7 @@ func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*
 			return
 		}
 	}
-
-	urlsKey := repoURL + name + version
-	if _, ok := urls[urlsKey]; ok {
-		url = urls[urlsKey]
-	} else {
-		url, err = repo.FindChartInRepoURL(repoURL, name, version, certFile, keyFile, caFile, m.Getters)
-	}
-
+	url, err = repo.FindChartInRepoURL(repoURL, name, version, certFile, keyFile, caFile, m.Getters)
 	if err == nil {
 		return url, username, password, false, false, "", "", "", err
 	}


Test patch:
diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go
index a4c4ef351..a79852175 100644
--- a/internal/resolver/resolver_test.go
+++ b/internal/resolver/resolver_test.go
@@ -144,7 +144,7 @@ func TestResolve(t *testing.T) {
 	r := New("testdata/chartpath", "testdata/repository", registryClient)
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			l, _, err := r.Resolve(tt.req, repoNames)
+			l, err := r.Resolve(tt.req, repoNames)
 			if err != nil {
 				if tt.err {
 					return
diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go
index a2ced2e5d..db2487d16 100644
--- a/pkg/downloader/manager_test.go
+++ b/pkg/downloader/manager_test.go
@@ -84,7 +84,7 @@ func TestFindChartURL(t *testing.T) {
 	version := "0.1.0"
 	repoURL := "http://example.com/charts"
 
-	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(name, version, repoURL, repos, make(map[string]string))
+	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(name, version, repoURL, repos)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -109,7 +109,7 @@ func TestFindChartURL(t *testing.T) {
 	version = "1.2.3"
 	repoURL = "https://example-https-insecureskiptlsverify.com"
 
-	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos, make(map[string]string))
+	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -255,7 +255,7 @@ func TestDownloadAll(t *testing.T) {
 	if err := os.MkdirAll(filepath.Join(chartPath, "tmpcharts"), 0755); err != nil {
 		t.Fatal(err)
 	}
-	if err := m.downloadAll([]*chart.Dependency{signDep, localDep}, make(map[string]string)); err != nil {
+	if err := m.downloadAll([]*chart.Dependency{signDep, localDep}); err != nil {
 		t.Error(err)
 	}
 
@@ -284,7 +284,7 @@ version: 0.1.0`
 		Version:    "0.1.0",
 	}
 
-	err = m.downloadAll([]*chart.Dependency{badLocalDep}, make(map[string]string))
+	err = m.downloadAll([]*chart.Dependency{badLocalDep})
 	if err == nil {
 		t.Fatal("Expected error for bad dependency name")
 	}