From 6900f8856f8cd9a6c94a156b9e4a9fee0c16f807 Mon Sep 17 00:00:00 2001 From: David Eads Date: Tue, 24 Apr 2018 18:31:41 -0400 Subject: [PATCH 1/2] rest mappings cannot logically be object converters --- pkg/apis/core/install/install_test.go | 5 -- pkg/kubectl/cmd/annotate.go | 2 +- pkg/kubectl/cmd/autoscale.go | 10 +-- pkg/kubectl/cmd/convert.go | 2 +- pkg/kubectl/cmd/create/BUILD | 1 + pkg/kubectl/cmd/create/create.go | 8 ++- pkg/kubectl/cmd/drain.go | 2 +- pkg/kubectl/cmd/expose.go | 10 +-- pkg/kubectl/cmd/get/BUILD | 4 +- pkg/kubectl/cmd/get/get.go | 14 +++- pkg/kubectl/cmd/get/get_test.go | 59 ++++++---------- pkg/kubectl/cmd/rollingupdate.go | 7 +- pkg/kubectl/cmd/run.go | 10 +-- pkg/kubectl/cmd/set/set_env.go | 2 +- pkg/kubectl/cmd/taint.go | 2 +- pkg/kubectl/cmd/testing/fake.go | 18 ++--- pkg/kubectl/cmd/util/factory_builder.go | 19 +++--- pkg/kubectl/cmd/util/factory_test.go | 9 +-- pkg/kubectl/resource/builder.go | 24 ++++--- pkg/kubectl/resource/builder_test.go | 9 +-- pkg/kubectl/resource/mapper.go | 14 ++-- pkg/kubectl/resource/selector.go | 10 ++- pkg/kubectl/resource/visitor.go | 21 ++---- pkg/printers/humanreadable.go | 15 +++-- .../apps/deployment/storage/storage.go | 4 ++ .../apps/replicaset/storage/storage.go | 4 ++ .../apps/statefulset/storage/storage.go | 4 ++ pkg/registry/core/pod/storage/eviction.go | 5 ++ .../replicationcontroller/storage/storage.go | 4 ++ .../core/serviceaccount/storage/token.go | 5 ++ .../pkg/registry/customresource/etcd.go | 4 ++ .../apimachinery/pkg/api/meta/interfaces.go | 11 +-- .../apimachinery/pkg/api/meta/restmapper.go | 26 +------ .../pkg/api/meta/restmapper_test.go | 9 +-- .../apimachinery/pkg/apimachinery/BUILD | 1 + .../apimachinery/announced/group_factory.go | 5 +- .../apimachinery/pkg/apimachinery/types.go | 3 + .../src/k8s.io/apiserver/pkg/endpoints/BUILD | 2 +- .../apiserver/pkg/endpoints/apiserver_test.go | 61 ++++++----------- .../apiserver/pkg/endpoints/groupversion.go | 5 +- .../apiserver/pkg/endpoints/installer.go | 67 ++++++------------- .../apiserver/pkg/registry/rest/rest.go | 1 + .../apiserver/pkg/server/genericapiserver.go | 2 +- test/integration/apiserver/print_test.go | 2 +- 44 files changed, 232 insertions(+), 270 deletions(-) diff --git a/pkg/apis/core/install/install_test.go b/pkg/apis/core/install/install_test.go index 7fcfa85b06c..d15e58ffaec 100644 --- a/pkg/apis/core/install/install_test.go +++ b/pkg/apis/core/install/install_test.go @@ -103,11 +103,6 @@ func TestRESTMapper(t *testing.T) { t.Errorf("incorrect version: %v", mapping) } - interfaces, _ := legacyscheme.Registry.GroupOrDie(internal.GroupName).InterfacesFor(version) - if mapping.ObjectConvertor != interfaces.ObjectConvertor { - t.Errorf("unexpected: %#v, expected: %#v", mapping, interfaces) - } - rc := &internal.ReplicationController{ObjectMeta: metav1.ObjectMeta{Name: "foo"}} name, err := meta.NewAccessor().Name(rc) if err != nil { diff --git a/pkg/kubectl/cmd/annotate.go b/pkg/kubectl/cmd/annotate.go index 0e8e2ac6d2d..52b9deddc4c 100644 --- a/pkg/kubectl/cmd/annotate.go +++ b/pkg/kubectl/cmd/annotate.go @@ -248,7 +248,7 @@ func (o AnnotateOptions) RunAnnotate(f cmdutil.Factory, cmd *cobra.Command) erro var outputObj runtime.Object var obj runtime.Object - obj, err = info.Mapping.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupVersion()) + obj, err = info.ObjectConverter.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupVersion()) if err != nil { return err } diff --git a/pkg/kubectl/cmd/autoscale.go b/pkg/kubectl/cmd/autoscale.go index 27ca4dc02eb..b9b548ca37e 100644 --- a/pkg/kubectl/cmd/autoscale.go +++ b/pkg/kubectl/cmd/autoscale.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" @@ -230,10 +231,11 @@ func (o *AutoscaleOptions) Run() error { } resourceMapper := &resource.Mapper{ - ObjectTyper: o.Typer, - RESTMapper: o.Mapper, - ClientMapper: resource.ClientMapperFunc(o.ClientForMapping), - Decoder: cmdutil.InternalVersionDecoder(), + ObjectTyper: o.Typer, + ObjectConverter: legacyscheme.Scheme, + RESTMapper: o.Mapper, + ClientMapper: resource.ClientMapperFunc(o.ClientForMapping), + Decoder: cmdutil.InternalVersionDecoder(), } hpa, err := resourceMapper.InfoForObject(object, nil) if err != nil { diff --git a/pkg/kubectl/cmd/convert.go b/pkg/kubectl/cmd/convert.go index 52ada9c1100..a76c6a30a2e 100644 --- a/pkg/kubectl/cmd/convert.go +++ b/pkg/kubectl/cmd/convert.go @@ -282,7 +282,7 @@ func asVersionedObjects(infos []*resource.Info, specifiedOutputVersion schema.Gr } } - converted, err := tryConvert(info.Mapping.ObjectConvertor, info.Object, targetVersions...) + converted, err := tryConvert(scheme.Scheme, info.Object, targetVersions...) if err != nil { return nil, err } diff --git a/pkg/kubectl/cmd/create/BUILD b/pkg/kubectl/cmd/create/BUILD index 6a6445ce082..b322e982ca4 100644 --- a/pkg/kubectl/cmd/create/BUILD +++ b/pkg/kubectl/cmd/create/BUILD @@ -23,6 +23,7 @@ go_library( importpath = "k8s.io/kubernetes/pkg/kubectl/cmd/create", visibility = ["//build/visible_to:pkg_kubectl_cmd_create_CONSUMERS"], deps = [ + "//pkg/api/legacyscheme:go_default_library", "//pkg/kubectl:go_default_library", "//pkg/kubectl/cmd/templates:go_default_library", "//pkg/kubectl/cmd/util:go_default_library", diff --git a/pkg/kubectl/cmd/create/create.go b/pkg/kubectl/cmd/create/create.go index 9b267419510..1bb06f423eb 100644 --- a/pkg/kubectl/cmd/create/create.go +++ b/pkg/kubectl/cmd/create/create.go @@ -31,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" kruntime "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" @@ -404,9 +405,10 @@ func RunCreateSubcommand(f cmdutil.Factory, options *CreateSubcommandOptions) er return err } resourceMapper := &resource.Mapper{ - ObjectTyper: typer, - RESTMapper: mapper, - ClientMapper: resource.ClientMapperFunc(f.ClientForMapping), + ObjectTyper: typer, + ObjectConverter: legacyscheme.Scheme, + RESTMapper: mapper, + ClientMapper: resource.ClientMapperFunc(f.ClientForMapping), } info, err := resourceMapper.InfoForObject(obj, nil) if err != nil { diff --git a/pkg/kubectl/cmd/drain.go b/pkg/kubectl/cmd/drain.go index cc4372a800d..fdcb57a2453 100644 --- a/pkg/kubectl/cmd/drain.go +++ b/pkg/kubectl/cmd/drain.go @@ -718,7 +718,7 @@ func (o *DrainOptions) RunCordonOrUncordon(desired bool) error { for _, nodeInfo := range o.nodeInfos { if nodeInfo.Mapping.GroupVersionKind.Kind == "Node" { - obj, err := nodeInfo.Mapping.ConvertToVersion(nodeInfo.Object, nodeInfo.Mapping.GroupVersionKind.GroupVersion()) + obj, err := nodeInfo.ObjectConverter.ConvertToVersion(nodeInfo.Object, nodeInfo.Mapping.GroupVersionKind.GroupVersion()) if err != nil { fmt.Printf("error: unable to %s node %q: %v", cordonOrUncordon, nodeInfo.Name, err) continue diff --git a/pkg/kubectl/cmd/expose.go b/pkg/kubectl/cmd/expose.go index c39316374cf..b7d92e552cc 100644 --- a/pkg/kubectl/cmd/expose.go +++ b/pkg/kubectl/cmd/expose.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" @@ -311,10 +312,11 @@ func (o *ExposeServiceOptions) RunExpose(cmd *cobra.Command, args []string) erro } resourceMapper := &resource.Mapper{ - ObjectTyper: o.Typer, - RESTMapper: o.Mapper, - ClientMapper: resource.ClientMapperFunc(o.ClientForMapping), - Decoder: cmdutil.InternalVersionDecoder(), + ObjectTyper: o.Typer, + ObjectConverter: legacyscheme.Scheme, + RESTMapper: o.Mapper, + ClientMapper: resource.ClientMapperFunc(o.ClientForMapping), + Decoder: cmdutil.InternalVersionDecoder(), } info, err = resourceMapper.InfoForObject(object, nil) if err != nil { diff --git a/pkg/kubectl/cmd/get/BUILD b/pkg/kubectl/cmd/get/BUILD index c36439140b9..010f663e883 100644 --- a/pkg/kubectl/cmd/get/BUILD +++ b/pkg/kubectl/cmd/get/BUILD @@ -24,6 +24,7 @@ go_library( importpath = "k8s.io/kubernetes/pkg/kubectl/cmd/get", visibility = ["//visibility:public"], deps = [ + "//pkg/api/legacyscheme:go_default_library", "//pkg/apis/core:go_default_library", "//pkg/kubectl:go_default_library", "//pkg/kubectl/cmd/templates:go_default_library", @@ -66,7 +67,6 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/api/legacyscheme:go_default_library", - "//pkg/api/testapi:go_default_library", "//pkg/api/testing:go_default_library", "//pkg/apis/core:go_default_library", "//pkg/apis/core/v1:go_default_library", @@ -77,8 +77,8 @@ go_test( "//pkg/kubectl/genericclioptions:go_default_library", "//pkg/kubectl/scheme:go_default_library", "//pkg/printers:go_default_library", + "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer/json:go_default_library", diff --git a/pkg/kubectl/cmd/get/get.go b/pkg/kubectl/cmd/get/get.go index a364f2f3200..763bf76c13f 100644 --- a/pkg/kubectl/cmd/get/get.go +++ b/pkg/kubectl/cmd/get/get.go @@ -38,6 +38,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/rest" + "k8s.io/kubernetes/pkg/api/legacyscheme" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" @@ -428,7 +429,14 @@ func (o *GetOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e lastMapping = mapping } - printer.PrintObj(info.AsInternal(), w) + internalObj, err := legacyscheme.Scheme.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupKind().WithVersion(runtime.APIVersionInternal).GroupVersion()) + if err != nil { + // if there's an error, try to print what you have (mirrors old behavior). + glog.V(1).Info(err) + printer.PrintObj(info.Object, w) + } else { + printer.PrintObj(internalObj, w) + } } w.Flush() if nonEmptyObjCount == 0 && !o.IgnoreNotFound { @@ -542,7 +550,7 @@ func (o *GetOptions) watch(f cmdutil.Factory, cmd *cobra.Command, args []string) if !o.IsGeneric { // printing always takes the internal version, but the watch event uses externals internalGV := mapping.GroupVersionKind.GroupKind().WithVersion(runtime.APIVersionInternal).GroupVersion() - objToPrint = attemptToConvertToInternal(objToPrint, mapping, internalGV) + objToPrint = attemptToConvertToInternal(objToPrint, legacyscheme.Scheme, internalGV) } if err := printer.PrintObj(objToPrint, writer); err != nil { return fmt.Errorf("unable to output the provided object: %v", err) @@ -570,7 +578,7 @@ func (o *GetOptions) watch(f cmdutil.Factory, cmd *cobra.Command, args []string) // printing always takes the internal version, but the watch event uses externals // TODO fix printing to use server-side or be version agnostic internalGV := mapping.GroupVersionKind.GroupKind().WithVersion(runtime.APIVersionInternal).GroupVersion() - if err := printer.PrintObj(attemptToConvertToInternal(e.Object, mapping, internalGV), o.Out); err != nil { + if err := printer.PrintObj(attemptToConvertToInternal(e.Object, legacyscheme.Scheme, internalGV), o.Out); err != nil { return false, err } return false, nil diff --git a/pkg/kubectl/cmd/get/get_test.go b/pkg/kubectl/cmd/get/get_test.go index aa7f1089990..28837df82be 100644 --- a/pkg/kubectl/cmd/get/get_test.go +++ b/pkg/kubectl/cmd/get/get_test.go @@ -27,24 +27,22 @@ import ( "strings" "testing" + api "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer/json" "k8s.io/apimachinery/pkg/runtime/serializer/streaming" - "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/dynamic" restclient "k8s.io/client-go/rest" "k8s.io/client-go/rest/fake" restclientwatch "k8s.io/client-go/rest/watch" "k8s.io/kube-openapi/pkg/util/proto" - "k8s.io/kubernetes/pkg/api/testapi" apitesting "k8s.io/kubernetes/pkg/api/testing" - api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/v1" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/kubernetes/pkg/api/legacyscheme" cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" @@ -104,11 +102,11 @@ func testData() (*api.PodList, *api.ServiceList, *api.ReplicationControllerList) Items: []api.Pod{ { ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "test", ResourceVersion: "10"}, - Spec: apitesting.DeepEqualSafePodSpec(), + Spec: apitesting.V1DeepEqualSafePodSpec(), }, { ObjectMeta: metav1.ObjectMeta{Name: "bar", Namespace: "test", ResourceVersion: "11"}, - Spec: apitesting.DeepEqualSafePodSpec(), + Spec: apitesting.V1DeepEqualSafePodSpec(), }, }, } @@ -126,6 +124,8 @@ func testData() (*api.PodList, *api.ServiceList, *api.ReplicationControllerList) }, }, } + + one := int32(1) rc := &api.ReplicationControllerList{ ListMeta: metav1.ListMeta{ ResourceVersion: "17", @@ -134,7 +134,7 @@ func testData() (*api.PodList, *api.ServiceList, *api.ReplicationControllerList) { ObjectMeta: metav1.ObjectMeta{Name: "rc1", Namespace: "test", ResourceVersion: "18"}, Spec: api.ReplicationControllerSpec{ - Replicas: 1, + Replicas: &one, }, }, }, @@ -193,25 +193,6 @@ func TestGetUnknownSchemaObject(t *testing.T) { tf.Namespace = "test" tf.ClientConfigVal = defaultClientConfig() - mapper, _ := tf.Object() - m, err := mapper.RESTMapping(schema.GroupKind{Group: "apitest", Kind: "Type"}) - if err != nil { - t.Fatal(err) - } - convertedObj, err := m.ConvertToVersion(&unstructured.Unstructured{ - Object: map[string]interface{}{ - "kind": "Type", - "apiVersion": "apitest/unlikelyversion", - "name": "foo", - }, - }, schema.GroupVersion{Group: "apitest", Version: "unlikelyversion"}) - if err != nil { - t.Fatal(err) - } - if !reflect.DeepEqual(convertedObj, obj) { - t.Fatalf("unexpected conversion of unstructured object to structured: %s", diff.ObjectReflectDiff(convertedObj, obj)) - } - streams, _, buf, _ := genericclioptions.NewTestIOStreams() cmd := NewCmdGet("kubectl", tf, streams) cmd.SetOutput(buf) @@ -246,7 +227,9 @@ func TestGetUnknownSchemaObject(t *testing.T) { func TestGetSchemaObject(t *testing.T) { tf := cmdtesting.NewTestFactory() defer tf.Cleanup() - codec := testapi.Default.Codec() + codec := legacyscheme.Codecs.LegacyCodec(schema.GroupVersion{Version: "v1"}) + t.Logf("%v", string(runtime.EncodeOrDie(codec, &api.ReplicationController{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}))) + tf.UnstructuredClient = &fake.RESTClient{ NegotiatedSerializer: unstructuredSerializer, Resp: &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, &api.ReplicationController{ObjectMeta: metav1.ObjectMeta{Name: "foo"}})}, @@ -401,15 +384,15 @@ func TestGetSortedObjects(t *testing.T) { Items: []api.Pod{ { ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: "test", ResourceVersion: "10"}, - Spec: apitesting.DeepEqualSafePodSpec(), + Spec: apitesting.V1DeepEqualSafePodSpec(), }, { ObjectMeta: metav1.ObjectMeta{Name: "b", Namespace: "test", ResourceVersion: "11"}, - Spec: apitesting.DeepEqualSafePodSpec(), + Spec: apitesting.V1DeepEqualSafePodSpec(), }, { ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: "test", ResourceVersion: "9"}, - Spec: apitesting.DeepEqualSafePodSpec(), + Spec: apitesting.V1DeepEqualSafePodSpec(), }, }, } @@ -701,7 +684,6 @@ func TestGetMultipleTypeObjectsAsList(t *testing.T) { "containers": null, "dnsPolicy": "ClusterFirst", "restartPolicy": "Always", - "schedulerName": "default-scheduler", "securityContext": {}, "terminationGracePeriodSeconds": 30 }, @@ -720,7 +702,6 @@ func TestGetMultipleTypeObjectsAsList(t *testing.T) { "containers": null, "dnsPolicy": "ClusterFirst", "restartPolicy": "Always", - "schedulerName": "default-scheduler", "securityContext": {}, "terminationGracePeriodSeconds": 30 }, @@ -752,7 +733,7 @@ func TestGetMultipleTypeObjectsAsList(t *testing.T) { } ` if e, a := expected, buf.String(); e != a { - t.Errorf("expected %v, got %v", e, a) + t.Errorf("did not match: %v", diff.StringDiff(e, a)) } } @@ -896,7 +877,7 @@ func watchTestData() ([]api.Pod, []watch.Event) { Namespace: "test", ResourceVersion: "9", }, - Spec: apitesting.DeepEqualSafePodSpec(), + Spec: apitesting.V1DeepEqualSafePodSpec(), }, { ObjectMeta: metav1.ObjectMeta{ @@ -904,7 +885,7 @@ func watchTestData() ([]api.Pod, []watch.Event) { Namespace: "test", ResourceVersion: "10", }, - Spec: apitesting.DeepEqualSafePodSpec(), + Spec: apitesting.V1DeepEqualSafePodSpec(), }, } events := []watch.Event{ @@ -917,7 +898,7 @@ func watchTestData() ([]api.Pod, []watch.Event) { Namespace: "test", ResourceVersion: "9", }, - Spec: apitesting.DeepEqualSafePodSpec(), + Spec: apitesting.V1DeepEqualSafePodSpec(), }, }, { @@ -928,7 +909,7 @@ func watchTestData() ([]api.Pod, []watch.Event) { Namespace: "test", ResourceVersion: "10", }, - Spec: apitesting.DeepEqualSafePodSpec(), + Spec: apitesting.V1DeepEqualSafePodSpec(), }, }, // resource events @@ -940,7 +921,7 @@ func watchTestData() ([]api.Pod, []watch.Event) { Namespace: "test", ResourceVersion: "11", }, - Spec: apitesting.DeepEqualSafePodSpec(), + Spec: apitesting.V1DeepEqualSafePodSpec(), }, }, { @@ -951,7 +932,7 @@ func watchTestData() ([]api.Pod, []watch.Event) { Namespace: "test", ResourceVersion: "12", }, - Spec: apitesting.DeepEqualSafePodSpec(), + Spec: apitesting.V1DeepEqualSafePodSpec(), }, }, } diff --git a/pkg/kubectl/cmd/rollingupdate.go b/pkg/kubectl/cmd/rollingupdate.go index 4792bc0dc67..0ae18b3e725 100644 --- a/pkg/kubectl/cmd/rollingupdate.go +++ b/pkg/kubectl/cmd/rollingupdate.go @@ -220,7 +220,12 @@ func RunRollingUpdate(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args switch t := infos[0].AsVersioned().(type) { case *v1.ReplicationController: replicasDefaulted = t.Spec.Replicas == nil - newRc, _ = infos[0].AsInternal().(*api.ReplicationController) + + // previous code ignored the error. Seem like it's very unlikely to fail, so ok for now. + uncastObj, err := legacyscheme.Scheme.ConvertToVersion(t, api.SchemeGroupVersion) + if err == nil { + newRc, _ = uncastObj.(*api.ReplicationController) + } } if newRc == nil { glog.V(4).Infof("Object %T is not a ReplicationController", infos[0].Object) diff --git a/pkg/kubectl/cmd/run.go b/pkg/kubectl/cmd/run.go index 95a0a42e7ab..12d3bb96780 100644 --- a/pkg/kubectl/cmd/run.go +++ b/pkg/kubectl/cmd/run.go @@ -32,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/watch" + "k8s.io/kubernetes/pkg/api/legacyscheme" api "k8s.io/kubernetes/pkg/apis/core" coreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion" "k8s.io/kubernetes/pkg/kubectl" @@ -673,10 +674,11 @@ func (o *RunOptions) createGeneratedObject(f cmdutil.Factory, cmd *cobra.Command versioned := obj if !o.DryRun { resourceMapper := &resource.Mapper{ - ObjectTyper: typer, - RESTMapper: mapper, - ClientMapper: resource.ClientMapperFunc(f.ClientForMapping), - Decoder: cmdutil.InternalVersionDecoder(), + ObjectTyper: typer, + ObjectConverter: legacyscheme.Scheme, + RESTMapper: mapper, + ClientMapper: resource.ClientMapperFunc(f.ClientForMapping), + Decoder: cmdutil.InternalVersionDecoder(), } info, err := resourceMapper.InfoForObject(obj, nil) if err != nil { diff --git a/pkg/kubectl/cmd/set/set_env.go b/pkg/kubectl/cmd/set/set_env.go index 837ba77e388..efbed9c2299 100644 --- a/pkg/kubectl/cmd/set/set_env.go +++ b/pkg/kubectl/cmd/set/set_env.go @@ -272,7 +272,7 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error { } for _, info := range infos { - versionedObject, err := info.Mapping.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupVersion()) + versionedObject, err := info.ObjectConverter.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupVersion()) if err != nil { return err } diff --git a/pkg/kubectl/cmd/taint.go b/pkg/kubectl/cmd/taint.go index 72e8dde1a9c..49a20d9e1f8 100644 --- a/pkg/kubectl/cmd/taint.go +++ b/pkg/kubectl/cmd/taint.go @@ -233,7 +233,7 @@ func (o TaintOptions) RunTaint() error { return err } - obj, err := info.Mapping.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupVersion()) + obj, err := info.ObjectConverter.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupVersion()) if err != nil { return err } diff --git a/pkg/kubectl/cmd/testing/fake.go b/pkg/kubectl/cmd/testing/fake.go index ce43c413bd4..8fae54e02a3 100644 --- a/pkg/kubectl/cmd/testing/fake.go +++ b/pkg/kubectl/cmd/testing/fake.go @@ -346,16 +346,18 @@ func (f *TestFactory) NewBuilder() *resource.Builder { return resource.NewBuilder( &resource.Mapper{ - RESTMapper: mapper, - ObjectTyper: typer, - ClientMapper: resource.ClientMapperFunc(f.ClientForMapping), - Decoder: cmdutil.InternalVersionDecoder(), + RESTMapper: mapper, + ObjectTyper: typer, + ClientMapper: resource.ClientMapperFunc(f.ClientForMapping), + ObjectConverter: legacyscheme.Scheme, + Decoder: cmdutil.InternalVersionDecoder(), }, &resource.Mapper{ - RESTMapper: mapper, - ObjectTyper: typer, - ClientMapper: resource.ClientMapperFunc(f.UnstructuredClientForMapping), - Decoder: unstructured.UnstructuredJSONScheme, + RESTMapper: mapper, + ObjectTyper: typer, + ClientMapper: resource.ClientMapperFunc(f.UnstructuredClientForMapping), + ObjectConverter: unstructured.UnstructuredObjectConverter{}, + Decoder: unstructured.UnstructuredJSONScheme, }, f.CategoryExpander(), ) diff --git a/pkg/kubectl/cmd/util/factory_builder.go b/pkg/kubectl/cmd/util/factory_builder.go index 1d2edd9c506..4476ff1ba43 100644 --- a/pkg/kubectl/cmd/util/factory_builder.go +++ b/pkg/kubectl/cmd/util/factory_builder.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/dynamic" scaleclient "k8s.io/client-go/scale" + "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/kubectl/plugins" "k8s.io/kubernetes/pkg/kubectl/resource" @@ -55,16 +56,18 @@ func (f *ring2Factory) NewBuilder() *resource.Builder { return resource.NewBuilder( &resource.Mapper{ - RESTMapper: mapper, - ObjectTyper: typer, - ClientMapper: clientMapperFunc, - Decoder: InternalVersionDecoder(), + RESTMapper: mapper, + ObjectTyper: typer, + ObjectConverter: legacyscheme.Scheme, + ClientMapper: clientMapperFunc, + Decoder: InternalVersionDecoder(), }, &resource.Mapper{ - RESTMapper: mapper, - ObjectTyper: typer, - ClientMapper: unstructuredClientMapperFunc, - Decoder: unstructured.UnstructuredJSONScheme, + RESTMapper: mapper, + ObjectTyper: typer, + ObjectConverter: unstructured.UnstructuredObjectConverter{}, + ClientMapper: unstructuredClientMapperFunc, + Decoder: unstructured.UnstructuredJSONScheme, }, categoryExpander, ) diff --git a/pkg/kubectl/cmd/util/factory_test.go b/pkg/kubectl/cmd/util/factory_test.go index 6c6dfa16ce7..30aad3e2d44 100644 --- a/pkg/kubectl/cmd/util/factory_test.go +++ b/pkg/kubectl/cmd/util/factory_test.go @@ -474,10 +474,11 @@ func TestDiscoveryReplaceAliases(t *testing.T) { mapper := NewShortcutExpander(testapi.Default.RESTMapper(), ds) b := resource.NewBuilder( &resource.Mapper{ - RESTMapper: mapper, - ObjectTyper: legacyscheme.Scheme, - ClientMapper: fakeClient(), - Decoder: testapi.Default.Codec(), + RESTMapper: mapper, + ObjectTyper: legacyscheme.Scheme, + ObjectConverter: legacyscheme.Scheme, + ClientMapper: fakeClient(), + Decoder: testapi.Default.Codec(), }, nil, categories.LegacyCategoryExpander, diff --git a/pkg/kubectl/resource/builder.go b/pkg/kubectl/resource/builder.go index 40744777013..30fa1220aef 100644 --- a/pkg/kubectl/resource/builder.go +++ b/pkg/kubectl/resource/builder.go @@ -760,7 +760,7 @@ func (b *Builder) visitBySelector() *Result { if mapping.Scope.Name() != meta.RESTScopeNameNamespace { selectorNamespace = "" } - visitors = append(visitors, NewSelector(client, mapping, selectorNamespace, labelSelector, fieldSelector, b.export, b.includeUninitialized, b.limitChunks)) + visitors = append(visitors, NewSelector(client, mapping, b.mapper.ObjectConverter, selectorNamespace, labelSelector, fieldSelector, b.export, b.includeUninitialized, b.limitChunks)) } if b.continueOnError { result.visitor = EagerVisitorList(visitors) @@ -835,11 +835,12 @@ func (b *Builder) visitByResource() *Result { } info := &Info{ - Client: client, - Mapping: mapping, - Namespace: selectorNamespace, - Name: tuple.Name, - Export: b.export, + Client: client, + Mapping: mapping, + ObjectConverter: b.mapper.ObjectConverter, + Namespace: selectorNamespace, + Name: tuple.Name, + Export: b.export, } items = append(items, info) } @@ -900,11 +901,12 @@ func (b *Builder) visitByName() *Result { visitors := []Visitor{} for _, name := range b.names { info := &Info{ - Client: client, - Mapping: mapping, - Namespace: selectorNamespace, - Name: name, - Export: b.export, + Client: client, + Mapping: mapping, + ObjectConverter: b.mapper.ObjectConverter, + Namespace: selectorNamespace, + Name: name, + Export: b.export, } visitors = append(visitors, info) } diff --git a/pkg/kubectl/resource/builder_test.go b/pkg/kubectl/resource/builder_test.go index a70a3846803..32b8188263b 100644 --- a/pkg/kubectl/resource/builder_test.go +++ b/pkg/kubectl/resource/builder_test.go @@ -270,10 +270,11 @@ func newDefaultBuilder() *Builder { func newDefaultBuilderWith(client ClientMapper) *Builder { return NewBuilder( &Mapper{ - RESTMapper: restmapper, - ObjectTyper: scheme.Scheme, - ClientMapper: client, - Decoder: corev1Codec, + RESTMapper: restmapper, + ObjectTyper: scheme.Scheme, + ObjectConverter: scheme.Scheme, + ClientMapper: client, + Decoder: corev1Codec, }, nil, categories.LegacyCategoryExpander, diff --git a/pkg/kubectl/resource/mapper.go b/pkg/kubectl/resource/mapper.go index 80d269e30d7..9d479c87f28 100644 --- a/pkg/kubectl/resource/mapper.go +++ b/pkg/kubectl/resource/mapper.go @@ -29,6 +29,8 @@ import ( // needed to create Info for arbitrary objects. type Mapper struct { runtime.ObjectTyper + ObjectConverter runtime.ObjectConvertor + meta.RESTMapper ClientMapper runtime.Decoder @@ -70,8 +72,9 @@ func (m *Mapper) InfoForData(data []byte, source string) (*Info, error) { resourceVersion, _ := metadataAccessor.ResourceVersion(obj) return &Info{ - Client: client, - Mapping: mapping, + Client: client, + Mapping: mapping, + ObjectConverter: m.ObjectConverter, Source: source, Namespace: namespace, @@ -109,8 +112,9 @@ func (m *Mapper) InfoForObject(obj runtime.Object, preferredGVKs []schema.GroupV namespace, _ := metadataAccessor.Namespace(obj) resourceVersion, _ := metadataAccessor.ResourceVersion(obj) return &Info{ - Client: client, - Mapping: mapping, + Client: client, + Mapping: mapping, + ObjectConverter: m.ObjectConverter, Namespace: namespace, Name: name, @@ -199,7 +203,6 @@ func (m relaxedMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*me return &meta.RESTMapping{ GroupVersionKind: gk.WithVersion(versions[0]), Scope: meta.RESTScopeRoot, - ObjectConvertor: identityConvertor{}, }, nil } return mapping, err @@ -211,7 +214,6 @@ func (m relaxedMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([] { GroupVersionKind: gk.WithVersion(versions[0]), Scope: meta.RESTScopeRoot, - ObjectConvertor: identityConvertor{}, }, }, nil } diff --git a/pkg/kubectl/resource/selector.go b/pkg/kubectl/resource/selector.go index f36508bd4a9..41c47703461 100644 --- a/pkg/kubectl/resource/selector.go +++ b/pkg/kubectl/resource/selector.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/watch" ) @@ -29,6 +30,7 @@ import ( type Selector struct { Client RESTClient Mapping *meta.RESTMapping + ObjectConverter runtime.ObjectConvertor Namespace string LabelSelector string FieldSelector string @@ -38,10 +40,11 @@ type Selector struct { } // NewSelector creates a resource selector which hides details of getting items by their label selector. -func NewSelector(client RESTClient, mapping *meta.RESTMapping, namespace, labelSelector, fieldSelector string, export, includeUninitialized bool, limitChunks int64) *Selector { +func NewSelector(client RESTClient, mapping *meta.RESTMapping, objectConverter runtime.ObjectConvertor, namespace, labelSelector, fieldSelector string, export, includeUninitialized bool, limitChunks int64) *Selector { return &Selector{ Client: client, Mapping: mapping, + ObjectConverter: objectConverter, Namespace: namespace, LabelSelector: labelSelector, FieldSelector: fieldSelector, @@ -91,8 +94,9 @@ func (r *Selector) Visit(fn VisitorFunc) error { resourceVersion, _ := metadataAccessor.ResourceVersion(list) nextContinueToken, _ := metadataAccessor.Continue(list) info := &Info{ - Client: r.Client, - Mapping: r.Mapping, + Client: r.Client, + Mapping: r.Mapping, + ObjectConverter: r.ObjectConverter, Namespace: r.Namespace, ResourceVersion: resourceVersion, diff --git a/pkg/kubectl/resource/visitor.go b/pkg/kubectl/resource/visitor.go index bad03b8de0f..e46ffd92c14 100644 --- a/pkg/kubectl/resource/visitor.go +++ b/pkg/kubectl/resource/visitor.go @@ -78,6 +78,9 @@ type Info struct { // Mapping may be nil if the object has no available metadata, but is still parseable // from disk. Mapping *meta.RESTMapping + // ObjectConverter allows conversion + ObjectConverter runtime.ObjectConvertor + // Namespace will be set if the object is namespaced and has a specified value. Namespace string Name string @@ -180,23 +183,9 @@ func (i *Info) ResourceMapping() *meta.RESTMapping { return i.Mapping } -// Internal attempts to convert the provided object to an internal type or returns an error. -func (i *Info) Internal() (runtime.Object, error) { - return i.Mapping.ConvertToVersion(i.Object, i.Mapping.GroupVersionKind.GroupKind().WithVersion(runtime.APIVersionInternal).GroupVersion()) -} - -// AsInternal returns the object in internal form if possible, or i.Object if it cannot be -// converted. -func (i *Info) AsInternal() runtime.Object { - if obj, err := i.Internal(); err == nil { - return obj - } - return i.Object -} - // Versioned returns the object as a Go type in the mapping's version or returns an error. func (i *Info) Versioned() (runtime.Object, error) { - return i.Mapping.ConvertToVersion(i.Object, i.Mapping.GroupVersionKind.GroupVersion()) + return i.ObjectConverter.ConvertToVersion(i.Object, i.Mapping.GroupVersionKind.GroupVersion()) } // AsVersioned returns the object as a Go object in the external form if possible (matching the @@ -219,7 +208,7 @@ func (i *Info) Unstructured() (runtime.Unstructured, error) { return out.(runtime.Unstructured), err default: out := &unstructured.Unstructured{} - if err := i.Mapping.Convert(i.Object, out, nil); err != nil { + if err := i.ObjectConverter.Convert(i.Object, out, nil); err != nil { return nil, err } return out, nil diff --git a/pkg/printers/humanreadable.go b/pkg/printers/humanreadable.go index e75f2f24b92..f9806b14e44 100644 --- a/pkg/printers/humanreadable.go +++ b/pkg/printers/humanreadable.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -297,7 +298,7 @@ func (h *HumanReadablePrinter) PrintObj(obj runtime.Object, output io.Writer) er // check if the object is unstructured. If so, let's attempt to convert it to a type we can understand before // trying to print, since the printers are keyed by type. This is extremely expensive. if h.encoder != nil && h.decoder != nil { - obj, _ = decodeUnknownObject(obj, h.encoder, h.decoder) + obj, _ = decodeUnknownObject(obj, h.decoder) } // print with a registered handler @@ -817,15 +818,19 @@ func AppendAllLabels(showLabels bool, itemLabels map[string]string) string { } // check if the object is unstructured. If so, attempt to convert it to a type we can understand. -func decodeUnknownObject(obj runtime.Object, encoder runtime.Encoder, decoder runtime.Decoder) (runtime.Object, error) { +func decodeUnknownObject(obj runtime.Object, decoder runtime.Decoder) (runtime.Object, error) { var err error - switch obj.(type) { - case runtime.Unstructured, *runtime.Unknown: - if objBytes, err := runtime.Encode(encoder, obj); err == nil { + switch t := obj.(type) { + case runtime.Unstructured: + if objBytes, err := runtime.Encode(unstructured.UnstructuredJSONScheme, obj); err == nil { if decodedObj, err := runtime.Decode(decoder, objBytes); err == nil { obj = decodedObj } } + case *runtime.Unknown: + if decodedObj, err := runtime.Decode(decoder, t.Raw); err == nil { + obj = decodedObj + } } return obj, err diff --git a/pkg/registry/apps/deployment/storage/storage.go b/pkg/registry/apps/deployment/storage/storage.go index c8b469ec45d..46e274a6626 100644 --- a/pkg/registry/apps/deployment/storage/storage.go +++ b/pkg/registry/apps/deployment/storage/storage.go @@ -223,6 +223,10 @@ func (r *ScaleREST) GroupVersionKind(containingGV schema.GroupVersion) schema.Gr } } +func (*ScaleREST) ClusterScoped() bool { + return false +} + // New creates a new Scale object func (r *ScaleREST) New() runtime.Object { return &autoscaling.Scale{} diff --git a/pkg/registry/apps/replicaset/storage/storage.go b/pkg/registry/apps/replicaset/storage/storage.go index aa8ee4e1754..0a116772dd9 100644 --- a/pkg/registry/apps/replicaset/storage/storage.go +++ b/pkg/registry/apps/replicaset/storage/storage.go @@ -151,6 +151,10 @@ func (r *ScaleREST) GroupVersionKind(containingGV schema.GroupVersion) schema.Gr } } +func (*ScaleREST) ClusterScoped() bool { + return false +} + // New creates a new Scale object func (r *ScaleREST) New() runtime.Object { return &autoscaling.Scale{} diff --git a/pkg/registry/apps/statefulset/storage/storage.go b/pkg/registry/apps/statefulset/storage/storage.go index 5ec3dfb2a20..be3c674bb9d 100644 --- a/pkg/registry/apps/statefulset/storage/storage.go +++ b/pkg/registry/apps/statefulset/storage/storage.go @@ -140,6 +140,10 @@ func (r *ScaleREST) GroupVersionKind(containingGV schema.GroupVersion) schema.Gr } } +func (*ScaleREST) ClusterScoped() bool { + return false +} + // New creates a new Scale object func (r *ScaleREST) New() runtime.Object { return &autoscaling.Scale{} diff --git a/pkg/registry/core/pod/storage/eviction.go b/pkg/registry/core/pod/storage/eviction.go index 5eb66cb6e76..5d58cdbdc42 100644 --- a/pkg/registry/core/pod/storage/eviction.go +++ b/pkg/registry/core/pod/storage/eviction.go @@ -72,6 +72,11 @@ func (r *EvictionREST) GroupVersionKind(containingGV schema.GroupVersion) schema return schema.GroupVersionKind{Group: "policy", Version: "v1beta1", Kind: "Eviction"} } +// ClusterScoped fulfills GroupVersionKindProvider +func (*EvictionREST) ClusterScoped() bool { + return false +} + // New creates a new eviction resource func (r *EvictionREST) New() runtime.Object { return &policy.Eviction{} diff --git a/pkg/registry/core/replicationcontroller/storage/storage.go b/pkg/registry/core/replicationcontroller/storage/storage.go index 0c5217f37c5..4e0fc937be1 100644 --- a/pkg/registry/core/replicationcontroller/storage/storage.go +++ b/pkg/registry/core/replicationcontroller/storage/storage.go @@ -140,6 +140,10 @@ func (r *ScaleREST) GroupVersionKind(containingGV schema.GroupVersion) schema.Gr } } +func (*ScaleREST) ClusterScoped() bool { + return false +} + // New creates a new Scale object func (r *ScaleREST) New() runtime.Object { return &autoscaling.Scale{} diff --git a/pkg/registry/core/serviceaccount/storage/token.go b/pkg/registry/core/serviceaccount/storage/token.go index 2194f6da4db..265e15faa09 100644 --- a/pkg/registry/core/serviceaccount/storage/token.go +++ b/pkg/registry/core/serviceaccount/storage/token.go @@ -46,6 +46,7 @@ type TokenREST struct { } var _ = rest.NamedCreater(&TokenREST{}) +var _ = rest.GroupVersionKindProvider(&TokenREST{}) func (r *TokenREST) Create(ctx context.Context, name string, obj runtime.Object, createValidation rest.ValidateObjectFunc, includeUninitialized bool) (runtime.Object, error) { if err := createValidation(obj); err != nil { @@ -120,6 +121,10 @@ func (r *TokenREST) GroupVersionKind(containingGV schema.GroupVersion) schema.Gr } } +func (*TokenREST) ClusterScoped() bool { + return false +} + type getter interface { Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd.go index f0cb5fda8f8..33b27672523 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd.go @@ -145,6 +145,10 @@ func (r *ScaleREST) GroupVersionKind(containingGV schema.GroupVersion) schema.Gr return autoscalingv1.SchemeGroupVersion.WithKind("Scale") } +func (*ScaleREST) ClusterScoped() bool { + return false +} + // New creates a new Scale object func (r *ScaleREST) New() runtime.Object { return &autoscalingv1.Scale{} diff --git a/staging/src/k8s.io/apimachinery/pkg/api/meta/interfaces.go b/staging/src/k8s.io/apimachinery/pkg/api/meta/interfaces.go index 91500805242..b67054626bc 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/meta/interfaces.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/meta/interfaces.go @@ -25,7 +25,7 @@ import ( // VersionInterfaces contains the interfaces one should use for dealing with types of a particular version. type VersionInterfaces struct { - runtime.ObjectConvertor + ObjectConvertor runtime.ObjectConvertor } type ListMetaAccessor interface { @@ -91,13 +91,6 @@ const ( type RESTScope interface { // Name of the scope Name() RESTScopeName - // ParamName is the optional name of the parameter that should be inserted in the resource url - // If empty, no param will be inserted - ParamName() string - // ArgumentName is the optional name that should be used for the variable holding the value. - ArgumentName() string - // ParamDescription is the optional description to use to document the parameter in api documentation - ParamDescription() string } // RESTMapping contains the information needed to deal with objects of a specific @@ -110,8 +103,6 @@ type RESTMapping struct { // Scope contains the information needed to deal with REST Resources that are in a resource hierarchy Scope RESTScope - - runtime.ObjectConvertor } // RESTMapper allows clients to map resources to kind, and map kind and version diff --git a/staging/src/k8s.io/apimachinery/pkg/api/meta/restmapper.go b/staging/src/k8s.io/apimachinery/pkg/api/meta/restmapper.go index 75dc1c0ebca..28145724145 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/meta/restmapper.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/meta/restmapper.go @@ -28,30 +28,15 @@ import ( // Implements RESTScope interface type restScope struct { - name RESTScopeName - paramName string - argumentName string - paramDescription string + name RESTScopeName } func (r *restScope) Name() RESTScopeName { return r.name } -func (r *restScope) ParamName() string { - return r.paramName -} -func (r *restScope) ArgumentName() string { - return r.argumentName -} -func (r *restScope) ParamDescription() string { - return r.paramDescription -} var RESTScopeNamespace = &restScope{ - name: RESTScopeNameNamespace, - paramName: "namespaces", - argumentName: "namespace", - paramDescription: "object name and auth scope, such as for teams and projects", + name: RESTScopeNameNamespace, } var RESTScopeRoot = &restScope{ @@ -526,17 +511,10 @@ func (m *DefaultRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string return nil, fmt.Errorf("the provided version %q and kind %q cannot be mapped to a supported scope", gvk.GroupVersion(), gvk.Kind) } - interfaces, err := m.interfacesFunc(gvk.GroupVersion()) - if err != nil { - return nil, fmt.Errorf("the provided version %q has no relevant versions: %v", gvk.GroupVersion().String(), err) - } - mappings = append(mappings, &RESTMapping{ Resource: res.Resource, GroupVersionKind: gvk, Scope: scope, - - ObjectConvertor: interfaces.ObjectConvertor, }) } diff --git a/staging/src/k8s.io/apimachinery/pkg/api/meta/restmapper_test.go b/staging/src/k8s.io/apimachinery/pkg/api/meta/restmapper_test.go index 87d9021d3fb..34ef5d785a5 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/meta/restmapper_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/meta/restmapper_test.go @@ -577,10 +577,6 @@ func TestRESTMapperRESTMapping(t *testing.T) { t.Errorf("%d: unexpected resource: %#v", i, mapping) } - if mapping.ObjectConvertor == nil { - t.Errorf("%d: missing codec: %#v", i, mapping) - } - groupVersion := testCase.ExpectedGroupVersion if groupVersion == nil { groupVersion = &testCase.APIGroupVersions[0] @@ -727,9 +723,6 @@ func TestRESTMapperRESTMappings(t *testing.T) { if mapping.Resource != exp.Resource { t.Errorf("%d - %d: unexpected resource: %#v", i, j, mapping) } - if mapping.ObjectConvertor == nil { - t.Errorf("%d - %d: missing codec: %#v", i, j, mapping) - } if mapping.GroupVersionKind != exp.GroupVersionKind { t.Errorf("%d - %d: unexpected GroupVersionKind: %#v", i, j, mapping) } @@ -744,7 +737,7 @@ func TestRESTMapperReportsErrorOnBadVersion(t *testing.T) { mapper := NewDefaultRESTMapper([]schema.GroupVersion{expectedGroupVersion1, expectedGroupVersion2}, unmatchedVersionInterfaces) mapper.Add(expectedGroupVersion1.WithKind("InternalObject"), RESTScopeNamespace) - _, err := mapper.RESTMapping(internalObjectGK, expectedGroupVersion1.Version) + _, err := mapper.RESTMapping(internalObjectGK, "test3") if err == nil { t.Errorf("unexpected non-error") } diff --git a/staging/src/k8s.io/apimachinery/pkg/apimachinery/BUILD b/staging/src/k8s.io/apimachinery/pkg/apimachinery/BUILD index b1071fb463d..84137760b11 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apimachinery/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/apimachinery/BUILD @@ -24,6 +24,7 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", ], ) diff --git a/staging/src/k8s.io/apimachinery/pkg/apimachinery/announced/group_factory.go b/staging/src/k8s.io/apimachinery/pkg/apimachinery/announced/group_factory.go index b12aa2c48a2..2802948dc4f 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apimachinery/announced/group_factory.go +++ b/staging/src/k8s.io/apimachinery/pkg/apimachinery/announced/group_factory.go @@ -153,8 +153,9 @@ func (gmf *GroupMetaFactory) Register(m *registered.APIRegistrationManager, sche accessor := meta.NewAccessor() groupMeta := &apimachinery.GroupMeta{ - GroupVersions: externalVersions, - SelfLinker: runtime.SelfLinker(accessor), + GroupVersions: externalVersions, + SelfLinker: runtime.SelfLinker(accessor), + RootScopedKinds: gmf.GroupArgs.RootScopedKinds, } for _, v := range externalVersions { gvf := gmf.VersionArgs[v.Version] diff --git a/staging/src/k8s.io/apimachinery/pkg/apimachinery/types.go b/staging/src/k8s.io/apimachinery/pkg/apimachinery/types.go index 4b05fe13fbd..c762a7afc07 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apimachinery/types.go +++ b/staging/src/k8s.io/apimachinery/pkg/apimachinery/types.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" ) // GroupMeta stores the metadata of a group. @@ -29,6 +30,8 @@ type GroupMeta struct { // GroupVersions is Group + all versions in that group. GroupVersions []schema.GroupVersion + RootScopedKinds sets.String + // SelfLinker can set or get the SelfLink field of all API types. // TODO: when versioning changes, make this part of each API definition. // TODO(lavalamp): Combine SelfLinker & ResourceVersioner interfaces, force all uses diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/BUILD index 0fd8f5f195a..843ae6bd7d6 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/BUILD @@ -65,13 +65,13 @@ go_library( importpath = "k8s.io/apiserver/pkg/endpoints", deps = [ "//vendor/github.com/emicklei/go-restful:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/conversion:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", "//vendor/k8s.io/apiserver/pkg/endpoints/discovery:go_default_library", "//vendor/k8s.io/apiserver/pkg/endpoints/handlers:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go index 869a8338b63..78ef764479f 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go @@ -117,7 +117,6 @@ var parameterCodec = runtime.NewParameterCodec(scheme) var accessor = meta.NewAccessor() var selfLinker runtime.SelfLinker = accessor -var mapper, namespaceMapper meta.RESTMapper // The mappers with namespace and with legacy namespace scopes. var admissionControl admission.Interface func init() { @@ -203,25 +202,6 @@ func init() { addTestTypes() addNewTestTypes() - nsMapper := newMapper() - - // enumerate all supported versions, get the kinds, and register with - // the mapper how to address our resources - for _, gv := range groupVersions { - for kind := range scheme.KnownTypes(gv) { - gvk := gv.WithKind(kind) - root := bool(kind == "SimpleRoot") - if root { - nsMapper.Add(gvk, meta.RESTScopeRoot) - } else { - nsMapper.Add(gvk, meta.RESTScopeNamespace) - } - } - } - - mapper = nsMapper - namespaceMapper = nsMapper - scheme.AddFieldLabelConversionFunc(grouplessGroupVersion.String(), "Simple", func(label, value string) (string, string, error) { return label, value, nil @@ -263,12 +243,12 @@ func handleInternal(storage map[string]rest.Storage, admissionControl admission. template := APIGroupVersion{ Storage: storage, - Creater: scheme, - Convertor: scheme, - Defaulter: scheme, - Typer: scheme, - Linker: selfLinker, - Mapper: namespaceMapper, + Creater: scheme, + Convertor: scheme, + Defaulter: scheme, + Typer: scheme, + Linker: selfLinker, + RootScopedKinds: sets.NewString("SimpleRoot"), ParameterCodec: parameterCodec, @@ -841,7 +821,6 @@ func TestNotFound(t *testing.T) { if response.StatusCode != v.Status { t.Errorf("Expected %d for %s (%s), Got %#v", v.Status, v.Method, k, response) - t.Errorf("MAPPER: %v", mapper) } } } @@ -3188,15 +3167,15 @@ func TestParentResourceIsRequired(t *testing.T) { Storage: map[string]rest.Storage{ "simple/sub": storage, }, - Root: "/" + prefix, - Creater: scheme, - Convertor: scheme, - Defaulter: scheme, - Typer: scheme, - Linker: selfLinker, + Root: "/" + prefix, + Creater: scheme, + Convertor: scheme, + Defaulter: scheme, + Typer: scheme, + Linker: selfLinker, + RootScopedKinds: sets.NewString("SimpleRoot"), - Admit: admissionControl, - Mapper: namespaceMapper, + Admit: admissionControl, GroupVersion: newGroupVersion, OptionsExternalVersion: &newGroupVersion, @@ -3225,8 +3204,7 @@ func TestParentResourceIsRequired(t *testing.T) { Typer: scheme, Linker: selfLinker, - Admit: admissionControl, - Mapper: namespaceMapper, + Admit: admissionControl, GroupVersion: newGroupVersion, OptionsExternalVersion: &newGroupVersion, @@ -3806,6 +3784,8 @@ type SimpleXGSubresourceRESTStorage struct { itemGVK schema.GroupVersionKind } +var _ = rest.GroupVersionKindProvider(&SimpleXGSubresourceRESTStorage{}) + func (storage *SimpleXGSubresourceRESTStorage) New() runtime.Object { return &genericapitesting.SimpleXGSubresource{} } @@ -3814,12 +3794,14 @@ func (storage *SimpleXGSubresourceRESTStorage) Get(ctx context.Context, id strin return storage.item.DeepCopyObject(), nil } -var _ = rest.GroupVersionKindProvider(&SimpleXGSubresourceRESTStorage{}) - func (storage *SimpleXGSubresourceRESTStorage) GroupVersionKind(containingGV schema.GroupVersion) schema.GroupVersionKind { return storage.itemGVK } +func (*SimpleXGSubresourceRESTStorage) ClusterScoped() bool { + return false +} + func TestXGSubresource(t *testing.T) { container := restful.NewContainer() container.Router(restful.CurlyRouter{}) @@ -3845,7 +3827,6 @@ func TestXGSubresource(t *testing.T) { Defaulter: scheme, Typer: scheme, Linker: selfLinker, - Mapper: namespaceMapper, ParameterCodec: parameterCodec, diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go b/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go index a8c62dd5b62..7060eb73897 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go @@ -22,11 +22,11 @@ import ( "github.com/emicklei/go-restful" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/endpoints/discovery" "k8s.io/apiserver/pkg/registry/rest" @@ -55,7 +55,8 @@ type APIGroupVersion struct { // version (for when the inevitable meta/v2 group emerges). MetaGroupVersion *schema.GroupVersion - Mapper meta.RESTMapper + // RootScopedKinds are the root scoped kinds for the primary GroupVersion + RootScopedKinds sets.String // Serializer is used to determine how to convert responses from API methods into bytes to send over // the wire. diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go index 70921e78131..2cddaa9d889 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go @@ -28,7 +28,6 @@ import ( restful "github.com/emicklei/go-restful" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/conversion" "k8s.io/apimachinery/pkg/runtime" @@ -133,18 +132,18 @@ func (a *APIInstaller) newWebService() *restful.WebService { } // getResourceKind returns the external group version kind registered for the given storage -// object. If the storage object is a subresource and has an override supplied for it, it returns +// object and whether or not the kind is cluster scoped. If the storage object is a subresource and has an override supplied for it, it returns // the group version kind supplied in the override. -func (a *APIInstaller) getResourceKind(path string, storage rest.Storage) (schema.GroupVersionKind, error) { +func (a *APIInstaller) getResourceKind(path string, storage rest.Storage) (schema.GroupVersionKind, bool, error) { // Let the storage tell us exactly what GVK it has if gvkProvider, ok := storage.(rest.GroupVersionKindProvider); ok { - return gvkProvider.GroupVersionKind(a.group.GroupVersion), nil + return gvkProvider.GroupVersionKind(a.group.GroupVersion), gvkProvider.ClusterScoped(), nil } object := storage.New() fqKinds, _, err := a.group.Typer.ObjectKinds(object) if err != nil { - return schema.GroupVersionKind{}, err + return schema.GroupVersionKind{}, false, err } // a given go type can have multiple potential fully qualified kinds. Find the one that corresponds with the group @@ -157,32 +156,11 @@ func (a *APIInstaller) getResourceKind(path string, storage rest.Storage) (schem } } if fqKindToRegister.Empty() { - return schema.GroupVersionKind{}, fmt.Errorf("unable to locate fully qualified kind for %v: found %v when registering for %v", reflect.TypeOf(object), fqKinds, a.group.GroupVersion) + return schema.GroupVersionKind{}, false, fmt.Errorf("unable to locate fully qualified kind for %v: found %v when registering for %v", reflect.TypeOf(object), fqKinds, a.group.GroupVersion) } - return fqKindToRegister, nil -} -// restMapping returns rest mapper for the resource. -// Example REST paths that this mapper maps. -// 1. Resource only, no subresource: -// Resource Type: batch/v1.Job (input args: resource = "jobs") -// REST path: /apis/batch/v1/namespaces/{namespace}/job/{name} -// 2. Subresource and its parent belong to different API groups and/or versions: -// Resource Type: extensions/v1beta1.ReplicaSet (input args: resource = "replicasets") -// Subresource Type: autoscaling/v1.Scale -// REST path: /apis/extensions/v1beta1/namespaces/{namespace}/replicaset/{name}/scale -func (a *APIInstaller) restMapping(resource string) (*meta.RESTMapping, error) { - // subresources must have parent resources, and follow the namespacing rules of their parent. - // So get the storage of the resource (which is the parent resource in case of subresources) - storage, ok := a.group.Storage[resource] - if !ok { - return nil, fmt.Errorf("unable to locate the storage object for resource: %s", resource) - } - fqKindToRegister, err := a.getResourceKind(resource, storage) - if err != nil { - return nil, fmt.Errorf("unable to locate fully qualified kind for mapper resource %s: %v", resource, err) - } - return a.group.Mapper.RESTMapping(fqKindToRegister.GroupKind(), fqKindToRegister.Version) + // group is guaranteed to match based on the check above + return fqKindToRegister, a.group.RootScopedKinds.Has(fqKindToRegister.Kind), nil } func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storage, ws *restful.WebService) (*metav1.APIResource, error) { @@ -198,12 +176,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag return nil, err } - mapping, err := a.restMapping(resource) - if err != nil { - return nil, err - } - - fqKindToRegister, err := a.getResourceKind(path, storage) + fqKindToRegister, clusterScoped, err := a.getResourceKind(path, storage) if err != nil { return nil, err } @@ -338,7 +311,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag } allowWatchList := isWatcher && isLister // watching on lists is allowed only for kinds that support both watch and list. - scope := mapping.Scope nameParam := ws.PathParameter("name", "name of the "+kind).DataType("string") pathParam := ws.PathParameter("path", "path to the resource").DataType("string") @@ -357,8 +329,8 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag var apiResource metav1.APIResource // Get the list of actions for the given scope. - switch scope.Name() { - case meta.RESTScopeNameRoot: + switch { + case clusterScoped: // Handle non-namespace scoped resources like nodes. resourcePath := resource resourceParams := params @@ -402,10 +374,11 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag actions = appendIf(actions, action{"CONNECT", itemPath, nameParams, namer, false}, isConnecter) actions = appendIf(actions, action{"CONNECT", itemPath + "/{path:*}", proxyParams, namer, false}, isConnecter && connectSubpath) break - case meta.RESTScopeNameNamespace: + default: + namespaceParamName := "namespaces" // Handler for standard REST verbs (GET, PUT, POST and DELETE). - namespaceParam := ws.PathParameter(scope.ArgumentName(), scope.ParamDescription()).DataType("string") - namespacedPath := scope.ParamName() + "/{" + scope.ArgumentName() + "}/" + resource + namespaceParam := ws.PathParameter("namespace", "object name and auth scope, such as for teams and projects").DataType("string") + namespacedPath := namespaceParamName + "/{" + "namespace" + "}/" + resource namespaceParams := []*restful.Parameter{namespaceParam} resourcePath := namespacedPath @@ -426,7 +399,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag namer := handlers.ContextBasedNaming{ SelfLinker: a.group.Linker, ClusterScoped: false, - SelfLinkPathPrefix: gpath.Join(a.prefix, scope.ParamName()) + "/", + SelfLinkPathPrefix: gpath.Join(a.prefix, namespaceParamName) + "/", SelfLinkPathSuffix: itemPathSuffix, } @@ -455,8 +428,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag actions = appendIf(actions, action{"WATCHLIST", "watch/" + resource, params, namer, true}, allowWatchList) } break - default: - return nil, fmt.Errorf("unsupported restscope: %s", scope.Name()) } // Create Routes for the actions. @@ -539,7 +510,11 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag // If there is a subresource, kind should be the parent's kind. if hasSubresource { - fqParentKind, err := a.getResourceKind(resource, a.group.Storage[resource]) + parentStorage, ok := a.group.Storage[resource] + if !ok { + return nil, fmt.Errorf("missing parent storage: %q", resource) + } + fqParentKind, _, err := a.getResourceKind(resource, parentStorage) if err != nil { return nil, err } @@ -654,7 +629,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag string(types.MergePatchType), string(types.StrategicMergePatchType), } - handler := metrics.InstrumentRouteFunc(action.Verb, resource, subresource, requestScope, restfulPatchResource(patcher, reqScope, admit, mapping.ObjectConvertor, supportedTypes)) + handler := metrics.InstrumentRouteFunc(action.Verb, resource, subresource, requestScope, restfulPatchResource(patcher, reqScope, admit, a.group.Convertor, supportedTypes)) route := ws.PATCH(action.Path).To(handler). Doc(doc). Param(ws.QueryParameter("pretty", "If 'true', then the output is pretty printed.")). diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go index 19564b18164..1e440bf64e5 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go @@ -83,6 +83,7 @@ type CategoriesProvider interface { // TODO KindProvider (only used by federation) should be removed and replaced with this, but that presents greater risk late in 1.8. type GroupVersionKindProvider interface { GroupVersionKind(containingGV schema.GroupVersion) schema.GroupVersionKind + ClusterScoped() bool } // Lister is an object that can retrieve resources that match the provided field and label criteria. diff --git a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go index 6e25a93911d..aac1569af9d 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go +++ b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go @@ -423,7 +423,7 @@ func (s *GenericAPIServer) newAPIGroupVersion(apiGroupInfo *APIGroupInfo, groupV Defaulter: apiGroupInfo.Scheme, Typer: apiGroupInfo.Scheme, Linker: apiGroupInfo.GroupMeta.SelfLinker, - Mapper: apiGroupInfo.GroupMeta.RESTMapper, + RootScopedKinds: apiGroupInfo.GroupMeta.RootScopedKinds, Admit: s.admissionControl, MinRequestTimeout: s.minRequestTimeout, diff --git a/test/integration/apiserver/print_test.go b/test/integration/apiserver/print_test.go index 3f0fc636868..3eeb3afde44 100644 --- a/test/integration/apiserver/print_test.go +++ b/test/integration/apiserver/print_test.go @@ -192,7 +192,7 @@ func TestServerSidePrint(t *testing.T) { continue } intGV := gvk.GroupKind().WithVersion(runtime.APIVersionInternal).GroupVersion() - intObj, err := mapping.ConvertToVersion(obj, intGV) + intObj, err := legacyscheme.Scheme.ConvertToVersion(obj, intGV) if err != nil { t.Errorf("unexpected error converting %s to internal: %v", gvk, err) continue From b8177bb9af74f2a49942a659269b43e73201837d Mon Sep 17 00:00:00 2001 From: David Eads Date: Thu, 26 Apr 2018 11:21:41 -0400 Subject: [PATCH 2/2] tighten .Info for kubectl to avoid unpredictable conversion --- pkg/kubectl/cmd/annotate.go | 7 +---- pkg/kubectl/cmd/apply.go | 6 ++-- pkg/kubectl/cmd/create/BUILD | 1 + pkg/kubectl/cmd/create/create_job.go | 8 ++++- pkg/kubectl/cmd/drain.go | 3 +- pkg/kubectl/cmd/label.go | 2 +- pkg/kubectl/cmd/patch.go | 4 +-- pkg/kubectl/cmd/replace.go | 4 +-- pkg/kubectl/cmd/rollingupdate.go | 8 +++-- pkg/kubectl/cmd/set/BUILD | 1 + pkg/kubectl/cmd/set/set_env.go | 3 +- pkg/kubectl/cmd/taint.go | 3 +- pkg/kubectl/cmd/testing/fake.go | 9 +++--- pkg/kubectl/cmd/util/editor/editoptions.go | 10 +++---- pkg/kubectl/cmd/util/factory_builder.go | 9 +++--- pkg/kubectl/resource/BUILD | 1 - pkg/kubectl/resource/builder.go | 26 ++++++++-------- pkg/kubectl/resource/mapper.go | 12 ++++---- pkg/kubectl/resource/selector.go | 6 ++-- pkg/kubectl/resource/visitor.go | 35 ++++------------------ 20 files changed, 71 insertions(+), 87 deletions(-) diff --git a/pkg/kubectl/cmd/annotate.go b/pkg/kubectl/cmd/annotate.go index 52b9deddc4c..8a37c00980e 100644 --- a/pkg/kubectl/cmd/annotate.go +++ b/pkg/kubectl/cmd/annotate.go @@ -246,12 +246,7 @@ func (o AnnotateOptions) RunAnnotate(f cmdutil.Factory, cmd *cobra.Command) erro } var outputObj runtime.Object - var obj runtime.Object - - obj, err = info.ObjectConverter.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupVersion()) - if err != nil { - return err - } + obj := info.Object if o.dryrun || o.local { if err := o.updateAnnotations(obj); err != nil { diff --git a/pkg/kubectl/cmd/apply.go b/pkg/kubectl/cmd/apply.go index d3e47730569..5d0a14b0b9a 100644 --- a/pkg/kubectl/cmd/apply.go +++ b/pkg/kubectl/cmd/apply.go @@ -364,7 +364,7 @@ func (o *ApplyOptions) Run(f cmdutil.Factory, cmd *cobra.Command) error { if err != nil { return err } - return printer.PrintObj(info.AsVersioned(), o.Out) + return printer.PrintObj(info.Object, o.Out) } if !o.DryRun { @@ -415,7 +415,7 @@ func (o *ApplyOptions) Run(f cmdutil.Factory, cmd *cobra.Command) error { if err != nil { return err } - return printer.PrintObj(info.AsVersioned(), o.Out) + return printer.PrintObj(info.Object, o.Out) } } count++ @@ -429,7 +429,7 @@ func (o *ApplyOptions) Run(f cmdutil.Factory, cmd *cobra.Command) error { if err != nil { return err } - return printer.PrintObj(info.AsVersioned(), o.Out) + return printer.PrintObj(info.Object, o.Out) }) if err != nil { return err diff --git a/pkg/kubectl/cmd/create/BUILD b/pkg/kubectl/cmd/create/BUILD index b322e982ca4..53a84bd9ea8 100644 --- a/pkg/kubectl/cmd/create/BUILD +++ b/pkg/kubectl/cmd/create/BUILD @@ -30,6 +30,7 @@ go_library( "//pkg/kubectl/cmd/util/editor:go_default_library", "//pkg/kubectl/genericclioptions:go_default_library", "//pkg/kubectl/resource:go_default_library", + "//pkg/kubectl/scheme:go_default_library", "//pkg/kubectl/util/i18n:go_default_library", "//pkg/printers:go_default_library", "//vendor/github.com/golang/glog:go_default_library", diff --git a/pkg/kubectl/cmd/create/create_job.go b/pkg/kubectl/cmd/create/create_job.go index e52b6b52f42..74462cc9ce0 100644 --- a/pkg/kubectl/cmd/create/create_job.go +++ b/pkg/kubectl/cmd/create/create_job.go @@ -30,6 +30,7 @@ import ( cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/genericclioptions" "k8s.io/kubernetes/pkg/kubectl/resource" + "k8s.io/kubernetes/pkg/kubectl/scheme" "k8s.io/kubernetes/pkg/kubectl/util/i18n" ) @@ -144,7 +145,12 @@ func (o *CreateJobOptions) RunCreateJob() error { if len(infos) != 1 { return fmt.Errorf("from must be an existing cronjob") } - cronJob, ok := infos[0].AsVersioned().(*batchv1beta1.CronJob) + + uncastVersionedObj, err := scheme.Scheme.ConvertToVersion(infos[0].Object, batchv1beta1.SchemeGroupVersion) + if err != nil { + return fmt.Errorf("from must be an existing cronjob: %v", err) + } + cronJob, ok := uncastVersionedObj.(*batchv1beta1.CronJob) if !ok { return fmt.Errorf("from must be an existing cronjob") } diff --git a/pkg/kubectl/cmd/drain.go b/pkg/kubectl/cmd/drain.go index fdcb57a2453..cde9e048e7d 100644 --- a/pkg/kubectl/cmd/drain.go +++ b/pkg/kubectl/cmd/drain.go @@ -43,6 +43,7 @@ import ( restclient "k8s.io/client-go/rest" "k8s.io/kubernetes/pkg/kubectl/genericclioptions" + "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" @@ -718,7 +719,7 @@ func (o *DrainOptions) RunCordonOrUncordon(desired bool) error { for _, nodeInfo := range o.nodeInfos { if nodeInfo.Mapping.GroupVersionKind.Kind == "Node" { - obj, err := nodeInfo.ObjectConverter.ConvertToVersion(nodeInfo.Object, nodeInfo.Mapping.GroupVersionKind.GroupVersion()) + obj, err := legacyscheme.Scheme.ConvertToVersion(nodeInfo.Object, nodeInfo.Mapping.GroupVersionKind.GroupVersion()) if err != nil { fmt.Printf("error: unable to %s node %q: %v", cordonOrUncordon, nodeInfo.Name, err) continue diff --git a/pkg/kubectl/cmd/label.go b/pkg/kubectl/cmd/label.go index 4592b9e6893..2641ef05de4 100644 --- a/pkg/kubectl/cmd/label.go +++ b/pkg/kubectl/cmd/label.go @@ -332,7 +332,7 @@ func (o *LabelOptions) RunLabel(f cmdutil.Factory, cmd *cobra.Command) error { if err != nil { return err } - printer.PrintObj(info.AsVersioned(), o.Out) + printer.PrintObj(info.Object, o.Out) return nil }) } diff --git a/pkg/kubectl/cmd/patch.go b/pkg/kubectl/cmd/patch.go index 9e959cdf067..f8d92f6fb1c 100644 --- a/pkg/kubectl/cmd/patch.go +++ b/pkg/kubectl/cmd/patch.go @@ -242,7 +242,7 @@ func (o *PatchOptions) RunPatch(f cmdutil.Factory, out io.Writer, cmd *cobra.Com if err != nil { return err } - printer.PrintObj(info.AsVersioned(), out) + printer.PrintObj(info.Object, out) // if object was not successfully patched, exit with error code 1 if !didPatch { @@ -285,7 +285,7 @@ func (o *PatchOptions) RunPatch(f cmdutil.Factory, out io.Writer, cmd *cobra.Com if err != nil { return err } - return printer.PrintObj(info.AsVersioned(), out) + return printer.PrintObj(info.Object, out) }) if err != nil { return err diff --git a/pkg/kubectl/cmd/replace.go b/pkg/kubectl/cmd/replace.go index 3c73025097c..8e4e4aa53f9 100644 --- a/pkg/kubectl/cmd/replace.go +++ b/pkg/kubectl/cmd/replace.go @@ -234,7 +234,7 @@ func (o *ReplaceOptions) Run() error { } info.Refresh(obj, true) - return o.PrintObj(info.AsVersioned()) + return o.PrintObj(info.Object) }) } @@ -330,7 +330,7 @@ func (o *ReplaceOptions) forceReplace() error { count++ info.Refresh(obj, true) - return o.PrintObj(info.AsVersioned()) + return o.PrintObj(info.Object) }) if err != nil { return err diff --git a/pkg/kubectl/cmd/rollingupdate.go b/pkg/kubectl/cmd/rollingupdate.go index 0ae18b3e725..748a4ecb897 100644 --- a/pkg/kubectl/cmd/rollingupdate.go +++ b/pkg/kubectl/cmd/rollingupdate.go @@ -217,12 +217,16 @@ func RunRollingUpdate(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args return cmdutil.UsageErrorf(cmd, "please make sure %s exists and is not empty", filename) } - switch t := infos[0].AsVersioned().(type) { + uncastVersionedObj, err := legacyscheme.Scheme.ConvertToVersion(infos[0].Object, v1.SchemeGroupVersion) + if err != nil { + return err + } + switch t := uncastVersionedObj.(type) { case *v1.ReplicationController: replicasDefaulted = t.Spec.Replicas == nil // previous code ignored the error. Seem like it's very unlikely to fail, so ok for now. - uncastObj, err := legacyscheme.Scheme.ConvertToVersion(t, api.SchemeGroupVersion) + uncastObj, err := legacyscheme.Scheme.ConvertToVersion(uncastVersionedObj, api.SchemeGroupVersion) if err == nil { newRc, _ = uncastObj.(*api.ReplicationController) } diff --git a/pkg/kubectl/cmd/set/BUILD b/pkg/kubectl/cmd/set/BUILD index bdde3d4ce8b..a7dd1844c99 100644 --- a/pkg/kubectl/cmd/set/BUILD +++ b/pkg/kubectl/cmd/set/BUILD @@ -19,6 +19,7 @@ go_library( importpath = "k8s.io/kubernetes/pkg/kubectl/cmd/set", visibility = ["//build/visible_to:pkg_kubectl_cmd_set_CONSUMERS"], deps = [ + "//pkg/api/legacyscheme:go_default_library", "//pkg/apis/rbac:go_default_library", "//pkg/kubectl:go_default_library", "//pkg/kubectl/cmd/templates:go_default_library", diff --git a/pkg/kubectl/cmd/set/set_env.go b/pkg/kubectl/cmd/set/set_env.go index efbed9c2299..247f7b643e9 100644 --- a/pkg/kubectl/cmd/set/set_env.go +++ b/pkg/kubectl/cmd/set/set_env.go @@ -35,6 +35,7 @@ import ( "k8s.io/kubernetes/pkg/printers" utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/kubectl/genericclioptions" ) @@ -272,7 +273,7 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error { } for _, info := range infos { - versionedObject, err := info.ObjectConverter.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupVersion()) + versionedObject, err := legacyscheme.Scheme.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupVersion()) if err != nil { return err } diff --git a/pkg/kubectl/cmd/taint.go b/pkg/kubectl/cmd/taint.go index 49a20d9e1f8..2688ec94ce0 100644 --- a/pkg/kubectl/cmd/taint.go +++ b/pkg/kubectl/cmd/taint.go @@ -30,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" @@ -233,7 +234,7 @@ func (o TaintOptions) RunTaint() error { return err } - obj, err := info.ObjectConverter.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupVersion()) + obj, err := legacyscheme.Scheme.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupVersion()) if err != nil { return err } diff --git a/pkg/kubectl/cmd/testing/fake.go b/pkg/kubectl/cmd/testing/fake.go index 8fae54e02a3..e745da1a34a 100644 --- a/pkg/kubectl/cmd/testing/fake.go +++ b/pkg/kubectl/cmd/testing/fake.go @@ -353,11 +353,10 @@ func (f *TestFactory) NewBuilder() *resource.Builder { Decoder: cmdutil.InternalVersionDecoder(), }, &resource.Mapper{ - RESTMapper: mapper, - ObjectTyper: typer, - ClientMapper: resource.ClientMapperFunc(f.UnstructuredClientForMapping), - ObjectConverter: unstructured.UnstructuredObjectConverter{}, - Decoder: unstructured.UnstructuredJSONScheme, + RESTMapper: mapper, + ObjectTyper: typer, + ClientMapper: resource.ClientMapperFunc(f.UnstructuredClientForMapping), + Decoder: unstructured.UnstructuredJSONScheme, }, f.CategoryExpander(), ) diff --git a/pkg/kubectl/cmd/util/editor/editoptions.go b/pkg/kubectl/cmd/util/editor/editoptions.go index dd16245521c..324670dda44 100644 --- a/pkg/kubectl/cmd/util/editor/editoptions.go +++ b/pkg/kubectl/cmd/util/editor/editoptions.go @@ -441,7 +441,7 @@ func (o *EditOptions) visitToApplyEditPatch(originalInfos []*resource.Info, patc if err != nil { return err } - printer.PrintObj(info.AsVersioned(), o.Out) + printer.PrintObj(info.Object, o.Out) return nil } else { err := o.annotationPatch(info) @@ -453,7 +453,7 @@ func (o *EditOptions) visitToApplyEditPatch(originalInfos []*resource.Info, patc if err != nil { return err } - printer.PrintObj(info.AsVersioned(), o.Out) + printer.PrintObj(info.Object, o.Out) return nil } }) @@ -576,7 +576,7 @@ func (o *EditOptions) visitToPatch(originalInfos []*resource.Info, patchVisitor if err != nil { return err } - printer.PrintObj(info.AsVersioned(), o.Out) + printer.PrintObj(info.Object, o.Out) return nil } @@ -634,7 +634,7 @@ func (o *EditOptions) visitToPatch(originalInfos []*resource.Info, patchVisitor if err != nil { return err } - printer.PrintObj(info.AsVersioned(), o.Out) + printer.PrintObj(info.Object, o.Out) return nil }) return err @@ -649,7 +649,7 @@ func (o *EditOptions) visitToCreate(createVisitor resource.Visitor) error { if err != nil { return err } - printer.PrintObj(info.AsVersioned(), o.Out) + printer.PrintObj(info.Object, o.Out) return nil }) return err diff --git a/pkg/kubectl/cmd/util/factory_builder.go b/pkg/kubectl/cmd/util/factory_builder.go index 4476ff1ba43..b837f1330bc 100644 --- a/pkg/kubectl/cmd/util/factory_builder.go +++ b/pkg/kubectl/cmd/util/factory_builder.go @@ -63,11 +63,10 @@ func (f *ring2Factory) NewBuilder() *resource.Builder { Decoder: InternalVersionDecoder(), }, &resource.Mapper{ - RESTMapper: mapper, - ObjectTyper: typer, - ObjectConverter: unstructured.UnstructuredObjectConverter{}, - ClientMapper: unstructuredClientMapperFunc, - Decoder: unstructured.UnstructuredJSONScheme, + RESTMapper: mapper, + ObjectTyper: typer, + ClientMapper: unstructuredClientMapperFunc, + Decoder: unstructured.UnstructuredJSONScheme, }, categoryExpander, ) diff --git a/pkg/kubectl/resource/BUILD b/pkg/kubectl/resource/BUILD index 142fc8ce1e9..1253463daa7 100644 --- a/pkg/kubectl/resource/BUILD +++ b/pkg/kubectl/resource/BUILD @@ -29,7 +29,6 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", "//vendor/k8s.io/apimachinery/pkg/fields:go_default_library", "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", diff --git a/pkg/kubectl/resource/builder.go b/pkg/kubectl/resource/builder.go index 30fa1220aef..1fbe71e0afa 100644 --- a/pkg/kubectl/resource/builder.go +++ b/pkg/kubectl/resource/builder.go @@ -186,6 +186,8 @@ func (b *Builder) Unstructured() *Builder { return b } b.mapper = b.unstructured + // the unstructured mapper doesn't do any conversion + b.mapper.ObjectConverter = nil return b } @@ -835,12 +837,12 @@ func (b *Builder) visitByResource() *Result { } info := &Info{ - Client: client, - Mapping: mapping, - ObjectConverter: b.mapper.ObjectConverter, - Namespace: selectorNamespace, - Name: tuple.Name, - Export: b.export, + Client: client, + Mapping: mapping, + toVersionedObjectConverter: b.mapper.ObjectConverter, + Namespace: selectorNamespace, + Name: tuple.Name, + Export: b.export, } items = append(items, info) } @@ -901,12 +903,12 @@ func (b *Builder) visitByName() *Result { visitors := []Visitor{} for _, name := range b.names { info := &Info{ - Client: client, - Mapping: mapping, - ObjectConverter: b.mapper.ObjectConverter, - Namespace: selectorNamespace, - Name: name, - Export: b.export, + Client: client, + Mapping: mapping, + toVersionedObjectConverter: b.mapper.ObjectConverter, + Namespace: selectorNamespace, + Name: name, + Export: b.export, } visitors = append(visitors, info) } diff --git a/pkg/kubectl/resource/mapper.go b/pkg/kubectl/resource/mapper.go index 9d479c87f28..b834b6ca125 100644 --- a/pkg/kubectl/resource/mapper.go +++ b/pkg/kubectl/resource/mapper.go @@ -72,9 +72,9 @@ func (m *Mapper) InfoForData(data []byte, source string) (*Info, error) { resourceVersion, _ := metadataAccessor.ResourceVersion(obj) return &Info{ - Client: client, - Mapping: mapping, - ObjectConverter: m.ObjectConverter, + Client: client, + Mapping: mapping, + toVersionedObjectConverter: m.ObjectConverter, Source: source, Namespace: namespace, @@ -112,9 +112,9 @@ func (m *Mapper) InfoForObject(obj runtime.Object, preferredGVKs []schema.GroupV namespace, _ := metadataAccessor.Namespace(obj) resourceVersion, _ := metadataAccessor.ResourceVersion(obj) return &Info{ - Client: client, - Mapping: mapping, - ObjectConverter: m.ObjectConverter, + Client: client, + Mapping: mapping, + toVersionedObjectConverter: m.ObjectConverter, Namespace: namespace, Name: name, diff --git a/pkg/kubectl/resource/selector.go b/pkg/kubectl/resource/selector.go index 41c47703461..ce973484640 100644 --- a/pkg/kubectl/resource/selector.go +++ b/pkg/kubectl/resource/selector.go @@ -94,9 +94,9 @@ func (r *Selector) Visit(fn VisitorFunc) error { resourceVersion, _ := metadataAccessor.ResourceVersion(list) nextContinueToken, _ := metadataAccessor.Continue(list) info := &Info{ - Client: r.Client, - Mapping: r.Mapping, - ObjectConverter: r.ObjectConverter, + Client: r.Client, + Mapping: r.Mapping, + toVersionedObjectConverter: r.ObjectConverter, Namespace: r.Namespace, ResourceVersion: resourceVersion, diff --git a/pkg/kubectl/resource/visitor.go b/pkg/kubectl/resource/visitor.go index e46ffd92c14..ded1dda3b59 100644 --- a/pkg/kubectl/resource/visitor.go +++ b/pkg/kubectl/resource/visitor.go @@ -32,7 +32,6 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -79,7 +78,7 @@ type Info struct { // from disk. Mapping *meta.RESTMapping // ObjectConverter allows conversion - ObjectConverter runtime.ObjectConvertor + toVersionedObjectConverter runtime.ObjectConvertor // Namespace will be set if the object is namespaced and has a specified value. Namespace string @@ -185,45 +184,21 @@ func (i *Info) ResourceMapping() *meta.RESTMapping { // Versioned returns the object as a Go type in the mapping's version or returns an error. func (i *Info) Versioned() (runtime.Object, error) { - return i.ObjectConverter.ConvertToVersion(i.Object, i.Mapping.GroupVersionKind.GroupVersion()) + return i.toVersionedObjectConverter.ConvertToVersion(i.Object, i.Mapping.GroupVersionKind.GroupVersion()) } // AsVersioned returns the object as a Go object in the external form if possible (matching the // group version kind of the mapping, or i.Object if it cannot be converted. func (i *Info) AsVersioned() runtime.Object { + if i.toVersionedObjectConverter == nil { + panic("attempt to call AsVersioned object using .Unstructured builder") + } if obj, err := i.Versioned(); err == nil { return obj } return i.Object } -// Unstructured returns the current object in unstructured form (as a runtime.Unstructured) -func (i *Info) Unstructured() (runtime.Unstructured, error) { - switch t := i.Object.(type) { - case runtime.Unstructured: - return t, nil - case *runtime.Unknown: - gvk := i.Mapping.GroupVersionKind - out, _, err := unstructured.UnstructuredJSONScheme.Decode(t.Raw, &gvk, nil) - return out.(runtime.Unstructured), err - default: - out := &unstructured.Unstructured{} - if err := i.ObjectConverter.Convert(i.Object, out, nil); err != nil { - return nil, err - } - return out, nil - } -} - -// AsUnstructured returns the object as a Go object in external form as a runtime.Unstructured -// (map of JSON equivalent values) or as i.Object if it cannot be converted. -func (i *Info) AsUnstructured() runtime.Object { - if out, err := i.Unstructured(); err == nil { - return out - } - return i.Object -} - // VisitorList implements Visit for the sub visitors it contains. The first error // returned from a child Visitor will terminate iteration. type VisitorList []Visitor