Merge pull request #14461 from JanetKuo/kubectl-api-group-cmd

Auto commit by PR queue bot

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.

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
 }

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
+}

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"

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.

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 `(<type1>[,<type2>,...]|<type> <name1>[,<name2>,...])`. 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
+// `(<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{}
@@ -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) {

pkg/kubectl/resource/builder_test.go

@@ -557,6 +557,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 }

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,