Merge pull request #41304 from liggitt/edit-refactor

Automatic merge from submit-queue

Make kubectl edit work with unstructured objects

Fixes https://github.com/kubernetes/kubernetes/issues/35993


1. First (before any other changes), added several test cases for complex edit scenarios:
   - [x] ensure the edit loop bails out if given the same result that already caused errors
   - [x] ensure an edited file with a syntax error is reopened preserving the input
   - [x] ensure objects with existing "caused-by" annotations get updated with the current command

2. Refactored the edit code to prep for switching to unstructured:
   - [x] made editFn operate on a slice of resource.Info objects passed as an arg, regardless of edit mode
   - [x] simplified short-circuiting logic when re-editing a file containing an error
   - [x] refactored how we build the various visitors (namespace enforcement, annotation application, patching, creating) so we could easily switch to just using a single visitor over a set of resource infos read from the updated input for all of them

3. Switched to using a resource builder to parse the stream of the user's edited output
   - [x] improve the error message you get on syntax errors
   - [x] preserve the user's input more faithfully (see how the captured testcase requests to the server changed to reflect exactly what the user edited)
   - [x] stopped doing client-side conversion (means deprecating `--output-version`)

4. Switched edit to work with generic objects
   - [x] use unstructured objects
   - [x] fall back to generic json merge patch for unrecognized group/version/kinds

5. Added new test cases
   - [x] schemaless objects falls back to generic json merge (covers TPR scenario)
   - [x] edit unknown version of known kind (version "v0" of storageclass) falls back to generic json merge

```release-note
`kubectl edit` now edits objects exactly as they were retrieved from the API. This allows using `kubectl edit` with third-party resources and extension API servers. Because client-side conversion is no longer done, the `--output-version` option is deprecated for `kubectl edit`. To edit using a particular API version, fully-qualify the resource, version, and group used to fetch the object (for example, `job.v1.batch/myjob`)
```

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,39 +132,30 @@ 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 {
 		return err
 	}
 
-	mapper, resourceMapper, r, cmdNamespace, err := getMapperAndResult(f, args, options, editMode)
-	if err != nil {
-		return err
-	}
-
-	clientConfig, err := f.ClientConfig()
+	mapper, originalResult, updatedResultsGetter, cmdNamespace, err := getMapperAndResult(f, args, options, editMode)
 	if err != nil {
 		return err
 	}
 
 	encoder := f.JSONEncoder()
-	defaultVersion, err := cmdutil.OutputVersion(cmd, clientConfig.GroupVersion)
-	if err != nil {
-		return err
-	}
-
-	normalEditInfos, err := r.Infos()
-	if err != nil {
-		return err
-	}
 
 	var (
 		windowsLineEndings = cmdutil.GetFlagBool(cmd, "windows-line-endings")
 		edit               = editor.NewDefaultEditor(f.EditorEnvs())
 	)
 
-	editFn := func(info *resource.Info, err error) error {
+	// 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{}
 			original = []byte{}
@@ -166,22 +164,27 @@ func runEdit(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 		)
 
 		containsError := false
-		var infos []*resource.Info
-		for {
-			switch editMode {
-			case NormalEditMode:
-				infos = normalEditInfos
-			case EditBeforeCreateMode:
-				infos = []*resource.Info{info}
-			default:
-				err = fmt.Errorf("Not supported edit mode %q", editMode)
-			}
-			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{}
@@ -195,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()
@@ -212,17 +215,10 @@ func runEdit(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 			if err != nil {
 				return preservedFile(err, results.file, errOut)
 			}
-			if editMode == NormalEditMode || containsError {
-				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 :)
-					return preservedFile(fmt.Errorf("%s", "Edit cancelled, no valid changes were saved."), file, errOut)
-				}
+			// If we're retrying the loop because of an error, and no change was made in the file, short-circuit
+			if containsError && bytes.Equal(stripComments(editedDiff), stripComments(edited)) {
+				return preservedFile(fmt.Errorf("%s", "Edit cancelled, no valid changes were saved."), file, errOut)
 			}
-
 			// cleanup any file from the previous pass
 			if len(results.file) > 0 {
 				os.Remove(results.file)
@@ -266,7 +262,7 @@ func runEdit(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 			}
 
 			// parse the edited file
-			updates, err := resourceMapper.InfoForData(edited, "edited-file")
+			updatedInfos, err := updatedResultsGetter(edited).Infos()
 			if err != nil {
 				// syntax error
 				containsError = true
@@ -275,31 +271,25 @@ func runEdit(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 			}
 			// not a syntax error as it turns out...
 			containsError = false
+			updatedVisitor := resource.InfoListVisitor(updatedInfos)
 
-			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 {
+			if err := updatedVisitor.Visit(resource.RequireNamespace(cmdNamespace)); err != nil {
 				return preservedFile(err, file, errOut)
 			}
 
 			// iterate through all items to apply annotations
-			mutatedObjects, err := visitAnnotation(cmd, f, updates, resourceMapper, encoder)
-			if err != nil {
+			if err := visitAnnotation(cmd, f, updatedVisitor, encoder); 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)
-			}
-
 			switch editMode {
 			case NormalEditMode:
-				err = visitToPatch(originalObj, updates, mapper, resourceMapper, encoder, out, errOut, defaultVersion, &results, file)
+				err = visitToPatch(infos, updatedVisitor, mapper, encoder, out, errOut, &results, file)
 			case EditBeforeCreateMode:
-				err = visitToCreate(updates, mapper, resourceMapper, 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)
@@ -337,12 +327,18 @@ func runEdit(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 	switch editMode {
 	// If doing normal edit we cannot use Visit because we need to edit a list for convenience. Ref: #20519
 	case NormalEditMode:
-		return editFn(nil, nil)
+		infos, err := originalResult.Infos()
+		if err != nil {
+			return err
+		}
+		return editFn(infos)
 	// If doing an edit before created, we don't want a list and instead want the normal behavior as kubectl create.
 	case EditBeforeCreateMode:
-		return r.Visit(editFn)
+		return originalResult.Visit(func(info *resource.Info, err error) error {
+			return editFn([]*resource.Info{info})
+		})
 	default:
-		return fmt.Errorf("Not supported edit mode %q", editMode)
+		return fmt.Errorf("Unsupported edit mode %q", editMode)
 	}
 }
 
@@ -366,106 +362,87 @@ func getPrinter(cmd *cobra.Command) (*editPrinterOptions, error) {
 	}
 }
 
-func getMapperAndResult(f cmdutil.Factory, args []string, options *resource.FilenameOptions, editMode EditMode) (meta.RESTMapper, *resource.Mapper, *resource.Result, string, 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
 	}
-	resourceMapper := &resource.Mapper{
-		ObjectTyper:  typer,
-		RESTMapper:   mapper,
-		ClientMapper: resource.ClientMapperFunc(f.ClientForMapping),
 
-		// NB: we use `f.Decoder(false)` to get a plain deserializer for
-		// the resourceMapper, since it's used to read in edits and
-		// we don't want to convert into the internal version when
-		// reading in edits (this would cause us to potentially try to
-		// compare two different GroupVersions).
-		Decoder: f.Decoder(false),
+	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()
 	}
-	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)
-	}
-	r := b.NamespaceParam(cmdNamespace).DefaultNamespace().
+
+	originalResult := b.NamespaceParam(cmdNamespace).DefaultNamespace().
 		FilenameParam(enforceNamespace, options).
 		ContinueOnError().
 		Flatten().
 		Do()
-	err = r.Err()
+	err = originalResult.Err()
 	if err != nil {
 		return nil, nil, nil, "", err
 	}
-	return mapper, resourceMapper, r, cmdNamespace, err
+
+	updatedResultGetter := func(data []byte) *resource.Result {
+		// 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,
-	updates *resource.Info,
+	originalInfos []*resource.Info,
+	patchVisitor resource.Visitor,
 	mapper meta.RESTMapper,
-	resourceMapper *resource.Mapper,
 	encoder runtime.Encoder,
 	out, errOut io.Writer,
-	defaultVersion schema.GroupVersion,
 	results *editResults,
 	file string,
 ) error {
-
-	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)
-			}
-
+		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
 		}
