Merge pull request #16054 from smarterclayton/revert_extension_syntax

Auto commit by PR queue bot

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
 }

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
+		// See if this input string is of type/name format
-		tuple, ok, err := splitGroupResourceTypeName(name)
+		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 {
+// ResourceTypeOrNameArgs indicates that the builder should accept arguments
-	if len(args) > 1 {
+// of the form `(<type1>[,<type2>,...]|<type> <name1>[,<name2>,...])`. When one argument is
-		return true
+// 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).
-	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
-// `(<type1>[,<type2>,...] [<name1>[ <name2> ...]]|<type1>/<name1> [<type2>/<name2>...]|)`
-// 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).
-// <type> = (<group>/<resource type> | <resource type>)
 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) {

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 }