mirror of
https://github.com/k3s-io/kubernetes.git
synced 2025-07-28 05:57:25 +00:00
Merge pull request #20918 from deads2k/fix-list-edit
Automatic merge from submit-queue fix edit on list Fixes https://github.com/kubernetes/kubernetes/issues/20519 This reverts the implementation that removed list editing capability, but leaves its tests intact. This allows edits of lists to work, while still allowing mutation of the annotations. It does this by walking each item and building per item patches. The current implementation will do funny things if you delete entire list entries. A followup could be written to locate the correct list item by name. Right now, it just rejects the patch because its trying to change an immutable field. @janetkuo @kubernetes/kubectl @kargakis
This commit is contained in:
commit
6936b9ff21
@ -23,11 +23,13 @@ import (
|
|||||||
"io"
|
"io"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"reflect"
|
||||||
gruntime "runtime"
|
gruntime "runtime"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"k8s.io/kubernetes/pkg/api"
|
"k8s.io/kubernetes/pkg/api"
|
||||||
"k8s.io/kubernetes/pkg/api/errors"
|
"k8s.io/kubernetes/pkg/api/errors"
|
||||||
|
"k8s.io/kubernetes/pkg/api/meta"
|
||||||
"k8s.io/kubernetes/pkg/api/unversioned"
|
"k8s.io/kubernetes/pkg/api/unversioned"
|
||||||
"k8s.io/kubernetes/pkg/kubectl"
|
"k8s.io/kubernetes/pkg/kubectl"
|
||||||
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
|
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
|
||||||
@ -159,8 +161,8 @@ func RunEdit(f *cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
|
|||||||
NamespaceParam(cmdNamespace).DefaultNamespace().
|
NamespaceParam(cmdNamespace).DefaultNamespace().
|
||||||
FilenameParam(enforceNamespace, options.Recursive, options.Filenames...).
|
FilenameParam(enforceNamespace, options.Recursive, options.Filenames...).
|
||||||
ResourceTypeOrNameArgs(true, args...).
|
ResourceTypeOrNameArgs(true, args...).
|
||||||
Latest().
|
|
||||||
Flatten().
|
Flatten().
|
||||||
|
Latest().
|
||||||
Do()
|
Do()
|
||||||
err = r.Err()
|
err = r.Err()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@ -182,7 +184,7 @@ func RunEdit(f *cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
objs, err := resource.AsVersionedObjects(infos, defaultVersion, encoder)
|
originalObj, err := resource.AsVersionedObject(infos, false, defaultVersion, encoder)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
@ -196,126 +198,189 @@ func RunEdit(f *cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
|
|||||||
file string
|
file string
|
||||||
)
|
)
|
||||||
|
|
||||||
outter:
|
containsError := false
|
||||||
for i := range objs {
|
|
||||||
obj := objs[i]
|
|
||||||
// some bookkeeping
|
|
||||||
results.header.flush()
|
|
||||||
containsError := false
|
|
||||||
|
|
||||||
for {
|
for {
|
||||||
// generate the file to edit
|
// infos mutates over time to be the list of things we've tried and failed to edit
|
||||||
buf := &bytes.Buffer{}
|
// this means that our overall list changes over time.
|
||||||
var w io.Writer = buf
|
objToEdit, err := resource.AsVersionedObject(infos, false, defaultVersion, encoder)
|
||||||
if windowsLineEndings {
|
if err != nil {
|
||||||
w = crlf.NewCRLFWriter(w)
|
return err
|
||||||
}
|
}
|
||||||
if err := results.header.writeTo(w); err != nil {
|
|
||||||
|
// generate the file to edit
|
||||||
|
buf := &bytes.Buffer{}
|
||||||
|
var w io.Writer = buf
|
||||||
|
if windowsLineEndings {
|
||||||
|
w = crlf.NewCRLFWriter(w)
|
||||||
|
}
|
||||||
|
if err := results.header.writeTo(w); err != nil {
|
||||||
|
return preservedFile(err, results.file, errOut)
|
||||||
|
}
|
||||||
|
if !containsError {
|
||||||
|
if err := printer.PrintObj(objToEdit, w); err != nil {
|
||||||
return preservedFile(err, results.file, errOut)
|
return preservedFile(err, results.file, errOut)
|
||||||
}
|
}
|
||||||
if !containsError {
|
original = buf.Bytes()
|
||||||
if err := printer.PrintObj(obj, w); err != nil {
|
} else {
|
||||||
return preservedFile(err, results.file, errOut)
|
// In case of an error, preserve the edited file.
|
||||||
}
|
// Remove the comments (header) from it since we already
|
||||||
original = buf.Bytes()
|
// have included the latest header in the buffer above.
|
||||||
} else {
|
buf.Write(manualStrip(edited))
|
||||||
// In case of an error, preserve the edited file.
|
}
|
||||||
// Remove the comments (header) from it since we already
|
|
||||||
// have included the latest header in the buffer above.
|
|
||||||
buf.Write(manualStrip(edited))
|
|
||||||
}
|
|
||||||
|
|
||||||
// launch the editor
|
// launch the editor
|
||||||
editedDiff := edited
|
editedDiff := edited
|
||||||
edited, file, err = edit.LaunchTempFile(fmt.Sprintf("%s-edit-", filepath.Base(os.Args[0])), ext, buf)
|
edited, file, err = edit.LaunchTempFile(fmt.Sprintf("%s-edit-", filepath.Base(os.Args[0])), ext, buf)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return preservedFile(err, results.file, errOut)
|
return preservedFile(err, results.file, errOut)
|
||||||
}
|
}
|
||||||
if bytes.Equal(stripComments(editedDiff), stripComments(edited)) {
|
if bytes.Equal(stripComments(editedDiff), stripComments(edited)) {
|
||||||
// Ugly hack right here. We will hit this either (1) when we try to
|
// Ugly hack right here. We will hit this either (1) when we try to
|
||||||
// save the same changes we tried to save in the previous iteration
|
// save the same changes we tried to save in the previous iteration
|
||||||
// which means our changes are invalid or (2) when we exit the second
|
// which means our changes are invalid or (2) when we exit the second
|
||||||
// time. The second case is more usual so we can probably live with it.
|
// time. The second case is more usual so we can probably live with it.
|
||||||
// TODO: A less hacky fix would be welcome :)
|
// TODO: A less hacky fix would be welcome :)
|
||||||
fmt.Fprintln(errOut, "Edit cancelled, no valid changes were saved.")
|
fmt.Fprintln(errOut, "Edit cancelled, no valid changes were saved.")
|
||||||
continue outter
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// cleanup any file from the previous pass
|
// cleanup any file from the previous pass
|
||||||
if len(results.file) > 0 {
|
if len(results.file) > 0 {
|
||||||
os.Remove(results.file)
|
os.Remove(results.file)
|
||||||
}
|
}
|
||||||
glog.V(4).Infof("User edited:\n%s", string(edited))
|
glog.V(4).Infof("User edited:\n%s", string(edited))
|
||||||
|
|
||||||
// Compare content without comments
|
// Compare content without comments
|
||||||
if bytes.Equal(stripComments(original), stripComments(edited)) {
|
if bytes.Equal(stripComments(original), stripComments(edited)) {
|
||||||
os.Remove(file)
|
os.Remove(file)
|
||||||
fmt.Fprintln(errOut, "Edit cancelled, no changes made.")
|
fmt.Fprintln(errOut, "Edit cancelled, no changes made.")
|
||||||
continue outter
|
return nil
|
||||||
}
|
}
|
||||||
lines, err := hasLines(bytes.NewBuffer(edited))
|
lines, err := hasLines(bytes.NewBuffer(edited))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return preservedFile(err, file, errOut)
|
return preservedFile(err, file, errOut)
|
||||||
}
|
}
|
||||||
if !lines {
|
if !lines {
|
||||||
os.Remove(file)
|
os.Remove(file)
|
||||||
fmt.Fprintln(errOut, "Edit cancelled, saved file was empty.")
|
fmt.Fprintln(errOut, "Edit cancelled, saved file was empty.")
|
||||||
continue outter
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
results = editResults{
|
results = editResults{
|
||||||
file: file,
|
file: file,
|
||||||
}
|
}
|
||||||
|
|
||||||
// parse the edited file
|
// parse the edited file
|
||||||
updates, err := resourceMapper.InfoForData(edited, "edited-file")
|
updates, err := resourceMapper.InfoForData(edited, "edited-file")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
// syntax error
|
// syntax error
|
||||||
containsError = true
|
containsError = true
|
||||||
results.header.reasons = append(results.header.reasons, editReason{head: fmt.Sprintf("The edited file had a syntax error: %v", err)})
|
results.header.reasons = append(results.header.reasons, editReason{head: fmt.Sprintf("The edited file had a syntax error: %v", err)})
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
// not a syntax error as it turns out...
|
// not a syntax error as it turns out...
|
||||||
containsError = false
|
containsError = false
|
||||||
|
|
||||||
|
namespaceVisitor := resource.NewFlattenListVisitor(updates, resourceMapper)
|
||||||
|
// need to make sure the original namespace wasn't changed while editing
|
||||||
|
if err = namespaceVisitor.Visit(resource.RequireNamespace(cmdNamespace)); err != nil {
|
||||||
|
return preservedFile(err, file, errOut)
|
||||||
|
}
|
||||||
|
|
||||||
|
mutatedObjects := []runtime.Object{}
|
||||||
|
annotationVisitor := resource.NewFlattenListVisitor(updates, resourceMapper)
|
||||||
|
// iterate through all items to apply annotations
|
||||||
|
if err = annotationVisitor.Visit(func(info *resource.Info, incomingErr error) error {
|
||||||
// put configuration annotation in "updates"
|
// put configuration annotation in "updates"
|
||||||
if err := kubectl.CreateOrUpdateAnnotation(cmdutil.GetFlagBool(cmd, cmdutil.ApplyAnnotationsFlag), updates, encoder); err != nil {
|
if err := kubectl.CreateOrUpdateAnnotation(cmdutil.GetFlagBool(cmd, cmdutil.ApplyAnnotationsFlag), info, encoder); err != nil {
|
||||||
return preservedFile(err, file, errOut)
|
return err
|
||||||
}
|
}
|
||||||
if cmdutil.ShouldRecord(cmd, updates) {
|
if cmdutil.ShouldRecord(cmd, info) {
|
||||||
err = cmdutil.RecordChangeCause(updates.Object, f.Command())
|
if err := cmdutil.RecordChangeCause(info.Object, f.Command()); err != nil {
|
||||||
if err != nil {
|
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
editedCopy := edited
|
mutatedObjects = append(mutatedObjects, info.Object)
|
||||||
if editedCopy, err = runtime.Encode(encoder, updates.Object); err != nil {
|
|
||||||
return preservedFile(err, file, errOut)
|
return nil
|
||||||
|
|
||||||
|
}); err != nil {
|
||||||
|
return preservedFile(err, file, errOut)
|
||||||
|
}
|
||||||
|
|
||||||
|
// if we mutated a list in the visitor, persist the changes on the overall object
|
||||||
|
if meta.IsListType(updates.Object) {
|
||||||
|
meta.SetList(updates.Object, mutatedObjects)
|
||||||
|
}
|
||||||
|
|
||||||
|
patchVisitor := resource.NewFlattenListVisitor(updates, resourceMapper)
|
||||||
|
err = patchVisitor.Visit(func(info *resource.Info, incomingErr error) error {
|
||||||
|
currOriginalObj := originalObj
|
||||||
|
|
||||||
|
// if we're editing a list, then navigate the list to find the item that we're currently trying to edit
|
||||||
|
if meta.IsListType(originalObj) {
|
||||||
|
currOriginalObj = nil
|
||||||
|
editObjUID, err := meta.NewAccessor().UID(info.Object)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
listItems, err := meta.ExtractList(originalObj)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// iterate through the list to find the item with the matching UID
|
||||||
|
for i := range listItems {
|
||||||
|
originalObjUID, err := meta.NewAccessor().UID(listItems[i])
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if editObjUID == originalObjUID {
|
||||||
|
currOriginalObj = listItems[i]
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if currOriginalObj == nil {
|
||||||
|
return fmt.Errorf("no original object found for %#v", info.Object)
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
visitor := resource.NewFlattenListVisitor(updates, resourceMapper)
|
originalSerialization, err := runtime.Encode(encoder, currOriginalObj)
|
||||||
|
if err != nil {
|
||||||
// need to make sure the original namespace wasn't changed while editing
|
return err
|
||||||
if err = visitor.Visit(resource.RequireNamespace(cmdNamespace)); err != nil {
|
}
|
||||||
return preservedFile(err, file, errOut)
|
editedSerialization, err := runtime.Encode(encoder, info.Object)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// compute the patch on a per-item basis
|
||||||
// use strategic merge to create a patch
|
// use strategic merge to create a patch
|
||||||
originalJS, err := yaml.ToJSON(original)
|
originalJS, err := yaml.ToJSON(originalSerialization)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return preservedFile(err, file, errOut)
|
return err
|
||||||
}
|
}
|
||||||
editedJS, err := yaml.ToJSON(editedCopy)
|
editedJS, err := yaml.ToJSON(editedSerialization)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return preservedFile(err, file, errOut)
|
return err
|
||||||
}
|
}
|
||||||
patch, err := strategicpatch.CreateStrategicMergePatch(originalJS, editedJS, obj)
|
|
||||||
|
if reflect.DeepEqual(originalJS, editedJS) {
|
||||||
|
// no edit, so just skip it.
|
||||||
|
cmdutil.PrintSuccess(mapper, false, out, info.Mapping.Resource, info.Name, "skipped")
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
patch, err := strategicpatch.CreateStrategicMergePatch(originalJS, editedJS, currOriginalObj)
|
||||||
// TODO: change all jsonmerge to strategicpatch
|
// TODO: change all jsonmerge to strategicpatch
|
||||||
// for checking preconditions
|
// for checking preconditions
|
||||||
preconditions := []jsonmerge.PreconditionFunc{}
|
preconditions := []jsonmerge.PreconditionFunc{}
|
||||||
if err != nil {
|
if err != nil {
|
||||||
glog.V(4).Infof("Unable to calculate diff, no merge is possible: %v", err)
|
glog.V(4).Infof("Unable to calculate diff, no merge is possible: %v", err)
|
||||||
return preservedFile(err, file, errOut)
|
return err
|
||||||
} else {
|
} else {
|
||||||
preconditions = append(preconditions, jsonmerge.RequireKeyUnchanged("apiVersion"))
|
preconditions = append(preconditions, jsonmerge.RequireKeyUnchanged("apiVersion"))
|
||||||
preconditions = append(preconditions, jsonmerge.RequireKeyUnchanged("kind"))
|
preconditions = append(preconditions, jsonmerge.RequireKeyUnchanged("kind"))
|
||||||
@ -324,45 +389,49 @@ outter:
|
|||||||
}
|
}
|
||||||
|
|
||||||
if hold, msg := jsonmerge.TestPreconditionsHold(patch, preconditions); !hold {
|
if hold, msg := jsonmerge.TestPreconditionsHold(patch, preconditions); !hold {
|
||||||
fmt.Fprintf(errOut, "error: %s\n", msg)
|
fmt.Fprintf(errOut, "error: %s", msg)
|
||||||
return preservedFile(nil, file, errOut)
|
return preservedFile(nil, file, errOut)
|
||||||
}
|
}
|
||||||
|
|
||||||
errorMsg := ""
|
patched, err := resource.NewHelper(info.Client, info.Mapping).Patch(info.Namespace, info.Name, api.StrategicMergePatchType, patch)
|
||||||
err = visitor.Visit(func(info *resource.Info, err error) error {
|
if err != nil {
|
||||||
patched, err := resource.NewHelper(info.Client, info.Mapping).Patch(info.Namespace, info.Name, api.StrategicMergePatchType, patch)
|
fmt.Fprintln(out, results.addError(err, info))
|
||||||
if err != nil {
|
|
||||||
errorMsg = results.addError(err, info)
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
info.Refresh(patched, true)
|
|
||||||
cmdutil.PrintSuccess(mapper, false, out, info.Mapping.Resource, info.Name, "edited")
|
|
||||||
return nil
|
return nil
|
||||||
})
|
|
||||||
if err == nil {
|
|
||||||
os.Remove(file)
|
|
||||||
continue outter
|
|
||||||
}
|
}
|
||||||
// Handle all possible errors
|
info.Refresh(patched, true)
|
||||||
//
|
cmdutil.PrintSuccess(mapper, false, out, info.Mapping.Resource, info.Name, "edited")
|
||||||
// 1. retryable: propose kubectl replace -f
|
return nil
|
||||||
// 2. notfound: indicate the location of the saved configuration of the deleted resource
|
})
|
||||||
// 3. invalid: retry those on the spot by looping ie. reloading the editor
|
if err != nil {
|
||||||
if results.retryable > 0 {
|
return preservedFile(err, results.file, errOut)
|
||||||
fmt.Fprintln(errOut, errorMsg)
|
|
||||||
fmt.Fprintf(errOut, "You can run `%s replace -f %s` to try this update again.\n", filepath.Base(os.Args[0]), file)
|
|
||||||
continue outter
|
|
||||||
}
|
|
||||||
if results.notfound > 0 {
|
|
||||||
fmt.Fprintln(errOut, errorMsg)
|
|
||||||
fmt.Fprintf(errOut, "The edits you made on deleted resources have been saved to %q\n", file)
|
|
||||||
continue outter
|
|
||||||
}
|
|
||||||
// validation error
|
|
||||||
containsError = true
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Handle all possible errors
|
||||||
|
//
|
||||||
|
// 1. retryable: propose kubectl replace -f
|
||||||
|
// 2. notfound: indicate the location of the saved configuration of the deleted resource
|
||||||
|
// 3. invalid: retry those on the spot by looping ie. reloading the editor
|
||||||
|
if results.retryable > 0 {
|
||||||
|
fmt.Fprintf(errOut, "You can run `%s replace -f %s` to try this update again.\n", filepath.Base(os.Args[0]), file)
|
||||||
|
return errExit
|
||||||
|
}
|
||||||
|
if results.notfound > 0 {
|
||||||
|
fmt.Fprintf(errOut, "The edits you made on deleted resources have been saved to %q\n", file)
|
||||||
|
return errExit
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(results.edit) == 0 {
|
||||||
|
if results.notfound == 0 {
|
||||||
|
os.Remove(file)
|
||||||
|
} else {
|
||||||
|
fmt.Fprintf(out, "The edits you made on deleted resources have been saved to %q\n", file)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// loop again and edit the remaining items
|
||||||
|
infos = results.edit
|
||||||
}
|
}
|
||||||
return nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// editReason preserves a message about the reason this file must be edited again
|
// editReason preserves a message about the reason this file must be edited again
|
||||||
|
Loading…
Reference in New Issue
Block a user