From 25f0fe1adb199697565487b9dfacc4ed8ecdccbb Mon Sep 17 00:00:00 2001 From: Wojciech Tyczynski Date: Mon, 22 May 2017 16:50:24 +0200 Subject: [PATCH 1/2] Pass Context to GenerateLink --- .../apiserver/pkg/endpoints/handlers/namer.go | 6 ++--- .../apiserver/pkg/endpoints/handlers/rest.go | 22 +++++++++---------- .../pkg/endpoints/handlers/rest_test.go | 4 ++-- .../apiserver/pkg/endpoints/handlers/watch.go | 11 +++++++++- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer.go index 019e37e25e..88c0dc44c1 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer.go @@ -45,7 +45,7 @@ type ScopeNamer interface { SetSelfLink(obj runtime.Object, url string) error // GenerateLink creates an encoded URI for a given runtime object that represents the canonical path // and query. - GenerateLink(req *http.Request, obj runtime.Object) (uri string, err error) + GenerateLink(ctx request.Context, obj runtime.Object) (uri string, err error) // GenerateListLink creates an encoded URI for a list that represents the canonical path and query. GenerateListLink(req *http.Request) (uri string, err error) } @@ -90,8 +90,8 @@ func (n ContextBasedNaming) Name(req *http.Request) (namespace, name string, err return ns, requestInfo.Name, nil } -func (n ContextBasedNaming) GenerateLink(req *http.Request, obj runtime.Object) (uri string, err error) { - requestInfo, ok := request.RequestInfoFrom(n.GetContext(req)) +func (n ContextBasedNaming) GenerateLink(ctx request.Context, obj runtime.Object) (uri string, err error) { + requestInfo, ok := request.RequestInfoFrom(ctx) if !ok { return "", fmt.Errorf("missing requestInfo") } 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 7e2c972bc7..834995f90b 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go @@ -102,7 +102,7 @@ func getResourceHandler(scope RequestScope, getter getterFunc) http.HandlerFunc scope.err(err, w, req) return } - if err := setSelfLink(result, req, scope.Namer); err != nil { + if err := setSelfLink(result, ctx, scope.Namer); err != nil { scope.err(err, w, req) return } @@ -321,7 +321,7 @@ func ListResource(r rest.Lister, rw rest.Watcher, scope RequestScope, forceWatch return } trace.Step("Listing from storage done") - numberOfItems, err := setListSelfLink(result, req, scope.Namer) + numberOfItems, err := setListSelfLink(result, ctx, req, scope.Namer) if err != nil { scope.err(err, w, req) return @@ -419,7 +419,7 @@ func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.Object } trace.Step("Object stored in database") - if err := setSelfLink(result, req, scope.Namer); err != nil { + if err := setSelfLink(result, ctx, scope.Namer); err != nil { scope.err(err, w, req) return } @@ -512,7 +512,7 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface return } - if err := setSelfLink(result, req, scope.Namer); err != nil { + if err := setSelfLink(result, ctx, scope.Namer); err != nil { scope.err(err, w, req) return } @@ -829,7 +829,7 @@ func UpdateResource(r rest.Updater, scope RequestScope, typer runtime.ObjectType } trace.Step("Object stored in database") - if err := setSelfLink(result, req, scope.Namer); err != nil { + if err := setSelfLink(result, ctx, scope.Namer); err != nil { scope.err(err, w, req) return } @@ -943,7 +943,7 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestSco } else { // when a non-status response is returned, set the self link if _, ok := result.(*metav1.Status); !ok { - if err := setSelfLink(result, req, scope.Namer); err != nil { + if err := setSelfLink(result, ctx, scope.Namer); err != nil { scope.err(err, w, req) return } @@ -1045,7 +1045,7 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestSco } else { // when a non-status response is returned, set the self link if _, ok := result.(*metav1.Status); !ok { - if _, err := setListSelfLink(result, req, scope.Namer); err != nil { + if _, err := setListSelfLink(result, ctx, req, scope.Namer); err != nil { scope.err(err, w, req) return } @@ -1113,9 +1113,9 @@ func transformDecodeError(typer runtime.ObjectTyper, baseErr error, into runtime // setSelfLink sets the self link of an object (or the child items in a list) to the base URL of the request // plus the path and query generated by the provided linkFunc -func setSelfLink(obj runtime.Object, req *http.Request, namer ScopeNamer) error { +func setSelfLink(obj runtime.Object, ctx request.Context, namer ScopeNamer) error { // TODO: SelfLink generation should return a full URL? - uri, err := namer.GenerateLink(req, obj) + uri, err := namer.GenerateLink(ctx, obj) if err != nil { return nil } @@ -1160,7 +1160,7 @@ func checkName(obj runtime.Object, name, namespace string, namer ScopeNamer) err // 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, req *http.Request, namer ScopeNamer) (int, error) { +func setListSelfLink(obj runtime.Object, ctx request.Context, req *http.Request, namer ScopeNamer) (int, error) { if !meta.IsListType(obj) { return 0, nil } @@ -1176,7 +1176,7 @@ func setListSelfLink(obj runtime.Object, req *http.Request, namer ScopeNamer) (i count := 0 err = meta.EachListItem(obj, func(obj runtime.Object) error { count++ - return setSelfLink(obj, req, namer) + return setSelfLink(obj, ctx, namer) }) return count, err } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go index 3d6aad640d..2c6c9eb093 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go @@ -160,11 +160,11 @@ func (p *testNamer) SetSelfLink(obj runtime.Object, url string) error { } // GenerateLink creates a path and query for a given runtime object that represents the canonical path. -func (p *testNamer) GenerateLink(req *http.Request, obj runtime.Object) (uri string, err error) { +func (p *testNamer) GenerateLink(ctx request.Context, obj runtime.Object) (uri string, err error) { return "", errors.New("not implemented") } -// GenerateLink creates a path and query for a list that represents the canonical path. +// GenerateListLink creates a path and query for a list that represents the canonical path. func (p *testNamer) GenerateListLink(req *http.Request) (uri string, err error) { return "", errors.New("not implemented") } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go index 077169a9aa..2a8d81e195 100755 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go @@ -30,6 +30,7 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/watch" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" + "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/server/httplog" "k8s.io/apiserver/pkg/util/wsstream" @@ -88,6 +89,14 @@ func serveWatch(watcher watch.Interface, scope RequestScope, req *http.Request, mediaType += ";stream=watch" } + namespace, _, err := scope.Namer.Name(req) + if err != nil { + scope.err(fmt.Errorf("unexpected error from Namer.Name: %v", err), w, req) + return + } + ctx := scope.ContextFunc(req) + ctx = request.WithNamespace(ctx, namespace) + server := &WatchServer{ Watching: watcher, Scope: scope, @@ -98,7 +107,7 @@ func serveWatch(watcher watch.Interface, scope RequestScope, req *http.Request, Encoder: encoder, EmbeddedEncoder: embeddedEncoder, Fixup: func(obj runtime.Object) { - if err := setSelfLink(obj, req, scope.Namer); err != nil { + if err := setSelfLink(obj, ctx, scope.Namer); err != nil { utilruntime.HandleError(fmt.Errorf("failed to set link for object %v: %v", reflect.TypeOf(obj), err)) } }, From b4018f7da18f1e61e59c5c48cae4178db2714f85 Mon Sep 17 00:00:00 2001 From: Wojciech Tyczynski Date: Mon, 22 May 2017 19:26:02 +0200 Subject: [PATCH 2/2] Pass RequestInfo to GenerateLink --- .../apiserver/pkg/endpoints/handlers/namer.go | 9 +--- .../apiserver/pkg/endpoints/handlers/rest.go | 45 +++++++++++++++---- .../pkg/endpoints/handlers/rest_test.go | 2 +- .../apiserver/pkg/endpoints/handlers/watch.go | 11 +++-- 4 files changed, 45 insertions(+), 22 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer.go index 88c0dc44c1..8e7a4ef33b 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer.go @@ -45,7 +45,7 @@ type ScopeNamer interface { SetSelfLink(obj runtime.Object, url string) error // GenerateLink creates an encoded URI for a given runtime object that represents the canonical path // and query. - GenerateLink(ctx request.Context, obj runtime.Object) (uri string, err error) + GenerateLink(requestInfo *request.RequestInfo, obj runtime.Object) (uri string, err error) // GenerateListLink creates an encoded URI for a list that represents the canonical path and query. GenerateListLink(req *http.Request) (uri string, err error) } @@ -90,12 +90,7 @@ func (n ContextBasedNaming) Name(req *http.Request) (namespace, name string, err return ns, requestInfo.Name, nil } -func (n ContextBasedNaming) GenerateLink(ctx request.Context, obj runtime.Object) (uri string, err error) { - requestInfo, ok := request.RequestInfoFrom(ctx) - if !ok { - return "", fmt.Errorf("missing requestInfo") - } - +func (n ContextBasedNaming) GenerateLink(requestInfo *request.RequestInfo, obj runtime.Object) (uri string, err error) { namespace, name, err := n.ObjectName(obj) if err == errEmptyName && len(requestInfo.Name) > 0 { name = requestInfo.Name 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 834995f90b..3e348c2a02 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go @@ -102,7 +102,12 @@ func getResourceHandler(scope RequestScope, getter getterFunc) http.HandlerFunc scope.err(err, w, req) return } - if err := setSelfLink(result, ctx, scope.Namer); err != nil { + 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 } @@ -419,7 +424,12 @@ func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.Object } trace.Step("Object stored in database") - if err := setSelfLink(result, ctx, scope.Namer); err != nil { + 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 } @@ -512,7 +522,12 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface return } - if err := setSelfLink(result, ctx, scope.Namer); err != nil { + 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 } @@ -829,7 +844,12 @@ func UpdateResource(r rest.Updater, scope RequestScope, typer runtime.ObjectType } trace.Step("Object stored in database") - if err := setSelfLink(result, ctx, scope.Namer); err != nil { + 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 } @@ -942,8 +962,13 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestSco } } 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, ctx, scope.Namer); err != nil { + if err := setSelfLink(result, requestInfo, scope.Namer); err != nil { scope.err(err, w, req) return } @@ -1113,9 +1138,9 @@ func transformDecodeError(typer runtime.ObjectTyper, baseErr error, into runtime // setSelfLink sets the self link of an object (or the child items in a list) to the base URL of the request // plus the path and query generated by the provided linkFunc -func setSelfLink(obj runtime.Object, ctx request.Context, namer ScopeNamer) error { +func setSelfLink(obj runtime.Object, requestInfo *request.RequestInfo, namer ScopeNamer) error { // TODO: SelfLink generation should return a full URL? - uri, err := namer.GenerateLink(ctx, obj) + uri, err := namer.GenerateLink(requestInfo, obj) if err != nil { return nil } @@ -1172,11 +1197,15 @@ func setListSelfLink(obj runtime.Object, ctx request.Context, req *http.Request, if err := namer.SetSelfLink(obj, uri); err != nil { glog.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") + } count := 0 err = meta.EachListItem(obj, func(obj runtime.Object) error { count++ - return setSelfLink(obj, ctx, namer) + return setSelfLink(obj, requestInfo, namer) }) return count, err } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go index 2c6c9eb093..a8dcd3e99a 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go @@ -160,7 +160,7 @@ func (p *testNamer) SetSelfLink(obj runtime.Object, url string) error { } // GenerateLink creates a path and query for a given runtime object that represents the canonical path. -func (p *testNamer) GenerateLink(ctx request.Context, obj runtime.Object) (uri string, err error) { +func (p *testNamer) GenerateLink(requestInfo *request.RequestInfo, obj runtime.Object) (uri string, err error) { return "", errors.New("not implemented") } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go index 2a8d81e195..16a8708563 100755 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go @@ -89,13 +89,12 @@ func serveWatch(watcher watch.Interface, scope RequestScope, req *http.Request, mediaType += ";stream=watch" } - namespace, _, err := scope.Namer.Name(req) - if err != nil { - scope.err(fmt.Errorf("unexpected error from Namer.Name: %v", err), w, req) + ctx := scope.ContextFunc(req) + requestInfo, ok := request.RequestInfoFrom(ctx) + if !ok { + scope.err(fmt.Errorf("missing requestInfo"), w, req) return } - ctx := scope.ContextFunc(req) - ctx = request.WithNamespace(ctx, namespace) server := &WatchServer{ Watching: watcher, @@ -107,7 +106,7 @@ func serveWatch(watcher watch.Interface, scope RequestScope, req *http.Request, Encoder: encoder, EmbeddedEncoder: embeddedEncoder, Fixup: func(obj runtime.Object) { - if err := setSelfLink(obj, ctx, scope.Namer); err != nil { + if err := setSelfLink(obj, requestInfo, scope.Namer); err != nil { utilruntime.HandleError(fmt.Errorf("failed to set link for object %v: %v", reflect.TypeOf(obj), err)) } },