diff --git a/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/combined.go b/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/combined.go index 8a1832097e3..eb3c3763556 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/combined.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/combined.go @@ -38,7 +38,7 @@ type combinedSchemaResolver struct { // If the DefinitionsSchemaResolver knows the gvk, the DefinitionsSchemaResolver handles the resolution, // otherwise, the secondary does. func (r *combinedSchemaResolver) ResolveSchema(gvk schema.GroupVersionKind) (*spec.Schema, error) { - if _, ok := r.definitions.gvkToSchema[gvk]; ok { + if _, ok := r.definitions.gvkToRef[gvk]; ok { return r.definitions.ResolveSchema(gvk) } return r.secondary.ResolveSchema(gvk) diff --git a/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/definitions.go b/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/definitions.go index 9641a2421e3..12b353b0bcc 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/definitions.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/definitions.go @@ -29,8 +29,8 @@ import ( // DefinitionsSchemaResolver resolves the schema of a built-in type // by looking up the OpenAPI definitions. type DefinitionsSchemaResolver struct { - defs map[string]common.OpenAPIDefinition - gvkToSchema map[schema.GroupVersionKind]*spec.Schema + defs map[string]common.OpenAPIDefinition + gvkToRef map[schema.GroupVersionKind]string } // NewDefinitionsSchemaResolver creates a new DefinitionsSchemaResolver. @@ -38,31 +38,30 @@ type DefinitionsSchemaResolver struct { // getDefinitions = "k8s.io/kubernetes/pkg/generated/openapi".GetOpenAPIDefinitions // scheme = "k8s.io/client-go/kubernetes/scheme".Scheme func NewDefinitionsSchemaResolver(getDefinitions common.GetOpenAPIDefinitions, schemes ...*runtime.Scheme) *DefinitionsSchemaResolver { - gvkToSchema := make(map[schema.GroupVersionKind]*spec.Schema) + gvkToRef := make(map[schema.GroupVersionKind]string) namer := openapi.NewDefinitionNamer(schemes...) defs := getDefinitions(func(path string) spec.Ref { return spec.MustCreateRef(path) }) - for name, def := range defs { + for name := range defs { _, e := namer.GetDefinitionName(name) gvks := extensionsToGVKs(e) - s := def.Schema // map value not addressable, make copy for _, gvk := range gvks { - gvkToSchema[gvk] = &s + gvkToRef[gvk] = name } } return &DefinitionsSchemaResolver{ - gvkToSchema: gvkToSchema, - defs: defs, + gvkToRef: gvkToRef, + defs: defs, } } func (d *DefinitionsSchemaResolver) ResolveSchema(gvk schema.GroupVersionKind) (*spec.Schema, error) { - s, ok := d.gvkToSchema[gvk] + ref, ok := d.gvkToRef[gvk] if !ok { return nil, fmt.Errorf("cannot resolve %v: %w", gvk, ErrSchemaNotFound) } - s, err := populateRefs(func(ref string) (*spec.Schema, bool) { + s, err := PopulateRefs(func(ref string) (*spec.Schema, bool) { // find the schema by the ref string, and return a deep copy def, ok := d.defs[ref] if !ok { @@ -70,7 +69,7 @@ func (d *DefinitionsSchemaResolver) ResolveSchema(gvk schema.GroupVersionKind) ( } s := def.Schema return &s, true - }, s) + }, ref) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/discovery.go b/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/discovery.go index 53cbc7054b3..9c6cefce8de 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/discovery.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/discovery.go @@ -53,34 +53,34 @@ func (r *ClientDiscoveryResolver) ResolveSchema(gvk schema.GroupVersionKind) (*s if err != nil { return nil, err } - s, err := resolveType(resp, gvk) + ref, err := resolveRef(resp, gvk) if err != nil { return nil, err } - s, err = populateRefs(func(ref string) (*spec.Schema, bool) { + s, err := PopulateRefs(func(ref string) (*spec.Schema, bool) { s, ok := resp.Components.Schemas[strings.TrimPrefix(ref, refPrefix)] return s, ok - }, s) + }, ref) if err != nil { return nil, err } return s, nil } -func resolveType(resp *schemaResponse, gvk schema.GroupVersionKind) (*spec.Schema, error) { - for _, s := range resp.Components.Schemas { +func resolveRef(resp *schemaResponse, gvk schema.GroupVersionKind) (string, error) { + for ref, s := range resp.Components.Schemas { var gvks []schema.GroupVersionKind err := s.Extensions.GetObject(extGVK, &gvks) if err != nil { - return nil, err + return "", err } for _, g := range gvks { if g == gvk { - return s, nil + return ref, nil } } } - return nil, fmt.Errorf("cannot resolve group version kind %q: %w", gvk, ErrSchemaNotFound) + return "", fmt.Errorf("cannot resolve group version kind %q: %w", gvk, ErrSchemaNotFound) } func resourcePathFromGV(gv schema.GroupVersion) string { diff --git a/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/refs.go b/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/refs.go index 49321bab47d..56e2a4bbd3a 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/refs.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/openapi/resolver/refs.go @@ -19,19 +19,41 @@ package resolver import ( "fmt" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kube-openapi/pkg/validation/spec" ) -// populateRefs recursively replaces Refs in the schema with the referred one. +// PopulateRefs recursively replaces Refs in the schema with the referred one. // schemaOf is the callback to find the corresponding schema by the ref. // This function will not mutate the original schema. If the schema needs to be // mutated, a copy will be returned, otherwise it returns the original schema. -func populateRefs(schemaOf func(ref string) (*spec.Schema, bool), schema *spec.Schema) (*spec.Schema, error) { +func PopulateRefs(schemaOf func(ref string) (*spec.Schema, bool), rootRef string) (*spec.Schema, error) { + visitedRefs := sets.New[string]() + rootSchema, ok := schemaOf(rootRef) + visitedRefs.Insert(rootRef) + if !ok { + return nil, fmt.Errorf("internal error: cannot resolve Ref for root schema %q: %w", rootRef, ErrSchemaNotFound) + } + return populateRefs(schemaOf, visitedRefs, rootSchema) +} + +func populateRefs(schemaOf func(ref string) (*spec.Schema, bool), visited sets.Set[string], schema *spec.Schema) (*spec.Schema, error) { result := *schema changed := false ref, isRef := refOf(schema) if isRef { + if visited.Has(ref) { + return &spec.Schema{ + // for circular ref, return an empty object as placeholder + SchemaProps: spec.SchemaProps{Type: []string{"object"}}, + }, nil + } + visited.Insert(ref) + // restore visited state at the end of the recursion. + defer func() { + visited.Delete(ref) + }() // replace the whole schema with the referred one. resolved, ok := schemaOf(ref) if !ok { @@ -44,7 +66,7 @@ func populateRefs(schemaOf func(ref string) (*spec.Schema, bool), schema *spec.S props := make(map[string]spec.Schema, len(schema.Properties)) propsChanged := false for name, prop := range result.Properties { - populated, err := populateRefs(schemaOf, &prop) + populated, err := populateRefs(schemaOf, visited, &prop) if err != nil { return nil, err } @@ -58,7 +80,7 @@ func populateRefs(schemaOf func(ref string) (*spec.Schema, bool), schema *spec.S result.Properties = props } if result.AdditionalProperties != nil && result.AdditionalProperties.Schema != nil { - populated, err := populateRefs(schemaOf, result.AdditionalProperties.Schema) + populated, err := populateRefs(schemaOf, visited, result.AdditionalProperties.Schema) if err != nil { return nil, err } @@ -69,7 +91,7 @@ func populateRefs(schemaOf func(ref string) (*spec.Schema, bool), schema *spec.S } // schema is a list, populate its items if result.Items != nil && result.Items.Schema != nil { - populated, err := populateRefs(schemaOf, result.Items.Schema) + populated, err := populateRefs(schemaOf, visited, result.Items.Schema) if err != nil { return nil, err } diff --git a/test/integration/apiserver/cel/typeresolution_test.go b/test/integration/apiserver/cel/typeresolution_test.go index 869921c1226..b8685e4c856 100644 --- a/test/integration/apiserver/cel/typeresolution_test.go +++ b/test/integration/apiserver/cel/typeresolution_test.go @@ -36,6 +36,7 @@ import ( storagev1 "k8s.io/api/storage/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" extclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + apiextensionsscheme "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -79,7 +80,7 @@ func TestTypeResolver(t *testing.T) { } }(crd) discoveryResolver := &resolver.ClientDiscoveryResolver{Discovery: client.Discovery()} - definitionsResolver := resolver.NewDefinitionsSchemaResolver(openapi.GetOpenAPIDefinitions, k8sscheme.Scheme) + definitionsResolver := resolver.NewDefinitionsSchemaResolver(openapi.GetOpenAPIDefinitions, k8sscheme.Scheme, apiextensionsscheme.Scheme) // wait until the CRD schema is published at the OpenAPI v3 endpoint err = wait.PollImmediate(time.Second, time.Minute, func() (done bool, err error) { p, err := client.OpenAPIV3().Paths() @@ -330,7 +331,7 @@ func TestBuiltinResolution(t *testing.T) { }{ { name: "definitions", - resolver: resolver.NewDefinitionsSchemaResolver(openapi.GetOpenAPIDefinitions, k8sscheme.Scheme), + resolver: resolver.NewDefinitionsSchemaResolver(openapi.GetOpenAPIDefinitions, k8sscheme.Scheme, apiextensionsscheme.Scheme), scheme: buildTestScheme(), }, { @@ -354,6 +355,14 @@ func TestBuiltinResolution(t *testing.T) { if gvk.Version == "__internal" { continue } + // apiextensions.k8s.io/v1beta1 not published + if tc.name == "discovery" && gvk.Group == "apiextensions.k8s.io" && gvk.Version == "v1beta1" { + continue + } + // apiextensions.k8s.io ConversionReview not published + if tc.name == "discovery" && gvk.Group == "apiextensions.k8s.io" && gvk.Kind == "ConversionReview" { + continue + } _, err = tc.resolver.ResolveSchema(gvk) if err != nil { t.Errorf("resolver %q cannot resolve %v", tc.name, gvk) @@ -506,5 +515,6 @@ func buildTestScheme() *runtime.Scheme { _ = networkingv1.AddToScheme(scheme) _ = nodev1.AddToScheme(scheme) _ = storagev1.AddToScheme(scheme) + _ = apiextensionsscheme.AddToScheme(scheme) return scheme }