diff --git a/pkg/schema/definitions/handler.go b/pkg/schema/definitions/handler.go index 155950d9..c54da3f9 100644 --- a/pkg/schema/definitions/handler.go +++ b/pkg/schema/definitions/handler.go @@ -1,7 +1,6 @@ package definitions import ( - "errors" "fmt" "net/http" "sync" @@ -10,7 +9,7 @@ import ( "github.com/rancher/apiserver/pkg/types" "github.com/rancher/steve/pkg/schema/converter" "github.com/rancher/wrangler/v2/pkg/schemas/validation" - "k8s.io/apimachinery/pkg/runtime/schema" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/discovery" "k8s.io/kube-openapi/pkg/util/proto" ) @@ -43,8 +42,6 @@ type SchemaDefinitionHandler struct { // Refresh writeLocks and updates the cache with new schemaDefinitions. Will result in a call to kubernetes to retrieve // the openAPI schemas. func (s *SchemaDefinitionHandler) Refresh() error { - s.Lock() - defer s.Unlock() openapi, err := s.client.OpenAPISchema() if err != nil { return fmt.Errorf("unable to fetch openapi definition: %w", err) @@ -53,16 +50,15 @@ func (s *SchemaDefinitionHandler) Refresh() error { if err != nil { return fmt.Errorf("unable to parse openapi definition into models: %w", err) } - s.models = &models - nameIndex, err := s.indexSchemaNames(models) - // indexSchemaNames may successfully refresh some definitions, but still return an error - // in these cases, store what we could find, but still return up an error - if nameIndex != nil { - s.schemaToModel = nameIndex - } + groups, err := s.client.ServerGroups() if err != nil { - return fmt.Errorf("unable to index schema name to model name: %w", err) + return fmt.Errorf("unable to retrieve groups: %w", err) } + s.Lock() + defer s.Unlock() + nameIndex := s.indexSchemaNames(models, groups) + s.schemaToModel = nameIndex + s.models = &models return nil } @@ -113,34 +109,12 @@ func (s *SchemaDefinitionHandler) byIDHandler(request *types.APIRequest) (types. } // indexSchemaNames returns a map of schemaID to the modelName for a given schema. Will use the preferred version of a -// resource if possible. May return a map and an error if it was able to index some schemas but not others. -func (s *SchemaDefinitionHandler) indexSchemaNames(models proto.Models) (map[string]string, error) { - _, resourceLists, err := s.client.ServerGroupsAndResources() - // this may occasionally fail to discover certain groups, but we still can refresh the others in those cases - if _, ok := err.(*discovery.ErrGroupDiscoveryFailed); err != nil && !ok { - return nil, fmt.Errorf("unable to retrieve groups and resources: %w", err) - } - preferredResourceVersions := map[schema.GroupKind]string{} - for _, resourceList := range resourceLists { - if resourceList == nil { - continue - } - groupVersion, gvErr := schema.ParseGroupVersion(resourceList.GroupVersion) - // we may fail to parse the GV of one group, but can still parse out the others - if gvErr != nil { - err = errors.Join(err, fmt.Errorf("unable to parse group version %s: %w", resourceList.GroupVersion, gvErr)) - continue - } - for _, resource := range resourceList.APIResources { - gk := schema.GroupKind{ - Group: groupVersion.Group, - Kind: resource.Kind, - } - // per the resource docs, if the resource.Version is empty, the preferred version for - // this resource is the version of the APIResourceList it is in - if resource.Version == "" || resource.Version == groupVersion.Version { - preferredResourceVersions[gk] = groupVersion.Version - } +// resource if possible. Can return an error if unable to find groups. +func (s *SchemaDefinitionHandler) indexSchemaNames(models proto.Models, groups *metav1.APIGroupList) map[string]string { + preferredResourceVersions := map[string]string{} + if groups != nil { + for _, group := range groups.Groups { + preferredResourceVersions[group.Name] = group.PreferredVersion.Version } } schemaToModel := map[string]string{} @@ -156,17 +130,13 @@ func (s *SchemaDefinitionHandler) indexSchemaNames(models proto.Models) (map[str // we can safely continue continue } - gk := schema.GroupKind{ - Group: gvk.Group, - Kind: gvk.Kind, - } - prefVersion, ok := preferredResourceVersions[gk] + 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 !ok || prefVersion == gvk.Version { + if prefVersion == "" || prefVersion == gvk.Version { schemaID := converter.GVKToSchemaID(*gvk) schemaToModel[schemaID] = modelName } } - return schemaToModel, err + return schemaToModel } diff --git a/pkg/schema/definitions/handler_test.go b/pkg/schema/definitions/handler_test.go index 1ad676a4..f7a96b7d 100644 --- a/pkg/schema/definitions/handler_test.go +++ b/pkg/schema/definitions/handler_test.go @@ -10,7 +10,6 @@ import ( wschemas "github.com/rancher/wrangler/v2/pkg/schemas" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/version" "k8s.io/client-go/discovery" "k8s.io/client-go/openapi" @@ -24,17 +23,19 @@ 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.v2.GlobalRole", + "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", } tests := []struct { - name string - openapiError error - serverGroupsResourcesErr error - useBadOpenApiDoc bool - unparseableGV bool - wantModels *proto.Models - wantSchemaToModel map[string]string - wantError bool + name string + openapiError error + serverGroupsErr error + useBadOpenApiDoc bool + nilGroups bool + wantModels *proto.Models + wantSchemaToModel map[string]string + wantError bool }{ { name: "success", @@ -52,31 +53,19 @@ func TestRefresh(t *testing.T) { wantError: true, }, { - name: "error - unable to retrieve groups and resources", - serverGroupsResourcesErr: fmt.Errorf("server not available"), - wantModels: &defaultModels, - wantError: true, + name: "error - unable to retrieve groups and resources", + serverGroupsErr: fmt.Errorf("server not available"), + wantError: true, }, { - name: "error - unable to retrieve all groups and resources", - serverGroupsResourcesErr: &discovery.ErrGroupDiscoveryFailed{ - Groups: map[schema.GroupVersion]error{ - { - Group: "other.cattle.io", - Version: "v1", - }: fmt.Errorf("some group error"), - }, + name: "no groups or error from server", + 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", }, - wantModels: &defaultModels, - wantSchemaToModel: defaultSchemaToModel, - wantError: true, - }, - { - name: "error - unparesable gv", - unparseableGV: true, - wantModels: &defaultModels, - wantSchemaToModel: defaultSchemaToModel, - wantError: true, }, } for _, test := range tests { @@ -85,17 +74,15 @@ func TestRefresh(t *testing.T) { t.Parallel() client, err := buildDefaultDiscovery() client.DocumentErr = test.openapiError - client.GroupResourcesErr = test.serverGroupsResourcesErr + client.GroupsErr = test.serverGroupsErr if test.useBadOpenApiDoc { schema := client.Document.Definitions.AdditionalProperties[0] schema.Value.Type = &openapi_v2.TypeItem{ Value: []string{"multiple", "entries"}, } } - if test.unparseableGV { - client.Resources = append(client.Resources, &metav1.APIResourceList{ - GroupVersion: "not/parse/able", - }) + if test.nilGroups { + client.Groups = nil } require.Nil(t, err) handler := SchemaDefinitionHandler{ @@ -293,66 +280,64 @@ func buildDefaultDiscovery() (*fakeDiscovery, error) { if err != nil { return nil, fmt.Errorf("unable to parse openapi document %w", err) } - groups := []*metav1.APIGroup{ + groups := []metav1.APIGroup{ { Name: "management.cattle.io", PreferredVersion: metav1.GroupVersionForDiscovery{ - Version: "v2", + GroupVersion: "management.cattle.io/v2", + Version: "v1", }, - }, - } - resources := []*metav1.APIResourceList{ - { - GroupVersion: schema.GroupVersion{ - Group: "management.cattle.io", - Version: "v2", - }.String(), - APIResources: []metav1.APIResource{ + Versions: []metav1.GroupVersionForDiscovery{ { - Group: "management.cattle.io", - Kind: "GlobalRole", - Version: "v2", + GroupVersion: "management.cattle.io/v1", + Version: "v1", + }, + { + GroupVersion: "management.cattle.io/v2", + Version: "v2", }, }, }, { - GroupVersion: schema.GroupVersion{ - Group: "management.cattle.io", - Version: "v1", - }.String(), - APIResources: []metav1.APIResource{ + Name: "noversion.cattle.io", + Versions: []metav1.GroupVersionForDiscovery{ { - Group: "management.cattle.io", - Kind: "GlobalRole", - Version: "v2", + GroupVersion: "noversion.cattle.io/v1", + Version: "v1", + }, + { + GroupVersion: "noversion.cattle.io/v2", + Version: "v2", }, }, }, - nil, } return &fakeDiscovery{ - Groups: groups, - Resources: resources, - Document: document, + Groups: &metav1.APIGroupList{ + Groups: groups, + }, + Document: document, }, nil } type fakeDiscovery struct { - Groups []*metav1.APIGroup - Resources []*metav1.APIResourceList - Document *openapi_v2.Document - GroupResourcesErr error - DocumentErr error + Groups *metav1.APIGroupList + Document *openapi_v2.Document + GroupsErr error + DocumentErr error } -// ServerGroupsAndResources is the only method we actually need for the test - just returns what is on the struct -func (f *fakeDiscovery) ServerGroupsAndResources() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { - return f.Groups, f.Resources, f.GroupResourcesErr +// ServerGroups is the only method that needs to be mocked +func (f *fakeDiscovery) ServerGroups() (*metav1.APIGroupList, error) { + return f.Groups, f.GroupsErr } // The rest of these methods are just here to conform to discovery.DiscoveryInterface -func (f *fakeDiscovery) RESTClient() restclient.Interface { return nil } -func (f *fakeDiscovery) ServerGroups() (*metav1.APIGroupList, error) { return nil, nil } +func (f *fakeDiscovery) ServerGroupsAndResources() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { + return nil, nil, nil +} + +func (f *fakeDiscovery) RESTClient() restclient.Interface { return nil } func (f *fakeDiscovery) ServerResourcesForGroupVersion(groupVersion string) (*metav1.APIResourceList, error) { return nil, nil } diff --git a/pkg/schema/definitions/openapi_test.go b/pkg/schema/definitions/openapi_test.go index a0dad44a..1ab4fac3 100644 --- a/pkg/schema/definitions/openapi_test.go +++ b/pkg/schema/definitions/openapi_test.go @@ -82,6 +82,128 @@ definitions: - group: "management.cattle.io" version: "v2" kind: "GlobalRole" + io.cattle.noversion.v2.Resource: + description: "A No Version V2 resource is for a group with no preferred version" + 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" + newField: + description: "A new field not present in v1" + type: "string" + x-kubernetes-group-version-kind: + - group: "noversion.cattle.io" + version: "v2" + kind: "Resource" + io.cattle.noversion.v1.Resource: + description: "A No Version V1 resource is for a group with no preferred version" + 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: "noversion.cattle.io" + version: "v1" + kind: "Resource" + io.cattle.missinggroup.v2.Resource: + description: "A Missing Group V2 resource is for a group not listed by server groups" + 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" + newField: + description: "A new field not present in v1" + type: "string" + x-kubernetes-group-version-kind: + - group: "missinggroup.cattle.io" + version: "v2" + kind: "Resource" + io.cattle.missinggroup.v1.Resource: + description: "A Missing Group V1 resource is for a group not listed by server groups" + 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: "missinggroup.cattle.io" + version: "v1" + kind: "Resource" io.cattle.management.NotAKind: type: "string" description: "Some string which isn't a kind"