From c6b887c1cb0a2142403a36341f9d829edcfa20c1 Mon Sep 17 00:00:00 2001 From: Michael Bolot Date: Fri, 26 Apr 2024 08:50:06 -0500 Subject: [PATCH] Fixing schema definitions bugs Fixes two bugs with the schema definitions: - Adds resources that are a part of the baseSchema (but aren't k8s resources). - Adds logic to handle resources which aren't in a prefered version, but are still in some version. --- pkg/schema/definitions/handler.go | 65 +++++++++++++++- pkg/schema/definitions/handler_test.go | 101 ++++++++++++++++++++++--- pkg/schema/definitions/openapi_test.go | 29 +++++++ pkg/schema/definitions/schema.go | 3 +- 4 files changed, 183 insertions(+), 15 deletions(-) diff --git a/pkg/schema/definitions/handler.go b/pkg/schema/definitions/handler.go index c54da3f9..c0a869f7 100644 --- a/pkg/schema/definitions/handler.go +++ b/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, "" +} diff --git a/pkg/schema/definitions/handler_test.go b/pkg/schema/definitions/handler_test.go index f7a96b7d..9adb3693 100644 --- a/pkg/schema/definitions/handler_test.go +++ b/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{ { diff --git a/pkg/schema/definitions/openapi_test.go b/pkg/schema/definitions/openapi_test.go index 1ab4fac3..b49d90e5 100644 --- a/pkg/schema/definitions/openapi_test.go +++ b/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" diff --git a/pkg/schema/definitions/schema.go b/pkg/schema/definitions/schema.go index b9ba1ebc..a4ee622f 100644 --- a/pkg/schema/definitions/schema.go +++ b/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{