Merge pull request #5917 from bacongobbler/ref-getter-options

ref(getter): introduce Options for passing in getter parameters

pkg/action/install.go

@@ -751,7 +751,7 @@ func readFile(filePath string, settings cli.EnvSettings) ([]byte, error) {
 		return ioutil.ReadFile(filePath)
 	}
 
-	getter, err := getterConstructor(filePath, "", "", "")
+	getter, err := getterConstructor(getter.WithURL(filePath))
 	if err != nil {
 		return []byte{}, err
 	}

pkg/downloader/chart_downloader.go

@@ -156,7 +156,7 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, ge
 	}
 
 	// TODO add user-agent
-	g, err := getter.NewHTTPGetter(ref, "", "", "")
+	g, err := getter.NewHTTPGetter(getter.WithURL(ref))
 	if err != nil {
 		return u, nil, err
 	}
@@ -240,7 +240,7 @@ 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(rc.URL, "", "", "")
+		g, err := getter.NewHTTPGetter(getter.WithURL(rc.URL))
 		if err != nil {
 			return repoURL, nil, err
 		}

pkg/downloader/chart_downloader_test.go

@@ -99,7 +99,7 @@ func TestDownload(t *testing.T) {
 		t.Fatal("No http provider found")
 	}
 
-	g, err := provider.New(srv.URL, "", "", "")
+	g, err := provider.New(getter.WithURL(srv.URL))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -124,11 +124,13 @@ func TestDownload(t *testing.T) {
 	defer basicAuthSrv.Close()
 
 	u, _ := url.ParseRequestURI(basicAuthSrv.URL)
-	httpgetter, err := getter.NewHTTPGetter(u.String(), "", "", "")
+	httpgetter, err := getter.NewHTTPGetter(
+		getter.WithURL(u.String()),
+		getter.WithBasicAuth("username", "password"),
+	)
 	if err != nil {
 		t.Fatal(err)
 	}
-	httpgetter.SetBasicAuth("username", "password")
 	got, err = httpgetter.Get(u.String())
 	if err != nil {
 		t.Fatal(err)

pkg/getter/getter.go

@@ -24,6 +24,55 @@ import (
 	"helm.sh/helm/pkg/cli"
 )
 
+// options are generic parameters to be provided to the getter during instantiation.
+//
+// Getters may or may not ignore these parameters as they are passed in.
+type options struct {
+	url       string
+	certFile  string
+	keyFile   string
+	caFile    string
+	username  string
+	password  string
+	userAgent string
+}
+
+// Option allows specifying various settings configurable by the user for overriding the defaults
+// used when performing Get operations with the Getter.
+type Option func(*options)
+
+// WithURL informs the getter the server name that will be used when fetching objects. Used in conjunction with
+// WithTLSClientConfig to set the TLSClientConfig's server name.
+func WithURL(url string) Option {
+	return func(opts *options) {
+		opts.url = url
+	}
+}
+
+// WithBasicAuth sets the request's Authorization header to use the provided credentials
+func WithBasicAuth(username, password string) Option {
+	return func(opts *options) {
+		opts.username = username
+		opts.password = password
+	}
+}
+
+// WithUserAgent sets the request's User-Agent header to use the provided agent name.
+func WithUserAgent(userAgent string) Option {
+	return func(opts *options) {
+		opts.userAgent = userAgent
+	}
+}
+
+// WithTLSClientConfig sets the client client auth with the provided credentials.
+func WithTLSClientConfig(certFile, keyFile, caFile string) Option {
+	return func(opts *options) {
+		opts.certFile = certFile
+		opts.keyFile = keyFile
+		opts.caFile = caFile
+	}
+}
+
 // Getter is an interface to support GET to the specified URL.
 type Getter interface {
 	//Get file content by url string
@@ -32,7 +81,7 @@ type Getter interface {
 
 // Constructor is the function for every getter which creates a specific instance
 // according to the configuration
-type Constructor func(URL, CertFile, KeyFile, CAFile string) (Getter, error)
+type Constructor func(options ...Option) (Getter, error)
 
 // Provider represents any getter and the schemes that it supports.
 //
@@ -69,8 +118,8 @@ func (p Providers) ByScheme(scheme string) (Constructor, error) {
 }
 
 // All finds all of the registered getters as a list of Provider instances.
-// Currently the build-in http/https getter and the discovered
-// plugins with downloader notations are collected.
+// Currently, the built-in getters and the discovered plugins with downloader
+// notations are collected.
 func All(settings cli.EnvSettings) Providers {
 	result := Providers{
 		{

pkg/getter/getter_test.go

@@ -23,7 +23,7 @@ import (
 func TestProvider(t *testing.T) {
 	p := Provider{
 		[]string{"one", "three"},
-		func(h, e, l, m string) (Getter, error) { return nil, nil },
+		func(_ ...Option) (Getter, error) { return nil, nil },
 	}
 
 	if !p.Provides("three") {
@@ -33,8 +33,8 @@ func TestProvider(t *testing.T) {
 
 func TestProviders(t *testing.T) {
 	ps := Providers{
-		{[]string{"one", "three"}, func(h, e, l, m string) (Getter, error) { return nil, nil }},
-		{[]string{"two", "four"}, func(h, e, l, m string) (Getter, error) { return nil, nil }},
+		{[]string{"one", "three"}, func(_ ...Option) (Getter, error) { return nil, nil }},
+		{[]string{"two", "four"}, func(_ ...Option) (Getter, error) { return nil, nil }},
 	}
 
 	if _, err := ps.ByScheme("one"); err != nil {

pkg/getter/httpgetter.go

@@ -28,21 +28,19 @@ import (
 
 // HTTPGetter is the efault HTTP(/S) backend handler
 type HTTPGetter struct {
-	client    *http.Client
-	username  string
-	password  string
-	userAgent string
+	client *http.Client
+	opts   options
 }
 
-// SetBasicAuth sets the credentials for the getter
+// SetBasicAuth sets the request's Authorization header to use the provided credentials.
 func (g *HTTPGetter) SetBasicAuth(username, password string) {
-	g.username = username
-	g.password = password
+	g.opts.username = username
+	g.opts.password = password
 }
 
-// SetUserAgent sets the HTTP User-Agent for the getter
+// SetUserAgent sets the request's User-Agent header to use the provided agent name.
 func (g *HTTPGetter) SetUserAgent(userAgent string) {
-	g.userAgent = userAgent
+	g.opts.userAgent = userAgent
 }
 
 //Get performs a Get from repo.Getter and returns the body.
@@ -60,12 +58,12 @@ func (g *HTTPGetter) get(href string) (*bytes.Buffer, error) {
 		return buf, err
 	}
 	// req.Header.Set("User-Agent", "Helm/"+strings.TrimPrefix(version.GetVersion(), "v"))
-	if g.userAgent != "" {
-		req.Header.Set("User-Agent", g.userAgent)
+	if g.opts.userAgent != "" {
+		req.Header.Set("User-Agent", g.opts.userAgent)
 	}
 
-	if g.username != "" && g.password != "" {
-		req.SetBasicAuth(g.username, g.password)
+	if g.opts.username != "" && g.opts.password != "" {
+		req.SetBasicAuth(g.opts.username, g.opts.password)
 	}
 
 	resp, err := g.client.Do(req)
@@ -82,21 +80,26 @@ func (g *HTTPGetter) get(href string) (*bytes.Buffer, error) {
 }
 
 // newHTTPGetter constructs a valid http/https client as Getter
-func newHTTPGetter(url, certFile, keyFile, caFile string) (Getter, error) {
-	return NewHTTPGetter(url, certFile, keyFile, caFile)
+func newHTTPGetter(options ...Option) (Getter, error) {
+	return NewHTTPGetter(options...)
 }
 
 // NewHTTPGetter constructs a valid http/https client as HTTPGetter
-func NewHTTPGetter(url, certFile, keyFile, caFile string) (*HTTPGetter, error) {
+func NewHTTPGetter(options ...Option) (*HTTPGetter, error) {
 	var client HTTPGetter
-	if certFile != "" && keyFile != "" {
-		tlsConf, err := tlsutil.NewClientTLS(certFile, keyFile, caFile)
+
+	for _, opt := range options {
+		opt(&client.opts)
+	}
+
+	if client.opts.certFile != "" && client.opts.keyFile != "" {
+		tlsConf, err := tlsutil.NewClientTLS(client.opts.certFile, client.opts.keyFile, client.opts.caFile)
 		if err != nil {
 			return &client, errors.Wrap(err, "can't create TLS config for client")
 		}
 		tlsConf.BuildNameToCertificate()
 
-		sni, err := urlutil.ExtractHostname(url)
+		sni, err := urlutil.ExtractHostname(client.opts.url)
 		if err != nil {
 			return &client, err
 		}
@@ -111,5 +114,6 @@ func NewHTTPGetter(url, certFile, keyFile, caFile string) (*HTTPGetter, error) {
 	} else {
 		client.client = http.DefaultClient
 	}
+
 	return &client, nil
 }

pkg/getter/httpgetter_test.go

@@ -19,10 +19,12 @@ import (
 	"net/http"
 	"path/filepath"
 	"testing"
+
+	"helm.sh/helm/internal/test"
 )
 
 func TestHTTPGetter(t *testing.T) {
-	g, err := newHTTPGetter("http://example.com", "", "", "")
+	g, err := newHTTPGetter(WithURL("http://example.com"))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -37,12 +39,44 @@ func TestHTTPGetter(t *testing.T) {
 	cd := "../../testdata"
 	join := filepath.Join
 	ca, pub, priv := join(cd, "ca.pem"), join(cd, "crt.pem"), join(cd, "key.pem")
-	g, err = newHTTPGetter("http://example.com/", pub, priv, ca)
+	g, err = newHTTPGetter(
+		WithURL("http://example.com"),
+		WithTLSClientConfig(pub, priv, ca),
+	)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	if _, ok := g.(*HTTPGetter); !ok {
+	hg, ok := g.(*HTTPGetter)
+	if !ok {
 		t.Fatal("Expected newHTTPGetter to produce an httpGetter")
 	}
+
+	transport, ok := hg.client.Transport.(*http.Transport)
+	if !ok {
+		t.Errorf("Expected newHTTPGetter to set up an HTTP transport")
+	}
+
+	test.AssertGoldenString(t, transport.TLSClientConfig.ServerName, "output/httpgetter-servername.txt")
+
+	// Test other options
+	hg, err = NewHTTPGetter(
+		WithBasicAuth("I", "Am"),
+		WithUserAgent("Groot"),
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if hg.opts.username != "I" {
+		t.Errorf("Expected NewHTTPGetter to contain %q as the username, got %q", "I", hg.opts.username)
+	}
+
+	if hg.opts.password != "Am" {
+		t.Errorf("Expected NewHTTPGetter to contain %q as the password, got %q", "Am", hg.opts.password)
+	}
+
+	if hg.opts.userAgent != "Groot" {
+		t.Errorf("Expected NewHTTPGetter to contain %q as the user agent, got %q", "Groot", hg.opts.userAgent)
+	}
 }

pkg/getter/plugingetter.go

@@ -54,16 +54,16 @@ func collectPlugins(settings cli.EnvSettings) (Providers, error) {
 // pluginGetter is a generic type to invoke custom downloaders,
 // implemented in plugins.
 type pluginGetter struct {
-	command                   string
-	certFile, keyFile, cAFile string
-	settings                  cli.EnvSettings
-	name                      string
-	base                      string
+	command  string
+	settings cli.EnvSettings
+	name     string
+	base     string
+	opts     options
 }
 
 // Get runs downloader plugin command
 func (p *pluginGetter) Get(href string) (*bytes.Buffer, error) {
-	argv := []string{p.certFile, p.keyFile, p.cAFile, href}
+	argv := []string{p.opts.certFile, p.opts.keyFile, p.opts.caFile, href}
 	prog := exec.Command(filepath.Join(p.base, p.command), argv...)
 	plugin.SetupPluginEnv(p.settings, p.name, p.base)
 	prog.Env = os.Environ()
@@ -82,16 +82,16 @@ func (p *pluginGetter) Get(href string) (*bytes.Buffer, error) {
 
 // newPluginGetter constructs a valid plugin getter
 func newPluginGetter(command string, settings cli.EnvSettings, name, base string) Constructor {
-	return func(URL, CertFile, KeyFile, CAFile string) (Getter, error) {
+	return func(options ...Option) (Getter, error) {
 		result := &pluginGetter{
 			command:  command,
-			certFile: CertFile,
-			keyFile:  KeyFile,
-			cAFile:   CAFile,
 			settings: settings,
 			name:     name,
 			base:     base,
 		}
+		for _, opt := range options {
+			opt(&result.opts)
+		}
 		return result, nil
 	}
 }

pkg/getter/plugingetter_test.go

@@ -78,7 +78,7 @@ func TestPluginGetter(t *testing.T) {
 
 	env := hh(false)
 	pg := newPluginGetter("echo", env, "test", ".")
-	g, err := pg("test://foo/bar", "", "", "")
+	g, err := pg()
 	if err != nil {
 		t.Fatal(err)
 	}

pkg/getter/testdata/output/httpgetter-servername.txt

@@ -0,0 +1 @@
+example.com

pkg/plugin/installer/http_installer.go

@@ -84,7 +84,7 @@ func NewHTTPInstaller(source string, home helmpath.Home) (*HTTPInstaller, error)
 		return nil, err
 	}
 
-	get, err := getConstructor.New(source, "", "", "")
+	get, err := getConstructor.New(getter.WithURL(source))
 	if err != nil {
 		return nil, err
 	}

pkg/repo/chartrepo.go

@@ -63,7 +63,10 @@ func NewChartRepository(cfg *Entry, getters getter.Providers) (*ChartRepository,
 	if err != nil {
 		return nil, errors.Errorf("could not find protocol handler for: %s", u.Scheme)
 	}
-	client, err := getterConstructor(cfg.URL, cfg.CertFile, cfg.KeyFile, cfg.CAFile)
+	client, err := getterConstructor(
+		getter.WithURL(cfg.URL),
+		getter.WithTLSClientConfig(cfg.CertFile, cfg.KeyFile, cfg.CAFile),
+	)
 	if err != nil {
 		return nil, errors.Wrapf(err, "could not construct protocol handler for: %s", u.Scheme)
 	}
@@ -121,11 +124,14 @@ func (r *ChartRepository) DownloadIndexFile(cachePath string) error {
 
 	indexURL = parsedURL.String()
 	// TODO add user-agent
-	g, err := getter.NewHTTPGetter(indexURL, r.Config.CertFile, r.Config.KeyFile, r.Config.CAFile)
+	g, err := getter.NewHTTPGetter(
+		getter.WithURL(indexURL),
+		getter.WithTLSClientConfig(r.Config.CertFile, r.Config.KeyFile, r.Config.CAFile),
+		getter.WithBasicAuth(r.Config.Username, r.Config.Password),
+	)
 	if err != nil {
 		return err
 	}
-	g.SetBasicAuth(r.Config.Username, r.Config.Password)
 	resp, err := g.Get(indexURL)
 	if err != nil {
 		return err