From fa54ce221e62e881c5c7920e9b053fe80fd34792 Mon Sep 17 00:00:00 2001 From: Tony Au-Yeung Date: Fri, 27 Sep 2024 11:03:42 -0500 Subject: [PATCH] fix: oras-go client should fallback to docker config if no credentials specified (#18133) * oras-go client should fallback to docker config if no credentials specified Signed-off-by: Tony Au-Yeung * Fix tests Signed-off-by: Tony Au-Yeung * Fix lint Signed-off-by: Tony Au-Yeung * gofumpt Signed-off-by: Tony Au-Yeung * Validate auth header Signed-off-by: Tony Au-Yeung --------- Signed-off-by: Tony Au-Yeung --- util/helm/client.go | 23 +++++++--- util/helm/client_test.go | 97 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 111 insertions(+), 9 deletions(-) diff --git a/util/helm/client.go b/util/helm/client.go index 3ddbcec4333a6..d9972adb04968 100644 --- a/util/helm/client.go +++ b/util/helm/client.go @@ -25,6 +25,7 @@ import ( "gopkg.in/yaml.v2" "oras.land/oras-go/v2/registry/remote" "oras.land/oras-go/v2/registry/remote/auth" + "oras.land/oras-go/v2/registry/remote/credentials" "github.com/argoproj/argo-cd/v2/util/cache" argoio "github.com/argoproj/argo-cd/v2/util/io" @@ -447,13 +448,23 @@ func (c *nativeHelmChart) GetTags(chart string, noCache bool) (*TagsList, error) }} repoHost, _, _ := strings.Cut(tagsURL, "/") + credential := auth.StaticCredential(repoHost, auth.Credential{ + Username: c.creds.Username, + Password: c.creds.Password, + }) + + // Try to fallback to the environment config, but we shouldn't error if the file is not set + if c.creds.Username == "" && c.creds.Password == "" { + store, _ := credentials.NewStoreFromDocker(credentials.StoreOptions{}) + if store != nil { + credential = credentials.Credential(store) + } + } + repo.Client = &auth.Client{ - Client: client, - Cache: nil, - Credential: auth.StaticCredential(repoHost, auth.Credential{ - Username: c.creds.Username, - Password: c.creds.Password, - }), + Client: client, + Cache: nil, + Credential: credential, } ctx := context.Background() diff --git a/util/helm/client_test.go b/util/helm/client_test.go index f03bd15bf096d..cae4574e2e86b 100644 --- a/util/helm/client_test.go +++ b/util/helm/client_test.go @@ -9,6 +9,7 @@ import ( "net/http/httptest" "net/url" "os" + "path/filepath" "strings" "testing" @@ -214,6 +215,91 @@ func TestGetTagsFromUrl(t *testing.T) { } func TestGetTagsFromURLPrivateRepoAuthentication(t *testing.T) { + username := "my-username" + password := "my-password" + expectedAuthorization := "Basic bXktdXNlcm5hbWU6bXktcGFzc3dvcmQ=" // base64(user:password) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Logf("called %s", r.URL.Path) + + authorization := r.Header.Get("Authorization") + + if authorization == "" { + w.Header().Set("WWW-Authenticate", `Basic realm="helm repo to get tags"`) + w.WriteHeader(http.StatusUnauthorized) + return + } + + assert.Equal(t, expectedAuthorization, 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) + + serverURL, err := url.Parse(server.URL) + require.NoError(t, err) + + 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: username, + Password: password, + }, true, "", "") + + tags, err := client.GetTags("mychart", true) + + require.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", + }) + }) + } +} + +func TestGetTagsFromURLEnvironmentAuthentication(t *testing.T) { + bearerToken := "Zm9vOmJhcg==" + expectedAuthorization := "Basic " + bearerToken server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { t.Logf("called %s", r.URL.Path) @@ -224,7 +310,7 @@ func TestGetTagsFromURLPrivateRepoAuthentication(t *testing.T) { return } - t.Logf("authorization received %s", authorization) + assert.Equal(t, expectedAuthorization, authorization) responseTags := TagsList{ Tags: []string{ @@ -248,6 +334,13 @@ func TestGetTagsFromURLPrivateRepoAuthentication(t *testing.T) { serverURL, err := url.Parse(server.URL) require.NoError(t, err) + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.json") + t.Setenv("DOCKER_CONFIG", tempDir) + + config := fmt.Sprintf(`{"auths":{"%s":{"auth":"%s"}}}`, server.URL, bearerToken) + require.NoError(t, os.WriteFile(configPath, []byte(config), 0o666)) + testCases := []struct { name string repoURL string @@ -274,8 +367,6 @@ func TestGetTagsFromURLPrivateRepoAuthentication(t *testing.T) { 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)