diff --git a/docs/devel/kubectl-conventions.md b/docs/devel/kubectl-conventions.md index 2931640729c..fe2e51a195d 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 6edb69cad2b..3260627441a 100755 --- a/hack/make-rules/test-cmd.sh +++ b/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 # ##################################### diff --git a/pkg/kubectl/cmd/cmd_test.go b/pkg/kubectl/cmd/cmd_test.go index 9aee95ce493..08859e30d8d 100644 --- a/pkg/kubectl/cmd/cmd_test.go +++ b/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. diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 7a24443c56b..1540b399667 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/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) diff --git a/pkg/kubectl/cmd/util/factory_test.go b/pkg/kubectl/cmd/util/factory_test.go index 1a5b2b319eb..0583328b854 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" @@ -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) + } + } +} diff --git a/pkg/kubectl/cmd/util/shortcut_restmapper.go b/pkg/kubectl/cmd/util/shortcut_restmapper.go index 389796ddce0..60c781d8174 100644 --- a/pkg/kubectl/cmd/util/shortcut_restmapper.go +++ b/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 { - return ShortcutExpander{All: userResources, RESTMapper: delegate} +func NewShortcutExpander(delegate meta.RESTMapper, client *discovery.DiscoveryClient) ShortcutExpander { + 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" { - return e.All, true + if strings.ToLower(resource) == "all" { + 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) } diff --git a/pkg/kubectl/cmd/util/shortcut_restmapper_test.go b/pkg/kubectl/cmd/util/shortcut_restmapper_test.go index 888418bc7aa..be445c03623 100644 --- a/pkg/kubectl/cmd/util/shortcut_restmapper_test.go +++ b/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{} diff --git a/pkg/kubectl/resource/builder.go b/pkg/kubectl/resource/builder.go index 6de96852537..ae7d9dfce2d 100644 --- a/pkg/kubectl/resource/builder.go +++ b/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 {