Merge pull request #32493 from deads2k/client-06-discoveryrestmapper

Automatic merge from submit-queue

use discovery restmapper for kubectl

Updates the `kubectl` factory to use a discovery rest mapper for locating resources.  This allows generic gets.

@kargakis @sttts @fabianofranz I'll let you guys fight over it. :)

pkg/api/install/install.go

@@ -86,9 +86,6 @@ func enableVersions(externalVersions []unversioned.GroupVersion) error {
 	return nil
 }
 
-// userResources is a group of resources mostly used by a kubectl user
-var userResources = []string{"rc", "svc", "pods", "pvc"}
-
 func newRESTMapper(externalVersions []unversioned.GroupVersion) meta.RESTMapper {
 	// the list of kinds that are scoped at the root of the api hierarchy
 	// if a kind is not enumerated here, it is assumed to have a namespace scope
@@ -115,8 +112,6 @@ func newRESTMapper(externalVersions []unversioned.GroupVersion) meta.RESTMapper
 		"ThirdPartyResourceList")
 
 	mapper := api.NewDefaultRESTMapper(externalVersions, interfacesFor, importPrefix, ignoredKinds, rootScoped)
-	// setup aliases for groups of resources
-	mapper.AddResourceAlias("all", userResources...)
 
 	return mapper
 }

pkg/api/meta/firsthit_restmapper.go

