diff --git a/pkg/action/install.go b/pkg/action/install.go index 1d8a89ddb..e64b218dd 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -630,8 +630,9 @@ func (c *ChartPathOptions) LocateChart(name string, settings cli.EnvSettings) (s Out: os.Stdout, Keyring: c.Keyring, Getters: getter.All(settings), - Username: c.Username, - Password: c.Password, + Options: []getter.Option{ + getter.WithBasicAuth(c.Username, c.Password), + }, } if c.Verify { dl.Verify = downloader.VerifyAlways diff --git a/pkg/action/pull.go b/pkg/action/pull.go index 8925c7190..812c00720 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -62,8 +62,9 @@ func (p *Pull) Run(chartRef string) (string, error) { Keyring: p.Keyring, Verify: downloader.VerifyNever, Getters: getter.All(p.Settings), - Username: p.Username, - Password: p.Password, + Options: []getter.Option{ + getter.WithBasicAuth(p.Username, p.Password), + }, } if p.Verify { diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 918a0fce9..ba24e570d 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -68,10 +68,8 @@ type ChartDownloader struct { HelmHome helmpath.Home // Getter collection for the operation Getters getter.Providers - // Chart repository username - Username string - // Chart repository password - Password string + // Options provide parameters to be passed along to the Getter being initialized. + Options []getter.Option } // DownloadTo retrieves a chart. Depending on the settings, it may also download a provenance file. @@ -86,7 +84,17 @@ 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, g, err := c.ResolveChartVersion(ref, version) + u, err := c.ResolveChartVersion(ref, version) + if err != nil { + return "", nil, err + } + + constructor, err := c.Getters.ByScheme(u.Scheme) + if err != nil { + return "", nil, err + } + + g, err := constructor(c.Options...) if err != nil { return "", nil, err } @@ -132,8 +140,8 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven // ResolveChartVersion resolves a chart reference to a URL. // -// It returns the URL as well as a preconfigured repo.Getter that can fetch -// the URL. +// It returns the URL and sets the ChartDownloader's Options that can fetch +// the URL using the appropriate Getter. // // A reference may be an HTTP URL, a 'reponame/chartname' reference, or a local path. // @@ -144,21 +152,16 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven // * If version is non-empty, this will return the URL for that version // * 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) { +func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, error) { u, err := url.Parse(ref) if err != nil { - return nil, nil, errors.Errorf("invalid chart URL format: %s", ref) + return nil, errors.Errorf("invalid chart URL format: %s", ref) } + c.Options = append(c.Options, getter.WithURL(ref)) rf, err := repo.LoadFile(c.HelmHome.RepositoryFile()) if err != nil { - return u, nil, err - } - - // TODO add user-agent - g, err := getter.NewHTTPGetter(getter.WithURL(ref)) - if err != nil { - return u, nil, err + return u, err } if u.IsAbs() && len(u.Host) > 0 && len(u.Path) > 0 { @@ -173,24 +176,31 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, ge // 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.SetBasicAuth(c.getRepoCredentials(r)) - return u, g, nil + return u, nil } - return u, nil, err + return u, err } - r, err := repo.NewChartRepository(rc, c.Getters) // 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.Client, err + // up the URL. We have it already. So we just set the parameters and return. + c.Options = append( + c.Options, + getter.WithURL(rc.URL), + getter.WithTLSClientConfig(rc.CertFile, rc.KeyFile, rc.CAFile), + ) + if rc.Username != "" && rc.Password != "" { + c.Options = append( + c.Options, + getter.WithBasicAuth(rc.Username, rc.Password), + ) + } + return u, nil } // See if it's of the form: repo/path_to_chart p := strings.SplitN(u.Path, "/", 2) if len(p) < 2 { - return u, nil, errors.Errorf("non-absolute URLs should be in form of repo_name/path_to_chart, got: %s", u) + return u, errors.Errorf("non-absolute URLs should be in form of repo_name/path_to_chart, got: %s", u) } repoName := p[0] @@ -198,41 +208,43 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, ge rc, err := pickChartRepositoryConfigByName(repoName, rf.Repositories) if err != nil { - return u, nil, err + return u, err } r, err := repo.NewChartRepository(rc, c.Getters) if err != nil { - return u, nil, err + return u, err + } + if r != nil && r.Config != nil && r.Config.Username != "" && r.Config.Password != "" { + c.Options = append(c.Options, getter.WithBasicAuth(r.Config.Username, r.Config.Password)) } - g.SetBasicAuth(c.getRepoCredentials(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, g, errors.Wrap(err, "no cached repo found. (try 'helm repo update')") + return u, errors.Wrap(err, "no cached repo found. (try 'helm repo update')") } cv, err := i.Get(chartName, version) if err != nil { - return u, g, errors.Wrapf(err, "chart %q matching %s not found in %s index. (try 'helm repo update')", chartName, version, r.Config.Name) + return u, errors.Wrapf(err, "chart %q matching %s not found in %s index. (try 'helm repo update')", chartName, version, r.Config.Name) } if len(cv.URLs) == 0 { - return u, g, errors.Errorf("chart %q has no downloadable URLs", ref) + return u, errors.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, g, errors.Errorf("invalid chart URL format: %s", ref) + return u, errors.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, nil, err + return repoURL, err } q := repoURL.Query() // We need a trailing slash for ResolveReference to work, but make sure there isn't already one @@ -240,32 +252,17 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, ge u = repoURL.ResolveReference(u) u.RawQuery = q.Encode() // TODO add user-agent - g, err := getter.NewHTTPGetter(getter.WithURL(rc.URL)) - if err != nil { - return repoURL, nil, err + if _, err := getter.NewHTTPGetter(getter.WithURL(rc.URL)); err != nil { + return repoURL, err } - g.SetBasicAuth(c.getRepoCredentials(r)) - return u, g, err - } - - return u, g, nil -} - -// If this ChartDownloader is not configured to use credentials, and the chart repository sent as an argument is, -// then the repository's configured credentials are returned. -// Else, this ChartDownloader's credentials are returned. -func (c *ChartDownloader) getRepoCredentials(r *repo.ChartRepository) (username, password string) { - username = c.Username - password = c.Password - if r != nil && r.Config != nil { - if username == "" { - username = r.Config.Username - } - if password == "" { - password = r.Config.Password + if r != nil && r.Config != nil && r.Config.Username != "" && r.Config.Password != "" { + c.Options = append(c.Options, getter.WithBasicAuth(r.Config.Username, r.Config.Password)) } + return u, err } - return + + // TODO add user-agent + return u, nil } // VerifyChart takes a path to a chart archive and a keyring, and verifies the chart. diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go index 1128ac728..005a49554 100644 --- a/pkg/downloader/chart_downloader_test.go +++ b/pkg/downloader/chart_downloader_test.go @@ -16,11 +16,8 @@ limitations under the License. package downloader import ( - "fmt" "io/ioutil" "net/http" - "net/http/httptest" - "net/url" "os" "path/filepath" "testing" @@ -61,7 +58,7 @@ func TestResolveChartRef(t *testing.T) { } for _, tt := range tests { - u, _, err := c.ResolveChartVersion(tt.ref, tt.version) + u, err := c.ResolveChartVersion(tt.ref, tt.version) if err != nil { if tt.fail { continue @@ -87,78 +84,88 @@ func TestVerifyChart(t *testing.T) { } } -func TestDownload(t *testing.T) { - expect := "Call me Ishmael" - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, expect) - })) - defer srv.Close() - - provider, err := getter.ByScheme("http", cli.EnvSettings{}) - if err != nil { - t.Fatal("No http provider found") +func TestIsTar(t *testing.T) { + tests := map[string]bool{ + "foo.tgz": true, + "foo/bar/baz.tgz": true, + "foo-1.2.3.4.5.tgz": true, + "foo.tar.gz": false, // for our purposes + "foo.tgz.1": false, + "footgz": false, } - g, err := provider.New(getter.WithURL(srv.URL)) - if err != nil { - t.Fatal(err) + for src, expect := range tests { + if isTar(src) != expect { + t.Errorf("%q should be %t", src, expect) + } } - got, err := g.Get(srv.URL) +} + +func TestDownloadTo(t *testing.T) { + tmp, err := ioutil.TempDir("", "helm-downloadto-") if err != nil { t.Fatal(err) } + defer os.RemoveAll(tmp) - if got.String() != expect { - t.Errorf("Expected %q, got %q", expect, got.String()) + hh := helmpath.Home(tmp) + dest := filepath.Join(hh.String(), "dest") + configDirectories := []string{ + hh.String(), + hh.Repository(), + hh.Cache(), + dest, } - - // test with server backed by basic auth - basicAuthSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - username, password, ok := r.BasicAuth() - if !ok || username != "username" || password != "password" { - t.Errorf("Expected request to use basic auth and for username == 'username' and password == 'password', got '%v', '%s', '%s'", ok, username, password) + for _, p := range configDirectories { + if fi, err := os.Stat(p); err != nil { + if err := os.MkdirAll(p, 0755); err != nil { + t.Fatalf("Could not create %s: %s", p, err) + } + } else if !fi.IsDir() { + t.Fatalf("%s must be a directory", p) } - fmt.Fprint(w, expect) - })) - - defer basicAuthSrv.Close() + } - u, _ := url.ParseRequestURI(basicAuthSrv.URL) - httpgetter, err := getter.NewHTTPGetter( - getter.WithURL(u.String()), - getter.WithBasicAuth("username", "password"), - ) - if err != nil { + // Set up a fake repo + srv := repotest.NewServer(tmp) + defer srv.Stop() + if _, err := srv.CopyCharts("testdata/*.tgz*"); err != nil { + t.Error(err) + return + } + if err := srv.LinkIndices(); err != nil { t.Fatal(err) } - got, err = httpgetter.Get(u.String()) + + c := ChartDownloader{ + HelmHome: hh, + Out: os.Stderr, + Verify: VerifyAlways, + Keyring: "testdata/helm-test-key.pub", + Getters: getter.All(cli.EnvSettings{}), + } + cname := "/signtest-0.1.0.tgz" + where, v, err := c.DownloadTo(srv.URL()+cname, "", dest) if err != nil { - t.Fatal(err) + t.Error(err) + return } - if got.String() != expect { - t.Errorf("Expected %q, got %q", expect, got.String()) + if expect := filepath.Join(dest, cname); where != expect { + t.Errorf("Expected download to %s, got %s", expect, where) } -} -func TestIsTar(t *testing.T) { - tests := map[string]bool{ - "foo.tgz": true, - "foo/bar/baz.tgz": true, - "foo-1.2.3.4.5.tgz": true, - "foo.tar.gz": false, // for our purposes - "foo.tgz.1": false, - "footgz": false, + if v.FileHash == "" { + t.Error("File hash was empty, but verification is required.") } - for src, expect := range tests { - if isTar(src) != expect { - t.Errorf("%q should be %t", src, expect) - } + if _, err := os.Stat(filepath.Join(dest, cname)); err != nil { + t.Error(err) + return } } -func TestDownloadTo(t *testing.T) { +func TestDownloadTo_WithOptions(t *testing.T) { tmp, err := ioutil.TempDir("", "helm-downloadto-") if err != nil { t.Fatal(err) @@ -183,8 +190,16 @@ func TestDownloadTo(t *testing.T) { } } - // Set up a fake repo + // Set up a fake repo with basic auth enabled srv := repotest.NewServer(tmp) + srv.Stop() + srv.WithMiddleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + username, password, ok := r.BasicAuth() + if !ok || username != "username" || password != "password" { + t.Errorf("Expected request to use basic auth and for username == 'username' and password == 'password', got '%v', '%s', '%s'", ok, username, password) + } + })) + srv.Start() defer srv.Stop() if _, err := srv.CopyCharts("testdata/*.tgz*"); err != nil { t.Error(err) @@ -200,6 +215,9 @@ func TestDownloadTo(t *testing.T) { Verify: VerifyAlways, Keyring: "testdata/helm-test-key.pub", Getters: getter.All(cli.EnvSettings{}), + Options: []getter.Option{ + getter.WithBasicAuth("username", "password"), + }, } cname := "/signtest-0.1.0.tgz" where, v, err := c.DownloadTo(srv.URL()+cname, "", dest) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 3042c21b1..08f1959f2 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -236,8 +236,9 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { Keyring: m.Keyring, HelmHome: m.HelmHome, Getters: m.Getters, - Username: username, - Password: password, + Options: []getter.Option{ + getter.WithBasicAuth(username, password), + }, } if _, _, err := dl.DownloadTo(churl, "", destPath); err != nil { diff --git a/pkg/getter/httpgetter_test.go b/pkg/getter/httpgetter_test.go index 5f964582d..fdefc268d 100644 --- a/pkg/getter/httpgetter_test.go +++ b/pkg/getter/httpgetter_test.go @@ -16,11 +16,15 @@ limitations under the License. package getter import ( + "fmt" "net/http" + "net/http/httptest" + "net/url" "path/filepath" "testing" "helm.sh/helm/internal/test" + "helm.sh/helm/pkg/cli" ) func TestHTTPGetter(t *testing.T) { @@ -80,3 +84,57 @@ func TestHTTPGetter(t *testing.T) { t.Errorf("Expected NewHTTPGetter to contain %q as the user agent, got %q", "Groot", hg.opts.userAgent) } } + +func TestDownload(t *testing.T) { + expect := "Call me Ishmael" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, expect) + })) + defer srv.Close() + + provider, err := ByScheme("http", cli.EnvSettings{}) + if err != nil { + t.Fatal("No http provider found") + } + + g, err := provider.New(WithURL(srv.URL)) + if err != nil { + t.Fatal(err) + } + got, err := g.Get(srv.URL) + if err != nil { + t.Fatal(err) + } + + if got.String() != expect { + t.Errorf("Expected %q, got %q", expect, got.String()) + } + + // test with server backed by basic auth + basicAuthSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + username, password, ok := r.BasicAuth() + if !ok || username != "username" || password != "password" { + t.Errorf("Expected request to use basic auth and for username == 'username' and password == 'password', got '%v', '%s', '%s'", ok, username, password) + } + fmt.Fprint(w, expect) + })) + + defer basicAuthSrv.Close() + + u, _ := url.ParseRequestURI(basicAuthSrv.URL) + httpgetter, err := NewHTTPGetter( + WithURL(u.String()), + WithBasicAuth("username", "password"), + ) + if err != nil { + t.Fatal(err) + } + got, err = httpgetter.Get(u.String()) + if err != nil { + t.Fatal(err) + } + + if got.String() != expect { + t.Errorf("Expected %q, got %q", expect, got.String()) + } +} diff --git a/pkg/repo/repotest/server.go b/pkg/repo/repotest/server.go index 5afd67617..5b63b9e24 100644 --- a/pkg/repo/repotest/server.go +++ b/pkg/repo/repotest/server.go @@ -69,7 +69,7 @@ func NewServer(docroot string) *Server { srv := &Server{ docroot: root, } - srv.start() + srv.Start() // Add the testing repository as the only repo. if err := setTestingRepository(helmpath.Home(docroot), "test", srv.URL()); err != nil { panic(err) @@ -79,8 +79,15 @@ func NewServer(docroot string) *Server { // Server is an implementation of a repository server for testing. type Server struct { - docroot string - srv *httptest.Server + docroot string + srv *httptest.Server + middleware http.HandlerFunc +} + +// WithMiddleware injects middleware in front of the server. This can be used to inject +// additional functionality like layering in an authentication frontend. +func (s *Server) WithMiddleware(middleware http.HandlerFunc) { + s.middleware = middleware } // Root gets the docroot for the server. @@ -129,8 +136,13 @@ func (s *Server) CreateIndex() error { return ioutil.WriteFile(ifile, d, 0755) } -func (s *Server) start() { - s.srv = httptest.NewServer(http.FileServer(http.Dir(s.docroot))) +func (s *Server) Start() { + s.srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if s.middleware != nil { + s.middleware.ServeHTTP(w, r) + } + http.FileServer(http.Dir(s.docroot)).ServeHTTP(w, r) + })) } // Stop stops the server and closes all connections.