kubernetes: kubectl edit: error UX is confusing and inconsistent

I edit a pod with some metadata such as:

# Please edit the object below. Lines beginning with a '#' will be ignored,
# and an empty file will abort the edit. If an error occurs while saving this file will be
# reopened with the relevant failures.
#
apiVersion: v1
kind: Pod
metadata:
  generateName: guestbook-
  labels:
    app: guestbook

Then I close the file and it pops back open with this message after I add fun: true label:

# Please edit the object below. Lines beginning with a '#' will be ignored,
# and an empty file will abort the edit. If an error occurs while saving this file will be
# reopened with the relevant failures.
#
# The edited file had a syntax error: unable to decode "edited-file": [pos 453]: json: expect char '"' but got char 't'
#
apiVersion: v1
kind: Pod
metadata:
  generateName: guestbook-
  labels:
    app: guestbook
    fun: true

Putting the error in the comment header is OK but I was very confused when this happened to me and didn’t notice the errors at all. A few potential fixes:

Print out the errors and then ask the user to “press enter” to fix, then launch the $EDITOR again
Put the error message above the line that generated the error
Automatically quote true/false values before converting to JSON in fields like labels.

Given the three options 1 is probably easiest and would fix the issue well enough for now.

cc @janetkuo as we discussed this might be a duplicate but I can’t find the other one.

About this issue

Original URL
State: open
Created 8 years ago
Reactions: 1
Comments: 18 (13 by maintainers)

Commits related to this issue

Merge pull request #33973 from ymqytw/fix_edit Automatic merge from submit-queue Improve edit experience Improve edit experience a bit according [#26050(comment)](https://github.com/kubernetes/kube... — committed to kubernetes/kubernetes by deleted user 8 years ago

Most upvoted comments

@MorrisLaw I didn’t get time to work on this previously but now I’m able to. So I’d still like to work on this 😃 I’ll add you in the PR so you can review too

mrajashree on Dec 27, 2017

@mrajashree Feel free to take it, thanks for the interest! Ping us if you need any help.

fabianofranz on Sep 15, 2017

Hi @pwittrock, I’m interested in contributing to kubernetes. Can I take this up if it’s not already taken?

mrajashree on Sep 14, 2017

@nikhita Sorry no one ever responded. If you are still interested in this @apelisse may be able to help you

pwittrock on Jun 2, 2017

Hi, I am new here and noticed that this was marked as a good issue for new contributors.

Regarding putting validation errors (eg, swagger-based schema validation) next to lines in question, I found that it was a little difficult to get line numbers out of the functions that are being used for the validation: https://github.com/kubernetes/kubernetes/blob/master/pkg/kubectl/cmd/util/factory.go#L489. Maybe I’m missing something here? Is there a better way to achieve this?

Any pointers to get started would be much appreciated, thanks.

nikhita on Mar 19, 2017

I agree. In fact, I think it doesn’t go far enough.

Sometimes I get this:

$ kubectl edit ns test
A copy of your changes has been stored to "/tmp/kubectl-edit-oh43e.yaml"
error: expected type string, for field metadata.finalizers[1], got bool

Sometimes it pops me back into an editor with comments:

# namespaces "test" was not valid:
# * metadata.finalizers: Invalid value: "foobar": name is neither a standard finalizer name nor is it fully qualified

…but it has actually deleted my changes! If I made any other changes that were not errors, they are erased. This is really bad.

Other times, as in the case of #32398, it accepts my changes, generates a do-nothing patch and exits. If I don’t read carefully, I might think it succeeded, but didn’t do anything.

So there are a number of bugs here. I think it should:

a) always go back to the editor b) always retain what I hand-edited, even if that has to be in comments c) put validation errors in comments next to the lines in question d) treat YAML errors the same as validation errors (I know that is significant work) e) detect do-nothing PATCHES and give a big error (even if you have to save the upstream state and diff the saved state against it - detect it!) f) probably not use PATCH at all.

thockin on Sep 10, 2016