diff --git a/staging/src/k8s.io/apimachinery/pkg/api/meta/help.go b/staging/src/k8s.io/apimachinery/pkg/api/meta/help.go index c70b3d2b6c7..3425055f6ec 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/meta/help.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/meta/help.go @@ -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), diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go index 515d2663592..7774f849e1b 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go @@ -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) } } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go index b8b4cdc2315..e38e1c2972c 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go @@ -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) } } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/get.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/get.go index 0f1c59946a3..f80056c05e2 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/get.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/get.go @@ -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))) } } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index df9c38d165f..8ca8b47f856 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -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) } } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/response.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/response.go index e140c081746..29a15a0f152 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/response.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/response.go @@ -33,17 +33,14 @@ 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, 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) { + if err := setObjectSelfLink(ctx, result, req, scope.Namer); err != nil { + scope.err(err, w, req) return } + trace := scope.Trace + // If conversion was allowed by the scope, perform it before writing the response if target := mediaType.Convert; target != nil { switch { @@ -51,7 +48,7 @@ func transformResponseObject(ctx context.Context, scope RequestScope, req *http. 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)) + err := newNotAcceptableError(fmt.Sprintf("you requested PartialObjectMetadata, but the requested object is a list (%T)", result)) scope.err(err, w, req) return } @@ -77,7 +74,7 @@ func transformResponseObject(ctx context.Context, scope RequestScope, req *http. 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)) + err := newNotAcceptableError(fmt.Sprintf("you requested PartialObjectMetadataList, but the requested object is not a list (%T)", result)) scope.err(err, w, req) return } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go index 56da2271838..1586d9f6586 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go @@ -280,23 +280,30 @@ 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 + // 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") + } + 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 +311,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 { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go index 1bcde7f28b4..ad2fc6ef197 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go @@ -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) } }