fix: oras-go client should fallback to docker config if no credential…

…s specified (argoproj#18133) * oras-go client should fallback to docker config if no credentials specified Signed-off-by: Tony Au-Yeung <tony@elevenlabs.io> * Fix tests Signed-off-by: Tony Au-Yeung <tony@elevenlabs.io> * Fix lint Signed-off-by: Tony Au-Yeung <tony@elevenlabs.io> * gofumpt Signed-off-by: Tony Au-Yeung <tony@elevenlabs.io> * Validate auth header Signed-off-by: Tony Au-Yeung <tony@elevenlabs.io> --------- Signed-off-by: Tony Au-Yeung <tony@elevenlabs.io>
blakepettersson · Sep 27, 2024 · fa54ce2 · fa54ce2
1 parent ea71067
commit fa54ce2
Show file tree

Hide file tree

Showing 2 changed files with 111 additions and 9 deletions.
diff --git 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
@@ -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)