Make kubectl edit not convert GV on edits #23437
Conversation
|
cc @stevekuznetsov cc @adohe |
| @@ -120,12 +120,18 @@ func RunEdit(f *cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args | |||
| return err | |||
| } | |||
|
|
|||
| // NB: we use `f.Decoder(false)` to get a plain deserializer for | |||
There was a problem hiding this comment.
I would suggest moving this comment to immediately before the f.Decoder(false) line.
There was a problem hiding this comment.
I would, but I didn't want to have a large block comment in the middle of a struct instantiation. WDYT?
There was a problem hiding this comment.
I don't think that's an issue. If the comment isn't living right next to this line, I could foresee someone changing it back to true without noticing the comment.
k8s-github-robot
added
the
size/XS
|
GCE e2e build/test failed for commit 95f08be. Please reference the list of currently known flakes when examining this failure. If you request a re-test, you must reference the issue describing the flake. |
Previously, kubectl edit was using a decoder to load in edits that converted to the internal version. It would then re-encode this decoded value to produce a patch. However, if you were editing in the object in a GroupVersion that was not the internal version, this would cause the kubectl edit command to attempt to produce a patch which changed the GroupVersion, which would fail. Now, we use a plain deserializer instead, so no conversion or defaulting occurs when loading in the edited file. Fixes kubernetes#23378
DirectXMan12
force-pushed
the
bug/cannot-edit-across-gv
branch
from
95f08be to
bc41430
Compare
|
GCE e2e build/test passed for commit bc41430. |
|
The author of this PR is not in the whitelist for merge, can one of the admins add the 'ok-to-merge' label? |
|
Over to @janetkuo since she knows this code better than I do. |
|
@k8s-bot e2e test this issue: #IGNORE (Build step 'Execute shell' marked build as failure?) |
|
GCE e2e build/test passed for commit bc41430. |
|
@k8s-bot node e2e test this |
janetkuo
added
lgtm
|
@DirectXMan12 Do you plan to solve the get/validation issue in this PR too? If not, the "fixes" keyword needs to be removed from the PR description. Otherwise the issue will be closed. |
|
GCE e2e build/test passed for commit bc41430. |
|
@janetkuo I recall not being able to reproduce the validation issue, just the edit issue (which is why, IIRC, I put it as fixes). I'll double-check that. |
|
@janetkuo just double checked, and I was able to repeat the validation "issue", although I'm not entirely certain it's an issue -- if you don't explicitly pass |
janetkuo
added
the
lgtm
|
Let's merge this first and keep the issue open then. |
|
GCE e2e build/test passed for commit bc41430. |
|
GCE e2e build/test passed for commit bc41430. |
|
GCE e2e build/test passed for commit bc41430. |
|
Automatic merge from submit-queue |
Previously, kubectl edit was using a decoder to load in edits that
converted to the internal version. It would then re-encode this
decoded value to produce a patch. However, if you were editing
in the object in a GroupVersion that was not the internal version,
this would cause the kubectl edit command to attempt to produce
a patch which changed the GroupVersion, which would fail.
Now, we use a plain deserializer instead, so no conversion or
defaulting occurs when loading in the edited file.
Ref #23378