From c2fa72ebcdc9a12381eec29b642374af75f5b45c Mon Sep 17 00:00:00 2001 From: eyalbe4 Date: Wed, 25 Apr 2018 03:44:47 +0300 Subject: [PATCH 1/2] Fix for - Downloader plugins not used when downloading new repo's index.yaml #3938 --- pkg/downloader/chart_downloader.go | 73 ++++++++++++++---------------- pkg/repo/chartrepo.go | 18 +++++--- 2 files changed, 45 insertions(+), 46 deletions(-) diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 8b386fc09..6861a8270 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -23,6 +23,7 @@ import ( "net/url" "os" "path/filepath" + "reflect" "strings" "k8s.io/helm/pkg/getter" @@ -85,7 +86,7 @@ type ChartDownloader struct { // Returns a string path to the location where the file was downloaded and a verification // (if provenance was verified), or an error if something bad happened. func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *provenance.Verification, error) { - u, r, g, err := c.ResolveChartVersionAndGetRepo(ref, version) + u, g, err := c.ResolveChartVersion(ref, version) if err != nil { return "", nil, err } @@ -104,7 +105,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven // If provenance is requested, verify it. ver := &provenance.Verification{} if c.Verify > VerifyNever { - body, err := r.Client.Get(u.String() + ".prov") + body, err := g.Get(u.String() + ".prov") if err != nil { if c.Verify == VerifyAlways { return destfile, ver, fmt.Errorf("Failed to fetch provenance %q", u.String()+".prov") @@ -144,28 +145,14 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven // * If version is empty, this will return the URL for the latest version // * If no version can be found, an error is returned func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, getter.Getter, error) { - u, r, _, err := c.ResolveChartVersionAndGetRepo(ref, version) - if r != nil { - return u, r.Client, err - } - return u, nil, err -} - -// ResolveChartVersionAndGetRepo is the same as the ResolveChartVersion method, but returns the chart repositoryy. -func (c *ChartDownloader) ResolveChartVersionAndGetRepo(ref, version string) (*url.URL, *repo.ChartRepository, *getter.HttpGetter, error) { u, err := url.Parse(ref) if err != nil { - return nil, nil, nil, fmt.Errorf("invalid chart URL format: %s", ref) + return nil, nil, fmt.Errorf("invalid chart URL format: %s", ref) } rf, err := repo.LoadRepositoriesFile(c.HelmHome.RepositoryFile()) if err != nil { - return u, nil, nil, err - } - - g, err := getter.NewHTTPGetter(ref, "", "", "") - if err != nil { - return u, nil, nil, err + return u, nil, err } if u.IsAbs() && len(u.Host) > 0 && len(u.Path) > 0 { @@ -180,23 +167,26 @@ func (c *ChartDownloader) ResolveChartVersionAndGetRepo(ref, version string) (*u // If there is no special config, return the default HTTP client and // swallow the error. if err == ErrNoOwnerRepo { - r := &repo.ChartRepository{} - r.Client = g - g.SetCredentials(c.getRepoCredentials(r)) - return u, r, g, nil + getterConstructor, err := c.Getters.ByScheme(u.Scheme) + if err != nil { + return u, nil, err + } + getter, err := getterConstructor(ref, "", "", "") + return u, getter, err } - return u, nil, nil, err + return u, nil, err } r, err := repo.NewChartRepository(rc, c.Getters) + c.setCredentials(r) // If we get here, we don't need to go through the next phase of looking // up the URL. We have it already. So we just return. - return u, r, g, err + return u, r.Client, err } // See if it's of the form: repo/path_to_chart p := strings.SplitN(u.Path, "/", 2) if len(p) < 2 { - return u, nil, nil, fmt.Errorf("Non-absolute URLs should be in form of repo_name/path_to_chart, got: %s", u) + return u, nil, fmt.Errorf("Non-absolute URLs should be in form of repo_name/path_to_chart, got: %s", u) } repoName := p[0] @@ -204,56 +194,59 @@ func (c *ChartDownloader) ResolveChartVersionAndGetRepo(ref, version string) (*u rc, err := pickChartRepositoryConfigByName(repoName, rf.Repositories) if err != nil { - return u, nil, nil, err + return u, nil, err } r, err := repo.NewChartRepository(rc, c.Getters) if err != nil { - return u, nil, nil, err + return u, nil, err } - g.SetCredentials(c.getRepoCredentials(r)) + c.setCredentials(r) // Next, we need to load the index, and actually look up the chart. i, err := repo.LoadIndexFile(c.HelmHome.CacheIndex(r.Config.Name)) if err != nil { - return u, r, g, fmt.Errorf("no cached repo found. (try 'helm repo update'). %s", err) + return u, r.Client, fmt.Errorf("no cached repo found. (try 'helm repo update'). %s", err) } cv, err := i.Get(chartName, version) if err != nil { - return u, r, g, fmt.Errorf("chart %q matching %s not found in %s index. (try 'helm repo update'). %s", chartName, version, r.Config.Name, err) + return u, r.Client, fmt.Errorf("chart %q matching %s not found in %s index. (try 'helm repo update'). %s", chartName, version, r.Config.Name, err) } if len(cv.URLs) == 0 { - return u, r, g, fmt.Errorf("chart %q has no downloadable URLs", ref) + return u, r.Client, fmt.Errorf("chart %q has no downloadable URLs", ref) } // TODO: Seems that picking first URL is not fully correct u, err = url.Parse(cv.URLs[0]) if err != nil { - return u, r, g, fmt.Errorf("invalid chart URL format: %s", ref) + return u, r.Client, fmt.Errorf("invalid chart URL format: %s", ref) } // If the URL is relative (no scheme), prepend the chart repo's base URL if !u.IsAbs() { repoURL, err := url.Parse(rc.URL) if err != nil { - return repoURL, r, nil, err + return repoURL, r.Client, err } q := repoURL.Query() // We need a trailing slash for ResolveReference to work, but make sure there isn't already one repoURL.Path = strings.TrimSuffix(repoURL.Path, "/") + "/" u = repoURL.ResolveReference(u) u.RawQuery = q.Encode() - g, err := getter.NewHTTPGetter(rc.URL, "", "", "") - if err != nil { - return repoURL, r, nil, err - } - g.SetCredentials(c.getRepoCredentials(r)) - return u, r, g, err + return u, r.Client, err } - return u, r, g, nil + return u, r.Client, nil +} + +// If HttpGetter is used, this method sets the configured repository credentials on the HttpGetter. +func (c *ChartDownloader) setCredentials(r *repo.ChartRepository) { + var t *getter.HttpGetter + if reflect.TypeOf(r.Client) == reflect.TypeOf(t) { + r.Client.(*getter.HttpGetter).SetCredentials(c.getRepoCredentials(r)) + } } // If this ChartDownloader is not configured to use credentials, and the chart repository sent as an argument is, diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index bf03a68bb..ba1e6f4ff 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -22,6 +22,7 @@ import ( "net/url" "os" "path/filepath" + "reflect" "strings" "github.com/ghodss/yaml" @@ -119,12 +120,9 @@ func (r *ChartRepository) DownloadIndexFile(cachePath string) error { parsedURL.Path = strings.TrimSuffix(parsedURL.Path, "/") + "/index.yaml" indexURL = parsedURL.String() - g, err := getter.NewHTTPGetter(indexURL, r.Config.CertFile, r.Config.KeyFile, r.Config.CAFile) - if err != nil { - return err - } - g.SetCredentials(r.Config.Username, r.Config.Password) - resp, err := g.Get(indexURL) + + r.setCredentials() + resp, err := r.Client.Get(indexURL) if err != nil { return err } @@ -152,6 +150,14 @@ func (r *ChartRepository) DownloadIndexFile(cachePath string) error { return ioutil.WriteFile(cp, index, 0644) } +// If HttpGetter is used, this method sets the configured repository credentials on the HttpGetter. +func (r *ChartRepository) setCredentials() { + var t *getter.HttpGetter + if reflect.TypeOf(r.Client) == reflect.TypeOf(t) { + r.Client.(*getter.HttpGetter).SetCredentials(r.Config.Username, r.Config.Password) + } +} + // Index generates an index for the chart repository and writes an index.yaml file. func (r *ChartRepository) Index() error { err := r.generateIndex() From d9395bcc0668429a48cdfe8ff2389bb9d00f4e0e Mon Sep 17 00:00:00 2001 From: Matthew Fisher Date: Wed, 25 Apr 2018 14:55:05 -0700 Subject: [PATCH 2/2] remove need for type reflection --- pkg/downloader/chart_downloader.go | 6 ++---- pkg/repo/chartrepo.go | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 6861a8270..59b9d4d75 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -23,7 +23,6 @@ import ( "net/url" "os" "path/filepath" - "reflect" "strings" "k8s.io/helm/pkg/getter" @@ -243,9 +242,8 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, ge // If HttpGetter is used, this method sets the configured repository credentials on the HttpGetter. func (c *ChartDownloader) setCredentials(r *repo.ChartRepository) { - var t *getter.HttpGetter - if reflect.TypeOf(r.Client) == reflect.TypeOf(t) { - r.Client.(*getter.HttpGetter).SetCredentials(c.getRepoCredentials(r)) + if t, ok := r.Client.(*getter.HttpGetter); ok { + t.SetCredentials(c.getRepoCredentials(r)) } } diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index ba1e6f4ff..438f66d7c 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -22,7 +22,6 @@ import ( "net/url" "os" "path/filepath" - "reflect" "strings" "github.com/ghodss/yaml" @@ -152,9 +151,8 @@ func (r *ChartRepository) DownloadIndexFile(cachePath string) error { // If HttpGetter is used, this method sets the configured repository credentials on the HttpGetter. func (r *ChartRepository) setCredentials() { - var t *getter.HttpGetter - if reflect.TypeOf(r.Client) == reflect.TypeOf(t) { - r.Client.(*getter.HttpGetter).SetCredentials(r.Config.Username, r.Config.Password) + if t, ok := r.Client.(*getter.HttpGetter); ok { + t.SetCredentials(r.Config.Username, r.Config.Password) } }