Repository: helm/helm (go)

Issue:
Support reading values from STDIN for `upgrade` command
#2709 introduced support for using STDIN with the "-" character for `install`. However, I don't think this works with the `upgrade` command. 

My example usecase is decrypting a sops file and using the output in the `helm upgrade` command:


```sh
# I can do this and values in secrets.sops.yaml are available to the release:
sops -d secrets.sops.yaml | helm install -f - -f values.yaml my-release release/my-chart 

# but this doesn't work:
sops -d secrets.sops.yaml | helm upgrade -f - -f values.yaml my-release release/my-chart 
```




Pull request:
fix(helm): stdin values for helm upgrade --install
Fixes #7002

The cause of this issue is the helm upgrade cmd parses the command line options (which reads from std in) before determining if it needs to call the helm install command (because of the missing release).  Then when helm install is called, it rereads the command line args and this time reads nothing from stdin.

There were never any test cases that tested using the "--values -"/"-f -" syntax to pass values via stdin. Because of this there was no test harness or way to achieve this. I updated helm_test.go to support passing stdin as a *os.File.

The actual fix was in cmd/helm/upgrade.go and involved changing the order of operations of the parsing of arguments and calling install if necessary.

I added test cases to actually test "helm upgrade --install" (there were none) in combination with using the stdin values and another for just "helm upgrade" with stdin values.

Signed-off-by: Matt Morrissette <yinzara@gmail.com>

Patch:
diff --git a/cmd/helm/root.go b/cmd/helm/root.go
index 3ebea3bae..3c6a0d18e 100644
--- a/cmd/helm/root.go
+++ b/cmd/helm/root.go
@@ -26,6 +26,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/tools/clientcmd"
 
+	"helm.sh/helm/v3/cmd/helm/require"
 	"helm.sh/helm/v3/internal/completion"
 	"helm.sh/helm/v3/internal/experimental/registry"
 	"helm.sh/helm/v3/pkg/action"
@@ -76,6 +77,7 @@ func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string
 		Short:                  "The Helm package manager for Kubernetes.",
 		Long:                   globalUsage,
 		SilenceUsage:           true,
+		Args:                   require.NoArgs,
 		BashCompletionFunction: completion.GetBashCustomFunction(),
 	}
 	flags := cmd.PersistentFlags()
