From 72384a606d57ff4dd235c5c7f86961cf272c9027 Mon Sep 17 00:00:00 2001 From: Eric Promislow Date: Fri, 26 Jul 2024 12:07:29 -0700 Subject: [PATCH 1/3] Handle schema-less CRDs --- pkg/schema/definitions/handler.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/schema/definitions/handler.go b/pkg/schema/definitions/handler.go index d45eeedf..cedf4fc6 100644 --- a/pkg/schema/definitions/handler.go +++ b/pkg/schema/definitions/handler.go @@ -223,7 +223,11 @@ func listGVKModels(models proto.Models, groups *metav1.APIGroupList, crdCache wa Version: version.Name, Kind: crd.Spec.Names.Kind, } - gvkToCRD[gvk] = version.Schema.OpenAPIV3Schema + if version.Schema != nil { + gvkToCRD[gvk] = version.Schema.OpenAPIV3Schema + } else { + gvkToCRD[gvk] = nil + } } } From 8ce0b83be71a1bf81a9a870275d5cce1274fc599 Mon Sep 17 00:00:00 2001 From: Eric Promislow Date: Mon, 29 Jul 2024 16:31:39 -0700 Subject: [PATCH 2/3] Add unit tests for schemaless CRDs --- pkg/schema/definitions/fixtures_test.go | 113 ++++++++++++++++++++++++ pkg/schema/definitions/handler_test.go | 112 +++++++++++++++++++++++ 2 files changed, 225 insertions(+) diff --git a/pkg/schema/definitions/fixtures_test.go b/pkg/schema/definitions/fixtures_test.go index afc4f811..2eaf1623 100644 --- a/pkg/schema/definitions/fixtures_test.go +++ b/pkg/schema/definitions/fixtures_test.go @@ -4,8 +4,10 @@ import ( "bytes" "fmt" + openapi_v2 "github.com/google/gnostic-models/openapiv2" "github.com/rancher/wrangler/v3/pkg/yaml" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/kube-openapi/pkg/util/proto" ) var ( @@ -398,3 +400,114 @@ definitions: kind: "ConfigMap" version: "v1" ` + +var ( + rawSchemalessCRDs = ` +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: schemaless.management.cattle.io +spec: + conversion: + strategy: None + group: management.cattle.io + names: + kind: Schemaless + listKind: SchemalessList + plural: schemalese + singular: schemaless + scope: Cluster + versions: + - name: v2 + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + rkeConfig: + type: object + nullable: true + x-kubernetes-preserve-unknown-fields: true + served: true + storage: true +` + + rawSchemalessModels = ` +swagger: "2.0" +info: + title: "Test openapi spec" + version: "v1.0.0" +paths: + /apis/management.cattle.io/v3/globalroles: + get: + description: "get a global role" + responses: + 200: + description: "OK" +definitions: + io.cattle.management.v2.schemaless: + description: "this kind has no schema" + 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 resource" + type: "object" + required: + - "name" + properties: + name: + description: "The name of the resource" + type: "string" + notRequired: + description: "Some field that isn't required" + type: "boolean" + x-kubernetes-group-version-kind: + - group: "management.cattle.io" + version: "v2" + kind: "Schemaless" + io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta: + description: "Object Metadata" + properties: + annotations: + description: "annotations of the resource" + type: "object" + additionalProperties: + type: "string" + name: + description: "name of the resource" + type: "string" +` +) + +func getSchemalessCRDs() ([]*apiextv1.CustomResourceDefinition, error) { + crds, err := yaml.UnmarshalWithJSONDecoder[*apiextv1.CustomResourceDefinition](bytes.NewBuffer([]byte(rawSchemalessCRDs))) + if err != nil { + return nil, fmt.Errorf("unmarshaling raw CRDs: %w", err) + } + for _, crd := range crds { + for _, crdVersion := range crd.Spec.Versions { + crdVersion.Schema = nil + } + } + return crds, err +} + +func getSchemalessModels() (proto.Models, error) { + doc, err := openapi_v2.ParseDocument([]byte(rawSchemalessModels)) + if err != nil { + return nil, fmt.Errorf("unmarshaling raw models: %w", err) + } + models, err := proto.NewOpenAPIData(doc) + return models, err +} diff --git a/pkg/schema/definitions/handler_test.go b/pkg/schema/definitions/handler_test.go index 2010f0ce..85cc20f0 100644 --- a/pkg/schema/definitions/handler_test.go +++ b/pkg/schema/definitions/handler_test.go @@ -196,6 +196,59 @@ func TestRefresh(t *testing.T) { } } +func TestRefreshSchemalessCRDs(t *testing.T) { + schemalessModels, err := getSchemalessModels() + require.NoError(t, err) + + crds, err := getSchemalessCRDs() + require.NoError(t, err) + + for _, crd := range crds { + for _, crdVersion := range crd.Spec.Versions { + crdVersion.Schema = nil + } + } + + test := struct { + name string + nilGroups bool + wantModels proto.Models + wantGVKModels map[string]gvkModel + }{ + name: "problem - missing schema", + wantModels: schemalessModels, + wantGVKModels: map[string]gvkModel{ + "management.cattle.io.schemaless": { + ModelName: "io.cattle.management.v2.schemaless", + }, + }, + } + t.Run(test.name, func(t *testing.T) { + client, err := buildDefaultServerlessDiscovery() + require.Nil(t, err) + baseSchemas := types.EmptyAPISchemas() + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + crdCache := fake.NewMockNonNamespacedCacheInterface[*apiextv1.CustomResourceDefinition](ctrl) + crdCache.EXPECT().List(labels.Everything()).Return(crds, nil).AnyTimes() + + handler := NewSchemaDefinitionHandler(baseSchemas, crdCache, client) + err = handler.Refresh() + require.NoError(t, err) + + handler.lock.RLock() + defer handler.lock.RUnlock() + require.Equal(t, test.wantModels, handler.models) + // Just test the model names, because the schema and crd will be null in the input + require.Equal(t, len(test.wantGVKModels), len(handler.gvkModels)) + for k, v := range test.wantGVKModels { + require.Equal(t, v.ModelName, handler.gvkModels[k].ModelName) + } + }) +} + func Test_byID(t *testing.T) { discoveryClient, err := buildDefaultDiscovery() require.NoError(t, err) @@ -735,6 +788,65 @@ func buildDefaultDiscovery() (*fakeDiscovery, error) { }, nil } +func buildDefaultServerlessDiscovery() (*fakeDiscovery, error) { + document, err := openapi_v2.ParseDocument([]byte(rawSchemalessModels)) + if err != nil { + return nil, fmt.Errorf("unable to parse rawSchemalessModels: %w", err) + } + groups := []metav1.APIGroup{ + // The core groups (eg: Pods, ConfigMaps, etc) + { + Name: "", + PreferredVersion: metav1.GroupVersionForDiscovery{ + GroupVersion: "v1", + Version: "v1", + }, + Versions: []metav1.GroupVersionForDiscovery{ + { + GroupVersion: "v1", + Version: "v1", + }, + }, + }, + { + Name: "management.cattle.io", + PreferredVersion: metav1.GroupVersionForDiscovery{ + GroupVersion: "management.cattle.io/v2", + Version: "v2", + }, + Versions: []metav1.GroupVersionForDiscovery{ + { + GroupVersion: "management.cattle.io/v1", + Version: "v1", + }, + { + GroupVersion: "management.cattle.io/v2", + Version: "v2", + }, + }, + }, + { + Name: "noversion.cattle.io", + Versions: []metav1.GroupVersionForDiscovery{ + { + GroupVersion: "noversion.cattle.io/v1", + Version: "v1", + }, + { + GroupVersion: "noversion.cattle.io/v2", + Version: "v2", + }, + }, + }, + } + return &fakeDiscovery{ + Groups: &metav1.APIGroupList{ + Groups: groups, + }, + Document: document, + }, nil +} + type fakeDiscovery struct { Groups *metav1.APIGroupList Document *openapi_v2.Document From dfae21cb393889a255ca938f50efd2d0d7c89019 Mon Sep 17 00:00:00 2001 From: Michael Bolot Date: Mon, 5 Aug 2024 12:34:59 -0500 Subject: [PATCH 3/3] Adding changes from code review --- pkg/schema/definitions/fixtures_test.go | 162 +++++++----------------- pkg/schema/definitions/handler.go | 2 - pkg/schema/definitions/handler_test.go | 124 ++---------------- 3 files changed, 60 insertions(+), 228 deletions(-) diff --git a/pkg/schema/definitions/fixtures_test.go b/pkg/schema/definitions/fixtures_test.go index 2eaf1623..6874845f 100644 --- a/pkg/schema/definitions/fixtures_test.go +++ b/pkg/schema/definitions/fixtures_test.go @@ -4,10 +4,8 @@ import ( "bytes" "fmt" - openapi_v2 "github.com/google/gnostic-models/openapiv2" "github.com/rancher/wrangler/v3/pkg/yaml" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/kube-openapi/pkg/util/proto" ) var ( @@ -65,6 +63,26 @@ spec: nullable: true served: true storage: true +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: schemaless.management.cattle.io +spec: + conversion: + strategy: None + group: management.cattle.io + names: + kind: Schemaless + listKind: SchemalessList + plural: schemalese + singular: schemaless + scope: Cluster + preserveUnkownFields: true + versions: + - name: v2 + served: true + storage: true ` ) @@ -348,6 +366,35 @@ definitions: - group: "management.cattle.io" version: "v2" kind: "Nullable" + io.cattle.management.v2.Schemaless: + description: "this kind has no schema" + 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 resource" + type: "object" + required: + - "name" + properties: + name: + description: "The name of the resource" + type: "string" + notRequired: + description: "Some field that isn't required" + type: "boolean" + x-kubernetes-group-version-kind: + - group: "management.cattle.io" + version: "v2" + kind: "Schemaless" io.cattle.management.NotAKind: type: "string" description: "Some string which isn't a kind" @@ -400,114 +447,3 @@ definitions: kind: "ConfigMap" version: "v1" ` - -var ( - rawSchemalessCRDs = ` -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - name: schemaless.management.cattle.io -spec: - conversion: - strategy: None - group: management.cattle.io - names: - kind: Schemaless - listKind: SchemalessList - plural: schemalese - singular: schemaless - scope: Cluster - versions: - - name: v2 - schema: - openAPIV3Schema: - type: object - properties: - spec: - type: object - properties: - rkeConfig: - type: object - nullable: true - x-kubernetes-preserve-unknown-fields: true - served: true - storage: true -` - - rawSchemalessModels = ` -swagger: "2.0" -info: - title: "Test openapi spec" - version: "v1.0.0" -paths: - /apis/management.cattle.io/v3/globalroles: - get: - description: "get a global role" - responses: - 200: - description: "OK" -definitions: - io.cattle.management.v2.schemaless: - description: "this kind has no schema" - 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 resource" - type: "object" - required: - - "name" - properties: - name: - description: "The name of the resource" - type: "string" - notRequired: - description: "Some field that isn't required" - type: "boolean" - x-kubernetes-group-version-kind: - - group: "management.cattle.io" - version: "v2" - kind: "Schemaless" - io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta: - description: "Object Metadata" - properties: - annotations: - description: "annotations of the resource" - type: "object" - additionalProperties: - type: "string" - name: - description: "name of the resource" - type: "string" -` -) - -func getSchemalessCRDs() ([]*apiextv1.CustomResourceDefinition, error) { - crds, err := yaml.UnmarshalWithJSONDecoder[*apiextv1.CustomResourceDefinition](bytes.NewBuffer([]byte(rawSchemalessCRDs))) - if err != nil { - return nil, fmt.Errorf("unmarshaling raw CRDs: %w", err) - } - for _, crd := range crds { - for _, crdVersion := range crd.Spec.Versions { - crdVersion.Schema = nil - } - } - return crds, err -} - -func getSchemalessModels() (proto.Models, error) { - doc, err := openapi_v2.ParseDocument([]byte(rawSchemalessModels)) - if err != nil { - return nil, fmt.Errorf("unmarshaling raw models: %w", err) - } - models, err := proto.NewOpenAPIData(doc) - return models, err -} diff --git a/pkg/schema/definitions/handler.go b/pkg/schema/definitions/handler.go index cedf4fc6..ad87f8c4 100644 --- a/pkg/schema/definitions/handler.go +++ b/pkg/schema/definitions/handler.go @@ -225,8 +225,6 @@ func listGVKModels(models proto.Models, groups *metav1.APIGroupList, crdCache wa } if version.Schema != nil { gvkToCRD[gvk] = version.Schema.OpenAPIV3Schema - } else { - gvkToCRD[gvk] = nil } } } diff --git a/pkg/schema/definitions/handler_test.go b/pkg/schema/definitions/handler_test.go index 85cc20f0..8e06a41c 100644 --- a/pkg/schema/definitions/handler_test.go +++ b/pkg/schema/definitions/handler_test.go @@ -34,7 +34,7 @@ func TestRefresh(t *testing.T) { require.NotNil(t, userAttributesV2) nullableV2 := getJSONSchema(crds, "nullable.management.cattle.io", "v2") - require.NotNil(t, userAttributesV2) + require.NotNil(t, nullableV2) tests := []struct { name string @@ -85,6 +85,11 @@ func TestRefresh(t *testing.T) { Schema: defaultModels.LookupModel("io.cattle.management.v2.Nullable"), CRD: nullableV2, }, + "management.cattle.io.schemaless": { + ModelName: "io.cattle.management.v2.Schemaless", + Schema: defaultModels.LookupModel("io.cattle.management.v2.Schemaless"), + CRD: nil, + }, }, }, { @@ -147,6 +152,11 @@ func TestRefresh(t *testing.T) { Schema: defaultModels.LookupModel("io.cattle.management.v2.Nullable"), CRD: nullableV2, }, + "management.cattle.io.schemaless": { + ModelName: "io.cattle.management.v2.Schemaless", + Schema: defaultModels.LookupModel("io.cattle.management.v2.Schemaless"), + CRD: nil, + }, }, }, } @@ -196,59 +206,6 @@ func TestRefresh(t *testing.T) { } } -func TestRefreshSchemalessCRDs(t *testing.T) { - schemalessModels, err := getSchemalessModels() - require.NoError(t, err) - - crds, err := getSchemalessCRDs() - require.NoError(t, err) - - for _, crd := range crds { - for _, crdVersion := range crd.Spec.Versions { - crdVersion.Schema = nil - } - } - - test := struct { - name string - nilGroups bool - wantModels proto.Models - wantGVKModels map[string]gvkModel - }{ - name: "problem - missing schema", - wantModels: schemalessModels, - wantGVKModels: map[string]gvkModel{ - "management.cattle.io.schemaless": { - ModelName: "io.cattle.management.v2.schemaless", - }, - }, - } - t.Run(test.name, func(t *testing.T) { - client, err := buildDefaultServerlessDiscovery() - require.Nil(t, err) - baseSchemas := types.EmptyAPISchemas() - - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - crdCache := fake.NewMockNonNamespacedCacheInterface[*apiextv1.CustomResourceDefinition](ctrl) - crdCache.EXPECT().List(labels.Everything()).Return(crds, nil).AnyTimes() - - handler := NewSchemaDefinitionHandler(baseSchemas, crdCache, client) - err = handler.Refresh() - require.NoError(t, err) - - handler.lock.RLock() - defer handler.lock.RUnlock() - require.Equal(t, test.wantModels, handler.models) - // Just test the model names, because the schema and crd will be null in the input - require.Equal(t, len(test.wantGVKModels), len(handler.gvkModels)) - for k, v := range test.wantGVKModels { - require.Equal(t, v.ModelName, handler.gvkModels[k].ModelName) - } - }) -} - func Test_byID(t *testing.T) { discoveryClient, err := buildDefaultDiscovery() require.NoError(t, err) @@ -788,65 +745,6 @@ func buildDefaultDiscovery() (*fakeDiscovery, error) { }, nil } -func buildDefaultServerlessDiscovery() (*fakeDiscovery, error) { - document, err := openapi_v2.ParseDocument([]byte(rawSchemalessModels)) - if err != nil { - return nil, fmt.Errorf("unable to parse rawSchemalessModels: %w", err) - } - groups := []metav1.APIGroup{ - // The core groups (eg: Pods, ConfigMaps, etc) - { - Name: "", - PreferredVersion: metav1.GroupVersionForDiscovery{ - GroupVersion: "v1", - Version: "v1", - }, - Versions: []metav1.GroupVersionForDiscovery{ - { - GroupVersion: "v1", - Version: "v1", - }, - }, - }, - { - Name: "management.cattle.io", - PreferredVersion: metav1.GroupVersionForDiscovery{ - GroupVersion: "management.cattle.io/v2", - Version: "v2", - }, - Versions: []metav1.GroupVersionForDiscovery{ - { - GroupVersion: "management.cattle.io/v1", - Version: "v1", - }, - { - GroupVersion: "management.cattle.io/v2", - Version: "v2", - }, - }, - }, - { - Name: "noversion.cattle.io", - Versions: []metav1.GroupVersionForDiscovery{ - { - GroupVersion: "noversion.cattle.io/v1", - Version: "v1", - }, - { - GroupVersion: "noversion.cattle.io/v2", - Version: "v2", - }, - }, - }, - } - return &fakeDiscovery{ - Groups: &metav1.APIGroupList{ - Groups: groups, - }, - Document: document, - }, nil -} - type fakeDiscovery struct { Groups *metav1.APIGroupList Document *openapi_v2.Document