diff --git a/internal/chart/v3/dependency.go b/internal/chart/v3/dependency.go index 2d956b548..68b90131e 100644 --- a/internal/chart/v3/dependency.go +++ b/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 diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 3efe94f10..ac2e03789 100644 --- a/internal/resolver/resolver.go +++ b/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 } } diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 1e33837a9..7f0b4768c 100644 --- a/internal/resolver/resolver_test.go +++ b/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 { diff --git a/pkg/chart/v2/dependency.go b/pkg/chart/v2/dependency.go index 8a590a036..73435599b 100644 --- a/pkg/chart/v2/dependency.go +++ b/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 diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index d41b8fdb4..eb28df83d 100644 --- a/pkg/downloader/manager.go +++ b/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 } diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index 9e27f183f..41be7670a 100644 --- a/pkg/downloader/manager_test.go +++ b/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{