From ebb7643af154b6eb0051bcd5fa0f41052fe6be73 Mon Sep 17 00:00:00 2001 From: Payal Godhani Date: Tue, 28 Jan 2025 14:06:10 -0800 Subject: [PATCH] Consolidate and refactor chart repository functions into FindChartInAuthRepoURL -e Signed-off-by: Payal Godhani --- pkg/repo/chartrepo.go | 39 ++++++++++---------------------------- pkg/repo/chartrepo_test.go | 18 +++++++++--------- 2 files changed, 19 insertions(+), 38 deletions(-) diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index e20c7e20f..5545295ef 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -196,34 +196,9 @@ func (r *ChartRepository) generateIndex() error { return nil } -// FindChartInRepoURL finds chart in chart repository pointed by repoURL -// without adding repo to repositories -func FindChartInRepoURL(repoURL, chartName, chartVersion, certFile, keyFile, caFile string, getters getter.Providers) (string, error) { - return FindChartInAuthRepoURL(repoURL, "", "", chartName, chartVersion, certFile, keyFile, caFile, getters) -} - // FindChartInAuthRepoURL finds chart in chart repository pointed by repoURL -// without adding repo to repositories, like FindChartInRepoURL, -// but it also receives credentials for the chart repository. -func FindChartInAuthRepoURL(repoURL, username, password, chartName, chartVersion, certFile, keyFile, caFile string, getters getter.Providers) (string, error) { - return FindChartInAuthAndTLSRepoURL(repoURL, username, password, chartName, chartVersion, certFile, keyFile, caFile, false, getters) -} - -// FindChartInAuthAndTLSRepoURL finds chart in chart repository pointed by repoURL -// without adding repo to repositories, like FindChartInRepoURL, -// but it also receives credentials and TLS verify flag for the chart repository. -// TODO Helm 4, FindChartInAuthAndTLSRepoURL should be integrated into FindChartInAuthRepoURL. -func FindChartInAuthAndTLSRepoURL(repoURL, username, password, chartName, chartVersion, certFile, keyFile, caFile string, insecureSkipTLSverify bool, getters getter.Providers) (string, error) { - return FindChartInAuthAndTLSAndPassRepoURL(repoURL, username, password, chartName, chartVersion, certFile, keyFile, caFile, insecureSkipTLSverify, false, getters) -} - -// FindChartInAuthAndTLSAndPassRepoURL finds chart in chart repository pointed by repoURL -// without adding repo to repositories, like FindChartInRepoURL, -// but it also receives credentials, TLS verify flag, and if credentials should -// be passed on to other domains. -// TODO Helm 4, FindChartInAuthAndTLSAndPassRepoURL should be integrated into FindChartInAuthRepoURL. -func FindChartInAuthAndTLSAndPassRepoURL(repoURL, username, password, chartName, chartVersion, certFile, keyFile, caFile string, insecureSkipTLSverify, passCredentialsAll bool, getters getter.Providers) (string, error) { - +// without adding repo to repositories, and supports authentication, TLS settings, and optional passing of credentials to other domains. +func FindChartInAuthRepoURL(repoURL, username, password, chartName, chartVersion, certFile, keyFile, caFile string, insecureSkipTLSverify, passCredentialsAll bool, getters getter.Providers) (string, error) { // Download and write the index file to a temporary location buf := make([]byte, 20) rand.Read(buf) @@ -244,6 +219,8 @@ func FindChartInAuthAndTLSAndPassRepoURL(repoURL, username, password, chartName, if err != nil { return "", err } + + // Download the index file from the repository idx, err := r.DownloadIndexFile() if err != nil { return "", errors.Wrapf(err, "looks like %q is not a valid chart repository or cannot be reached", repoURL) @@ -253,27 +230,31 @@ func FindChartInAuthAndTLSAndPassRepoURL(repoURL, username, password, chartName, os.RemoveAll(filepath.Join(r.CachePath, helmpath.CacheIndexFile(r.Config.Name))) }() - // Read the index file for the repository to get chart information and return chart URL + // Read the index file to get chart information repoIndex, err := LoadIndexFile(idx) if err != nil { return "", err } + // Build error message for missing chart version errMsg := fmt.Sprintf("chart %q", chartName) if chartVersion != "" { errMsg = fmt.Sprintf("%s version %q", errMsg, chartVersion) } + + // Find chart in the index cv, err := repoIndex.Get(chartName, chartVersion) if err != nil { return "", errors.Errorf("%s not found in %s repository", errMsg, repoURL) } + // Ensure the chart has downloadable URLs if len(cv.URLs) == 0 { return "", errors.Errorf("%s has no downloadable URLs", errMsg) } + // Resolve the chart URL chartURL := cv.URLs[0] - absoluteChartURL, err := ResolveReferenceURL(repoURL, chartURL) if err != nil { return "", errors.Wrap(err, "failed to make chart URL absolute") diff --git a/pkg/repo/chartrepo_test.go b/pkg/repo/chartrepo_test.go index 341bafadc..73219552d 100644 --- a/pkg/repo/chartrepo_test.go +++ b/pkg/repo/chartrepo_test.go @@ -290,14 +290,14 @@ func startLocalTLSServerForTests(handler http.Handler) (*httptest.Server, error) return httptest.NewTLSServer(handler), nil } -func TestFindChartInAuthAndTLSAndPassRepoURL(t *testing.T) { +func TestChartInAuthRepoURL(t *testing.T) { srv, err := startLocalTLSServerForTests(nil) if err != nil { t.Fatal(err) } defer srv.Close() - chartURL, err := FindChartInAuthAndTLSAndPassRepoURL(srv.URL, "", "", "nginx", "", "", "", "", true, false, getter.All(&cli.EnvSettings{})) + chartURL, err := FindChartInAuthRepoURL(srv.URL, "", "", "nginx", "", "", "", "", true, false, getter.All(&cli.EnvSettings{})) if err != nil { t.Fatalf("%v", err) } @@ -306,7 +306,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 = FindChartInAuthRepoURL(srv.URL, "", "", "nginx", "0.1.0", "", "", "", false, false, getter.All(&cli.EnvSettings{})) // 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 +327,7 @@ func TestFindChartInRepoURL(t *testing.T) { } defer srv.Close() - chartURL, err := FindChartInRepoURL(srv.URL, "nginx", "", "", "", "", getter.All(&cli.EnvSettings{})) + chartURL, err := FindChartInAuthRepoURL(srv.URL, "", "", "nginx", "", "", "", "", false, false, getter.All(&cli.EnvSettings{})) if err != nil { t.Fatalf("%v", err) } @@ -335,7 +335,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 = FindChartInAuthRepoURL(srv.URL, "", "", "nginx", "0.1.0", "", "", "", false, false, getter.All(&cli.EnvSettings{})) if err != nil { t.Errorf("%s", err) } @@ -350,7 +350,7 @@ func TestErrorFindChartInRepoURL(t *testing.T) { RepositoryCache: t.TempDir(), }) - if _, err := FindChartInRepoURL("http://someserver/something", "nginx", "", "", "", "", g); err == nil { + if _, err := FindChartInAuthRepoURL("http://someserver/something", "", "", "nginx", "", "", "", "", false, false, 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 +362,19 @@ func TestErrorFindChartInRepoURL(t *testing.T) { } defer srv.Close() - if _, err = FindChartInRepoURL(srv.URL, "nginx1", "", "", "", "", g); err == nil { + if _, err = FindChartInAuthRepoURL(srv.URL, "", "", "nginx1", "", "", "", "", false, false, 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 = FindChartInAuthRepoURL(srv.URL, "", "", "nginx1", "0.1.0", "", "", "", false, false, g); 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 = FindChartInAuthRepoURL(srv.URL, "", "", "chartWithNoURL", "", "", "", "", false, false, 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)