Set chartURL in dependency instead of passing around a cache

Signed-off-by: MichaelMorris <michael.morris@est.tech>

internal/chart/v3/dependency.go

@@ -31,6 +31,8 @@ type Dependency struct {
 	// A lock file will always produce a single version, while a dependency
 	// may contain a semantic version range.
 	Version string `json:"version,omitempty" yaml:"version,omitempty"`
+	// The URL to the dependency.
+	ChartURL string `json:"charturl,omitempty" yaml:"charturl,omitempty"`
 	// The URL to the repository.
 	//
 	// Appending `index.yaml` to this string should result in a URL that can be

internal/resolver/resolver.go

@@ -52,25 +52,23 @@ func New(chartpath, cachepath string, registryClient *registry.Client) *Resolver
 	}
 }
 
-// Resolve resolves dependencies and returns a lock file with the resolution and a map containaing the chart URLs for the dependencies.
-// The key to the map is generated by concatenating the repo URL, name and version for the dependency
-func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string) (*chart.Lock, map[string]string, error) {
+// Resolve resolves dependencies and returns a lock file with the resolution.
+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{}
 	chartRepoIndexURLs := 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, fmt.Errorf("dependency %q has an invalid version/constraint format: %w", d.Name, err)
+			return nil, fmt.Errorf("dependency %q has an invalid version/constraint format: %w", d.Name, err)
 		}
 
 		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{
@@ -83,12 +81,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)
@@ -137,7 +135,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 				repoIndex, err = repo.LoadIndexFile(repoFilepath)
 				chartRepoIndexURLs[repoFilepath] = repoIndex
 				if err != nil {
-				return nil, nil, fmt.Errorf("no cached repository for %s found. (try 'helm repo update'): %w", repoName, err)
+					return nil, fmt.Errorf("no cached repository for %s found. (try 'helm repo update'): %w", repoName, err)
 				}
 			} else {
 				repoIndex = indexFile
@@ -145,7 +143,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 
 			vs, ok = repoIndex.Entries[d.Name]
 			if !ok {
-				return nil, nil, fmt.Errorf("%s chart not found in repo %s", d.Name, d.Repository)
+				return nil, fmt.Errorf("%s chart not found in repo %s", d.Name, d.Repository)
 			}
 			found = false
 		} else {
@@ -167,7 +165,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, fmt.Errorf("could not retrieve list of tags for repository %s: %w", d.Repository, err)
+					return nil, fmt.Errorf("could not retrieve list of tags for repository %s: %w", d.Repository, err)
 				}
 
 				vs = make(repo.ChartVersions, len(tags))
@@ -199,9 +197,13 @@ 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].ChartURL, err = repo.ResolveReferenceURL(d.Repository, ver.URLs[0])
+					if err != nil {
+						return nil, err
+					}
 				}
 				locked[i].Version = v.Original()
+
 				break
 			}
 		}
@@ -211,19 +213,19 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 		}
 	}
 	if len(missing) > 0 {
-		return nil, nil, fmt.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, fmt.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.

internal/resolver/resolver_test.go

@@ -81,7 +81,7 @@ func TestResolve(t *testing.T) {
 			},
 			expect: &chart.Lock{
 				Dependencies: []*chart.Dependency{
-					{Name: "alpine", Repository: "http://example.com", Version: "0.2.0"},
+					{Name: "alpine", Repository: "http://example.com", Version: "0.2.0", ChartURL: "https://charts.helm.sh/stable/alpine-0.1.0.tgz"},
 				},
 			},
 		},
@@ -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

pkg/chart/v2/dependency.go

@@ -31,6 +31,8 @@ type Dependency struct {
 	// A lock file will always produce a single version, while a dependency
 	// may contain a semantic version range.
 	Version string `json:"version,omitempty" yaml:"version,omitempty"`
+	// The URL to the dependency.
+	ChartURL string `json:"charturl,omitempty" yaml:"charturl,omitempty"`
 	// The URL to the repository.
 	//
 	// Appending `index.yaml` to this string should result in a URL that can be

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,8 +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
-// and a map containaing the URLs for the dependencies. The key to the map is generated by concatenating the repo URL, name and version for the dependency.
-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)
 }
