Merge pull request #1322 from technosophos/ref/move-urljoin

ref(repo): move urlJoin to the right package

cmd/helm/downloader/chart_downloader.go

@@ -24,7 +24,6 @@ import (
 	"net/http"
 	"net/url"
 	"os"
-	"path"
 	"path/filepath"
 	"strings"
 
@@ -176,24 +175,6 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er
 	return url.Parse(cv.URLs[0])
 }
 
-// urlJoin joins a base URL to one or more path components.
-//
-// It's like filepath.Join for URLs. If the baseURL is pathish, this will still
-// perform a join.
-//
-// If the URL is unparsable, this returns an error.
-func urlJoin(baseURL string, paths ...string) (string, error) {
-	u, err := url.Parse(baseURL)
-	if err != nil {
-		return "", err
-	}
-	// We want path instead of filepath because path always uses /.
-	all := []string{u.Path}
-	all = append(all, paths...)
-	u.Path = path.Join(all...)
-	return u.String(), nil
-}
-
 func findRepoEntry(name string, repos []*repo.Entry) (*repo.Entry, error) {
 	for _, re := range repos {
 		if re.Name == name {

cmd/helm/downloader/chart_downloader_test.go

@@ -153,24 +153,3 @@ func TestDownloadTo(t *testing.T) {
 		return
 	}
 }
-
-func TestUrlJoin(t *testing.T) {
-	tests := []struct {
-		name, url, expect string
-		paths             []string
-	}{
-		{name: "URL, one path", url: "http://example.com", paths: []string{"hello"}, expect: "http://example.com/hello"},
-		{name: "Long URL, one path", url: "http://example.com/but/first", paths: []string{"slurm"}, expect: "http://example.com/but/first/slurm"},
-		{name: "URL, two paths", url: "http://example.com", paths: []string{"hello", "world"}, expect: "http://example.com/hello/world"},
-		{name: "URL, no paths", url: "http://example.com", paths: []string{}, expect: "http://example.com"},
-		{name: "basepath, two paths", url: "../example.com", paths: []string{"hello", "world"}, expect: "../example.com/hello/world"},
-	}
-
-	for _, tt := range tests {
-		if got, err := urlJoin(tt.url, tt.paths...); err != nil {
-			t.Errorf("%s: error %q", tt.name, err)
-		} else if got != tt.expect {
-			t.Errorf("%s: expected %q, got %q", tt.name, tt.expect, got)
-		}
-	}
-}

pkg/repo/index.go

@@ -22,7 +22,9 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"net/url"
 	"os"
+	"path"
 	"path/filepath"
 	"sort"
 	"strings"
@@ -96,8 +98,12 @@ func NewIndexFile() *IndexFile {
 func (i IndexFile) Add(md *chart.Metadata, filename, baseURL, digest string) {
 	u := filename
 	if baseURL != "" {
+		var err error
 		_, file := filepath.Split(filename)
-		u = strings.TrimSuffix(baseURL, "/") + "/" + file
+		u, err = urlJoin(baseURL, file)
+		if err != nil {
+			u = filepath.Join(baseURL, file)
+		}
 	}
 	cr := &ChartVersion{
 		URLs:     []string{u},
@@ -295,3 +301,21 @@ func LoadIndexFile(path string) (*IndexFile, error) {
 	}
 	return LoadIndex(b)
 }
+
+// urlJoin joins a base URL to one or more path components.
+//
+// It's like filepath.Join for URLs. If the baseURL is pathish, this will still
+// perform a join.
+//
+// If the URL is unparsable, this returns an error.
+func urlJoin(baseURL string, paths ...string) (string, error) {
+	u, err := url.Parse(baseURL)
+	if err != nil {
+		return "", err
+	}
+	// We want path instead of filepath because path always uses /.
+	all := []string{u.Path}
+	all = append(all, paths...)
+	u.Path = path.Join(all...)
+	return u.String(), nil
+}

pkg/repo/index_test.go

@@ -292,3 +292,24 @@ func TestIndexAdd(t *testing.T) {
 		t.Errorf("Expected http://example.com/charts/deis-0.1.0.tgz, got %s", i.Entries["deis"][0].URLs[0])
 	}
 }
+
+func TestUrlJoin(t *testing.T) {
+	tests := []struct {
+		name, url, expect string
+		paths             []string
+	}{
+		{name: "URL, one path", url: "http://example.com", paths: []string{"hello"}, expect: "http://example.com/hello"},
+		{name: "Long URL, one path", url: "http://example.com/but/first", paths: []string{"slurm"}, expect: "http://example.com/but/first/slurm"},
+		{name: "URL, two paths", url: "http://example.com", paths: []string{"hello", "world"}, expect: "http://example.com/hello/world"},
+		{name: "URL, no paths", url: "http://example.com", paths: []string{}, expect: "http://example.com"},
+		{name: "basepath, two paths", url: "../example.com", paths: []string{"hello", "world"}, expect: "../example.com/hello/world"},
+	}
+
+	for _, tt := range tests {
+		if got, err := urlJoin(tt.url, tt.paths...); err != nil {
+			t.Errorf("%s: error %q", tt.name, err)
+		} else if got != tt.expect {
+			t.Errorf("%s: expected %q, got %q", tt.name, tt.expect, got)
+		}
+	}
+}