From c415d3f3d5b8c1873fa8117d21e5262f109586fb Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Fri, 15 Mar 2024 17:32:21 -0400 Subject: [PATCH] fix: registry argument to be only the host instead full URL (#17381) (#17535) Signed-off-by: Pablo Aguilar Co-authored-by: Pablo Aguilar --- util/helm/client.go | 13 +++- util/helm/client_test.go | 131 ++++++++++++++++++++++++++++++++------- 2 files changed, 121 insertions(+), 23 deletions(-) diff --git a/util/helm/client.go b/util/helm/client.go index 2b9e2912349cf..75bd30d1fea13 100644 --- a/util/helm/client.go +++ b/util/helm/client.go @@ -8,7 +8,6 @@ import ( "encoding/json" "errors" "fmt" - executil "github.com/argoproj/argo-cd/v2/util/exec" "io" "net/http" "net/url" @@ -19,6 +18,8 @@ import ( "strings" "time" + executil "github.com/argoproj/argo-cd/v2/util/exec" + "github.com/argoproj/pkg/sync" log "github.com/sirupsen/logrus" "gopkg.in/yaml.v2" @@ -34,6 +35,8 @@ import ( var ( globalLock = sync.NewKeyLock() indexLock = sync.NewKeyLock() + + OCINotEnabledErr = errors.New("could not perform the action when oci is not enabled") ) type Creds struct { @@ -401,6 +404,10 @@ func getIndexURL(rawURL string) (string, error) { } func (c *nativeHelmChart) GetTags(chart string, noCache bool) (*TagsList, error) { + if !c.enableOci { + return nil, OCINotEnabledErr + } + tagsURL := strings.Replace(fmt.Sprintf("%s/%s", c.repoURL, chart), "https://", "", 1) indexLock.Lock(tagsURL) defer indexLock.Unlock(tagsURL) @@ -428,10 +435,12 @@ func (c *nativeHelmChart) GetTags(chart string, noCache bool) (*TagsList, error) TLSClientConfig: tlsConf, DisableKeepAlives: true, }} + + repoHost, _, _ := strings.Cut(tagsURL, "/") repo.Client = &auth.Client{ Client: client, Cache: nil, - Credential: auth.StaticCredential(c.repoURL, auth.Credential{ + Credential: auth.StaticCredential(repoHost, auth.Credential{ Username: c.creds.Username, Password: c.creds.Password, }), diff --git a/util/helm/client_test.go b/util/helm/client_test.go index 3cda26feb5f0e..6fba279df07d0 100644 --- a/util/helm/client_test.go +++ b/util/helm/client_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "math" + "net/url" "os" "strings" "testing" @@ -159,41 +160,129 @@ func TestGetIndexURL(t *testing.T) { } func TestGetTagsFromUrl(t *testing.T) { + t.Run("should return tags correctly while following the link header", func(t *testing.T) { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Logf("called %s", r.URL.Path) + responseTags := TagsList{} + w.Header().Set("Content-Type", "application/json") + if !strings.Contains(r.URL.String(), "token") { + w.Header().Set("Link", fmt.Sprintf("; rel=next", r.Host, r.URL.Path)) + responseTags.Tags = []string{"first"} + } else { + responseTags.Tags = []string{ + "second", + "2.8.0", + "2.8.0-prerelease", + "2.8.0_build", + "2.8.0-prerelease_build", + "2.8.0-prerelease.1_build.1234", + } + } + w.WriteHeader(http.StatusOK) + err := json.NewEncoder(w).Encode(responseTags) + if err != nil { + t.Fatal(err) + } + })) + + client := NewClient(server.URL, Creds{InsecureSkipVerify: true}, true, "") + + tags, err := client.GetTags("mychart", true) + assert.NoError(t, err) + assert.ElementsMatch(t, tags.Tags, []string{ + "first", + "second", + "2.8.0", + "2.8.0-prerelease", + "2.8.0+build", + "2.8.0-prerelease+build", + "2.8.0-prerelease.1+build.1234", + }) + }) + + t.Run("should return an error not when oci is not enabled", func(t *testing.T) { + client := NewClient("example.com", Creds{}, false, "") + + _, err := client.GetTags("my-chart", true) + assert.ErrorIs(t, OCINotEnabledErr, err) + }) +} + +func TestGetTagsFromURLPrivateRepoAuthentication(t *testing.T) { server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { t.Logf("called %s", r.URL.Path) - responseTags := TagsList{} - w.Header().Set("Content-Type", "application/json") - if !strings.Contains(r.URL.String(), "token") { - w.Header().Set("Link", fmt.Sprintf("; rel=next", r.Host, r.URL.Path)) - responseTags.Tags = []string{"first"} - } else { - responseTags.Tags = []string{ - "second", + + authorization := r.Header.Get("Authorization") + if authorization == "" { + w.Header().Set("WWW-Authenticate", `Basic realm="helm repo to get tags"`) + w.WriteHeader(http.StatusUnauthorized) + return + } + + t.Logf("authorization received %s", authorization) + + responseTags := TagsList{ + Tags: []string{ "2.8.0", "2.8.0-prerelease", "2.8.0_build", "2.8.0-prerelease_build", "2.8.0-prerelease.1_build.1234", - } + }, } + + w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) err := json.NewEncoder(w).Encode(responseTags) if err != nil { t.Fatal(err) } })) + t.Cleanup(server.Close) - client := NewClient(server.URL, Creds{InsecureSkipVerify: true}, true, "") - - tags, err := client.GetTags("mychart", true) + serverURL, err := url.Parse(server.URL) assert.NoError(t, err) - assert.ElementsMatch(t, tags.Tags, []string{ - "first", - "second", - "2.8.0", - "2.8.0-prerelease", - "2.8.0+build", - "2.8.0-prerelease+build", - "2.8.0-prerelease.1+build.1234", - }) + + testCases := []struct { + name string + repoURL string + }{ + { + name: "should login correctly when the repo path is in the server root with http scheme", + repoURL: server.URL, + }, + { + name: "should login correctly when the repo path is not in the server root with http scheme", + repoURL: fmt.Sprintf("%s/my-repo", server.URL), + }, + { + name: "should login correctly when the repo path is in the server root without http scheme", + repoURL: serverURL.Host, + }, + { + name: "should login correctly when the repo path is not in the server root without http scheme", + repoURL: fmt.Sprintf("%s/my-repo", serverURL.Host), + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + client := NewClient(testCase.repoURL, Creds{ + InsecureSkipVerify: true, + Username: "my-username", + Password: "my-password", + }, true, "") + + tags, err := client.GetTags("mychart", true) + + assert.NoError(t, err) + assert.ElementsMatch(t, tags.Tags, []string{ + "2.8.0", + "2.8.0-prerelease", + "2.8.0+build", + "2.8.0-prerelease+build", + "2.8.0-prerelease.1+build.1234", + }) + }) + } }