incorrect arg parsing with helm plugins

[jkroepke]

Hi,

a user of the helm-secrets plugin reports an issue with --kube-insecure-skip-tls-verify since helm 3.16.

jkroepke/helm-secrets#492

It turn out that this boolean flag was ignored before (#12856). I expect that the internal behavior was broken before.

From my point of view, if --kube-insecure-skip-tls-verify is set, the value after the flag is parsed wrongly and lead to unexpected behavior.

Here is a simple plugin to indicate the issue:

# helm plugin install https://github.com/jkroepke/helm-arg-demo-plugin
Installed plugin: arg-demo
# helm arg-demo
Hello World
# helm arg-demo --kube-insecure-skip-tls-verify
Hello World
# helm arg-demo --kube-insecure-skip-tls-verify --version 1.0.3
Error: unknown flag: --version
# helm --debug arg-demo --kube-insecure-skip-tls-verify=true --version 1.0.3
Hello World --version 1.0.3
# helm --debug arg-demo --kube-insecure-skip-tls-verify true --version 1.0.3
Hello World --version 1.0.3

A workaround exists: A explicit boolean value must be set after the flag.

[gjenkins8]

The demo plugin might be private? (I get a 404)

[jkroepke]

The demo plugin might be private? (I get a 404)

@gjenkins8 thanks, it is public now.

[gjenkins8]

From a quick look, it seems manuallyProcessArgs (

helm/cmd/helm/load_plugins.go

Lines 180 to 185 in a142da0

	case isKnown(a):
	known = append(known, a)
	i++
	if i < len(args) {
	known = append(known, args[i])
	}

) assumes these "kvargs" flags are always followed by a value. And doesn't know how to infer defaults.

This logic is very old, so I am surprised it has not been an issue before. I suspect switching to a propper command-line flag parser would/should be done to solve.

[jkroepke]

@gjenkins8 I though the same and I tries to trigger this through an test https://github.com/helm/helm/pull/13675/files but I got no success there, that why I open an issue here.

[artemmikhalitsin]

helm/pkg/cmd/load_plugins.go

Lines 177 to 180 in 9fad3cd

	switch a := args[i]; a {
	case "--debug":
	known = append(known, a)
	case isKnown(a):

The quick and bad fix is to add --kube-insecure-skip-tls-verify here, but then --kube-insecure-skip-tls-verify=true will no longer work.

I assume this isn't a wanted resolution so I'll see if I can find a way to parse flags properly using cobra

As a sidenote, the reverse issue exists - --debug=true is also not recognized as a "known" command so will be echoed in the plugin that @jkroepke supplied

Edit: Didn't find a way to do it with cobra, but was able to fix the old logic to properly handle boolean flags of any format.

[github-actions]

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

[jkroepke]

Thanks for the info.

[artemmikhalitsin]

@gjenkins8
Possible to have someone review the PR? 🙏
#30614