Merge pull request #71542 from smarterclayton/endpoints_cleanup

Ensure all response object modification happens in one place
This commit is contained in:
Kubernetes Prow Robot 2018-12-10 18:44:37 -08:00 committed by GitHub
commit 3ed107ff0e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 235 additions and 232 deletions

View File

@ -158,6 +158,19 @@ func ExtractList(obj runtime.Object) ([]runtime.Object, error) {
// objectSliceType is the type of a slice of Objects
var objectSliceType = reflect.TypeOf([]runtime.Object{})
// LenList returns the length of this list or 0 if it is not a list.
func LenList(list runtime.Object) int {
itemsPtr, err := GetItemsPtr(list)
if err != nil {
return 0
}
items, err := conversion.EnforcePtr(itemsPtr)
if err != nil {
return 0
}
return items.Len()
}
// SetList sets the given list object's Items member have the elements given in
// objects.
// Returns an error if list is not a List type (does not have an Items member),

View File

@ -70,6 +70,11 @@ func createHandler(r rest.NamedCreater, scope RequestScope, admit admission.Inte
ctx := req.Context()
ctx = request.WithNamespace(ctx, namespace)
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, &scope)
if err != nil {
scope.err(err, w, req)
return
}
gv := scope.Kind.GroupVersion()
s, err := negotiation.NegotiateInputSerializer(req, false, scope.Serializer)
@ -77,6 +82,7 @@ func createHandler(r rest.NamedCreater, scope RequestScope, admit admission.Inte
scope.err(err, w, req)
return
}
decoder := scope.Serializer.DecoderToVersion(s.Serializer, scope.HubGroupVersion)
body, err := readBody(req)
@ -144,17 +150,6 @@ func createHandler(r rest.NamedCreater, scope RequestScope, admit admission.Inte
}
trace.Step("Object stored in database")
requestInfo, ok := request.RequestInfoFrom(ctx)
if !ok {
scope.err(fmt.Errorf("missing requestInfo"), w, req)
return
}
if err := setSelfLink(result, requestInfo, scope.Namer); err != nil {
scope.err(err, w, req)
return
}
trace.Step("Self-link added")
// If the object is partially initialized, always indicate it via StatusAccepted
code := http.StatusCreated
if accessor, err := meta.Accessor(result); err == nil {
@ -168,7 +163,7 @@ func createHandler(r rest.NamedCreater, scope RequestScope, admit admission.Inte
}
scope.Trace = trace
transformResponseObject(ctx, scope, req, w, code, result)
transformResponseObject(ctx, scope, req, w, code, outputMediaType, result)
}
}

View File

@ -64,6 +64,12 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestSco
ae := request.AuditEventFrom(ctx)
admit = admission.WithAudit(admit, ae)
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, &scope)
if err != nil {
scope.err(err, w, req)
return
}
options := &metav1.DeleteOptions{}
if allowsOptions {
body, err := readBody(req)
@ -160,23 +166,10 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestSco
Kind: scope.Kind.Kind,
},
}
} else {
// when a non-status response is returned, set the self link
requestInfo, ok := request.RequestInfoFrom(ctx)
if !ok {
scope.err(fmt.Errorf("missing requestInfo"), w, req)
return
}
if _, ok := result.(*metav1.Status); !ok {
if err := setSelfLink(result, requestInfo, scope.Namer); err != nil {
scope.err(err, w, req)
return
}
}
}
scope.Trace = trace
transformResponseObject(ctx, scope, req, w, status, result)
transformResponseObject(ctx, scope, req, w, status, outputMediaType, result)
}
}
@ -204,6 +197,12 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestSco
ctx = request.WithNamespace(ctx, namespace)
ae := request.AuditEventFrom(ctx)
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, &scope)
if err != nil {
scope.err(err, w, req)
return
}
listOptions := metainternalversion.ListOptions{}
if err := metainternalversion.ParameterCodec.DecodeParameters(req.URL.Query(), scope.MetaGroupVersion, &listOptions); err != nil {
err = errors.NewBadRequest(err.Error())
@ -304,17 +303,9 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestSco
Kind: scope.Kind.Kind,
},
}
} else {
// when a non-status response is returned, set the self link
if _, ok := result.(*metav1.Status); !ok {
if _, err := setListSelfLink(result, ctx, req, scope.Namer); err != nil {
scope.err(err, w, req)
return
}
}
}
scope.Trace = trace
transformResponseObject(ctx, scope, req, w, http.StatusOK, result)
transformResponseObject(ctx, scope, req, w, http.StatusOK, outputMediaType, result)
}
}

