Merge 35a3af2529 into 62c7e845d1

pkg/cmd/load_plugins.go

@@ -166,13 +166,51 @@ func processParent(cmd *cobra.Command, args []string) ([]string, error) {
 
 // manuallyProcessArgs processes an arg array, removing special args.
 //
-// Returns two sets of args: known and unknown (in that order)
+// Returns two sets of args: known and unknown (in that order).
+// Known args are flags consumed by the root helm command. Unknown args are
+// forwarded to the plugin. Help (-h/--help) is intentionally left for the plugin.
+//
+// Boolean root flags (--debug, --kube-insecure-skip-tls-verify) must not
+// unconditionally consume the next token: a bare `--flag` followed by a plugin
+// flag used to steal that plugin argument (see #13686). An explicit boolean
+// value after the flag (`--flag true`) is still accepted and normalized to
+// `--flag=value` for ParseFlags.
 func manuallyProcessArgs(args []string) ([]string, []string) {
 	known := []string{}
 	unknown := []string{}
-	kvargs := []string{"--kube-context", "--namespace", "-n", "--kubeconfig", "--kube-apiserver", "--kube-token", "--kube-as-user", "--kube-as-group", "--kube-ca-file", "--registry-config", "--repository-cache", "--repository-config", "--kube-insecure-skip-tls-verify", "--kube-tls-server-name"}
-	knownArg := func(a string) bool {
-		for _, pre := range kvargs {
+
+	boolFlags := []string{
+		"--debug",
+		"--kube-insecure-skip-tls-verify",
+	}
+	kvFlags := []string{
+		"--burst-limit",
+		"--color",
+		"--colour",
+		"--content-cache",
+		"--kube-apiserver",
+		"--kube-as-group",
+		"--kube-as-user",
+		"--kube-ca-file",
+		"--kube-context",
+		"--kube-tls-server-name",
+		"--kube-token",
+		"--kubeconfig",
+		"--namespace", "-n",
+		"--qps",
+		"--registry-config",
+		"--repository-cache",
+		"--repository-config",
+	}
+
+	isBoolFlag := func(a string) bool {
+		return slices.Contains(boolFlags, a)
+	}
+	isKVFlag := func(a string) bool {
+		return slices.Contains(kvFlags, a)
+	}
+	isKnownWithEquals := func(a string) bool {
+		for _, pre := range append(append([]string{}, boolFlags...), kvFlags...) {
 			if strings.HasPrefix(a, pre+"=") {
 				return true
 			}
@@ -180,28 +218,28 @@ func manuallyProcessArgs(args []string) ([]string, []string) {
 		return false
 	}
 
-	isKnown := func(v string) string {
-		if slices.Contains(kvargs, v) {
-			return v
+	for i := 0; i < len(args); i++ {
+		a := args[i]
+		switch {
+		case isKnownWithEquals(a):
+			known = append(known, a)
+		case isBoolFlag(a):
+			// Optional value: only consume the next token when it parses as a bool.
+			if i+1 < len(args) {
+				if _, err := strconv.ParseBool(args[i+1]); err == nil {
+					known = append(known, a+"="+args[i+1])
+					i++
+					continue
 				}
-		return ""
 			}
-
-	for i := 0; i < len(args); i++ {
-		switch a := args[i]; a {
-		case "--debug":
 			known = append(known, a)
-		case isKnown(a):
+		case isKVFlag(a):
 			known = append(known, a)
 			i++
 			if i < len(args) {
 				known = append(known, args[i])
 			}
 		default:
-			if knownArg(a) {
-				known = append(known, a)
-				continue
-			}
 			unknown = append(unknown, a)
 		}
 	}

pkg/cmd/plugin_test.go

@@ -79,6 +79,92 @@ func TestManuallyProcessArgs(t *testing.T) {
 	}
 }
 
+// TestManuallyProcessArgsBooleanFlags covers #13686: boolean root flags must not
+// consume the following plugin argument unless that token is an explicit bool value.
+func TestManuallyProcessArgsBooleanFlags(t *testing.T) {
+	tests := []struct {
+		name          string
+		input         []string
+		expectKnown   []string
+		expectUnknown []string
+	}{
+		{
+			name:          "bare kube insecure flag leaves following plugin flag",
+			input:         []string{"--kube-insecure-skip-tls-verify", "--version", "1.0.3"},
+			expectKnown:   []string{"--kube-insecure-skip-tls-verify"},
+			expectUnknown: []string{"--version", "1.0.3"},
+		},
+		{
+			name:          "separated true value is normalized",
+			input:         []string{"--kube-insecure-skip-tls-verify", "true", "--version", "1.0.3"},
+			expectKnown:   []string{"--kube-insecure-skip-tls-verify=true"},
+			expectUnknown: []string{"--version", "1.0.3"},
+		},
+		{
+			name:          "separated false value is normalized",
+			input:         []string{"--kube-insecure-skip-tls-verify", "false", "--foo", "bar"},
+			expectKnown:   []string{"--kube-insecure-skip-tls-verify=false"},
+			expectUnknown: []string{"--foo", "bar"},
+		},
+		{
+			name:          "inline boolean value",
+			input:         []string{"--kube-insecure-skip-tls-verify=true", "--version", "1.0.3"},
+			expectKnown:   []string{"--kube-insecure-skip-tls-verify=true"},
+			expectUnknown: []string{"--version", "1.0.3"},
+		},
+		{
+			name:          "bare debug leaves following plugin flag",
+			input:         []string{"--debug", "--plugin-flag", "value"},
+			expectKnown:   []string{"--debug"},
+			expectUnknown: []string{"--plugin-flag", "value"},
+		},
+		{
+			name:          "debug with separated bool value",
+			input:         []string{"--debug", "1", "command"},
+			expectKnown:   []string{"--debug=1"},
+			expectUnknown: []string{"command"},
+		},
+		{
+			name:          "debug with inline value",
+			input:         []string{"--debug=false", "command"},
+			expectKnown:   []string{"--debug=false"},
+			expectUnknown: []string{"command"},
+		},
+		{
+			name: "mixed bool and kv flags with plugin args",
+			input: []string{
+				"--debug",
+				"--kube-insecure-skip-tls-verify",
+				"--namespace", "default",
+				"--burst-limit", "100",
+				"--plugin-arg", "keep-me",
+				"subcommand",
+			},
+			expectKnown: []string{
+				"--debug",
+				"--kube-insecure-skip-tls-verify",
+				"--namespace", "default",
+				"--burst-limit", "100",
+			},
+			expectUnknown: []string{"--plugin-arg", "keep-me", "subcommand"},
+		},
+		{
+			name:          "boolean flag at end of args",
+			input:         []string{"command", "--kube-insecure-skip-tls-verify"},
+			expectKnown:   []string{"--kube-insecure-skip-tls-verify"},
+			expectUnknown: []string{"command"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			known, unknown := manuallyProcessArgs(tt.input)
+			assert.Equal(t, tt.expectKnown, known)
+			assert.Equal(t, tt.expectUnknown, unknown)
+		})
+	}
+}
+
 func TestLoadCLIPlugins(t *testing.T) {
 	settings.PluginsDirectory = "testdata/helmhome/helm/plugins"
 	settings.RepositoryConfig = "testdata/helmhome/helm/repositories.yaml"