@@ -491,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
@@ -515,10 +520,8 @@ func visitToPatch(
 	return err
 }
 
-func visitToCreate(updates *resource.Info, mapper meta.RESTMapper, resourceMapper *resource.Mapper, out, errOut io.Writer, defaultVersion schema.GroupVersion, results *editResults, file string) error {
-	createVisitor := resource.NewFlattenListVisitor(updates, resourceMapper)
+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
 		}
@@ -528,9 +531,7 @@ func visitToCreate(updates *resource.Info, mapper meta.RESTMapper, resourceMappe
 	return err
 }
 
-func visitAnnotation(cmd *cobra.Command, f cmdutil.Factory, updates *resource.Info, resourceMapper *resource.Mapper, encoder runtime.Encoder) ([]runtime.Object, error) {
-	mutatedObjects := []runtime.Object{}
-	annotationVisitor := resource.NewFlattenListVisitor(updates, resourceMapper)
+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"
@@ -542,12 +543,9 @@ func visitAnnotation(cmd *cobra.Command, f cmdutil.Factory, updates *resource.In
 				return err
 			}
 		}
-		mutatedObjects = append(mutatedObjects, info.Object)
-
 		return nil
-
 	})
-	return mutatedObjects, err
+	return err
 }
 
 type EditMode string

pkg/kubectl/cmd/edit_test.go

@@ -205,9 +205,9 @@ func TestEdit(t *testing.T) {
 			t.Fatalf("%s: %v", name, err)
 		}
 
-		f, tf, _, ns := cmdtesting.NewAPIFactory()
+		f, tf, _, _ := cmdtesting.NewAPIFactory()
 		tf.Printer = &testPrinter{}
