Switch 'kubectl edit' to use unstructured objects, handle schemaless objects

mark --output-version as deprecated, add example for fully-qualifying version to edit

Add 'kubectl edit' testcase for editing schemaed and schemaless data together

Add 'kubectl edit' testcase for editing unknown version of known group/kind

pkg/kubectl/cmd/edit.go

@@ -46,6 +46,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/crlf"
 	"k8s.io/kubernetes/pkg/util/i18n"
 
+	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/golang/glog"
 	"github.com/spf13/cobra"
 )
@@ -61,9 +62,12 @@ var (
 		accepts filenames as well as command line arguments, although the files you point to must
 		be previously saved versions of resources.
 
-		The files to edit will be output in the default API version, or a version specified
-		by --output-version. The default format is YAML - if you would like to edit in JSON
-		pass -o json. The flag --windows-line-endings can be used to force Windows line endings,
+		Editing is done with the API version used to fetch the resource.
+		To edit using a specific API version, fully-qualify the resource, version, and group.
+		
+		The default format is YAML. To edit in JSON, specify "-o json".
+		
+		The flag --windows-line-endings can be used to force Windows line endings,
 		otherwise the default for your operating system will be used.
 
 		In the event an error occurs while updating, a temporary file will be created on disk
@@ -79,8 +83,8 @@ var (
 		# Use an alternative editor
 		KUBE_EDITOR="nano" kubectl edit svc/docker-registry
 
-		# Edit the service 'docker-registry' in JSON using the v1 API format:
-		kubectl edit svc/docker-registry --output-version=v1 -o json`)
+		# Edit the job 'myjob' in JSON using the v1 API format:
+		kubectl edit job.v1.batch/myjob -o json`)
 )
 
 func NewCmdEdit(f cmdutil.Factory, out, errOut io.Writer) *cobra.Command {
@@ -113,7 +117,10 @@ func NewCmdEdit(f cmdutil.Factory, out, errOut io.Writer) *cobra.Command {
 	cmdutil.AddFilenameOptionFlags(cmd, options, usage)
 	cmdutil.AddValidateFlags(cmd)
 	cmd.Flags().StringP("output", "o", "yaml", "Output format. One of: yaml|json.")
-	cmd.Flags().String("output-version", "", "Output the formatted object with the given group version (for ex: 'extensions/v1beta1').")
+	cmd.Flags().String("output-version", "", "DEPRECATED: To edit using a specific API version, fully-qualify the resource, version, and group (for example: 'jobs.v1.batch/myjob').")
+	cmd.Flags().MarkDeprecated("output-version", "editing is now done using the resource exactly as fetched from the API. To edit using a specific API version, fully-qualify the resource, version, and group (for example: 'jobs.v1.batch/myjob').")
+	cmd.Flags().MarkHidden("output-version")
+
 	cmd.Flags().Bool("windows-line-endings", gruntime.GOOS == "windows", "Use Windows line-endings (default Unix line-endings)")
 	cmdutil.AddApplyAnnotationFlags(cmd)
 	cmdutil.AddRecordFlag(cmd)
@@ -125,6 +132,10 @@ func RunEdit(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 	return runEdit(f, out, errOut, cmd, args, options, NormalEditMode)
 }
 
+// runEdit performs an interactive edit on the resources specified by filename or resource builder args.
+// in NormalEditMode, all resources are edited as a single list.
+// in CreateEditMode, resources are edited one-by-one.
+// TODO: refactor runEdit and editFn into smaller simpler chunks
 func runEdit(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args []string, options *resource.FilenameOptions, editMode EditMode) error {
 	o, err := getPrinter(cmd)
 	if err != nil {
@@ -136,22 +147,14 @@ func runEdit(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 		return err
 	}
 
-	clientConfig, err := f.ClientConfig()
-	if err != nil {
-		return err
-	}
-
 	encoder := f.JSONEncoder()
-	defaultVersion, err := cmdutil.OutputVersion(cmd, clientConfig.GroupVersion)
-	if err != nil {
-		return err
-	}
 
 	var (
 		windowsLineEndings = cmdutil.GetFlagBool(cmd, "windows-line-endings")
 		edit               = editor.NewDefaultEditor(f.EditorEnvs())
 	)
 
+	// editFn is invoked for each edit session (once with a list for normal edit, once for each individual resource in a edit-on-create invocation)
 	editFn := func(infos []*resource.Info) error {
 		var (
 			results  = editResults{}
@@ -161,13 +164,27 @@ func runEdit(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 		)
 
 		containsError := false
-		for {
-			originalObj, err := resource.AsVersionedObject(infos, false, defaultVersion, encoder)
-			if err != nil {
-				return err
-			}
 
-			objToEdit := originalObj
+		// loop until we succeed or cancel editing
+		for {
+			// get the object we're going to serialize as input to the editor
+			var originalObj runtime.Object
+			switch len(infos) {
+			case 1:
+				originalObj = infos[0].Object
+			default:
+				l := &unstructured.UnstructuredList{
+					Object: map[string]interface{}{
+						"kind":       "List",
+						"apiVersion": "v1",
+						"metadata":   map[string]interface{}{},
+					},
+				}
+				for _, info := range infos {
+					l.Items = append(l.Items, info.Object.(*unstructured.Unstructured))
+				}
+				originalObj = l
+			}
 
 			// generate the file to edit
 			buf := &bytes.Buffer{}
@@ -181,7 +198,7 @@ func runEdit(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 			}
 
 			if !containsError {
-				if err := o.printer.PrintObj(objToEdit, w); err != nil {
+				if err := o.printer.PrintObj(originalObj, w); err != nil {
 					return preservedFile(err, results.file, errOut)
 				}
 				original = buf.Bytes()
@@ -245,39 +262,34 @@ func runEdit(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 			}
 
 			// parse the edited file
-			var updatedVisitor resource.Visitor
-			if updatedInfos, err := updatedResultsGetter(edited).Infos(); err != nil {
+			updatedInfos, err := updatedResultsGetter(edited).Infos()
+			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
-			} else {
-				updatedVisitor = resource.InfoListVisitor(updatedInfos)
 			}
 			// not a syntax error as it turns out...
 			containsError = false
+			updatedVisitor := resource.InfoListVisitor(updatedInfos)
 
-			namespaceVisitor := updatedVisitor
 			// need to make sure the original namespace wasn't changed while editing
-			if err = namespaceVisitor.Visit(resource.RequireNamespace(cmdNamespace)); err != nil {
+			if err := updatedVisitor.Visit(resource.RequireNamespace(cmdNamespace)); err != nil {
 				return preservedFile(err, file, errOut)
 			}
 
 			// iterate through all items to apply annotations
-			annotationVisitor := updatedVisitor
-			if _, err := visitAnnotation(cmd, f, annotationVisitor, encoder); err != nil {
+			if err := visitAnnotation(cmd, f, updatedVisitor, encoder); err != nil {
 				return preservedFile(err, file, errOut)
 			}
 
 			switch editMode {
 			case NormalEditMode:
-				patchVisitor := updatedVisitor
-				err = visitToPatch(originalObj, patchVisitor, mapper, encoder, out, errOut, defaultVersion, &results, file)
+				err = visitToPatch(infos, updatedVisitor, mapper, encoder, out, errOut, &results, file)
 			case EditBeforeCreateMode:
-				createVisitor := updatedVisitor
-				err = visitToCreate(createVisitor, mapper, out, errOut, defaultVersion, &results, file)
+				err = visitToCreate(updatedVisitor, mapper, out, errOut, &results, file)
 			default:
-				err = fmt.Errorf("Not supported edit mode %q", editMode)
+				err = fmt.Errorf("Unsupported edit mode %q", editMode)
 			}
 			if err != nil {
 				return preservedFile(err, results.file, errOut)
@@ -326,7 +338,7 @@ func runEdit(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 			return editFn([]*resource.Info{info})
 		})
 	default:
-		return fmt.Errorf("Not supported edit mode %q", editMode)
+		return fmt.Errorf("Unsupported edit mode %q", editMode)
 	}
 }
 
@@ -352,36 +364,30 @@ func getPrinter(cmd *cobra.Command) (*editPrinterOptions, error) {
 
 type resultGetter func([]byte) *resource.Result
 
+// getMapperAndResult obtains the initial set of resources to edit, and returns:
+// * mapper: restmapper used for printing objects
+// * result: initial set of resources to edit. contains latest versions from the server when in normal editing mode
+// * resultGetter: function that returns a set of resources parsed from user input. used to get resources from edited file.
+// * cmdNamespace: namespace the edit was invoked with. used to verify namespaces don't change during editing.
+// * error: any error that occurs fetching initial resources or building results.
 func getMapperAndResult(f cmdutil.Factory, args []string, options *resource.FilenameOptions, editMode EditMode) (meta.RESTMapper, *resource.Result, resultGetter, string, error) {
+	if editMode != NormalEditMode && editMode != EditBeforeCreateMode {
+		return nil, nil, nil, "", fmt.Errorf("Unsupported edit mode %q", editMode)
+	}
+
 	cmdNamespace, enforceNamespace, err := f.DefaultNamespace()
 	if err != nil {
 		return nil, nil, nil, "", err
 	}
-	var mapper meta.RESTMapper
-	var typer runtime.ObjectTyper
-	switch editMode {
-	case NormalEditMode:
-		mapper, typer = f.Object()
-	case EditBeforeCreateMode:
-		mapper, typer, err = f.UnstructuredObject()
-	default:
-		return nil, nil, nil, "", fmt.Errorf("Not supported edit mode %q", editMode)
-	}
+	mapper, typer, err := f.UnstructuredObject()
 	if err != nil {
 		return nil, nil, nil, "", err
 	}
 
-	// resource builder to read objects from the original file or arg invocation
-	var b *resource.Builder
-	switch editMode {
-	case NormalEditMode:
-		b = resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true)).
-			ResourceTypeOrNameArgs(true, args...).
-			Latest()
-	case EditBeforeCreateMode:
-		b = resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.UnstructuredClientForMapping), unstructured.UnstructuredJSONScheme)
-	default:
-		return nil, nil, nil, "", fmt.Errorf("Not supported edit mode %q", editMode)
+	b := resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.UnstructuredClientForMapping), unstructured.UnstructuredJSONScheme)
+	if editMode == NormalEditMode {
+		// if in normal mode, also read from args, and fetch latest from the server
+		b = b.ResourceTypeOrNameArgs(true, args...).Latest()
 	}
 
 	originalResult := b.NamespaceParam(cmdNamespace).DefaultNamespace().
@@ -395,64 +401,48 @@ func getMapperAndResult(f cmdutil.Factory, args []string, options *resource.File
 	}
 
 	updatedResultGetter := func(data []byte) *resource.Result {
-		// resource builder to read objects from the original file or arg invocation
-		var b *resource.Builder
-		switch editMode {
-		case NormalEditMode:
-			b = resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true))
-		case EditBeforeCreateMode:
-			b = resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.UnstructuredClientForMapping), unstructured.UnstructuredJSONScheme)
-		}
-		return b.Stream(bytes.NewReader(data), "edited-file").ContinueOnError().Flatten().Do()
+		// resource builder to read objects from edited data
+		return resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.UnstructuredClientForMapping), unstructured.UnstructuredJSONScheme).
+			Stream(bytes.NewReader(data), "edited-file").
+			ContinueOnError().
+			Flatten().
+			Do()
 	}
 
 	return mapper, originalResult, updatedResultGetter, cmdNamespace, err
 }
 
 func visitToPatch(
-	originalObj runtime.Object,
+	originalInfos []*resource.Info,
 	patchVisitor resource.Visitor,
 	mapper meta.RESTMapper,
 	encoder runtime.Encoder,
 	out, errOut io.Writer,
-	defaultVersion schema.GroupVersion,
 	results *editResults,
 	file string,
 ) error {
 	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)