View File

@ -33,6 +33,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apiserver/pkg/endpoints/handlers/negotiation"
"k8s.io/apiserver/pkg/endpoints/metrics"
"k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/rest"
@ -58,24 +59,21 @@ func getResourceHandler(scope RequestScope, getter getterFunc) http.HandlerFunc
ctx := req.Context()
ctx = request.WithNamespace(ctx, namespace)
result, err := getter(ctx, name, req, trace)
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, &scope)
if err != nil {
scope.err(err, w, req)
return
}
requestInfo, ok := request.RequestInfoFrom(ctx)
if !ok {
scope.err(fmt.Errorf("missing requestInfo"), w, req)
return
}
if err := setSelfLink(result, requestInfo, scope.Namer); err != nil {
result, err := getter(ctx, name, req, trace)
if err != nil {
scope.err(err, w, req)
return
}
trace.Step("About to write a response")
scope.Trace = trace
transformResponseObject(ctx, scope, req, w, http.StatusOK, result)
transformResponseObject(ctx, scope, req, w, http.StatusOK, outputMediaType, result)
trace.Step("Transformed response object")
}
}
@ -187,6 +185,12 @@ func ListResource(r rest.Lister, rw rest.Watcher, scope RequestScope, forceWatch
ctx := req.Context()
ctx = request.WithNamespace(ctx, namespace)
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, &scope)
if err != nil {
scope.err(err, w, req)
return
}
opts := metainternalversion.ListOptions{}
if err := metainternalversion.ParameterCodec.DecodeParameters(req.URL.Query(), scope.MetaGroupVersion, &opts); err != nil {
err = errors.NewBadRequest(err.Error())
@ -267,22 +271,9 @@ func ListResource(r rest.Lister, rw rest.Watcher, scope RequestScope, forceWatch
return
}
trace.Step("Listing from storage done")
numberOfItems, err := setListSelfLink(result, ctx, req, scope.Namer)
if err != nil {
scope.err(err, w, req)
return
}
trace.Step("Self-linking done")
// Ensure empty lists return a non-nil items slice
if numberOfItems == 0 && meta.IsListType(result) {
if err := meta.SetList(result, []runtime.Object{}); err != nil {
scope.err(err, w, req)
return
}
}
scope.Trace = trace
transformResponseObject(ctx, scope, req, w, http.StatusOK, result)
trace.Step(fmt.Sprintf("Writing http response done (%d items)", numberOfItems))
transformResponseObject(ctx, scope, req, w, http.StatusOK, outputMediaType, result)
trace.Step(fmt.Sprintf("Writing http response done (%d items)", meta.LenList(result)))
}
}

View File

@ -88,6 +88,12 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface
ctx := req.Context()
ctx = request.WithNamespace(ctx, namespace)
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, &scope)
if err != nil {
scope.err(err, w, req)
return
}
patchJS, err := readBody(req)
if err != nil {
scope.err(err, w, req)
@ -190,19 +196,8 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface
}
trace.Step("Object stored in database")
requestInfo, ok := request.RequestInfoFrom(ctx)
if !ok {
scope.err(fmt.Errorf("missing requestInfo"), w, req)
return
}
if err := setSelfLink(result, requestInfo, scope.Namer); err != nil {
scope.err(err, w, req)
return
}
trace.Step("Self-link added")
scope.Trace = trace
transformResponseObject(ctx, scope, req, w, http.StatusOK, result)
transformResponseObject(ctx, scope, req, w, http.StatusOK, outputMediaType, result)
}
}

