fix: registry argument to be only the host instead full URL (#17381) (#…

…17534)

Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br>
Co-authored-by: Pablo Aguilar <pablo.aguilar@outlook.com.br>

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,
 			}),

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("<https://%s%s?token=next-token>; 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("<https://%s%s?token=next-token>; 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",
+			})
+		})
+	}
 }