diff --git a/pkg/api/meta/restmapper.go b/pkg/api/meta/restmapper.go index 0754582d0c6..90a6c85e9a6 100644 --- a/pkg/api/meta/restmapper.go +++ b/pkg/api/meta/restmapper.go @@ -169,25 +169,16 @@ 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("no resource %q has been defined", resource) + return "", "", fmt.Errorf("in version and kind for resource, 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("no resource %q has been defined", resource) + return "", fmt.Errorf("in group for resource, no resource %q has been defined", resource) } return m.group, nil } diff --git a/pkg/kubectl/resource/builder.go b/pkg/kubectl/resource/builder.go index 8a898100fda..4a6f42a8d48 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 or group/type and one or more names, and creates tuples of +// ResourceNames accepts a default 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 or group/type/name format - tuple, ok, err := splitGroupResourceTypeName(name) + // See if this input string is of type/name format + tuple, ok, err := splitResourceTypeName(name) if err != nil { b.errs = append(b.errs, err) return b @@ -274,47 +274,12 @@ func (b *Builder) SelectAllParam(selectAll bool) *Builder { return b } -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). +// 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). // 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{} @@ -332,13 +297,13 @@ func (b *Builder) ResourceTypeOrNameArgs(allowEmptySelector bool, args ...string } } - if ok, err := hasCombinedTypeArgs(args); ok && hasNames { + if ok, err := hasCombinedTypeArgs(args); ok { if err != nil { b.errs = append(b.errs, err) return b } for _, s := range args { - tuple, ok, err := splitGroupResourceTypeName(s) + tuple, ok, err := splitResourceTypeName(s) if err != nil { b.errs = append(b.errs, err) return b @@ -377,7 +342,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(getResource(arg)); ok { + if aliases, ok := b.mapper.AliasesForResource(arg); ok { arg = strings.Join(aliases, ",") } replaced = append(replaced, arg) @@ -395,33 +360,13 @@ 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 5f7d839e94a..44d75e67224 100644 --- a/pkg/kubectl/resource/builder_test.go +++ b/pkg/kubectl/resource/builder_test.go @@ -557,112 +557,6 @@ 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 }