diff --git a/cmd/helm/downloader/chart_downloader.go b/cmd/helm/downloader/chart_downloader.go index 2871f1dea..84def6cd0 100644 --- a/cmd/helm/downloader/chart_downloader.go +++ b/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 { diff --git a/cmd/helm/downloader/chart_downloader_test.go b/cmd/helm/downloader/chart_downloader_test.go index 545943304..d2cf191f9 100644 --- a/cmd/helm/downloader/chart_downloader_test.go +++ b/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) - } - } -} diff --git a/pkg/repo/index.go b/pkg/repo/index.go index 5b591f7e5..d9d04762c 100644 --- a/pkg/repo/index.go +++ b/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 +} diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go index 33203c495..080023f24 100644 --- a/pkg/repo/index_test.go +++ b/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) + } + } +}