Fix arg parsing for plugins. Add missing global args.

Signed-off-by: Artem Mikhalitsin <artemmikhalitsin@gmail.com>
pull/30614/head
Artem Mikhalitsin 7 months ago
parent 9fad3cd907
commit 7d79f0fd37

@ -64,7 +64,6 @@ func loadPlugins(baseCmd *cobra.Command, out io.Writer) {
// Now we create commands for all of these. // Now we create commands for all of these.
for _, plug := range found { for _, plug := range found {
plug := plug
md := plug.Metadata md := plug.Metadata
if md.Usage == "" { if md.Usage == "" {
md.Usage = fmt.Sprintf("the %q plugin", md.Name) md.Usage = fmt.Sprintf("the %q plugin", md.Name)
@ -151,11 +150,36 @@ func callPluginExecutable(pluginName string, main string, argv []string, out io.
// manuallyProcessArgs processes an arg array, removing special args. // 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 that are consumed by the root `helm` command
// Unknown args are any args that will not be consumed by helm, and that may be
// processed by the plugin plus
// The single exception is -h or --help, which is passed to the plugin
func manuallyProcessArgs(args []string) ([]string, []string) { func manuallyProcessArgs(args []string) ([]string, []string) {
known := []string{} known := []string{}
unknown := []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"} boolArgs := []string{
knownArg := func(a string) bool { "--debug",
"--kube-insecure-skip-tls-verify",
}
kvargs := []string{
"--burst-limit",
"--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"}
kvargs = append(kvargs, boolArgs...) // bool args can also by kv i.e. `--debug=true`
// detects args with 'arg=val' syntax
isKnownKvWithEquals := func(a string) bool {
for _, pre := range kvargs { for _, pre := range kvargs {
if strings.HasPrefix(a, pre+"=") { if strings.HasPrefix(a, pre+"=") {
return true return true
@ -163,31 +187,60 @@ func manuallyProcessArgs(args []string) ([]string, []string) {
} }
return false return false
} }
// detects standalone boolean flags
isKnown := func(v string) string { isKnownBoolean := func(v string) bool {
for _, i := range boolArgs {
if i == v {
return true
}
}
return false
}
// detects boolean flags where the next arg can be parsed to boolean
isKnownBooleanWithNextValue := func(args []string, i int) bool {
if len(args)-i < 2 { // must be at least 2 args remaining
return false
}
a := args[i]
for _, known := range boolArgs {
if known == a {
possibleBool := args[i+1] // safe - checked that at least 2 args left in slice
_, err := strconv.ParseBool(possibleBool)
if err == nil {
return true
}
}
}
return false
}
isKnownKv := func(v string) bool {
for _, i := range kvargs { for _, i := range kvargs {
if i == v { if i == v {
return v return true
} }
} }
return "" return false
} }
for i := 0; i < len(args); i++ { for i := 0; i < len(args); i++ {
switch a := args[i]; a { a := args[i]
case "--debug": if isKnownKvWithEquals(a) { // is it an argument using `arg=val` syntax?
known = append(known, a) known = append(known, a)
case isKnown(a): } else if isKnownBooleanWithNextValue(args, i) { // is it a boolean flag using `--boolFlag true` syntax?
known = append(known, a) known = append(known, a)
i++ i++
if i < len(args) { if i < len(args) {
known = append(known, args[i]) known = append(known, args[i])
} }
default: } else if isKnownBoolean(a) { // is it a known boolean flag by itself? i.e. `--boolFlag`
if knownArg(a) {
known = append(known, a) known = append(known, a)
continue } else if isKnownKv(a) {
known = append(known, a) // is it a known kv flag?
i++
if i < len(args) {
known = append(known, args[i])
} }
} else {
unknown = append(unknown, a) unknown = append(unknown, a)
} }
} }

@ -25,6 +25,7 @@ import (
"github.com/spf13/cobra" "github.com/spf13/cobra"
"github.com/spf13/pflag" "github.com/spf13/pflag"
"github.com/stretchr/testify/assert"
release "helm.sh/helm/v4/pkg/release/v1" release "helm.sh/helm/v4/pkg/release/v1"
) )
@ -32,7 +33,16 @@ import (
func TestManuallyProcessArgs(t *testing.T) { func TestManuallyProcessArgs(t *testing.T) {
input := []string{ input := []string{
"--debug", "--debug",
"--debug=true",
"--debug", "true",
"--debug", "false",
"--kube-insecure-skip-tls-verify",
"--kube-insecure-skip-tls-verify=true",
"--kube-insecure-skip-tls-verify", "true",
"--kube-insecure-skip-tls-verify", "false",
"--foo", "bar", "--foo", "bar",
"--burst-limit", "123",
"--burst-limit=123",
"--kubeconfig=/home/foo", "--kubeconfig=/home/foo",
"--kubeconfig", "/home/foo", "--kubeconfig", "/home/foo",
"--kube-context=test1", "--kube-context=test1",
@ -44,12 +54,24 @@ func TestManuallyProcessArgs(t *testing.T) {
"-n", "test2", "-n", "test2",
"--namespace=test2", "--namespace=test2",
"--namespace", "test2", "--namespace", "test2",
"--qps", "22",
"--qps=22",
"--home=/tmp", "--home=/tmp",
"command", "command",
"--debug", // check a boolean flag at the end to test possible out of bounds exception
} }
expectKnown := []string{ expectKnown := []string{
"--debug", "--debug",
"--debug=true",
"--debug", "true",
"--debug", "false",
"--kube-insecure-skip-tls-verify",
"--kube-insecure-skip-tls-verify=true",
"--kube-insecure-skip-tls-verify", "true",
"--kube-insecure-skip-tls-verify", "false",
"--burst-limit", "123",
"--burst-limit=123",
"--kubeconfig=/home/foo", "--kubeconfig=/home/foo",
"--kubeconfig", "/home/foo", "--kubeconfig", "/home/foo",
"--kube-context=test1", "--kube-context=test1",
@ -61,6 +83,9 @@ func TestManuallyProcessArgs(t *testing.T) {
"-n", "test2", "-n", "test2",
"--namespace=test2", "--namespace=test2",
"--namespace", "test2", "--namespace", "test2",
"--qps", "22",
"--qps=22",
"--debug",
} }
expectUnknown := []string{ expectUnknown := []string{
@ -69,17 +94,8 @@ func TestManuallyProcessArgs(t *testing.T) {
known, unknown := manuallyProcessArgs(input) known, unknown := manuallyProcessArgs(input)
for i, k := range known { assert.Equal(t, expectKnown, known)
if k != expectKnown[i] { assert.Equal(t, expectUnknown, unknown)
t.Errorf("expected known flag %d to be %q, got %q", i, expectKnown[i], k)
}
}
for i, k := range unknown {
if k != expectUnknown[i] {
t.Errorf("expected unknown flag %d to be %q, got %q", i, expectUnknown[i], k)
}
}
} }
func TestLoadPlugins(t *testing.T) { func TestLoadPlugins(t *testing.T) {

Loading…
Cancel
Save