Merge pull request #25085 from metral/recursive-edit

Automatic merge from submit-queue

enable recursive processing in kubectl edit

This PR was split out of https://github.com/kubernetes/kubernetes/pull/23673 per @deads2k's suggestion: https://github.com/kubernetes/kubernetes/pull/23673#discussion_r61291178

It makes use of the recursive processing of a directory in `kubectl edit`

hack/test-cmd.sh

@@ -1026,6 +1026,19 @@ __EOF__
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'busybox0:busybox1:'
   kube::test::if_has_string "${output_message}" 'error validating data: kind not set'
 
+  ## Edit multiple busybox PODs by updating the image field of multiple PODs recursively from a directory. tmp-editor.sh is a fake editor
+  # Pre-condition: busybox0 & busybox1 PODs exist
+  kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'busybox0:busybox1:'
+  # Command
+  echo -e '#!/bin/bash\nsed -i "s/image: busybox/image: prom\/busybox/g" $1' > /tmp/tmp-editor.sh
+  chmod +x /tmp/tmp-editor.sh
+  output_message=$(! EDITOR=/tmp/tmp-editor.sh kubectl edit -f hack/testdata/recursive/pod --recursive 2>&1 "${kube_flags[@]}")
+  # Post-condition: busybox0 & busybox1 PODs are edited, and since busybox2 is malformed, it should error
+  kube::test::get_object_assert pods "{{range.items}}{{$image_field}}:{{end}}" 'prom/busybox:prom/busybox:'
+  kube::test::if_has_string "${output_message}" "Object 'Kind' is missing"
+  # cleaning
+  rm /tmp/tmp-editor.sh
+
   ## Replace multiple busybox PODs recursively from directory of YAML files
   # Pre-condition: busybox0 & busybox1 PODs exist
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'busybox0:busybox1:'

pkg/kubectl/cmd/edit.go

