From 3fdd3efd0d0a8202d6a9881c29ccf074cc766628 Mon Sep 17 00:00:00 2001
From: Federico Gimenez <federico@giantswarm.io>
Date: Sat, 16 Dec 2017 21:39:50 +0100
Subject: [PATCH] fix(helm): do not ignore plugin arguments with
 ignoreFlag=true

Makes it possible to not ignore the arguments given in a plugin call with
ignoreFlag=true until the first flag is found, from there the rest is removed.

Closes: #3218
---
 docs/plugins.md           |  4 ++-
 pkg/plugin/plugin.go      | 13 ++++++--
 pkg/plugin/plugin_test.go | 62 ++++++++++++++++++++++-----------------
 3 files changed, 49 insertions(+), 30 deletions(-)

diff --git a/docs/plugins.md b/docs/plugins.md
index c14063cf8..cb383822b 100644
--- a/docs/plugins.md
+++ b/docs/plugins.md
@@ -90,7 +90,9 @@ Restrictions on `name`:
 
 The `ignoreFlags` switch tells Helm to _not_ pass flags to the plugin. So if a
 plugin is called with `helm myplugin --foo` and `ignoreFlags: true`, then `--foo`
-is silently discarded.
+is silently discarded. The arguments provided until the first tag are kept, so
+`helm myplugin myarg --foo` and `ignoreFlags: true` will be equivalent to `helm
+myplugin myarg`.
 
 The `useTunnel` switch indicates that the plugin needs a tunnel to Tiller. This
 should be set to `true` _anytime a plugin talks to Tiller_. It will cause Helm
diff --git a/pkg/plugin/plugin.go b/pkg/plugin/plugin.go
index b3458c2d8..bfd102212 100644
--- a/pkg/plugin/plugin.go
+++ b/pkg/plugin/plugin.go
@@ -105,9 +105,18 @@ func (p *Plugin) PrepareCommand(extraArgs []string) (string, []string) {
 	if len(parts) > 1 {
 		baseArgs = parts[1:]
 	}
-	if !p.Metadata.IgnoreFlags {
-		baseArgs = append(baseArgs, extraArgs...)
+	extraIndex := len(extraArgs)
+	// determine the index of the first flag if we need to ignore them
+	if p.Metadata.IgnoreFlags {
+		for i, arg := range extraArgs {
+			if strings.HasPrefix(arg, "--") {
+				extraIndex = i
+				break
+			}
+		}
 	}
+	baseArgs = append(baseArgs, extraArgs[0:extraIndex]...)
+
 	return main, baseArgs
 }
 
diff --git a/pkg/plugin/plugin_test.go b/pkg/plugin/plugin_test.go
index 5ddbf15f3..e005ed0b1 100644
--- a/pkg/plugin/plugin_test.go
+++ b/pkg/plugin/plugin_test.go
@@ -28,37 +28,45 @@ func TestPrepareCommand(t *testing.T) {
 			Command: "echo -n foo",
 		},
 	}
-	argv := []string{"--debug", "--foo", "bar"}
-
-	cmd, args := p.PrepareCommand(argv)
-	if cmd != "echo" {
-		t.Errorf("Expected echo, got %q", cmd)
-	}
-
-	if l := len(args); l != 5 {
-		t.Errorf("expected 5 args, got %d", l)
+	testCases := []struct {
+		argv        []string
+		expect      []string
+		ignoreFlags bool
+	}{
+		{
+			argv:   []string{"--debug", "--foo", "bar"},
+			expect: []string{"-n", "foo", "--debug", "--foo", "bar"},
+		},
+		{
+			argv:   []string{},
+			expect: []string{"-n", "foo"},
+		},
+		{
+			argv:        []string{"--debug", "--foo", "bar"},
+			expect:      []string{"-n", "foo"},
+			ignoreFlags: true,
+		},
+		{
+			argv:        []string{"arg1", "arg2", "--debug", "--foo", "bar", "arg3"},
+			expect:      []string{"-n", "foo", "arg1", "arg2"},
+			ignoreFlags: true,
+		},
 	}
+	for _, tc := range testCases {
+		p.Metadata.IgnoreFlags = tc.ignoreFlags
+		cmd, args := p.PrepareCommand(tc.argv)
+		if cmd != "echo" {
+			t.Errorf("Expected echo, got %q", cmd)
+		}
 
-	expect := []string{"-n", "foo", "--debug", "--foo", "bar"}
-	for i := 0; i < len(args); i++ {
-		if expect[i] != args[i] {
-			t.Errorf("Expected arg=%q, got %q", expect[i], args[i])
+		if l, le := len(args), len(tc.expect); l != le {
+			t.Errorf("expected %d args, got %d", le, l)
 		}
-	}
 
-	// Test with IgnoreFlags. This should omit --debug, --foo, bar
-	p.Metadata.IgnoreFlags = true
-	cmd, args = p.PrepareCommand(argv)
-	if cmd != "echo" {
-		t.Errorf("Expected echo, got %q", cmd)
-	}
-	if l := len(args); l != 2 {
-		t.Errorf("expected 2 args, got %d", l)
-	}
-	expect = []string{"-n", "foo"}
-	for i := 0; i < len(args); i++ {
-		if expect[i] != args[i] {
-			t.Errorf("Expected arg=%q, got %q", expect[i], args[i])
+		for i := 0; i < len(args); i++ {
+			if tc.expect[i] != args[i] {
+				t.Errorf("Expected arg=%q, got %q", tc.expect[i], args[i])
+			}
 		}
 	}
 }