diff --git a/cmd/helm/template.go b/cmd/helm/template.go
index 91a398429..320718344 100644
--- a/cmd/helm/template.go
+++ b/cmd/helm/template.go
@@ -24,6 +24,8 @@ import (
 	"regexp"
 	"strings"
 
+	"helm.sh/helm/v3/pkg/releaseutil"
+
 	"github.com/spf13/cobra"
 
 	"helm.sh/helm/v3/cmd/helm/require"
@@ -31,7 +33,6 @@ import (
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/chartutil"
 	"helm.sh/helm/v3/pkg/cli/values"
-	"helm.sh/helm/v3/pkg/releaseutil"
 )
 
 const templateDesc = `
@@ -52,7 +53,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 
 	cmd := &cobra.Command{
 		Use:   "template [NAME] [CHART]",
-		Short: "locally render templates",
+		Short: fmt.Sprintf("locally render templates"),
 		Long:  templateDesc,
 		Args:  require.MinimumNArgs(1),
 		RunE: func(_ *cobra.Command, args []string) error {
diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go
index dea866e4d..dd5a6c478 100644
--- a/cmd/helm/upgrade.go
+++ b/cmd/helm/upgrade.go
@@ -75,21 +75,8 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 		RunE: func(cmd *cobra.Command, args []string) error {
 			client.Namespace = settings.Namespace()
 
-			if client.Version == "" && client.Devel {
-				debug("setting version to >0.0.0-0")
-				client.Version = ">0.0.0-0"
-			}
-
-			vals, err := valueOpts.MergeValues(getter.All(settings))
-			if err != nil {
-				return err
-			}
-
-			chartPath, err := client.ChartPathOptions.LocateChart(args[1], settings)
-			if err != nil {
-				return err
-			}
-
+			// Fixes #7002 - Support reading values from STDIN for `upgrade` command
+			// Must load values AFTER determining if we have to call install so that values loaded from stdin are are not read twice
 			if client.Install {
 				// If a release does not exist, install it. If another error occurs during
 				// the check, ignore the error and continue with the upgrade.
@@ -121,6 +108,21 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 				}
 			}
 
+			if client.Version == "" && client.Devel {
+				debug("setting version to >0.0.0-0")
+				client.Version = ">0.0.0-0"
+			}
+
+			chartPath, err := client.ChartPathOptions.LocateChart(args[1], settings)
+			if err != nil {
+				return err
+			}
+
+			vals, err := valueOpts.MergeValues(getter.All(settings))
+			if err != nil {
+				return err
+			}
+
 			// Check chart dependencies to make sure all are present in /charts
 			ch, err := loader.Load(chartPath)
 			if err != nil {
diff --git a/internal/completion/complete.go b/internal/completion/complete.go
index eaea4e914..c8f78868a 100644
--- a/internal/completion/complete.go
+++ b/internal/completion/complete.go
@@ -259,7 +259,7 @@ func NewCompleteCmd(settings *cli.EnvSettings, out io.Writer) *cobra.Command {
 			// completion script to parse.
 			// The directive integer must be that last character following a single :
 			// The completion script expects :directive
-			fmt.Fprintf(out, ":%d\n", directive)
+			fmt.Fprintln(out, fmt.Sprintf(":%d", directive))
 
 			// Print some helpful info to stderr for the user to understand.
 			// Output from stderr should be ignored from the completion script.
diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go
index 942c9d8af..81812983f 100644
--- a/pkg/action/rollback.go
+++ b/pkg/action/rollback.go
@@ -211,7 +211,7 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas
 	}
 
 	deployed, err := r.cfg.Releases.DeployedAll(currentRelease.Name)
-	if err != nil {
+	if err != nil && !strings.Contains(err.Error(), "has no deployed releases") {
 		return nil, err
 	}
 	// Supersede all previous deployments, see issue #2941.
diff --git a/pkg/storage/driver/memory.go b/pkg/storage/driver/memory.go
index a99b36ef0..91378f588 100644
--- a/pkg/storage/driver/memory.go
+++ b/pkg/storage/driver/memory.go
@@ -141,6 +141,11 @@ func (mem *Memory) Query(keyvals map[string]string) ([]*rspb.Release, error) {
 			break
 		}
 	}
+
+	if len(ls) == 0 {
+		return nil, ErrReleaseNotFound
+	}
+
 	return ls, nil
 }
 


Test patch:
diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go
index 94646a5a3..9ba9d78fb 100644
--- a/cmd/helm/helm_test.go
+++ b/cmd/helm/helm_test.go
@@ -97,10 +97,15 @@ func storageFixture() *storage.Storage {
 }
 
 func executeActionCommandC(store *storage.Storage, cmd string) (*cobra.Command, string, error) {
+	return executeActionCommandStdinC(store, nil, cmd)
+}
+
+func executeActionCommandStdinC(store *storage.Storage, in *os.File, cmd string) (*cobra.Command, string, error) {
 	args, err := shellwords.Parse(cmd)
 	if err != nil {
 		return nil, "", err
 	}
+
 	buf := new(bytes.Buffer)
 
 	actionConfig := &action.Configuration{
@@ -111,15 +116,26 @@ func executeActionCommandC(store *storage.Storage, cmd string) (*cobra.Command,
 	}
 
 	root := newRootCmd(actionConfig, buf, args)
-	root.SetOutput(buf)
+	root.SetOut(buf)
+	root.SetErr(buf)
 	root.SetArgs(args)
 
+	oldStdin := os.Stdin
+	if in != nil {
+		root.SetIn(in)
+		os.Stdin = in
+	}
+
 	if mem, ok := store.Driver.(*driver.Memory); ok {
 		mem.SetNamespace(settings.Namespace())
 	}
 	c, err := root.ExecuteC()
 
-	return c, buf.String(), err
+	result := buf.String()
+
+	os.Stdin = oldStdin
+
+	return c, result, err
 }
 
 // cmdTestCase describes a test case that works with releases.
diff --git a/cmd/helm/root_test.go b/cmd/helm/root_test.go
index e1fa1fc27..df592a96d 100644
--- a/cmd/helm/root_test.go
+++ b/cmd/helm/root_test.go
@@ -95,11 +95,3 @@ func TestRootCmd(t *testing.T) {
 		})
 	}
 }
-
-func TestUnknownSubCmd(t *testing.T) {
-	_, _, err := executeActionCommand("foobar")
-
-	if err == nil || err.Error() != `unknown command "foobar" for "helm"` {
-		t.Errorf("Expect unknown command error, got %q", err)
-	}
-}
diff --git a/cmd/helm/testdata/testcharts/alpine/README.md b/cmd/helm/testdata/testcharts/alpine/README.md
index 05d39dbbc..fcf7ee017 100644
--- a/cmd/helm/testdata/testcharts/alpine/README.md
+++ b/cmd/helm/testdata/testcharts/alpine/README.md
@@ -1,4 +1,4 @@
-# Alpine: A simple Helm chart
+#Alpine: A simple Helm chart
 
 Run a single pod of Alpine Linux.
 
diff --git a/cmd/helm/upgrade_test.go b/cmd/helm/upgrade_test.go
index 3cecbe6d3..1e413811a 100644
--- a/cmd/helm/upgrade_test.go
+++ b/cmd/helm/upgrade_test.go
@@ -19,6 +19,7 @@ package main
 import (
 	"fmt"
 	"io/ioutil"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -224,6 +225,69 @@ func TestUpgradeWithValuesFile(t *testing.T) {
 
 }
 
+func TestUpgradeWithValuesFromStdin(t *testing.T) {
+
+	releaseName := "funny-bunny-v5"
+	relMock, ch, chartPath := prepareMockRelease(releaseName, t)
+
+	defer resetEnv()()
+
+	store := storageFixture()
+
+	store.Create(relMock(releaseName, 3, ch))
+
+	in, err := os.Open("testdata/testcharts/upgradetest/values.yaml")
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	cmd := fmt.Sprintf("upgrade %s --values - '%s'", releaseName, chartPath)
+	_, _, err = executeActionCommandStdinC(store, in, cmd)
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	updatedRel, err := store.Get(releaseName, 4)
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	if !strings.Contains(updatedRel.Manifest, "drink: beer") {
+		t.Errorf("The value is not set correctly. manifest: %s", updatedRel.Manifest)
+	}
+}
+
+func TestUpgradeInstallWithValuesFromStdin(t *testing.T) {
+
+	releaseName := "funny-bunny-v6"
+	_, _, chartPath := prepareMockRelease(releaseName, t)
+
+	defer resetEnv()()
+
+	store := storageFixture()
+
+	in, err := os.Open("testdata/testcharts/upgradetest/values.yaml")
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	cmd := fmt.Sprintf("upgrade %s -f - --install '%s'", releaseName, chartPath)
+	_, _, err = executeActionCommandStdinC(store, in, cmd)
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	updatedRel, err := store.Get(releaseName, 1)
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	if !strings.Contains(updatedRel.Manifest, "drink: beer") {
+		t.Errorf("The value is not set correctly. manifest: %s", updatedRel.Manifest)
+	}
+
+}
+
 func prepareMockRelease(releaseName string, t *testing.T) (func(n string, v int, ch *chart.Chart) *release.Release, *chart.Chart, string) {
 	tmpChart := ensure.TempDir(t)
 	configmapData, err := ioutil.ReadFile("testdata/testcharts/upgradetest/templates/configmap.yaml")