Merge pull request #58861 from liggitt/prefer-resources-to-shortnames

Automatic merge from submit-queue (batch tested with PRs 58760, 58861). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Prefer exact resource name matches to shortname expansions

Prevents a shortname of "pod" or "pods" from taking precedence over an exact resource name match

```release-note
NONE
```

pkg/kubectl/cmd/util/shortcut_restmapper.go

@@ -22,6 +22,7 @@ import (
 	"github.com/golang/glog"
 
 	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/discovery"
 	"k8s.io/kubernetes/pkg/kubectl"
@@ -72,7 +73,7 @@ func (e shortcutExpander) RESTMappings(gk schema.GroupKind, versions ...string)
 // 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) {
+func (e shortcutExpander) getShortcutMappings() ([]*metav1.APIResourceList, []kubectl.ResourceShortcuts, error) {
 	res := []kubectl.ResourceShortcuts{}
 	// get server resources
 	// This can return an error *and* the results it was able to find.  We don't need to fail on the error.
@@ -81,13 +82,13 @@ func (e shortcutExpander) getShortcutMappings() ([]kubectl.ResourceShortcuts, er
 		glog.V(1).Infof("Error loading discovery information: %v", err)
 	}
 	for _, apiResources := range apiResList {
+		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
+		}
 		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},
@@ -99,7 +100,7 @@ func (e shortcutExpander) getShortcutMappings() ([]kubectl.ResourceShortcuts, er
 
 	// append hardcoded short forms at the end of the list
 	res = append(res, kubectl.ResourcesShortcutStatic...)
-	return res, nil
+	return apiResList, res, nil
 }
 
 // expandResourceShortcut will return the expanded version of resource
@@ -108,8 +109,27 @@ func (e shortcutExpander) getShortcutMappings() ([]kubectl.ResourceShortcuts, er
 // Lastly we will return resource unmodified.
 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 {
+	if allResources, shortcutResources, err := e.getShortcutMappings(); err == nil {
+		// avoid expanding if there's an exact match to a full resource name
+		for _, apiResources := range allResources {
+			gv, err := schema.ParseGroupVersion(apiResources.GroupVersion)
+			if err != nil {
+				continue
+			}
+			if len(resource.Group) != 0 && resource.Group != gv.Group {
+				continue
+			}
+			for _, apiRes := range apiResources.APIResources {
+				if resource.Resource == apiRes.Name {
+					return resource
+				}
+				if resource.Resource == apiRes.SingularName {
+					return resource
+				}
+			}
+		}
+
+		for _, item := range shortcutResources {
 			if len(resource.Group) != 0 && resource.Group != item.ShortForm.Group {
 				continue
 			}
@@ -124,12 +144,13 @@ func (e shortcutExpander) expandResourceShortcut(resource schema.GroupVersionRes
 		if len(resource.Group) == 0 {
 			return resource
 		}
-		for _, item := range resources {
+		for _, item := range shortcutResources {
 			if !strings.HasPrefix(item.ShortForm.Group, resource.Group) {
 				continue
 			}
 			if resource.Resource == item.ShortForm.Resource {
 				resource.Resource = item.LongForm.Resource
+				resource.Group = item.LongForm.Group
 				return resource
 			}
 		}

pkg/kubectl/cmd/util/shortcut_restmapper_test.go

@@ -93,6 +93,36 @@ func TestReplaceAliases(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:     "resource-match-preferred",
+			arg:      "pods",
+			expected: schema.GroupVersionResource{Resource: "pods", Group: ""},
+			srvRes: []*metav1.APIResourceList{
+				{
+					GroupVersion: "v1",
+					APIResources: []metav1.APIResource{{Name: "pods", SingularName: "pod"}},
+				},
+				{
+					GroupVersion: "acme.com/v1",
+					APIResources: []metav1.APIResource{{Name: "poddlers", ShortNames: []string{"pods", "pod"}}},
+				},
+			},
+		},
+		{
+			name:     "resource-match-singular-preferred",
+			arg:      "pod",
+			expected: schema.GroupVersionResource{Resource: "pod", Group: ""},
+			srvRes: []*metav1.APIResourceList{
+				{
+					GroupVersion: "v1",
+					APIResources: []metav1.APIResource{{Name: "pods", SingularName: "pod"}},
+				},
+				{
+					GroupVersion: "acme.com/v1",
+					APIResources: []metav1.APIResource{{Name: "poddlers", ShortNames: []string{"pods", "pod"}}},
+				},
+			},
+		},
 	}
 
 	ds := &fakeDiscoveryClient{}