From 4829fad3a39c86f3062ca2cd839b718246b43f79 Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Wed, 15 Feb 2017 12:30:13 -0700 Subject: [PATCH] fix(helm): fix broken cache paths in repositories A regression was committed during 2.2.0 that broke the repositories.yaml file format, switching the cache path from relative to absolute. This fixes the error. Closes #1974 --- cmd/helm/home.go | 13 ++++++++++++- cmd/helm/init.go | 4 +++- cmd/helm/repo_add.go | 2 +- cmd/helm/repo_update.go | 10 +++++----- cmd/helm/repo_update_test.go | 4 ++-- pkg/downloader/manager.go | 5 ++++- pkg/repo/chartrepo.go | 19 +++++++++++++++++-- pkg/repo/index_test.go | 2 +- 8 files changed, 45 insertions(+), 14 deletions(-) diff --git a/cmd/helm/home.go b/cmd/helm/home.go index 9dfe6a373..59344f075 100644 --- a/cmd/helm/home.go +++ b/cmd/helm/home.go @@ -21,6 +21,7 @@ import ( "io" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/helmpath" ) var longHomeHelp = ` @@ -34,7 +35,17 @@ func newHomeCmd(out io.Writer) *cobra.Command { Short: "displays the location of HELM_HOME", Long: longHomeHelp, Run: func(cmd *cobra.Command, args []string) { - fmt.Fprintf(out, homePath()+"\n") + h := helmpath.Home(homePath()) + fmt.Fprintf(out, "%s\n", h) + if flagDebug { + fmt.Fprintf(out, "Repository: %s\n", h.Repository()) + fmt.Fprintf(out, "RepositoryFile: %s\n", h.RepositoryFile()) + fmt.Fprintf(out, "Cache: %s\n", h.Cache()) + fmt.Fprintf(out, "Stable CacheIndex: %s\n", h.CacheIndex("stable")) + fmt.Fprintf(out, "Starters: %s\n", h.Starters()) + fmt.Fprintf(out, "LocalRepository: %s\n", h.LocalRepository()) + fmt.Fprintf(out, "Plugins: %s\n", h.Plugins()) + } }, } diff --git a/cmd/helm/init.go b/cmd/helm/init.go index 766cb8e62..dd6b949e3 100644 --- a/cmd/helm/init.go +++ b/cmd/helm/init.go @@ -228,7 +228,9 @@ func initStableRepo(cacheFile string) (*repo.Entry, error) { return nil, err } - if err := r.DownloadIndexFile(); err != nil { + // In this case, the cacheFile is always absolute. So passing empty string + // is safe. + if err := r.DownloadIndexFile(""); err != nil { return nil, fmt.Errorf("Looks like %q is not a valid chart repository or cannot be reached: %s", stableRepositoryURL, err.Error()) } diff --git a/cmd/helm/repo_add.go b/cmd/helm/repo_add.go index b349537a7..ec764ada9 100644 --- a/cmd/helm/repo_add.go +++ b/cmd/helm/repo_add.go @@ -102,7 +102,7 @@ func addRepository(name, url string, home helmpath.Home, certFile, keyFile, caFi return err } - if err := r.DownloadIndexFile(); err != nil { + if err := r.DownloadIndexFile(home.Cache()); err != nil { return fmt.Errorf("Looks like %q is not a valid chart repository or cannot be reached: %s", url, err.Error()) } diff --git a/cmd/helm/repo_update.go b/cmd/helm/repo_update.go index 8dfab9a5d..8e564c98b 100644 --- a/cmd/helm/repo_update.go +++ b/cmd/helm/repo_update.go @@ -41,7 +41,7 @@ var ( ) type repoUpdateCmd struct { - update func([]*repo.ChartRepository, io.Writer) + update func([]*repo.ChartRepository, io.Writer, helmpath.Home) home helmpath.Home out io.Writer } @@ -82,11 +82,11 @@ func (u *repoUpdateCmd) run() error { repos = append(repos, r) } - u.update(repos, u.out) + u.update(repos, u.out, u.home) return nil } -func updateCharts(repos []*repo.ChartRepository, out io.Writer) { +func updateCharts(repos []*repo.ChartRepository, out io.Writer, home helmpath.Home) { fmt.Fprintln(out, "Hang tight while we grab the latest from your chart repositories...") var wg sync.WaitGroup for _, re := range repos { @@ -94,10 +94,10 @@ func updateCharts(repos []*repo.ChartRepository, out io.Writer) { go func(re *repo.ChartRepository) { defer wg.Done() if re.Config.Name == localRepository { - fmt.Fprintf(out, "...Skip %s chart repository", re.Config.Name) + fmt.Fprintf(out, "...Skip %s chart repository\n", re.Config.Name) return } - err := re.DownloadIndexFile() + err := re.DownloadIndexFile(home.Cache()) if 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) } else { diff --git a/cmd/helm/repo_update_test.go b/cmd/helm/repo_update_test.go index 97f023f13..e32e92fbb 100644 --- a/cmd/helm/repo_update_test.go +++ b/cmd/helm/repo_update_test.go @@ -43,7 +43,7 @@ func TestUpdateCmd(t *testing.T) { out := bytes.NewBuffer(nil) // Instead of using the HTTP updater, we provide our own for this test. // The TestUpdateCharts test verifies the HTTP behavior independently. - updater := func(repos []*repo.ChartRepository, out io.Writer) { + updater := func(repos []*repo.ChartRepository, out io.Writer, hh helmpath.Home) { for _, re := range repos { fmt.Fprintln(out, re.Config.Name) } @@ -90,7 +90,7 @@ func TestUpdateCharts(t *testing.T) { } b := bytes.NewBuffer(nil) - updateCharts([]*repo.ChartRepository{r}, b) + updateCharts([]*repo.ChartRepository{r}, b, hh) got := b.String() if strings.Contains(got, "Unable to get an update") { diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 9a830c4d5..cc267abd1 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -30,7 +30,10 @@ import ( "github.com/Masterminds/semver" "github.com/ghodss/yaml" + // FIXME: This violates the package rules. A `cmd` should not be imported by + // something in 'pkg' "k8s.io/helm/cmd/helm/helmpath" + "k8s.io/helm/pkg/chartutil" "k8s.io/helm/pkg/proto/hapi/chart" "k8s.io/helm/pkg/repo" @@ -375,7 +378,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(m.HelmHome.Cache()); err != nil { fmt.Fprintf(out, "...Unable to get an update from the %q chart repository (%s):\n\t%s\n", r.Config.Name, r.Config.URL, err) } else { fmt.Fprintf(out, "...Successfully got an update from the %q chart repository\n", r.Config.Name) diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index 6e2a3676c..4a0751f57 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -129,7 +129,10 @@ func (r *ChartRepository) Load() error { } // DownloadIndexFile fetches the index from a repository. -func (r *ChartRepository) DownloadIndexFile() error { +// +// cachePath is prepended to any index that does not have an absolute path. This +// is for pre-2.2.0 repo files. +func (r *ChartRepository) DownloadIndexFile(cachePath string) error { var indexURL string indexURL = strings.TrimSuffix(r.Config.URL, "/") + "/index.yaml" @@ -148,7 +151,19 @@ func (r *ChartRepository) DownloadIndexFile() error { return err } - return ioutil.WriteFile(r.Config.Cache, index, 0644) + // In Helm 2.2.0 the config.cache was accidentally switched to an absolute + // path, which broke backward compatibility. This fixes it by prepending a + // global cache path to relative paths. + // + // It is changed on DownloadIndexFile because that was the method that + // originally carried the cache path. + cp := r.Config.Cache + if !filepath.IsAbs(cp) { + cp = filepath.Join(cachePath, cp) + } + println("Writing to", cp) + + return ioutil.WriteFile(cp, index, 0644) } // Index generates an index for the chart repository and writes an index.yaml file. diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go index 2f23950f1..ccf6cfd3c 100644 --- a/pkg/repo/index_test.go +++ b/pkg/repo/index_test.go @@ -142,7 +142,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Errorf("Problem creating chart repository from %s: %v", testRepo, err) } - if err := r.DownloadIndexFile(); err != nil { + if err := r.DownloadIndexFile(""); err != nil { t.Errorf("%#v", err) }