From 3144cf0fed84af00475620c77d184c306ffc6687 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Thu, 10 Oct 2019 18:55:48 +0900 Subject: [PATCH 1/3] fix(v3): fix regression on non-zero plugin exist status Fixes the regression in helm-3.0.0-beta.5 that swallows and rounds any non-zero exit status greater than 1 from a helm plugin to `1`. This, for example, breaks `helm-diff` which relies on helm able to return `2` when `helm-diff` returned `2`. Closese #6788 Signed-off-by: Yusuke Kuoka --- cmd/helm/helm.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index 0e935addb..b30d6b011 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -68,6 +68,11 @@ func main() { cmd := newRootCmd(actionConfig, os.Stdout, os.Args[1:]) if err := actionConfig.Init(settings, false, os.Getenv("HELM_DRIVER"), debug); err != nil { + debug("%+v", err) + os.Exit(1) + } + + if err := cmd.Execute(); err != nil { debug("%+v", err) switch e := err.(type) { case pluginError: @@ -76,11 +81,6 @@ func main() { os.Exit(1) } } - - if err := cmd.Execute(); err != nil { - debug("%+v", err) - os.Exit(1) - } } // wordSepNormalizeFunc changes all flags that contain "_" separators From 32d9b70d22819c400b777691915aa5087e4386a8 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 25 Oct 2019 22:09:09 +0900 Subject: [PATCH 2/3] Ensure plugins return `pluginError`s on non-zero exit statuses Signed-off-by: Yusuke Kuoka --- cmd/helm/plugin_test.go | 22 ++++++++++++++----- .../helm/plugins/exitwith/exitwith.sh | 2 ++ .../helm/plugins/exitwith/plugin.yaml | 4 ++++ 3 files changed, 23 insertions(+), 5 deletions(-) create mode 100755 cmd/helm/testdata/helmhome/helm/plugins/exitwith/exitwith.sh create mode 100644 cmd/helm/testdata/helmhome/helm/plugins/exitwith/plugin.yaml diff --git a/cmd/helm/plugin_test.go b/cmd/helm/plugin_test.go index e545a5d4f..7bc8fe70c 100644 --- a/cmd/helm/plugin_test.go +++ b/cmd/helm/plugin_test.go @@ -86,11 +86,13 @@ func TestLoadPlugins(t *testing.T) { long string expect string args []string + code int }{ - {"args", "echo args", "This echos args", "-a -b -c\n", []string{"-a", "-b", "-c"}}, - {"echo", "echo stuff", "This echos stuff", "hello\n", []string{}}, - {"env", "env stuff", "show the env", "env\n", []string{}}, - {"fullenv", "show env vars", "show all env vars", envs + "\n", []string{}}, + {"args", "echo args", "This echos args", "-a -b -c\n", []string{"-a", "-b", "-c"}, 0}, + {"echo", "echo stuff", "This echos stuff", "hello\n", []string{}, 0}, + {"env", "env stuff", "show the env", "env\n", []string{}, 0}, + {"exitwith", "exitwith code", "This exits with the specified exit code", "", []string{"2"}, 2}, + {"fullenv", "show env vars", "show all env vars", envs + "\n", []string{}, 0}, } plugins := cmd.Commands() @@ -117,7 +119,17 @@ func TestLoadPlugins(t *testing.T) { // tests until this is fixed if runtime.GOOS != "windows" { if err := pp.RunE(pp, tt.args); err != nil { - t.Errorf("Error running %s: %+v", tt.use, err) + if tt.code > 0 { + perr, ok := err.(pluginError) + if !ok { + t.Errorf("Expected %s to return pluginError: got %v(%T)", tt.use, err, err) + } + if perr.code != tt.code { + t.Errorf("Expected %s to return %d: got %d", tt.use, tt.code, perr.code) + } + } else { + t.Errorf("Error running %s: %+v", tt.use, err) + } } if out.String() != tt.expect { t.Errorf("Expected %s to output:\n%s\ngot\n%s", tt.use, tt.expect, out.String()) diff --git a/cmd/helm/testdata/helmhome/helm/plugins/exitwith/exitwith.sh b/cmd/helm/testdata/helmhome/helm/plugins/exitwith/exitwith.sh new file mode 100755 index 000000000..ec8469657 --- /dev/null +++ b/cmd/helm/testdata/helmhome/helm/plugins/exitwith/exitwith.sh @@ -0,0 +1,2 @@ +#!/bin/bash +exit $* diff --git a/cmd/helm/testdata/helmhome/helm/plugins/exitwith/plugin.yaml b/cmd/helm/testdata/helmhome/helm/plugins/exitwith/plugin.yaml new file mode 100644 index 000000000..5691d1712 --- /dev/null +++ b/cmd/helm/testdata/helmhome/helm/plugins/exitwith/plugin.yaml @@ -0,0 +1,4 @@ +name: exitwith +usage: "exitwith code" +description: "This exits with the specified exit code" +command: "$HELM_PLUGIN_DIR/exitwith.sh" From 5f27c0cbb7bcc3b356805233664a430d855e71e6 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 25 Oct 2019 22:39:21 +0900 Subject: [PATCH 3/3] Do test that a helm plugin can return non-zero exit code other than 1 Signed-off-by: Yusuke Kuoka --- cmd/helm/helm_test.go | 58 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go index 934f14f86..924e8e9d3 100644 --- a/cmd/helm/helm_test.go +++ b/cmd/helm/helm_test.go @@ -20,6 +20,8 @@ import ( "bytes" "io/ioutil" "os" + "os/exec" + "runtime" "strings" "testing" @@ -151,3 +153,59 @@ func testChdir(t *testing.T, dir string) func() { } return func() { os.Chdir(old) } } + +func TestPluginExitCode(t *testing.T) { + if os.Getenv("RUN_MAIN_FOR_TESTING") == "1" { + os.Args = []string{"helm", "exitwith", "2"} + + // We DO call helm's main() here. So this looks like a normal `helm` process. + main() + + // As main calls os.Exit, we never reach this line. + // But the test called this block of code catches and verifies the exit code. + return + } + + // Currently, plugins assume a Linux subsystem. Skip the execution + // tests until this is fixed + if runtime.GOOS != "windows" { + // Do a second run of this specific test(TestPluginExitCode) with RUN_MAIN_FOR_TESTING=1 set, + // So that the second run is able to run main() and this first run can verify the exit status returned by that. + // + // This technique originates from https://talks.golang.org/2014/testing.slide#23. + cmd := exec.Command(os.Args[0], "-test.run=TestPluginExitCode") + cmd.Env = append( + os.Environ(), + "RUN_MAIN_FOR_TESTING=1", + // See pkg/cli/environment.go for which envvars can be used for configuring these passes + // and also see plugin_test.go for how a plugin env can be set up. + // We just does the same setup as plugin_test.go via envvars + "HELM_PLUGINS=testdata/helmhome/helm/plugins", + "HELM_REPOSITORY_CONFIG=testdata/helmhome/helm/repositories.yaml", + "HELM_REPOSITORY_CACHE=testdata/helmhome/helm/repository", + ) + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + cmd.Stdout = stdout + cmd.Stderr = stderr + err := cmd.Run() + exiterr, ok := err.(*exec.ExitError) + + if !ok { + t.Fatalf("Unexpected error returned by os.Exit: %T", err) + } + + if stdout.String() != "" { + t.Errorf("Expected no write to stdout: Got %q", stdout.String()) + } + + expectedStderr := "Error: plugin \"exitwith\" exited with error\n" + if stderr.String() != expectedStderr { + t.Errorf("Expected %q written to stderr: Got %q", expectedStderr, stderr.String()) + } + + if exiterr.ExitCode() != 2 { + t.Errorf("Expected exit code 2: Got %d", exiterr.ExitCode()) + } + } +}