Repository: helm/helm
Pull request #8244 (2020-06-02 12:57:25)

Report what cause finding chart to fail
**What this PR does / why we need it**:
Currently, if finding a chart fails, the underlying error is discarded. That error can be non-obvious from the message produced (in my case a non-writeable cache directory)

**Special notes for your reviewer**:
-

**If applicable**:
- [ ] this PR contains documentation
- [ ] this PR contains unit tests
- [ ] this PR has been tested for backwards compatibility



Patch:
diff --git a/cmd/helm/show.go b/cmd/helm/show.go
index 91c35a53c..b335c5f76 100644
--- a/cmd/helm/show.go
+++ b/cmd/helm/show.go
@@ -136,8 +136,7 @@ func newShowCmd(out io.Writer) *cobra.Command {
 
 	cmds := []*cobra.Command{all, readmeSubCmd, valuesSubCmd, chartSubCmd}
 	for _, subCmd := range cmds {
-		addShowFlags(subCmd, client)
-		showCommand.AddCommand(subCmd)
+		addShowFlags(showCommand, subCmd, client)
 
 		// Register the completion function for each subcommand
 		completion.RegisterValidArgsFunc(subCmd, validArgsFunc)
@@ -146,11 +145,12 @@ func newShowCmd(out io.Writer) *cobra.Command {
 	return showCommand
 }
 
-func addShowFlags(subCmd *cobra.Command, client *action.Show) {
+func addShowFlags(showCmd *cobra.Command, subCmd *cobra.Command, client *action.Show) {
 	f := subCmd.Flags()
 
 	f.BoolVar(&client.Devel, "devel", false, "use development versions, too. Equivalent to version '>0.0.0-0'. If --version is set, this is ignored")
 	addChartPathOptionsFlags(f, &client.ChartPathOptions)
+	showCmd.AddCommand(subCmd)
 }
 
 func runShow(args []string, client *action.Show) (string, error) {
diff --git a/pkg/chartutil/save.go b/pkg/chartutil/save.go
index 2ce4eddaf..1011436b5 100644
--- a/pkg/chartutil/save.go
+++ b/pkg/chartutil/save.go
@@ -106,17 +106,12 @@ func Save(c *chart.Chart, outDir string) (string, error) {
 
 	filename := fmt.Sprintf("%s-%s.tgz", c.Name(), c.Metadata.Version)
 	filename = filepath.Join(outDir, filename)
-	dir := filepath.Dir(filename)
-	if stat, err := os.Stat(dir); err != nil {
-		if os.IsNotExist(err) {
-			if err2 := os.MkdirAll(dir, 0755); err2 != nil {
-				return "", err2
-			}
-		} else {
-			return "", errors.Wrapf(err, "stat %s", dir)
+	if stat, err := os.Stat(filepath.Dir(filename)); os.IsNotExist(err) {
+		if err := os.MkdirAll(filepath.Dir(filename), 0755); err != nil {
+			return "", err
 		}
 	} else if !stat.IsDir() {
-		return "", errors.Errorf("is not a directory: %s", dir)
+		return "", errors.Errorf("is not a directory: %s", filepath.Dir(filename))
 	}
 
 	f, err := os.Create(filename)
diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go
index 00198de0c..28a656e55 100644
--- a/pkg/downloader/manager.go
+++ b/pkg/downloader/manager.go
@@ -571,7 +571,7 @@ func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*
 	if err == nil {
 		return
 	}
-	err = errors.Errorf("chart %s not found in %s", name, repoURL)
+	err = errors.Errorf("chart %s not found in %s: %s", name, repoURL, err)
 	return
 }
 


Test patch:
diff --git a/internal/experimental/registry/client_test.go b/internal/experimental/registry/client_test.go
index 2d208b7b9..6e9d5db36 100644
--- a/internal/experimental/registry/client_test.go
+++ b/internal/experimental/registry/client_test.go
@@ -24,16 +24,11 @@ import (
 	"io/ioutil"
 	"net"
 	"net/http"
-	"net/http/httptest"
-	"net/url"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 	"time"
 
-	"github.com/containerd/containerd/errdefs"
-
 	auth "github.com/deislabs/oras/pkg/auth/docker"
 	"github.com/docker/distribution/configuration"
 	"github.com/docker/distribution/registry"
@@ -54,11 +49,10 @@ var (
 
 type RegistryClientTestSuite struct {
 	suite.Suite
-	Out                     io.Writer
-	DockerRegistryHost      string
-	CompromisedRegistryHost string
-	CacheRootDir            string
-	RegistryClient          *Client
+	Out                io.Writer
+	DockerRegistryHost string
+	CacheRootDir       string
+	RegistryClient     *Client
 }
 
 func (suite *RegistryClientTestSuite) SetupSuite() {
@@ -122,8 +116,6 @@ func (suite *RegistryClientTestSuite) SetupSuite() {
 	dockerRegistry, err := registry.NewRegistry(context.Background(), config)
 	suite.Nil(err, "no error creating test registry")
 
-	suite.CompromisedRegistryHost = initCompromisedRegistryTestServer()
-
 	// Start Docker registry
 	go dockerRegistry.ListenAndServe()
 }
@@ -240,16 +232,6 @@ func (suite *RegistryClientTestSuite) Test_7_Logout() {
 	suite.Nil(err, "no error logging out of registry")
 }
 
-func (suite *RegistryClientTestSuite) Test_8_ManInTheMiddle() {
-	ref, err := ParseReference(fmt.Sprintf("%s/testrepo/supposedlysafechart:9.9.9", suite.CompromisedRegistryHost))
-	suite.Nil(err)
-
-	// returns content that does not match the expected digest
-	err = suite.RegistryClient.PullChart(ref)
-	suite.NotNil(err)
-	suite.True(errdefs.IsFailedPrecondition(err))
-}
-
 func TestRegistryClientTestSuite(t *testing.T) {
 	suite.Run(t, new(RegistryClientTestSuite))
 }
@@ -268,43 +250,3 @@ func getFreePort() (int, error) {
 	defer l.Close()
 	return l.Addr().(*net.TCPAddr).Port, nil
 }
-
-func initCompromisedRegistryTestServer() string {
-	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if strings.Contains(r.URL.Path, "manifests") {
-			w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json")
-			w.WriteHeader(200)
-
-			// layers[0] is the blob []byte("a")
-			w.Write([]byte(
-				`{ "schemaVersion": 2, "config": {
-    "mediaType": "application/vnd.cncf.helm.config.v1+json",
-    "digest": "sha256:a705ee2789ab50a5ba20930f246dbd5cc01ff9712825bb98f57ee8414377f133",
-    "size": 181
-  },
-  "layers": [
-    {
-      "mediaType": "application/tar+gzip",
-      "digest": "sha256:ca978112ca1bbdcafac231b39a23dc4da786eff8147c4e72b9807785afee48bb",
-      "size": 1
-    }
-  ]
-}`))
-		} else if r.URL.Path == "/v2/testrepo/supposedlysafechart/blobs/sha256:a705ee2789ab50a5ba20930f246dbd5cc01ff9712825bb98f57ee8414377f133" {
-			w.Header().Set("Content-Type", "application/json")
-			w.WriteHeader(200)
-			w.Write([]byte("{\"name\":\"mychart\",\"version\":\"0.1.0\",\"description\":\"A Helm chart for Kubernetes\\n" +
-				"an 'application' or a 'library' chart.\",\"apiVersion\":\"v2\",\"appVersion\":\"1.16.0\",\"type\":" +
-				"\"application\"}"))
-		} else if r.URL.Path == "/v2/testrepo/supposedlysafechart/blobs/sha256:ca978112ca1bbdcafac231b39a23dc4da786eff8147c4e72b9807785afee48bb" {
-			w.Header().Set("Content-Type", "application/tar+gzip")
-			w.WriteHeader(200)
-			w.Write([]byte("b"))
-		} else {
-			w.WriteHeader(500)
-		}
-	}))
-
-	u, _ := url.Parse(s.URL)
-	return fmt.Sprintf("localhost:%s", u.Port())
-}