Better organize object conversion in response handling

Prepare to support watch by cleaning up the conversion method and
splitting out each transition into a smaller method.

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/response.go

@@ -27,6 +27,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/endpoints/handlers/negotiation"
 	"k8s.io/apiserver/pkg/endpoints/handlers/responsewriters"
 )
@@ -34,6 +35,13 @@ import (
 // transformResponseObject takes an object loaded from storage and performs any necessary transformations.
 // Will write the complete response object.
 func transformResponseObject(ctx context.Context, scope RequestScope, req *http.Request, w http.ResponseWriter, statusCode int, mediaType negotiation.MediaTypeOptions, result runtime.Object) {
+	// status objects are ignored for transformation
+	if _, ok := result.(*metav1.Status); ok {
+		responsewriters.WriteObject(statusCode, scope.Kind.GroupVersion(), scope.Serializer, result, w, req)
+		return
+	}
+
+	// ensure the self link and empty list array are set
 	if err := setObjectSelfLink(ctx, result, req, scope.Namer); err != nil {
 		scope.err(err, w, req)
 		return
@@ -42,142 +50,62 @@ func transformResponseObject(ctx context.Context, scope RequestScope, req *http.
 	trace := scope.Trace
 
 	// If conversion was allowed by the scope, perform it before writing the response
-	if target := mediaType.Convert; target != nil {
+	switch target := mediaType.Convert; {
-		switch {
 
-		case target.Kind == "PartialObjectMetadata" && target.GroupVersion() == metav1beta1.SchemeGroupVersion:
+	case target == nil:
-			if meta.IsListType(result) {
+		trace.Step("Writing response")
-				// TODO: this should be calculated earlier
+		responsewriters.WriteObject(statusCode, scope.Kind.GroupVersion(), scope.Serializer, result, w, req)
-				err := newNotAcceptableError(fmt.Sprintf("you requested PartialObjectMetadata, but the requested object is a list (%T)", result))
-				scope.err(err, w, req)
-				return
-			}
-			m, err := meta.Accessor(result)
-			if err != nil {
-				scope.err(err, w, req)
-				return
-			}
-			partial := meta.AsPartialObjectMetadata(m)
-			partial.GetObjectKind().SetGroupVersionKind(metav1beta1.SchemeGroupVersion.WithKind("PartialObjectMetadata"))
 
-			// renegotiate under the internal version
+	case target.Kind == "PartialObjectMetadata" && target.GroupVersion() == metav1beta1.SchemeGroupVersion:
-			_, info, err := negotiation.NegotiateOutputMediaType(req, metainternalversion.Codecs, &scope)
+		partial, err := asV1Beta1PartialObjectMetadata(result)
-			if err != nil {
+		if err != nil {
-				scope.err(err, w, req)
+			scope.err(err, w, req)
-				return
-			}
-			encoder := metainternalversion.Codecs.EncoderForVersion(info.Serializer, metav1beta1.SchemeGroupVersion)
-			trace.Step(fmt.Sprintf("Serializing response as type %s", info.MediaType))
-			responsewriters.SerializeObject(info.MediaType, encoder, w, req, statusCode, partial)
-			return
-
-		case target.Kind == "PartialObjectMetadataList" && target.GroupVersion() == metav1beta1.SchemeGroupVersion:
-			if !meta.IsListType(result) {
-				// TODO: this should be calculated earlier
-				err := newNotAcceptableError(fmt.Sprintf("you requested PartialObjectMetadataList, but the requested object is not a list (%T)", result))
-				scope.err(err, w, req)
-				return
-			}
-			list := &metav1beta1.PartialObjectMetadataList{}
-			trace.Step("Processing list items")
-			err := meta.EachListItem(result, func(obj runtime.Object) error {
-				m, err := meta.Accessor(obj)
-				if err != nil {
-					return err
-				}
-				partial := meta.AsPartialObjectMetadata(m)
-				partial.GetObjectKind().SetGroupVersionKind(metav1beta1.SchemeGroupVersion.WithKind("PartialObjectMetadata"))
-				list.Items = append(list.Items, partial)
-				return nil
-			})
-			if err != nil {
-				scope.err(err, w, req)
-				return
-			}
-
-			// renegotiate under the internal version
-			_, info, err := negotiation.NegotiateOutputMediaType(req, metainternalversion.Codecs, &scope)
-			if err != nil {
-				scope.err(err, w, req)
-				return
-			}
-			encoder := metainternalversion.Codecs.EncoderForVersion(info.Serializer, metav1beta1.SchemeGroupVersion)
-			trace.Step(fmt.Sprintf("Serializing response as type %s", info.MediaType))
-			responsewriters.SerializeObject(info.MediaType, encoder, w, req, statusCode, list)
-			return
-
-		case target.Kind == "Table" && target.GroupVersion() == metav1beta1.SchemeGroupVersion:
-			// TODO: relax the version abstraction
-			// TODO: skip if this is a status response (delete without body)?
-
-			opts := &metav1beta1.TableOptions{}
-			trace.Step("Decoding parameters")
-			if err := metav1beta1.ParameterCodec.DecodeParameters(req.URL.Query(), metav1beta1.SchemeGroupVersion, opts); err != nil {
-				scope.err(err, w, req)
-				return
-			}
-
-			trace.Step("Converting to table")
-			table, err := scope.TableConvertor.ConvertToTable(ctx, result, opts)
-			if err != nil {
-				scope.err(err, w, req)
-				return
-			}
-
-			trace.Step("Processing rows")
-			for i := range table.Rows {
-				item := &table.Rows[i]
-				switch opts.IncludeObject {
-				case metav1beta1.IncludeObject:
-					item.Object.Object, err = scope.Convertor.ConvertToVersion(item.Object.Object, scope.Kind.GroupVersion())
-					if err != nil {
-						scope.err(err, w, req)
-						return
-					}
-				// TODO: rely on defaulting for the value here?
-				case metav1beta1.IncludeMetadata, "":
-					m, err := meta.Accessor(item.Object.Object)
-					if err != nil {
-						scope.err(err, w, req)
-						return
-					}
-					// TODO: turn this into an internal type and do conversion in order to get object kind automatically set?
-					partial := meta.AsPartialObjectMetadata(m)
-					partial.GetObjectKind().SetGroupVersionKind(metav1beta1.SchemeGroupVersion.WithKind("PartialObjectMetadata"))
-					item.Object.Object = partial
-				case metav1beta1.IncludeNone:
-					item.Object.Object = nil
-				default:
-					// TODO: move this to validation on the table options?
-					err = errors.NewBadRequest(fmt.Sprintf("unrecognized includeObject value: %q", opts.IncludeObject))
-					scope.err(err, w, req)
-				}
-			}
-
-			// renegotiate under the internal version
-			_, info, err := negotiation.NegotiateOutputMediaType(req, metainternalversion.Codecs, &scope)
-			if err != nil {
-				scope.err(err, w, req)
-				return
-			}
-			encoder := metainternalversion.Codecs.EncoderForVersion(info.Serializer, metav1beta1.SchemeGroupVersion)
-			trace.Step(fmt.Sprintf("Serializing response as type %s", info.MediaType))
-			responsewriters.SerializeObject(info.MediaType, encoder, w, req, statusCode, table)
-			return
-
-		default:
-			// this block should only be hit if scope AllowsConversion is incorrect
-			accepted, _ := negotiation.MediaTypesForSerializer(metainternalversion.Codecs)
-			err := negotiation.NewNotAcceptableError(accepted)
-			status := responsewriters.ErrorToAPIStatus(err)
-			trace.Step("Writing raw JSON response")
-			responsewriters.WriteRawJSON(int(status.Code), status, w)
 			return
 		}
-	}
 
-	trace.Step("Writing response")
+		if err := writeMetaInternalVersion(partial, statusCode, w, req, &scope, target.GroupVersion()); err != nil {
-	responsewriters.WriteObject(statusCode, scope.Kind.GroupVersion(), scope.Serializer, result, w, req)
+			scope.err(err, w, req)
+			return
+		}
+
+	case target.Kind == "PartialObjectMetadataList" && target.GroupVersion() == metav1beta1.SchemeGroupVersion:
+		trace.Step("Processing list items")
+		partial, err := asV1Beta1PartialObjectMetadataList(result)
+		if err != nil {
+			scope.err(err, w, req)
+			return
+		}
+
+		if err := writeMetaInternalVersion(partial, statusCode, w, req, &scope, target.GroupVersion()); err != nil {
+			scope.err(err, w, req)
+			return
+		}
+
+	case target.Kind == "Table" && target.GroupVersion() == metav1beta1.SchemeGroupVersion:
+		opts := &metav1beta1.TableOptions{}
+		trace.Step("Decoding parameters")
+		if err := metav1beta1.ParameterCodec.DecodeParameters(req.URL.Query(), metav1beta1.SchemeGroupVersion, opts); err != nil {
+			scope.err(err, w, req)
+			return
+		}
+
+		table, err := asV1Beta1Table(ctx, result, opts, scope)
+		if err != nil {
+			scope.err(err, w, req)
+			return
+		}
+
+		if err := writeMetaInternalVersion(table, statusCode, w, req, &scope, target.GroupVersion()); err != nil {
+			scope.err(err, w, req)
+			return
+		}
+
+	default:
+		// this block should only be hit if scope AllowsConversion is incorrect
+		accepted, _ := negotiation.MediaTypesForSerializer(metainternalversion.Codecs)
+		err := negotiation.NewNotAcceptableError(accepted)
+		scope.err(err, w, req)
+	}
 }
 
 // errNotAcceptable indicates Accept negotiation has failed
@@ -201,3 +129,91 @@ func (e errNotAcceptable) Status() metav1.Status {
 		Message: e.Error(),
 	}
 }
+
+func asV1Beta1Table(ctx context.Context, result runtime.Object, opts *metav1beta1.TableOptions, scope RequestScope) (runtime.Object, error) {
+	trace := scope.Trace
+
+	trace.Step("Converting to table")
+	table, err := scope.TableConvertor.ConvertToTable(ctx, result, opts)
+	if err != nil {
+		return nil, err
+	}
+
+	trace.Step("Processing rows")
+	for i := range table.Rows {
+		item := &table.Rows[i]
+		switch opts.IncludeObject {
+		case metav1beta1.IncludeObject:
+			item.Object.Object, err = scope.Convertor.ConvertToVersion(item.Object.Object, scope.Kind.GroupVersion())
+			if err != nil {
+				return nil, err
+			}
+		// TODO: rely on defaulting for the value here?
+		case metav1beta1.IncludeMetadata, "":
+			m, err := meta.Accessor(item.Object.Object)
+			if err != nil {
+				return nil, err
+			}
+			// TODO: turn this into an internal type and do conversion in order to get object kind automatically set?
+			partial := meta.AsPartialObjectMetadata(m)
+			partial.GetObjectKind().SetGroupVersionKind(metav1beta1.SchemeGroupVersion.WithKind("PartialObjectMetadata"))
+			item.Object.Object = partial
+		case metav1beta1.IncludeNone:
+			item.Object.Object = nil
+		default:
+			// TODO: move this to validation on the table options?
+			err = errors.NewBadRequest(fmt.Sprintf("unrecognized includeObject value: %q", opts.IncludeObject))
+			return nil, err
+		}
+	}
+
+	return table, nil
+}
+
+func asV1Beta1PartialObjectMetadata(result runtime.Object) (runtime.Object, error) {
+	if meta.IsListType(result) {
+		// TODO: this should be calculated earlier
+		err := newNotAcceptableError(fmt.Sprintf("you requested PartialObjectMetadata, but the requested object is a list (%T)", result))
+		return nil, err
+	}
+	m, err := meta.Accessor(result)
+	if err != nil {
+		return nil, err
+	}
+	partial := meta.AsPartialObjectMetadata(m)
+	partial.GetObjectKind().SetGroupVersionKind(metav1beta1.SchemeGroupVersion.WithKind("PartialObjectMetadata"))
+	return partial, nil
+}
+
+func asV1Beta1PartialObjectMetadataList(result runtime.Object) (runtime.Object, error) {
+	if !meta.IsListType(result) {
+		// TODO: this should be calculated earlier
+		return nil, newNotAcceptableError(fmt.Sprintf("you requested PartialObjectMetadataList, but the requested object is not a list (%T)", result))
+	}
+	list := &metav1beta1.PartialObjectMetadataList{}
+	err := meta.EachListItem(result, func(obj runtime.Object) error {
+		m, err := meta.Accessor(obj)
+		if err != nil {
+			return err
+		}
+		partial := meta.AsPartialObjectMetadata(m)
+		partial.GetObjectKind().SetGroupVersionKind(metav1beta1.SchemeGroupVersion.WithKind("PartialObjectMetadata"))
+		list.Items = append(list.Items, partial)
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+	return list, nil
+}
+
+func writeMetaInternalVersion(obj runtime.Object, statusCode int, w http.ResponseWriter, req *http.Request, restrictions negotiation.EndpointRestrictions, target schema.GroupVersion) error {
+	// renegotiate under the internal version
+	_, info, err := negotiation.NegotiateOutputMediaType(req, metainternalversion.Codecs, restrictions)
+	if err != nil {
+		return err
+	}
+	encoder := metainternalversion.Codecs.EncoderForVersion(info.Serializer, target)
+	responsewriters.SerializeObject(info.MediaType, encoder, w, req, statusCode, obj)
+	return nil
+}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go

@@ -283,10 +283,6 @@ func checkName(obj runtime.Object, name, namespace string, namer ScopeNamer) err
 // setObjectSelfLink sets the self link of an object as needed.
 func setObjectSelfLink(ctx context.Context, obj runtime.Object, req *http.Request, namer ScopeNamer) error {
 	if !meta.IsListType(obj) {
-		// status needs no self link
-		if _, ok := obj.(*metav1.Status); ok {
-			return nil
-		}
 		requestInfo, ok := request.RequestInfoFrom(ctx)
 		if !ok {
 			return fmt.Errorf("missing requestInfo")