From dad8c6f644b618983c7b6eca8146d736c73d04fd Mon Sep 17 00:00:00 2001 From: Michal Cwienczek Date: Fri, 6 Oct 2017 16:59:11 +0200 Subject: [PATCH] Fix #2937 - helm always appends /index.yaml at the end of URL (#2988) * Closes #2937 Added required dependency to run make test in developer's guide * Fixed base URL appending when chart address is not absolute * Removed requirement from developers.md * Fixed unnecessary line breaks * Added tests for query string repo * Returning URL along with error --- pkg/downloader/chart_downloader.go | 7 ++++++- pkg/downloader/chart_downloader_test.go | 1 + .../cache/testing-querystring-index.yaml | 16 ++++++++++++++++ .../helmhome/repository/repositories.yaml | 2 ++ pkg/repo/chartrepo.go | 7 ++++++- 5 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 pkg/downloader/testdata/helmhome/repository/cache/testing-querystring-index.yaml diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 15e97f6a1..a8a1b5a57 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -217,7 +217,12 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, ge // If the URL is relative (no scheme), prepend the chart repo's base URL if !u.IsAbs() { - u, err = url.Parse(rc.URL + "/" + u.Path) + path := u.Path + u, err = url.Parse(rc.URL) + if err != nil { + return u, r.Client, err + } + u.Path = u.Path + path return u, r.Client, err } diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go index 4049b7979..73f9191c9 100644 --- a/pkg/downloader/chart_downloader_test.go +++ b/pkg/downloader/chart_downloader_test.go @@ -43,6 +43,7 @@ func TestResolveChartRef(t *testing.T) { {name: "reference, testing repo", ref: "testing/alpine", expect: "http://example.com/alpine-1.2.3.tgz"}, {name: "reference, version, testing repo", ref: "testing/alpine", version: "0.2.0", expect: "http://example.com/alpine-0.2.0.tgz"}, {name: "reference, version, malformed repo", ref: "malformed/alpine", version: "1.2.3", expect: "http://dl.example.com/alpine-1.2.3.tgz"}, + {name: "reference, querystring repo", ref: "testing-querystring/alpine", expect: "http://example.com/alpine-1.2.3.tgz?key=value"}, {name: "full URL, HTTPS, irrelevant version", ref: "https://example.com/foo-1.2.3.tgz", version: "0.1.0", expect: "https://example.com/foo-1.2.3.tgz", fail: true}, {name: "full URL, file", ref: "file:///foo-1.2.3.tgz", fail: true}, {name: "invalid", ref: "invalid-1.2.3", fail: true}, diff --git a/pkg/downloader/testdata/helmhome/repository/cache/testing-querystring-index.yaml b/pkg/downloader/testdata/helmhome/repository/cache/testing-querystring-index.yaml new file mode 100644 index 000000000..1956e9f83 --- /dev/null +++ b/pkg/downloader/testdata/helmhome/repository/cache/testing-querystring-index.yaml @@ -0,0 +1,16 @@ +apiVersion: v1 +entries: + alpine: + - name: alpine + urls: + - alpine-1.2.3.tgz + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + home: https://k8s.io/helm + sources: + - https://github.com/kubernetes/helm + version: 1.2.3 + description: Deploy a basic Alpine Linux pod + keywords: [] + maintainers: [] + engine: "" + icon: "" diff --git a/pkg/downloader/testdata/helmhome/repository/repositories.yaml b/pkg/downloader/testdata/helmhome/repository/repositories.yaml index 9b0cfe972..68efb461a 100644 --- a/pkg/downloader/testdata/helmhome/repository/repositories.yaml +++ b/pkg/downloader/testdata/helmhome/repository/repositories.yaml @@ -10,3 +10,5 @@ repositories: url: "http://example.com/charts" - name: malformed url: "http://dl.example.com" + - name: testing-querystring + url: "http://example.com?key=value" \ No newline at end of file diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index 97506e607..e7283dc10 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -110,8 +110,13 @@ func (r *ChartRepository) Load() error { // is for pre-2.2.0 repo files. func (r *ChartRepository) DownloadIndexFile(cachePath string) error { var indexURL string + parsedURL, err := url.Parse(r.Config.URL) + if err != nil { + return err + } + parsedURL.Path = strings.TrimSuffix(parsedURL.Path, "/") + "/index.yaml" - indexURL = strings.TrimSuffix(r.Config.URL, "/") + "/index.yaml" + indexURL = parsedURL.String() resp, err := r.Client.Get(indexURL) if err != nil { return err