Confusing Error Message When Using kn service update

[danielhelfand]

Bug report

When using kn service update with the helloworld-go service example, an odd error response comes when you run the following:

kn service update helloworld-go --env TARGET=Go Sample v2
'service update' requires the service name given as single argument

The issue has nothing to do with the service name argument. The issue is that --env TARGET=Go Sample v2 should be a string: --env TARGET="Go Sample v2".

Possibly related to #762 and #778.

Expected behavior

An error message along the lines of --env TARGET must be a string.

It seems as if this error comes up as unexpected in an area where a hard coded error message is in place for a specific issue.

Steps to reproduce the problem

1. Create the helloworld-go service mentioned above
2. Attempt to update the service with kn service update helloworld-go --env TARGET=Go Sample v2 without double quotes around the value of TARGET.

kn version

Version:      v0.14.0
Build Date:   2020-04-22 08:56:32
Git Revision: c41e9fd
Supported APIs:
* Serving
  - serving.knative.dev/v1 (knative-serving v0.14.0)
* Eventing
  - sources.knative.dev/v1alpha2 (knative-eventing v0.14.1)
  - eventing.knative.dev/v1alpha1 (knative-eventing v0.14.1)

Knative (serving/eventing) version

Serving: v0.14.0

[rhuss]

It's indeed confusing here, but it's hard or even impossible for kn to detect that you forgot quotes for your label. For kn you called kn service update with three, non-flag arguments: "hello-world-go", "Sample" and "v2". The error message reports that you must only use a single argument which has to be the name of the service to update.

Maybe instead of

'service update' requires the service name given as single argument

rephrasing this to

'service update' requires the service name as single non-flag argument

sounds better ?

Happy for any other suggestion, though.

[MIBc]

In kubectl , it raises error: exactly one NAME is required, got 2 :)

[navidshaikh]

I think we should document quoting space separated flag values. Handling at kn level (instead of cobra) would bring multiple combinations to handle and complexity.

Ideally, the error message should come from the flag parsing library where it recognizes the pattern where command requires arg(s) and there are multiple values passed after = for flags.

[danielhelfand]

Ideally, the error message should come from the flag parsing library where it recognizes the pattern where command requires arg(s) and there are multiple values passed after = for flags.

Agree on this. Would be great if there was some flag-level validation on this, but I also think I can agree that a documented example might be the best way to go here. I realize this issue is a bit nitpicky as far as this error is concerned.

[rhuss]

I don't think this is easily possible as it is pefectly valid to use a command like kn service create --env MYENV=value myservicename. It's just that a space is an argument separator for any command, and tbh I would expect that a user on the teminal is aware of this (as the same behaviour applies to any unix command).

But the error message should be better. Like when multiple arguments are given we can say: "Only one argument for the name is allowed, but multiple are given: 'helloworld-go Sample v2'. Maybe you have forgotten to quote a flag value ?"

[rhuss]

Repeating the detected arguments in the error message might be just good enough.

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[rhuss]

Reopen it as I think we can still do better with the error message.

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.