diff --git a/pkg/api/latest/latest.go b/pkg/api/latest/latest.go index c7e1cf7cde..9341466615 100644 --- a/pkg/api/latest/latest.go +++ b/pkg/api/latest/latest.go @@ -37,6 +37,8 @@ var ( // AllPreferredGroupVersions returns the preferred versions of all // registered groups in the form of "group1/version1,group2/version2,..." AllPreferredGroupVersions = allGroups.AllPreferredGroupVersions + // IsRegistered is a shortcut to allGroups.IsRegistered. + IsRegistered = allGroups.IsRegistered ) // GroupMetaMap is a map between group names and their metadata. @@ -65,6 +67,12 @@ func (g GroupMetaMap) Group(group string) (*GroupMeta, error) { return groupMeta, nil } +// IsRegistered takes a string and determines if it's one of the registered groups +func (g GroupMetaMap) IsRegistered(group string) bool { + _, found := g[group] + return found +} + // TODO: This is an expedient function, because we don't check if a Group is // supported throughout the code base. We will abandon this function and // checking the error returned by the Group() function. diff --git a/pkg/api/meta/interfaces.go b/pkg/api/meta/interfaces.go index 303a06305b..3d7455469e 100644 --- a/pkg/api/meta/interfaces.go +++ b/pkg/api/meta/interfaces.go @@ -157,4 +157,5 @@ type RESTMapper interface { RESTMapping(kind string, versions ...string) (*RESTMapping, error) AliasesForResource(resource string) ([]string, bool) ResourceSingularizer(resource string) (singular string, err error) + ResourceIsValid(resource string) bool } diff --git a/pkg/api/meta/restmapper.go b/pkg/api/meta/restmapper.go index 6ac54b2d8f..ab389b934d 100644 --- a/pkg/api/meta/restmapper.go +++ b/pkg/api/meta/restmapper.go @@ -169,16 +169,25 @@ func (m *DefaultRESTMapper) ResourceSingularizer(resource string) (singular stri // VersionAndKindForResource implements RESTMapper func (m *DefaultRESTMapper) VersionAndKindForResource(resource string) (defaultVersion, kind string, err error) { + if parts := strings.Split(resource, "/"); len(parts) > 1 { + if len(parts) > 2 { + return "", "", fmt.Errorf("resource %q has an invalid name", resource) + } + resource = parts[1] + if m.group != parts[0] { + return "", "", fmt.Errorf("no resource %q is defined for group %q", resource, m.group) + } + } meta, ok := m.mapping[strings.ToLower(resource)] if !ok { - return "", "", fmt.Errorf("in version and kind for resource, no resource %q has been defined", resource) + return "", "", fmt.Errorf("no resource %q has been defined", resource) } return meta.APIVersion, meta.Kind, nil } func (m *DefaultRESTMapper) GroupForResource(resource string) (string, error) { if _, ok := m.mapping[strings.ToLower(resource)]; !ok { - return "", fmt.Errorf("in group for resource, no resource %q has been defined", resource) + return "", fmt.Errorf("no resource %q has been defined", resource) } return m.group, nil } @@ -273,6 +282,12 @@ func (m *DefaultRESTMapper) AliasesForResource(alias string) ([]string, bool) { return nil, false } +// ResourceIsValid takes a string (kind) and checks if it's a valid resource +func (m *DefaultRESTMapper) ResourceIsValid(resource string) bool { + _, _, err := m.VersionAndKindForResource(resource) + return err == nil +} + // MultiRESTMapper is a wrapper for multiple RESTMappers. type MultiRESTMapper []RESTMapper @@ -335,3 +350,13 @@ func (m MultiRESTMapper) AliasesForResource(alias string) (aliases []string, ok } return nil, false } + +// ResourceIsValid takes a string (either group/kind or kind) and checks if it's a valid resource +func (m MultiRESTMapper) ResourceIsValid(resource string) bool { + for _, t := range m { + if t.ResourceIsValid(resource) { + return true + } + } + return false +} diff --git a/pkg/client/unversioned/helper.go b/pkg/client/unversioned/helper.go index 2aa90b203d..db903cd9f4 100644 --- a/pkg/client/unversioned/helper.go +++ b/pkg/client/unversioned/helper.go @@ -288,6 +288,7 @@ func NewInCluster() (*Client, error) { // SetKubernetesDefaults sets default values on the provided client config for accessing the // Kubernetes API or returns an error if any of the defaults are impossible or invalid. +// TODO: this method needs to be split into one that sets defaults per group, expected to be fix in PR "Refactoring clientcache.go and helper.go #14592" func SetKubernetesDefaults(config *Config) error { if config.Prefix == "" { config.Prefix = "/api" diff --git a/pkg/kubectl/kubectl.go b/pkg/kubectl/kubectl.go index e0aa1d1859..691866ec66 100644 --- a/pkg/kubectl/kubectl.go +++ b/pkg/kubectl/kubectl.go @@ -82,6 +82,12 @@ func (e ShortcutExpander) VersionAndKindForResource(resource string) (defaultVer return defaultVersion, kind, err } +// ResourceIsValid takes a string (kind) and checks if it's a valid resource. +// It expands the resource first, then invokes the wrapped mapper. +func (e ShortcutExpander) ResourceIsValid(resource string) bool { + return e.RESTMapper.ResourceIsValid(expandResourceShortcut(resource)) +} + // expandResourceShortcut will return the expanded version of resource // (something that a pkg/api/meta.RESTMapper can understand), if it is // indeed a shortcut. Otherwise, will return resource unmodified. diff --git a/pkg/kubectl/resource/builder.go b/pkg/kubectl/resource/builder.go index 9da9c10666..265626a650 100644 --- a/pkg/kubectl/resource/builder.go +++ b/pkg/kubectl/resource/builder.go @@ -186,12 +186,12 @@ func (b *Builder) ResourceTypes(types ...string) *Builder { return b } -// ResourceNames accepts a default type and one or more names, and creates tuples of +// ResourceNames accepts a default type or group/type and one or more names, and creates tuples of // resources func (b *Builder) ResourceNames(resource string, names ...string) *Builder { for _, name := range names { - // See if this input string is of type/name format - tuple, ok, err := splitResourceTypeName(name) + // See if this input string is of type/name or group/type/name format + tuple, ok, err := splitGroupResourceTypeName(name) if err != nil { b.errs = append(b.errs, err) return b @@ -274,12 +274,47 @@ func (b *Builder) SelectAllParam(selectAll bool) *Builder { return b } -// ResourceTypeOrNameArgs indicates that the builder should accept arguments -// of the form `([,,...]| [,,...])`. When one argument is -// received, the types provided will be retrieved from the server (and be comma delimited). -// When two or more arguments are received, they must be a single type and resource name(s). +func (b *Builder) hasNamesArg(args []string) bool { + if len(args) > 1 { + return true + } + if len(args) != 1 { + return false + } + // the first (and the only) arg could be (type[,group/type]), type/name, or group/type/name + s := args[0] + // type or group/type (no name) + if strings.Contains(s, ",") { + return false + } + // type (no name) + if !strings.Contains(s, "/") { + return false + } + // group/type/name, type/name or group/type + tuple := strings.Split(s, "/") + if len(tuple) != 3 && !b.mapper.ResourceIsValid(tuple[0]) { + // TODO: prints warning/error message here when tuple[0] may be resource or group (name duplication)? + return false + } + return true +} + +func getResource(groupResource string) string { + if strings.Contains(groupResource, "/") { + return strings.Split(groupResource, "/")[1] + } + return groupResource +} + +// ResourceTypeOrNameArgs indicates that the builder should accept arguments of the form +// `([,,...] [[ ...]]|/ [/...]|)` +// When one argument is received, the types provided will be retrieved from the server (and be comma delimited). // The allowEmptySelector permits to select all the resources (via Everything func). +// = (/ | ) func (b *Builder) ResourceTypeOrNameArgs(allowEmptySelector bool, args ...string) *Builder { + hasNames := b.hasNamesArg(args) + // convert multiple resources to resource tuples, a,b,c d as a transform to a/d b/d c/d if len(args) >= 2 { resources := []string{} @@ -297,13 +332,13 @@ func (b *Builder) ResourceTypeOrNameArgs(allowEmptySelector bool, args ...string } } - if ok, err := hasCombinedTypeArgs(args); ok { + if ok, err := hasCombinedTypeArgs(args); ok && hasNames { if err != nil { b.errs = append(b.errs, err) return b } for _, s := range args { - tuple, ok, err := splitResourceTypeName(s) + tuple, ok, err := splitGroupResourceTypeName(s) if err != nil { b.errs = append(b.errs, err) return b @@ -342,7 +377,7 @@ func (b *Builder) ResourceTypeOrNameArgs(allowEmptySelector bool, args ...string func (b *Builder) replaceAliases(input string) string { replaced := []string{} for _, arg := range strings.Split(input, ",") { - if aliases, ok := b.mapper.AliasesForResource(arg); ok { + if aliases, ok := b.mapper.AliasesForResource(getResource(arg)); ok { arg = strings.Join(aliases, ",") } replaced = append(replaced, arg) @@ -360,13 +395,33 @@ func hasCombinedTypeArgs(args []string) (bool, error) { switch { case hasSlash > 0 && hasSlash == len(args): return true, nil - case hasSlash > 0 && hasSlash != len(args): - return true, fmt.Errorf("when passing arguments in resource/name form, all arguments must include the resource") default: return false, nil } } +// splitGroupResourceTypeName handles group/type/name resource formats and returns a resource tuple +// (empty or not), whether it successfully found one, and an error +func splitGroupResourceTypeName(s string) (resourceTuple, bool, error) { + if !strings.Contains(s, "/") { + return resourceTuple{}, false, nil + } + seg := strings.Split(s, "/") + if len(seg) != 2 && len(seg) != 3 { + return resourceTuple{}, false, fmt.Errorf("arguments in group/resource/name form may not have more than two slashes") + } + resource, name := "", "" + if len(seg) == 2 { + resource, name = seg[0], seg[1] + } else { + resource, name = fmt.Sprintf("%s/%s", seg[0], seg[1]), seg[2] + } + if len(resource) == 0 || len(name) == 0 || len(SplitResourceArgument(resource)) != 1 { + return resourceTuple{}, false, fmt.Errorf("arguments in group/resource/name form must have a single resource and name") + } + return resourceTuple{Resource: resource, Name: name}, true, nil +} + // splitResourceTypeName handles type/name resource formats and returns a resource tuple // (empty or not), whether it successfully found one, and an error func splitResourceTypeName(s string) (resourceTuple, bool, error) { diff --git a/pkg/kubectl/resource/builder_test.go b/pkg/kubectl/resource/builder_test.go index c80a928fbb..6ba2059622 100644 --- a/pkg/kubectl/resource/builder_test.go +++ b/pkg/kubectl/resource/builder_test.go @@ -565,6 +565,112 @@ func TestSingleResourceType(t *testing.T) { } } +func TestHasNamesArg(t *testing.T) { + testCases := map[string]struct { + args []string + expected bool + }{ + "resource/name": { + args: []string{"pods/foo"}, + expected: true, + }, + "resource name": { + args: []string{"pods", "foo"}, + expected: true, + }, + "resource1,resource2 name": { + args: []string{"pods,rc", "foo"}, + expected: true, + }, + "resource1,group2/resource2 name": { + args: []string{"pods,experimental/deployments", "foo"}, + expected: true, + }, + "group/resource name": { + args: []string{"experimental/deployments", "foo"}, + expected: true, + }, + "group/resource/name": { + args: []string{"experimental/deployments/foo"}, + expected: true, + }, + "group1/resource1,group2/resource2": { + args: []string{"experimental/daemonsets,experimental/deployments"}, + expected: false, + }, + "resource1,group2/resource2": { + args: []string{"pods,experimental/deployments"}, + expected: false, + }, + "group/resource/name,group2/resource2": { + args: []string{"experimental/deployments/foo,controller/deamonset"}, + expected: false, + }, + } + for k, testCase := range testCases { + b := NewBuilder(testapi.Default.RESTMapper(), api.Scheme, fakeClient()) + if testCase.expected != b.hasNamesArg(testCase.args) { + t.Errorf("%s: unexpected argument - expected: %v", k, testCase.expected) + } + } +} + +func TestSplitGroupResourceTypeName(t *testing.T) { + expectNoErr := func(err error) bool { return err == nil } + expectErr := func(err error) bool { return err != nil } + testCases := map[string]struct { + arg string + expectedTuple resourceTuple + expectedOK bool + errFn func(error) bool + }{ + "group/type/name": { + arg: "experimental/deployments/foo", + expectedTuple: resourceTuple{Resource: "experimental/deployments", Name: "foo"}, + expectedOK: true, + errFn: expectNoErr, + }, + "type/name": { + arg: "pods/foo", + expectedTuple: resourceTuple{Resource: "pods", Name: "foo"}, + expectedOK: true, + errFn: expectNoErr, + }, + "type": { + arg: "pods", + expectedOK: false, + errFn: expectNoErr, + }, + "": { + arg: "", + expectedOK: false, + errFn: expectNoErr, + }, + "/": { + arg: "/", + expectedOK: false, + errFn: expectErr, + }, + "group/type/name/something": { + arg: "experimental/deployments/foo/something", + expectedOK: false, + errFn: expectErr, + }, + } + for k, testCase := range testCases { + tuple, ok, err := splitGroupResourceTypeName(testCase.arg) + if !testCase.errFn(err) { + t.Errorf("%s: unexpected error: %v", k, err) + } + if ok != testCase.expectedOK { + t.Errorf("%s: unexpected ok: %v", k, ok) + } + if testCase.expectedOK && !reflect.DeepEqual(tuple, testCase.expectedTuple) { + t.Errorf("%s: unexpected tuple - expected: %v, got: %v", k, testCase.expectedTuple, tuple) + } + } +} + func TestResourceTuple(t *testing.T) { expectNoErr := func(err error) bool { return err == nil } expectErr := func(err error) bool { return err != nil } diff --git a/pkg/registry/thirdpartyresourcedata/codec.go b/pkg/registry/thirdpartyresourcedata/codec.go index 6b426ee242..065971f151 100644 --- a/pkg/registry/thirdpartyresourcedata/codec.go +++ b/pkg/registry/thirdpartyresourcedata/codec.go @@ -82,6 +82,11 @@ func (t *thirdPartyResourceDataMapper) VersionAndKindForResource(resource string return t.mapper.VersionAndKindForResource(resource) } +// ResourceIsValid takes a string (kind) and checks if it's a valid resource +func (t *thirdPartyResourceDataMapper) ResourceIsValid(resource string) bool { + return t.isThirdPartyResource(resource) || t.mapper.ResourceIsValid(resource) +} + func NewMapper(mapper meta.RESTMapper, kind, version, group string) meta.RESTMapper { return &thirdPartyResourceDataMapper{ mapper: mapper,