Merge pull request #202 from MbolotSuse/schema-fix-1

Fixing schema definitions bugs

pkg/schema/definitions/handler.go

@@ -8,6 +8,7 @@ import (
 	"github.com/rancher/apiserver/pkg/apierror"
 	"github.com/rancher/apiserver/pkg/types"
 	"github.com/rancher/steve/pkg/schema/converter"
+	wranglerDefinition "github.com/rancher/wrangler/v2/pkg/schemas/definition"
 	"github.com/rancher/wrangler/v2/pkg/schemas/validation"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/discovery"
@@ -30,6 +31,8 @@ var (
 type SchemaDefinitionHandler struct {
 	sync.RWMutex
 
+	// baseSchema are the schemas (which may not represent a real CRD) added to the server
+	baseSchema *types.APISchemas
 	// client is the discovery client used to get the groups/resources/fields from kubernetes.
 	client discovery.DiscoveryInterface
 	// models are the cached models from the last response from kubernetes.
@@ -77,6 +80,20 @@ func (s *SchemaDefinitionHandler) byIDHandler(request *types.APIRequest) (types.
 	s.RLock()
 	defer s.RUnlock()
 
+	if baseSchema := s.baseSchema.LookupSchema(requestSchema.ID); baseSchema != nil {
+		// if this schema is a base schema it won't be in the model cache. In this case, and only this case, we process
+		// the fields independently
+		definitions := baseSchemaToDefinition(*requestSchema)
+		return types.APIObject{
+			ID:   request.Name,
+			Type: "schemaDefinition",
+			Object: schemaDefinition{
+				DefinitionType: requestSchema.ID,
+				Definitions:    definitions,
+			},
+		}, nil
+	}
+
 	if s.models == nil {
 		return types.APIObject{}, apierror.NewAPIError(notRefreshedErrorCode, "schema definitions not yet refreshed")
 	}
@@ -130,13 +147,53 @@ func (s *SchemaDefinitionHandler) indexSchemaNames(models proto.Models, groups *
 			// we can safely continue
 			continue
 		}
+		schemaID := converter.GVKToSchemaID(*gvk)
 		prefVersion := preferredResourceVersions[gvk.Group]
-		// if we don't have a known preferred version for this group or we are the preferred version
-		// add this as the model name for the schema
-		if prefVersion == "" || prefVersion == gvk.Version {
-			schemaID := converter.GVKToSchemaID(*gvk)
+		_, ok = schemaToModel[schemaID]
+		// we always add the preferred version to the map. However, if this isn't the preferred version the preferred group could
+		// be missing this resource (e.x. v1alpha1 has a resource, it's removed in v1). In those cases, we add the model name
+		// only if we don't already have an entry. This way we always choose the preferred, if possible, but still have 1 version
+		// for everything
+		if !ok || prefVersion == gvk.Version {
 			schemaToModel[schemaID] = modelName
 		}
 	}
 	return schemaToModel
 }
+
+// baseSchemaToDefinition converts a given schema to the definition map. This should only be used with baseSchemas, whose definitions
+// are expected to be set by another application and may not be k8s resources.
+func baseSchemaToDefinition(schema types.APISchema) map[string]definition {
+	definitions := map[string]definition{}
+	def := definition{
+		Description:    schema.Description,
+		Type:           schema.ID,
+		ResourceFields: map[string]definitionField{},
+	}
+	for fieldName, field := range schema.ResourceFields {
+		fieldType, subType := parseFieldType(field.Type)
+		def.ResourceFields[fieldName] = definitionField{
+			Type:        fieldType,
+			SubType:     subType,
+			Description: field.Description,
+			Required:    field.Required,
+		}
+	}
+	definitions[schema.ID] = def
+	return definitions
+}
+
+// parseFieldType parses a schemas.Field's type to a type (first return) and subType (second return)
+func parseFieldType(fieldType string) (string, string) {
+	subType := wranglerDefinition.SubType(fieldType)
+	if wranglerDefinition.IsMapType(fieldType) {
+		return "map", subType
+	}
+	if wranglerDefinition.IsArrayType(fieldType) {
+		return "array", subType
+	}
+	if wranglerDefinition.IsReferenceType(fieldType) {
+		return "reference", subType
+	}
+	return fieldType, ""
+}

pkg/schema/definitions/handler_test.go

@@ -23,9 +23,10 @@ func TestRefresh(t *testing.T) {
 	defaultModels, err := proto.NewOpenAPIData(defaultDocument)
 	require.NoError(t, err)
 	defaultSchemaToModel := map[string]string{
-		"management.cattle.io.globalrole": "io.cattle.management.v1.GlobalRole",
-		"noversion.cattle.io.resource":    "io.cattle.noversion.v2.Resource",
-		"missinggroup.cattle.io.resource": "io.cattle.missinggroup.v2.Resource",
+		"management.cattle.io.globalrole":  "io.cattle.management.v2.GlobalRole",
+		"management.cattle.io.newresource": "io.cattle.management.v2.NewResource",
+		"noversion.cattle.io.resource":     "io.cattle.noversion.v1.Resource",
+		"missinggroup.cattle.io.resource":  "io.cattle.missinggroup.v1.Resource",
 	}
 	tests := []struct {
 		name              string
@@ -62,9 +63,10 @@ func TestRefresh(t *testing.T) {
 			nilGroups:  true,
 			wantModels: &defaultModels,
 			wantSchemaToModel: map[string]string{
-				"management.cattle.io.globalrole": "io.cattle.management.v2.GlobalRole",
-				"noversion.cattle.io.resource":    "io.cattle.noversion.v2.Resource",
-				"missinggroup.cattle.io.resource": "io.cattle.missinggroup.v2.Resource",
+				"management.cattle.io.globalrole":  "io.cattle.management.v1.GlobalRole",
+				"management.cattle.io.newresource": "io.cattle.management.v2.NewResource",
+				"noversion.cattle.io.resource":     "io.cattle.noversion.v1.Resource",
+				"missinggroup.cattle.io.resource":  "io.cattle.missinggroup.v1.Resource",
 			},
 		},
 	}
@@ -110,7 +112,7 @@ func Test_byID(t *testing.T) {
 		"management.cattle.io.globalrole": "io.cattle.management.v2.GlobalRole",
 	}
 	schemas := types.EmptyAPISchemas()
-	addBaseSchema := func(names ...string) {
+	addSchema := func(names ...string) {
 		for _, name := range names {
 			schemas.MustAddSchema(types.APISchema{
 				Schema: &wschemas.Schema{
@@ -125,8 +127,41 @@ func Test_byID(t *testing.T) {
 	intPtr := func(input int) *int {
 		return &input
 	}
-
-	addBaseSchema("management.cattle.io.globalrole", "management.cattle.io.missingfrommodel", "management.cattle.io.notakind")
+	builtinSchema := types.APISchema{
+		Schema: &wschemas.Schema{
+			ID:                "builtin",
+			Description:       "some builtin type",
+			CollectionMethods: []string{"get"},
+			ResourceMethods:   []string{"get"},
+			ResourceFields: map[string]wschemas.Field{
+				"complex": {
+					Type:        "map[string]",
+					Description: "some complex field",
+				},
+				"complexArray": {
+					Type:        "array[string]",
+					Description: "some complex array field",
+				},
+				"complexRef": {
+					Type:        "reference[complex]",
+					Description: "some complex reference field",
+				},
+				"simple": {
+					Type:        "string",
+					Description: "some simple field",
+					Required:    true,
+				},
+				"leftBracket": {
+					Type:        "test[",
+					Description: "some field with a open bracket but no close bracket",
+				},
+			},
+		},
+	}
+	addSchema("management.cattle.io.globalrole", "management.cattle.io.missingfrommodel", "management.cattle.io.notakind")
+	baseSchemas := types.EmptyAPISchemas()
+	baseSchemas.MustAddSchema(builtinSchema)
+	schemas.MustAddSchema(builtinSchema)
 
 	tests := []struct {
 		name          string
@@ -213,6 +248,51 @@ func Test_byID(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:          "baseSchema",
+			schemaName:    "builtin",
+			models:        &defaultModels,
+			schemaToModel: defaultSchemaToModel,
+			wantObject: &types.APIObject{
+				ID:   "builtin",
+				Type: "schemaDefinition",
+				Object: schemaDefinition{
+					DefinitionType: "builtin",
+					Definitions: map[string]definition{
+						"builtin": {
+							ResourceFields: map[string]definitionField{
+								"complex": {
+									Type:        "map",
+									SubType:     "string",
+									Description: "some complex field",
+								},
+								"complexArray": {
+									Type:        "array",
+									SubType:     "string",
+									Description: "some complex array field",
+								},
+								"complexRef": {
+									Type:        "reference",
+									SubType:     "complex",
+									Description: "some complex reference field",
+								},
+								"simple": {
+									Type:        "string",
+									Description: "some simple field",
+									Required:    true,
+								},
+								"leftBracket": {
+									Type:        "test[",
+									Description: "some field with a open bracket but no close bracket",
+								},
+							},
+							Type:        "builtin",
+							Description: "some builtin type",
+						},
+					},
+				},
+			},
+		},
 		{
 			name:          "missing definition",
 			schemaName:    "management.cattle.io.cluster",
@@ -252,6 +332,7 @@ func Test_byID(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			t.Parallel()
 			handler := SchemaDefinitionHandler{
+				baseSchema:    baseSchemas,
 				models:        test.models,
 				schemaToModel: test.schemaToModel,
 			}
@@ -285,7 +366,7 @@ func buildDefaultDiscovery() (*fakeDiscovery, error) {
 			Name: "management.cattle.io",
 			PreferredVersion: metav1.GroupVersionForDiscovery{
 				GroupVersion: "management.cattle.io/v2",
-				Version:      "v1",
+				Version:      "v2",
 			},
 			Versions: []metav1.GroupVersionForDiscovery{
 				{

pkg/schema/definitions/openapi_test.go

@@ -82,6 +82,35 @@ definitions:
     - group: "management.cattle.io"
       version: "v2"
       kind: "GlobalRole"
+  io.cattle.management.v2.NewResource:
+    description: "A resource that's in the v2 group, but not v1"
+    type: "object"
+    properties:
+      apiVersion:
+        description: "The APIVersion of this resource"
+        type: "string"
+      kind:
+        description: "The kind"
+        type: "string"
+      metadata:
+        description: "The metadata"
+        $ref: "#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"
+      spec:
+        description: "The spec for the new resource"
+        type: "object"
+        required:
+        - "someRequired"
+        properties:
+          someRequired:
+            description: "A required field"
+            type: "string"
+          notRequired:
+            description: "Some field that isn't required"
+            type: "boolean"
+    x-kubernetes-group-version-kind:
+    - group: "management.cattle.io"
+      version: "v2"
+      kind: "NewResource"
   io.cattle.noversion.v2.Resource:
     description: "A No Version V2 resource is for a group with no preferred version"
     type: "object"

pkg/schema/definitions/schema.go

@@ -50,7 +50,8 @@ func Register(ctx context.Context,
 	crd apiextcontrollerv1.CustomResourceDefinitionController,
 	apiService v1.APIServiceController) {
 	handler := SchemaDefinitionHandler{
-		client: client,
+		baseSchema: baseSchema,
+		client:     client,
 	}
 	baseSchema.MustAddSchema(types.APISchema{
 		Schema: &schemas.Schema{