@@ -161,6 +161,7 @@ func RunEdit(f *cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 		NamespaceParam(cmdNamespace).DefaultNamespace().
 		FilenameParam(enforceNamespace, options.Recursive, options.Filenames...).
 		ResourceTypeOrNameArgs(true, args...).
+		ContinueOnError().
 		Flatten().
 		Latest().
 		Do()
@@ -169,11 +170,6 @@ func RunEdit(f *cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 		return err
 	}
 
-	infos, err := r.Infos()
-	if err != nil {
-		return err
-	}
-
 	clientConfig, err := f.ClientConfig()
 	if err != nil {
 		return err
@@ -184,254 +180,257 @@ func RunEdit(f *cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 	if err != nil {
 		return err
 	}
-	originalObj, err := resource.AsVersionedObject(infos, false, defaultVersion, encoder)
-	if err != nil {
-		return err
-	}
 
 	var (
 		windowsLineEndings = cmdutil.GetFlagBool(cmd, "windows-line-endings")
 		edit               = editor.NewDefaultEditor(f.EditorEnvs())
-		results            = editResults{}
-		original           = []byte{}
-		edited             = []byte{}
-		file               string
 	)
 
-	containsError := false
+	err = r.Visit(func(info *resource.Info, err error) error {
+		var (
+			results  = editResults{}
+			original = []byte{}
+			edited   = []byte{}
+			file     string
+		)
 
-	for {
+		containsError := false
-		// infos mutates over time to be the list of things we've tried and failed to edit
-		// this means that our overall list changes over time.
-		objToEdit, err := resource.AsVersionedObject(infos, false, defaultVersion, encoder)
-		if err != nil {
-			return err
-		}
 
-		// generate the file to edit
+		for {
-		buf := &bytes.Buffer{}
+			infos := []*resource.Info{info}
-		var w io.Writer = buf
+			originalObj, err := resource.AsVersionedObject(infos, false, defaultVersion, encoder)
-		if windowsLineEndings {
+			if err != nil {
-			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)
-			}
-			original = buf.Bytes()
-		} else {
-			// 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
-		editedDiff := edited
-		edited, file, err = edit.LaunchTempFile(fmt.Sprintf("%s-edit-", filepath.Base(os.Args[0])), ext, buf)
-		if err != nil {
-			return preservedFile(err, results.file, errOut)
-		}
-		if bytes.Equal(stripComments(editedDiff), stripComments(edited)) {
-			// 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
-			// 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.
-			// TODO: A less hacky fix would be welcome :)
-			fmt.Fprintln(errOut, "Edit cancelled, no valid changes were saved.")
-			return nil
-		}
-
-		// cleanup any file from the previous pass
-		if len(results.file) > 0 {
-			os.Remove(results.file)
-		}
-		glog.V(4).Infof("User edited:\n%s", string(edited))
-
-		// Compare content without comments
-		if bytes.Equal(stripComments(original), stripComments(edited)) {
-			os.Remove(file)
-			fmt.Fprintln(errOut, "Edit cancelled, no changes made.")
-			return nil
-		}
-		lines, err := hasLines(bytes.NewBuffer(edited))
-		if err != nil {
-			return preservedFile(err, file, errOut)
-		}
-		if !lines {
-			os.Remove(file)
-			fmt.Fprintln(errOut, "Edit cancelled, saved file was empty.")
-			return nil
-		}
-
-		results = editResults{
-			file: file,
-		}
-
-		// parse the edited file
-		updates, err := resourceMapper.InfoForData(edited, "edited-file")
-		if err != nil {
-			// syntax error
-			containsError = true
-			results.header.reasons = append(results.header.reasons, editReason{head: fmt.Sprintf("The edited file had a syntax error: %v", err)})
-			continue
-		}
-		// not a syntax error as it turns out...
-		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"
-			if err := kubectl.CreateOrUpdateAnnotation(cmdutil.GetFlagBool(cmd, cmdutil.ApplyAnnotationsFlag), info, encoder); err != nil {
 				return err
 			}
-			if cmdutil.ShouldRecord(cmd, info) {
+
-				if err := cmdutil.RecordChangeCause(info.Object, f.Command()); err != nil {
+			objToEdit := originalObj
-					return err
+
-				}
+			// generate the file to edit
+			buf := &bytes.Buffer{}
+			var w io.Writer = buf
+			if windowsLineEndings {
+				w = crlf.NewCRLFWriter(w)
 			}
-			mutatedObjects = append(mutatedObjects, info.Object)
 
-			return nil
+			results.header.writeTo(w)
 
-		}); err != nil {
+			if !containsError {
-			return preservedFile(err, file, errOut)
+				if err := printer.PrintObj(objToEdit, w); err != nil {
-		}
+					return preservedFile(err, results.file, errOut)
+				}
+				original = buf.Bytes()
+			} else {
+				// 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))
+			}
 
-		// if we mutated a list in the visitor, persist the changes on the overall object
+			// launch the editor
-		if meta.IsListType(updates.Object) {
+			editedDiff := edited
-			meta.SetList(updates.Object, mutatedObjects)
+			edited, file, err = edit.LaunchTempFile(fmt.Sprintf("%s-edit-", filepath.Base(os.Args[0])), ext, buf)
-		}
+			if err != nil {
+				return preservedFile(err, results.file, errOut)
+			}
+			if bytes.Equal(stripComments(editedDiff), stripComments(edited)) {
+				// 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
+				// 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.
+				// TODO: A less hacky fix would be welcome :)
+				fmt.Fprintln(errOut, "Edit cancelled, no valid changes were saved.")
+				return nil
+			}
 
-		patchVisitor := resource.NewFlattenListVisitor(updates, resourceMapper)
+			// cleanup any file from the previous pass
-		err = patchVisitor.Visit(func(info *resource.Info, incomingErr error) error {
+			if len(results.file) > 0 {
-			currOriginalObj := originalObj
+				os.Remove(results.file)
+			}
+			glog.V(4).Infof("User edited:\n%s", string(edited))
 
-			// if we're editing a list, then navigate the list to find the item that we're currently trying to edit
+			// Compare content without comments
-			if meta.IsListType(originalObj) {
+			if bytes.Equal(stripComments(original), stripComments(edited)) {
-				currOriginalObj = nil
+				os.Remove(file)
-				editObjUID, err := meta.NewAccessor().UID(info.Object)
+				fmt.Fprintln(errOut, "Edit cancelled, no changes made.")
-				if err != nil {
+				return nil
+			}
+			lines, err := hasLines(bytes.NewBuffer(edited))
+			if err != nil {
+				return preservedFile(err, file, errOut)
+			}
+			if !lines {
+				os.Remove(file)
+				fmt.Fprintln(errOut, "Edit cancelled, saved file was empty.")
+				return nil
+			}
+
+			results = editResults{
+				file: file,
+			}
+
+			// parse the edited file
+			updates, err := resourceMapper.InfoForData(edited, "edited-file")
+			if err != nil {
+				// syntax error
+				containsError = true
+				results.header.reasons = append(results.header.reasons, editReason{head: fmt.Sprintf("The edited file had a syntax error: %v", err)})
+				continue
+			}
+			// not a syntax error as it turns out...
+			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"
+				if err := kubectl.CreateOrUpdateAnnotation(cmdutil.GetFlagBool(cmd, cmdutil.ApplyAnnotationsFlag), info, encoder); err != nil {
 					return err
 				}
-
+				if cmdutil.ShouldRecord(cmd, info) {
-				listItems, err := meta.ExtractList(originalObj)
+					if err := cmdutil.RecordChangeCause(info.Object, f.Command()); err != nil {
-				if err != nil {
+						return err
-					return err
+					}
 				}
+				mutatedObjects = append(mutatedObjects, info.Object)
 
-				// iterate through the list to find the item with the matching UID
+				return nil
-				for i := range listItems {
+
-					originalObjUID, err := meta.NewAccessor().UID(listItems[i])
+			}); 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
 					}
-					if editObjUID == originalObjUID {
+
-						currOriginalObj = listItems[i]
+					listItems, err := meta.ExtractList(originalObj)
-						break
+					if err != nil {
+						return err
 					}
-				}
+
-				if currOriginalObj == nil {
+					// iterate through the list to find the item with the matching UID
-					return fmt.Errorf("no original object found for %#v", info.Object)
+					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)
+					}
+
 				}
 
+				originalSerialization, err := runtime.Encode(encoder, currOriginalObj)
+				if err != nil {
+					return err
+				}
+				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
+				originalJS, err := yaml.ToJSON(originalSerialization)
+				if err != nil {
+					return err
+				}
+				editedJS, err := yaml.ToJSON(editedSerialization)
+				if err != nil {
+					return err
+				}
+
+				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
+				// for checking preconditions
+				preconditions := []jsonmerge.PreconditionFunc{}
+				if err != nil {
+					glog.V(4).Infof("Unable to calculate diff, no merge is possible: %v", err)
+					return err
+				} else {
+					preconditions = append(preconditions, jsonmerge.RequireKeyUnchanged("apiVersion"))
+					preconditions = append(preconditions, jsonmerge.RequireKeyUnchanged("kind"))
+					preconditions = append(preconditions, jsonmerge.RequireMetadataKeyUnchanged("name"))
+					results.version = defaultVersion
+				}
+
+				if hold, msg := jsonmerge.TestPreconditionsHold(patch, preconditions); !hold {
+					fmt.Fprintf(errOut, "error: %s", msg)
+					return preservedFile(nil, file, errOut)
+				}
+
+				patched, err := resource.NewHelper(info.Client, info.Mapping).Patch(info.Namespace, info.Name, api.StrategicMergePatchType, patch)
+				if err != nil {
+					fmt.Fprintln(out, results.addError(err, info))
+					return nil
+				}
+				info.Refresh(patched, true)
+				cmdutil.PrintSuccess(mapper, false, out, info.Mapping.Resource, info.Name, "edited")
+				return nil
+			})
+			if err != nil {
+				return preservedFile(err, results.file, errOut)
 			}
 
-			originalSerialization, err := runtime.Encode(encoder, currOriginalObj)
+			// Handle all possible errors
-			if err != nil {
+			//
-				return err
+			// 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
 			}
-			editedSerialization, err := runtime.Encode(encoder, info.Object)
+			if results.notfound > 0 {
-			if err != nil {
+				fmt.Fprintf(errOut, "The edits you made on deleted resources have been saved to %q\n", file)
-				return err
+				return errExit
 			}
 
-			// compute the patch on a per-item basis
+			if len(results.edit) == 0 {
-			// use strategic merge to create a patch
+				if results.notfound == 0 {
-			originalJS, err := yaml.ToJSON(originalSerialization)
+					os.Remove(file)
-			if err != nil {
+				} else {
-				return err
+					fmt.Fprintf(out, "The edits you made on deleted resources have been saved to %q\n", file)
-			}
+				}
-			editedJS, err := yaml.ToJSON(editedSerialization)
-			if err != nil {
-				return err
-			}
-
-			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)
+			// loop again and edit the remaining items
-			// TODO: change all jsonmerge to strategicpatch
+			infos = results.edit
-			// for checking preconditions
-			preconditions := []jsonmerge.PreconditionFunc{}
-			if err != nil {
-				glog.V(4).Infof("Unable to calculate diff, no merge is possible: %v", err)
-				return err
-			} else {
-				preconditions = append(preconditions, jsonmerge.RequireKeyUnchanged("apiVersion"))
-				preconditions = append(preconditions, jsonmerge.RequireKeyUnchanged("kind"))
-				preconditions = append(preconditions, jsonmerge.RequireMetadataKeyUnchanged("name"))
-				results.version = defaultVersion
-			}
-
-			if hold, msg := jsonmerge.TestPreconditionsHold(patch, preconditions); !hold {
-				fmt.Fprintf(errOut, "error: %s", msg)
-				return preservedFile(nil, file, errOut)
-			}
-
-			patched, err := resource.NewHelper(info.Client, info.Mapping).Patch(info.Namespace, info.Name, api.StrategicMergePatchType, patch)
-			if err != nil {
-				fmt.Fprintln(out, results.addError(err, info))
-				return nil
-			}
-			info.Refresh(patched, true)
-			cmdutil.PrintSuccess(mapper, false, out, info.Mapping.Resource, info.Name, "edited")
-			return nil
-		})
-		if err != nil {
-			return preservedFile(err, results.file, errOut)
 		}
-
+	})
-		// Handle all possible errors
+	return err
-		//
-		// 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
-	}
 }
 
 // editReason preserves a message about the reason this file must be edited again