From 99f277a2f3de395160b473ac7a8cae6c380b6d1c Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Fri, 22 May 2020 09:51:48 -0400 Subject: [PATCH 1/6] Using flags instead of persistent flags on status Persistent flags are available on subcommands. Status does not need the persistent nature. Closes #8149 Signed-off-by: Matt Farina --- cmd/helm/status.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/helm/status.go b/cmd/helm/status.go index 6313b3975..2efb2006c 100644 --- a/cmd/helm/status.go +++ b/cmd/helm/status.go @@ -74,7 +74,7 @@ func newStatusCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return compListReleases(toComplete, cfg) }) - f := cmd.PersistentFlags() + f := cmd.Flags() f.IntVar(&client.Version, "revision", 0, "if set, display the status of the named release with revision") flag := f.Lookup("revision") From f182ebc11c9ea7a5bf75e35d8e061f3ac68482fd Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Fri, 22 May 2020 12:04:37 -0400 Subject: [PATCH 2/6] Fix issue with unhandled error on Stat If stat returns an error other than the directory not existing it was unhandled. When IsDir is called in one of these situations it causes a panic. Closes #8181 Signed-off-by: Matt Farina --- pkg/chartutil/save.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/pkg/chartutil/save.go b/pkg/chartutil/save.go index 1011436b5..2ce4eddaf 100644 --- a/pkg/chartutil/save.go +++ b/pkg/chartutil/save.go @@ -106,12 +106,17 @@ 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) - if stat, err := os.Stat(filepath.Dir(filename)); os.IsNotExist(err) { - if err := os.MkdirAll(filepath.Dir(filename), 0755); err != nil { - return "", err + 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) } } else if !stat.IsDir() { - return "", errors.Errorf("is not a directory: %s", filepath.Dir(filename)) + return "", errors.Errorf("is not a directory: %s", dir) } f, err := os.Create(filename) From 7dec5dcb8e2857dd14d0ea43f1c68876b5ac8a2d Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Fri, 29 May 2020 15:54:42 -0400 Subject: [PATCH 3/6] chore(helm): Avoid confusion in command usage Having both the `showCmd` and the `subCmd` passed to `addShowFlags()` can easily lead to mistakes in using the wrong command. Instead, `addShowFlags()` should only focus on the `subCmd` Signed-off-by: Marc Khouzam --- cmd/helm/show.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/helm/show.go b/cmd/helm/show.go index b335c5f76..91c35a53c 100644 --- a/cmd/helm/show.go +++ b/cmd/helm/show.go @@ -136,7 +136,8 @@ func newShowCmd(out io.Writer) *cobra.Command { cmds := []*cobra.Command{all, readmeSubCmd, valuesSubCmd, chartSubCmd} for _, subCmd := range cmds { - addShowFlags(showCommand, subCmd, client) + addShowFlags(subCmd, client) + showCommand.AddCommand(subCmd) // Register the completion function for each subcommand completion.RegisterValidArgsFunc(subCmd, validArgsFunc) @@ -145,12 +146,11 @@ func newShowCmd(out io.Writer) *cobra.Command { return showCommand } -func addShowFlags(showCmd *cobra.Command, subCmd *cobra.Command, client *action.Show) { +func addShowFlags(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) { From f11e74ee571f6a21ea5fd647513324d7268ff8d3 Mon Sep 17 00:00:00 2001 From: Peter Engelbert Date: Wed, 3 Jun 2020 15:23:26 -0500 Subject: [PATCH 4/6] Add unit test for man-in-the-middle attack on pull Add a unit test that proves the digest of the received content being checked. The check should ensure that the digest of the received content is identical to the digest provided by the manifest in the layers[0] descriptor. This check is currently implemented in containerd, so the unit test ensures security in the case a breaking change is made in containerd. Signed-off-by: Peter Engelbert --- internal/experimental/registry/client_test.go | 66 +++++++++++++++++-- 1 file changed, 62 insertions(+), 4 deletions(-) diff --git a/internal/experimental/registry/client_test.go b/internal/experimental/registry/client_test.go index 6e9d5db36..2d208b7b9 100644 --- a/internal/experimental/registry/client_test.go +++ b/internal/experimental/registry/client_test.go @@ -24,11 +24,16 @@ 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" @@ -49,10 +54,11 @@ var ( type RegistryClientTestSuite struct { suite.Suite - Out io.Writer - DockerRegistryHost string - CacheRootDir string - RegistryClient *Client + Out io.Writer + DockerRegistryHost string + CompromisedRegistryHost string + CacheRootDir string + RegistryClient *Client } func (suite *RegistryClientTestSuite) SetupSuite() { @@ -116,6 +122,8 @@ 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() } @@ -232,6 +240,16 @@ 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)) } @@ -250,3 +268,43 @@ 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()) +} From 562767b04015546e53677bb3c8aa5cae12897fa3 Mon Sep 17 00:00:00 2001 From: wawa0210 Date: Tue, 9 Jun 2020 17:49:02 +0800 Subject: [PATCH 5/6] Fix description is ignore when installed with upgrade Signed-off-by: wawa0210 --- cmd/helm/upgrade.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index c263d32e7..f8103b485 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -100,6 +100,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { instClient.PostRenderer = client.PostRenderer instClient.DisableOpenAPIValidation = client.DisableOpenAPIValidation instClient.SubNotes = client.SubNotes + instClient.Description = client.Description rel, err := runInstall(args, instClient, valueOpts, out) if err != nil { From e09e8604e6160775890c154b1fb8b6683a8bbb6f Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Fri, 29 May 2020 14:58:52 -0400 Subject: [PATCH 6/6] fix(doc): generic description for --version/verify The flags added by addChartPathOptionsFlags() are used for `helm install` and `helm upgrade` but also for `helm template`, `helm show` and `helm pull`. Because of the latter three, we should not use the word 'install' in the description of the --version and --verify flags. Signed-off-by: Marc Khouzam --- cmd/helm/flags.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go index 246cb0dd5..6b9c3050b 100644 --- a/cmd/helm/flags.go +++ b/cmd/helm/flags.go @@ -41,8 +41,8 @@ func addValueOptionsFlags(f *pflag.FlagSet, v *values.Options) { } func addChartPathOptionsFlags(f *pflag.FlagSet, c *action.ChartPathOptions) { - f.StringVar(&c.Version, "version", "", "specify the exact chart version to install. If this is not specified, the latest version is installed") - f.BoolVar(&c.Verify, "verify", false, "verify the package before installing it") + f.StringVar(&c.Version, "version", "", "specify the exact chart version to use. If this is not specified, the latest version is used") + f.BoolVar(&c.Verify, "verify", false, "verify the package before using it") f.StringVar(&c.Keyring, "keyring", defaultKeyring(), "location of public keys used for verification") f.StringVar(&c.RepoURL, "repo", "", "chart repository url where to locate the requested chart") f.StringVar(&c.Username, "username", "", "chart repository username where to locate the requested chart")