@@ -0,0 +1,97 @@
+/*
+Copyright 2014 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 meta
+
+import (
+	"fmt"
+
+	"k8s.io/kubernetes/pkg/api/unversioned"
+	utilerrors "k8s.io/kubernetes/pkg/util/errors"
+)
+
+// FirstHitRESTMapper is a wrapper for multiple RESTMappers which returns the
+// first successful result for the singular requests
+type FirstHitRESTMapper struct {
+	MultiRESTMapper
+}
+
+func (m FirstHitRESTMapper) String() string {
+	return fmt.Sprintf("FirstHitRESTMapper{\n\t%v\n}", m.MultiRESTMapper)
+}
+
+func (m FirstHitRESTMapper) ResourceFor(resource unversioned.GroupVersionResource) (unversioned.GroupVersionResource, error) {
+	errors := []error{}
+	for _, t := range m.MultiRESTMapper {
+		ret, err := t.ResourceFor(resource)
+		if err == nil {
+			return ret, nil
+		}
+		errors = append(errors, err)
+	}
+
+	return unversioned.GroupVersionResource{}, collapseAggregateErrors(errors)
+}
+
+func (m FirstHitRESTMapper) KindFor(resource unversioned.GroupVersionResource) (unversioned.GroupVersionKind, error) {
+	errors := []error{}
+	for _, t := range m.MultiRESTMapper {
+		ret, err := t.KindFor(resource)
+		if err == nil {
+			return ret, nil
+		}
+		errors = append(errors, err)
+	}
+
+	return unversioned.GroupVersionKind{}, collapseAggregateErrors(errors)
+}
+
+// RESTMapping provides the REST mapping for the resource based on the
+// kind and version. This implementation supports multiple REST schemas and
+// return the first match.
+func (m FirstHitRESTMapper) RESTMapping(gk unversioned.GroupKind, versions ...string) (*RESTMapping, error) {
+	errors := []error{}
+	for _, t := range m.MultiRESTMapper {
+		ret, err := t.RESTMapping(gk, versions...)
+		if err == nil {
+			return ret, nil
+		}
+		errors = append(errors, err)
+	}
+
+	return nil, collapseAggregateErrors(errors)
+}
+
+// collapseAggregateErrors returns the minimal errors.  it handles empty as nil, handles one item in a list
+// by returning the item, and collapses all NoMatchErrors to a single one (since they should all be the same)
+func collapseAggregateErrors(errors []error) error {
+	if len(errors) == 0 {
+		return nil
+	}
+	if len(errors) == 1 {
+		return errors[0]
+	}
+
+	allNoMatchErrors := true
+	for _, err := range errors {
+		allNoMatchErrors = allNoMatchErrors && IsNoMatchError(err)
+	}
+	if allNoMatchErrors {
+		return errors[0]
+	}
+
+	return utilerrors.NewAggregate(errors)
+}

pkg/api/meta/restmapper.go

@@ -526,7 +526,7 @@ func (m *DefaultRESTMapper) RESTMapping(gk unversioned.GroupKind, versions ...st
 
 	interfaces, err := m.interfacesFunc(gvk.GroupVersion())
 	if err != nil {
-		return nil, fmt.Errorf("the provided version %q has no relevant versions", gvk.GroupVersion().String())
+		return nil, fmt.Errorf("the provided version %q has no relevant versions: %v", gvk.GroupVersion().String(), err)
 	}
 
 	retVal := &RESTMapping{
@@ -565,7 +565,7 @@ func (m *DefaultRESTMapper) RESTMappings(gk unversioned.GroupKind) ([]*RESTMappi
 
 		interfaces, err := m.interfacesFunc(gvk.GroupVersion())
 		if err != nil {
-			return nil, fmt.Errorf("the provided version %q has no relevant versions", gvk.GroupVersion().String())
+			return nil, fmt.Errorf("the provided version %q has no relevant versions: %v", gvk.GroupVersion().String(), err)
 		}
 
 		mappings = append(mappings, &RESTMapping{

pkg/apimachinery/registered/registered.go

@@ -111,6 +111,7 @@ var (
 	EnableVersions                = DefaultAPIRegistrationManager.EnableVersions
 	RegisterGroup                 = DefaultAPIRegistrationManager.RegisterGroup
 	RegisterVersions              = DefaultAPIRegistrationManager.RegisterVersions
+	InterfacesFor                 = DefaultAPIRegistrationManager.InterfacesFor
 )
 
 // RegisterVersions adds the given group versions to the list of registered group versions.
@@ -265,6 +266,15 @@ func (m *APIRegistrationManager) AddThirdPartyAPIGroupVersions(gvs ...unversione
 	return skippedGVs
 }
 
+// InterfacesFor is a union meta.VersionInterfacesFunc func for all registered types
+func (m *APIRegistrationManager) InterfacesFor(version unversioned.GroupVersion) (*meta.VersionInterfaces, error) {
+	groupMeta, err := m.Group(version.Group)
+	if err != nil {
+		return nil, err
+	}
+	return groupMeta.InterfacesFor(version)
+}
+
 // TODO: This is an expedient function, because we don't check if a Group is
 // supported throughout the code base. We will abandon this function and
 // checking the error returned by the Group() function.

pkg/client/typed/discovery/restmapper.go

@@ -17,6 +17,7 @@ limitations under the License.
 package discovery
 
 import (
+	"fmt"
 	"sync"
 
 	"k8s.io/kubernetes/pkg/api/errors"
@@ -259,5 +260,13 @@ func (d *DeferredDiscoveryRESTMapper) ResourceSingularizer(resource string) (sin
 	return del.ResourceSingularizer(resource)
 }
 
+func (d *DeferredDiscoveryRESTMapper) String() string {
+	del, err := d.getDelegate()
+	if err != nil {
+		return fmt.Sprintf("DeferredDiscoveryRESTMapper{%v}", err)
+	}
+	return fmt.Sprintf("DeferredDiscoveryRESTMapper{\n\t%v\n}", del)
+}
+
 // Make sure it satisfies the interface
 var _ meta.RESTMapper = &DeferredDiscoveryRESTMapper{}

pkg/kubectl/cmd/cmd_test.go

@@ -299,7 +299,7 @@ func NewAPIFactory() (*cmdutil.Factory, *testFactory, runtime.Codec, runtime.Neg
 			mapper := discovery.NewRESTMapper(groupResources, meta.InterfacesForUnstructured)
 			typer := discovery.NewUnstructuredObjectTyper(groupResources)
 
-			return kubectl.ShortcutExpander{RESTMapper: mapper}, typer, nil
+			return cmdutil.NewShortcutExpander(mapper), typer, nil
 		},
 		ClientSet: func() (*internalclientset.Clientset, error) {
 			// Swap out the HTTP client out of the client with the fake's version.

pkg/kubectl/cmd/util/factory.go

@@ -32,6 +32,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/blang/semver"
 	"github.com/emicklei/go-restful/swagger"
 	"github.com/imdario/mergo"
 	"github.com/spf13/cobra"
@@ -47,9 +48,9 @@ import (
 	"k8s.io/kubernetes/pkg/apimachinery"
 	"k8s.io/kubernetes/pkg/apimachinery/registered"
 	"k8s.io/kubernetes/pkg/apis/apps"
-	"k8s.io/kubernetes/pkg/apis/autoscaling"
 	"k8s.io/kubernetes/pkg/apis/batch"
 	"k8s.io/kubernetes/pkg/apis/extensions"
+	extensionsv1beta1 "k8s.io/kubernetes/pkg/apis/extensions/v1beta1"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	coreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/unversioned"
 	"k8s.io/kubernetes/pkg/client/restclient"
@@ -278,8 +279,6 @@ func makeInterfacesFor(versionList []unversioned.GroupVersion) func(version unve
 // if optionalClientConfig is nil, then flags will be bound to a new clientcmd.ClientConfig.
 // if optionalClientConfig is not nil, then this factory will make use of it.
 func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
-	mapper := kubectl.ShortcutExpander{RESTMapper: registered.RESTMapper()}
-
 	flags := pflag.NewFlagSet("", pflag.ContinueOnError)
 	flags.SetNormalizeFunc(utilflag.WarnWordSepNormalizeFunc) // Warn for "_" flags
 
@@ -294,8 +293,6 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 		clients: clients,
 		flags:   flags,
 
-		// If discoverDynamicAPIs is true, make API calls to the discovery service to find APIs that
-		// have been dynamically added to the apiserver
 		Object: func(discoverDynamicAPIs bool) (meta.RESTMapper, runtime.ObjectTyper) {
 			cfg, err := clientConfig.ClientConfig()
 			checkErrWithPrefix("failed to get client config: ", err)
@@ -303,82 +300,36 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 			if cfg.GroupVersion != nil {
 				cmdApiVersion = *cfg.GroupVersion
 			}
-			if discoverDynamicAPIs {
-				clientset, err := clients.ClientSetForVersion(&unversioned.GroupVersion{Version: "v1"})
-				checkErrWithPrefix("failed to find client for version v1: ", err)
 
-				var versions []unversioned.GroupVersion
-				var gvks []unversioned.GroupVersionKind
-				retries := 3
-				for i := 0; i < retries; i++ {
-					versions, gvks, err = GetThirdPartyGroupVersions(clientset.Discovery())
-					// Retry if we got a NotFound error, because user may delete
-					// a thirdparty group when the GetThirdPartyGroupVersions is
-					// running.
-					if err == nil || !apierrors.IsNotFound(err) {
-						break
+			mapper := registered.RESTMapper()
+			// if we can find the server version and it's current enough to have discovery information, use it.  Otherwise,
+			// fallback to our hardcoded list
+			if discoveryClient, err := discovery.NewDiscoveryClientForConfig(cfg); err == nil {
+				if serverVersion, err := discoveryClient.ServerVersion(); err == nil && useDiscoveryRESTMapper(serverVersion.GitVersion) {
+					// register third party resources with the api machinery groups.  This probably should be done, but
+					// its consistent with old code, so we'll start with it.
+					if err := registerThirdPartyResources(discoveryClient); err != nil {
+						fmt.Fprintf(os.Stderr, "Unable to register third party resources: %v\n", err)
+					}
+					// ThirdPartyResourceData is special.  It's not discoverable, but needed for thirdparty resource listing
+					// TODO eliminate this once we're truly generic.
+					thirdPartyResourceDataMapper := meta.NewDefaultRESTMapper([]unversioned.GroupVersion{extensionsv1beta1.SchemeGroupVersion}, registered.InterfacesFor)
+					thirdPartyResourceDataMapper.Add(extensionsv1beta1.SchemeGroupVersion.WithKind("ThirdPartyResourceData"), meta.RESTScopeNamespace)
+
+					mapper = meta.FirstHitRESTMapper{
+						MultiRESTMapper: meta.MultiRESTMapper{
+							discovery.NewDeferredDiscoveryRESTMapper(discoveryClient, registered.InterfacesFor),
+							thirdPartyResourceDataMapper,
+						},
 					}
 				}
-				checkErrWithPrefix("failed to get third-party group versions: ", err)
-				if len(versions) > 0 {
-					priorityMapper, ok := mapper.RESTMapper.(meta.PriorityRESTMapper)
-					if !ok {
-						CheckErr(fmt.Errorf("expected PriorityMapper, saw: %v", mapper.RESTMapper))
-						return nil, nil
-					}
-					multiMapper, ok := priorityMapper.Delegate.(meta.MultiRESTMapper)
-					if !ok {
-						CheckErr(fmt.Errorf("unexpected type: %v", mapper.RESTMapper))
-						return nil, nil
-					}
-					groupsMap := map[string][]unversioned.GroupVersion{}
-					for _, version := range versions {
-						groupsMap[version.Group] = append(groupsMap[version.Group], version)
-					}
-					for group, versionList := range groupsMap {
-						preferredExternalVersion := versionList[0]
+			}
 
-						thirdPartyMapper, err := kubectl.NewThirdPartyResourceMapper(versionList, getGroupVersionKinds(gvks, group))
-						checkErrWithPrefix("failed to create third party resource mapper: ", err)
-						accessor := meta.NewAccessor()
-						groupMeta := apimachinery.GroupMeta{
-							GroupVersion:  preferredExternalVersion,
-							GroupVersions: versionList,
-							RESTMapper:    thirdPartyMapper,
-							SelfLinker:    runtime.SelfLinker(accessor),
-							InterfacesFor: makeInterfacesFor(versionList),
-						}
-
-						checkErrWithPrefix("failed to register group: ", registered.RegisterGroup(groupMeta))
-						registered.AddThirdPartyAPIGroupVersions(versionList...)
-						multiMapper = append(meta.MultiRESTMapper{thirdPartyMapper}, multiMapper...)
-					}
-					priorityMapper.Delegate = multiMapper
-					// Reassign to the RESTMapper here because priorityMapper is actually a copy, so if we
-					// don't reassign, the above assignement won't actually update mapper.RESTMapper
-					mapper.RESTMapper = priorityMapper
-				}
-			}
-			outputRESTMapper := kubectl.OutputVersionMapper{RESTMapper: mapper, OutputVersions: []unversioned.GroupVersion{cmdApiVersion}}
-			priorityRESTMapper := meta.PriorityRESTMapper{
-				Delegate: outputRESTMapper,
-			}
-			// TODO: this should come from registered versions
-			groups := []string{api.GroupName, autoscaling.GroupName, extensions.GroupName, federation.GroupName, batch.GroupName}
-			// set a preferred version
-			for _, group := range groups {
-				gvs := registered.EnabledVersionsForGroup(group)
-				if len(gvs) == 0 {
-					continue
-				}
-				priorityRESTMapper.ResourcePriority = append(priorityRESTMapper.ResourcePriority, unversioned.GroupVersionResource{Group: group, Version: gvs[0].Version, Resource: meta.AnyResource})
-				priorityRESTMapper.KindPriority = append(priorityRESTMapper.KindPriority, unversioned.GroupVersionKind{Group: group, Version: gvs[0].Version, Kind: meta.AnyKind})
-			}
-			for _, group := range groups {
-				priorityRESTMapper.ResourcePriority = append(priorityRESTMapper.ResourcePriority, unversioned.GroupVersionResource{Group: group, Version: meta.AnyVersion, Resource: meta.AnyResource})
-				priorityRESTMapper.KindPriority = append(priorityRESTMapper.KindPriority, unversioned.GroupVersionKind{Group: group, Version: meta.AnyVersion, Kind: meta.AnyKind})
-			}
-			return priorityRESTMapper, api.Scheme
+			// wrap with shortcuts
+			mapper = NewShortcutExpander(mapper)
+			// wrap with output preferences
+			mapper = kubectl.OutputVersionMapper{RESTMapper: mapper, OutputVersions: []unversioned.GroupVersion{cmdApiVersion}}
+			return mapper, api.Scheme
 		},
 		UnstructuredObject: func() (meta.RESTMapper, runtime.ObjectTyper, error) {
 			cfg, err := clients.ClientConfigForVersion(nil)
@@ -412,7 +363,7 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 
 			typer := discovery.NewUnstructuredObjectTyper(groupResources)
 
-			return kubectl.ShortcutExpander{RESTMapper: mapper}, typer, nil
+			return NewShortcutExpander(mapper), typer, nil
 		},
 		RESTClient: func() (*restclient.RESTClient, error) {
 			clientConfig, err := clients.ClientConfigForVersion(nil)
@@ -1305,3 +1256,68 @@ func (f *Factory) NewBuilder(thirdPartyDiscovery bool) *resource.Builder {
 
 	return resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true))
 }
+
+// useDiscoveryRESTMapper checks the server version to see if its recent enough to have
+// enough discovery information available to reliably build a RESTMapper.  If not, use the
+// hardcoded mapper in this client (legacy behavior)
+func useDiscoveryRESTMapper(serverVersion string) bool {
+	if len(serverVersion) == 0 {
+		return false
+	}
+	serverSemVer, err := semver.Parse(serverVersion[1:])
+	if err != nil {
+		return false
+	}
+	return serverSemVer.GE(semver.MustParse("1.3.0"))
+}
+
+// registerThirdPartyResources inspects the discovery endpoint to find thirdpartyresources in the discovery doc
+// and then registers them with the apimachinery code.  I think this is done so that scheme/codec stuff works,
+// but I really don't know.  Feels like this code should go away once kubectl is completely generic for generic
+// CRUD
+func registerThirdPartyResources(discoveryClient discovery.DiscoveryInterface) error {
+	var versions []unversioned.GroupVersion
+	var gvks []unversioned.GroupVersionKind
+	var err error
+	retries := 3
+	for i := 0; i < retries; i++ {
+		versions, gvks, err = GetThirdPartyGroupVersions(discoveryClient)
+		// Retry if we got a NotFound error, because user may delete
+		// a thirdparty group when the GetThirdPartyGroupVersions is
+		// running.
+		if err == nil || !apierrors.IsNotFound(err) {
+			break
+		}
+	}
+	if err != nil {
+		return err
+	}
+
+	groupsMap := map[string][]unversioned.GroupVersion{}
+	for _, version := range versions {
+		groupsMap[version.Group] = append(groupsMap[version.Group], version)
+	}
+	for group, versionList := range groupsMap {
+		preferredExternalVersion := versionList[0]
+
+		thirdPartyMapper, err := kubectl.NewThirdPartyResourceMapper(versionList, getGroupVersionKinds(gvks, group))
+		if err != nil {
+			return err
+		}
+
+		accessor := meta.NewAccessor()
+		groupMeta := apimachinery.GroupMeta{
+			GroupVersion:  preferredExternalVersion,
+			GroupVersions: versionList,
+			RESTMapper:    thirdPartyMapper,
+			SelfLinker:    runtime.SelfLinker(accessor),
+			InterfacesFor: makeInterfacesFor(versionList),
+		}
+		if err := registered.RegisterGroup(groupMeta); err != nil {
+			return err
+		}
+		registered.AddThirdPartyAPIGroupVersions(versionList...)
+	}
+
+	return nil
+}

pkg/kubectl/cmd/util/shortcut_restmapper.go

@@ -0,0 +1,89 @@
+/*
+Copyright 2016 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 util
+
+import (
+	"k8s.io/kubernetes/pkg/api/meta"
+	"k8s.io/kubernetes/pkg/api/unversioned"
+	"k8s.io/kubernetes/pkg/kubectl"
+)
+
+// ShortcutExpander is a RESTMapper that can be used for OpenShift resources.   It expands the resource first, then invokes the wrapped
+type ShortcutExpander struct {
+	RESTMapper meta.RESTMapper
+
+	All []string
+}
+
+var _ meta.RESTMapper = &ShortcutExpander{}
+
+func NewShortcutExpander(delegate meta.RESTMapper) ShortcutExpander {
+	return ShortcutExpander{All: userResources, RESTMapper: delegate}
+}
+
+func (e ShortcutExpander) KindFor(resource unversioned.GroupVersionResource) (unversioned.GroupVersionKind, error) {
+	return e.RESTMapper.KindFor(expandResourceShortcut(resource))
+}
+
+func (e ShortcutExpander) KindsFor(resource unversioned.GroupVersionResource) ([]unversioned.GroupVersionKind, error) {
+	return e.RESTMapper.KindsFor(expandResourceShortcut(resource))
+}
+
+func (e ShortcutExpander) ResourcesFor(resource unversioned.GroupVersionResource) ([]unversioned.GroupVersionResource, error) {
+	return e.RESTMapper.ResourcesFor(expandResourceShortcut(resource))
+}
+
+func (e ShortcutExpander) ResourceFor(resource unversioned.GroupVersionResource) (unversioned.GroupVersionResource, error) {
+	return e.RESTMapper.ResourceFor(expandResourceShortcut(resource))
+}
+
+func (e ShortcutExpander) ResourceSingularizer(resource string) (string, error) {
+	return e.RESTMapper.ResourceSingularizer(expandResourceShortcut(unversioned.GroupVersionResource{Resource: resource}).Resource)
+}
+
+func (e ShortcutExpander) RESTMapping(gk unversioned.GroupKind, versions ...string) (*meta.RESTMapping, error) {
+	return e.RESTMapper.RESTMapping(gk, versions...)
+}
+
+func (e ShortcutExpander) RESTMappings(gk unversioned.GroupKind) ([]*meta.RESTMapping, error) {
+	return e.RESTMapper.RESTMappings(gk)
+}
+
+// userResources 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.
+var userResources = []string{"rc", "svc", "pods", "pvc"}
+
+// AliasesForResource returns whether a resource has an alias or not
+func (e ShortcutExpander) AliasesForResource(resource string) ([]string, bool) {
+	if resource == "all" {
+		return e.All, true
+	}
+
+	expanded := expandResourceShortcut(unversioned.GroupVersionResource{Resource: resource}).Resource
+	return []string{expanded}, (expanded != resource)
+}
+
+// expandResourceShortcut will return the expanded version of resource
+// (something that a pkg/api/meta.RESTMapper can understand), if it is
+// indeed a shortcut. Otherwise, will return resource unmodified.
+func expandResourceShortcut(resource unversioned.GroupVersionResource) unversioned.GroupVersionResource {
+	if expanded, ok := kubectl.ShortForms[resource.Resource]; ok {
+		resource.Resource = expanded
+		return resource
+	}
+	return resource
+}

pkg/kubectl/cmd/util/shortcut_restmapper_test.go

@@ -0,0 +1,61 @@
+/*
+Copyright 2016 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 util
+
+import (
+	"strings"
+	"testing"
+
+	"k8s.io/kubernetes/pkg/api/testapi"
+)
+
+func TestReplaceAliases(t *testing.T) {
+	tests := []struct {
+		name     string
+		arg      string
+		expected string
+	}{
+		{
+			name:     "no-replacement",
+			arg:      "service",
+			expected: "service",
+		},
+		{
+			name:     "all-replacement",
+			arg:      "all",
+			expected: "rc,svc,pods,pvc",
+		},
+		{
+			name:     "alias-in-comma-separated-arg",
+			arg:      "all,secrets",
+			expected: "rc,svc,pods,pvc,secrets",
+		},
+	}
+
+	mapper := NewShortcutExpander(testapi.Default.RESTMapper())
+
+	for _, test := range tests {
+		resources := []string{}
+		for _, arg := range strings.Split(test.arg, ",") {
+			curr, _ := mapper.AliasesForResource(arg)
+			resources = append(resources, curr...)
+		}
+		if strings.Join(resources, ",") != test.expected {
+			t.Errorf("%s: unexpected argument: expected %s, got %s", test.name, test.expected, resources)
+		}
+	}
+}

pkg/kubectl/kubectl.go

@@ -97,48 +97,8 @@ func (m OutputVersionMapper) RESTMapping(gk unversioned.GroupKind, versions ...s
 	return m.RESTMapper.RESTMapping(gk, versions...)
 }
 
-// ShortcutExpander is a RESTMapper that can be used for Kubernetes
-// resources.  It expands the resource first, then invokes the wrapped RESTMapper
-type ShortcutExpander struct {
-	RESTMapper meta.RESTMapper
-}
-
-var _ meta.RESTMapper = &ShortcutExpander{}
-
-func (e ShortcutExpander) KindFor(resource unversioned.GroupVersionResource) (unversioned.GroupVersionKind, error) {
-	return e.RESTMapper.KindFor(expandResourceShortcut(resource))
-}
-
-func (e ShortcutExpander) KindsFor(resource unversioned.GroupVersionResource) ([]unversioned.GroupVersionKind, error) {
-	return e.RESTMapper.KindsFor(expandResourceShortcut(resource))
-}
-
-func (e ShortcutExpander) ResourcesFor(resource unversioned.GroupVersionResource) ([]unversioned.GroupVersionResource, error) {
-	return e.RESTMapper.ResourcesFor(expandResourceShortcut(resource))
-}
-
-func (e ShortcutExpander) ResourceFor(resource unversioned.GroupVersionResource) (unversioned.GroupVersionResource, error) {
-	return e.RESTMapper.ResourceFor(expandResourceShortcut(resource))
-}
-
-func (e ShortcutExpander) RESTMapping(gk unversioned.GroupKind, versions ...string) (*meta.RESTMapping, error) {
-	return e.RESTMapper.RESTMapping(gk, versions...)
-}
-
-func (e ShortcutExpander) RESTMappings(gk unversioned.GroupKind) ([]*meta.RESTMapping, error) {
-	return e.RESTMapper.RESTMappings(gk)
-}
-
-func (e ShortcutExpander) ResourceSingularizer(resource string) (string, error) {
-	return e.RESTMapper.ResourceSingularizer(expandResourceShortcut(unversioned.GroupVersionResource{Resource: resource}).Resource)
-}
-
-func (e ShortcutExpander) AliasesForResource(resource string) ([]string, bool) {
-	return e.RESTMapper.AliasesForResource(expandResourceShortcut(unversioned.GroupVersionResource{Resource: resource}).Resource)
-}
-
-// shortForms is the list of short names to their expanded names
-var shortForms = map[string]string{
+// ShortForms is the list of short names to their expanded names
+var ShortForms = map[string]string{
 	// Please keep this alphabetized
 	// If you add an entry here, please also take a look at pkg/kubectl/cmd/cmd.go
 	// and add an entry to valid_resources when appropriate.
@@ -165,30 +125,20 @@ var shortForms = map[string]string{
 	"svc":    "services",
 }
 
-// Look-up for resource short forms by value
+// ResourceShortFormFor looks up for a short form of resource names.
 func ResourceShortFormFor(resource string) (string, bool) {
 	var alias string
 	exists := false
-	for k, val := range shortForms {
+	for k, val := range ShortForms {
 		if val == resource {
 			alias = k
 			exists = true
+			break
 		}
 	}
 	return alias, exists
 }
 
-// expandResourceShortcut will return the expanded version of resource
-// (something that a pkg/api/meta.RESTMapper can understand), if it is
-// indeed a shortcut. Otherwise, will return resource unmodified.
-func expandResourceShortcut(resource unversioned.GroupVersionResource) unversioned.GroupVersionResource {
-	if expanded, ok := shortForms[resource.Resource]; ok {
-		// don't change the group or version that's already been specified
-		resource.Resource = expanded
-	}
-	return resource
-}
-
 // ResourceAliases returns the resource shortcuts and plural forms for the given resources.
 func ResourceAliases(rs []string) []string {
 	as := make([]string, 0, len(rs))
@@ -210,7 +160,7 @@ func ResourceAliases(rs []string) []string {
 		plurals[plural] = struct{}{}
 	}
 
-	for sf, r := range shortForms {
+	for sf, r := range ShortForms {
 		if _, found := plurals[r]; found {
 			as = append(as, sf)
 		}

pkg/kubectl/resource/builder_test.go

@@ -1177,39 +1177,6 @@ func TestReceiveMultipleErrors(t *testing.T) {
 	}
 }
 
-func TestReplaceAliases(t *testing.T) {
-	tests := []struct {
-		name     string
-		arg      string
-		expected string
-	}{
-		{
-			name:     "no-replacement",
-			arg:      "service",
-			expected: "service",
-		},
-		{
-			name:     "all-replacement",
-			arg:      "all",
-			expected: "rc,svc,pods,pvc",
-		},
-		{
-			name:     "alias-in-comma-separated-arg",
-			arg:      "all,secrets",
-			expected: "rc,svc,pods,pvc,secrets",
-		},
-	}
-
-	b := NewBuilder(testapi.Default.RESTMapper(), api.Scheme, fakeClient(), testapi.Default.Codec())
-
-	for _, test := range tests {
-		replaced := b.replaceAliases(test.arg)
-		if replaced != test.expected {
-			t.Errorf("%s: unexpected argument: expected %s, got %s", test.name, test.expected, replaced)
-		}
-	}
-}
-
 func TestHasNames(t *testing.T) {
 	tests := []struct {
 		args            []string