Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ensure all response object modification happens in one place #71542

Merged
merged 2 commits into from
Dec 11, 2018
Merged

Ensure all response object modification happens in one place #71542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
56a25d8
Ensure all response object modification happens in one place
smarterclayton Nov 29, 2018
63c49ba
Better organize object conversion in response handling
smarterclayton Nov 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions staging/src/k8s.io/apimachinery/pkg/api/meta/help.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
19 changes: 7 additions & 12 deletions staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,19 @@ 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)
if err != nil {
scope.err(err, w, req)
return
}

decoder := scope.Serializer.DecoderToVersion(s.Serializer, scope.HubGroupVersion)

body, err := readBody(req)
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
}

Expand Down
37 changes: 14 additions & 23 deletions staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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)
}
}
37 changes: 14 additions & 23 deletions staging/src/k8s.io/apiserver/pkg/endpoints/handlers/get.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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")
}
}
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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)))
}
}
19 changes: 7 additions & 12 deletions staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
}

Expand Down