From 071646cc6c3a2f7dfd9f6d0d55cc29e8eee50ae7 Mon Sep 17 00:00:00 2001 From: toby cabot Date: Wed, 11 Aug 2021 17:04:35 -0400 Subject: [PATCH] Allow repos with URL-encoded paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://github.com/helm/helm/issues/9977 Repos that have URL-encoded characters in the path cause the "helm install" command to fail. For example: ``` "purelb" has been added to your repositories Hang tight while we grab the latest from your chart repositories... ...Successfully got an update from the "purelb" chart repository Update Complete. ⎈ Happy Helming!⎈ install.go:158: [debug] Original chart version: "" install.go:160: [debug] setting version to >0.0.0-0 Error: failed to fetch https://gitlab.com/api/v4/projects/purelb/purelb/packages/helm/stable/charts/purelb-v0.0.0-open-source-helm-chart-latest.tgz : 404 Not Found helm.go:75: [debug] failed to fetch https://gitlab.com/api/v4/projects/purelb/purelb/packages/helm/stable/charts/purelb-v0.0.0-open-source-helm-chart-latest.tgz : 404 Not Found helm.sh/helm/v3/pkg/getter.(*HTTPGetter).get /home/circleci/helm.sh/helm/pkg/getter/httpgetter.go:72 helm.sh/helm/v3/pkg/getter.(*HTTPGetter).Get /home/circleci/helm.sh/helm/pkg/getter/httpgetter.go:40 helm.sh/helm/v3/pkg/downloader.(*ChartDownloader).DownloadTo /home/circleci/helm.sh/helm/pkg/downloader/chart_downloader.go:97 helm.sh/helm/v3/pkg/action.(*ChartPathOptions).LocateChart /home/circleci/helm.sh/helm/pkg/action/install.go:738 main.runInstall /home/circleci/helm.sh/helm/cmd/helm/install.go:170 main.newInstallCmd.func1 /home/circleci/helm.sh/helm/cmd/helm/install.go:117 github.com/spf13/cobra.(*Command).execute /go/pkg/mod/github.com/spf13/cobra@v0.0.5/command.go:826 github.com/spf13/cobra.(*Command).ExecuteC /go/pkg/mod/github.com/spf13/cobra@v0.0.5/command.go:914 github.com/spf13/cobra.(*Command).Execute /go/pkg/mod/github.com/spf13/cobra@v0.0.5/command.go:864 main.main /home/circleci/helm.sh/helm/cmd/helm/helm.go:74 runtime.main /usr/local/go/src/runtime/proc.go:203 runtime.goexit /usr/local/go/src/runtime/asm_amd64.s:1357 ``` Note that the "purelb%2Fpurelb" in the original repo URL has been un-escaped and is now "purelb/purelb". It turns out that the golang net/url package gets confused if the Path and RawPath fields diverge, and it looks like it throws away RawPath. That's speculation on my part. In any case, setting *both* Path and RawPath keeps the URL-encoding. Signed-off-by: Chilton Cabot Signed-off-by: toby cabot --- pkg/downloader/chart_downloader.go | 3 +++ pkg/downloader/chart_downloader_test.go | 1 + pkg/downloader/testdata/repositories.yaml | 2 ++ .../testing-url-encoded-path-index.yaml | 15 +++++++++++++++ 4 files changed, 21 insertions(+) create mode 100644 pkg/downloader/testdata/repository/testing-url-encoded-path-index.yaml diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 575c94151..dbee53712 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -268,6 +268,9 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er q := repoURL.Query() // We need a trailing slash for ResolveReference to work, but make sure there isn't already one repoURL.Path = strings.TrimSuffix(repoURL.Path, "/") + "/" + // The URL class has problems with URL-escaped paths if we don't + // modify both paths. See https://github.com/helm/helm/issues/9977 + repoURL.RawPath = strings.TrimSuffix(repoURL.RawPath, "/") + "/" u = repoURL.ResolveReference(u) u.RawQuery = q.Encode() // TODO add user-agent diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go index 38a06671c..8b9d110cd 100644 --- a/pkg/downloader/chart_downloader_test.go +++ b/pkg/downloader/chart_downloader_test.go @@ -49,6 +49,7 @@ func TestResolveChartRef(t *testing.T) { {name: "reference, testing-relative repo", ref: "testing-relative/bar", expect: "http://example.com/helm/bar-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: "reference, testing-url-encoded-path repo", ref: "testing-url-encoded-path/foo", expect: "http://example.com/path%2fpath/charts/foo-1.2.3.tgz"}, {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/repositories.yaml b/pkg/downloader/testdata/repositories.yaml index 32bc395a0..f174ce9f5 100644 --- a/pkg/downloader/testdata/repositories.yaml +++ b/pkg/downloader/testdata/repositories.yaml @@ -16,6 +16,8 @@ repositories: url: "http://example.com/helm" - name: testing-relative-trailing-slash url: "http://example.com/helm/" + - name: testing-url-encoded-path + url: "http://example.com/path%2fpath" - name: testing-ca-file url: "https://example.com" certFile: "cert" diff --git a/pkg/downloader/testdata/repository/testing-url-encoded-path-index.yaml b/pkg/downloader/testdata/repository/testing-url-encoded-path-index.yaml new file mode 100644 index 000000000..857248daa --- /dev/null +++ b/pkg/downloader/testdata/repository/testing-url-encoded-path-index.yaml @@ -0,0 +1,15 @@ +apiVersion: v1 +entries: + foo: + - name: foo + description: Foo Chart With URL-Encoded Path + home: https://helm.sh/helm + keywords: [] + maintainers: [] + sources: + - https://example.com/path%2fpath + urls: + - charts/foo-1.2.3.tgz + version: 1.2.3 + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2