diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 13a7baeb2..131b7098f 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -275,7 +275,11 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { fmt.Fprintf(m.Out, "Saving %d charts\n", len(deps)) var saveError error - churls := make(map[string]struct{}) + // Downloads are deduplicated by digest as well as URL, so an archive that + // another repository listed under a different digest, or none, is still + // checked against this one. + type download struct{ url, digest string } + churls := make(map[download]struct{}) for _, dep := range deps { // No repository means the chart is in charts directory if dep.Repository == "" { @@ -324,7 +328,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { break } - if _, ok := churls[churl]; ok { + if _, ok := churls[download{churl, digest}]; ok { fmt.Fprintf(m.Out, "Already downloaded %s from repo %s\n", dep.Name, dep.Repository) continue } @@ -365,7 +369,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { break } - churls[churl] = struct{}{} + churls[download{churl, digest}] = struct{}{} } // TODO: this should probably be refactored to be a []error, so we can capture and provide more information rather than "last error wins". diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index 4681bb217..524b9c46c 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -18,6 +18,8 @@ package downloader import ( "bytes" "io/fs" + "net/http" + "net/http/httptest" "os" "path/filepath" "testing" @@ -699,3 +701,41 @@ func TestDownloadAll_RejectsDependencyNotMatchingIndexDigest(t *testing.T) { }) } } + +func TestDownloadAll_ChecksEachIndexDigestForSharedURL(t *testing.T) { + ensure.HelmHome(t) + contentCache := t.TempDir() + srv, _, _ := tamperedChartServer(t, contentCache) + + // A second repository lists the same archive URL without a digest. When + // its entry is downloaded first, the one that does carry a digest must + // still be checked rather than skipped as already downloaded. + idx, err := repo.LoadIndexFile(filepath.Join(srv.Root(), "index.yaml")) + require.NoError(t, err) + for _, cv := range idx.Entries["signtest"] { + cv.Digest = "" + } + noDigest, err := yaml.Marshal(idx) + require.NoError(t, err) + other := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(noDigest) + })) + t.Cleanup(other.Close) + + chartPath := t.TempDir() + m := &Manager{ + Out: new(bytes.Buffer), + ChartPath: chartPath, + RepositoryConfig: filepath.Join(t.TempDir(), "repositories.yaml"), + RepositoryCache: srv.Root(), + ContentCache: contentCache, + Getters: getter.All(&cli.EnvSettings{}), + } + deps := []*chart.Dependency{ + {Name: "signtest", Repository: other.URL, Version: "0.1.0"}, + {Name: "signtest", Repository: srv.URL(), Version: "0.1.0", Alias: "signtest-indexed"}, + } + + err = m.downloadAll(deps) + require.ErrorContains(t, err, "does not match the digest recorded for it in the repository index") +}