View File

@ -27,160 +27,85 @@ 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"
)
// 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, result runtime.Object) {
// TODO: fetch the media type much earlier in request processing and pass it into this method.
trace := scope.Trace
mediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, &scope)
if err != nil {
status := responsewriters.ErrorToAPIStatus(err)
trace.Step("Writing raw JSON response")
responsewriters.WriteRawJSON(int(status.Code), status, w)
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
}
// If conversion was allowed by the scope, perform it before writing the response
if target := mediaType.Convert; target != nil {
switch {
case target.Kind == "PartialObjectMetadata" && target.GroupVersion() == metav1beta1.SchemeGroupVersion:
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))
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
_, 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, 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
}
// 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
}
trace.Step("Writing response")
responsewriters.WriteObject(statusCode, scope.Kind.GroupVersion(), scope.Serializer, result, w, req)
trace := scope.Trace
// If conversion was allowed by the scope, perform it before writing the response
switch target := mediaType.Convert; {
case target == nil:
trace.Step("Writing response")
responsewriters.WriteObject(statusCode, scope.Kind.GroupVersion(), scope.Serializer, result, w, req)
case target.Kind == "PartialObjectMetadata" && target.GroupVersion() == metav1beta1.SchemeGroupVersion:
partial, err := asV1Beta1PartialObjectMetadata(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 == "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
@ -204,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
}

View File

@ -280,23 +280,26 @@ func checkName(obj runtime.Object, name, namespace string, namer ScopeNamer) err
return nil
}
// setListSelfLink sets the self link of a list to the base URL, then sets the self links
// on all child objects returned. Returns the number of items in the list.
func setListSelfLink(obj runtime.Object, ctx context.Context, req *http.Request, namer ScopeNamer) (int, error) {
// 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) {
return 0, nil
requestInfo, ok := request.RequestInfoFrom(ctx)
if !ok {
return fmt.Errorf("missing requestInfo")
}
return setSelfLink(obj, requestInfo, namer)
}
uri, err := namer.GenerateListLink(req)
if err != nil {
return 0, err
return err
}
if err := namer.SetSelfLink(obj, uri); err != nil {
klog.V(4).Infof("Unable to set self link on object: %v", err)
}
requestInfo, ok := request.RequestInfoFrom(ctx)
if !ok {
return 0, fmt.Errorf("missing requestInfo")
return fmt.Errorf("missing requestInfo")
}
count := 0
@ -304,7 +307,14 @@ func setListSelfLink(obj runtime.Object, ctx context.Context, req *http.Request,
count++
return setSelfLink(obj, requestInfo, namer)
})
return count, err
if count == 0 {
if err := meta.SetList(obj, []runtime.Object{}); err != nil {
return err
}
}
return err
}
func summarizeData(data []byte, maxLength int) string {

View File

@ -64,6 +64,12 @@ func UpdateResource(r rest.Updater, scope RequestScope, admit admission.Interfac
ctx := req.Context()
ctx = request.WithNamespace(ctx, namespace)
outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, &scope)
if err != nil {
scope.err(err, w, req)
return
}
body, err := readBody(req)
if err != nil {
scope.err(err, w, req)
@ -174,24 +180,13 @@ func UpdateResource(r rest.Updater, scope RequestScope, admit admission.Interfac
}
trace.Step("Object stored in database")
requestInfo, ok := request.RequestInfoFrom(ctx)
if !ok {
scope.err(fmt.Errorf("missing requestInfo"), w, req)
return
}
if err := setSelfLink(result, requestInfo, scope.Namer); err != nil {
scope.err(err, w, req)
return
}
trace.Step("Self-link added")
status := http.StatusOK
if wasCreated {
status = http.StatusCreated
}
scope.Trace = trace
transformResponseObject(ctx, scope, req, w, status, result)
transformResponseObject(ctx, scope, req, w, status, outputMediaType, result)
}
}