Merge pull request #41343 from p0lyn0mial/kubectl_get_short_names_from_discovery_api

Automatic merge from submit-queue

shortcut expander will take the list of short names from the api ser…

**What this PR does / why we need it**: the shortcut expander will take the list of short names for resources from the API server during the discovery. For backward compatibility a hardcoded list of short names will always be appended while evaluating a short name.

hack/make-rules/test-cmd-util.sh

@@ -3225,6 +3225,7 @@ runTests() {
 }
 __EOF__
     kube::test::get_object_assert storageclass "{{range.items}}{{$id_field}}:{{end}}" 'storage-class-name:'
+    kube::test::get_object_assert sc "{{range.items}}{{$id_field}}:{{end}}" 'storage-class-name:'
     kubectl delete storageclass storage-class-name "${kube_flags[@]}"
     # Post-condition: no storage classes
     kube::test::get_object_assert storageclass "{{range.items}}{{$id_field}}:{{end}}" ''

pkg/kubectl/cmd/testing/fake.go

@@ -205,6 +205,10 @@ func (d *fakeCachedDiscoveryClient) Fresh() bool {
 func (d *fakeCachedDiscoveryClient) Invalidate() {
 }
 
+func (d *fakeCachedDiscoveryClient) ServerResources() ([]*metav1.APIResourceList, error) {
+	return []*metav1.APIResourceList{}, nil
+}
+
 type TestFactory struct {
 	Mapper             meta.RESTMapper
 	Typer              runtime.ObjectTyper
@@ -271,7 +275,9 @@ func (f *FakeFactory) UnstructuredObject() (meta.RESTMapper, runtime.ObjectTyper
 	mapper := discovery.NewRESTMapper(groupResources, meta.InterfacesForUnstructured)
 	typer := discovery.NewUnstructuredObjectTyper(groupResources)
 
-	return cmdutil.NewShortcutExpander(mapper, nil), typer, nil
+	fakeDs := &fakeCachedDiscoveryClient{}
+	expander, err := cmdutil.NewShortcutExpander(mapper, fakeDs)
+	return expander, typer, err
 }
 
 func (f *FakeFactory) Decoder(bool) runtime.Decoder {
@@ -519,8 +525,9 @@ func (f *fakeAPIFactory) UnstructuredObject() (meta.RESTMapper, runtime.ObjectTy
 	groupResources := testDynamicResources()
 	mapper := discovery.NewRESTMapper(groupResources, meta.InterfacesForUnstructured)
 	typer := discovery.NewUnstructuredObjectTyper(groupResources)
-
-	return cmdutil.NewShortcutExpander(mapper, nil), typer, nil
+	fakeDs := &fakeCachedDiscoveryClient{}
+	expander, err := cmdutil.NewShortcutExpander(mapper, fakeDs)
+	return expander, typer, err
 }
 
 func (f *fakeAPIFactory) Decoder(bool) runtime.Decoder {

pkg/kubectl/cmd/util/cached_discovery_test.go

@@ -101,6 +101,8 @@ type fakeDiscoveryClient struct {
 	resourceCalls int
 	versionCalls  int
 	swaggerCalls  int
+
+	serverResourcesHandler func() ([]*metav1.APIResourceList, error)
 }
 
 var _ discovery.DiscoveryInterface = &fakeDiscoveryClient{}
@@ -141,6 +143,9 @@ func (c *fakeDiscoveryClient) ServerResourcesForGroupVersion(groupVersion string
 
 func (c *fakeDiscoveryClient) ServerResources() ([]*metav1.APIResourceList, error) {
 	c.resourceCalls = c.resourceCalls + 1
+	if c.serverResourcesHandler != nil {
+		return c.serverResourcesHandler()
+	}
 	return []*metav1.APIResourceList{}, nil
 }
 

pkg/kubectl/cmd/util/factory_object_mapping.go

@@ -75,7 +75,8 @@ func (f *ring1Factory) Object() (meta.RESTMapper, runtime.ObjectTyper) {
 	}
 
 	// wrap with shortcuts
-	mapper = NewShortcutExpander(mapper, discoveryClient)
+	mapper, err = NewShortcutExpander(mapper, discoveryClient)
+	CheckErr(err)
 
 	// wrap with output preferences
 	cfg, err := f.clientAccessFactory.ClientConfigForVersion(nil)
@@ -104,7 +105,8 @@ func (f *ring1Factory) UnstructuredObject() (meta.RESTMapper, runtime.ObjectType
 
 	mapper := discovery.NewDeferredDiscoveryRESTMapper(discoveryClient, meta.InterfacesForUnstructured)
 	typer := discovery.NewUnstructuredObjectTyper(groupResources)
-	return NewShortcutExpander(mapper, discoveryClient), typer, nil
+	expander, err := NewShortcutExpander(mapper, discoveryClient)
+	return expander, typer, err
 }
 
 func (f *ring1Factory) ClientForMapping(mapping *meta.RESTMapping) (resource.RESTClient, error) {

pkg/kubectl/cmd/util/factory_test.go

@@ -749,7 +749,11 @@ func TestDiscoveryReplaceAliases(t *testing.T) {
 		},
 	}
 
-	mapper := NewShortcutExpander(testapi.Default.RESTMapper(), nil)
+	ds := &fakeDiscoveryClient{}
+	mapper, err := NewShortcutExpander(testapi.Default.RESTMapper(), ds)
+	if err != nil {
+		t.Fatalf("Unable to create shortcut expander, err = %s", err.Error())
+	}
 	b := resource.NewBuilder(mapper, api.Scheme, fakeClient(), testapi.Default.Codec())
 
 	for _, test := range tests {

pkg/kubectl/cmd/util/shortcut_restmapper.go

@@ -17,16 +17,18 @@ limitations under the License.
 package util
 
 import (
+	"errors"
 	"strings"
 
+	"github.com/golang/glog"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/discovery"
 	"k8s.io/kubernetes/pkg/kubectl"
 )
 
-// ShortcutExpander is a RESTMapper that can be used for Kubernetes resources.   It expands the resource first, then invokes the wrapped
-type ShortcutExpander struct {
+// shortcutExpander is a RESTMapper that can be used for Kubernetes resources.   It expands the resource first, then invokes the wrapped
+type shortcutExpander struct {
 	RESTMapper meta.RESTMapper
 
 	All []schema.GroupResource
@@ -34,17 +36,16 @@ type ShortcutExpander struct {
 	discoveryClient discovery.DiscoveryInterface
 }
 
-var _ meta.RESTMapper = &ShortcutExpander{}
+var _ meta.RESTMapper = &shortcutExpander{}
 
-func NewShortcutExpander(delegate meta.RESTMapper, client discovery.DiscoveryInterface) ShortcutExpander {
-	return ShortcutExpander{All: UserResources, RESTMapper: delegate, discoveryClient: client}
+func NewShortcutExpander(delegate meta.RESTMapper, client discovery.DiscoveryInterface) (shortcutExpander, error) {
+	if client == nil {
+		return shortcutExpander{}, errors.New("Please provide discovery client to shortcut expander")
+	}
+	return shortcutExpander{All: UserResources, RESTMapper: delegate, discoveryClient: client}, nil
 }
 
-func (e ShortcutExpander) getAll() []schema.GroupResource {
-	if e.discoveryClient == nil {
-		return e.All
-	}
-
+func (e shortcutExpander) getAll() []schema.GroupResource {
 	// Check if we have access to server resources
 	apiResources, err := e.discoveryClient.ServerResources()
 	if err != nil {
@@ -70,31 +71,31 @@ func (e ShortcutExpander) getAll() []schema.GroupResource {
 	return availableAll
 }
 
-func (e ShortcutExpander) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) {
+func (e shortcutExpander) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) {
 	return e.RESTMapper.KindFor(e.expandResourceShortcut(resource))
 }
 
-func (e ShortcutExpander) KindsFor(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) {
+func (e shortcutExpander) KindsFor(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) {
 	return e.RESTMapper.KindsFor(e.expandResourceShortcut(resource))
 }
 
-func (e ShortcutExpander) ResourcesFor(resource schema.GroupVersionResource) ([]schema.GroupVersionResource, error) {
+func (e shortcutExpander) ResourcesFor(resource schema.GroupVersionResource) ([]schema.GroupVersionResource, error) {
 	return e.RESTMapper.ResourcesFor(e.expandResourceShortcut(resource))
 }
 
-func (e ShortcutExpander) ResourceFor(resource schema.GroupVersionResource) (schema.GroupVersionResource, error) {
+func (e shortcutExpander) ResourceFor(resource schema.GroupVersionResource) (schema.GroupVersionResource, error) {
 	return e.RESTMapper.ResourceFor(e.expandResourceShortcut(resource))
 }
 
-func (e ShortcutExpander) ResourceSingularizer(resource string) (string, error) {
+func (e shortcutExpander) ResourceSingularizer(resource string) (string, error) {
 	return e.RESTMapper.ResourceSingularizer(e.expandResourceShortcut(schema.GroupVersionResource{Resource: resource}).Resource)
 }
 
-func (e ShortcutExpander) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) {
+func (e shortcutExpander) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) {
 	return e.RESTMapper.RESTMapping(gk, versions...)
 }
 
-func (e ShortcutExpander) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) {
+func (e shortcutExpander) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) {
 	return e.RESTMapper.RESTMappings(gk, versions...)
 }
 
@@ -114,7 +115,7 @@ var UserResources = []schema.GroupResource{
 }
 
 // AliasesForResource returns whether a resource has an alias or not
-func (e ShortcutExpander) AliasesForResource(resource string) ([]string, bool) {
+func (e shortcutExpander) AliasesForResource(resource string) ([]string, bool) {
 	if strings.ToLower(resource) == "all" {
 		var resources []schema.GroupResource
 		if resources = e.getAll(); len(resources) == 0 {
@@ -130,18 +131,31 @@ func (e ShortcutExpander) AliasesForResource(resource string) ([]string, bool) {
 	return []string{expanded}, (expanded != resource)
 }
 
-// getShortcutMappings returns a hardcoded set of tuples.
-// First the list of potential resources will be taken from the instance variable
-// which holds the anticipated result of the discovery API.
-// Next we will fall back to the hardcoded list of resources.
-// Note that the list is ordered by group priority.
-// TODO: Wire this to discovery API.
-func (e ShortcutExpander) getShortcutMappings() ([]kubectl.ResourceShortcuts, error) {
-	res := []kubectl.ResourceShortcuts{
-		{
-			ShortForm: schema.GroupResource{Group: "storage.k8s.io", Resource: "sc"},
-			LongForm:  schema.GroupResource{Group: "storage.k8s.io", Resource: "storageclasses"},
-		},
+// getShortcutMappings returns a set of tuples which holds short names for resources.
+// First the list of potential resources will be taken from the API server.
+// Next we will append the hardcoded list of resources - to be backward compatible with old servers.
+// NOTE that the list is ordered by group priority.
+func (e shortcutExpander) getShortcutMappings() ([]kubectl.ResourceShortcuts, error) {
+	res := []kubectl.ResourceShortcuts{}
+	// get server resources
+	apiResList, err := e.discoveryClient.ServerResources()
+	if err == nil {
+		for _, apiResources := range apiResList {
+			for _, apiRes := range apiResources.APIResources {
+				for _, shortName := range apiRes.ShortNames {
+					gv, err := schema.ParseGroupVersion(apiResources.GroupVersion)
+					if err != nil {
+						glog.V(1).Infof("Unable to parse groupversion = %s due to = %s", apiResources.GroupVersion, err.Error())
+						continue
+					}
+					rs := kubectl.ResourceShortcuts{
+						ShortForm: schema.GroupResource{Group: gv.Group, Resource: shortName},
+						LongForm:  schema.GroupResource{Group: gv.Group, Resource: apiRes.Name},
+					}
+					res = append(res, rs)
+				}
+			}
+		}
 	}
 
 	// append hardcoded short forms at the end of the list
@@ -153,7 +167,7 @@ func (e ShortcutExpander) getShortcutMappings() ([]kubectl.ResourceShortcuts, er
 // (something that a pkg/api/meta.RESTMapper can understand), if it is
 // indeed a shortcut. If no match has been found, we will match on group prefixing.
 // Lastly we will return resource unmodified.
-func (e ShortcutExpander) expandResourceShortcut(resource schema.GroupVersionResource) schema.GroupVersionResource {
+func (e shortcutExpander) expandResourceShortcut(resource schema.GroupVersionResource) schema.GroupVersionResource {
 	// get the shortcut mappings and return on first match.
 	if resources, err := e.getShortcutMappings(); err == nil {
 		for _, item := range resources {

pkg/kubectl/cmd/util/shortcut_restmapper_test.go

@@ -20,6 +20,7 @@ import (
 	"strings"
 	"testing"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/kubernetes/pkg/api/testapi"
 )
@@ -29,38 +30,76 @@ func TestReplaceAliases(t *testing.T) {
 		name     string
 		arg      string
 		expected string
+		srvRes   []*metav1.APIResourceList
 	}{
 		{
 			name:     "no-replacement",
 			arg:      "service",
 			expected: "service",
+			srvRes:   []*metav1.APIResourceList{},
 		},
 		{
 			name:     "all-replacement",
 			arg:      "all",
 			expected: "pods,replicationcontrollers,services,statefulsets,horizontalpodautoscalers,jobs,deployments,replicasets",
+			srvRes:   []*metav1.APIResourceList{},
 		},
 		{
 			name:     "alias-in-comma-separated-arg",
 			arg:      "all,secrets",
 			expected: "pods,replicationcontrollers,services,statefulsets,horizontalpodautoscalers,jobs,deployments,replicasets,secrets",
+			srvRes:   []*metav1.APIResourceList{},
 		},
 		{
-			name:     "sc-resolves-to-storageclasses",
-			arg:      "sc",
-			expected: "storageclasses",
+			name:     "rc-resolves-to-replicationcontrollers",
+			arg:      "rc",
+			expected: "replicationcontrollers",
+			srvRes:   []*metav1.APIResourceList{},
 		},
 		{
 			name:     "storageclasses-no-replacement",
 			arg:      "storageclasses",
 			expected: "storageclasses",
+			srvRes:   []*metav1.APIResourceList{},
+		},
+		{
+			name:     "hpa-priority",
+			arg:      "hpa",
+			expected: "superhorizontalpodautoscalers",
+			srvRes: []*metav1.APIResourceList{
+				{
+					GroupVersion: "autoscaling/v1",
+					APIResources: []metav1.APIResource{
+						{
+							Name:       "superhorizontalpodautoscalers",
+							ShortNames: []string{"hpa"},
+						},
+					},
+				},
+				{
+					GroupVersion: "autoscaling/v1",
+					APIResources: []metav1.APIResource{
+						{
+							Name:       "horizontalpodautoscalers",
+							ShortNames: []string{"hpa"},
+						},
+					},
+				},
+			},
 		},
 	}
 
-	mapper := NewShortcutExpander(testapi.Default.RESTMapper(), nil)
+	ds := &fakeDiscoveryClient{}
+	mapper, err := NewShortcutExpander(testapi.Default.RESTMapper(), ds)
+	if err != nil {
+		t.Fatalf("Unable to create shortcut expander, err %s", err.Error())
+	}
 
 	for _, test := range tests {
 		resources := []string{}
+		ds.serverResourcesHandler = func() ([]*metav1.APIResourceList, error) {
+			return test.srvRes, nil
+		}
 		for _, arg := range strings.Split(test.arg, ",") {
 			curr, _ := mapper.AliasesForResource(arg)
 			resources = append(resources, curr...)
@@ -70,24 +109,55 @@ func TestReplaceAliases(t *testing.T) {
 		}
 	}
 }
+
 func TestKindFor(t *testing.T) {
 	tests := []struct {
 		in       schema.GroupVersionResource
 		expected schema.GroupVersionKind
+		srvRes   []*metav1.APIResourceList
 	}{
 		{
 			in:       schema.GroupVersionResource{Group: "storage.k8s.io", Version: "", Resource: "sc"},
 			expected: schema.GroupVersionKind{Group: "storage.k8s.io", Version: "v1beta1", Kind: "StorageClass"},
+			srvRes: []*metav1.APIResourceList{
+				{
+					GroupVersion: "storage.k8s.io/v1beta1",
+					APIResources: []metav1.APIResource{
+						{
+							Name:       "storageclasses",
+							ShortNames: []string{"sc"},
+						},
+					},
+				},
+			},
 		},
 		{
 			in:       schema.GroupVersionResource{Group: "", Version: "", Resource: "sc"},
 			expected: schema.GroupVersionKind{Group: "storage.k8s.io", Version: "v1beta1", Kind: "StorageClass"},
+			srvRes: []*metav1.APIResourceList{
+				{
+					GroupVersion: "storage.k8s.io/v1beta1",
+					APIResources: []metav1.APIResource{
+						{
+							Name:       "storageclasses",
+							ShortNames: []string{"sc"},
+						},
+					},
+				},
+			},
 		},
 	}
 
-	mapper := NewShortcutExpander(testapi.Default.RESTMapper(), nil)
+	ds := &fakeDiscoveryClient{}
+	mapper, err := NewShortcutExpander(testapi.Default.RESTMapper(), ds)
+	if err != nil {
+		t.Fatalf("Unable to create shortcut expander, err %s", err.Error())
+	}
 
 	for i, test := range tests {
+		ds.serverResourcesHandler = func() ([]*metav1.APIResourceList, error) {
+			return test.srvRes, nil
+		}
 		ret, err := mapper.KindFor(test.in)
 		if err != nil {
 			t.Errorf("%d: unexpected error returned %s", i, err.Error())

pkg/registry/storage/storageclass/storage/BUILD

@@ -37,6 +37,7 @@ go_library(
         "//vendor:k8s.io/apimachinery/pkg/runtime",
         "//vendor:k8s.io/apiserver/pkg/registry/generic",
         "//vendor:k8s.io/apiserver/pkg/registry/generic/registry",
+        "//vendor:k8s.io/apiserver/pkg/registry/rest",
     ],
 )
 

pkg/registry/storage/storageclass/storage/storage.go

@@ -20,6 +20,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/registry/generic"
 	genericregistry "k8s.io/apiserver/pkg/registry/generic/registry"
+	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/kubernetes/pkg/api"
 	storageapi "k8s.io/kubernetes/pkg/apis/storage"
 	"k8s.io/kubernetes/pkg/registry/cachesize"
@@ -55,3 +56,11 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST {
 
 	return &REST{store}
 }
+
+// Implement ShortNamesProvider
+var _ rest.ShortNamesProvider = &REST{}
+
+// ShortNames implements the ShortNamesProvider interface. Returns a list of short names for a resource.
+func (r *REST) ShortNames() []string {
+	return []string{"sc"}
+}

pkg/registry/storage/storageclass/storage/storage_test.go

@@ -145,3 +145,11 @@ func TestWatch(t *testing.T) {
 		},
 	)
 }
+
+func TestShortNames(t *testing.T) {
+	storage, server := newStorage(t)
+	defer server.Terminate(t)
+	defer storage.Store.DestroyFunc()
+	expected := []string{"sc"}
+	registrytest.AssertShortNames(t, storage, expected)
+}