Repository: helm/helm (go)

Issue:
helm v3 packaging charts differently on windows and linux (breaks artifactory)
I'm trying to package a helm chart and deploy it to a private helm repo on artifactory 6.18.0.

This works fine if I package the chart on linux, but fails if the chart is packaged on windows.

It also works fine with charts packaged on windows with a v2 helm client, so it appears to be an issue introduced in helm v3.

I can also see that the resultant tgz files are different by listing their contents with tar (note the backslashes):

Good (linux or windows v2):
```
$ tar -tvf tds-proxy-1.0.0.tgz
-rwxr-xr-x 0/0             129 2020-03-09 11:47 tds-proxy/Chart.yaml
-rwxr-xr-x 0/0             195 2020-03-09 11:47 tds-proxy/values.yaml
-rwxr-xr-x 0/0            1033 2020-03-09 11:47 tds-proxy/templates/deployment.yaml
-rwxr-xr-x 0/0             547 2020-03-09 11:47 tds-proxy/templates/service.yaml
-rwxr-xr-x 0/0             364 2020-03-09 11:47 tds-proxy/.helmignore
```

Bad (helm v3 on windows):
```
$ tar -tvf tds-proxy-1.0.0.tgz
-rw-r--r-- 0/0             129 2020-03-09 11:57 tds-proxy\\Chart.yaml
-rw-r--r-- 0/0             195 2020-03-09 11:57 tds-proxy\\values.yaml
-rw-r--r-- 0/0            1033 2020-03-09 11:57 tds-proxy\\templates\\deployment.yaml
-rw-r--r-- 0/0             547 2020-03-09 11:57 tds-proxy\\templates\\service.yaml
-rw-r--r-- 0/0             364 2020-03-09 11:57 tds-proxy\\.helmignore
```

