Merge pull request #13579 from gjenkins8/rm_chart_repo_find_repo_dups

refactor: Remove duplicate `FindChartIn*RepoURL` functions

pkg/action/install.go

@@ -789,8 +789,16 @@ func (c *ChartPathOptions) LocateChart(name string, settings *cli.EnvSettings) (
 		dl.Verify = downloader.VerifyAlways
 	}
 	if c.RepoURL != "" {
-		chartURL, err := repo.FindChartInAuthAndTLSAndPassRepoURL(c.RepoURL, c.Username, c.Password, name, version,
+		chartURL, err := repo.FindChartInRepoURL(
-			c.CertFile, c.KeyFile, c.CaFile, c.InsecureSkipTLSverify, c.PassCredentialsAll, getter.All(settings))
+			c.RepoURL,
+			name,
+			getter.All(settings),
+			repo.WithChartVersion(version),
+			repo.WithClientTLS(c.CertFile, c.KeyFile, c.CaFile),
+			repo.WithUsernamePassword(c.Username, c.Password),
+			repo.WithInsecureSkipTLSverify(c.InsecureSkipTLSverify),
+			repo.WithPassCredentialsAll(c.PassCredentialsAll),
+		)
 		if err != nil {
 			return "", err
 		}

pkg/action/pull.go

@@ -117,7 +117,16 @@ func (p *Pull) Run(chartRef string) (string, error) {
 	}
 
 	if p.RepoURL != "" {
-		chartURL, err := repo.FindChartInAuthAndTLSAndPassRepoURL(p.RepoURL, p.Username, p.Password, chartRef, p.Version, p.CertFile, p.KeyFile, p.CaFile, p.InsecureSkipTLSverify, p.PassCredentialsAll, getter.All(p.Settings))
+		chartURL, err := repo.FindChartInRepoURL(
+			p.RepoURL,
+			chartRef,
+			getter.All(p.Settings),
+			repo.WithChartVersion(p.Version),
+			repo.WithClientTLS(p.CertFile, p.KeyFile, p.CaFile),
+			repo.WithUsernamePassword(p.Username, p.Password),
+			repo.WithInsecureSkipTLSverify(p.InsecureSkipTLSverify),
+			repo.WithPassCredentialsAll(p.PassCredentialsAll),
+		)
 		if err != nil {
 			return out.String(), err
 		}

pkg/downloader/manager.go

@@ -742,7 +742,7 @@ func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*
 			return
 		}
 	}
-	url, err = repo.FindChartInRepoURL(repoURL, name, version, certFile, keyFile, caFile, m.Getters)
+	url, err = repo.FindChartInRepoURL(repoURL, name, m.Getters, repo.WithChartVersion(version), repo.WithClientTLS(certFile, keyFile, caFile))
 	if err == nil {
 		return url, username, password, false, false, "", "", "", err
 	}

pkg/repo/chartrepo.go

@@ -196,33 +196,65 @@ func (r *ChartRepository) generateIndex() error {
 	return nil
 }
 
