Feat: add --strip-args-values-on-exit-error (#887)

* Add --strip-args-values-on-exit-error

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

cmd/root.go

@@ -120,6 +120,7 @@ func setGlobalOptionsForRootCmd(fs *pflag.FlagSet, globalOptions *config.GlobalO
 	fs.StringArrayVar(&globalOptions.StateValuesSet, "state-values-set", nil, "set state values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2). Used to override .Values within the helmfile template (not values template).")
 	fs.StringArrayVar(&globalOptions.StateValuesFile, "state-values-file", nil, "specify state values in a YAML file. Used to override .Values within the helmfile template (not values template).")
 	fs.BoolVar(&globalOptions.SkipDeps, "skip-deps", false, `skip running "helm repo update" and "helm dependency build"`)
+	fs.BoolVar(&globalOptions.StripArgsValuesOnExitError, "strip-args-values-on-exit-error", true, `Strip the potential secret values of the helm command args contained in a helmfile error message`)
 	fs.BoolVar(&globalOptions.DisableForceUpdate, "disable-force-update", false, `do not force helm repos to update when executing "helm repo add"`)
 	fs.BoolVarP(&globalOptions.Quiet, "quiet", "q", false, "Silence output. Equivalent to log-level warn")
 	fs.StringVar(&globalOptions.KubeContext, "kube-context", "", "Set kubectl context. Uses current context by default")

docs/index.md

@@ -210,7 +210,7 @@ helmDefaults:
   # propagate `--post-renderer` to helmv3 template and helm install
   postRenderer: "path/to/postRenderer"
   #	cascade `--cascade` to helmv3 delete, available values: background, foreground, or orphan, default: background
-  cascade: "background" 
+  cascade: "background"
   # insecureSkipTLSVerify is true if the TLS verification should be skipped when fetching remote chart
   insecureSkipTLSVerify: false
 
@@ -312,7 +312,7 @@ releases:
     # propagate `--post-renderer` to helmv3 template and helm install
     postRenderer: "path/to/postRenderer"
     # cascade `--cascade` to helmv3 delete, available values: background, foreground, or orphan, default: background
-    cascade: "background" 
+    cascade: "background"
     # insecureSkipTLSVerify is true if the TLS verification should be skipped when fetching remote chart
     insecureSkipTLSVerify: false
 
@@ -539,31 +539,32 @@ Available Commands:
   write-values Write values files for releases. Similar to `helmfile template`, write values files instead of manifests.
 
 Flags:
-      --allow-no-matching-release       Do not exit with an error code if the provided selector has no matching releases.
+      --allow-no-matching-release         Do not exit with an error code if the provided selector has no matching releases.
-  -c, --chart string                    Set chart. Uses the chart set in release by default, and is available in template as {{ .Chart }}
+  -c, --chart string                      Set chart. Uses the chart set in release by default, and is available in template as {{ .Chart }}
-      --color                           Output with color
+      --color                             Output with color
-      --debug                           Enable verbose output for Helm and set log-level to debug, this disables --quiet/-q effect
+      --debug                             Enable verbose output for Helm and set log-level to debug, this disables --quiet/-q effect
-      --disable-force-update            do not force helm repos to update when executing "helm repo add"
+      --disable-force-update              do not force helm repos to update when executing "helm repo add"
-      --enable-live-output              Show live output from the Helm binary Stdout/Stderr into Helmfile own Stdout/Stderr.
+      --enable-live-output                Show live output from the Helm binary Stdout/Stderr into Helmfile own Stdout/Stderr.
-                                        It only applies for the Helm CLI commands, Stdout/Stderr for Hooks are still displayed only when it's execution finishes.
+                                          It only applies for the Helm CLI commands, Stdout/Stderr for Hooks are still displayed only when it's execution finishes.
-  -e, --environment string              specify the environment name. Overrides "HELMFILE_ENVIRONMENT" OS environment variable when specified. defaults to "default"
+  -e, --environment string                specify the environment name. Overrides "HELMFILE_ENVIRONMENT" OS environment variable when specified. defaults to "default"
-  -f, --file helmfile.yaml              load config from file or directory. defaults to "helmfile.yaml" or "helmfile.yaml.gotmpl" or "helmfile.d" (means "helmfile.d/*.yaml" or "helmfile.d/*.yaml.gotmpl") in this preference. Specify - to load the config from the standard input.
+  -f, --file helmfile.yaml                load config from file or directory. defaults to "helmfile.yaml" or "helmfile.yaml.gotmpl" or "helmfile.d" (means "helmfile.d/*.yaml" or "helmfile.d/*.yaml.gotmpl") in this preference. Specify - to load the config from the standard input.
-  -b, --helm-binary string              Path to the helm binary (default "helm")
+  -b, --helm-binary string                Path to the helm binary (default "helm")
-  -h, --help                            help for helmfile
+  -h, --help                              help for helmfile
-  -i, --interactive                     Request confirmation before attempting to modify clusters
+  -i, --interactive                       Request confirmation before attempting to modify clusters
-      --kube-context string             Set kubectl context. Uses current context by default
+      --kube-context string               Set kubectl context. Uses current context by default
-      --log-level string                Set log level, default info (default "info")
+      --log-level string                  Set log level, default info (default "info")
-  -n, --namespace string                Set namespace. Uses the namespace set in the context by default, and is available in templates as {{ .Namespace }}
+  -n, --namespace string                  Set namespace. Uses the namespace set in the context by default, and is available in templates as {{ .Namespace }}
-      --no-color                        Output without color
+      --no-color                          Output without color
-  -q, --quiet                           Silence output. Equivalent to log-level warn
+  -q, --quiet                             Silence output. Equivalent to log-level warn
-  -l, --selector stringArray            Only run using the releases that match labels. Labels can take the form of foo=bar or foo!=bar.
+  -l, --selector stringArray              Only run using the releases that match labels. Labels can take the form of foo=bar or foo!=bar.
-                                        A release must match all labels in a group in order to be used. Multiple groups can be specified at once.
+                                          A release must match all labels in a group in order to be used. Multiple groups can be specified at once.
-                                        "--selector tier=frontend,tier!=proxy --selector tier=backend" will match all frontend, non-proxy releases AND all backend releases.
+                                          "--selector tier=frontend,tier!=proxy --selector tier=backend" will match all frontend, non-proxy releases AND all backend releases.
-                                        The name of a release can be used as a label: "--selector name=myrelease"
+                                          The name of a release can be used as a label: "--selector name=myrelease"
-      --skip-deps                       skip running "helm repo update" and "helm dependency build"
+      --skip-deps                         skip running "helm repo update" and "helm dependency build"
-      --state-values-file stringArray   specify state values in a YAML file. Used to override .Values within the helmfile template (not values template).
+      --state-values-file stringArray     specify state values in a YAML file. Used to override .Values within the helmfile template (not values template).
-      --state-values-set stringArray    set state values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2). Used to override .Values within the helmfile template (not values template).
+      --state-values-set stringArray      set state values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2). Used to override .Values within the helmfile template (not values template).
-  -v, --version                         version for helmfile
+      --strip-args-values-on-exit-error   On exit error, strip the values of the args
+  -v, --version                           version for helmfile
 
 Use "helmfile [command] --help" for more information about a command.
 ```

pkg/app/app.go

@@ -28,10 +28,11 @@ var Cancel goContext.CancelFunc
 
 // App is the main application object.
 type App struct {
-	OverrideKubeContext string
+	OverrideKubeContext        string
-	OverrideHelmBinary  string
+	OverrideHelmBinary         string
-	EnableLiveOutput    bool
+	EnableLiveOutput           bool
-	DisableForceUpdate  bool
+	StripArgsValuesOnExitError bool
+	DisableForceUpdate         bool
 
 	Logger      *zap.SugaredLogger
 	Env         string
@@ -71,21 +72,22 @@ func New(conf ConfigProvider) *App {
 	ctx, Cancel = goContext.WithCancel(ctx)
 
 	return Init(&App{
-		OverrideKubeContext: conf.KubeContext(),
+		OverrideKubeContext:        conf.KubeContext(),
-		OverrideHelmBinary:  conf.HelmBinary(),
+		OverrideHelmBinary:         conf.HelmBinary(),
-		EnableLiveOutput:    conf.EnableLiveOutput(),
+		EnableLiveOutput:           conf.EnableLiveOutput(),
-		DisableForceUpdate:  conf.DisableForceUpdate(),
+		StripArgsValuesOnExitError: conf.StripArgsValuesOnExitError(),
-		Logger:              conf.Logger(),
+		DisableForceUpdate:         conf.DisableForceUpdate(),
-		Env:                 conf.Env(),
+		Logger:                     conf.Logger(),
-		Namespace:           conf.Namespace(),
+		Env:                        conf.Env(),
-		Chart:               conf.Chart(),
+		Namespace:                  conf.Namespace(),
-		Selectors:           conf.Selectors(),
+		Chart:                      conf.Chart(),
-		Args:                conf.Args(),
+		Selectors:                  conf.Selectors(),
-		FileOrDir:           conf.FileOrDir(),
+		Args:                       conf.Args(),
-		ValuesFiles:         conf.StateValuesFiles(),
+		FileOrDir:                  conf.FileOrDir(),
-		Set:                 conf.StateValuesSet(),
+		ValuesFiles:                conf.StateValuesFiles(),
-		fs:                  filesystem.DefaultFileSystem(),
+		Set:                        conf.StateValuesSet(),
-		ctx:                 ctx,
+		fs:                         filesystem.DefaultFileSystem(),
+		ctx:                        ctx,
 	})
 }
 
@@ -105,8 +107,9 @@ func Init(app *App) *App {
 
 func (a *App) Init(c InitConfigProvider) error {
 	runner := &helmexec.ShellRunner{
-		Logger: a.Logger,
+		Logger:                     a.Logger,
-		Ctx:    a.ctx,
+		Ctx:                        a.ctx,
+		StripArgsValuesOnExitError: a.StripArgsValuesOnExitError,
 	}
 	helmfileInit := NewHelmfileInit(a.OverrideHelmBinary, c, a.Logger, runner)
 	return helmfileInit.Initialize()
@@ -797,8 +800,9 @@ func (a *App) getHelm(st *state.HelmState) helmexec.Interface {
 
 	if _, ok := a.helms[key]; !ok {
 		a.helms[key] = helmexec.New(bin, helmexec.HelmExecOptions{EnableLiveOutput: a.EnableLiveOutput, DisableForceUpdate: a.DisableForceUpdate}, a.Logger, kubectx, &helmexec.ShellRunner{
-			Logger: a.Logger,
+			Logger:                     a.Logger,
-			Ctx:    a.ctx,
+			Ctx:                        a.ctx,
+			StripArgsValuesOnExitError: a.StripArgsValuesOnExitError,
 		})
 	}
 

pkg/app/config.go

@@ -6,6 +6,7 @@ type ConfigProvider interface {
 	Args() string
 	HelmBinary() string
 	EnableLiveOutput() bool
+	StripArgsValuesOnExitError() bool
 	DisableForceUpdate() bool
 	SkipDeps() bool
 

pkg/config/global.go

@@ -25,6 +25,8 @@ type GlobalOptions struct {
 	StateValuesFile []string
 	// SkipDeps is true if the running "helm repo update" and "helm dependency build" should be skipped
 	SkipDeps bool
+	// StripArgsValuesOnExitError is true if the ARGS output on exit error should be suppressed
+	StripArgsValuesOnExitError bool
 	// DisableForceUpdate is true if force updating repos is not desirable when executing "helm repo add"
 	DisableForceUpdate bool
 	// Quiet is true if the output should be quiet.
@@ -141,6 +143,11 @@ func (g *GlobalImpl) SkipDeps() bool {
 	return g.GlobalOptions.SkipDeps
 }
 
+// StripArgsValuesOnExitError return if the ARGS output on exit error should be suppressed
+func (g *GlobalImpl) StripArgsValuesOnExitError() bool {
+	return g.GlobalOptions.StripArgsValuesOnExitError
+}
+
 // DisableForceUpdate return when to disable forcing updates to repos upon adding
 func (g *GlobalImpl) DisableForceUpdate() bool {
 	return g.GlobalOptions.DisableForceUpdate

pkg/helmexec/exit_error.go

@@ -5,13 +5,16 @@ import (
 	"strings"
 )
 
-func newExitError(path string, args []string, exitStatus int, err error, stderr, combined string) ExitError {
+func newExitError(path string, args []string, exitStatus int, err error, stderr, combined string, stripArgsValuesOnExitError bool) ExitError {
 	var out string
 
 	out += fmt.Sprintf("PATH:\n%s", Indent(path, "  "))
 
 	out += "\n\nARGS:"
 	for i, a := range args {
+		if i > 0 && strings.HasPrefix(args[i-1], "--set") && stripArgsValuesOnExitError {
+			a = "*** STRIP ***"
+		}
 		out += fmt.Sprintf("\n%s", Indent(fmt.Sprintf("%d: %s (%d bytes)", i, a, len(a)), "  "))
 	}
 

pkg/helmexec/exit_error_test.go

@@ -0,0 +1,62 @@
+package helmexec
+
+import (
+	"errors"
+	"testing"
+)
+
+func TestNewExitError(t *testing.T) {
+	for _, tt := range []struct {
+		name                       string
+		stripArgsValuesOnExitError bool
+		want                       string
+	}{
+		{
+			name:                       "newExitError with stripArgsValuesOnExitError false",
+			stripArgsValuesOnExitError: false,
+			want: `command "helm" exited with non-zero status:
+
+PATH:
+  helm
+
+ARGS:
+  0: --set (5 bytes)
+  1: a=b (3 bytes)
+  2: --set-string (12 bytes)
+  3: a=b (3 bytes)
+
+ERROR:
+  test
+
+EXIT STATUS
+  1`,
+		},
+		{
+			name:                       "newExitError with stripArgsValuesOnExitError true",
+			stripArgsValuesOnExitError: true,
+			want: `command "helm" exited with non-zero status:
+
+PATH:
+  helm
+
+ARGS:
+  0: --set (5 bytes)
+  1: *** STRIP *** (13 bytes)
+  2: --set-string (12 bytes)
+  3: *** STRIP *** (13 bytes)
+
+ERROR:
+  test
+
+EXIT STATUS
+  1`,
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			exitError := newExitError("helm", []string{"--set", "a=b", "--set-string", "a=b"}, 1, errors.New("test"), "", "", tt.stripArgsValuesOnExitError)
+			if want, have := tt.want, exitError.Error(); want != have {
+				t.Errorf("want %q, have %q", want, have)
+			}
+		})
+	}
+}

pkg/helmexec/runner.go

@@ -28,6 +28,8 @@ type Runner interface {
 type ShellRunner struct {
 	Dir string
 
+	StripArgsValuesOnExitError bool
+
 	Logger *zap.SugaredLogger
 	Ctx    context.Context
 }
@@ -39,11 +41,11 @@ func (shell ShellRunner) Execute(cmd string, args []string, env map[string]strin
 	preparedCmd.Env = mergeEnv(os.Environ(), env)
 
 	if !enableLiveOutput {
-		return Output(shell.Ctx, preparedCmd, &logWriterGenerator{
+		return Output(shell.Ctx, preparedCmd, shell.StripArgsValuesOnExitError, &logWriterGenerator{
 			log: shell.Logger,
 		})
 	} else {
-		return LiveOutput(shell.Ctx, preparedCmd, os.Stdout)
+		return LiveOutput(shell.Ctx, preparedCmd, shell.StripArgsValuesOnExitError, os.Stdout)
 	}
 }
 
@@ -53,12 +55,12 @@ func (shell ShellRunner) ExecuteStdIn(cmd string, args []string, env map[string]
 	preparedCmd.Dir = shell.Dir
 	preparedCmd.Env = mergeEnv(os.Environ(), env)
 	preparedCmd.Stdin = stdin
-	return Output(shell.Ctx, preparedCmd, &logWriterGenerator{
+	return Output(shell.Ctx, preparedCmd, shell.StripArgsValuesOnExitError, &logWriterGenerator{
 		log: shell.Logger,
 	})
 }
 
-func Output(ctx context.Context, c *exec.Cmd, logWriterGenerators ...*logWriterGenerator) ([]byte, error) {
+func Output(ctx context.Context, c *exec.Cmd, stripArgsValuesOnExitError bool, logWriterGenerators ...*logWriterGenerator) ([]byte, error) {
 	if c.Stdout != nil {
 		return nil, errors.New("exec: Stdout already set")
 	}
@@ -114,7 +116,7 @@ func Output(ctx context.Context, c *exec.Cmd, logWriterGenerators ...*logWriterG
 			// so that helmfile could return its own exit code accordingly
 			waitStatus := ee.Sys().(syscall.WaitStatus)
 			exitStatus := waitStatus.ExitStatus()
-			err = newExitError(c.Path, c.Args, exitStatus, ee, stderr.String(), combined.String())
+			err = newExitError(c.Path, c.Args, exitStatus, ee, stderr.String(), combined.String(), stripArgsValuesOnExitError)
 		default:
 			panic(fmt.Sprintf("unexpected error: %v", err))
 		}
@@ -123,7 +125,7 @@ func Output(ctx context.Context, c *exec.Cmd, logWriterGenerators ...*logWriterG
 	return stdout.Bytes(), err
 }
 
-func LiveOutput(ctx context.Context, c *exec.Cmd, stdout io.Writer) ([]byte, error) {
+func LiveOutput(ctx context.Context, c *exec.Cmd, stripArgsValuesOnExitError bool, stdout io.Writer) ([]byte, error) {
 	reader, writer := io.Pipe()
 	scannerStopped := make(chan struct{})
 
@@ -162,7 +164,7 @@ func LiveOutput(ctx context.Context, c *exec.Cmd, stdout io.Writer) ([]byte, err
 			// so that helmfile could return its own exit code accordingly
 			waitStatus := ee.Sys().(syscall.WaitStatus)
 			exitStatus := waitStatus.ExitStatus()
-			err = newExitError(c.Path, c.Args, exitStatus, ee, "", "")
+			err = newExitError(c.Path, c.Args, exitStatus, ee, "", "", stripArgsValuesOnExitError)
 		default:
 			panic(fmt.Sprintf("unexpected error: %v", err))
 		}

pkg/helmexec/runner_test.go

@@ -74,7 +74,7 @@ Usage:  helm template [NAME] [CHART] [flags]
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			w := &bytes.Buffer{}
-			got, err := LiveOutput(context.Background(), tt.cmd, w)
+			got, err := LiveOutput(context.Background(), tt.cmd, false, w)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("LiveOutput() error = %v, wantErr %v", err, tt.wantErr)
 				return

pkg/tmpl/context_funcs.go

@@ -175,7 +175,7 @@ func (c *Context) EnvExec(envs map[string]interface{}, command string, args []in
 	g.Go(func() error {
 		// We use CombinedOutput to produce helpful error messages
 		// See https://github.com/roboll/helmfile/issues/1158
-		bs, err := helmexec.Output(context.Background(), cmd)
+		bs, err := helmexec.Output(context.Background(), cmd, false)
 		if err != nil {
 			return err
 		}