-		tf.ClientForMappingFunc = func(mapping *meta.RESTMapping) (resource.RESTClient, error) {
+		tf.UnstructuredClientForMappingFunc = func(mapping *meta.RESTMapping) (resource.RESTClient, error) {
 			versionedAPIPath := ""
 			if mapping.GroupVersionKind.Group == "" {
 				versionedAPIPath = "/api/" + mapping.GroupVersionKind.Version
@@ -217,7 +217,7 @@ func TestEdit(t *testing.T) {
 			return &fake.RESTClient{
 				APIRegistry:          api.Registry,
 				VersionedAPIPath:     versionedAPIPath,
-				NegotiatedSerializer: ns, //unstructuredSerializer,
+				NegotiatedSerializer: unstructuredSerializer,
 				Client:               fake.CreateHTTPClient(reqResp),
 			}, nil
 		}
@@ -226,6 +226,7 @@ func TestEdit(t *testing.T) {
 			tf.Namespace = testcase.Namespace
 		}
 		tf.ClientConfig = defaultClientConfig()
+		tf.Command = "edit test cmd invocation"
 		buf := bytes.NewBuffer([]byte{})
 		errBuf := bytes.NewBuffer([]byte{})
 

pkg/kubectl/cmd/testdata/edit/testcase-create-list-error/1.request

@@ -1,31 +1,32 @@
 {
-	"kind": "Service",
 	"apiVersion": "v1",
+	"kind": "Service",
 	"metadata": {
-		"name": "svc1",
-		"namespace": "edit-test",
-		"selfLink": "/api/v1/namespaces/edit-test/services/svc1",
-		"uid": "4149f70e-e9dc-11e6-8c3b-acbc32c1ca87",
 		"creationTimestamp": "2017-02-03T06:44:47Z",
 		"labels": {
 			"app": "svc1modified"
-		}
+		},
+		"name": "svc1",
+		"namespace": "edit-test",
+		"resourceVersion": "",
+		"selfLink": "/api/v1/namespaces/edit-test/services/svc1",
+		"uid": "4149f70e-e9dc-11e6-8c3b-acbc32c1ca87"
 	},
 	"spec": {
+		"clusterIP": "10.0.0.118",
 		"ports": [
 			{
 				"name": "81",
-				"protocol": "TCP",
 				"port": 82,
+				"protocol": "TCP",
 				"targetPort": 81
 			}
 		],
 		"selector": {
 			"app": "svc1"
 		},
-		"clusterIP": "10.0.0.118",
-		"type": "ClusterIP",
-		"sessionAffinity": "None"
+		"sessionAffinity": "None",
+		"type": "ClusterIP"
 	},
 	"status": {
 		"loadBalancer": {}

pkg/kubectl/cmd/testdata/edit/testcase-create-list-error/3.request

@@ -1,31 +1,32 @@
 {
-	"kind": "Service",
 	"apiVersion": "v1",
+	"kind": "Service",
 	"metadata": {
-		"name": "svc2",
-		"namespace": "edit-test",
-		"selfLink": "/api/v1/namespaces/edit-test/services/svc2",
-		"uid": "3e9b10db-e9dc-11e6-8c3b-acbc32c1ca87",
 		"creationTimestamp": "2017-02-03T06:44:43Z",
 		"labels": {
 			"app": "svc2modified"
-		}
+		},
+		"name": "svc2",
+		"namespace": "edit-test",
+		"resourceVersion": "",
+		"selfLink": "/api/v1/namespaces/edit-test/services/svc2",
+		"uid": "3e9b10db-e9dc-11e6-8c3b-acbc32c1ca87"
 	},
 	"spec": {
+		"clusterIP": "10.0.0.182.1",
 		"ports": [
 			{
 				"name": "80",
-				"protocol": "VHF",
 				"port": 80,
+				"protocol": "VHF",
 				"targetPort": 80
 			}
 		],
 		"selector": {
 			"app": "svc2"
 		},
-		"clusterIP": "10.0.0.182.1",
-		"type": "ClusterIP",
-		"sessionAffinity": "None"
+		"sessionAffinity": "None",
+		"type": "ClusterIP"
 	},
 	"status": {
 		"loadBalancer": {}

pkg/kubectl/cmd/testdata/edit/testcase-create-list/1.request

@@ -1,31 +1,27 @@
 {
-	"kind": "Service",
 	"apiVersion": "v1",
+	"kind": "Service",
 	"metadata": {
-		"name": "svc1",
-		"namespace": "edit-test",
-		"creationTimestamp": null,
 		"labels": {
 			"app": "svc1",
 			"new-label": "new-value"
-		}
+		},
+		"name": "svc1",
+		"namespace": "edit-test"
 	},
 	"spec": {
 		"ports": [
 			{
 				"name": "81",
-				"protocol": "TCP",
 				"port": 82,
+				"protocol": "TCP",
 				"targetPort": 81
 			}
 		],
 		"selector": {
 			"app": "svc1"
 		},
-		"type": "ClusterIP",
-		"sessionAffinity": "None"
-	},
-	"status": {
-		"loadBalancer": {}
+		"sessionAffinity": "None",
+		"type": "ClusterIP"
 	}
 }

pkg/kubectl/cmd/testdata/edit/testcase-create-list/3.request

@@ -1,20 +1,19 @@
 {
-	"kind": "Service",
 	"apiVersion": "v1",
+	"kind": "Service",
 	"metadata": {
-		"name": "svc2",
-		"namespace": "edit-test",
-		"creationTimestamp": null,
 		"labels": {
 			"app": "svc2"
-		}
+		},
+		"name": "svc2",
+		"namespace": "edit-test"
 	},
 	"spec": {
 		"ports": [
 			{
 				"name": "80",
-				"protocol": "TCP",
 				"port": 80,
+				"protocol": "TCP",
 				"targetPort": 81
 			}
 		],
@@ -22,10 +21,7 @@
 			"app": "svc2",
 			"new-label": "new-value"
 		},
-		"type": "ClusterIP",
-		"sessionAffinity": "None"
-	},
-	"status": {
-		"loadBalancer": {}
+		"sessionAffinity": "None",
+		"type": "ClusterIP"
 	}
 }

pkg/kubectl/cmd/testdata/edit/testcase-list-record/0.response

@@ -0,0 +1,19 @@
+{
+	"kind": "ConfigMap",
+	"apiVersion": "v1",
+	"metadata": {
+		"name": "cm1",
+		"namespace": "edit-test",
+		"selfLink": "/api/v1/namespaces/edit-test/configmaps/cm1",
+		"uid": "b09bffab-e9d7-11e6-8c3b-acbc32c1ca87",
+		"resourceVersion": "1414",
+		"creationTimestamp": "2017-02-03T06:12:07Z",
+		"annotations":{"kubernetes.io/change-cause":"original creating command a"}
+	},
+	"data": {
+		"baz": "qux",
+		"foo": "changed-value",
+		"new-data": "new-value",
+		"new-data2": "new-value"
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-list-record/1.response

@@ -0,0 +1,33 @@
+{
+	"kind": "Service",
+	"apiVersion": "v1",
+	"metadata": {
+		"name": "svc1",
+		"namespace": "edit-test",
+		"selfLink": "/api/v1/namespaces/edit-test/services/svc1",
+		"uid": "9bec82be-e9d7-11e6-8c3b-acbc32c1ca87",
+		"resourceVersion": "1064",
+		"creationTimestamp": "2017-02-03T06:11:32Z",
+		"annotations":{"kubernetes.io/change-cause":"original creating command b"},
+		"labels": {
+			"app": "svc1",
+			"new-label": "foo"
+		}
+	},
+	"spec": {
+		"ports": [
+			{
+				"name": "80",
+				"protocol": "TCP",
+				"port": 81,
+				"targetPort": 81
+			}
+		],
+		"clusterIP": "10.0.0.248",
+		"type": "ClusterIP",
+		"sessionAffinity": "None"
+	},
+	"status": {
+		"loadBalancer": {}
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-list-record/2.edited

@@ -0,0 +1,51 @@
+# 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
+items:
+- apiVersion: v1
+  data:
+    baz: qux
+    foo: changed-value
+    new-data: new-value
+    new-data2: new-value
+    new-data3: newivalue
+  kind: ConfigMap
+  metadata:
+    annotations:
+      kubernetes.io/change-cause: original creating command a
+    creationTimestamp: 2017-02-03T06:12:07Z
+    name: cm1
+    namespace: edit-test
+    resourceVersion: "1414"
+    selfLink: /api/v1/namespaces/edit-test/configmaps/cm1
+    uid: b09bffab-e9d7-11e6-8c3b-acbc32c1ca87
+- apiVersion: v1
+  kind: Service
+  metadata:
+    annotations:
+      kubernetes.io/change-cause: original creating command b
+    creationTimestamp: 2017-02-03T06:11:32Z
+    labels:
+      app: svc1
+      new-label: foo
+      new-label2: foo2
+    name: svc1
+    namespace: edit-test
+    resourceVersion: "1064"
+    selfLink: /api/v1/namespaces/edit-test/services/svc1
+    uid: 9bec82be-e9d7-11e6-8c3b-acbc32c1ca87
+  spec:
+    clusterIP: 10.0.0.248
+    ports:
+    - name: "80"
+      port: 82
+      protocol: TCP
+      targetPort: 81
+    sessionAffinity: None
+    type: ClusterIP
+  status:
+    loadBalancer: {}
+kind: List
+metadata: {}

pkg/kubectl/cmd/testdata/edit/testcase-list-record/2.original

@@ -0,0 +1,49 @@
+# 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
+items:
+- apiVersion: v1
+  data:
+    baz: qux
+    foo: changed-value
+    new-data: new-value
+    new-data2: new-value
+  kind: ConfigMap
+  metadata:
+    annotations:
+      kubernetes.io/change-cause: original creating command a
+    creationTimestamp: 2017-02-03T06:12:07Z
+    name: cm1
+    namespace: edit-test
+    resourceVersion: "1414"
+    selfLink: /api/v1/namespaces/edit-test/configmaps/cm1
+    uid: b09bffab-e9d7-11e6-8c3b-acbc32c1ca87
+- apiVersion: v1
+  kind: Service
+  metadata:
+    annotations:
+      kubernetes.io/change-cause: original creating command b
+    creationTimestamp: 2017-02-03T06:11:32Z
+    labels:
+      app: svc1
+      new-label: foo
+    name: svc1
+    namespace: edit-test
+    resourceVersion: "1064"
+    selfLink: /api/v1/namespaces/edit-test/services/svc1
+    uid: 9bec82be-e9d7-11e6-8c3b-acbc32c1ca87
+  spec:
+    clusterIP: 10.0.0.248
+    ports:
+    - name: "80"
+      port: 81
+      protocol: TCP
+      targetPort: 81
+    sessionAffinity: None
+    type: ClusterIP
+  status:
+    loadBalancer: {}
+kind: List
+metadata: {}

pkg/kubectl/cmd/testdata/edit/testcase-list-record/3.request

@@ -0,0 +1,10 @@
+{
+	"data": {
+		"new-data3": "newivalue"
+	},
+	"metadata": {
+		"annotations": {
+			"kubernetes.io/change-cause": "edit test cmd invocation"
+		}
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-list-record/3.response

@@ -0,0 +1,20 @@
+{
+	"kind": "ConfigMap",
+	"apiVersion": "v1",
+	"metadata": {
+		"name": "cm1",
+		"namespace": "edit-test",
+		"selfLink": "/api/v1/namespaces/edit-test/configmaps/cm1",
+		"uid": "b09bffab-e9d7-11e6-8c3b-acbc32c1ca87",
+		"resourceVersion": "1465",
+		"creationTimestamp": "2017-02-03T06:12:07Z",
+		"annotations":{"kubernetes.io/change-cause":"edit test cmd invocation"}
+	},
+	"data": {
+		"baz": "qux",
+		"foo": "changed-value",
+		"new-data": "new-value",
+		"new-data2": "new-value",
+		"new-data3": "newivalue"
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-list-record/4.request

@@ -0,0 +1,24 @@
+{
+	"metadata": {
+		"annotations": {
+			"kubernetes.io/change-cause": "edit test cmd invocation"
+		},
+		"labels": {
+			"new-label2": "foo2"
+		}
+	},
+	"spec": {
+		"ports": [
+			{
+				"$patch": "delete",
+				"port": 81
+			},
+			{
+				"name": "80",
+				"port": 82,
+				"protocol": "TCP",
+				"targetPort": 81
+			}
+		]
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-list-record/4.response

@@ -0,0 +1,34 @@
+{
+	"kind": "Service",
+	"apiVersion": "v1",
+	"metadata": {
+		"name": "svc1",
+		"namespace": "edit-test",
+		"selfLink": "/api/v1/namespaces/edit-test/services/svc1",
+		"uid": "9bec82be-e9d7-11e6-8c3b-acbc32c1ca87",
+		"resourceVersion": "1466",
+		"creationTimestamp": "2017-02-03T06:11:32Z",
+		"annotations":{"kubernetes.io/change-cause":"edit test cmd invocation"},
+		"labels": {
+			"app": "svc1",
+			"new-label": "foo",
+			"new-label2": "foo2"
+		}
+	},
+	"spec": {
+		"ports": [
+			{
+				"name": "80",
+				"protocol": "TCP",
+				"port": 82,
+				"targetPort": 81
+			}
+		],
+		"clusterIP": "10.0.0.248",
+		"type": "ClusterIP",
+		"sessionAffinity": "None"
+	},
+	"status": {
+		"loadBalancer": {}
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-list-record/test.yaml

@@ -0,0 +1,40 @@
+description: add a testcase description
+mode: edit
+args:
+- configmaps/cm1
+- service/svc1
+namespace: "edit-test"
+expectedStdout:
+- configmap "cm1" edited
+- service "svc1" edited
+expectedExitCode: 0
+steps:
+- type: request
+  expectedMethod: GET
+  expectedPath: /api/v1/namespaces/edit-test/configmaps/cm1
+  expectedInput: 0.request
+  resultingStatusCode: 200
+  resultingOutput: 0.response
+- type: request
+  expectedMethod: GET
+  expectedPath: /api/v1/namespaces/edit-test/services/svc1
+  expectedInput: 1.request
+  resultingStatusCode: 200
+  resultingOutput: 1.response
+- type: edit
+  expectedInput: 2.original
+  resultingOutput: 2.edited
+- type: request
+  expectedMethod: PATCH
+  expectedPath: /api/v1/namespaces/edit-test/configmaps/cm1
+  expectedContentType: application/strategic-merge-patch+json
+  expectedInput: 3.request
+  resultingStatusCode: 200
+  resultingOutput: 3.response
+- type: request
+  expectedMethod: PATCH
+  expectedPath: /api/v1/namespaces/edit-test/services/svc1
+  expectedContentType: application/strategic-merge-patch+json
+  expectedInput: 4.request
+  resultingStatusCode: 200
+  resultingOutput: 4.response

pkg/kubectl/cmd/testdata/edit/testcase-repeat-error/0.response

@@ -0,0 +1,32 @@
+{
+	"kind": "Service",
+	"apiVersion": "v1",
+	"metadata": {
+		"name": "kubernetes",
+		"namespace": "default",
+		"selfLink": "/api/v1/namespaces/default/services/kubernetes",
+		"uid": "6a8e8829-f15f-11e6-b041-acbc32c1ca87",
+		"resourceVersion": "8",
+		"creationTimestamp": "2017-02-12T20:11:19Z",
+		"labels": {
+			"component": "apiserver",
+			"provider": "kubernetes"
+		}
+	},
+	"spec": {
+		"ports": [
+			{
+				"name": "https",
+				"protocol": "TCP",
+				"port": 443,
+				"targetPort": 443
+			}
+		],
+		"clusterIP": "10.0.0.1",
+		"type": "ClusterIP",
+		"sessionAffinity": "ClientIP"
+	},
+	"status": {
+		"loadBalancer": {}
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-repeat-error/1.edited

@@ -0,0 +1,27 @@
+# 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: Service
+metadata:
+  creationTimestamp: 2017-02-12T20:11:19Z
+  labels:
+    component: apiserver
+    provider: kubernetes
+  name: kubernetes
+  namespace: default
+  resourceVersion: "8"
+  selfLink: /api/v1/namespaces/default/services/kubernetes
+  uid: 6a8e8829-f15f-11e6-b041-acbc32c1ca87
+spec:
+  clusterIP: 10.0.0.1.1
+  ports:
+  - name: https
+    port: 443
+    protocol: TCP
+    targetPort: 443
+  sessionAffinity: ClientIP
+  type: ClusterIP
+status:
+  loadBalancer: {}

pkg/kubectl/cmd/testdata/edit/testcase-repeat-error/1.original

@@ -0,0 +1,27 @@
+# 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: Service
+metadata:
+  creationTimestamp: 2017-02-12T20:11:19Z
+  labels:
+    component: apiserver
+    provider: kubernetes
+  name: kubernetes
+  namespace: default
+  resourceVersion: "8"
+  selfLink: /api/v1/namespaces/default/services/kubernetes
+  uid: 6a8e8829-f15f-11e6-b041-acbc32c1ca87
+spec:
+  clusterIP: 10.0.0.1
+  ports:
+  - name: https
+    port: 443
+    protocol: TCP
+    targetPort: 443
+  sessionAffinity: ClientIP
+  type: ClusterIP
+status:
+  loadBalancer: {}

pkg/kubectl/cmd/testdata/edit/testcase-repeat-error/2.request

@@ -0,0 +1,5 @@
+{
+	"spec": {
+		"clusterIP": "10.0.0.1.1"
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-repeat-error/2.response

@@ -0,0 +1,25 @@
+{
+	"kind": "Status",
+	"apiVersion": "v1",
+	"metadata": {},
+	"status": "Failure",
+	"message": "Service \"kubernetes\" is invalid: [spec.clusterIP: Invalid value: \"10.0.0.1.1\": field is immutable, spec.clusterIP: Invalid value: \"10.0.0.1.1\": must be empty, 'None', or a valid IP address]",
+	"reason": "Invalid",
+	"details": {
+		"name": "kubernetes",
+		"kind": "Service",
+		"causes": [
+			{
+				"reason": "FieldValueInvalid",
+				"message": "Invalid value: \"10.0.0.1.1\": field is immutable",
+				"field": "spec.clusterIP"
+			},
+			{
+				"reason": "FieldValueInvalid",
+				"message": "Invalid value: \"10.0.0.1.1\": must be empty, 'None', or a valid IP address",
+				"field": "spec.clusterIP"
+			}
+		]
+	},
+	"code": 422
+}

pkg/kubectl/cmd/testdata/edit/testcase-repeat-error/3.edited

@@ -0,0 +1,31 @@
+# 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.
+#
+# services "kubernetes" was not valid:
+# * spec.clusterIP: Invalid value: "10.0.0.1.1": field is immutable
+# * spec.clusterIP: Invalid value: "10.0.0.1.1": must be empty, 'None', or a valid IP address
+#
+apiVersion: v1
+kind: Service
+metadata:
+  creationTimestamp: 2017-02-12T20:11:19Z
+  labels:
+    component: apiserver
+    provider: kubernetes
+  name: kubernetes
+  namespace: default
+  resourceVersion: "8"
+  selfLink: /api/v1/namespaces/default/services/kubernetes
+  uid: 6a8e8829-f15f-11e6-b041-acbc32c1ca87
+spec:
+  clusterIP: 10.0.0.1.1
+  ports:
+  - name: https
+    port: 443
+    protocol: TCP
+    targetPort: 443
+  sessionAffinity: ClientIP
+  type: ClusterIP
+status:
+  loadBalancer: {}

pkg/kubectl/cmd/testdata/edit/testcase-repeat-error/3.original

@@ -0,0 +1,31 @@
+# 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.
+#
+# services "kubernetes" was not valid:
+# * spec.clusterIP: Invalid value: "10.0.0.1.1": field is immutable
+# * spec.clusterIP: Invalid value: "10.0.0.1.1": must be empty, 'None', or a valid IP address
+#
+apiVersion: v1
+kind: Service
+metadata:
+  creationTimestamp: 2017-02-12T20:11:19Z
+  labels:
+    component: apiserver
+    provider: kubernetes
+  name: kubernetes
+  namespace: default
+  resourceVersion: "8"
+  selfLink: /api/v1/namespaces/default/services/kubernetes
+  uid: 6a8e8829-f15f-11e6-b041-acbc32c1ca87
+spec:
+  clusterIP: 10.0.0.1.1
+  ports:
+  - name: https
+    port: 443
+    protocol: TCP
+    targetPort: 443
+  sessionAffinity: ClientIP
+  type: ClusterIP
+status:
+  loadBalancer: {}

pkg/kubectl/cmd/testdata/edit/testcase-repeat-error/test.yaml

@@ -0,0 +1,30 @@
+description: add a testcase description
+mode: edit
+args:
+- service/kubernetes
+namespace: default
+expectedStderr:
+- "services \"kubernetes\" is invalid"
+- A copy of your changes has been stored
+- Edit cancelled, no valid changes were saved
+expectedExitCode: 1
+steps:
+- type: request
+  expectedMethod: GET
+  expectedPath: /api/v1/namespaces/default/services/kubernetes
+  expectedInput: 0.request
+  resultingStatusCode: 200
+  resultingOutput: 0.response
+- type: edit
+  expectedInput: 1.original
+  resultingOutput: 1.edited
+- type: request
+  expectedMethod: PATCH
+  expectedPath: /api/v1/namespaces/default/services/kubernetes
+  expectedContentType: application/strategic-merge-patch+json
+  expectedInput: 2.request
+  resultingStatusCode: 422
+  resultingOutput: 2.response
+- type: edit
+  expectedInput: 3.original
+  resultingOutput: 3.edited

pkg/kubectl/cmd/testdata/edit/testcase-schemaless-list/0.response

@@ -0,0 +1,32 @@
+{
+	"kind": "Service",
+	"apiVersion": "v1",
+	"metadata": {
+		"name": "kubernetes",
+		"namespace": "default",
+		"selfLink": "/api/v1/namespaces/default/services/kubernetes",
+		"uid": "6a8e8829-f15f-11e6-b041-acbc32c1ca87",
+		"resourceVersion": "16953",
+		"creationTimestamp": "2017-02-12T20:11:19Z",
+		"labels": {
+			"component": "apiserver",
+			"provider": "kubernetes"
+		}
+	},
+	"spec": {
+		"ports": [
+			{
+				"name": "https",
+				"protocol": "TCP",
+				"port": 443,
+				"targetPort": 443
+			}
+		],
+		"clusterIP": "10.0.0.1",
+		"type": "ClusterIP",
+		"sessionAffinity": "ClientIP"
+	},
+	"status": {
+		"loadBalancer": {}
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-schemaless-list/1.response

@@ -0,0 +1,16 @@
+{
+	"apiVersion": "company.com/v1",
+	"kind": "Bar",
+	"metadata": {
+		"name": "test",
+		"namespace": "default",
+		"selfLink": "/apis/company.com/v1/namespaces/default/bars/test",
+		"uid": "fd16c23d-f185-11e6-b041-acbc32c1ca87",
+		"resourceVersion": "16954",
+		"creationTimestamp": "2017-02-13T00:47:26Z"
+	},
+	"some-field": "field1",
+	"third-field": {
+		"sub-field": "bar2"
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-schemaless-list/2.response

@@ -0,0 +1,21 @@
+{
+	"apiVersion": "company.com/v1",
+	"field1": "value1",
+	"field2": true,
+	"field3": [
+		1
+	],
+	"field4": {
+		"a": true,
+		"b": false
+	},
+	"kind": "Bar",
+	"metadata": {
+		"name": "test2",
+		"namespace": "default",
+		"selfLink": "/apis/company.com/v1/namespaces/default/bars/test2",
+		"uid": "5ef5b446-f186-11e6-b041-acbc32c1ca87",
+		"resourceVersion": "16955",
+		"creationTimestamp": "2017-02-13T00:50:10Z"
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-schemaless-list/3.edited

@@ -0,0 +1,62 @@
+# 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
+items:
+- apiVersion: v1
+  kind: Service
+  metadata:
+    creationTimestamp: 2017-02-12T20:11:19Z
+    labels:
+      component: apiserver
+      provider: kubernetes
+      new-label: new-value
+    name: kubernetes
+    namespace: default
+    resourceVersion: "16953"
+    selfLink: /api/v1/namespaces/default/services/kubernetes
+    uid: 6a8e8829-f15f-11e6-b041-acbc32c1ca87
+  spec:
+    clusterIP: 10.0.0.1
+    ports:
+    - name: https
+      port: 443
+      protocol: TCP
+      targetPort: 443
+    sessionAffinity: ClientIP
+    type: ClusterIP
+  status:
+    loadBalancer: {}
+- apiVersion: company.com/v1
+  kind: Bar
+  metadata:
+    creationTimestamp: 2017-02-13T00:47:26Z
+    name: test
+    namespace: default
+    resourceVersion: "16954"
+    selfLink: /apis/company.com/v1/namespaces/default/bars/test
+    uid: fd16c23d-f185-11e6-b041-acbc32c1ca87
+  some-field: field1
+  other-field: other-value
+  third-field:
+    sub-field: bar2
+- apiVersion: company.com/v1
+  field1: value1
+  field2: true
+  field3:
+  - 1
+  - 2
+  field4:
+    a: true
+    b: false
+  kind: Bar
+  metadata:
+    creationTimestamp: 2017-02-13T00:50:10Z
+    name: test2
+    namespace: default
+    resourceVersion: "16955"
+    selfLink: /apis/company.com/v1/namespaces/default/bars/test2
+    uid: 5ef5b446-f186-11e6-b041-acbc32c1ca87
+kind: List
+metadata: {}

pkg/kubectl/cmd/testdata/edit/testcase-schemaless-list/3.original

@@ -0,0 +1,59 @@
+# 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
+items:
+- apiVersion: v1
+  kind: Service
+  metadata:
+    creationTimestamp: 2017-02-12T20:11:19Z
+    labels:
+      component: apiserver
+      provider: kubernetes
+    name: kubernetes
+    namespace: default
+    resourceVersion: "16953"
+    selfLink: /api/v1/namespaces/default/services/kubernetes
+    uid: 6a8e8829-f15f-11e6-b041-acbc32c1ca87
+  spec:
+    clusterIP: 10.0.0.1
+    ports:
+    - name: https
+      port: 443
+      protocol: TCP
+      targetPort: 443
+    sessionAffinity: ClientIP
+    type: ClusterIP
+  status:
+    loadBalancer: {}
+- apiVersion: company.com/v1
+  kind: Bar
+  metadata:
+    creationTimestamp: 2017-02-13T00:47:26Z
+    name: test
+    namespace: default
+    resourceVersion: "16954"
+    selfLink: /apis/company.com/v1/namespaces/default/bars/test
+    uid: fd16c23d-f185-11e6-b041-acbc32c1ca87
+  some-field: field1
+  third-field:
+    sub-field: bar2
+- apiVersion: company.com/v1
+  field1: value1
+  field2: true
+  field3:
+  - 1
+  field4:
+    a: true
+    b: false
+  kind: Bar
+  metadata:
+    creationTimestamp: 2017-02-13T00:50:10Z
+    name: test2
+    namespace: default
+    resourceVersion: "16955"
+    selfLink: /apis/company.com/v1/namespaces/default/bars/test2
+    uid: 5ef5b446-f186-11e6-b041-acbc32c1ca87
+kind: List
+metadata: {}

pkg/kubectl/cmd/testdata/edit/testcase-schemaless-list/4.request

@@ -0,0 +1,7 @@
+{
+	"metadata": {
+		"labels": {
+			"new-label": "new-value"
+		}
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-schemaless-list/4.response

@@ -0,0 +1,33 @@
+{
+	"kind": "Service",
+	"apiVersion": "v1",
+	"metadata": {
+		"name": "kubernetes",
+		"namespace": "default",
+		"selfLink": "/api/v1/namespaces/default/services/kubernetes",
+		"uid": "6a8e8829-f15f-11e6-b041-acbc32c1ca87",
+		"resourceVersion": "17087",
+		"creationTimestamp": "2017-02-12T20:11:19Z",
+		"labels": {
+			"component": "apiserver",
+			"new-label": "new-value",
+			"provider": "kubernetes"
+		}
+	},
+	"spec": {
+		"ports": [
+			{
+				"name": "https",
+				"protocol": "TCP",
+				"port": 443,
+				"targetPort": 443
+			}
+		],
+		"clusterIP": "10.0.0.1",
+		"type": "ClusterIP",
+		"sessionAffinity": "ClientIP"
+	},
+	"status": {
+		"loadBalancer": {}
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-schemaless-list/5.request

@@ -0,0 +1,3 @@
+{
+	"other-field": "other-value"
+}

pkg/kubectl/cmd/testdata/edit/testcase-schemaless-list/5.response

@@ -0,0 +1,17 @@
+{
+	"apiVersion": "company.com/v1",
+	"kind": "Bar",
+	"metadata": {
+		"name": "test",
+		"namespace": "default",
+		"selfLink": "/apis/company.com/v1/namespaces/default/bars/test",
+		"uid": "fd16c23d-f185-11e6-b041-acbc32c1ca87",
+		"resourceVersion": "17088",
+		"creationTimestamp": "2017-02-13T00:47:26Z"
+	},
+	"other-field": "other-value",
+	"some-field": "field1",
+	"third-field": {
+		"sub-field": "bar2"
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-schemaless-list/6.request

@@ -0,0 +1,6 @@
+{
+	"field3": [
+		1,
+		2
+	]
+}

pkg/kubectl/cmd/testdata/edit/testcase-schemaless-list/6.response

@@ -0,0 +1,22 @@
+{
+	"apiVersion": "company.com/v1",
+	"field1": "value1",
+	"field2": true,
+	"field3": [
+		1,
+		2
+	],
+	"field4": {
+		"a": true,
+		"b": false
+	},
+	"kind": "Bar",
+	"metadata": {
+		"name": "test2",
+		"namespace": "default",
+		"selfLink": "/apis/company.com/v1/namespaces/default/bars/test2",
+		"uid": "5ef5b446-f186-11e6-b041-acbc32c1ca87",
+		"resourceVersion": "17089",
+		"creationTimestamp": "2017-02-13T00:50:10Z"
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-schemaless-list/test.yaml

@@ -0,0 +1,55 @@
+description: edit a mix of schema and schemaless data
+mode: edit
+args:
+- service/kubernetes
+- bars/test
+- bars/test2
+namespace: default
+expectedStdout:
+- "service \"kubernetes\" edited"
+- "bar \"test\" edited"
+- "bar \"test2\" edited"
+expectedExitCode: 0
+steps:
+- type: request
+  expectedMethod: GET
+  expectedPath: /api/v1/namespaces/default/services/kubernetes
+  expectedInput: 0.request
+  resultingStatusCode: 200
+  resultingOutput: 0.response
+- type: request
+  expectedMethod: GET
+  expectedPath: /apis/company.com/v1/namespaces/default/bars/test
+  expectedInput: 1.request
+  resultingStatusCode: 200
+  resultingOutput: 1.response
+- type: request
+  expectedMethod: GET
+  expectedPath: /apis/company.com/v1/namespaces/default/bars/test2
+  expectedInput: 2.request
+  resultingStatusCode: 200
+  resultingOutput: 2.response
+- type: edit
+  expectedInput: 3.original
+  resultingOutput: 3.edited
+- type: request
+  expectedMethod: PATCH
+  expectedPath: /api/v1/namespaces/default/services/kubernetes
+  expectedContentType: application/strategic-merge-patch+json
+  expectedInput: 4.request
+  resultingStatusCode: 200
+  resultingOutput: 4.response
+- type: request
+  expectedMethod: PATCH
+  expectedPath: /apis/company.com/v1/namespaces/default/bars/test
+  expectedContentType: application/merge-patch+json
+  expectedInput: 5.request
+  resultingStatusCode: 200
+  resultingOutput: 5.response
+- type: request
+  expectedMethod: PATCH
+  expectedPath: /apis/company.com/v1/namespaces/default/bars/test2
+  expectedContentType: application/merge-patch+json
+  expectedInput: 6.request
+  resultingStatusCode: 200
+  resultingOutput: 6.response

pkg/kubectl/cmd/testdata/edit/testcase-syntax-error/0.response

@@ -0,0 +1,32 @@
+{
+	"kind": "Service",
+	"apiVersion": "v1",
+	"metadata": {
+		"name": "kubernetes",
+		"namespace": "default",
+		"selfLink": "/api/v1/namespaces/default/services/kubernetes",
+		"uid": "6a8e8829-f15f-11e6-b041-acbc32c1ca87",
+		"resourceVersion": "8",
+		"creationTimestamp": "2017-02-12T20:11:19Z",
+		"labels": {
+			"component": "apiserver",
+			"provider": "kubernetes"
+		}
+	},
+	"spec": {
+		"ports": [
+			{
+				"name": "https",
+				"protocol": "TCP",
+				"port": 443,
+				"targetPort": 443
+			}
+		],
+		"clusterIP": "10.0.0.1",
+		"type": "ClusterIP",
+		"sessionAffinity": "ClientIP"
+	},
+	"status": {
+		"loadBalancer": {}
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-syntax-error/1.edited

@@ -0,0 +1,27 @@
+# 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: Service
+metadata:
+  creationTimestamp: 2017-02-12T20:11:19Z
+  labels:
+    component: apiserver
+    provider: kubernetes
+  name: kubernetes
+  namespace: default
+  resourceVersion: "8"
+  selfLink: /api/v1/namespaces/default/services/kubernetes
+  uid: 6a8e8829-f15f-11e6-b041-acbc32c1ca87
+spec
+  clusterIP: 10.0.0.1
+  ports:
+    name: https
+    port: 443
+    protocol: TCP
+    targetPort: 443
+  sessionAffinity: ClientIP
+  type: ClusterIP
+status:
+  loadBalancer: {

pkg/kubectl/cmd/testdata/edit/testcase-syntax-error/1.original

@@ -0,0 +1,27 @@
+# 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: Service
+metadata:
+  creationTimestamp: 2017-02-12T20:11:19Z
+  labels:
+    component: apiserver
+    provider: kubernetes
+  name: kubernetes
+  namespace: default
+  resourceVersion: "8"
+  selfLink: /api/v1/namespaces/default/services/kubernetes
+  uid: 6a8e8829-f15f-11e6-b041-acbc32c1ca87
+spec:
+  clusterIP: 10.0.0.1
+  ports:
+  - name: https
+    port: 443
+    protocol: TCP
+    targetPort: 443
+  sessionAffinity: ClientIP
+  type: ClusterIP
+status:
+  loadBalancer: {}

pkg/kubectl/cmd/testdata/edit/testcase-syntax-error/2.edited

@@ -0,0 +1,30 @@
+# 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: error converting YAML to JSON: yaml: line 17: could not find expected ':'
+#
+apiVersion: v1
+kind: Service
+metadata:
+  creationTimestamp: 2017-02-12T20:11:19Z
+  labels:
+    component: apiserver
+    provider: kubernetes
+    new-label: foo
+  name: kubernetes
+  namespace: default
+  resourceVersion: "8"
+  selfLink: /api/v1/namespaces/default/services/kubernetes
+  uid: 6a8e8829-f15f-11e6-b041-acbc32c1ca87
+spec:
+  clusterIP: 10.0.0.1
+  ports:
+  - name: https
+    port: 443
+    protocol: TCP
+    targetPort: 443
+  sessionAffinity: ClientIP
+  type: ClusterIP
+status:
+  loadBalancer: {}

pkg/kubectl/cmd/testdata/edit/testcase-syntax-error/2.original

@@ -0,0 +1,29 @@
+# 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: error converting YAML to JSON: yaml: line 17: could not find expected ':'
+#
+apiVersion: v1
+kind: Service
+metadata:
+  creationTimestamp: 2017-02-12T20:11:19Z
+  labels:
+    component: apiserver
+    provider: kubernetes
+  name: kubernetes
+  namespace: default
+  resourceVersion: "8"
+  selfLink: /api/v1/namespaces/default/services/kubernetes
+  uid: 6a8e8829-f15f-11e6-b041-acbc32c1ca87
+spec
+  clusterIP: 10.0.0.1
+  ports:
+    name: https
+    port: 443
+    protocol: TCP
+    targetPort: 443
+  sessionAffinity: ClientIP
+  type: ClusterIP
+status:
+  loadBalancer: {

pkg/kubectl/cmd/testdata/edit/testcase-syntax-error/3.request

@@ -0,0 +1,7 @@
+{
+	"metadata": {
+		"labels": {
+			"new-label": "foo"
+		}
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-syntax-error/3.response

@@ -0,0 +1,33 @@
+{
+	"kind": "Service",
+	"apiVersion": "v1",
+	"metadata": {
+		"name": "kubernetes",
+		"namespace": "default",
+		"selfLink": "/api/v1/namespaces/default/services/kubernetes",
+		"uid": "6a8e8829-f15f-11e6-b041-acbc32c1ca87",
+		"resourceVersion": "1174",
+		"creationTimestamp": "2017-02-12T20:11:19Z",
+		"labels": {
+			"component": "apiserver",
+			"new-label": "foo",
+			"provider": "kubernetes"
+		}
+	},
+	"spec": {
+		"ports": [
+			{
+				"name": "https",
+				"protocol": "TCP",
+				"port": 443,
+				"targetPort": 443
+			}
+		],
+		"clusterIP": "10.0.0.1",
+		"type": "ClusterIP",
+		"sessionAffinity": "ClientIP"
+	},
+	"status": {
+		"loadBalancer": {}
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-syntax-error/test.yaml

@@ -0,0 +1,28 @@
+description: edit with a syntax error, then re-edit and save
+mode: edit
+args:
+- service/kubernetes
+namespace: default
+expectedStdout:
+- "service \"kubernetes\" edited"
+expectedExitCode: 0
+steps:
+- type: request
+  expectedMethod: GET
+  expectedPath: /api/v1/namespaces/default/services/kubernetes
+  expectedInput: 0.request
+  resultingStatusCode: 200
+  resultingOutput: 0.response
+- type: edit
+  expectedInput: 1.original
+  resultingOutput: 1.edited
+- type: edit
+  expectedInput: 2.original
+  resultingOutput: 2.edited
+- type: request
+  expectedMethod: PATCH
+  expectedPath: /api/v1/namespaces/default/services/kubernetes
+  expectedContentType: application/strategic-merge-patch+json
+  expectedInput: 3.request
+  resultingStatusCode: 200
+  resultingOutput: 3.response

pkg/kubectl/cmd/testdata/edit/testcase-unknown-version-known-group-kind/0.response

@@ -0,0 +1,22 @@
+{
+	"kind": "StorageClass",
+	"apiVersion": "storage.k8s.io/v0",
+	"metadata": {
+		"name": "foo",
+		"selfLink": "/apis/storage.k8s.io/v0/storageclassesfoo",
+		"uid": "b2287558-f190-11e6-b041-acbc32c1ca87",
+		"resourceVersion": "21388",
+		"creationTimestamp": "2017-02-13T02:04:04Z",
+		"labels": {
+			"label1": "value1"
+		}
+	},
+	"provisioner": "foo",
+	"parameters": {
+		"baz": "qux",
+		"foo": "bar"
+	},
+	"extraField": {
+		"otherData": true
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-unknown-version-known-group-kind/1.edited

@@ -0,0 +1,22 @@
+# 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: storage.k8s.io/v0
+extraField:
+  otherData: true
+  addedData: "foo"
+kind: StorageClass
+metadata:
+  creationTimestamp: 2017-02-13T02:04:04Z
+  labels:
+    label1: value1
+    label2: value2
+  name: foo
+  resourceVersion: "21388"
+  selfLink: /apis/storage.k8s.io/v0/storageclassesfoo
+  uid: b2287558-f190-11e6-b041-acbc32c1ca87
+parameters:
+  baz: qux
+  foo: bar
+provisioner: foo

pkg/kubectl/cmd/testdata/edit/testcase-unknown-version-known-group-kind/1.original

@@ -0,0 +1,20 @@
+# 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: storage.k8s.io/v0
+extraField:
+  otherData: true
+kind: StorageClass
+metadata:
+  creationTimestamp: 2017-02-13T02:04:04Z
+  labels:
+    label1: value1
+  name: foo
+  resourceVersion: "21388"
+  selfLink: /apis/storage.k8s.io/v0/storageclassesfoo
+  uid: b2287558-f190-11e6-b041-acbc32c1ca87
+parameters:
+  baz: qux
+  foo: bar
+provisioner: foo

pkg/kubectl/cmd/testdata/edit/testcase-unknown-version-known-group-kind/2.request

@@ -0,0 +1,11 @@
+{
+	"extraField": {
+		"addedData": "foo"
+	},
+	"metadata": {
+		"labels": {
+			"label2": "value2"
+		},
+		"namespace": ""
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-unknown-version-known-group-kind/2.response

@@ -0,0 +1,24 @@
+{
+	"kind": "StorageClass",
+	"apiVersion": "storage.k8s.io/v0",
+	"metadata": {
+		"name": "foo",
+		"selfLink": "/apis/storage.k8s.io/v0/storageclassesfoo",
+		"uid": "b2287558-f190-11e6-b041-acbc32c1ca87",
+		"resourceVersion": "21431",
+		"creationTimestamp": "2017-02-13T02:04:04Z",
+		"labels": {
+			"label1": "value1",
+			"label2": "value2"
+		}
+	},
+	"provisioner": "foo",
+	"parameters": {
+		"baz": "qux",
+		"foo": "bar"
+	},
+	"extraField": {
+		"otherData": true,
+		"addedData": true
+	}
+}

pkg/kubectl/cmd/testdata/edit/testcase-unknown-version-known-group-kind/test.yaml

@@ -0,0 +1,25 @@
+description: edit an unknown version of a known group/kind
+mode: edit
+args:
+- storageclasses.v0.storage.k8s.io/foo
+namespace: default
+expectedStdout:
+- "storageclass \"foo\" edited"
+expectedExitCode: 0
+steps:
+- type: request
+  expectedMethod: GET
+  expectedPath: /apis/storage.k8s.io/v0/storageclasses/foo
+  expectedInput: 0.request
+  resultingStatusCode: 200
+  resultingOutput: 0.response
+- type: edit
+  expectedInput: 1.original
+  resultingOutput: 1.edited
+- type: request
+  expectedMethod: PATCH
+  expectedPath: /apis/storage.k8s.io/v0/storageclasses/foo
+  expectedContentType: application/merge-patch+json
+  expectedInput: 2.request
+  resultingStatusCode: 200
+  resultingOutput: 2.response

pkg/kubectl/cmd/testing/fake.go

@@ -216,6 +216,7 @@ type TestFactory struct {
 	Namespace          string
 	ClientConfig       *restclient.Config
 	Err                error
+	Command            string
 
 	ClientForMappingFunc             func(mapping *meta.RESTMapping) (resource.RESTClient, error)
 	UnstructuredClientForMappingFunc func(mapping *meta.RESTMapping) (resource.RESTClient, error)
@@ -431,7 +432,7 @@ func (f *FakeFactory) PrintObjectSpecificMessage(obj runtime.Object, out io.Writ
 }
 
 func (f *FakeFactory) Command() string {
-	return ""
+	return f.tf.Command
 }
 
 func (f *FakeFactory) BindFlags(flags *pflag.FlagSet) {
@@ -630,6 +631,10 @@ func (f *fakeAPIFactory) DefaultNamespace() (string, bool, error) {
 	return f.tf.Namespace, false, f.tf.Err
 }
 
+func (f *fakeAPIFactory) Command() string {
+	return f.tf.Command
+}
+
 func (f *fakeAPIFactory) Generators(cmdName string) map[string]kubectl.Generator {
 	return cmdutil.DefaultGenerators(cmdName)
 }
@@ -714,5 +719,38 @@ func testDynamicResources() []*discovery.APIGroupResources {
 				},
 			},
 		},
+		{
+			Group: metav1.APIGroup{
+				Name: "storage.k8s.io",
+				Versions: []metav1.GroupVersionForDiscovery{
+					{Version: "v1beta1"},
+					{Version: "v0"},
+				},
+				PreferredVersion: metav1.GroupVersionForDiscovery{Version: "v1beta1"},
+			},
+			VersionedResources: map[string][]metav1.APIResource{
+				"v1beta1": {
+					{Name: "storageclasses", Namespaced: false, Kind: "StorageClass"},
+				},
+				// bogus version of a known group/version/resource to make sure kubectl falls back to generic object mode
+				"v0": {
+					{Name: "storageclasses", Namespaced: false, Kind: "StorageClass"},
+				},
+			},
+		},
+		{
+			Group: metav1.APIGroup{
+				Name: "company.com",
+				Versions: []metav1.GroupVersionForDiscovery{
+					{Version: "v1"},
+				},
+				PreferredVersion: metav1.GroupVersionForDiscovery{Version: "v1"},
+			},
+			VersionedResources: map[string][]metav1.APIResource{
+				"v1": {
+					{Name: "bars", Namespaced: true, Kind: "Bar"},
+				},
+			},
+		},
 	}
 }

pkg/kubectl/resource/visitor.go

@@ -695,3 +695,13 @@ func FilterBySelector(s labels.Selector) FilterFunc {
 		return true, nil
 	}
 }
+
+type InfoListVisitor []*Info
+
+func (infos InfoListVisitor) Visit(fn VisitorFunc) error {
+	var err error
+	for _, i := range infos {
+		err = fn(i, err)
+	}
+	return err
+}