-// FindChartInRepoURL finds chart in chart repository pointed by repoURL
+type findChartInRepoURLOptions struct {
-// without adding repo to repositories
+	Username              string
-func FindChartInRepoURL(repoURL, chartName, chartVersion, certFile, keyFile, caFile string, getters getter.Providers) (string, error) {
+	Password              string
-	return FindChartInAuthRepoURL(repoURL, "", "", chartName, chartVersion, certFile, keyFile, caFile, getters)
+	PassCredentialsAll    bool
+	InsecureSkipTLSverify bool
+	CertFile              string
+	KeyFile               string
+	CAFile                string
+	ChartVersion          string
+}
+
+type FindChartInRepoURLOption func(*findChartInRepoURLOptions)
+
+// WithChartVersion specifies the chart version to find
+func WithChartVersion(chartVersion string) FindChartInRepoURLOption {
+	return func(options *findChartInRepoURLOptions) {
+		options.ChartVersion = chartVersion
+	}
+}
+
+// WithUsernamePassword specifies the username/password credntials for the repository
+func WithUsernamePassword(username, password string) FindChartInRepoURLOption {
+	return func(options *findChartInRepoURLOptions) {
+		options.Username = username
+		options.Password = password
+	}
 }
 
-// FindChartInAuthRepoURL finds chart in chart repository pointed by repoURL
+// WithPassCredentialsAll flags whether credentials should be passed on to other domains
-// without adding repo to repositories, like FindChartInRepoURL,
+func WithPassCredentialsAll(passCredentialsAll bool) FindChartInRepoURLOption {
-// but it also receives credentials for the chart repository.
+	return func(options *findChartInRepoURLOptions) {
-func FindChartInAuthRepoURL(repoURL, username, password, chartName, chartVersion, certFile, keyFile, caFile string, getters getter.Providers) (string, error) {
+		options.PassCredentialsAll = passCredentialsAll
-	return FindChartInAuthAndTLSRepoURL(repoURL, username, password, chartName, chartVersion, certFile, keyFile, caFile, false, getters)
+	}
 }
 
-// FindChartInAuthAndTLSRepoURL finds chart in chart repository pointed by repoURL
+// WithClientTLS species the cert, key, and CA files for client mTLS
-// without adding repo to repositories, like FindChartInRepoURL,
+func WithClientTLS(certFile, keyFile, caFile string) FindChartInRepoURLOption {
-// but it also receives credentials and TLS verify flag for the chart repository.
+	return func(options *findChartInRepoURLOptions) {
-// TODO Helm 4, FindChartInAuthAndTLSRepoURL should be integrated into FindChartInAuthRepoURL.
+		options.CertFile = certFile
-func FindChartInAuthAndTLSRepoURL(repoURL, username, password, chartName, chartVersion, certFile, keyFile, caFile string, insecureSkipTLSverify bool, getters getter.Providers) (string, error) {
+		options.KeyFile = keyFile
-	return FindChartInAuthAndTLSAndPassRepoURL(repoURL, username, password, chartName, chartVersion, certFile, keyFile, caFile, insecureSkipTLSverify, false, getters)
+		options.CAFile = caFile
+	}
+}
+
+// WithInsecureSkipTLSverify skips TLS verification for repostory communication
+func WithInsecureSkipTLSverify(insecureSkipTLSverify bool) FindChartInRepoURLOption {
+	return func(options *findChartInRepoURLOptions) {
+		options.InsecureSkipTLSverify = insecureSkipTLSverify
+	}
 }
 
-// FindChartInAuthAndTLSAndPassRepoURL finds chart in chart repository pointed by repoURL
+// FindChartInRepoURL finds chart in chart repository pointed by repoURL
-// without adding repo to repositories, like FindChartInRepoURL,
+// without adding repo to repositories
-// but it also receives credentials, TLS verify flag, and if credentials should
+func FindChartInRepoURL(repoURL string, chartName string, getters getter.Providers, options ...FindChartInRepoURLOption) (string, error) {
-// be passed on to other domains.
+
-// TODO Helm 4, FindChartInAuthAndTLSAndPassRepoURL should be integrated into FindChartInAuthRepoURL.
+	opts := findChartInRepoURLOptions{}
-func FindChartInAuthAndTLSAndPassRepoURL(repoURL, username, password, chartName, chartVersion, certFile, keyFile, caFile string, insecureSkipTLSverify, passCredentialsAll bool, getters getter.Providers) (string, error) {
+	for _, option := range options {
+		option(&opts)
+	}
 
 	// Download and write the index file to a temporary location
 	buf := make([]byte, 20)
@@ -231,14 +263,14 @@ func FindChartInAuthAndTLSAndPassRepoURL(repoURL, username, password, chartName,
 
 	c := Entry{
 		URL:                   repoURL,
-		Username:              username,
+		Username:              opts.Username,
-		Password:              password,
+		Password:              opts.Password,
-		PassCredentialsAll:    passCredentialsAll,
+		PassCredentialsAll:    opts.PassCredentialsAll,
-		CertFile:              certFile,
+		CertFile:              opts.CertFile,
-		KeyFile:               keyFile,
+		KeyFile:               opts.KeyFile,
-		CAFile:                caFile,
+		CAFile:                opts.CAFile,
 		Name:                  name,
-		InsecureSkipTLSverify: insecureSkipTLSverify,
+		InsecureSkipTLSverify: opts.InsecureSkipTLSverify,
 	}
 	r, err := NewChartRepository(&c, getters)
 	if err != nil {
@@ -260,10 +292,10 @@ func FindChartInAuthAndTLSAndPassRepoURL(repoURL, username, password, chartName,
 	}
 
 	errMsg := fmt.Sprintf("chart %q", chartName)
-	if chartVersion != "" {
+	if opts.ChartVersion != "" {
-		errMsg = fmt.Sprintf("%s version %q", errMsg, chartVersion)
+		errMsg = fmt.Sprintf("%s version %q", errMsg, opts.ChartVersion)
 	}
-	cv, err := repoIndex.Get(chartName, chartVersion)
+	cv, err := repoIndex.Get(chartName, opts.ChartVersion)
 	if err != nil {
 		return "", errors.Errorf("%s not found in %s repository", errMsg, repoURL)
 	}

pkg/repo/chartrepo_test.go

@@ -297,7 +297,12 @@ func TestFindChartInAuthAndTLSAndPassRepoURL(t *testing.T) {
 	}
 	defer srv.Close()
 
-	chartURL, err := FindChartInAuthAndTLSAndPassRepoURL(srv.URL, "", "", "nginx", "", "", "", "", true, false, getter.All(&cli.EnvSettings{}))
+	chartURL, err := FindChartInRepoURL(
+		srv.URL,
+		"nginx",
+		getter.All(&cli.EnvSettings{}),
+		WithInsecureSkipTLSverify(true),
+	)
 	if err != nil {
 		t.Fatalf("%v", err)
 	}
@@ -306,7 +311,7 @@ func TestFindChartInAuthAndTLSAndPassRepoURL(t *testing.T) {
 	}
 
 	// If the insecureSkipTLSVerify is false, it will return an error that contains "x509: certificate signed by unknown authority".
-	_, err = FindChartInAuthAndTLSAndPassRepoURL(srv.URL, "", "", "nginx", "0.1.0", "", "", "", false, false, getter.All(&cli.EnvSettings{}))
+	_, err = FindChartInRepoURL(srv.URL, "nginx", getter.All(&cli.EnvSettings{}), WithChartVersion("0.1.0"))
 	// Go communicates with the platform and different platforms return different messages. Go itself tests darwin
 	// differently for its message. On newer versions of Darwin the message includes the "Acme Co" portion while older
 	// versions of Darwin do not. As there are people developing Helm using both old and new versions of Darwin we test
@@ -327,7 +332,7 @@ func TestFindChartInRepoURL(t *testing.T) {
 	}
 	defer srv.Close()
 
-	chartURL, err := FindChartInRepoURL(srv.URL, "nginx", "", "", "", "", getter.All(&cli.EnvSettings{}))
+	chartURL, err := FindChartInRepoURL(srv.URL, "nginx", getter.All(&cli.EnvSettings{}))
 	if err != nil {
 		t.Fatalf("%v", err)
 	}
@@ -335,7 +340,7 @@ func TestFindChartInRepoURL(t *testing.T) {
 		t.Errorf("%s is not the valid URL", chartURL)
 	}
 
-	chartURL, err = FindChartInRepoURL(srv.URL, "nginx", "0.1.0", "", "", "", getter.All(&cli.EnvSettings{}))
+	chartURL, err = FindChartInRepoURL(srv.URL, "nginx", getter.All(&cli.EnvSettings{}), WithChartVersion("0.1.0"))
 	if err != nil {
 		t.Errorf("%s", err)
 	}
@@ -350,7 +355,7 @@ func TestErrorFindChartInRepoURL(t *testing.T) {
 		RepositoryCache: t.TempDir(),
 	})
 
-	if _, err := FindChartInRepoURL("http://someserver/something", "nginx", "", "", "", "", g); err == nil {
+	if _, err := FindChartInRepoURL("http://someserver/something", "nginx", g); err == nil {
 		t.Errorf("Expected error for bad chart URL, but did not get any errors")
 	} else if !strings.Contains(err.Error(), `looks like "http://someserver/something" is not a valid chart repository or cannot be reached`) {
 		t.Errorf("Expected error for bad chart URL, but got a different error (%v)", err)
@@ -362,19 +367,19 @@ func TestErrorFindChartInRepoURL(t *testing.T) {
 	}
 	defer srv.Close()
 
-	if _, err = FindChartInRepoURL(srv.URL, "nginx1", "", "", "", "", g); err == nil {
+	if _, err = FindChartInRepoURL(srv.URL, "nginx1", g); err == nil {
 		t.Errorf("Expected error for chart not found, but did not get any errors")
 	} else if err.Error() != `chart "nginx1" not found in `+srv.URL+` repository` {
 		t.Errorf("Expected error for chart not found, but got a different error (%v)", err)
 	}
 
-	if _, err = FindChartInRepoURL(srv.URL, "nginx1", "0.1.0", "", "", "", g); err == nil {
+	if _, err = FindChartInRepoURL(srv.URL, "nginx1", g, WithChartVersion("0.1.0")); err == nil {
 		t.Errorf("Expected error for chart not found, but did not get any errors")
 	} else if err.Error() != `chart "nginx1" version "0.1.0" not found in `+srv.URL+` repository` {
 		t.Errorf("Expected error for chart not found, but got a different error (%v)", err)
 	}
 
-	if _, err = FindChartInRepoURL(srv.URL, "chartWithNoURL", "", "", "", "", g); err == nil {
+	if _, err = FindChartInRepoURL(srv.URL, "chartWithNoURL", g); err == nil {
 		t.Errorf("Expected error for no chart URLs available, but did not get any errors")
 	} else if err.Error() != `chart "chartWithNoURL" has no downloadable URLs` {
 		t.Errorf("Expected error for chart not found, but got a different error (%v)", err)