This has been [raised as an issue with artifactory](https://www.jfrog.com/jira/browse/RTFACT-21145), but I feel it's up to helm to package consistently irrespective of where helm package is run. This looks like a bug in the helm v3 windows client.

Output of `helm version`:
`version.BuildInfo{Version:"v3.1.1", GitCommit:"afe70585407b420d0097d07b21c47dc511525ac8", GitTreeState:"clean", GoVersion:"go1.13.8"}`

Output of `kubectl version`:
```
Client Version: version.Info{Major:"1", Minor:"17", GitVersion:"v1.17.2", GitCommit:"59603c6e503c87169aea6106f57b9f242f64df89", GitTreeState:"clean", BuildDate:"2020-01-18T23:30:10Z", GoVersion:"go1.13.5", Compiler:"gc", Platform:"windows/amd64"}
Server Version: version.Info{Major:"1", Minor:"17", GitVersion:"v1.17.2", GitCommit:"59603c6e503c87169aea6106f57b9f242f64df89", GitTreeState:"clean", BuildDate:"2020-01-18T23:22:30Z", GoVersion:"go1.13.5", Compiler:"gc", Platform:"linux/amd64"}
```

Cloud Provider/Platform (AKS, GKE, Minikube etc.): 
not relevant (bare metal ish)



Pull request:
Fixing issue where archives created on windows have broken paths
When archives are created on windows the path spearator in the
archive file is \\. This causes issues when the file is unpacked.
For example, on Linux the files are unpacked in a flat structure
and \ is part of the file name. This causes comp issues. In Helm
v2 the path was set as / when the archive was written. This works
on both Windows and POSIX systems.

The fix being implemented is to use the ToSlash function to ensure
/ is used as the separator.

Fixes #7748

Patch:
diff --git a/pkg/action/install.go b/pkg/action/install.go
index fa13bbade..e7b481d47 100644
--- a/pkg/action/install.go
+++ b/pkg/action/install.go
@@ -744,7 +744,6 @@ func (c *ChartPathOptions) LocateChart(name string, settings *cli.EnvSettings) (
 		Getters: getter.All(settings),
 		Options: []getter.Option{
 			getter.WithBasicAuth(c.Username, c.Password),
-			getter.WithTLSClientConfig(c.CertFile, c.KeyFile, c.CaFile),
 		},
 		RepositoryConfig: settings.RepositoryConfig,
 		RepositoryCache:  settings.RepositoryCache,
diff --git a/pkg/chartutil/save.go b/pkg/chartutil/save.go
index a2c6a9225..be5d151d7 100644
--- a/pkg/chartutil/save.go
+++ b/pkg/chartutil/save.go
@@ -223,7 +223,7 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
 func writeToTar(out *tar.Writer, name string, body []byte) error {
 	// TODO: Do we need to create dummy parent directory names if none exist?
 	h := &tar.Header{
-		Name:    name,
+		Name:    filepath.ToSlash(name),
 		Mode:    0644,
 		Size:    int64(len(body)),
 		ModTime: time.Now(),
diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go
index 340a65472..f3d4321c5 100644
--- a/pkg/downloader/chart_downloader.go
+++ b/pkg/downloader/chart_downloader.go
@@ -181,10 +181,8 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er
 		c.Options = append(
 			c.Options,
 			getter.WithURL(rc.URL),
+			getter.WithTLSClientConfig(rc.CertFile, rc.KeyFile, rc.CAFile),
 		)
-		if rc.CertFile != "" || rc.KeyFile != "" || rc.CAFile != "" {
-			c.Options = append(c.Options, getter.WithTLSClientConfig(rc.CertFile, rc.KeyFile, rc.CAFile))
-		}
 		if rc.Username != "" && rc.Password != "" {
 			c.Options = append(
 				c.Options,
@@ -212,14 +210,12 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er
 	if err != nil {
 		return u, err
 	}
+	if r != nil && r.Config != nil && r.Config.Username != "" && r.Config.Password != "" {
+		c.Options = append(c.Options, getter.WithBasicAuth(r.Config.Username, r.Config.Password))
+	}
 
-	if r != nil && r.Config != nil {
-		if r.Config.CertFile != "" || r.Config.KeyFile != "" || r.Config.CAFile != "" {
-			c.Options = append(c.Options, getter.WithTLSClientConfig(r.Config.CertFile, r.Config.KeyFile, r.Config.CAFile))
-		}
-		if r.Config.Username != "" && r.Config.Password != "" {
-			c.Options = append(c.Options, getter.WithBasicAuth(r.Config.Username, r.Config.Password))
-		}
+	if r.Config.CertFile != "" || r.Config.KeyFile != "" || r.Config.CAFile != "" {
+		c.Options = append(c.Options, getter.WithTLSClientConfig(r.Config.CertFile, r.Config.KeyFile, r.Config.CAFile))
 	}
 
 	// Next, we need to load the index, and actually look up the chart.
@@ -259,6 +255,9 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er
 		if _, err := getter.NewHTTPGetter(getter.WithURL(rc.URL)); err != nil {
 			return repoURL, err
 		}
+		if r != nil && r.Config != nil && r.Config.Username != "" && r.Config.Password != "" {
+			c.Options = append(c.Options, getter.WithBasicAuth(r.Config.Username, r.Config.Password))
+		}
 		return u, err
 	}
 


Test patch:
diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go
index abfb007ff..e0692c8c8 100644
--- a/pkg/downloader/chart_downloader_test.go
+++ b/pkg/downloader/chart_downloader_test.go
@@ -227,58 +227,6 @@ func TestDownloadTo(t *testing.T) {
 	}
 }
 
-func TestDownloadTo_TLS(t *testing.T) {
-	// Set up mock server w/ tls enabled
-	srv, err := repotest.NewTempServer("testdata/*.tgz*")
-	srv.Stop()
-	if err != nil {
-		t.Fatal(err)
-	}
-	srv.StartTLS()
-	defer srv.Stop()
-	if err := srv.CreateIndex(); err != nil {
-		t.Fatal(err)
-	}
-	if err := srv.LinkIndices(); err != nil {
-		t.Fatal(err)
-	}
-
-	repoConfig := filepath.Join(srv.Root(), "repositories.yaml")
-	repoCache := srv.Root()
-
-	c := ChartDownloader{
-		Out:              os.Stderr,
-		Verify:           VerifyAlways,
-		Keyring:          "testdata/helm-test-key.pub",
-		RepositoryConfig: repoConfig,
-		RepositoryCache:  repoCache,
-		Getters: getter.All(&cli.EnvSettings{
-			RepositoryConfig: repoConfig,
-			RepositoryCache:  repoCache,
-		}),
-		Options: []getter.Option{},
-	}
-	cname := "test/signtest"
-	dest := srv.Root()
-	where, v, err := c.DownloadTo(cname, "", dest)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	target := filepath.Join(dest, "signtest-0.1.0.tgz")
-	if expect := target; where != expect {
-		t.Errorf("Expected download to %s, got %s", expect, where)
-	}
-
-	if v.FileHash == "" {
-		t.Error("File hash was empty, but verification is required.")
-	}
-
-	if _, err := os.Stat(target); err != nil {
-		t.Error(err)
-	}
-}
-
 func TestDownloadTo_VerifyLater(t *testing.T) {
 	defer ensure.HelmHome(t)()
 
diff --git a/pkg/repo/repotest/server.go b/pkg/repo/repotest/server.go
index b18bce49c..96a8bbfcc 100644
--- a/pkg/repo/repotest/server.go
+++ b/pkg/repo/repotest/server.go
@@ -22,8 +22,6 @@ import (
 	"os"
 	"path/filepath"
 
-	"helm.sh/helm/v3/internal/tlsutil"
-
 	"sigs.k8s.io/yaml"
 
 	"helm.sh/helm/v3/pkg/repo"
@@ -145,40 +143,6 @@ func (s *Server) Start() {
 	}))
 }
 
-func (s *Server) StartTLS() {
-	cd := "../../testdata"
-	ca, pub, priv := filepath.Join(cd, "rootca.crt"), filepath.Join(cd, "crt.pem"), filepath.Join(cd, "key.pem")
-
-	s.srv = httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if s.middleware != nil {
-			s.middleware.ServeHTTP(w, r)
-		}
-		http.FileServer(http.Dir(s.Root())).ServeHTTP(w, r)
-	}))
-	tlsConf, err := tlsutil.NewClientTLS(pub, priv, ca)
-	if err != nil {
-		panic(err)
-	}
-	tlsConf.BuildNameToCertificate()
-	tlsConf.ServerName = "helm.sh"
-	s.srv.TLS = tlsConf
-	s.srv.StartTLS()
-
-	// Set up repositories config with ca file
-	repoConfig := filepath.Join(s.Root(), "repositories.yaml")
-
-	r := repo.NewFile()
-	r.Add(&repo.Entry{
-		Name:   "test",
-		URL:    s.URL(),
-		CAFile: filepath.Join("../../testdata", "rootca.crt"),
-	})
-
-	if err := r.WriteFile(repoConfig, 0644); err != nil {
-		panic(err)
-	}
-}
-
 // Stop stops the server and closes all connections.
 //
 // It should be called explicitly.