From ccf4e4d61edd1aaff52e6da1accf2285a6e467c7 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Thu, 14 Jul 2016 07:48:32 -0400 Subject: [PATCH] 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 Related to a suggestion in issue #22337 --- docs/devel/kubectl-conventions.md | 16 ++++++++++ hack/make-rules/test-cmd.sh | 12 ++++++++ pkg/api/install/install.go | 9 +----- pkg/kubectl/cmd/util/factory.go | 46 ++++++++++++++++++++++++++-- pkg/kubectl/cmd/util/factory_test.go | 42 +++++++++++++++++++++++++ pkg/kubectl/kubectl.go | 33 ++++++++++++++++++++ pkg/kubectl/resource/builder.go | 6 ++-- pkg/kubectl/resource/builder_test.go | 33 -------------------- 8 files changed, 151 insertions(+), 46 deletions(-) diff --git a/docs/devel/kubectl-conventions.md b/docs/devel/kubectl-conventions.md index 8705d28561..2259302576 100644 --- a/docs/devel/kubectl-conventions.md +++ b/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 diff --git a/hack/make-rules/test-cmd.sh b/hack/make-rules/test-cmd.sh index 926127ecab..18d59367f6 100755 --- a/hack/make-rules/test-cmd.sh +++ b/hack/make-rules/test-cmd.sh @@ -1014,6 +1014,18 @@ __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 200 OK" + kube::test::if_has_string "${output_message}" "/apis 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 OK" + kube::test::if_has_string "${output_message}" "/apis/extensions/v1beta1/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/deployments 200 OK" + + ##################################### # Third Party Resources # ##################################### diff --git a/pkg/api/install/install.go b/pkg/api/install/install.go index 937920d28a..2c8454c4a6 100644 --- a/pkg/api/install/install.go +++ b/pkg/api/install/install.go @@ -88,9 +88,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 @@ -116,11 +113,7 @@ func newRESTMapper(externalVersions []unversioned.GroupVersion) meta.RESTMapper "ThirdPartyResourceData", "ThirdPartyResourceList") - mapper := api.NewDefaultRESTMapper(externalVersions, interfacesFor, importPrefix, ignoredKinds, rootScoped) - // setup aliases for groups of resources - mapper.AddResourceAlias("all", userResources...) - - return mapper + return api.NewDefaultRESTMapper(externalVersions, interfacesFor, importPrefix, ignoredKinds, rootScoped) } // InterfacesFor returns the default Codec and ResourceVersioner for a given version diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 3bcf7bf54f..6e8c789be0 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -230,12 +230,53 @@ func makeInterfacesFor(versionList []unversioned.GroupVersion) func(version unve } } +func DiscoveryRESTMapper(clients *ClientCache, delegate meta.RESTMapper) kubectl.ShortcutExpander { + defaultMapper := kubectl.NewShortcutExpander(delegate) + if clients == nil { + return defaultMapper + } + + client, err := clients.ClientForVersion(&unversioned.GroupVersion{Version: "v1"}) + if err != nil { + return defaultMapper + } + + // Check if we have access to server resources + apiResources, err := client.Discovery().ServerResources() + if err != nil { + return defaultMapper + } + + availableResources := []unversioned.GroupVersionResource{} + for groupVersionString, resourceList := range apiResources { + currVersion, err := unversioned.ParseGroupVersion(groupVersionString) + if err != nil { + return defaultMapper + } + + for _, resource := range resourceList.APIResources { + availableResources = append(availableResources, currVersion.WithResource(resource.Name)) + } + } + + availableAll := []unversioned.GroupResource{} + for _, requestedResource := range defaultMapper.All { + for _, availableResource := range availableResources { + if requestedResource.Group == availableResource.Group && + requestedResource.Resource == availableResource.Resource { + availableAll = append(availableAll, requestedResource) + break + } + } + } + + return kubectl.ShortcutExpander{All: availableAll, RESTMapper: delegate} +} + // NewFactory creates a factory with the default Kubernetes resources defined // 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 @@ -245,6 +286,7 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory { } clients := NewClientCache(clientConfig) + mapper := DiscoveryRESTMapper(clients, registered.RESTMapper()) return &Factory{ clients: clients, diff --git a/pkg/kubectl/cmd/util/factory_test.go b/pkg/kubectl/cmd/util/factory_test.go index cee417937c..9753be5301 100644 --- a/pkg/kubectl/cmd/util/factory_test.go +++ b/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" @@ -42,6 +43,7 @@ import ( "k8s.io/kubernetes/pkg/client/unversioned/testclient" "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" @@ -714,3 +716,43 @@ func TestMakePortsString(t *testing.T) { } } } + +func fakeClient() resource.ClientMapper { + return resource.ClientMapperFunc(func(*meta.RESTMapping) (resource.RESTClient, error) { + return &fake.RESTClient{}, nil + }) +} + +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: "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 := DiscoveryRESTMapper(nil, testapi.Default.RESTMapper()) + 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) + } + } +} diff --git a/pkg/kubectl/kubectl.go b/pkg/kubectl/kubectl.go index 667fb2ecd8..6f14fab842 100644 --- a/pkg/kubectl/kubectl.go +++ b/pkg/kubectl/kubectl.go @@ -38,6 +38,18 @@ serviceaccounts (aka 'sa'), ingresses (aka 'ing'), horizontalpodautoscalers (aka componentstatuses (aka 'cs), endpoints (aka 'ep'), petsets (alpha feature, may be unstable) and secrets.` ) +// userResources is a group of resources mostly used by a kubectl user +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"}, +} + type NamespaceInfo struct { Namespace string } @@ -107,6 +119,15 @@ func (m OutputVersionMapper) RESTMapping(gk unversioned.GroupKind, versions ...s // resources. It expands the resource first, then invokes the wrapped RESTMapper type ShortcutExpander struct { RESTMapper meta.RESTMapper + + All []unversioned.GroupResource +} + +func NewShortcutExpander(delegate meta.RESTMapper) ShortcutExpander { + return ShortcutExpander{ + All: userResources, + RESTMapper: delegate, + } } var _ meta.RESTMapper = &ShortcutExpander{} @@ -139,7 +160,19 @@ func (e ShortcutExpander) ResourceSingularizer(resource string) (string, error) return e.RESTMapper.ResourceSingularizer(expandResourceShortcut(unversioned.GroupVersionResource{Resource: resource}).Resource) } +// AliasesForResource returns whether a resource has an alias or not func (e ShortcutExpander) AliasesForResource(resource string) ([]string, bool) { + if strings.ToLower(resource) == "all" { + var resources []unversioned.GroupResource + if resources = e.All; len(e.All) == 0 { + resources = userResources + } + aliases := []string{} + for _, r := range resources { + aliases = append(aliases, r.Resource) + } + return aliases, true + } return e.RESTMapper.AliasesForResource(expandResourceShortcut(unversioned.GroupVersionResource{Resource: resource}).Resource) } diff --git a/pkg/kubectl/resource/builder.go b/pkg/kubectl/resource/builder.go index f7d4d6cf84..ae0e017891 100644 --- a/pkg/kubectl/resource/builder.go +++ b/pkg/kubectl/resource/builder.go @@ -317,7 +317,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: @@ -338,9 +338,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 { diff --git a/pkg/kubectl/resource/builder_test.go b/pkg/kubectl/resource/builder_test.go index 0569852082..e6d2931c1c 100644 --- a/pkg/kubectl/resource/builder_test.go +++ b/pkg/kubectl/resource/builder_test.go @@ -1154,39 +1154,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