Merge 6fa46f47a3 into 5b43b744b8

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

@@ -58,6 +58,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 	// Now we clone the dependencies, locking as we go.
 	locked := make([]*chart.Dependency, len(reqs))
 	missing := []string{}
+	chartRepoIndexURLs := make(map[string]*repo.IndexFile)
 	for i, d := range reqs {
 		constraint, err := semver.NewConstraint(d.Version)
 		if err != nil {
@@ -123,9 +124,21 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 		var ok bool
 		found := true
 		if !registry.IsOCI(d.Repository) {
-			repoIndex, err := repo.LoadIndexFile(filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName)))
-			if err != nil {
-				return nil, fmt.Errorf("no cached repository for %s found. (try 'helm repo update'): %w", repoName, err)
+			repoFilepath := 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 := chartRepoIndexURLs[repoFilepath]; !loaded {
+				var err error
+				repoIndex, err = repo.LoadIndexFile(repoFilepath)
+				chartRepoIndexURLs[repoFilepath] = repoIndex
+				if err != nil {
+					return nil, fmt.Errorf("no cached repository for %s found. (try 'helm repo update'): %w", repoName, err)
+				}
+			} else {
+				repoIndex = indexFile
 			}
 
 			vs, ok = repoIndex.Entries[d.Name]
@@ -183,7 +196,14 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 			}
 			if constraint.Check(v) {
 				found = true
+				if len(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
 			}
 		}

internal/resolver/resolver_test.go

@@ -81,7 +81,20 @@ 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"},
+				},
+			},
+		},
+		{
+			name: "two dependencies same repo",
+			req: []*chart.Dependency{
+				{Name: "alpine", Repository: "http://example.com", Version: ">=0.1.0"},
+				{Name: "mariadb", Repository: "http://example.com", Version: ">=0.1.0"},
+			},
+			expect: &chart.Lock{
+				Dependencies: []*chart.Dependency{
+					{Name: "alpine", Repository: "http://example.com", Version: "0.2.0", ChartURL: "https://charts.helm.sh/stable/alpine-0.1.0.tgz"},
+					{Name: "mariadb", Repository: "http://example.com", Version: "0.3.0", ChartURL: "https://charts.helm.sh/stable/mariadb-0.3.0.tgz"},
 				},
 			},
 		},
@@ -139,7 +152,7 @@ func TestResolve(t *testing.T) {
 		},
 	}
 
-	repoNames := map[string]string{"alpine": "kubernetes-charts", "redis": "kubernetes-charts"}
+	repoNames := map[string]string{"alpine": "kubernetes-charts", "redis": "kubernetes-charts", "mariadb": "kubernetes-charts"}
 	registryClient, _ := registry.NewClient()
 	r := New("testdata/chartpath", "testdata/repository", registryClient)
 	for _, tt := range tests {

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

@@ -232,7 +232,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.
+// 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, error) {
 	res := resolver.New(m.ChartPath, m.RepositoryCache, m.RegistryClient)
 	return res.Resolve(req, repoNames)
@@ -315,7 +315,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
 
 		// 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)
+		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
@@ -505,6 +505,7 @@ 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
@@ -532,6 +533,14 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart.
 
 		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
@@ -717,23 +726,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
-//
 // 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) (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
@@ -742,12 +749,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
@@ -763,11 +770,17 @@ func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*
 			return
 		}
 	}
-	url, err = repo.FindChartInRepoURL(repoURL, name, m.Getters, repo.WithChartVersion(version), repo.WithClientTLS(certFile, keyFile, caFile))
+
+	if dep.ChartURL != "" {
+		url = dep.ChartURL
+	} else {
+		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)
+	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)
+	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,6 +149,44 @@ func TestFindChartURL(t *testing.T) {
 	}
 }
 
+func TestFindChartURLAlreadyKnownUrl(t *testing.T) {
+	var b bytes.Buffer
+	m := &Manager{
+		Out:              &b,
+		RepositoryConfig: repoConfig,
+		RepositoryCache:  repoCache,
+	}
+	repos := make(map[string]*repo.ChartRepository)
+
+	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",
+	}
+
+	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(dep, repos)
+	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)
+	}
+}
+
 func TestGetRepoNames(t *testing.T) {
 	b := bytes.NewBuffer(nil)
 	m := &Manager{