Merge pull request #30199 from dims/re-add-roadmap-extend-all

Automatic merge from submit-queue

Extend all to more resources

Added more things from the list here:
https://github.com/kubernetes/kubernetes/blob/master/pkg/kubectl/cmd/cmd.go#L159

Update the devel/kubectl-conventions.md with the rules mentioned by
a few folks on which resources could be added to the special 'all' alias

docs/devel/kubectl-conventions.md

@@ -43,6 +43,7 @@ Updated: 8/27/2015
   - [Principles](#principles)
   - [Command conventions](#command-conventions)
     - [Create commands](#create-commands)
+    - [Rules for extending special resource alias - "all"](#rules-for-extending-special-resource-alias---all)
   - [Flag conventions](#flag-conventions)
   - [Output conventions](#output-conventions)
   - [Documentation conventions](#documentation-conventions)
@@ -118,6 +119,21 @@ creating tls secrets. You create these as separate commands to get distinct
 flags and separate help that is tailored for the particular usage.
 
 
+### Rules for extending special resource alias - "all"
+
+Here are the rules to add a new resource to the `kubectl get all` output.
+
+* No cluster scoped resources
+
+* No namespace admin level resources (limits, quota, policy, authorization
+rules)
+
+* No resources that are potentially unrecoverable (secrets and pvc)
+
+* Resources that are considered "similar" to #3 should be grouped
+the same (configmaps)
+
+
 ## Flag conventions
 
 * Flags are all lowercase, with words separated by hyphens

hack/make-rules/test-cmd.sh

@@ -1048,6 +1048,19 @@ __EOF__
     exit 1
   fi
 
+  ### Test kubectl get all
+  output_message=$(kubectl --v=6 --namespace default get all 2>&1 "${kube_flags[@]}")
+  # Post-condition: Check if we get 200 OK from all the url(s)
+  kube::test::if_has_string "${output_message}" "/api/v1/namespaces/default/pods 200 OK"
+  kube::test::if_has_string "${output_message}" "/api/v1/namespaces/default/replicationcontrollers 200 OK"
+  kube::test::if_has_string "${output_message}" "/api/v1/namespaces/default/services 200 OK"
+  kube::test::if_has_string "${output_message}" "/apis/apps/v1alpha1/namespaces/default/petsets 200 OK"
+  kube::test::if_has_string "${output_message}" "/apis/autoscaling/v1/namespaces/default/horizontalpodautoscalers 200"
+  kube::test::if_has_string "${output_message}" "/apis/batch/v1/namespaces/default/jobs 200 OK"
+  kube::test::if_has_string "${output_message}" "/apis/extensions/v1beta1/namespaces/default/deployments 200 OK"
+  kube::test::if_has_string "${output_message}" "/apis/extensions/v1beta1/namespaces/default/replicasets 200 OK"
+
+
   #####################################
   # Third Party Resources             #
   #####################################

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 cmdutil.NewShortcutExpander(mapper), typer, nil
+			return cmdutil.NewShortcutExpander(mapper, nil), 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

@@ -301,9 +301,10 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 			}
 
 			mapper := registered.RESTMapper()
+			discoveryClient, err := discovery.NewDiscoveryClientForConfig(cfg)
 			// 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 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.
@@ -325,7 +326,7 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 			}
 
 			// wrap with shortcuts
-			mapper = NewShortcutExpander(mapper)
+			mapper = NewShortcutExpander(mapper, discoveryClient)
 			// wrap with output preferences
 			mapper = kubectl.OutputVersionMapper{RESTMapper: mapper, OutputVersions: []unversioned.GroupVersion{cmdApiVersion}}
 			return mapper, api.Scheme
@@ -362,7 +363,7 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 
 			typer := discovery.NewUnstructuredObjectTyper(groupResources)
 
-			return NewShortcutExpander(mapper), typer, nil
+			return NewShortcutExpander(mapper, dc), typer, nil
 		},
 		RESTClient: func() (*restclient.RESTClient, error) {
 			clientConfig, err := clients.ClientConfigForVersion(nil)

pkg/kubectl/cmd/util/factory_test.go

@@ -32,6 +32,7 @@ import (
 	"time"
 
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/meta"
 	"k8s.io/kubernetes/pkg/api/testapi"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/api/validation"
@@ -43,6 +44,7 @@ import (
 	manualfake "k8s.io/kubernetes/pkg/client/unversioned/fake"
 	"k8s.io/kubernetes/pkg/controller"
 	"k8s.io/kubernetes/pkg/kubectl"
+	"k8s.io/kubernetes/pkg/kubectl/resource"
 	"k8s.io/kubernetes/pkg/labels"
 	"k8s.io/kubernetes/pkg/runtime"
 	"k8s.io/kubernetes/pkg/util/flag"
@@ -712,3 +714,43 @@ func TestMakePortsString(t *testing.T) {
 		}
 	}
 }
+
+func fakeClient() resource.ClientMapper {
+	return resource.ClientMapperFunc(func(*meta.RESTMapping) (resource.RESTClient, error) {
+		return &manualfake.RESTClient{}, nil
+	})
+}
+
+func TestDiscoveryReplaceAliases(t *testing.T) {
+	tests := []struct {
+		name     string
+		arg      string
+		expected string
+	}{
+		{
+			name:     "no-replacement",
+			arg:      "service",
+			expected: "service",
+		},
+		{
+			name:     "all-replacement",
+			arg:      "all",
+			expected: "pods,replicationcontrollers,services,petsets,horizontalpodautoscalers,jobs,deployments,replicasets",
+		},
+		{
+			name:     "alias-in-comma-separated-arg",
+			arg:      "all,secrets",
+			expected: "pods,replicationcontrollers,services,petsets,horizontalpodautoscalers,jobs,deployments,replicasets,secrets",
+		},
+	}
+
+	mapper := NewShortcutExpander(testapi.Default.RESTMapper(), nil)
+	b := resource.NewBuilder(mapper, 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)
+		}
+	}
+}

pkg/kubectl/cmd/util/shortcut_restmapper.go

@@ -17,8 +17,11 @@ limitations under the License.
 package util
 
 import (
+	"strings"
+
 	"k8s.io/kubernetes/pkg/api/meta"
 	"k8s.io/kubernetes/pkg/api/unversioned"
+	"k8s.io/kubernetes/pkg/client/typed/discovery"
 	"k8s.io/kubernetes/pkg/kubectl"
 )
 
@@ -26,13 +29,52 @@ import (
 type ShortcutExpander struct {
 	RESTMapper meta.RESTMapper
 
-	All []string
+	All []unversioned.GroupResource
+
+	discoveryClient *discovery.DiscoveryClient
 }
 
 var _ meta.RESTMapper = &ShortcutExpander{}
 
-func NewShortcutExpander(delegate meta.RESTMapper) ShortcutExpander {
+func NewShortcutExpander(delegate meta.RESTMapper, client *discovery.DiscoveryClient) ShortcutExpander {
-	return ShortcutExpander{All: userResources, RESTMapper: delegate}
+	return ShortcutExpander{All: userResources, RESTMapper: delegate, discoveryClient: client}
+}
+
+func (e ShortcutExpander) getAll() []unversioned.GroupResource {
+	if e.discoveryClient == nil {
+		return e.All
+	}
+
+	// Check if we have access to server resources
+	apiResources, err := e.discoveryClient.ServerResources()
+	if err != nil {
+		return e.All
+	}
+
+	availableResources := []unversioned.GroupVersionResource{}
+	for groupVersionString, resourceList := range apiResources {
+		currVersion, err := unversioned.ParseGroupVersion(groupVersionString)
+		if err != nil {
+			return e.All
+		}
+
+		for _, resource := range resourceList.APIResources {
+			availableResources = append(availableResources, currVersion.WithResource(resource.Name))
+		}
+	}
+
+	availableAll := []unversioned.GroupResource{}
+	for _, requestedResource := range e.All {
+		for _, availableResource := range availableResources {
+			if requestedResource.Group == availableResource.Group &&
+				requestedResource.Resource == availableResource.Resource {
+				availableAll = append(availableAll, requestedResource)
+				break
+			}
+		}
+	}
+
+	return availableAll
 }
 
 func (e ShortcutExpander) KindFor(resource unversioned.GroupVersionResource) (unversioned.GroupVersionKind, error) {
@@ -65,14 +107,30 @@ func (e ShortcutExpander) RESTMappings(gk unversioned.GroupKind) ([]*meta.RESTMa
 
 // 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"}
+var userResources = []unversioned.GroupResource{
+	{Group: "", Resource: "pods"},
+	{Group: "", Resource: "replicationcontrollers"},
+	{Group: "", Resource: "services"},
+	{Group: "apps", Resource: "petsets"},
+	{Group: "autoscaling", Resource: "horizontalpodautoscalers"},
+	{Group: "extensions", Resource: "jobs"},
+	{Group: "extensions", Resource: "deployments"},
+	{Group: "extensions", Resource: "replicasets"},
+}
 
 // AliasesForResource returns whether a resource has an alias or not
 func (e ShortcutExpander) AliasesForResource(resource string) ([]string, bool) {
-	if resource == "all" {
+	if strings.ToLower(resource) == "all" {
-		return e.All, true
+		var resources []unversioned.GroupResource
+		if resources = e.getAll(); len(resources) == 0 {
+			resources = userResources
+		}
+		aliases := []string{}
+		for _, r := range resources {
+			aliases = append(aliases, r.Resource)
+		}
+		return aliases, true
 	}
-
 	expanded := expandResourceShortcut(unversioned.GroupVersionResource{Resource: resource}).Resource
 	return []string{expanded}, (expanded != resource)
 }

pkg/kubectl/cmd/util/shortcut_restmapper_test.go

@@ -37,16 +37,16 @@ func TestReplaceAliases(t *testing.T) {
 		{
 			name:     "all-replacement",
 			arg:      "all",
-			expected: "rc,svc,pods,pvc",
+			expected: "pods,replicationcontrollers,services,petsets,horizontalpodautoscalers,jobs,deployments,replicasets",
 		},
 		{
 			name:     "alias-in-comma-separated-arg",
 			arg:      "all,secrets",
-			expected: "rc,svc,pods,pvc,secrets",
+			expected: "pods,replicationcontrollers,services,petsets,horizontalpodautoscalers,jobs,deployments,replicasets,secrets",
 		},
 	}
 
-	mapper := NewShortcutExpander(testapi.Default.RESTMapper())
+	mapper := NewShortcutExpander(testapi.Default.RESTMapper(), nil)
 
 	for _, test := range tests {
 		resources := []string{}

pkg/kubectl/resource/builder.go

@@ -343,7 +343,7 @@ func (b *Builder) ResourceTypeOrNameArgs(allowEmptySelector bool, args ...string
 	}
 	if len(args) > 0 {
 		// Try replacing aliases only in types
-		args[0] = b.replaceAliases(args[0])
+		args[0] = b.ReplaceAliases(args[0])
 	}
 	switch {
 	case len(args) > 2:
@@ -364,9 +364,9 @@ func (b *Builder) ResourceTypeOrNameArgs(allowEmptySelector bool, args ...string
 	return b
 }
 
-// replaceAliases accepts an argument and tries to expand any existing
+// ReplaceAliases accepts an argument and tries to expand any existing
 // aliases found in it
-func (b *Builder) replaceAliases(input string) string {
+func (b *Builder) ReplaceAliases(input string) string {
 	replaced := []string{}
 	for _, arg := range strings.Split(input, ",") {
 		if aliases, ok := b.mapper.AliasesForResource(arg); ok {