-			}
-
+		editObjUID, err := meta.NewAccessor().UID(info.Object)
+		if err != nil {
+			return err
 		}
 
-		originalSerialization, err := runtime.Encode(encoder, currOriginalObj)
+		var originalInfo *resource.Info
+		for _, i := range originalInfos {
+			originalObjUID, err := meta.NewAccessor().UID(i.Object)
+			if err != nil {
+				return err
+			}
+			if editObjUID == originalObjUID {
+				originalInfo = i
+				break
+			}
+		}
+		if originalInfo == nil {
+			return fmt.Errorf("no original object found for %#v", info.Object)
+		}
+
+		originalSerialization, err := runtime.Encode(encoder, originalInfo.Object)
 		if err != nil {
 			return err
 		}
@@ -478,19 +468,47 @@ func visitToPatch(
 			return nil
 		}
 
-		preconditions := []mergepatch.PreconditionFunc{mergepatch.RequireKeyUnchanged("apiVersion"),
-			mergepatch.RequireKeyUnchanged("kind"), mergepatch.RequireMetadataKeyUnchanged("name")}
-		patch, err := strategicpatch.CreateTwoWayMergePatch(originalJS, editedJS, currOriginalObj, preconditions...)
-		if err != nil {
-			glog.V(4).Infof("Unable to calculate diff, no merge is possible: %v", err)
-			if mergepatch.IsPreconditionFailed(err) {
-				return fmt.Errorf("%s", "At least one of apiVersion, kind and name was changed")
-			}
-			return err
+		preconditions := []mergepatch.PreconditionFunc{
+			mergepatch.RequireKeyUnchanged("apiVersion"),
+			mergepatch.RequireKeyUnchanged("kind"),
+			mergepatch.RequireMetadataKeyUnchanged("name"),
 		}
 
-		results.version = defaultVersion
-		patched, err := resource.NewHelper(info.Client, info.Mapping).Patch(info.Namespace, info.Name, types.StrategicMergePatchType, patch)
+		// Create the versioned struct from the type defined in the mapping
+		// (which is the API version we'll be submitting the patch to)
+		versionedObject, err := api.Scheme.New(info.Mapping.GroupVersionKind)
+		var patchType types.PatchType
+		var patch []byte
+		switch {
+		case runtime.IsNotRegisteredError(err):
+			// fall back to generic JSON merge patch
+			patchType = types.MergePatchType
+			patch, err = jsonpatch.CreateMergePatch(originalJS, editedJS)
+			if err != nil {
+				glog.V(4).Infof("Unable to calculate diff, no merge is possible: %v", err)
+				return err
+			}
+			for _, precondition := range preconditions {
+				if !precondition(patch) {
+					glog.V(4).Infof("Unable to calculate diff, no merge is possible: %v", err)
+					return fmt.Errorf("%s", "At least one of apiVersion, kind and name was changed")
+				}
+			}
+		case err != nil:
+			return err
+		default:
+			patchType = types.StrategicMergePatchType
+			patch, err = strategicpatch.CreateTwoWayMergePatch(originalJS, editedJS, versionedObject, preconditions...)
+			if err != nil {
+				glog.V(4).Infof("Unable to calculate diff, no merge is possible: %v", err)
+				if mergepatch.IsPreconditionFailed(err) {
+					return fmt.Errorf("%s", "At least one of apiVersion, kind and name was changed")
+				}
+				return err
+			}
+		}
+
+		patched, err := resource.NewHelper(info.Client, info.Mapping).Patch(info.Namespace, info.Name, patchType, patch)
 		if err != nil {
 			fmt.Fprintln(errOut, results.addError(err, info))
 			return nil
@@ -502,9 +520,8 @@ func visitToPatch(
 	return err
 }
 
-func visitToCreate(createVisitor resource.Visitor, mapper meta.RESTMapper, out, errOut io.Writer, defaultVersion schema.GroupVersion, results *editResults, file string) error {
+func visitToCreate(createVisitor resource.Visitor, mapper meta.RESTMapper, out, errOut io.Writer, results *editResults, file string) error {
 	err := createVisitor.Visit(func(info *resource.Info, incomingErr error) error {
-		results.version = defaultVersion
 		if err := createAndRefresh(info); err != nil {
 			return err
 		}
@@ -514,8 +531,7 @@ func visitToCreate(createVisitor resource.Visitor, mapper meta.RESTMapper, out,
 	return err
 }
 
-func visitAnnotation(cmd *cobra.Command, f cmdutil.Factory, annotationVisitor resource.Visitor, encoder runtime.Encoder) ([]runtime.Object, error) {
-	mutatedObjects := []runtime.Object{}
+func visitAnnotation(cmd *cobra.Command, f cmdutil.Factory, annotationVisitor resource.Visitor, encoder runtime.Encoder) error {
 	// iterate through all items to apply annotations
 	err := annotationVisitor.Visit(func(info *resource.Info, incomingErr error) error {
 		// put configuration annotation in "updates"
@@ -527,12 +543,9 @@ func visitAnnotation(cmd *cobra.Command, f cmdutil.Factory, annotationVisitor re
 				return err
 			}
 		}
-		mutatedObjects = append(mutatedObjects, info.Object)
-
 		return nil
-
 	})
-	return mutatedObjects, err
+	return err
 }
 
 type EditMode string