fix: correctly concat absolute URIs in repo cache

There used to be two implemenations for concatenating the repo URL with
the chart URI / URL. In case the chart specified an absolute URI, one of
the implementations performed an incorrect concatenation between the
two, resulting in a URL which looks like <repo-URL><absolute-chart-URI>.

This commit removes the faulty implementation and uses the other correct
one instead.

Signed-off-by: Omri Steiner <omri@steiners.co.il>

pkg/downloader/chart_downloader_test.go

@@ -46,6 +46,7 @@ func TestResolveChartRef(t *testing.T) {
 		{name: "reference, querystring repo", ref: "testing-querystring/alpine", expect: "http://example.com/alpine-1.2.3.tgz?key=value"},
 		{name: "reference, testing-relative repo", ref: "testing-relative/foo", expect: "http://example.com/helm/charts/foo-1.2.3.tgz"},
 		{name: "reference, testing-relative repo", ref: "testing-relative/bar", expect: "http://example.com/helm/bar-1.2.3.tgz"},
+		{name: "reference, testing-relative repo", ref: "testing-relative/baz", expect: "http://example.com/path/to/baz-1.2.3.tgz"},
 		{name: "reference, testing-relative-trailing-slash repo", ref: "testing-relative-trailing-slash/foo", expect: "http://example.com/helm/charts/foo-1.2.3.tgz"},
 		{name: "reference, testing-relative-trailing-slash repo", ref: "testing-relative-trailing-slash/bar", expect: "http://example.com/helm/bar-1.2.3.tgz"},
 		{name: "encoded URL", ref: "encoded-url/foobar", expect: "http://example.com/with%2Fslash/charts/foobar-4.2.1.tgz"},

pkg/downloader/manager.go

@@ -25,7 +25,6 @@ import (
 	"log"
 	"net/url"
 	"os"
-	"path"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -728,7 +727,6 @@ func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*
 	}
 
 	for _, cr := range repos {
-
 		if urlutil.Equal(repoURL, cr.Config.URL) {
 			var entry repo.ChartVersions
 			entry, err = findEntryByName(name, cr)
@@ -745,7 +743,7 @@ func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*
 				//nolint:nakedret
 				return
 			}
-			url, err = normalizeURL(repoURL, ve.URLs[0])
+			url, err = repo.ResolveReferenceURL(repoURL, ve.URLs[0])
 			if err != nil {
 				//nolint:nakedret
 				return
@@ -811,24 +809,6 @@ func versionEquals(v1, v2 string) bool {
 	return sv1.Equal(sv2)
 }
 
-func normalizeURL(baseURL, urlOrPath string) (string, error) {
-	u, err := url.Parse(urlOrPath)
-	if err != nil {
-		return urlOrPath, err
-	}
-	if u.IsAbs() {
-		return u.String(), nil
-	}
-	u2, err := url.Parse(baseURL)
-	if err != nil {
-		return urlOrPath, fmt.Errorf("base URL failed to parse: %w", err)
-	}
-
-	u2.RawPath = path.Join(u2.RawPath, urlOrPath)
-	u2.Path = path.Join(u2.Path, urlOrPath)
-	return u2.String(), nil
-}
-
 // loadChartRepositories reads the repositories.yaml, and then builds a map of
 // ChartRepositories.
 //

pkg/downloader/manager_test.go

@@ -53,26 +53,6 @@ func TestVersionEquals(t *testing.T) {
 	}
 }
 
-func TestNormalizeURL(t *testing.T) {
-	tests := []struct {
-		name, base, path, expect string
-	}{
-		{name: "basic URL", base: "https://example.com", path: "http://helm.sh/foo", expect: "http://helm.sh/foo"},
-		{name: "relative path", base: "https://helm.sh/charts", path: "foo", expect: "https://helm.sh/charts/foo"},
-		{name: "Encoded path", base: "https://helm.sh/a%2Fb/charts", path: "foo", expect: "https://helm.sh/a%2Fb/charts/foo"},
-	}
-
-	for _, tt := range tests {
-		got, err := normalizeURL(tt.base, tt.path)
-		if err != nil {
-			t.Errorf("%s: error %s", tt.name, err)
-			continue
-		} else if got != tt.expect {
-			t.Errorf("%s: expected %q, got %q", tt.name, tt.expect, got)
-		}
-	}
-}
-
 func TestFindChartURL(t *testing.T) {
 	var b bytes.Buffer
 	m := &Manager{
@@ -134,6 +114,31 @@ func TestFindChartURL(t *testing.T) {
 	if passcredentialsall != false {
 		t.Errorf("Unexpected passcredentialsall %t", passcredentialsall)
 	}
+
+	name = "baz"
+	version = "1.2.3"
+	repoURL = "http://example.com/helm"
+
+	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if churl != "http://example.com/path/to/baz-1.2.3.tgz" {
+		t.Errorf("Unexpected URL %q", churl)
+	}
+	if username != "" {
+		t.Errorf("Unexpected username %q", username)
+	}
+	if password != "" {
+		t.Errorf("Unexpected password %q", password)
+	}
+	if passcredentialsall != false {
+		t.Errorf("Unexpected passcredentialsall %t", passcredentialsall)
+	}
+	if insecureSkipTLSVerify {
+		t.Errorf("Unexpected insecureSkipTLSVerify %t", insecureSkipTLSVerify)
+	}
 }
 
 func TestGetRepoNames(t *testing.T) {

pkg/downloader/testdata/repository/testing-relative-index.yaml

@@ -26,3 +26,16 @@ entries:
       version: 1.2.3
       checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d
       apiVersion: v2
+  baz:
+    - name: baz
+      description: Baz Chart With Absolute Path
+      home: https://helm.sh/helm
+      keywords: []
+      maintainers: []
+      sources:
+        - https://github.com/helm/charts
+      urls:
+        - /path/to/baz-1.2.3.tgz
+      version: 1.2.3
+      checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d
+      apiVersion: v2

pkg/repo/chartrepo_test.go

@@ -224,11 +224,15 @@ func TestResolveReferenceURL(t *testing.T) {
 	for _, tt := range []struct {
 		baseURL, refURL, chartURL string
 	}{
+		{"http://localhost:8123/", "/nginx-0.2.0.tgz", "http://localhost:8123/nginx-0.2.0.tgz"},
 		{"http://localhost:8123/charts/", "nginx-0.2.0.tgz", "http://localhost:8123/charts/nginx-0.2.0.tgz"},
+		{"http://localhost:8123/charts/", "/nginx-0.2.0.tgz", "http://localhost:8123/nginx-0.2.0.tgz"},
 		{"http://localhost:8123/charts-with-no-trailing-slash", "nginx-0.2.0.tgz", "http://localhost:8123/charts-with-no-trailing-slash/nginx-0.2.0.tgz"},
 		{"http://localhost:8123", "https://charts.helm.sh/stable/nginx-0.2.0.tgz", "https://charts.helm.sh/stable/nginx-0.2.0.tgz"},
 		{"http://localhost:8123/charts%2fwith%2fescaped%2fslash", "nginx-0.2.0.tgz", "http://localhost:8123/charts%2fwith%2fescaped%2fslash/nginx-0.2.0.tgz"},
+		{"http://localhost:8123/charts%2fwith%2fescaped%2fslash", "/nginx-0.2.0.tgz", "http://localhost:8123/nginx-0.2.0.tgz"},
 		{"http://localhost:8123/charts?with=queryparameter", "nginx-0.2.0.tgz", "http://localhost:8123/charts/nginx-0.2.0.tgz?with=queryparameter"},
+		{"http://localhost:8123/charts?with=queryparameter", "/nginx-0.2.0.tgz", "http://localhost:8123/nginx-0.2.0.tgz?with=queryparameter"},
 	} {
 		chartURL, err := ResolveReferenceURL(tt.baseURL, tt.refURL)
 		if err != nil {