fix(kubectl): explain crds with the same resource name with builtin objects
This commit is contained in:
		@@ -123,19 +123,14 @@ func (o *ExplainOptions) Run(args []string) error {
 | 
				
			|||||||
	// TODO: After we figured out the new syntax to separate group and resource, allow
 | 
						// TODO: After we figured out the new syntax to separate group and resource, allow
 | 
				
			||||||
	// the users to use it in explain (kubectl explain <group><syntax><resource>).
 | 
						// the users to use it in explain (kubectl explain <group><syntax><resource>).
 | 
				
			||||||
	// Refer to issue #16039 for why we do this. Refer to PR #15808 that used "/" syntax.
 | 
						// Refer to issue #16039 for why we do this. Refer to PR #15808 that used "/" syntax.
 | 
				
			||||||
	inModel, fieldsPath, err := explain.SplitAndParseResourceRequest(args[0], o.Mapper)
 | 
						fullySpecifiedGVR, fieldsPath, err := explain.SplitAndParseResourceRequest(args[0], o.Mapper)
 | 
				
			||||||
	if err != nil {
 | 
						if err != nil {
 | 
				
			||||||
		return err
 | 
							return err
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	// TODO: We should deduce the group for a resource by discovering the supported resources at server.
 | 
						gvk, _ := o.Mapper.KindFor(fullySpecifiedGVR)
 | 
				
			||||||
	fullySpecifiedGVR, groupResource := schema.ParseResourceArg(inModel)
 | 
					 | 
				
			||||||
	gvk := schema.GroupVersionKind{}
 | 
					 | 
				
			||||||
	if fullySpecifiedGVR != nil {
 | 
					 | 
				
			||||||
		gvk, _ = o.Mapper.KindFor(*fullySpecifiedGVR)
 | 
					 | 
				
			||||||
	}
 | 
					 | 
				
			||||||
	if gvk.Empty() {
 | 
						if gvk.Empty() {
 | 
				
			||||||
		gvk, err = o.Mapper.KindFor(groupResource.WithVersion(""))
 | 
							gvk, err = o.Mapper.KindFor(fullySpecifiedGVR.GroupResource().WithVersion(""))
 | 
				
			||||||
		if err != nil {
 | 
							if err != nil {
 | 
				
			||||||
			return err
 | 
								return err
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
@@ -151,7 +146,7 @@ func (o *ExplainOptions) Run(args []string) error {
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
	schema := o.Schema.LookupResource(gvk)
 | 
						schema := o.Schema.LookupResource(gvk)
 | 
				
			||||||
	if schema == nil {
 | 
						if schema == nil {
 | 
				
			||||||
		return fmt.Errorf("Couldn't find resource for %q", gvk)
 | 
							return fmt.Errorf("couldn't find resource for %q", gvk)
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	return explain.PrintModelDescription(fieldsPath, o.Out, schema, gvk, recursive)
 | 
						return explain.PrintModelDescription(fieldsPath, o.Out, schema, gvk, recursive)
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -20,9 +20,10 @@ import (
 | 
				
			|||||||
	"io"
 | 
						"io"
 | 
				
			||||||
	"strings"
 | 
						"strings"
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						"k8s.io/kube-openapi/pkg/util/proto"
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	"k8s.io/apimachinery/pkg/api/meta"
 | 
						"k8s.io/apimachinery/pkg/api/meta"
 | 
				
			||||||
	"k8s.io/apimachinery/pkg/runtime/schema"
 | 
						"k8s.io/apimachinery/pkg/runtime/schema"
 | 
				
			||||||
	"k8s.io/kube-openapi/pkg/util/proto"
 | 
					 | 
				
			||||||
)
 | 
					)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
type fieldsPrinter interface {
 | 
					type fieldsPrinter interface {
 | 
				
			||||||
@@ -43,10 +44,13 @@ func splitDotNotation(model string) (string, []string) {
 | 
				
			|||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
// SplitAndParseResourceRequest separates the users input into a model and fields
 | 
					// SplitAndParseResourceRequest separates the users input into a model and fields
 | 
				
			||||||
func SplitAndParseResourceRequest(inResource string, mapper meta.RESTMapper) (string, []string, error) {
 | 
					func SplitAndParseResourceRequest(inResource string, mapper meta.RESTMapper) (schema.GroupVersionResource, []string, error) {
 | 
				
			||||||
	inResource, fieldsPath := splitDotNotation(inResource)
 | 
						inResource, fieldsPath := splitDotNotation(inResource)
 | 
				
			||||||
	inResource, _ = mapper.ResourceSingularizer(inResource)
 | 
						gvr, err := mapper.ResourceFor(schema.GroupVersionResource{Resource: inResource})
 | 
				
			||||||
	return inResource, fieldsPath, nil
 | 
						if err != nil {
 | 
				
			||||||
 | 
							return schema.GroupVersionResource{}, nil, err
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
 | 
						return gvr, fieldsPath, nil
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
// PrintModelDescription prints the description of a specific model or dot path.
 | 
					// PrintModelDescription prints the description of a specific model or dot path.
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -21,37 +21,38 @@ import (
 | 
				
			|||||||
	"testing"
 | 
						"testing"
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	"k8s.io/apimachinery/pkg/api/meta/testrestmapper"
 | 
						"k8s.io/apimachinery/pkg/api/meta/testrestmapper"
 | 
				
			||||||
 | 
						"k8s.io/apimachinery/pkg/runtime/schema"
 | 
				
			||||||
	"k8s.io/kubectl/pkg/scheme"
 | 
						"k8s.io/kubectl/pkg/scheme"
 | 
				
			||||||
)
 | 
					)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
func TestSplitAndParseResourceRequest(t *testing.T) {
 | 
					func TestSplitAndParseResourceRequest(t *testing.T) {
 | 
				
			||||||
	tests := []struct {
 | 
						tests := []struct {
 | 
				
			||||||
		name       string
 | 
							name       string
 | 
				
			||||||
		inresource string
 | 
							inResource string
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		expectedInResource string
 | 
							expectedGVR        schema.GroupVersionResource
 | 
				
			||||||
		expectedFieldsPath []string
 | 
							expectedFieldsPath []string
 | 
				
			||||||
		expectedErr        bool
 | 
							expectedErr        bool
 | 
				
			||||||
	}{
 | 
						}{
 | 
				
			||||||
		{
 | 
							{
 | 
				
			||||||
			name:       "no trailing period",
 | 
								name:       "no trailing period",
 | 
				
			||||||
			inresource: "field1.field2.field3",
 | 
								inResource: "pods.field2.field3",
 | 
				
			||||||
 | 
					
 | 
				
			||||||
			expectedInResource: "field1",
 | 
								expectedGVR:        schema.GroupVersionResource{Resource: "pods", Version: "v1"},
 | 
				
			||||||
			expectedFieldsPath: []string{"field2", "field3"},
 | 
								expectedFieldsPath: []string{"field2", "field3"},
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
		{
 | 
							{
 | 
				
			||||||
			name:       "trailing period with correct fieldsPath",
 | 
								name:       "trailing period with correct fieldsPath",
 | 
				
			||||||
			inresource: "field1.field2.field3.",
 | 
								inResource: "service.field2.field3.",
 | 
				
			||||||
 | 
					
 | 
				
			||||||
			expectedInResource: "field1",
 | 
								expectedGVR:        schema.GroupVersionResource{Resource: "services", Version: "v1"},
 | 
				
			||||||
			expectedFieldsPath: []string{"field2", "field3"},
 | 
								expectedFieldsPath: []string{"field2", "field3"},
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
		{
 | 
							{
 | 
				
			||||||
			name:       "trailing period with incorrect fieldsPath",
 | 
								name:       "trailing period with incorrect fieldsPath",
 | 
				
			||||||
			inresource: "field1.field2.field3.",
 | 
								inResource: "node.field2.field3.",
 | 
				
			||||||
 | 
					
 | 
				
			||||||
			expectedInResource: "field1",
 | 
								expectedGVR:        schema.GroupVersionResource{Resource: "nodes", Version: "v1"},
 | 
				
			||||||
			expectedFieldsPath: []string{"field2", "field3", ""},
 | 
								expectedFieldsPath: []string{"field2", "field3", ""},
 | 
				
			||||||
			expectedErr:        true,
 | 
								expectedErr:        true,
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
@@ -60,13 +61,13 @@ func TestSplitAndParseResourceRequest(t *testing.T) {
 | 
				
			|||||||
	mapper := testrestmapper.TestOnlyStaticRESTMapper(scheme.Scheme, scheme.Scheme.PrioritizedVersionsAllGroups()...)
 | 
						mapper := testrestmapper.TestOnlyStaticRESTMapper(scheme.Scheme, scheme.Scheme.PrioritizedVersionsAllGroups()...)
 | 
				
			||||||
	for _, tt := range tests {
 | 
						for _, tt := range tests {
 | 
				
			||||||
		t.Run(tt.name, func(t *testing.T) {
 | 
							t.Run(tt.name, func(t *testing.T) {
 | 
				
			||||||
			gotInResource, gotFieldsPath, err := SplitAndParseResourceRequest(tt.inresource, mapper)
 | 
								gotGVR, gotFieldsPath, err := SplitAndParseResourceRequest(tt.inResource, mapper)
 | 
				
			||||||
			if err != nil {
 | 
								if err != nil {
 | 
				
			||||||
				t.Errorf("unexpected error: %v", err)
 | 
									t.Errorf("unexpected error: %v", err)
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
			if !reflect.DeepEqual(tt.expectedInResource, gotInResource) && !tt.expectedErr {
 | 
								if !reflect.DeepEqual(tt.expectedGVR, gotGVR) && !tt.expectedErr {
 | 
				
			||||||
				t.Errorf("%s: expected inresource: %s, got: %s", tt.name, tt.expectedInResource, gotInResource)
 | 
									t.Errorf("%s: expected inResource: %s, got: %s", tt.name, tt.expectedGVR, gotGVR)
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
			if !reflect.DeepEqual(tt.expectedFieldsPath, gotFieldsPath) && !tt.expectedErr {
 | 
								if !reflect.DeepEqual(tt.expectedFieldsPath, gotFieldsPath) && !tt.expectedErr {
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -68,6 +68,12 @@ var _ = SIGDescribe("CustomResourcePublishOpenAPI [Privileged:ClusterAdmin]", fu
 | 
				
			|||||||
			framework.Failf("%v", err)
 | 
								framework.Failf("%v", err)
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
							customServiceShortName := "ksvc"
 | 
				
			||||||
 | 
							crdSvc, err := setupCRDWithShortName(f, schemaCustomService, "service", customServiceShortName, "v1alpha1")
 | 
				
			||||||
 | 
							if err != nil {
 | 
				
			||||||
 | 
								framework.Failf("%v", err)
 | 
				
			||||||
 | 
							}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		meta := fmt.Sprintf(metaPattern, crd.Crd.Spec.Names.Kind, crd.Crd.Spec.Group, crd.Crd.Spec.Versions[0].Name, "test-foo")
 | 
							meta := fmt.Sprintf(metaPattern, crd.Crd.Spec.Names.Kind, crd.Crd.Spec.Group, crd.Crd.Spec.Versions[0].Name, "test-foo")
 | 
				
			||||||
		ns := fmt.Sprintf("--namespace=%v", f.Namespace.Name)
 | 
							ns := fmt.Sprintf("--namespace=%v", f.Namespace.Name)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -120,6 +126,11 @@ var _ = SIGDescribe("CustomResourcePublishOpenAPI [Privileged:ClusterAdmin]", fu
 | 
				
			|||||||
			framework.Failf("%v", err)
 | 
								framework.Failf("%v", err)
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
							ginkgo.By("kubectl explain works for CR with the same resource name as built-in object")
 | 
				
			||||||
 | 
							if err := verifyKubectlExplain(f.Namespace.Name, customServiceShortName+".spec", `(?s)DESCRIPTION:.*Specification of CustomService.*FIELDS:.*dummy.*<string>.*Dummy property`); err != nil {
 | 
				
			||||||
 | 
								framework.Failf("%v", err)
 | 
				
			||||||
 | 
							}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		ginkgo.By("kubectl explain works to return error when explain is called on property that doesn't exist")
 | 
							ginkgo.By("kubectl explain works to return error when explain is called on property that doesn't exist")
 | 
				
			||||||
		if _, err := framework.RunKubectl(f.Namespace.Name, "explain", crd.Crd.Spec.Names.Plural+".spec.bars2"); err == nil || !strings.Contains(err.Error(), `field "bars2" does not exist`) {
 | 
							if _, err := framework.RunKubectl(f.Namespace.Name, "explain", crd.Crd.Spec.Names.Plural+".spec.bars2"); err == nil || !strings.Contains(err.Error(), `field "bars2" does not exist`) {
 | 
				
			||||||
			framework.Failf("unexpected no error when explaining property that doesn't exist: %v", err)
 | 
								framework.Failf("unexpected no error when explaining property that doesn't exist: %v", err)
 | 
				
			||||||
@@ -128,6 +139,9 @@ var _ = SIGDescribe("CustomResourcePublishOpenAPI [Privileged:ClusterAdmin]", fu
 | 
				
			|||||||
		if err := cleanupCRD(f, crd); err != nil {
 | 
							if err := cleanupCRD(f, crd); err != nil {
 | 
				
			||||||
			framework.Failf("%v", err)
 | 
								framework.Failf("%v", err)
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
 | 
							if err := cleanupCRD(f, crdSvc); err != nil {
 | 
				
			||||||
 | 
								framework.Failf("%v", err)
 | 
				
			||||||
 | 
							}
 | 
				
			||||||
	})
 | 
						})
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	/*
 | 
						/*
 | 
				
			||||||
@@ -476,7 +490,24 @@ func setupCRD(f *framework.Framework, schema []byte, groupSuffix string, version
 | 
				
			|||||||
	return setupCRDAndVerifySchema(f, schema, expect, groupSuffix, versions...)
 | 
						return setupCRDAndVerifySchema(f, schema, expect, groupSuffix, versions...)
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					func setupCRDWithShortName(f *framework.Framework, schema []byte, groupSuffix, shortName string, versions ...string) (*crd.TestCrd, error) {
 | 
				
			||||||
 | 
						expect := schema
 | 
				
			||||||
 | 
						if schema == nil {
 | 
				
			||||||
 | 
							// to be backwards compatible, we expect CRD controller to treat
 | 
				
			||||||
 | 
							// CRD with nil schema specially and publish an empty schema
 | 
				
			||||||
 | 
							expect = []byte(`type: object`)
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
 | 
						setShortName := func(crd *apiextensionsv1.CustomResourceDefinition) {
 | 
				
			||||||
 | 
							crd.Spec.Names.ShortNames = []string{shortName}
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
 | 
						return setupCRDAndVerifySchemaWithOptions(f, schema, expect, groupSuffix, versions, setShortName)
 | 
				
			||||||
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
func setupCRDAndVerifySchema(f *framework.Framework, schema, expect []byte, groupSuffix string, versions ...string) (*crd.TestCrd, error) {
 | 
					func setupCRDAndVerifySchema(f *framework.Framework, schema, expect []byte, groupSuffix string, versions ...string) (*crd.TestCrd, error) {
 | 
				
			||||||
 | 
						return setupCRDAndVerifySchemaWithOptions(f, schema, expect, groupSuffix, versions)
 | 
				
			||||||
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					func setupCRDAndVerifySchemaWithOptions(f *framework.Framework, schema, expect []byte, groupSuffix string, versions []string, options ...crd.Option) (*crd.TestCrd, error) {
 | 
				
			||||||
	group := fmt.Sprintf("%s-test-%s.example.com", f.BaseName, groupSuffix)
 | 
						group := fmt.Sprintf("%s-test-%s.example.com", f.BaseName, groupSuffix)
 | 
				
			||||||
	if len(versions) == 0 {
 | 
						if len(versions) == 0 {
 | 
				
			||||||
		return nil, fmt.Errorf("require at least one version for CRD")
 | 
							return nil, fmt.Errorf("require at least one version for CRD")
 | 
				
			||||||
@@ -489,7 +520,7 @@ func setupCRDAndVerifySchema(f *framework.Framework, schema, expect []byte, grou
 | 
				
			|||||||
		}
 | 
							}
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	crd, err := crd.CreateMultiVersionTestCRD(f, group, func(crd *apiextensionsv1.CustomResourceDefinition) {
 | 
						options = append(options, func(crd *apiextensionsv1.CustomResourceDefinition) {
 | 
				
			||||||
		var apiVersions []apiextensionsv1.CustomResourceDefinitionVersion
 | 
							var apiVersions []apiextensionsv1.CustomResourceDefinitionVersion
 | 
				
			||||||
		for i, version := range versions {
 | 
							for i, version := range versions {
 | 
				
			||||||
			version := apiextensionsv1.CustomResourceDefinitionVersion{
 | 
								version := apiextensionsv1.CustomResourceDefinitionVersion{
 | 
				
			||||||
@@ -514,6 +545,7 @@ func setupCRDAndVerifySchema(f *framework.Framework, schema, expect []byte, grou
 | 
				
			|||||||
		}
 | 
							}
 | 
				
			||||||
		crd.Spec.Versions = apiVersions
 | 
							crd.Spec.Versions = apiVersions
 | 
				
			||||||
	})
 | 
						})
 | 
				
			||||||
 | 
						crd, err := crd.CreateMultiVersionTestCRD(f, group, options...)
 | 
				
			||||||
	if err != nil {
 | 
						if err != nil {
 | 
				
			||||||
		return nil, fmt.Errorf("failed to create CRD: %v", err)
 | 
							return nil, fmt.Errorf("failed to create CRD: %v", err)
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
@@ -728,6 +760,18 @@ properties:
 | 
				
			|||||||
              pattern: in-tree|out-of-tree
 | 
					              pattern: in-tree|out-of-tree
 | 
				
			||||||
              type: string`)
 | 
					              type: string`)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					var schemaCustomService = []byte(`description: CustomService CRD for Testing
 | 
				
			||||||
 | 
					type: object
 | 
				
			||||||
 | 
					properties:
 | 
				
			||||||
 | 
					  spec:
 | 
				
			||||||
 | 
					    description: Specification of CustomService
 | 
				
			||||||
 | 
					    type: object
 | 
				
			||||||
 | 
					    properties:
 | 
				
			||||||
 | 
					      dummy:
 | 
				
			||||||
 | 
					        description: Dummy property.
 | 
				
			||||||
 | 
					        type: string
 | 
				
			||||||
 | 
					`)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
var schemaWaldo = []byte(`description: Waldo CRD for Testing
 | 
					var schemaWaldo = []byte(`description: Waldo CRD for Testing
 | 
				
			||||||
type: object
 | 
					type: object
 | 
				
			||||||
properties:
 | 
					properties:
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user