Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

shortcut expander will take the list of short names from the api ser… #41343

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions hack/make-rules/test-cmd-util.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3225,6 +3225,7 @@ runTests() {
}
__EOF__
kube::test::get_object_assert storageclass "{{range.items}}{{$id_field}}:{{end}}" 'storage-class-name:'
kube::test::get_object_assert sc "{{range.items}}{{$id_field}}:{{end}}" 'storage-class-name:'
kubectl delete storageclass storage-class-name "${kube_flags[@]}"
# Post-condition: no storage classes
kube::test::get_object_assert storageclass "{{range.items}}{{$id_field}}:{{end}}" ''
Expand Down
13 changes: 10 additions & 3 deletions pkg/kubectl/cmd/testing/fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,10 @@ func (d *fakeCachedDiscoveryClient) Fresh() bool {
func (d *fakeCachedDiscoveryClient) Invalidate() {
}

func (d *fakeCachedDiscoveryClient) ServerResources() ([]*metav1.APIResourceList, error) {
return []*metav1.APIResourceList{}, nil
}

type TestFactory struct {
Mapper meta.RESTMapper
Typer runtime.ObjectTyper
Expand Down Expand Up @@ -270,7 +274,9 @@ func (f *FakeFactory) UnstructuredObject() (meta.RESTMapper, runtime.ObjectTyper
mapper := discovery.NewRESTMapper(groupResources, meta.InterfacesForUnstructured)
typer := discovery.NewUnstructuredObjectTyper(groupResources)

return cmdutil.NewShortcutExpander(mapper, nil), typer, nil
fakeDs := &fakeCachedDiscoveryClient{}
expander, err := cmdutil.NewShortcutExpander(mapper, fakeDs)
return expander, typer, err
}

func (f *FakeFactory) Decoder(bool) runtime.Decoder {
Expand Down Expand Up @@ -518,8 +524,9 @@ func (f *fakeAPIFactory) UnstructuredObject() (meta.RESTMapper, runtime.ObjectTy
groupResources := testDynamicResources()
mapper := discovery.NewRESTMapper(groupResources, meta.InterfacesForUnstructured)
typer := discovery.NewUnstructuredObjectTyper(groupResources)

return cmdutil.NewShortcutExpander(mapper, nil), typer, nil
fakeDs := &fakeCachedDiscoveryClient{}
expander, err := cmdutil.NewShortcutExpander(mapper, fakeDs)
return expander, typer, err
}

func (f *fakeAPIFactory) Decoder(bool) runtime.Decoder {
Expand Down
5 changes: 5 additions & 0 deletions pkg/kubectl/cmd/util/cached_discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ type fakeDiscoveryClient struct {
resourceCalls int
versionCalls int
swaggerCalls int

serverResourcesHandler func() ([]*metav1.APIResourceList, error)
}

var _ discovery.DiscoveryInterface = &fakeDiscoveryClient{}
Expand Down Expand Up @@ -141,6 +143,9 @@ func (c *fakeDiscoveryClient) ServerResourcesForGroupVersion(groupVersion string

func (c *fakeDiscoveryClient) ServerResources() ([]*metav1.APIResourceList, error) {
c.resourceCalls = c.resourceCalls + 1
if c.serverResourcesHandler != nil {
return c.serverResourcesHandler()
}
return []*metav1.APIResourceList{}, nil
}

Expand Down
6 changes: 4 additions & 2 deletions pkg/kubectl/cmd/util/factory_object_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ func (f *ring1Factory) Object() (meta.RESTMapper, runtime.ObjectTyper) {
}

// wrap with shortcuts
mapper = NewShortcutExpander(mapper, discoveryClient)
mapper, err = NewShortcutExpander(mapper, discoveryClient)
CheckErr(err)

// wrap with output preferences
cfg, err := f.clientAccessFactory.ClientConfigForVersion(nil)
Expand Down Expand Up @@ -104,7 +105,8 @@ func (f *ring1Factory) UnstructuredObject() (meta.RESTMapper, runtime.ObjectType

mapper := discovery.NewDeferredDiscoveryRESTMapper(discoveryClient, meta.InterfacesForUnstructured)
typer := discovery.NewUnstructuredObjectTyper(groupResources)
return NewShortcutExpander(mapper, discoveryClient), typer, nil
expander, err := NewShortcutExpander(mapper, discoveryClient)
return expander, typer, err
}

func (f *ring1Factory) ClientForMapping(mapping *meta.RESTMapping) (resource.RESTClient, error) {
Expand Down
6 changes: 5 additions & 1 deletion pkg/kubectl/cmd/util/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,11 @@ func TestDiscoveryReplaceAliases(t *testing.T) {
},
}

mapper := NewShortcutExpander(testapi.Default.RESTMapper(), nil)
ds := &fakeDiscoveryClient{}
mapper, err := NewShortcutExpander(testapi.Default.RESTMapper(), ds)
if err != nil {
t.Fatalf("Unable to create shortcut expander, err = %s", err.Error())
}
b := resource.NewBuilder(mapper, api.Scheme, fakeClient(), testapi.Default.Codec())

for _, test := range tests {
Expand Down
76 changes: 45 additions & 31 deletions pkg/kubectl/cmd/util/shortcut_restmapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,34 +17,35 @@ limitations under the License.
package util

import (
"errors"
"strings"

"github.com/golang/glog"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/discovery"
"k8s.io/kubernetes/pkg/kubectl"
)

// ShortcutExpander is a RESTMapper that can be used for Kubernetes resources. It expands the resource first, then invokes the wrapped
type ShortcutExpander struct {
// shortcutExpander is a RESTMapper that can be used for Kubernetes resources. It expands the resource first, then invokes the wrapped
type shortcutExpander struct {
RESTMapper meta.RESTMapper

All []schema.GroupResource

discoveryClient discovery.DiscoveryInterface
}

var _ meta.RESTMapper = &ShortcutExpander{}
var _ meta.RESTMapper = &shortcutExpander{}

func NewShortcutExpander(delegate meta.RESTMapper, client discovery.DiscoveryInterface) ShortcutExpander {
return ShortcutExpander{All: UserResources, RESTMapper: delegate, discoveryClient: client}
}

func (e ShortcutExpander) getAll() []schema.GroupResource {
if e.discoveryClient == nil {
return e.All
func NewShortcutExpander(delegate meta.RESTMapper, client discovery.DiscoveryInterface) (shortcutExpander, error) {
if client == nil {
return shortcutExpander{}, errors.New("Please provide discovery client to shortcut expander")
}
return shortcutExpander{All: UserResources, RESTMapper: delegate, discoveryClient: client}, nil
}

func (e shortcutExpander) getAll() []schema.GroupResource {
// Check if we have access to server resources
apiResources, err := e.discoveryClient.ServerResources()
if err != nil {
Expand All @@ -70,31 +71,31 @@ func (e ShortcutExpander) getAll() []schema.GroupResource {
return availableAll
}

func (e ShortcutExpander) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) {
func (e shortcutExpander) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) {
return e.RESTMapper.KindFor(e.expandResourceShortcut(resource))
}

func (e ShortcutExpander) KindsFor(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) {
func (e shortcutExpander) KindsFor(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) {
return e.RESTMapper.KindsFor(e.expandResourceShortcut(resource))
}

func (e ShortcutExpander) ResourcesFor(resource schema.GroupVersionResource) ([]schema.GroupVersionResource, error) {
func (e shortcutExpander) ResourcesFor(resource schema.GroupVersionResource) ([]schema.GroupVersionResource, error) {
return e.RESTMapper.ResourcesFor(e.expandResourceShortcut(resource))
}

func (e ShortcutExpander) ResourceFor(resource schema.GroupVersionResource) (schema.GroupVersionResource, error) {
func (e shortcutExpander) ResourceFor(resource schema.GroupVersionResource) (schema.GroupVersionResource, error) {
return e.RESTMapper.ResourceFor(e.expandResourceShortcut(resource))
}

func (e ShortcutExpander) ResourceSingularizer(resource string) (string, error) {
func (e shortcutExpander) ResourceSingularizer(resource string) (string, error) {
return e.RESTMapper.ResourceSingularizer(e.expandResourceShortcut(schema.GroupVersionResource{Resource: resource}).Resource)
}

func (e ShortcutExpander) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) {
func (e shortcutExpander) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) {
return e.RESTMapper.RESTMapping(gk, versions...)
}

func (e ShortcutExpander) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) {
func (e shortcutExpander) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) {
return e.RESTMapper.RESTMappings(gk, versions...)
}

Expand All @@ -114,7 +115,7 @@ var UserResources = []schema.GroupResource{
}

// AliasesForResource returns whether a resource has an alias or not
func (e ShortcutExpander) AliasesForResource(resource string) ([]string, bool) {
func (e shortcutExpander) AliasesForResource(resource string) ([]string, bool) {
if strings.ToLower(resource) == "all" {
var resources []schema.GroupResource
if resources = e.getAll(); len(resources) == 0 {
Expand All @@ -130,18 +131,31 @@ func (e ShortcutExpander) AliasesForResource(resource string) ([]string, bool) {
return []string{expanded}, (expanded != resource)
}

// getShortcutMappings returns a hardcoded set of tuples.
// First the list of potential resources will be taken from the instance variable
// which holds the anticipated result of the discovery API.
// Next we will fall back to the hardcoded list of resources.
// Note that the list is ordered by group priority.
// TODO: Wire this to discovery API.
func (e ShortcutExpander) getShortcutMappings() ([]kubectl.ResourceShortcuts, error) {
res := []kubectl.ResourceShortcuts{
{
ShortForm: schema.GroupResource{Group: "storage.k8s.io", Resource: "sc"},
LongForm: schema.GroupResource{Group: "storage.k8s.io", Resource: "storageclasses"},
},
// getShortcutMappings returns a set of tuples which holds short names for resources.
// 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) {
res := []kubectl.ResourceShortcuts{}
// get server resources
apiResList, err := e.discoveryClient.ServerResources()
if err == nil {
for _, apiResources := range apiResList {
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},
}
res = append(res, rs)
}
}
}
}

// append hardcoded short forms at the end of the list
Expand All @@ -153,7 +167,7 @@ func (e ShortcutExpander) getShortcutMappings() ([]kubectl.ResourceShortcuts, er
// (something that a pkg/api/meta.RESTMapper can understand), if it is
// indeed a shortcut. If no match has been found, we will match on group prefixing.
// Lastly we will return resource unmodified.
func (e ShortcutExpander) expandResourceShortcut(resource schema.GroupVersionResource) schema.GroupVersionResource {
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 {
Expand Down
80 changes: 75 additions & 5 deletions pkg/kubectl/cmd/util/shortcut_restmapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"strings"
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/kubernetes/pkg/api/testapi"
)
Expand All @@ -29,38 +30,76 @@ func TestReplaceAliases(t *testing.T) {
name string
arg string
expected string
srvRes []*metav1.APIResourceList
}{
{
name: "no-replacement",
arg: "service",
expected: "service",
srvRes: []*metav1.APIResourceList{},
},
{
name: "all-replacement",
arg: "all",
expected: "pods,replicationcontrollers,services,statefulsets,horizontalpodautoscalers,jobs,deployments,replicasets",
srvRes: []*metav1.APIResourceList{},
},
{
name: "alias-in-comma-separated-arg",
arg: "all,secrets",
expected: "pods,replicationcontrollers,services,statefulsets,horizontalpodautoscalers,jobs,deployments,replicasets,secrets",
srvRes: []*metav1.APIResourceList{},
},
{
name: "sc-resolves-to-storageclasses",
arg: "sc",
expected: "storageclasses",
name: "rc-resolves-to-replicationcontrollers",
arg: "rc",
expected: "replicationcontrollers",
srvRes: []*metav1.APIResourceList{},
},
{
name: "storageclasses-no-replacement",
arg: "storageclasses",
expected: "storageclasses",
srvRes: []*metav1.APIResourceList{},
},
{
name: "hpa-priority",
arg: "hpa",
expected: "superhorizontalpodautoscalers",
srvRes: []*metav1.APIResourceList{
{
GroupVersion: "autoscaling/v1",
APIResources: []metav1.APIResource{
{
Name: "superhorizontalpodautoscalers",
ShortNames: []string{"hpa"},
},
},
},
{
GroupVersion: "autoscaling/v1",
APIResources: []metav1.APIResource{
{
Name: "horizontalpodautoscalers",
ShortNames: []string{"hpa"},
},
},
},
},
},
}

mapper := NewShortcutExpander(testapi.Default.RESTMapper(), nil)
ds := &fakeDiscoveryClient{}
mapper, err := NewShortcutExpander(testapi.Default.RESTMapper(), ds)
if err != nil {
t.Fatalf("Unable to create shortcut expander, err %s", err.Error())
}

for _, test := range tests {
resources := []string{}
ds.serverResourcesHandler = func() ([]*metav1.APIResourceList, error) {
return test.srvRes, nil
}
for _, arg := range strings.Split(test.arg, ",") {
curr, _ := mapper.AliasesForResource(arg)
resources = append(resources, curr...)
Expand All @@ -70,24 +109,55 @@ func TestReplaceAliases(t *testing.T) {
}
}
}

func TestKindFor(t *testing.T) {
tests := []struct {
in schema.GroupVersionResource
expected schema.GroupVersionKind
srvRes []*metav1.APIResourceList
}{
{
in: schema.GroupVersionResource{Group: "storage.k8s.io", Version: "", Resource: "sc"},
expected: schema.GroupVersionKind{Group: "storage.k8s.io", Version: "v1beta1", Kind: "StorageClass"},
srvRes: []*metav1.APIResourceList{
{
GroupVersion: "storage.k8s.io/v1beta1",
APIResources: []metav1.APIResource{
{
Name: "storageclasses",
ShortNames: []string{"sc"},
},
},
},
},
},
{
in: schema.GroupVersionResource{Group: "", Version: "", Resource: "sc"},
expected: schema.GroupVersionKind{Group: "storage.k8s.io", Version: "v1beta1", Kind: "StorageClass"},
srvRes: []*metav1.APIResourceList{
{
GroupVersion: "storage.k8s.io/v1beta1",
APIResources: []metav1.APIResource{
{
Name: "storageclasses",
ShortNames: []string{"sc"},
},
},
},
},
},
}

mapper := NewShortcutExpander(testapi.Default.RESTMapper(), nil)
ds := &fakeDiscoveryClient{}
mapper, err := NewShortcutExpander(testapi.Default.RESTMapper(), ds)
if err != nil {
t.Fatalf("Unable to create shortcut expander, err %s", err.Error())
}

for i, test := range tests {
ds.serverResourcesHandler = func() ([]*metav1.APIResourceList, error) {
return test.srvRes, nil
}
ret, err := mapper.KindFor(test.in)
if err != nil {
t.Errorf("%d: unexpected error returned %s", i, err.Error())
Expand Down
1 change: 1 addition & 0 deletions pkg/registry/storage/storageclass/storage/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ go_library(
"//vendor:k8s.io/apimachinery/pkg/runtime",
"//vendor:k8s.io/apiserver/pkg/registry/generic",
"//vendor:k8s.io/apiserver/pkg/registry/generic/registry",
"//vendor:k8s.io/apiserver/pkg/registry/rest",
],
)

Expand Down
Loading