From ad87219b2cba1d22ac2a808568da7322775972ae Mon Sep 17 00:00:00 2001 From: David Eads Date: Wed, 9 May 2018 11:12:32 -0400 Subject: [PATCH] category expansion can only come from the server --- hack/.golint_failures | 1 - pkg/kubectl/BUILD | 1 - pkg/kubectl/categories/BUILD | 26 -------- pkg/kubectl/categories/categories.go | 46 ------------- pkg/kubectl/cmd/testing/BUILD | 1 - pkg/kubectl/cmd/testing/fake.go | 5 +- pkg/kubectl/cmd/util/BUILD | 2 - pkg/kubectl/cmd/util/factory.go | 2 +- pkg/kubectl/cmd/util/factory_builder.go | 6 +- .../cmd/util/factory_object_mapping.go | 19 ++---- pkg/kubectl/cmd/util/factory_test.go | 3 +- pkg/kubectl/resource/BUILD | 2 +- pkg/kubectl/resource/builder.go | 3 + pkg/kubectl/resource/builder_test.go | 4 +- pkg/kubectl/resource/fake.go | 40 ++++++++++++ .../restmapper/category_expansion.go | 65 +++---------------- .../restmapper/category_expansion_test.go | 2 +- 17 files changed, 66 insertions(+), 162 deletions(-) delete mode 100644 pkg/kubectl/categories/BUILD delete mode 100644 pkg/kubectl/categories/categories.go create mode 100644 pkg/kubectl/resource/fake.go diff --git a/hack/.golint_failures b/hack/.golint_failures index 40b156e9d0..35c9166c2d 100644 --- a/hack/.golint_failures +++ b/hack/.golint_failures @@ -134,7 +134,6 @@ pkg/kubeapiserver/authorizer/modes pkg/kubeapiserver/options pkg/kubeapiserver/server pkg/kubectl -pkg/kubectl/categories pkg/kubectl/cmd pkg/kubectl/cmd/auth pkg/kubectl/cmd/config diff --git a/pkg/kubectl/BUILD b/pkg/kubectl/BUILD index d0cda5d351..e28fdf408b 100644 --- a/pkg/kubectl/BUILD +++ b/pkg/kubectl/BUILD @@ -200,7 +200,6 @@ filegroup( ":package-srcs", "//pkg/kubectl/apply:all-srcs", "//pkg/kubectl/apps:all-srcs", - "//pkg/kubectl/categories:all-srcs", "//pkg/kubectl/cmd:all-srcs", "//pkg/kubectl/explain:all-srcs", "//pkg/kubectl/genericclioptions:all-srcs", diff --git a/pkg/kubectl/categories/BUILD b/pkg/kubectl/categories/BUILD deleted file mode 100644 index 0e195f3d58..0000000000 --- a/pkg/kubectl/categories/BUILD +++ /dev/null @@ -1,26 +0,0 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") - -go_library( - name = "go_default_library", - srcs = ["categories.go"], - importpath = "k8s.io/kubernetes/pkg/kubectl/categories", - visibility = ["//visibility:public"], - deps = [ - "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", - "//vendor/k8s.io/client-go/restmapper:go_default_library", - ], -) - -filegroup( - name = "package-srcs", - srcs = glob(["**"]), - tags = ["automanaged"], - visibility = ["//visibility:private"], -) - -filegroup( - name = "all-srcs", - srcs = [":package-srcs"], - tags = ["automanaged"], - visibility = ["//visibility:public"], -) diff --git a/pkg/kubectl/categories/categories.go b/pkg/kubectl/categories/categories.go deleted file mode 100644 index 0d00e2cc62..0000000000 --- a/pkg/kubectl/categories/categories.go +++ /dev/null @@ -1,46 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package categories - -import ( - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/restmapper" -) - -// legacyUserResources are the resource names that apply to the primary, user facing resources used by -// client tools. They are in deletion-first order - dependent resources should be last. -// Should remain exported in order to expose a current list of resources to downstream -// composition that wants to build on the concept of 'all' for their CLIs. -var legacyUserResources = []schema.GroupResource{ - {Group: "", Resource: "pods"}, - {Group: "", Resource: "replicationcontrollers"}, - {Group: "", Resource: "services"}, - {Group: "apps", Resource: "statefulsets"}, - {Group: "autoscaling", Resource: "horizontalpodautoscalers"}, - {Group: "batch", Resource: "jobs"}, - {Group: "batch", Resource: "cronjobs"}, - {Group: "extensions", Resource: "daemonsets"}, - {Group: "extensions", Resource: "deployments"}, - {Group: "extensions", Resource: "replicasets"}, -} - -// LegacyCategoryExpander is the old hardcoded expansion -var LegacyCategoryExpander restmapper.CategoryExpander = restmapper.SimpleCategoryExpander{ - Expansions: map[string][]schema.GroupResource{ - "all": legacyUserResources, - }, -} diff --git a/pkg/kubectl/cmd/testing/BUILD b/pkg/kubectl/cmd/testing/BUILD index 1d4ed96271..c2f577e9f6 100644 --- a/pkg/kubectl/cmd/testing/BUILD +++ b/pkg/kubectl/cmd/testing/BUILD @@ -13,7 +13,6 @@ go_library( "//pkg/apis/core:go_default_library", "//pkg/client/clientset_generated/internalclientset:go_default_library", "//pkg/kubectl:go_default_library", - "//pkg/kubectl/categories:go_default_library", "//pkg/kubectl/cmd/util:go_default_library", "//pkg/kubectl/cmd/util/openapi:go_default_library", "//pkg/kubectl/cmd/util/openapi/testing:go_default_library", diff --git a/pkg/kubectl/cmd/testing/fake.go b/pkg/kubectl/cmd/testing/fake.go index 4e69bdf061..e96f95abdb 100644 --- a/pkg/kubectl/cmd/testing/fake.go +++ b/pkg/kubectl/cmd/testing/fake.go @@ -47,7 +47,6 @@ import ( api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" "k8s.io/kubernetes/pkg/kubectl" - "k8s.io/kubernetes/pkg/kubectl/categories" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi" openapitesting "k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi/testing" @@ -287,8 +286,8 @@ func (f *TestFactory) Cleanup() { os.Remove(f.tempConfigFile.Name()) } -func (f *TestFactory) CategoryExpander() restmapper.CategoryExpander { - return categories.LegacyCategoryExpander +func (f *TestFactory) CategoryExpander() (restmapper.CategoryExpander, error) { + return resource.FakeCategoryExpander, nil } func (f *TestFactory) ClientConfig() (*restclient.Config, error) { diff --git a/pkg/kubectl/cmd/util/BUILD b/pkg/kubectl/cmd/util/BUILD index d28586d799..3ac8cfa552 100644 --- a/pkg/kubectl/cmd/util/BUILD +++ b/pkg/kubectl/cmd/util/BUILD @@ -26,7 +26,6 @@ go_library( "//pkg/client/clientset_generated/internalclientset/typed/core/internalversion:go_default_library", "//pkg/controller:go_default_library", "//pkg/kubectl:go_default_library", - "//pkg/kubectl/categories:go_default_library", "//pkg/kubectl/cmd/templates:go_default_library", "//pkg/kubectl/cmd/util/openapi:go_default_library", "//pkg/kubectl/cmd/util/openapi/validation:go_default_library", @@ -97,7 +96,6 @@ go_test( "//pkg/client/clientset_generated/internalclientset/fake:go_default_library", "//pkg/controller:go_default_library", "//pkg/kubectl:go_default_library", - "//pkg/kubectl/categories:go_default_library", "//pkg/kubectl/resource:go_default_library", "//vendor/github.com/googleapis/gnostic/OpenAPIv2:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index fc1bbaeb40..2b4e2fc5c0 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -169,7 +169,7 @@ type ClientAccessFactory interface { // Generally they provide object typing and functions that build requests based on the negotiated clients. type ObjectMappingFactory interface { // Returns interface for expanding categories like `all`. - CategoryExpander() restmapper.CategoryExpander + CategoryExpander() (restmapper.CategoryExpander, error) // Returns a RESTClient for working with the specified RESTMapping or an error. This is intended // for working with arbitrary resources and is not guaranteed to point to a Kubernetes APIServer. ClientForMapping(mapping *meta.RESTMapping) (resource.RESTClient, error) diff --git a/pkg/kubectl/cmd/util/factory_builder.go b/pkg/kubectl/cmd/util/factory_builder.go index 6e9eedacd3..571e9dc19f 100644 --- a/pkg/kubectl/cmd/util/factory_builder.go +++ b/pkg/kubectl/cmd/util/factory_builder.go @@ -46,13 +46,15 @@ func NewBuilderFactory(clientAccessFactory ClientAccessFactory, objectMappingFac // NewBuilder returns a new resource builder for structured api objects. func (f *ring2Factory) NewBuilder() *resource.Builder { mapper, mapperErr := f.clientAccessFactory.RESTMapper() + categoryExpander, categoryExpanderError := f.objectMappingFactory.CategoryExpander() - categoryExpander := f.objectMappingFactory.CategoryExpander() return resource.NewBuilder( f.clientAccessFactory.ClientConfig, mapper, categoryExpander, - ).AddError(mapperErr) + ). + AddError(mapperErr). + AddError(categoryExpanderError) } // PluginLoader loads plugins from a path set by the KUBECTL_PLUGINS_PATH env var. diff --git a/pkg/kubectl/cmd/util/factory_object_mapping.go b/pkg/kubectl/cmd/util/factory_object_mapping.go index efb80d6507..fd62e3afde 100644 --- a/pkg/kubectl/cmd/util/factory_object_mapping.go +++ b/pkg/kubectl/cmd/util/factory_object_mapping.go @@ -48,7 +48,6 @@ import ( "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/kubectl" - "k8s.io/kubernetes/pkg/kubectl/categories" "k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi" openapivalidation "k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi/validation" "k8s.io/kubernetes/pkg/kubectl/resource" @@ -76,23 +75,13 @@ func NewObjectMappingFactory(clientAccessFactory ClientAccessFactory) ObjectMapp return f } -func (f *ring1Factory) CategoryExpander() restmapper.CategoryExpander { - legacyExpander := categories.LegacyCategoryExpander - +func (f *ring1Factory) CategoryExpander() (restmapper.CategoryExpander, error) { discoveryClient, err := f.clientAccessFactory.DiscoveryClient() - if err == nil { - // fallback is the legacy expander wrapped with discovery based filtering - fallbackExpander, err := restmapper.NewDiscoveryFilteredExpander(legacyExpander, discoveryClient) - CheckErr(err) - - // by default use the expander that discovers based on "categories" field from the API - discoveryCategoryExpander, err := restmapper.NewDiscoveryCategoryExpander(fallbackExpander, discoveryClient) - CheckErr(err) - - return discoveryCategoryExpander + if err != nil { + return nil, err } - return legacyExpander + return restmapper.NewDiscoveryCategoryExpander(discoveryClient) } func (f *ring1Factory) ClientForMapping(mapping *meta.RESTMapping) (resource.RESTClient, error) { diff --git a/pkg/kubectl/cmd/util/factory_test.go b/pkg/kubectl/cmd/util/factory_test.go index f737a9aec4..c9111cee01 100644 --- a/pkg/kubectl/cmd/util/factory_test.go +++ b/pkg/kubectl/cmd/util/factory_test.go @@ -40,7 +40,6 @@ import ( "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/kubectl" - "k8s.io/kubernetes/pkg/kubectl/categories" "k8s.io/kubernetes/pkg/kubectl/resource" ) @@ -472,7 +471,7 @@ func TestDiscoveryReplaceAliases(t *testing.T) { ds := &fakeDiscoveryClient{} mapper := restmapper.NewShortcutExpander(testrestmapper.TestOnlyStaticRESTMapper(legacyscheme.Scheme), ds) - b := resource.NewFakeBuilder(fakeClient(), mapper, categories.LegacyCategoryExpander) + b := resource.NewFakeBuilder(fakeClient(), mapper, resource.FakeCategoryExpander) for _, test := range tests { replaced := b.ReplaceAliases(test.arg) diff --git a/pkg/kubectl/resource/BUILD b/pkg/kubectl/resource/BUILD index ea2f78bb10..ed6223d03a 100644 --- a/pkg/kubectl/resource/BUILD +++ b/pkg/kubectl/resource/BUILD @@ -10,6 +10,7 @@ go_library( "builder.go", "client.go", "doc.go", + "fake.go", "helper.go", "interfaces.go", "mapper.go", @@ -61,7 +62,6 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/apis/core/install:go_default_library", - "//pkg/kubectl/categories:go_default_library", "//pkg/kubectl/scheme:go_default_library", "//vendor/github.com/davecgh/go-spew/spew:go_default_library", "//vendor/github.com/ghodss/yaml:go_default_library", diff --git a/pkg/kubectl/resource/builder.go b/pkg/kubectl/resource/builder.go index 47f3df6644..0ab4f920c6 100644 --- a/pkg/kubectl/resource/builder.go +++ b/pkg/kubectl/resource/builder.go @@ -543,6 +543,9 @@ func (b *Builder) ResourceTypeOrNameArgs(allowEmptySelector bool, args ...string func (b *Builder) ReplaceAliases(input string) string { replaced := []string{} for _, arg := range strings.Split(input, ",") { + if b.categoryExpander == nil { + continue + } if resources, ok := b.categoryExpander.Expand(arg); ok { asStrings := []string{} for _, resource := range resources { diff --git a/pkg/kubectl/resource/builder_test.go b/pkg/kubectl/resource/builder_test.go index ceb29ced46..f13f280b65 100644 --- a/pkg/kubectl/resource/builder_test.go +++ b/pkg/kubectl/resource/builder_test.go @@ -47,7 +47,6 @@ import ( "k8s.io/client-go/rest/fake" restclientwatch "k8s.io/client-go/rest/watch" utiltesting "k8s.io/client-go/util/testing" - "k8s.io/kubernetes/pkg/kubectl/categories" "k8s.io/kubernetes/pkg/kubectl/scheme" // install the pod scheme into the legacy scheme for test typer resolution @@ -59,7 +58,6 @@ var ( corev1GV = schema.GroupVersion{Version: "v1"} corev1Codec = scheme.Codecs.CodecForVersions(scheme.Codecs.LegacyCodec(corev1GV), scheme.Codecs.UniversalDecoder(corev1GV), corev1GV, corev1GV) metaAccessor = meta.NewAccessor() - restmapper = testrestmapper.TestOnlyStaticRESTMapper(scheme.Scheme) ) func stringBody(body string) io.ReadCloser { @@ -273,7 +271,7 @@ func newDefaultBuilder() *Builder { } func newDefaultBuilderWith(fakeClientFn FakeClientFunc) *Builder { - return NewFakeBuilder(fakeClientFn, restmapper, categories.LegacyCategoryExpander). + return NewFakeBuilder(fakeClientFn, testrestmapper.TestOnlyStaticRESTMapper(scheme.Scheme), FakeCategoryExpander). WithScheme(scheme.Scheme, scheme.Scheme.PrioritizedVersionsAllGroups()...) } diff --git a/pkg/kubectl/resource/fake.go b/pkg/kubectl/resource/fake.go new file mode 100644 index 0000000000..276c343e21 --- /dev/null +++ b/pkg/kubectl/resource/fake.go @@ -0,0 +1,40 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resource + +import ( + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/restmapper" +) + +// FakeCategoryExpander is for testing only +var FakeCategoryExpander restmapper.CategoryExpander = restmapper.SimpleCategoryExpander{ + Expansions: map[string][]schema.GroupResource{ + "all": { + {Group: "", Resource: "pods"}, + {Group: "", Resource: "replicationcontrollers"}, + {Group: "", Resource: "services"}, + {Group: "apps", Resource: "statefulsets"}, + {Group: "autoscaling", Resource: "horizontalpodautoscalers"}, + {Group: "batch", Resource: "jobs"}, + {Group: "batch", Resource: "cronjobs"}, + {Group: "extensions", Resource: "daemonsets"}, + {Group: "extensions", Resource: "deployments"}, + {Group: "extensions", Resource: "replicasets"}, + }, + }, +} diff --git a/staging/src/k8s.io/client-go/restmapper/category_expansion.go b/staging/src/k8s.io/client-go/restmapper/category_expansion.go index 207a572935..fe6f884bce 100644 --- a/staging/src/k8s.io/client-go/restmapper/category_expansion.go +++ b/staging/src/k8s.io/client-go/restmapper/category_expansion.go @@ -33,6 +33,7 @@ type SimpleCategoryExpander struct { Expansions map[string][]schema.GroupResource } +// Expand fulfills CategoryExpander func (e SimpleCategoryExpander) Expand(category string) ([]schema.GroupResource, bool) { ret, ok := e.Expansions[category] return ret, ok @@ -41,33 +42,32 @@ func (e SimpleCategoryExpander) Expand(category string) ([]schema.GroupResource, // discoveryCategoryExpander struct lets a REST Client wrapper (discoveryClient) to retrieve list of APIResourceList, // and then convert to fallbackExpander type discoveryCategoryExpander struct { - fallbackExpander CategoryExpander - discoveryClient discovery.DiscoveryInterface + discoveryClient discovery.DiscoveryInterface } // NewDiscoveryCategoryExpander returns a category expander that makes use of the "categories" fields from // the API, found through the discovery client. In case of any error or no category found (which likely // means we're at a cluster prior to categories support, fallback to the expander provided. -func NewDiscoveryCategoryExpander(fallbackExpander CategoryExpander, client discovery.DiscoveryInterface) (discoveryCategoryExpander, error) { +func NewDiscoveryCategoryExpander(client discovery.DiscoveryInterface) (CategoryExpander, error) { if client == nil { panic("Please provide discovery client to shortcut expander") } - return discoveryCategoryExpander{fallbackExpander: fallbackExpander, discoveryClient: client}, nil + return discoveryCategoryExpander{discoveryClient: client}, nil } +// Expand fulfills CategoryExpander func (e discoveryCategoryExpander) Expand(category string) ([]schema.GroupResource, bool) { // Get all supported resources for groups and versions from server, if no resource found, fallback anyway. apiResourceLists, _ := e.discoveryClient.ServerResources() if len(apiResourceLists) == 0 { - return e.fallbackExpander.Expand(category) + return nil, false } discoveredExpansions := map[string][]schema.GroupResource{} - for _, apiResourceList := range apiResourceLists { gv, err := schema.ParseGroupVersion(apiResourceList.GroupVersion) if err != nil { - return e.fallbackExpander.Expand(category) + continue } // Collect GroupVersions by categories for _, apiResource := range apiResourceList.APIResources { @@ -83,64 +83,15 @@ func (e discoveryCategoryExpander) Expand(category string) ([]schema.GroupResour } } - if len(discoveredExpansions) == 0 { - // We don't know if the server really don't have any resource with categories, - // or we're on a cluster version prior to categories support. Anyways, fallback. - return e.fallbackExpander.Expand(category) - } - ret, ok := discoveredExpansions[category] return ret, ok } -// discoveryFilteredExpander expands the given CategoryExpander (delegate) to filter group and resource returned from server -type discoveryFilteredExpander struct { - delegate CategoryExpander - - discoveryClient discovery.DiscoveryInterface -} - -// NewDiscoveryFilteredExpander returns a category expander that filters the returned groupresources -// by what the server has available -func NewDiscoveryFilteredExpander(delegate CategoryExpander, client discovery.DiscoveryInterface) (discoveryFilteredExpander, error) { - if client == nil { - panic("Please provide discovery client to shortcut expander") - } - return discoveryFilteredExpander{delegate: delegate, discoveryClient: client}, nil -} - -func (e discoveryFilteredExpander) Expand(category string) ([]schema.GroupResource, bool) { - delegateExpansion, ok := e.delegate.Expand(category) - - // Check if we have access to server resources - apiResources, err := e.discoveryClient.ServerResources() - if err != nil { - return delegateExpansion, ok - } - - availableResources, err := discovery.GroupVersionResources(apiResources) - if err != nil { - return delegateExpansion, ok - } - - available := []schema.GroupResource{} - for _, requestedResource := range delegateExpansion { - for availableResource := range availableResources { - if requestedResource.Group == availableResource.Group && - requestedResource.Resource == availableResource.Resource { - available = append(available, requestedResource) - break - } - } - } - - return available, ok -} - // UnionCategoryExpander implements CategoryExpander interface. // It maps given category string to union of expansions returned by all the CategoryExpanders in the list. type UnionCategoryExpander []CategoryExpander +// Expand fulfills CategoryExpander func (u UnionCategoryExpander) Expand(category string) ([]schema.GroupResource, bool) { ret := []schema.GroupResource{} ok := false diff --git a/staging/src/k8s.io/client-go/restmapper/category_expansion_test.go b/staging/src/k8s.io/client-go/restmapper/category_expansion_test.go index ac42307f69..bda77a89c4 100644 --- a/staging/src/k8s.io/client-go/restmapper/category_expansion_test.go +++ b/staging/src/k8s.io/client-go/restmapper/category_expansion_test.go @@ -135,7 +135,7 @@ func TestDiscoveryCategoryExpander(t *testing.T) { dc.serverResourcesHandler = func() ([]*metav1.APIResourceList, error) { return test.serverResponse, nil } - expander, err := NewDiscoveryCategoryExpander(SimpleCategoryExpander{}, dc) + expander, err := NewDiscoveryCategoryExpander(dc) if err != nil { t.Fatalf("unexpected error %v", err) }