fix repo url being decoded

Signed-off-by: Karuppiah Natarajan <karuppiah7890@gmail.com>

cmd/helm/init.go

@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"github.com/spf13/afero"
 	"io"
 	"os"
 	"time"
@@ -411,7 +412,7 @@ func initStableRepo(cacheFile string, out io.Writer, skipRefresh bool, home helm
 
 	// In this case, the cacheFile is always absolute. So passing empty string
 	// is safe.
-	if err := r.DownloadIndexFile(""); err != nil {
+	if err := r.DownloadIndexFile("", afero.NewOsFs()); err != nil {
 		return nil, fmt.Errorf("Looks like %q is not a valid chart repository or cannot be reached: %s", stableRepositoryURL, err.Error())
 	}
 

cmd/helm/repo_add.go

@@ -18,6 +18,7 @@ package main
 
 import (
 	"fmt"
+	"github.com/spf13/afero"
 	"io"
 
 	"github.com/spf13/cobra"
@@ -127,7 +128,7 @@ func addRepository(name, url, username, password string, home helmpath.Home, cer
 		return err
 	}
 
-	if err := r.DownloadIndexFile(home.Cache()); err != nil {
+	if err := r.DownloadIndexFile(home.Cache(), afero.NewOsFs()); err != nil {
 		return fmt.Errorf("Looks like %q is not a valid chart repository or cannot be reached: %s", url, err.Error())
 	}
 

cmd/helm/repo_update.go

@@ -19,6 +19,7 @@ package main
 import (
 	"errors"
 	"fmt"
+	"github.com/spf13/afero"
 	"io"
 	"sync"
 
@@ -96,7 +97,7 @@ func updateCharts(repos []*repo.ChartRepository, out io.Writer, home helmpath.Ho
 				fmt.Fprintf(out, "...Skip %s chart repository\n", re.Config.Name)
 				return
 			}
-			err := re.DownloadIndexFile(home.Cache())
+			err := re.DownloadIndexFile(home.Cache(), afero.NewOsFs())
 			if err != nil {
 				fmt.Fprintf(out, "...Unable to get an update from the %q chart repository (%s):\n\t%s\n", re.Config.Name, re.Config.URL, err)
 			} else {

glide.lock

@@ -251,6 +251,10 @@ imports:
   version: 10ef21a441db47d8b13ebcc5fd2310f636973c77
 - name: github.com/sirupsen/logrus
   version: 89742aefa4b206dcf400792f3bd35b542998eb3b
+- name: github.com/spf13/afero
+  version: 588a75ec4f32903aa5e39a2619ba6a4631e28424
+  subpackages:
+  - mem
 - name: github.com/spf13/cobra
   version: c439c4fa093711d42e1b01acb1235b52004753c1
   subpackages:
@@ -778,6 +782,8 @@ imports:
   subpackages:
   - sortorder
 testImports:
+- name: github.com/golang/mock
+  version: 9fa652df1129bef0e734c9cf9bf6dbae9ef3b9fa
 - name: github.com/pmezard/go-difflib
   version: d8ed2627bdf02c080bf22230dbb337003b7aba2d
   subpackages:

glide.yaml

@@ -61,9 +61,8 @@ import:
   version: kubernetes-1.11.1
 - package: github.com/cyphar/filepath-securejoin
   version: ^0.2.1
-
+- package: github.com/spf13/afero
-testImports:
+  version: ^1.2.2
-- package: github.com/stretchr/testify
+testImport:
-  version: ^1.1.4
+- package: github.com/golang/mock
-  subpackages:
+  version: ^1.3.1
-  - assert

pkg/downloader/manager.go

@@ -18,6 +18,7 @@ package downloader
 import (
 	"errors"
 	"fmt"
+	"github.com/spf13/afero"
 	"io"
 	"io/ioutil"
 	"net/url"
@@ -453,7 +454,7 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error {
 		}
 		wg.Add(1)
 		go func(r *repo.ChartRepository) {
-			if err := r.DownloadIndexFile(m.HelmHome.Cache()); err != nil {
+			if err := r.DownloadIndexFile(m.HelmHome.Cache(), afero.NewOsFs()); err != nil {
 				fmt.Fprintf(out, "...Unable to get an update from the %q chart repository (%s):\n\t%s\n", r.Config.Name, r.Config.URL, err)
 			} else {
 				fmt.Fprintf(out, "...Successfully got an update from the %q chart repository\n", r.Config.Name)

pkg/getter/mocks/getter.go

@@ -0,0 +1,67 @@
+/*
+Copyright The Helm Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// This is a mock for the Getter interface
+
+// Code generated by MockGen. DO NOT EDIT.
+// Source: pkg/getter/getter.go
+// Command To Generate: mockgen -source pkg/getter/getter.go -package mocks Getter > pkg/getter/mocks/getter.go
+
+// Package mocks is a generated GoMock package.
+package mocks
+
+import (
+	bytes "bytes"
+	gomock "github.com/golang/mock/gomock"
+	reflect "reflect"
+)
+
+// MockGetter is a mock of Getter interface
+type MockGetter struct {
+	ctrl     *gomock.Controller
+	recorder *MockGetterMockRecorder
+}
+
+// MockGetterMockRecorder is the mock recorder for MockGetter
+type MockGetterMockRecorder struct {
+	mock *MockGetter
+}
+
+// NewMockGetter creates a new mock instance
+func NewMockGetter(ctrl *gomock.Controller) *MockGetter {
+	mock := &MockGetter{ctrl: ctrl}
+	mock.recorder = &MockGetterMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use
+func (m *MockGetter) EXPECT() *MockGetterMockRecorder {
+	return m.recorder
+}
+
+// Get mocks base method
+func (m *MockGetter) Get(url string) (*bytes.Buffer, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Get", url)
+	ret0, _ := ret[0].(*bytes.Buffer)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// Get indicates an expected call of Get
+func (mr *MockGetterMockRecorder) Get(url interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockGetter)(nil).Get), url)
+}

pkg/repo/chartrepo.go

@@ -18,9 +18,11 @@ package repo // import "k8s.io/helm/pkg/repo"
 
 import (
 	"fmt"
+	"github.com/spf13/afero"
 	"io/ioutil"
 	"net/url"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 
@@ -110,15 +112,14 @@ func (r *ChartRepository) Load() error {
 //
 // cachePath is prepended to any index that does not have an absolute path. This
 // is for pre-2.2.0 repo files.
-func (r *ChartRepository) DownloadIndexFile(cachePath string) error {
+func (r *ChartRepository) DownloadIndexFile(cachePath string, fs afero.Fs) error {
-	var indexURL string
 	parsedURL, err := url.Parse(r.Config.URL)
 	if err != nil {
 		return err
 	}
-	parsedURL.Path = strings.TrimSuffix(parsedURL.Path, "/") + "/index.yaml"
+	parsedURL.RawPath = path.Join(parsedURL.RawPath, "index.yaml")
-
+	parsedURL.Path = path.Join(parsedURL.Path, "index.yaml")
-	indexURL = parsedURL.String()
+	indexURL := parsedURL.String()
 
 	r.setCredentials()
 	resp, err := r.Client.Get(indexURL)
@@ -146,7 +147,7 @@ func (r *ChartRepository) DownloadIndexFile(cachePath string) error {
 		cp = filepath.Join(cachePath, cp)
 	}
 
-	return ioutil.WriteFile(cp, index, 0644)
+	return afero.WriteFile(fs, cp, index, 0644)
 }
 
 // If HttpGetter is used, this method sets the configured repository credentials on the HttpGetter.
@@ -224,7 +225,7 @@ func FindChartInAuthRepoURL(repoURL, username, password, chartName, chartVersion
 	if err != nil {
 		return "", err
 	}
-	if err := r.DownloadIndexFile(tempIndexFile.Name()); err != nil {
+	if err := r.DownloadIndexFile(tempIndexFile.Name(), afero.NewOsFs()); err != nil {
 		return "", fmt.Errorf("Looks like %q is not a valid chart repository or cannot be reached: %s", repoURL, err)
 	}
 

pkg/repo/chartrepo_test.go

@@ -17,7 +17,12 @@ limitations under the License.
 package repo
 
 import (
+	"bytes"
+	"github.com/golang/mock/gomock"
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/assert"
 	"io/ioutil"
+	"k8s.io/helm/pkg/getter/mocks"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -311,3 +316,48 @@ func TestResolveReferenceURL(t *testing.T) {
 		t.Errorf("%s contains query string from base URL when it shouldn't", chartURL)
 	}
 }
+
+func TestChartRepository_DownloadIndexFile(t *testing.T) {
+	t.Run("should not decode the path in the repo url", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+
+		r, err := NewChartRepository(&Entry{
+			Name:  "testrepo",
+			URL:   "http://example-charts.com/some%2Fpath/test",
+			Cache: "/repository/cache/testrepo-index.yaml",
+		}, getter.All(environment.EnvSettings{}))
+		if err != nil {
+			t.Errorf("Problem creating chart repository from %s: %v", "testrepo", err)
+		}
+
+		fs := afero.NewMemMapFs()
+		err = fs.MkdirAll("/repository/cache/", 0644)
+		assert.NoError(t, err, "problem creating test repository cache")
+
+		mockGetter := mocks.NewMockGetter(ctrl)
+		r.Client = mockGetter
+		indexYamlContent := `apiVersion: v1
+entries:
+  testchart:
+  - apiVersion: v1
+    appVersion: 1.2.3
+    created: 2019-07-06T17:56:26.384768233Z
+    description: test
+    digest: 93f924d4498d588bcdda88c7401e27c6fa0f50ff0601e78885eca13eb683c1e2
+    home: https://github.com/test/test
+    icon: https://github.com/test/test/test.png
+    name: testchart
+    sources:
+    - https://github.com/test/test
+    urls:
+    - https://test.com/test-1.0.0.tgz
+    version: 1.0.0`
+
+		mockGetter.EXPECT().Get("http://example-charts.com/some%2Fpath/test/index.yaml").
+			Return(bytes.NewBufferString(indexYamlContent), nil)
+
+		err = r.DownloadIndexFile("", fs)
+		assert.NoError(t, err, "problem downloading index file")
+	})
+}

pkg/repo/index_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package repo
 
 import (
+	"github.com/spf13/afero"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -151,7 +152,7 @@ func TestDownloadIndexFile(t *testing.T) {
 		t.Errorf("Problem creating chart repository from %s: %v", testRepo, err)
 	}
 
-	if err := r.DownloadIndexFile(""); err != nil {
+	if err := r.DownloadIndexFile("", afero.NewOsFs()); err != nil {
 		t.Errorf("%#v", err)
 	}