From ce187f9c6a0bd9dfa8cc9d4f4a83a7e686e46ecb Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 1 Nov 2016 16:20:37 -0400 Subject: [PATCH 1/5] Avoid double decoding all client responses restclient must be able to deal with multiple types of servers. Alter the behavior of restclient.Result#Raw() to not process the body on error, but instead to return the generic error (which still matches the error checking cases in api/error like IsBadRequest). If the caller uses .Error(), .Into(), or .Get(), try decoding the body as a Status. For older servers, continue to default apiVersion "v1" when calling restclient.Result#Error(). This was only for 1.1 servers and the extensions group, which we have since fixed. This removes a double decode of very large objects (like LIST). --- pkg/client/restclient/client_test.go | 49 +++++++++-- pkg/client/restclient/request.go | 119 +++++++++++++++++++------- pkg/client/restclient/request_test.go | 71 ++++++++++++++- 3 files changed, 198 insertions(+), 41 deletions(-) diff --git a/pkg/client/restclient/client_test.go b/pkg/client/restclient/client_test.go index 009a2febcca..65f8492d28e 100644 --- a/pkg/client/restclient/client_test.go +++ b/pkg/client/restclient/client_test.go @@ -79,13 +79,52 @@ func TestDoRequestFailed(t *testing.T) { testServer := httptest.NewServer(&fakeHandler) defer testServer.Close() + c, err := restClient(testServer) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + err = c.Get().Do().Error() + if err == nil { + t.Errorf("unexpected non-error") + } + ss, ok := err.(errors.APIStatus) + if !ok { + t.Errorf("unexpected error type %v", err) + } + actual := ss.Status() + if !reflect.DeepEqual(status, &actual) { + t.Errorf("Unexpected mis-match: %s", diff.ObjectReflectDiff(status, &actual)) + } +} + +func TestDoRawRequestFailed(t *testing.T) { + status := &unversioned.Status{ + Code: http.StatusNotFound, + Status: unversioned.StatusFailure, + Reason: unversioned.StatusReasonNotFound, + Message: "the server could not find the requested resource", + Details: &unversioned.StatusDetails{ + Causes: []unversioned.StatusCause{ + {Type: unversioned.CauseTypeUnexpectedServerResponse, Message: "unknown"}, + }, + }, + } + expectedBody, _ := runtime.Encode(testapi.Default.Codec(), status) + fakeHandler := utiltesting.FakeHandler{ + StatusCode: 404, + ResponseBody: string(expectedBody), + T: t, + } + testServer := httptest.NewServer(&fakeHandler) + defer testServer.Close() + c, err := restClient(testServer) if err != nil { t.Fatalf("unexpected error: %v", err) } body, err := c.Get().Do().Raw() - if err == nil || body != nil { + if err == nil || body == nil { t.Errorf("unexpected non-error: %#v", body) } ss, ok := err.(errors.APIStatus) @@ -93,12 +132,8 @@ func TestDoRequestFailed(t *testing.T) { t.Errorf("unexpected error type %v", err) } actual := ss.Status() - expected := *status - // The decoder will apply the default Version and Kind to the Status. - expected.APIVersion = "v1" - expected.Kind = "Status" - if !reflect.DeepEqual(&expected, &actual) { - t.Errorf("Unexpected mis-match: %s", diff.ObjectDiff(status, &actual)) + if !reflect.DeepEqual(status, &actual) { + t.Errorf("Unexpected mis-match: %s", diff.ObjectReflectDiff(status, &actual)) } } diff --git a/pkg/client/restclient/request.go b/pkg/client/restclient/request.go index afddea093bf..0f17d0badc0 100644 --- a/pkg/client/restclient/request.go +++ b/pkg/client/restclient/request.go @@ -930,33 +930,21 @@ func (r *Request) transformResponse(resp *http.Response, req *http.Request) Resu } } - // Did the server give us a status response? - isStatusResponse := false - status := &unversioned.Status{} - // Because release-1.1 server returns Status with empty APIVersion at paths - // to the Extensions resources, we need to use DecodeInto here to provide - // default groupVersion, otherwise a status response won't be correctly - // decoded. - err := runtime.DecodeInto(decoder, body, status) - if err == nil && len(status.Status) > 0 { - isStatusResponse = true - } - switch { case resp.StatusCode == http.StatusSwitchingProtocols: // no-op, we've been upgraded case resp.StatusCode < http.StatusOK || resp.StatusCode > http.StatusPartialContent: - if !isStatusResponse { - return Result{err: r.transformUnstructuredResponseError(resp, req, body)} + // calculate an unstructured error from the response which the Result object may use if the caller + // did not return a structured error. + retryAfter, _ := retryAfterSeconds(resp) + err := r.newUnstructuredResponseError(body, isTextResponse(resp), resp.StatusCode, req.Method, retryAfter) + return Result{ + body: body, + contentType: contentType, + statusCode: resp.StatusCode, + decoder: decoder, + err: err, } - return Result{err: errors.FromObject(status)} - } - - // If the server gave us a status back, look at what it was. - success := resp.StatusCode >= http.StatusOK && resp.StatusCode <= http.StatusPartialContent - if isStatusResponse && (status.Status != unversioned.StatusSuccess && !success) { - // "Failed" requests are clearly just an error and it makes sense to return them as such. - return Result{err: errors.FromObject(status)} } return Result{ @@ -967,6 +955,9 @@ func (r *Request) transformResponse(resp *http.Response, req *http.Request) Resu } } +// maxUnstructuredResponseTextBytes is an upper bound on how much output to include in the unstructured error. +const maxUnstructuredResponseTextBytes = 2048 + // transformUnstructuredResponseError handles an error from the server that is not in a structured form. // It is expected to transform any response that is not recognizable as a clear server sent error from the // K8S API using the information provided with the request. In practice, HTTP proxies and client libraries @@ -987,20 +978,29 @@ func (r *Request) transformResponse(resp *http.Response, req *http.Request) Resu // TODO: introduce transformation of generic http.Client.Do() errors that separates 4. func (r *Request) transformUnstructuredResponseError(resp *http.Response, req *http.Request, body []byte) error { if body == nil && resp.Body != nil { - if data, err := ioutil.ReadAll(resp.Body); err == nil { + if data, err := ioutil.ReadAll(&io.LimitedReader{R: resp.Body, N: maxUnstructuredResponseTextBytes}); err == nil { body = data } } + retryAfter, _ := retryAfterSeconds(resp) + return r.newUnstructuredResponseError(body, isTextResponse(resp), resp.StatusCode, req.Method, retryAfter) +} + +// newUnstructuredResponseError instantiates the appropriate generic error for the provided input. It also logs the body. +func (r *Request) newUnstructuredResponseError(body []byte, isTextResponse bool, statusCode int, method string, retryAfter int) error { + // cap the amount of output we create + if len(body) > maxUnstructuredResponseTextBytes { + body = body[:maxUnstructuredResponseTextBytes] + } glog.V(8).Infof("Response Body: %#v", string(body)) message := "unknown" - if isTextResponse(resp) { + if isTextResponse { message = strings.TrimSpace(string(body)) } - retryAfter, _ := retryAfterSeconds(resp) return errors.NewGenericServerResponse( - resp.StatusCode, - req.Method, + statusCode, + method, unversioned.GroupResource{ Group: r.content.GroupVersion.Group, Resource: r.resource, @@ -1064,15 +1064,31 @@ func (r Result) Raw() ([]byte, error) { return r.body, r.err } -// Get returns the result as an object. +// Get returns the result as an object, which means it passes through the decoder. +// If the returned object is of type Status and has .Status != StatusSuccess, the +// additional information in Status will be used to enrich the error. func (r Result) Get() (runtime.Object, error) { if r.err != nil { - return nil, r.err + // Check whether the result has a Status object in the body and prefer that. + return nil, r.Error() } if r.decoder == nil { return nil, fmt.Errorf("serializer for %s doesn't exist", r.contentType) } - return runtime.Decode(r.decoder, r.body) + + // decode, but if the result is Status return that as an error instead. + out, _, err := r.decoder.Decode(r.body, nil, nil) + if err != nil { + return nil, err + } + switch t := out.(type) { + case *unversioned.Status: + // any status besides StatusSuccess is considered an error. + if t.Status != unversioned.StatusSuccess { + return nil, errors.FromObject(t) + } + } + return out, nil } // StatusCode returns the HTTP status code of the request. (Only valid if no @@ -1083,14 +1099,31 @@ func (r Result) StatusCode(statusCode *int) Result { } // Into stores the result into obj, if possible. If obj is nil it is ignored. +// If the returned object is of type Status and has .Status != StatusSuccess, the +// additional information in Status will be used to enrich the error. func (r Result) Into(obj runtime.Object) error { if r.err != nil { - return r.err + // Check whether the result has a Status object in the body and prefer that. + return r.Error() } if r.decoder == nil { return fmt.Errorf("serializer for %s doesn't exist", r.contentType) } - return runtime.DecodeInto(r.decoder, r.body, obj) + + out, _, err := r.decoder.Decode(r.body, nil, obj) + if err != nil || out == obj { + return err + } + // if a different object is returned, see if it is Status and avoid double decoding + // the object. + switch t := out.(type) { + case *unversioned.Status: + // any status besides StatusSuccess is considered an error. + if t.Status != unversioned.StatusSuccess { + return errors.FromObject(t) + } + } + return nil } // WasCreated updates the provided bool pointer to whether the server returned @@ -1101,7 +1134,29 @@ func (r Result) WasCreated(wasCreated *bool) Result { } // Error returns the error executing the request, nil if no error occurred. +// If the returned object is of type Status and has Status != StatusSuccess, the +// additional information in Status will be used to enrich the error. // See the Request.Do() comment for what errors you might get. func (r Result) Error() error { + // if we have received an unexpected server error, and we have a body and decoder, we can try to extract + // a Status object. + if r.err == nil || !errors.IsUnexpectedServerError(r.err) || len(r.body) == 0 || r.decoder == nil { + return r.err + } + + // attempt to convert the body into a Status object + // to be backwards compatible with old servers that do not return a version, default to "v1" + out, _, err := r.decoder.Decode(r.body, &unversioned.GroupVersionKind{Version: "v1"}, nil) + if err != nil { + glog.V(5).Infof("body was not decodable (unable to check for Status): %v", err) + return r.err + } + switch t := out.(type) { + case *unversioned.Status: + // because we default the kind, we *must* check for StatusFailure + if t.Status == unversioned.StatusFailure { + return errors.FromObject(t) + } + } return r.err } diff --git a/pkg/client/restclient/request_test.go b/pkg/client/restclient/request_test.go index 3bf5de8d650..514be7c2be0 100755 --- a/pkg/client/restclient/request_test.go +++ b/pkg/client/restclient/request_test.go @@ -41,6 +41,7 @@ import ( "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/runtime/serializer/streaming" "k8s.io/kubernetes/pkg/util/clock" + "k8s.io/kubernetes/pkg/util/diff" "k8s.io/kubernetes/pkg/util/flowcontrol" "k8s.io/kubernetes/pkg/util/httpstream" "k8s.io/kubernetes/pkg/util/intstr" @@ -582,7 +583,8 @@ func TestTransformUnstructuredError(t *testing.T) { Resource string Name string - ErrFn func(error) bool + ErrFn func(error) bool + Transformed error }{ { Resource: "foo", @@ -626,9 +628,46 @@ func TestTransformUnstructuredError(t *testing.T) { }, ErrFn: apierrors.IsBadRequest, }, + { + // status in response overrides transformed result + Req: &http.Request{}, + Res: &http.Response{StatusCode: http.StatusBadRequest, Body: ioutil.NopCloser(bytes.NewReader([]byte(`{"kind":"Status","apiVersion":"v1","status":"Failure","code":404}`)))}, + ErrFn: apierrors.IsBadRequest, + Transformed: &apierrors.StatusError{ + ErrStatus: unversioned.Status{Status: unversioned.StatusFailure, Code: http.StatusNotFound}, + }, + }, + { + // successful status is ignored + Req: &http.Request{}, + Res: &http.Response{StatusCode: http.StatusBadRequest, Body: ioutil.NopCloser(bytes.NewReader([]byte(`{"kind":"Status","apiVersion":"v1","status":"Success","code":404}`)))}, + ErrFn: apierrors.IsBadRequest, + }, + { + // empty object does not change result + Req: &http.Request{}, + Res: &http.Response{StatusCode: http.StatusBadRequest, Body: ioutil.NopCloser(bytes.NewReader([]byte(`{}`)))}, + ErrFn: apierrors.IsBadRequest, + }, + { + // we default apiVersion for backwards compatibility with old clients + // TODO: potentially remove in 1.7 + Req: &http.Request{}, + Res: &http.Response{StatusCode: http.StatusBadRequest, Body: ioutil.NopCloser(bytes.NewReader([]byte(`{"kind":"Status","status":"Failure","code":404}`)))}, + ErrFn: apierrors.IsBadRequest, + Transformed: &apierrors.StatusError{ + ErrStatus: unversioned.Status{Status: unversioned.StatusFailure, Code: http.StatusNotFound}, + }, + }, + { + // we do not default kind + Req: &http.Request{}, + Res: &http.Response{StatusCode: http.StatusBadRequest, Body: ioutil.NopCloser(bytes.NewReader([]byte(`{"status":"Failure","code":404}`)))}, + ErrFn: apierrors.IsBadRequest, + }, } - for _, testCase := range testCases { + for i, testCase := range testCases { r := &Request{ content: defaultContentConfig(), serializers: defaultSerializers(), @@ -641,12 +680,40 @@ func TestTransformUnstructuredError(t *testing.T) { t.Errorf("unexpected error: %v", err) continue } + if !apierrors.IsUnexpectedServerError(err) { + t.Errorf("%d: unexpected error type: %v", i, err) + } if len(testCase.Name) != 0 && !strings.Contains(err.Error(), testCase.Name) { t.Errorf("unexpected error string: %s", err) } if len(testCase.Resource) != 0 && !strings.Contains(err.Error(), testCase.Resource) { t.Errorf("unexpected error string: %s", err) } + + // verify Error() properly transforms the error + transformed := result.Error() + expect := testCase.Transformed + if expect == nil { + expect = err + } + if !reflect.DeepEqual(expect, transformed) { + t.Errorf("%d: unexpected Error(): %s", i, diff.ObjectReflectDiff(expect, transformed)) + } + + // verify result.Get properly transforms the error + if _, err := result.Get(); !reflect.DeepEqual(expect, err) { + t.Errorf("%d: unexpected error on Get(): %s", i, diff.ObjectReflectDiff(expect, err)) + } + + // verify result.Into properly handles the error + if err := result.Into(&api.Pod{}); !reflect.DeepEqual(expect, err) { + t.Errorf("%d: unexpected error on Into(): %s", i, diff.ObjectReflectDiff(expect, err)) + } + + // verify result.Raw leaves the error in the untransformed state + if _, err := result.Raw(); !reflect.DeepEqual(result.err, err) { + t.Errorf("%d: unexpected error on Raw(): %s", i, diff.ObjectReflectDiff(expect, err)) + } } } From f0fa26bcf629041ed3e26d1b5be5be460d52441e Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 1 Nov 2016 21:31:21 -0400 Subject: [PATCH 2/5] ExportOptions should be registered to all public schemas --- cmd/kubeadm/app/apis/kubeadm/register.go | 1 + cmd/kubeadm/app/apis/kubeadm/v1alpha1/register.go | 1 + federation/apis/federation/v1beta1/register.go | 1 + pkg/apis/apps/v1alpha1/register.go | 1 + pkg/apis/authorization/v1beta1/register.go | 1 + pkg/apis/autoscaling/v1/register.go | 1 + pkg/apis/batch/v1/register.go | 1 + pkg/apis/certificates/v1alpha1/register.go | 1 + pkg/apis/extensions/v1beta1/register.go | 1 + pkg/apis/policy/v1alpha1/register.go | 1 + 10 files changed, 10 insertions(+) diff --git a/cmd/kubeadm/app/apis/kubeadm/register.go b/cmd/kubeadm/app/apis/kubeadm/register.go index 359ee2f572e..9d0171553b2 100644 --- a/cmd/kubeadm/app/apis/kubeadm/register.go +++ b/cmd/kubeadm/app/apis/kubeadm/register.go @@ -50,6 +50,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { &ClusterInfo{}, &api.ListOptions{}, &api.DeleteOptions{}, + &api.ExportOptions{}, ) return nil } diff --git a/cmd/kubeadm/app/apis/kubeadm/v1alpha1/register.go b/cmd/kubeadm/app/apis/kubeadm/v1alpha1/register.go index 2a1ab13bbea..00df41fbebf 100644 --- a/cmd/kubeadm/app/apis/kubeadm/v1alpha1/register.go +++ b/cmd/kubeadm/app/apis/kubeadm/v1alpha1/register.go @@ -50,6 +50,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { &ClusterInfo{}, &v1.ListOptions{}, &v1.DeleteOptions{}, + &v1.ExportOptions{}, ) return nil } diff --git a/federation/apis/federation/v1beta1/register.go b/federation/apis/federation/v1beta1/register.go index 8fe059b140c..1e8405ae679 100644 --- a/federation/apis/federation/v1beta1/register.go +++ b/federation/apis/federation/v1beta1/register.go @@ -40,6 +40,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { &ClusterList{}, &v1.ListOptions{}, &v1.DeleteOptions{}, + &v1.ExportOptions{}, ) versionedwatch.AddToGroupVersion(scheme, SchemeGroupVersion) return nil diff --git a/pkg/apis/apps/v1alpha1/register.go b/pkg/apis/apps/v1alpha1/register.go index 33d443f998e..3d88f2970f0 100644 --- a/pkg/apis/apps/v1alpha1/register.go +++ b/pkg/apis/apps/v1alpha1/register.go @@ -41,6 +41,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { &StatefulSetList{}, &v1.ListOptions{}, &v1.DeleteOptions{}, + &v1.ExportOptions{}, ) versionedwatch.AddToGroupVersion(scheme, SchemeGroupVersion) return nil diff --git a/pkg/apis/authorization/v1beta1/register.go b/pkg/apis/authorization/v1beta1/register.go index 1b072d6152e..64635994128 100644 --- a/pkg/apis/authorization/v1beta1/register.go +++ b/pkg/apis/authorization/v1beta1/register.go @@ -39,6 +39,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { scheme.AddKnownTypes(SchemeGroupVersion, &v1.ListOptions{}, &v1.DeleteOptions{}, + &v1.ExportOptions{}, &SelfSubjectAccessReview{}, &SubjectAccessReview{}, diff --git a/pkg/apis/autoscaling/v1/register.go b/pkg/apis/autoscaling/v1/register.go index d7b7df928f3..1563a42a58a 100644 --- a/pkg/apis/autoscaling/v1/register.go +++ b/pkg/apis/autoscaling/v1/register.go @@ -42,6 +42,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { &Scale{}, &v1.ListOptions{}, &v1.DeleteOptions{}, + &v1.ExportOptions{}, ) versionedwatch.AddToGroupVersion(scheme, SchemeGroupVersion) return nil diff --git a/pkg/apis/batch/v1/register.go b/pkg/apis/batch/v1/register.go index aa97b0eafbf..2d59089ba39 100644 --- a/pkg/apis/batch/v1/register.go +++ b/pkg/apis/batch/v1/register.go @@ -41,6 +41,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { &JobList{}, &v1.ListOptions{}, &v1.DeleteOptions{}, + &v1.ExportOptions{}, ) versionedwatch.AddToGroupVersion(scheme, SchemeGroupVersion) return nil diff --git a/pkg/apis/certificates/v1alpha1/register.go b/pkg/apis/certificates/v1alpha1/register.go index cc6cd3c2219..c17dd08a977 100644 --- a/pkg/apis/certificates/v1alpha1/register.go +++ b/pkg/apis/certificates/v1alpha1/register.go @@ -51,6 +51,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { &CertificateSigningRequestList{}, &v1.ListOptions{}, &v1.DeleteOptions{}, + &v1.ExportOptions{}, ) // Add the watch version that applies diff --git a/pkg/apis/extensions/v1beta1/register.go b/pkg/apis/extensions/v1beta1/register.go index 697918c150c..6d8bb24e999 100644 --- a/pkg/apis/extensions/v1beta1/register.go +++ b/pkg/apis/extensions/v1beta1/register.go @@ -56,6 +56,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { &IngressList{}, &v1.ListOptions{}, &v1.DeleteOptions{}, + &v1.ExportOptions{}, &ReplicaSet{}, &ReplicaSetList{}, &PodSecurityPolicy{}, diff --git a/pkg/apis/policy/v1alpha1/register.go b/pkg/apis/policy/v1alpha1/register.go index 8fcb0be023f..34418c10b2f 100644 --- a/pkg/apis/policy/v1alpha1/register.go +++ b/pkg/apis/policy/v1alpha1/register.go @@ -42,6 +42,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { &Eviction{}, &v1.ListOptions{}, &v1.DeleteOptions{}, + &v1.ExportOptions{}, ) // Add the watch version that applies versionedwatch.AddToGroupVersion(scheme, SchemeGroupVersion) From 898701993180e64439dde343f713060b3b33daad Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 1 Nov 2016 21:31:46 -0400 Subject: [PATCH 3/5] Improve the common kinds API test to catch more errors --- pkg/api/serialization_test.go | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/pkg/api/serialization_test.go b/pkg/api/serialization_test.go index 3fbab963531..ad101efb05b 100644 --- a/pkg/api/serialization_test.go +++ b/pkg/api/serialization_test.go @@ -244,16 +244,38 @@ var nonRoundTrippableTypes = sets.NewString( "WatchEvent", ) -var commonKinds = []string{"ListOptions", "DeleteOptions"} +var commonKinds = []string{"Status", "ListOptions", "DeleteOptions", "ExportOptions"} -// verify all external group/versions have the common kinds like the ListOptions, DeleteOptions are registered. +// verify all external group/versions have the common kinds registered. func TestCommonKindsRegistered(t *testing.T) { for _, kind := range commonKinds { for _, group := range testapi.Groups { gv := group.GroupVersion() - if _, err := api.Scheme.New(gv.WithKind(kind)); err != nil { + gvk := gv.WithKind(kind) + obj, err := api.Scheme.New(gvk) + if err != nil { t.Error(err) } + defaults := gv.WithKind("") + if _, got, err := api.Codecs.LegacyCodec().Decode([]byte(`{"kind":"`+kind+`"}`), &defaults, nil); err != nil || gvk != *got { + t.Errorf("expected %v: %v %v", gvk, got, err) + } + data, err := runtime.Encode(api.Codecs.LegacyCodec(*gv), obj) + if err != nil { + t.Errorf("expected %v: %v\n%s", gvk, err, string(data)) + continue + } + if !bytes.Contains(data, []byte(`"kind":"`+kind+`","apiVersion":"`+gv.String()+`"`)) { + if kind != "Status" { + t.Errorf("expected %v: %v\n%s", gvk, err, string(data)) + continue + } + // TODO: this is wrong, but legacy clients expect it + if !bytes.Contains(data, []byte(`"kind":"`+kind+`","apiVersion":"v1"`)) { + t.Errorf("expected %v: %v\n%s", gvk, err, string(data)) + continue + } + } } } } From 95116c776e2425d160f8d62d127926340f494234 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 1 Nov 2016 21:32:08 -0400 Subject: [PATCH 4/5] Authorization should be registered in testapi --- pkg/api/testapi/BUILD | 1 + pkg/api/testapi/testapi.go | 36 ++++++++++++++++++++++++------------ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/pkg/api/testapi/BUILD b/pkg/api/testapi/BUILD index edccf97eec0..410d65a5f5d 100644 --- a/pkg/api/testapi/BUILD +++ b/pkg/api/testapi/BUILD @@ -27,6 +27,7 @@ go_library( "//pkg/apis/apps:go_default_library", "//pkg/apis/apps/install:go_default_library", "//pkg/apis/authentication/install:go_default_library", + "//pkg/apis/authorization:go_default_library", "//pkg/apis/authorization/install:go_default_library", "//pkg/apis/autoscaling:go_default_library", "//pkg/apis/autoscaling/install:go_default_library", diff --git a/pkg/api/testapi/testapi.go b/pkg/api/testapi/testapi.go index 3bdaea35eb3..50a18e34edd 100644 --- a/pkg/api/testapi/testapi.go +++ b/pkg/api/testapi/testapi.go @@ -37,6 +37,7 @@ import ( "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/apimachinery/registered" "k8s.io/kubernetes/pkg/apis/apps" + "k8s.io/kubernetes/pkg/apis/authorization" "k8s.io/kubernetes/pkg/apis/autoscaling" "k8s.io/kubernetes/pkg/apis/batch" "k8s.io/kubernetes/pkg/apis/certificates" @@ -66,18 +67,19 @@ import ( ) var ( - Groups = make(map[string]TestGroup) - Default TestGroup - Autoscaling TestGroup - Batch TestGroup - Extensions TestGroup - Apps TestGroup - Policy TestGroup - Federation TestGroup - Rbac TestGroup - Certificates TestGroup - Storage TestGroup - ImagePolicy TestGroup + Groups = make(map[string]TestGroup) + Default TestGroup + Authorization TestGroup + Autoscaling TestGroup + Batch TestGroup + Extensions TestGroup + Apps TestGroup + Policy TestGroup + Federation TestGroup + Rbac TestGroup + Certificates TestGroup + Storage TestGroup + ImagePolicy TestGroup serializer runtime.SerializerInfo storageSerializer runtime.SerializerInfo @@ -258,6 +260,15 @@ func init() { externalTypes: api.Scheme.KnownTypes(externalGroupVersion), } } + if _, ok := Groups[authorization.GroupName]; !ok { + externalGroupVersion := unversioned.GroupVersion{Group: authorization.GroupName, Version: registered.GroupOrDie(authorization.GroupName).GroupVersion.Version} + Groups[authorization.GroupName] = TestGroup{ + externalGroupVersion: externalGroupVersion, + internalGroupVersion: authorization.SchemeGroupVersion, + internalTypes: api.Scheme.KnownTypes(authorization.SchemeGroupVersion), + externalTypes: api.Scheme.KnownTypes(externalGroupVersion), + } + } if _, ok := Groups[kubeadm.GroupName]; !ok { externalGroupVersion := unversioned.GroupVersion{Group: kubeadm.GroupName, Version: registered.GroupOrDie(kubeadm.GroupName).GroupVersion.Version} Groups[kubeadm.GroupName] = TestGroup{ @@ -279,6 +290,7 @@ func init() { Rbac = Groups[rbac.GroupName] Storage = Groups[storage.GroupName] ImagePolicy = Groups[imagepolicy.GroupName] + Authorization = Groups[authorization.GroupName] } func (g TestGroup) ContentConfig() (string, *unversioned.GroupVersion, runtime.Codec) { From 051382687d4ddcb85ffea9c0f95d1671e6dd18c2 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 1 Nov 2016 21:37:13 -0400 Subject: [PATCH 5/5] Unversioned types should decode no matter what the requested version is Status is exposed as v1 in the current schema (so all groups are returning v1.Status). However, if you give a codec only "mygroup" "myversion", it will fail to convert Status to v1. For now, unversioned types should be allowed to be projected into all group versions, and when we add the server group we'll rip out the unversioned concept entirely. --- pkg/runtime/scheme.go | 9 +++++++++ pkg/runtime/scheme_test.go | 11 +++++++++++ 2 files changed, 20 insertions(+) diff --git a/pkg/runtime/scheme.go b/pkg/runtime/scheme.go index fdd4d1f44fa..0b42feae420 100644 --- a/pkg/runtime/scheme.go +++ b/pkg/runtime/scheme.go @@ -519,6 +519,15 @@ func (s *Scheme) convertToVersion(copy bool, in Object, target GroupVersioner) ( gvk, ok := target.KindForGroupVersionKinds(kinds) if !ok { + // try to see if this type is listed as unversioned (for legacy support) + // TODO: when we move to server API versions, we should completely remove the unversioned concept + if unversionedKind, ok := s.unversionedTypes[t]; ok { + if gvk, ok := target.KindForGroupVersionKinds([]unversioned.GroupVersionKind{unversionedKind}); ok { + return copyAndSetTargetKind(copy, s, in, gvk) + } + return copyAndSetTargetKind(copy, s, in, unversionedKind) + } + // TODO: should this be a typed error? return nil, fmt.Errorf("%v is not suitable for converting to %q", t, target) } diff --git a/pkg/runtime/scheme_test.go b/pkg/runtime/scheme_test.go index 2d71d412714..b51a8aff287 100644 --- a/pkg/runtime/scheme_test.go +++ b/pkg/runtime/scheme_test.go @@ -625,6 +625,17 @@ func TestConvertToVersion(t *testing.T) { A: "test", }, }, + // unversioned type returned when not included in the target types + { + scheme: GetTestScheme(), + in: &UnversionedType{A: "test"}, + gv: unversioned.GroupVersions{{Group: "other", Version: "v2"}}, + same: true, + out: &UnversionedType{ + MyWeirdCustomEmbeddedVersionKindField: MyWeirdCustomEmbeddedVersionKindField{APIVersion: "v1", ObjectKind: "UnversionedType"}, + A: "test", + }, + }, // detected as already being in the target version { scheme: GetTestScheme(),