From 24fa3d910d774b9d7f40f1fc8002bc1fb55565ca Mon Sep 17 00:00:00 2001 From: CI Date: Thu, 21 Jul 2022 12:29:14 -0400 Subject: [PATCH 1/5] fix: clean up temp files in FindChartInAuthAndTLSAndPassRepoURL (#11171) Signed-off-by: CI --- cmd/helm/repo_add.go | 2 +- cmd/helm/repo_update.go | 2 +- pkg/downloader/manager.go | 2 +- pkg/repo/chartrepo.go | 17 ++++++++++------- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/cmd/helm/repo_add.go b/cmd/helm/repo_add.go index e13df7ad8..a4743d605 100644 --- a/cmd/helm/repo_add.go +++ b/cmd/helm/repo_add.go @@ -207,7 +207,7 @@ func (o *repoAddOptions) run(out io.Writer) error { if o.repoCache != "" { r.CachePath = o.repoCache } - if _, err := r.DownloadIndexFile(); err != nil { + if _, _, err := r.DownloadIndexFile(); err != nil { return errors.Wrapf(err, "looks like %q is not a valid chart repository or cannot be reached", o.url) } diff --git a/cmd/helm/repo_update.go b/cmd/helm/repo_update.go index 27661674c..122293253 100644 --- a/cmd/helm/repo_update.go +++ b/cmd/helm/repo_update.go @@ -122,7 +122,7 @@ func updateCharts(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdate wg.Add(1) go func(re *repo.ChartRepository) { defer wg.Done() - if _, err := re.DownloadIndexFile(); err != nil { + if _, _, err := re.DownloadIndexFile(); err != nil { fmt.Fprintf(out, "...Unable to get an update from the %q chart repository (%s):\n\t%s\n", re.Config.Name, re.Config.URL, err) repoFailList = append(repoFailList, re.Config.URL) } else { diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 18b28dde1..226aa1867 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -670,7 +670,7 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error { } wg.Add(1) go func(r *repo.ChartRepository) { - if _, err := r.DownloadIndexFile(); err != nil { + if _, _, err := r.DownloadIndexFile(); err != nil { // For those dependencies that are not known to helm and using a // generated key name we display the repo url. if strings.HasPrefix(r.Config.Name, managerKeyPrefix) { diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index 956997cc9..3a5f34eeb 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -115,10 +115,10 @@ func (r *ChartRepository) Load() error { } // DownloadIndexFile fetches the index from a repository. -func (r *ChartRepository) DownloadIndexFile() (string, error) { +func (r *ChartRepository) DownloadIndexFile() (string, string, error) { parsedURL, err := url.Parse(r.Config.URL) if err != nil { - return "", err + return "", "", err } parsedURL.RawPath = path.Join(parsedURL.RawPath, "index.yaml") parsedURL.Path = path.Join(parsedURL.Path, "index.yaml") @@ -133,17 +133,17 @@ func (r *ChartRepository) DownloadIndexFile() (string, error) { getter.WithPassCredentialsAll(r.Config.PassCredentialsAll), ) if err != nil { - return "", err + return "", "", err } index, err := ioutil.ReadAll(resp) if err != nil { - return "", err + return "", "", err } indexFile, err := loadIndex(index, r.Config.URL) if err != nil { - return "", err + return "", "", err } // Create the chart list file in the cache directory @@ -158,7 +158,7 @@ func (r *ChartRepository) DownloadIndexFile() (string, error) { // Create the index file in the cache directory fname := filepath.Join(r.CachePath, helmpath.CacheIndexFile(r.Config.Name)) os.MkdirAll(filepath.Dir(fname), 0755) - return fname, ioutil.WriteFile(fname, index, 0644) + return fname, chartsFile, ioutil.WriteFile(fname, index, 0644) } // Index generates an index for the chart repository and writes an index.yaml file. @@ -249,10 +249,13 @@ func FindChartInAuthAndTLSAndPassRepoURL(repoURL, username, password, chartName, if err != nil { return "", err } - idx, err := r.DownloadIndexFile() + idx, chart, err := r.DownloadIndexFile() if err != nil { return "", errors.Wrapf(err, "looks like %q is not a valid chart repository or cannot be reached", repoURL) } + // Clean up temp files. + defer os.RemoveAll(idx) + defer os.RemoveAll(chart) // Read the index file for the repository to get chart information and return chart URL repoIndex, err := LoadIndexFile(idx) From 32a41fcfac9ca1b4f4997a6660bacba9a01a9d45 Mon Sep 17 00:00:00 2001 From: CI Date: Thu, 21 Jul 2022 12:42:56 -0400 Subject: [PATCH 2/5] fix tests Signed-off-by: CI --- pkg/repo/chartrepo_test.go | 2 +- pkg/repo/index_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/repo/chartrepo_test.go b/pkg/repo/chartrepo_test.go index 0f317b2fd..6aa36ba98 100644 --- a/pkg/repo/chartrepo_test.go +++ b/pkg/repo/chartrepo_test.go @@ -156,7 +156,7 @@ func TestIndexCustomSchemeDownload(t *testing.T) { } defer os.Remove(tempIndexFile.Name()) - idx, err := repo.DownloadIndexFile() + idx, _, err := repo.DownloadIndexFile() if err != nil { t.Fatalf("Failed to download index file to %s: %v", idx, err) } diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go index a75a4177a..99508ca81 100644 --- a/pkg/repo/index_test.go +++ b/pkg/repo/index_test.go @@ -237,7 +237,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Errorf("Problem creating chart repository from %s: %v", testRepo, err) } - idx, err := r.DownloadIndexFile() + idx, _, err := r.DownloadIndexFile() if err != nil { t.Fatalf("Failed to download index file to %s: %#v", idx, err) } @@ -290,7 +290,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Errorf("Problem creating chart repository from %s: %v", testRepo, err) } - idx, err := r.DownloadIndexFile() + idx, _, err := r.DownloadIndexFile() if err != nil { t.Fatalf("Failed to download index file to %s: %#v", idx, err) } From cd76fcd80557490d2f2ee1204b1bdbf78c738ec9 Mon Sep 17 00:00:00 2001 From: CI Date: Fri, 22 Jul 2022 12:58:54 -0400 Subject: [PATCH 3/5] avoid adding new public function Signed-off-by: CI --- cmd/helm/repo_add.go | 2 +- cmd/helm/repo_update.go | 2 +- pkg/downloader/manager.go | 2 +- pkg/repo/chartrepo.go | 19 ++++++++++--------- pkg/repo/chartrepo_test.go | 2 +- pkg/repo/index_test.go | 4 ++-- 6 files changed, 16 insertions(+), 15 deletions(-) diff --git a/cmd/helm/repo_add.go b/cmd/helm/repo_add.go index a4743d605..e13df7ad8 100644 --- a/cmd/helm/repo_add.go +++ b/cmd/helm/repo_add.go @@ -207,7 +207,7 @@ func (o *repoAddOptions) run(out io.Writer) error { if o.repoCache != "" { r.CachePath = o.repoCache } - if _, _, err := r.DownloadIndexFile(); err != nil { + if _, err := r.DownloadIndexFile(); err != nil { return errors.Wrapf(err, "looks like %q is not a valid chart repository or cannot be reached", o.url) } diff --git a/cmd/helm/repo_update.go b/cmd/helm/repo_update.go index 122293253..27661674c 100644 --- a/cmd/helm/repo_update.go +++ b/cmd/helm/repo_update.go @@ -122,7 +122,7 @@ func updateCharts(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdate wg.Add(1) go func(re *repo.ChartRepository) { defer wg.Done() - if _, _, err := re.DownloadIndexFile(); err != nil { + if _, err := re.DownloadIndexFile(); err != nil { fmt.Fprintf(out, "...Unable to get an update from the %q chart repository (%s):\n\t%s\n", re.Config.Name, re.Config.URL, err) repoFailList = append(repoFailList, re.Config.URL) } else { diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 226aa1867..18b28dde1 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -670,7 +670,7 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error { } wg.Add(1) go func(r *repo.ChartRepository) { - if _, _, err := r.DownloadIndexFile(); err != nil { + if _, err := r.DownloadIndexFile(); err != nil { // For those dependencies that are not known to helm and using a // generated key name we display the repo url. if strings.HasPrefix(r.Config.Name, managerKeyPrefix) { diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index 3a5f34eeb..414e0a714 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -115,10 +115,10 @@ func (r *ChartRepository) Load() error { } // DownloadIndexFile fetches the index from a repository. -func (r *ChartRepository) DownloadIndexFile() (string, string, error) { +func (r *ChartRepository) DownloadIndexFile() (string, error) { parsedURL, err := url.Parse(r.Config.URL) if err != nil { - return "", "", err + return "", err } parsedURL.RawPath = path.Join(parsedURL.RawPath, "index.yaml") parsedURL.Path = path.Join(parsedURL.Path, "index.yaml") @@ -133,17 +133,17 @@ func (r *ChartRepository) DownloadIndexFile() (string, string, error) { getter.WithPassCredentialsAll(r.Config.PassCredentialsAll), ) if err != nil { - return "", "", err + return "", err } index, err := ioutil.ReadAll(resp) if err != nil { - return "", "", err + return "", err } indexFile, err := loadIndex(index, r.Config.URL) if err != nil { - return "", "", err + return "", err } // Create the chart list file in the cache directory @@ -158,7 +158,7 @@ func (r *ChartRepository) DownloadIndexFile() (string, string, error) { // Create the index file in the cache directory fname := filepath.Join(r.CachePath, helmpath.CacheIndexFile(r.Config.Name)) os.MkdirAll(filepath.Dir(fname), 0755) - return fname, chartsFile, ioutil.WriteFile(fname, index, 0644) + return fname, ioutil.WriteFile(fname, index, 0644) } // Index generates an index for the chart repository and writes an index.yaml file. @@ -249,13 +249,14 @@ func FindChartInAuthAndTLSAndPassRepoURL(repoURL, username, password, chartName, if err != nil { return "", err } - idx, chart, err := r.DownloadIndexFile() + // Use a random path name so it can be deleted for cleanup. + r.CachePath = helmpath.CachePath(name) + idx, err := r.DownloadIndexFile() if err != nil { return "", errors.Wrapf(err, "looks like %q is not a valid chart repository or cannot be reached", repoURL) } // Clean up temp files. - defer os.RemoveAll(idx) - defer os.RemoveAll(chart) + defer os.RemoveAll(r.CachePath) // Read the index file for the repository to get chart information and return chart URL repoIndex, err := LoadIndexFile(idx) diff --git a/pkg/repo/chartrepo_test.go b/pkg/repo/chartrepo_test.go index 6aa36ba98..0f317b2fd 100644 --- a/pkg/repo/chartrepo_test.go +++ b/pkg/repo/chartrepo_test.go @@ -156,7 +156,7 @@ func TestIndexCustomSchemeDownload(t *testing.T) { } defer os.Remove(tempIndexFile.Name()) - idx, _, err := repo.DownloadIndexFile() + idx, err := repo.DownloadIndexFile() if err != nil { t.Fatalf("Failed to download index file to %s: %v", idx, err) } diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go index 99508ca81..a75a4177a 100644 --- a/pkg/repo/index_test.go +++ b/pkg/repo/index_test.go @@ -237,7 +237,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Errorf("Problem creating chart repository from %s: %v", testRepo, err) } - idx, _, err := r.DownloadIndexFile() + idx, err := r.DownloadIndexFile() if err != nil { t.Fatalf("Failed to download index file to %s: %#v", idx, err) } @@ -290,7 +290,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Errorf("Problem creating chart repository from %s: %v", testRepo, err) } - idx, _, err := r.DownloadIndexFile() + idx, err := r.DownloadIndexFile() if err != nil { t.Fatalf("Failed to download index file to %s: %#v", idx, err) } From 781ddba690afa20c80f443a121c3134f668dc43a Mon Sep 17 00:00:00 2001 From: CI Date: Fri, 29 Jul 2022 16:12:16 -0400 Subject: [PATCH 4/5] don't change r.CachePath Signed-off-by: CI --- pkg/repo/chartrepo.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index 414e0a714..69b2de9d1 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -249,14 +249,12 @@ func FindChartInAuthAndTLSAndPassRepoURL(repoURL, username, password, chartName, if err != nil { return "", err } - // Use a random path name so it can be deleted for cleanup. - r.CachePath = helmpath.CachePath(name) idx, err := r.DownloadIndexFile() if err != nil { return "", errors.Wrapf(err, "looks like %q is not a valid chart repository or cannot be reached", repoURL) } - // Clean up temp files. - defer os.RemoveAll(r.CachePath) + defer os.RemoveAll(filepath.Join(r.CachePath, helmpath.CacheChartsFile(r.Config.Name))) + defer os.RemoveAll(filepath.Join(r.CachePath, helmpath.CacheIndexFile(r.Config.Name))) // Read the index file for the repository to get chart information and return chart URL repoIndex, err := LoadIndexFile(idx) From 3b19ddeb56fae17a1d176130702ae5b779b20460 Mon Sep 17 00:00:00 2001 From: CI Date: Fri, 5 Aug 2022 10:02:18 -0400 Subject: [PATCH 5/5] one defer Signed-off-by: CI --- pkg/repo/chartrepo.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index 69b2de9d1..fce947e4c 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -253,8 +253,10 @@ func FindChartInAuthAndTLSAndPassRepoURL(repoURL, username, password, chartName, if err != nil { return "", errors.Wrapf(err, "looks like %q is not a valid chart repository or cannot be reached", repoURL) } - defer os.RemoveAll(filepath.Join(r.CachePath, helmpath.CacheChartsFile(r.Config.Name))) - defer os.RemoveAll(filepath.Join(r.CachePath, helmpath.CacheIndexFile(r.Config.Name))) + defer func() { + os.RemoveAll(filepath.Join(r.CachePath, helmpath.CacheChartsFile(r.Config.Name))) + os.RemoveAll(filepath.Join(r.CachePath, helmpath.CacheIndexFile(r.Config.Name))) + }() // Read the index file for the repository to get chart information and return chart URL repoIndex, err := LoadIndexFile(idx)