@@ -240,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
@@ -313,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, repos)
 		if err != nil {
 			saveError = fmt.Errorf("could not find %s: %w", churl, err)
 			break
@@ -723,25 +722,21 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error {
 	return nil
 }
 
-// findChartURL searches the cache of repo data for a chart that has the name and the repoURL specified.
+// findChartURL searches the cache of repo data for the specified dependency.
 //
-// 'name' is the name of the chart. Version is an exact semver, or an empty string. If empty, the
+// The version in the given dependency is an exact semver, or an empty string. If empty, the
 // newest version will be returned.
 //
-// repoURL is the repository to search
-//
-// resolvedChartUrls is a map of already resolved chart URLs keyed by a concatenation of the the repo URL, chart name and version. The URL shall be read from this map if present, rather than downloading and reading the index file
-//
 // 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, resolvedChartUrls map[string]string) (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
+func (m *Manager) findChartURL(dep *chart.Dependency, repos map[string]*repo.ChartRepository) (url, username, password string, insecureskiptlsverify, passcredentialsall bool, caFile, certFile, keyFile string, err error) {
+	if registry.IsOCI(dep.Repository) {
+		return fmt.Sprintf("%s/%s:%s", dep.Repository, dep.Name, dep.Version), "", "", false, false, "", "", "", nil
 	}
 
 	for _, cr := range repos {
-		if urlutil.Equal(repoURL, cr.Config.URL) {
+		if urlutil.Equal(dep.Repository, cr.Config.URL) {
 			var entry repo.ChartVersions
-			entry, err = findEntryByName(name, cr)
+			entry, err = findEntryByName(dep.Name, cr)
 			if err != nil {
 				// TODO: Where linting is skipped in this function we should
 				// refactor to remove naked returns while ensuring the same
@@ -750,12 +745,12 @@ func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*
 				return
 			}
 			var ve *repo.ChartVersion
-			ve, err = findVersionedEntry(version, entry)
+			ve, err = findVersionedEntry(dep.Version, entry)
 			if err != nil {
 				//nolint:nakedret
 				return
 			}
-			url, err = repo.ResolveReferenceURL(repoURL, ve.URLs[0])
+			url, err = repo.ResolveReferenceURL(dep.Repository, ve.URLs[0])
 			if err != nil {
 				//nolint:nakedret
 				return
@@ -772,17 +767,16 @@ func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*
 		}
 	}
 
-	urlsKey := repoURL + name + version
-	if chartURL, ok := resolvedChartUrls[urlsKey]; ok {
-		url, err = repo.ResolveReferenceURL(repoURL, chartURL)
+	if dep.ChartURL != "" {
+		url = dep.ChartURL
 	} else {
-		url, err = repo.FindChartInRepoURL(repoURL, name, m.Getters, repo.WithChartVersion(version), repo.WithClientTLS(certFile, keyFile, caFile))
+		url, err = repo.FindChartInRepoURL(dep.Repository, dep.Name, m.Getters, repo.WithChartVersion(dep.Version), repo.WithClientTLS(certFile, keyFile, caFile))
 	}
 
 	if err == nil {
 		return url, username, password, false, false, "", "", "", err
 	}
-	err = fmt.Errorf("chart %s not found in %s: %w", name, repoURL, err)
+	err = fmt.Errorf("chart %s not found in %s: %w", dep.Name, dep.Repository, err)
 	return url, username, password, false, false, "", "", "", err
 }
 

pkg/downloader/manager_test.go

@@ -67,11 +67,13 @@ func TestFindChartURL(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	name := "alpine"
-	version := "0.1.0"
-	repoURL := "http://example.com/charts"
+	dep := &chart.Dependency{
+		Name:       "alpine",
+		Version:    "0.1.0",
+		Repository: "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(dep, repos)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -92,11 +94,13 @@ func TestFindChartURL(t *testing.T) {
 		t.Errorf("Unexpected insecureSkipTLSVerify %t", insecureSkipTLSVerify)
 	}
 
-	name = "tlsfoo"
-	version = "1.2.3"
-	repoURL = "https://example-https-insecureskiptlsverify.com"
+	dep = &chart.Dependency{
+		Name:       "tlsfoo",
+		Version:    "1.2.3",
+		Repository: "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(dep, repos)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -117,11 +121,13 @@ func TestFindChartURL(t *testing.T) {
 		t.Errorf("Unexpected passcredentialsall %t", passcredentialsall)
 	}
 
-	name = "foo"
-	version = "1.2.3"
-	repoURL = "http://example.com/helm"
+	dep = &chart.Dependency{
+		Name:       "foo",
+		Version:    "1.2.3",
+		Repository: "http://example.com/helm",
+	}
 
-	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos)
+	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(dep, repos)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -143,7 +149,7 @@ func TestFindChartURL(t *testing.T) {
 	}
 }
 
-func TestFindChartURLBasedOnUrlMap(t *testing.T) {
+func TestFindChartURLAlreadyKnownUrl(t *testing.T) {
 	var b bytes.Buffer
 	m := &Manager{
 		Out:              &b,
@@ -152,43 +158,19 @@ func TestFindChartURLBasedOnUrlMap(t *testing.T) {
 	}
 	repos := make(map[string]*repo.ChartRepository)
 
-	name := "alpine"
-	version := "0.1.0"
-	repoURL := "http://example.com/charts"
-
-	urlMap := make(map[string]string)
-	urlKey := repoURL + name + version
-	urlMap[urlKey] = "https://charts.helm.sh/stable/alpine-0.1.0.tgzFromMap"
-
-	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(name, version, repoURL, repos, urlMap)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if churl != "https://charts.helm.sh/stable/alpine-0.1.0.tgzFromMap" {
-		t.Errorf("Unexpected URL %q", churl)
-	}
-	if username != "" {
-		t.Errorf("Unexpected username %q", username)
-	}
-	if password != "" {
-		t.Errorf("Unexpected password %q", password)
-	}
-	if passcredentialsall != false {
-		t.Errorf("Unexpected passcredentialsall %t", passcredentialsall)
-	}
-	if insecureSkipTLSVerify {
-		t.Errorf("Unexpected insecureSkipTLSVerify %t", insecureSkipTLSVerify)
+	dep := &chart.Dependency{
+		Name:       "alpine",
+		Version:    "0.1.0",
+		Repository: "http://example.com/charts",
+		ChartURL:   "https://charts.helm.sh/stable/alpine-0.1.0.tgzFromMap",
 	}
 
-	urlMap[urlKey] = "alpine-0.1.0.tgz"
-
-	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos, urlMap)
+	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(dep, repos)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	if churl != "http://example.com/charts/alpine-0.1.0.tgz" {
+	if churl != "https://charts.helm.sh/stable/alpine-0.1.0.tgzFromMap" {
 		t.Errorf("Unexpected URL %q", churl)
 	}
 	if username != "" {
@@ -329,7 +311,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)
 	}
 
@@ -358,7 +340,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")
 	}