From 81bd325a72ec207668a1cce53dbcff2106288c9c Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Mon, 11 Apr 2022 13:19:54 -0400 Subject: [PATCH] Reuse structural schema and cel decls during CRD validation --- .../validation/cel_validation.go | 335 ++++++++++++++++++ .../apiextensions/validation/validation.go | 219 +++--------- .../validation/validation_test.go | 325 ++++++++++++++++- .../schema/cel/celcoststability_test.go | 2 +- .../pkg/apiserver/schema/cel/compilation.go | 13 +- .../apiserver/schema/cel/compilation_test.go | 7 +- .../pkg/apiserver/schema/cel/validation.go | 36 +- .../apiserver/schema/cel/validation_test.go | 213 ++++++++++- .../apiserver/schema/defaulting/validation.go | 6 +- .../apiserver/validation/validation_test.go | 21 +- .../pkg/registry/customresource/strategy.go | 5 +- .../forked/celopenapi/model/types.go | 20 +- .../forked/celopenapi/model/types_test.go | 2 +- 13 files changed, 976 insertions(+), 228 deletions(-) create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/cel_validation.go diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/cel_validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/cel_validation.go new file mode 100644 index 00000000000..a529e7abfb6 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/cel_validation.go @@ -0,0 +1,335 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "fmt" + "math" + "sort" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" + "k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +// unbounded uses nil to represent an unbounded cardinality value. +var unbounded *uint64 = nil + +// CELSchemaContext keeps track of data used by x-kubernetes-validations rules for a specific schema node. +type CELSchemaContext struct { + // withinValidationRuleScope is true if the schema at the current level or above have x-kubernetes-validations rules. typeInfo + // should only be populated for schema nodes where this is true. + withinValidationRuleScope bool + + // typeInfo is lazily loaded for schema nodes withinValidationRuleScope and may be + // populated one of two possible ways: + // 1. Using a typeInfoAccessor to access it from the parent's type info. This is a cheap operation and should be + // used when a schema at a higher level already has type info. + // 2. Using a converter to construct type info from the jsonSchema. This is an expensive operation. + typeInfo *CELTypeInfo + // typeInfoErr is any cached error resulting from an attempt to lazily load typeInfo. + typeInfoErr error + + // parent is the context of the parent schema node, or nil if this is the context for the root schema node. + parent *CELSchemaContext + // typeInfoAccessor provides a way to access the type info of this schema node from the parent CELSchemaContext. + // nil if not extraction is possible, or the parent is nil. + typeInfoAccessor typeInfoAccessor + + // jsonSchema is the schema for this CELSchemaContext node. It must be non-nil. + jsonSchema *apiextensions.JSONSchemaProps + // converter converts a JSONSchemaProps to CELTypeInfo. + // Tests that check how many conversions are performed during CRD validation wrap DefaultConverter + // with a converter that counts how many conversion operations. + converter converter + + // MaxCardinality represents a limit to the number of data elements that can exist for the current + // schema based on MaxProperties or MaxItems limits present on parent schemas, If all parent + // map and array schemas have MaxProperties or MaxItems limits declared MaxCardinality is + // an int pointer representing the product of these limits. If least one parent map or list schema + // does not have a MaxProperties or MaxItems limits set, the MaxCardinality is nil, indicating + // that the parent schemas offer no bound to the number of times a data element for the current + // schema can exist. + MaxCardinality *uint64 + // TotalCost accumulates the x-kubernetes-validators estimated rule cost total for an entire custom resource + // definition. A single TotalCost is allocated for each CustomResourceDefinition and passed through the stack as the + // CustomResourceDefinition's OpenAPIv3 schema is recursively validated. + TotalCost *TotalCost +} + +// TypeInfo returns the CELTypeInfo for this CELSchemaContext node. Returns nil, nil if this CELSchemaContext is nil, +// or if current level or above does not have x-kubernetes-validations rules. The returned type info is shared and +// should not be modified by the caller. +func (c *CELSchemaContext) TypeInfo() (*CELTypeInfo, error) { + if c == nil || !c.withinValidationRuleScope { + return nil, nil + } + if c.typeInfo != nil || c.typeInfoErr != nil { + return c.typeInfo, c.typeInfoErr // return already computed result if available + } + + // If able to get the type info from the parent's type info, prefer this approach + // since it is more efficient. + if c.parent != nil { + parentTypeInfo, parentErr := c.parent.TypeInfo() + if parentErr != nil { + c.typeInfoErr = parentErr + return nil, parentErr + } + if parentTypeInfo != nil && c.typeInfoAccessor != nil { + c.typeInfo = c.typeInfoAccessor.accessTypeInfo(parentTypeInfo) + if c.typeInfo != nil { + return c.typeInfo, nil + } + } + } + // If unable to get the type info from the parent, convert the jsonSchema to type info. + // This is expensive for large schemas. + c.typeInfo, c.typeInfoErr = c.converter(c.jsonSchema, c.parent == nil || c.jsonSchema.XEmbeddedResource) + return c.typeInfo, c.typeInfoErr +} + +// CELTypeInfo represents all the typeInfo needed by CEL to compile x-kubernetes-validations rules for a schema node. +type CELTypeInfo struct { + // Schema is a structural schema for this CELSchemaContext node. It must be non-nil. + Schema *structuralschema.Structural + // DeclType is a CEL declaration representation of Schema of this CELSchemaContext node. It must be non-nil. + DeclType *model.DeclType +} + +// converter converts from JSON schema to a structural schema and a CEL declType, or returns an error if the conversion +// fails. This should be defaultConverter except in tests where it is useful to wrap it with a converter that tracks +// how many conversions have been performed. +type converter func(schema *apiextensions.JSONSchemaProps, isRoot bool) (*CELTypeInfo, error) + +func defaultConverter(schema *apiextensions.JSONSchemaProps, isRoot bool) (*CELTypeInfo, error) { + structural, err := structuralschema.NewStructural(schema) + if err != nil { + return nil, err + } + declType := model.SchemaDeclType(structural, isRoot) + if declType == nil { + return nil, fmt.Errorf("unable to convert structural schema to CEL declarations") + } + return &CELTypeInfo{structural, declType}, nil +} + +// RootCELContext constructs CELSchemaContext for the given root schema. +func RootCELContext(schema *apiextensions.JSONSchemaProps) *CELSchemaContext { + rootCardinality := uint64(1) + r := &CELSchemaContext{ + jsonSchema: schema, + withinValidationRuleScope: len(schema.XValidations) > 0, + MaxCardinality: &rootCardinality, + TotalCost: &TotalCost{}, + converter: defaultConverter, + } + return r +} + +// ChildPropertyContext returns nil, nil if this CELSchemaContext is nil, otherwise constructs and returns a +// CELSchemaContext for propertyName. +func (c *CELSchemaContext) ChildPropertyContext(propSchema *apiextensions.JSONSchemaProps, propertyName string) *CELSchemaContext { + if c == nil { + return nil + } + return c.childContext(propSchema, propertyTypeInfoAccessor{propertyName: propertyName}) +} + +// ChildAdditionalPropertiesContext returns nil, nil if this CELSchemaContext is nil, otherwise it constructs and returns +// a CELSchemaContext for the properties of an object if this CELSchemaContext is an object. +// schema must be non-nil and have a non-nil schema.AdditionalProperties. +func (c *CELSchemaContext) ChildAdditionalPropertiesContext(propsSchema *apiextensions.JSONSchemaProps) *CELSchemaContext { + if c == nil { + return nil + } + return c.childContext(propsSchema, additionalItemsTypeInfoAccessor{}) +} + +// ChildItemsContext returns nil, nil if this CELSchemaContext is nil, otherwise it constructs and returns a CELSchemaContext +// for the items of an array if this CELSchemaContext is an array. +func (c *CELSchemaContext) ChildItemsContext(itemsSchema *apiextensions.JSONSchemaProps) *CELSchemaContext { + if c == nil { + return nil + } + return c.childContext(itemsSchema, itemsTypeInfoAccessor{}) +} + +// childContext returns nil, nil if this CELSchemaContext is nil, otherwise it constructs a new CELSchemaContext for the +// given child schema of the current schema context. +// accessor optionally provides a way to access CELTypeInfo of the child from the current schema context's CELTypeInfo. +// childContext returns a CELSchemaContext where the MaxCardinality is multiplied by the +// factor that the schema increases the cardinality of its children. If the CELSchemaContext's +// MaxCardinality is unbounded (nil) or the factor that the schema increase the cardinality +// is unbounded, the resulting CELSchemaContext's MaxCardinality is also unbounded. +func (c *CELSchemaContext) childContext(child *apiextensions.JSONSchemaProps, accessor typeInfoAccessor) *CELSchemaContext { + result := &CELSchemaContext{ + parent: c, + typeInfoAccessor: accessor, + withinValidationRuleScope: c.withinValidationRuleScope, + TotalCost: c.TotalCost, + MaxCardinality: unbounded, + converter: c.converter, + } + if child != nil { + result.jsonSchema = child + if len(child.XValidations) > 0 { + result.withinValidationRuleScope = true + } + } + if c.jsonSchema == nil { + // nil schemas can be passed since we call ChildSchemaContext + // before ValidateCustomResourceDefinitionOpenAPISchema performs its nil check + return result + } + if c.MaxCardinality == unbounded { + return result + } + maxElements := extractMaxElements(c.jsonSchema) + if maxElements == unbounded { + return result + } + result.MaxCardinality = uint64ptr(multiplyWithOverflowGuard(*c.MaxCardinality, *maxElements)) + return result +} + +type typeInfoAccessor interface { + // accessTypeInfo looks up type information for a child schema from a non-nil parentTypeInfo and returns it, + // or returns nil if the child schema information is not accessible. For example, a nil + // return value is expected when a property name is unescapable in CEL. + // The caller MUST ensure the provided parentTypeInfo is non-nil. + accessTypeInfo(parentTypeInfo *CELTypeInfo) *CELTypeInfo +} + +type propertyTypeInfoAccessor struct { + // propertyName is the property name in the parent schema that this schema is declared at. + propertyName string +} + +func (c propertyTypeInfoAccessor) accessTypeInfo(parentTypeInfo *CELTypeInfo) *CELTypeInfo { + if parentTypeInfo.Schema.Properties != nil { + propSchema := parentTypeInfo.Schema.Properties[c.propertyName] + if escapedPropName, ok := model.Escape(c.propertyName); ok { + if fieldDeclType, ok := parentTypeInfo.DeclType.Fields[escapedPropName]; ok { + return &CELTypeInfo{Schema: &propSchema, DeclType: fieldDeclType.Type} + } // else fields with unknown types are omitted from CEL validation entirely + } // fields with unescapable names are expected to be absent + } + return nil +} + +type itemsTypeInfoAccessor struct{} + +func (c itemsTypeInfoAccessor) accessTypeInfo(parentTypeInfo *CELTypeInfo) *CELTypeInfo { + if parentTypeInfo.Schema.Items != nil { + itemsSchema := parentTypeInfo.Schema.Items + itemsDeclType := parentTypeInfo.DeclType.ElemType + return &CELTypeInfo{Schema: itemsSchema, DeclType: itemsDeclType} + } + return nil +} + +type additionalItemsTypeInfoAccessor struct{} + +func (c additionalItemsTypeInfoAccessor) accessTypeInfo(parentTypeInfo *CELTypeInfo) *CELTypeInfo { + if parentTypeInfo.Schema.AdditionalProperties != nil { + propsSchema := parentTypeInfo.Schema.AdditionalProperties.Structural + valuesDeclType := parentTypeInfo.DeclType.ElemType + return &CELTypeInfo{Schema: propsSchema, DeclType: valuesDeclType} + } + return nil +} + +// TotalCost tracks the total cost of evaluating all the x-kubernetes-validations rules of a CustomResourceDefinition. +type TotalCost struct { + // Total accumulates the x-kubernetes-validations estimated rule cost total. + Total uint64 + // MostExpensive accumulates the top 4 most expensive rules contributing to the Total. Only rules + // that accumulate at least 1% of total cost limit are included. + MostExpensive []RuleCost +} + +// ObserveExpressionCost accumulates the cost of evaluating a -kubernetes-validations rule. +func (c *TotalCost) ObserveExpressionCost(path *field.Path, cost uint64) { + if math.MaxUint64-c.Total < cost { + c.Total = math.MaxUint64 + } else { + c.Total += cost + } + + if cost < StaticEstimatedCRDCostLimit/100 { // ignore rules that contribute < 1% of total cost limit + return + } + c.MostExpensive = append(c.MostExpensive, RuleCost{Path: path, Cost: cost}) + sort.Slice(c.MostExpensive, func(i, j int) bool { + // sort in descending order so the most expensive rule is first + return c.MostExpensive[i].Cost > c.MostExpensive[j].Cost + }) + if len(c.MostExpensive) > 4 { + c.MostExpensive = c.MostExpensive[:4] + } +} + +// RuleCost represents the cost of evaluating a single x-kubernetes-validations rule. +type RuleCost struct { + Path *field.Path + Cost uint64 +} + +// extractMaxElements returns the factor by which the schema increases the cardinality +// (number of possible data elements) of its children. If schema is a map and has +// MaxProperties or an array has MaxItems, the int pointer of the max value is returned. +// If schema is a map or array and does not have MaxProperties or MaxItems, +// unbounded (nil) is returned to indicate that there is no limit to the possible +// number of data elements imposed by the current schema. If the schema is an object, 1 is +// returned to indicate that there is no increase to the number of possible data elements +// for its children. Primitives do not have children, but 1 is returned for simplicity. +func extractMaxElements(schema *apiextensions.JSONSchemaProps) *uint64 { + switch schema.Type { + case "object": + if schema.AdditionalProperties != nil { + if schema.MaxProperties != nil { + maxProps := uint64(zeroIfNegative(*schema.MaxProperties)) + return &maxProps + } + return unbounded + } + // return 1 to indicate that all fields of an object exist at most one for + // each occurrence of the object they are fields of + return uint64ptr(1) + case "array": + if schema.MaxItems != nil { + maxItems := uint64(zeroIfNegative(*schema.MaxItems)) + return &maxItems + } + return unbounded + default: + return uint64ptr(1) + } +} + +func zeroIfNegative(v int64) int64 { + if v < 0 { + return 0 + } + return v +} + +func uint64ptr(i uint64) *uint64 { + return &i +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go index b97493d040b..1f51c641930 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go @@ -22,7 +22,6 @@ import ( "math" "reflect" "regexp" - "sort" "strings" "unicode" "unicode/utf8" @@ -723,17 +722,17 @@ func validateCustomResourceDefinitionValidation(ctx context.Context, customResou requireValidPropertyType: opts.requireValidPropertyType, } - costInfo := rootCostInfo() - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema, fldPath.Child("openAPIV3Schema"), openAPIV3Schema, true, &opts, costInfo)...) + celContext := RootCELContext(schema) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema, fldPath.Child("openAPIV3Schema"), openAPIV3Schema, true, &opts, celContext)...) - if costInfo.TotalCost != nil { - if costInfo.TotalCost.totalCost > StaticEstimatedCRDCostLimit { - for _, expensive := range costInfo.TotalCost.mostExpensive { + if celContext.TotalCost != nil { + if celContext.TotalCost.Total > StaticEstimatedCRDCostLimit { + for _, expensive := range celContext.TotalCost.MostExpensive { costErrorMsg := fmt.Sprintf("contributed to estimated rule cost total exceeding cost limit for entire OpenAPIv3 schema") - allErrs = append(allErrs, field.Forbidden(expensive.path, costErrorMsg)) + allErrs = append(allErrs, field.Forbidden(expensive.Path, costErrorMsg)) } - costErrorMsg := getCostErrorMessage("x-kubernetes-validations estimated rule cost total for entire OpenAPIv3 schema", costInfo.TotalCost.totalCost, StaticEstimatedCRDCostLimit) + costErrorMsg := getCostErrorMessage("x-kubernetes-validations estimated rule cost total for entire OpenAPIv3 schema", celContext.TotalCost.Total, StaticEstimatedCRDCostLimit) allErrs = append(allErrs, field.Forbidden(fldPath.Child("openAPIV3Schema"), costErrorMsg)) } } @@ -764,134 +763,10 @@ func validateCustomResourceDefinitionValidation(ctx context.Context, customResou return allErrs } -// unbounded uses nil to represent an unbounded cardinality value. -var unbounded *uint64 = nil - -type costInfo struct { - // MaxCardinality represents a limit to the number of data elements that can exist for the current - // schema based on MaxProperties or MaxItems limits present on parent schemas, If all parent - // map and array schemas have MaxProperties or MaxItems limits declared MaxCardinality is - // an int pointer representing the product of these limits. If least one parent map or list schema - // does not have a MaxProperties or MaxItems limits set, the MaxCardinality is nil, indicating - // that the parent schemas offer no bound to the number of times a data element for the current - // schema can exist. - MaxCardinality *uint64 - // TotalCost accumulates the x-kubernetes-validators estimated rule cost total for an entire custom resource - // definition. A single totalCost is allocated for each validation call and passed through the stack as the - // custom resource definition's OpenAPIv3 schema is recursively validated. - TotalCost *totalCost -} - -type totalCost struct { - // totalCost accumulates the x-kubernetes-validators estimated rule cost total. - totalCost uint64 - // mostExpensive accumulates the top 4 most expensive rules contributing to the totalCost. Only rules - // that accumulate at least 1% of total cost limit are included. - mostExpensive []ruleCost -} - -func (c *totalCost) observeExpressionCost(path *field.Path, cost uint64) { - if math.MaxUint64-c.totalCost < cost { - c.totalCost = math.MaxUint64 - } else { - c.totalCost += cost - } - - if cost < StaticEstimatedCRDCostLimit/100 { // ignore rules that contribute < 1% of total cost limit - return - } - c.mostExpensive = append(c.mostExpensive, ruleCost{path: path, cost: cost}) - sort.Slice(c.mostExpensive, func(i, j int) bool { - // sort in descending order so the most expensive rule is first - return c.mostExpensive[i].cost > c.mostExpensive[j].cost - }) - if len(c.mostExpensive) > 4 { - c.mostExpensive = c.mostExpensive[:4] - } -} - -type ruleCost struct { - path *field.Path - cost uint64 -} - -// MultiplyByElementCost returns a costInfo where the MaxCardinality is multiplied by the -// factor that the schema increases the cardinality of its children. If the costInfo's -// MaxCardinality is unbounded (nil) or the factor that the schema increase the cardinality -// is unbounded, the resulting costInfo's MaxCardinality is also unbounded. -func (c *costInfo) MultiplyByElementCost(schema *apiextensions.JSONSchemaProps) costInfo { - result := costInfo{TotalCost: c.TotalCost, MaxCardinality: unbounded} - if schema == nil { - // nil schemas can be passed since we call MultiplyByElementCost - // before ValidateCustomResourceDefinitionOpenAPISchema performs its nil check - return result - } - if c.MaxCardinality == unbounded { - return result - } - maxElements := extractMaxElements(schema) - if maxElements == unbounded { - return result - } - result.MaxCardinality = uint64ptr(multiplyWithOverflowGuard(*c.MaxCardinality, *maxElements)) - return result -} - -// extractMaxElements returns the factor by which the schema increases the cardinality -// (number of possible data elements) of its children. If schema is a map and has -// MaxProperties or an array has MaxItems, the int pointer of the max value is returned. -// If schema is a map or array and does not have MaxProperties or MaxItems, -// unbounded (nil) is returned to indicate that there is no limit to the possible -// number of data elements imposed by the current schema. If the schema is an object, 1 is -// returned to indicate that there is no increase to the number of possible data elements -// for its children. Primitives do not have children, but 1 is returned for simplicity. -func extractMaxElements(schema *apiextensions.JSONSchemaProps) *uint64 { - switch schema.Type { - case "object": - if schema.AdditionalProperties != nil { - if schema.MaxProperties != nil { - maxProps := uint64(zeroIfNegative(*schema.MaxProperties)) - return &maxProps - } - return unbounded - } - // return 1 to indicate that all fields of an object exist at most one for - // each occurrence of the object they are fields of - return uint64ptr(1) - case "array": - if schema.MaxItems != nil { - maxItems := uint64(zeroIfNegative(*schema.MaxItems)) - return &maxItems - } - return unbounded - default: - return uint64ptr(1) - } -} - -func zeroIfNegative(v int64) int64 { - if v < 0 { - return 0 - } - return v -} - -func uint64ptr(i uint64) *uint64 { - return &i -} - -func rootCostInfo() costInfo { - rootCardinality := uint64(1) - return costInfo{ - MaxCardinality: &rootCardinality, - TotalCost: &totalCost{}, - } -} - var metaFields = sets.NewString("metadata", "kind", "apiVersion") // ValidateCustomResourceDefinitionOpenAPISchema statically validates -func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSchemaProps, fldPath *field.Path, ssv specStandardValidator, isRoot bool, opts *validationOptions, nodeCostInfo costInfo) field.ErrorList { +func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSchemaProps, fldPath *field.Path, ssv specStandardValidator, isRoot bool, opts *validationOptions, celContext *CELSchemaContext) field.ErrorList { allErrs := field.ErrorList{} if schema == nil { @@ -925,7 +800,7 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch // we have to forbid defaults inside additionalProperties because pruning without actual value is ambiguous subSsv = ssv.withForbiddenDefaults("inside additionalProperties applying to object metadata") } - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.AdditionalProperties.Schema, fldPath.Child("additionalProperties"), subSsv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.AdditionalProperties.Schema, fldPath.Child("additionalProperties"), subSsv, false, opts, celContext.ChildAdditionalPropertiesContext(schema.AdditionalProperties.Schema))...) } if len(schema.Properties) != 0 { @@ -943,33 +818,38 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch subSsv = subSsv.withForbiddenDefaults(fmt.Sprintf("in top-level %s", property)) } } - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("properties").Key(property), subSsv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) + propertySchema := jsonSchema + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&propertySchema, fldPath.Child("properties").Key(property), subSsv, false, opts, celContext.ChildPropertyContext(&propertySchema, property))...) } } - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Not, fldPath.Child("not"), ssv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Not, fldPath.Child("not"), ssv, false, opts, nil)...) if len(schema.AllOf) != 0 { for i, jsonSchema := range schema.AllOf { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("allOf").Index(i), ssv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) + allOfSchema := jsonSchema + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&allOfSchema, fldPath.Child("allOf").Index(i), ssv, false, opts, nil)...) } } if len(schema.OneOf) != 0 { for i, jsonSchema := range schema.OneOf { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("oneOf").Index(i), ssv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) + oneOfSchema := jsonSchema + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&oneOfSchema, fldPath.Child("oneOf").Index(i), ssv, false, opts, nil)...) } } if len(schema.AnyOf) != 0 { for i, jsonSchema := range schema.AnyOf { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("anyOf").Index(i), ssv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) + anyOfSchema := jsonSchema + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&anyOfSchema, fldPath.Child("anyOf").Index(i), ssv, false, opts, nil)...) } } if len(schema.Definitions) != 0 { for definition, jsonSchema := range schema.Definitions { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("definitions").Key(definition), ssv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) + definitionSchema := jsonSchema + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&definitionSchema, fldPath.Child("definitions").Key(definition), ssv, false, opts, nil)...) } } @@ -983,17 +863,18 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch subSsv = subSsv.withForbidOldSelfValidations(fldPath) } - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), subSsv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), subSsv, false, opts, celContext.ChildItemsContext(schema.Items.Schema))...) if len(schema.Items.JSONSchemas) != 0 { for i, jsonSchema := range schema.Items.JSONSchemas { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("items").Index(i), subSsv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) + itemsSchema := jsonSchema + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&itemsSchema, fldPath.Child("items").Index(i), subSsv, false, opts, celContext.ChildItemsContext(&itemsSchema))...) } } } if schema.Dependencies != nil { for dependency, jsonSchemaPropsOrStringArray := range schema.Dependencies { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(jsonSchemaPropsOrStringArray.Schema, fldPath.Child("dependencies").Key(dependency), ssv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(jsonSchemaPropsOrStringArray.Schema, fldPath.Child("dependencies").Key(dependency), ssv, false, opts, nil)...) } } @@ -1095,31 +976,37 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch } } - structural, err := structuralschema.NewStructural(schema) - if err == nil { - compResults, err := cel.Compile(structural, isRoot, cel.PerCallLimit) + if celContext != nil { + typeInfo, err := celContext.TypeInfo() if err != nil { - allErrs = append(allErrs, field.InternalError(fldPath.Child("x-kubernetes-validations"), err)) + allErrs = append(allErrs, field.InternalError(fldPath.Child("x-kubernetes-validations"), fmt.Errorf("internal error: failed to construct type information for x-kubernetes-validations rules: %s", err))) + } else if typeInfo == nil { + allErrs = append(allErrs, field.InternalError(fldPath.Child("x-kubernetes-validations"), fmt.Errorf("internal error: failed to retrieve type information for x-kubernetes-validations"))) } else { - for i, cr := range compResults { - expressionCost := getExpressionCost(cr, nodeCostInfo) - if expressionCost > StaticEstimatedCostLimit { - costErrorMsg := getCostErrorMessage("estimated rule cost", expressionCost, StaticEstimatedCostLimit) - allErrs = append(allErrs, field.Forbidden(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), costErrorMsg)) - } - if nodeCostInfo.TotalCost != nil { - nodeCostInfo.TotalCost.observeExpressionCost(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), expressionCost) - } - if cr.Error != nil { - if cr.Error.Type == cel.ErrorTypeRequired { - allErrs = append(allErrs, field.Required(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), cr.Error.Detail)) - } else { - allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i], cr.Error.Detail)) + compResults, err := cel.Compile(typeInfo.Schema, typeInfo.DeclType, cel.PerCallLimit) + if err != nil { + allErrs = append(allErrs, field.InternalError(fldPath.Child("x-kubernetes-validations"), err)) + } else { + for i, cr := range compResults { + expressionCost := getExpressionCost(cr, celContext) + if expressionCost > StaticEstimatedCostLimit { + costErrorMsg := getCostErrorMessage("estimated rule cost", expressionCost, StaticEstimatedCostLimit) + allErrs = append(allErrs, field.Forbidden(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), costErrorMsg)) } - } - if cr.TransitionRule { - if uncorrelatablePath := ssv.forbidOldSelfValidations(); uncorrelatablePath != nil { - allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i].Rule, fmt.Sprintf("oldSelf cannot be used on the uncorrelatable portion of the schema within %v", uncorrelatablePath))) + if celContext.TotalCost != nil { + celContext.TotalCost.ObserveExpressionCost(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), expressionCost) + } + if cr.Error != nil { + if cr.Error.Type == cel.ErrorTypeRequired { + allErrs = append(allErrs, field.Required(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), cr.Error.Detail)) + } else { + allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i], cr.Error.Detail)) + } + } + if cr.TransitionRule { + if uncorrelatablePath := ssv.forbidOldSelfValidations(); uncorrelatablePath != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i].Rule, fmt.Sprintf("oldSelf cannot be used on the uncorrelatable portion of the schema within %v", uncorrelatablePath))) + } } } } @@ -1146,7 +1033,7 @@ func multiplyWithOverflowGuard(baseCost, cardinality uint64) uint64 { return baseCost * cardinality } -func getExpressionCost(cr cel.CompilationResult, cardinalityCost costInfo) uint64 { +func getExpressionCost(cr cel.CompilationResult, cardinalityCost *CELSchemaContext) uint64 { if cardinalityCost.MaxCardinality != unbounded { return multiplyWithOverflowGuard(cr.MaxCost, *cardinalityCost.MaxCardinality) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go index 56ead47c0b7..05f3d6a9b9f 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go @@ -8151,6 +8151,249 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { forbidden("spec.validation.openAPIV3Schema"), }, }, + { + name: "x-kubernetes-validations rule validated for escaped property name", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "f/2": { + Type: "string", + }, + }, + XValidations: apiextensions.ValidationRules{ + {Rule: "self.f__slash__2 == 1"}, // invalid comparison of string and int + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[0].rule"), + }, + }, + { + name: "x-kubernetes-validations rule validated under array items", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "a": { + Type: "array", + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + XValidations: apiextensions.ValidationRules{ + {Rule: "self == 1"}, // invalid comparison of string and int + }, + }, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.properties[a].items.x-kubernetes-validations[0].rule"), + }, + }, + { + name: "x-kubernetes-validations rule validated under array items, parent has rule", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "a": {Type: "array", + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + XValidations: apiextensions.ValidationRules{ + {Rule: "self == 1"}, // invalid comparison of string and int + }, + }, + }, + XValidations: apiextensions.ValidationRules{ + {Rule: "1 == 1"}, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.properties[a].items.x-kubernetes-validations[0].rule"), + }, + }, + { + name: "x-kubernetes-validations rule validated under additionalProperties", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "a": { + Type: "object", + AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + XValidations: apiextensions.ValidationRules{ + {Rule: "self == 1"}, // invalid comparison of string and int + }, + }, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.properties[a].additionalProperties.x-kubernetes-validations[0].rule"), + }, + }, + { + name: "x-kubernetes-validations rule validated under additionalProperties, parent has rule", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "a": { + Type: "object", + AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + XValidations: apiextensions.ValidationRules{ + {Rule: "self == 1"}, // invalid comparison of string and int + }, + }, + }, + XValidations: apiextensions.ValidationRules{ + {Rule: "1 == 1"}, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.properties[a].additionalProperties.x-kubernetes-validations[0].rule"), + }, + }, + { + name: "x-kubernetes-validations rule validated under unescaped property name", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "f": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + {Rule: "self == 1"}, // invalid comparison of string and int + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.properties[f].x-kubernetes-validations[0].rule"), + }, + }, + { + name: "x-kubernetes-validations rule validated under unescaped property name, parent has rule", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "f": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + {Rule: "self == 1"}, // invalid comparison of string and int + }, + }, + }, + XValidations: apiextensions.ValidationRules{ + {Rule: "1 == 1"}, + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.properties[f].x-kubernetes-validations[0].rule"), + }, + }, + { + name: "x-kubernetes-validations rule validated under escaped property name", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "f/2": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + {Rule: "self == 1"}, // invalid comparison of string and int + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.properties[f/2].x-kubernetes-validations[0].rule"), + }, + }, + { + name: "x-kubernetes-validations rule validated under escaped property name, parent has rule", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "f/2": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + {Rule: "self == 1"}, // invalid comparison of string and int + }, + }, + }, + XValidations: apiextensions.ValidationRules{ + {Rule: "1 == 1"}, + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.properties[f/2].x-kubernetes-validations[0].rule"), + }, + }, + { + name: "x-kubernetes-validations rule validated under unescapable property name", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "f@2": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + {Rule: "self == 1"}, // invalid comparison of string and int + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.properties[f@2].x-kubernetes-validations[0].rule"), + }, + }, + { + name: "x-kubernetes-validations rule validated under unescapable property name, parent has rule", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "f@2": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + {Rule: "self == 1"}, // invalid comparison of string and int + }, + }, + }, + XValidations: apiextensions.ValidationRules{ + {Rule: "1 == 1"}, + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.properties[f@2].x-kubernetes-validations[0].rule"), + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -8506,10 +8749,11 @@ func TestCostInfo(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - curCostInfo := rootCostInfo() // simulate the recursive validation calls - for _, schema := range tt.schema { - curCostInfo = curCostInfo.MultiplyByElementCost(schema) + schemas := append(tt.schema, &apiextensions.JSONSchemaProps{Type: "string"}) // append a leaf type + curCostInfo := RootCELContext(schemas[0]) + for i := 1; i < len(schemas); i++ { + curCostInfo = curCostInfo.childContext(schemas[i], nil) } if tt.expectedMaxCardinality == nil && curCostInfo.MaxCardinality == nil { // unbounded cardinality case, test ran correctly @@ -8524,6 +8768,63 @@ func TestCostInfo(t *testing.T) { } } +func TestCelContext(t *testing.T) { + tests := []struct { + name string + schema *apiextensions.JSONSchemaProps + }{ + { + name: "verify that schemas are converted only once and then reused", + schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: []apiextensions.ValidationRule{{Rule: "self.size() < 100"}}, + AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: []apiextensions.ValidationRule{{Rule: "has(self.field)"}}, + Properties: map[string]apiextensions.JSONSchemaProps{ + "field": { + XValidations: []apiextensions.ValidationRule{{Rule: "self.startsWith('abc')"}}, + Type: "string", + }, + }, + }, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // simulate the recursive validation calls + conversionCount := 0 + converter := func(schema *apiextensions.JSONSchemaProps, isRoot bool) (*CELTypeInfo, error) { + conversionCount++ + return defaultConverter(schema, isRoot) + } + celContext := RootCELContext(tt.schema) + celContext.converter = converter + opts := validationOptions{} + openAPIV3Schema := &specStandardValidatorV3{ + allowDefaults: opts.allowDefaults, + disallowDefaultsReason: opts.disallowDefaultsReason, + requireValidPropertyType: opts.requireValidPropertyType, + } + errors := ValidateCustomResourceDefinitionOpenAPISchema(tt.schema, field.NewPath("openAPIV3Schema"), openAPIV3Schema, true, &opts, celContext) + if len(errors) != 0 { + t.Errorf("Expected no validate errors but got %v", errors) + } + if conversionCount != 1 { + t.Errorf("Expected 1 conversion to be performed by cel context during schema traversal but observed %d conversions", conversionCount) + } + }) + } +} + func TestPerCRDEstimatedCost(t *testing.T) { tests := []struct { name string @@ -8582,20 +8883,20 @@ func TestPerCRDEstimatedCost(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - crdCost := rootCostInfo().TotalCost + crdCost := TotalCost{} for _, cost := range tt.costs { - crdCost.observeExpressionCost(nil, cost) + crdCost.ObserveExpressionCost(nil, cost) } - if len(crdCost.mostExpensive) != len(tt.expectedExpensive) { - t.Fatalf("expected %d largest costs but got %d: %v", len(tt.expectedExpensive), len(crdCost.mostExpensive), crdCost.mostExpensive) + if len(crdCost.MostExpensive) != len(tt.expectedExpensive) { + t.Fatalf("expected %d largest costs but got %d: %v", len(tt.expectedExpensive), len(crdCost.MostExpensive), crdCost.MostExpensive) } - for i, expensive := range crdCost.mostExpensive { - if tt.expectedExpensive[i] != expensive.cost { - t.Errorf("expected largest cost of %d at index %d but got %d", tt.expectedExpensive[i], i, expensive.cost) + for i, expensive := range crdCost.MostExpensive { + if tt.expectedExpensive[i] != expensive.Cost { + t.Errorf("expected largest cost of %d at index %d but got %d", tt.expectedExpensive[i], i, expensive.Cost) } } - if tt.expectedTotal != crdCost.totalCost { - t.Errorf("expected total cost of %d but got %d", tt.expectedTotal, crdCost.totalCost) + if tt.expectedTotal != crdCost.Total { + t.Errorf("expected total cost of %d but got %d", tt.expectedTotal, crdCost.Total) } }) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/celcoststability_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/celcoststability_test.go index 20a38d4171b..b0c59b25f5b 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/celcoststability_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/celcoststability_test.go @@ -1094,7 +1094,7 @@ func TestCelCostStability(t *testing.T) { t.Run(testName, func(t *testing.T) { t.Parallel() s := withRule(*tt.schema, validRule) - celValidator := NewValidator(&s, PerCallLimit) + celValidator := NewValidator(&s, true, PerCallLimit) if celValidator == nil { t.Fatal("expected non nil validator") } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation.go index 46a035efe7d..7f226d7379f 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation.go @@ -89,16 +89,16 @@ func getBaseEnv() (*cel.Env, error) { } // Compile compiles all the XValidations rules (without recursing into the schema) and returns a slice containing a -// CompilationResult for each ValidationRule, or an error. +// CompilationResult for each ValidationRule, or an error. declType is expected to be a CEL DeclType corresponding +// to the structural schema. // Each CompilationResult may contain: /// - non-nil Program, nil Error: The program was compiled successfully // - nil Program, non-nil Error: Compilation resulted in an error // - nil Program, nil Error: The provided rule was empty so compilation was not attempted // perCallLimit was added for testing purpose only. Callers should always use const PerCallLimit as input. -func Compile(s *schema.Structural, isResourceRoot bool, perCallLimit uint64) ([]CompilationResult, error) { +func Compile(s *schema.Structural, declType *celmodel.DeclType, perCallLimit uint64) ([]CompilationResult, error) { t := time.Now() defer metrics.Metrics.ObserveCompilation(time.Since(t)) - if len(s.Extensions.XValidations) == 0 { return nil, nil } @@ -113,7 +113,7 @@ func Compile(s *schema.Structural, isResourceRoot bool, perCallLimit uint64) ([] } reg := celmodel.NewRegistry(baseEnv) scopedTypeName := generateUniqueSelfTypeName() - rt, err := celmodel.NewRuleTypes(scopedTypeName, s, isResourceRoot, reg) + rt, err := celmodel.NewRuleTypes(scopedTypeName, declType, reg) if err != nil { return nil, err } @@ -126,11 +126,10 @@ func Compile(s *schema.Structural, isResourceRoot bool, perCallLimit uint64) ([] } root, ok = rt.FindDeclType(scopedTypeName) if !ok { - rootDecl := celmodel.SchemaDeclType(s, isResourceRoot) - if rootDecl == nil { + if declType == nil { return nil, fmt.Errorf("rule declared on schema that does not support validation rules type: '%s' x-kubernetes-preserve-unknown-fields: '%t'", s.Type, s.XPreserveUnknownFields) } - root = rootDecl.MaybeAssignTypeName(scopedTypeName) + root = declType.MaybeAssignTypeName(scopedTypeName) } propDecls = append(propDecls, cel.Variable(ScopedVarName, root.CelType())) propDecls = append(propDecls, cel.Variable(OldScopedVarName, root.CelType())) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation_test.go index a1c0b0a67f5..847fec2e698 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation_test.go @@ -24,6 +24,7 @@ import ( apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" + "k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model" ) const ( @@ -642,7 +643,7 @@ func TestCelCompilation(t *testing.T) { for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { - compilationResults, err := Compile(&tt.input, false, PerCallLimit) + compilationResults, err := Compile(&tt.input, model.SchemaDeclType(&tt.input, false), PerCallLimit) if err != nil { t.Errorf("Expected no error, but got: %v", err) } @@ -1078,7 +1079,7 @@ func genMapWithCustomItemRule(item *schema.Structural, rule string) func(maxProp // if expectedCostExceedsLimit is non-zero. Typically, only expectedCost or expectedCostExceedsLimit is non-zero, not both. func schemaChecker(schema *schema.Structural, expectedCost uint64, expectedCostExceedsLimit uint64, t *testing.T) func(t *testing.T) { return func(t *testing.T) { - compilationResults, err := Compile(schema, false, PerCallLimit) + compilationResults, err := Compile(schema, model.SchemaDeclType(schema, false), PerCallLimit) if err != nil { t.Errorf("Expected no error, got: %v", err) } @@ -1614,7 +1615,7 @@ func BenchmarkCompile(b *testing.B) { b.ReportAllocs() b.ResetTimer() for i := 0; i < b.N; i++ { - _, err := Compile(s, false, uint64(math.MaxInt64)) + _, err := Compile(s, model.SchemaDeclType(s, false), uint64(math.MaxInt64)) if err != nil { b.Fatal(err) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go index 9e57daafdb3..32f0467e483 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go @@ -65,27 +65,47 @@ type Validator struct { // Returns nil only if there no validator rules in the Structural schema. May return a validator containing // only errors. // Adding perCallLimit as input arg for testing purpose only. Callers should always use const PerCallLimit as input -func NewValidator(s *schema.Structural, perCallLimit uint64) *Validator { - return validator(s, true, perCallLimit) +func NewValidator(s *schema.Structural, isResourceRoot bool, perCallLimit uint64) *Validator { + return validator(s, true, model.SchemaDeclType(s, isResourceRoot), perCallLimit) } -func validator(s *schema.Structural, isResourceRoot bool, perCallLimit uint64) *Validator { - compiledRules, err := Compile(s, isResourceRoot, perCallLimit) +// validator creates a Validator for all x-kubernetes-validations at the level of the provided schema and lower and +// returns the Validator if any x-kubernetes-validations exist in the schema, or nil if no x-kubernetes-validations +// exist. declType is expected to be a CEL DeclType corresponding to the structural schema. +func validator(s *schema.Structural, isResourceRoot bool, declType *model.DeclType, perCallLimit uint64) *Validator { + compiledRules, err := Compile(s, declType, perCallLimit) var itemsValidator, additionalPropertiesValidator *Validator var propertiesValidators map[string]Validator if s.Items != nil { - itemsValidator = validator(s.Items, s.Items.XEmbeddedResource, perCallLimit) + itemsValidator = validator(s.Items, s.Items.XEmbeddedResource, declType.ElemType, perCallLimit) } if len(s.Properties) > 0 { propertiesValidators = make(map[string]Validator, len(s.Properties)) - for k, prop := range s.Properties { - if p := validator(&prop, prop.XEmbeddedResource, perCallLimit); p != nil { + for k, p := range s.Properties { + prop := p + var fieldType *model.DeclType + if escapedPropName, ok := model.Escape(k); ok { + if f, ok := declType.Fields[escapedPropName]; ok { + fieldType = f.Type + } else { + // fields with unknown types are omitted from CEL validation entirely + continue + } + } else { + // field may be absent from declType if the property name is unescapable, in which case we should convert + // the field value type to a DeclType. + fieldType = model.SchemaDeclType(&prop, prop.XEmbeddedResource) + if fieldType == nil { + continue + } + } + if p := validator(&prop, prop.XEmbeddedResource, fieldType, perCallLimit); p != nil { propertiesValidators[k] = *p } } } if s.AdditionalProperties != nil && s.AdditionalProperties.Structural != nil { - additionalPropertiesValidator = validator(s.AdditionalProperties.Structural, s.AdditionalProperties.Structural.XEmbeddedResource, perCallLimit) + additionalPropertiesValidator = validator(s.AdditionalProperties.Structural, s.AdditionalProperties.Structural.XEmbeddedResource, declType.ElemType, perCallLimit) } if len(compiledRules) > 0 || err != nil || itemsValidator != nil || additionalPropertiesValidator != nil || len(propertiesValidators) > 0 { var activationFactory func(*schema.Structural, interface{}, interface{}) interpreter.Activation = validationActivationWithoutOldSelf diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go index 06f52a7350c..7086e71fd58 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go @@ -24,10 +24,12 @@ import ( "testing" "time" + "k8s.io/kube-openapi/pkg/validation/strfmt" + apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" + "k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model" "k8s.io/apimachinery/pkg/util/validation/field" - "k8s.io/kube-openapi/pkg/validation/strfmt" ) // TestValidationExpressions tests CEL integration with custom resource values and OpenAPIv3. @@ -1767,7 +1769,7 @@ func TestValidationExpressions(t *testing.T) { t.Run(testName, func(t *testing.T) { t.Parallel() s := withRule(*tt.schema, validRule) - celValidator := validator(&s, tt.isRoot, PerCallLimit) + celValidator := validator(&s, tt.isRoot, model.SchemaDeclType(&s, tt.isRoot), PerCallLimit) if celValidator == nil { t.Fatal("expected non nil validator") } @@ -1791,7 +1793,7 @@ func TestValidationExpressions(t *testing.T) { } t.Run(testName, func(t *testing.T) { s := withRule(*tt.schema, rule) - celValidator := NewValidator(&s, PerCallLimit) + celValidator := NewValidator(&s, true, PerCallLimit) if celValidator == nil { t.Fatal("expected non nil validator") } @@ -1810,6 +1812,181 @@ func TestValidationExpressions(t *testing.T) { } } +// TestValidationExpressionsInSchema tests CEL integration with custom resource values and OpenAPIv3 for cases +// where the validation rules are defined at any level within the schema. +func TestValidationExpressionsAtSchemaLevels(t *testing.T) { + tests := []struct { + name string + schema *schema.Structural + obj interface{} + oldObj interface{} + errors []string // strings that error message must contain + }{ + {name: "invalid rule under array items", + obj: map[string]interface{}{ + "f": []interface{}{1}, + }, + schema: objectTypePtr(map[string]schema.Structural{ + "f": listType(cloneWithRule(&integerType, "self == 'abc'")), + }), + errors: []string{"found no matching overload for '_==_' applied to '(int, string)"}, + }, + {name: "invalid rule under array items, parent has rule", + obj: map[string]interface{}{ + "f": []interface{}{1}, + }, + schema: objectTypePtr(map[string]schema.Structural{ + "f": withRule(listType(cloneWithRule(&integerType, "self == 'abc'")), "1 == 1"), + }), + errors: []string{"found no matching overload for '_==_' applied to '(int, string)"}, + }, + {name: "invalid rule under additionalProperties", + obj: map[string]interface{}{ + "f": map[string]interface{}{"k": 1}, + }, + schema: objectTypePtr(map[string]schema.Structural{ + "f": mapType(cloneWithRule(&integerType, "self == 'abc'")), + }), + errors: []string{"found no matching overload for '_==_' applied to '(int, string)"}, + }, + {name: "invalid rule under additionalProperties, parent has rule", + obj: map[string]interface{}{ + "f": map[string]interface{}{"k": 1}, + }, + schema: objectTypePtr(map[string]schema.Structural{ + "f": withRule(mapType(cloneWithRule(&integerType, "self == 'abc'")), "1 == 1"), + }), + errors: []string{"found no matching overload for '_==_' applied to '(int, string)"}, + }, + {name: "invalid rule under unescaped field name", + obj: map[string]interface{}{ + "f": map[string]interface{}{ + "m": 1, + }, + }, + schema: objectTypePtr(map[string]schema.Structural{ + "f": withRule(objectType(map[string]schema.Structural{"m": integerType}), "self.m == 'abc'"), + }), + errors: []string{"found no matching overload for '_==_' applied to '(int, string)"}, + }, + {name: "invalid rule under unescaped field name, parent has rule", + obj: map[string]interface{}{ + "f": map[string]interface{}{ + "m": 1, + }, + }, + schema: withRulePtr(objectTypePtr(map[string]schema.Structural{ + "f": withRule(objectType(map[string]schema.Structural{"m": integerType}), "self.m == 'abc'"), + }), "1 == 1"), + errors: []string{"found no matching overload for '_==_' applied to '(int, string)"}, + }, + // check that escaped field names do not impact CEL rule validation + {name: "invalid rule under escaped field name", + obj: map[string]interface{}{ + "f/2": map[string]interface{}{ + "m": 1, + }, + }, + schema: objectTypePtr(map[string]schema.Structural{ + "f/2": withRule(objectType(map[string]schema.Structural{"m": integerType}), "self.m == 'abc'"), + }), + errors: []string{"found no matching overload for '_==_' applied to '(int, string)"}, + }, + {name: "invalid rule under escaped field name, parent has rule", + obj: map[string]interface{}{ + "f/2": map[string]interface{}{ + "m": 1, + }, + }, + schema: withRulePtr(objectTypePtr(map[string]schema.Structural{ + "f/2": withRule(objectType(map[string]schema.Structural{"m": integerType}), "self.m == 'abc'"), + }), "1 == 1"), + errors: []string{"found no matching overload for '_==_' applied to '(int, string)"}, + }, + {name: "failing rule under escaped field name", + obj: map[string]interface{}{ + "f/2": map[string]interface{}{ + "m": 1, + }, + }, + schema: objectTypePtr(map[string]schema.Structural{ + "f/2": withRule(objectType(map[string]schema.Structural{"m": integerType}), "self.m == 2"), + }), + errors: []string{"Invalid value: \"object\": failed rule: self.m == 2"}, + }, + // unescapable field names that are not accessed by the CEL rule are allowed and should not impact CEL rule validation + {name: "invalid rule under unescapable field name", + obj: map[string]interface{}{ + "a@b": map[string]interface{}{ + "m": 1, + }, + }, + schema: objectTypePtr(map[string]schema.Structural{ + "a@b": withRule(objectType(map[string]schema.Structural{"m": integerType}), "self.m == 'abc'"), + }), + errors: []string{"found no matching overload for '_==_' applied to '(int, string)"}, + }, + {name: "invalid rule under unescapable field name, parent has rule", + obj: map[string]interface{}{ + "f@2": map[string]interface{}{ + "m": 1, + }, + }, + schema: withRulePtr(objectTypePtr(map[string]schema.Structural{ + "f@2": withRule(objectType(map[string]schema.Structural{"m": integerType}), "self.m == 'abc'"), + }), "1 == 1"), + errors: []string{"found no matching overload for '_==_' applied to '(int, string)"}, + }, + {name: "failing rule under unescapable field name", + obj: map[string]interface{}{ + "a@b": map[string]interface{}{ + "m": 1, + }, + }, + schema: objectTypePtr(map[string]schema.Structural{ + "a@b": withRule(objectType(map[string]schema.Structural{"m": integerType}), "self.m == 2"), + }), + errors: []string{"Invalid value: \"object\": failed rule: self.m == 2"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctx := context.TODO() + celValidator := validator(tt.schema, true, model.SchemaDeclType(tt.schema, true), PerCallLimit) + if celValidator == nil { + t.Fatal("expected non nil validator") + } + errs, _ := celValidator.Validate(ctx, field.NewPath("root"), tt.schema, tt.obj, tt.oldObj, math.MaxInt) + unmatched := map[string]struct{}{} + for _, e := range tt.errors { + unmatched[e] = struct{}{} + } + for _, err := range errs { + if err.Type != field.ErrorTypeInvalid { + t.Errorf("expected only ErrorTypeInvalid errors, but got: %v", err) + continue + } + matched := false + for expected := range unmatched { + if strings.Contains(err.Error(), expected) { + delete(unmatched, expected) + matched = true + break + } + } + if !matched { + t.Errorf("expected error to contain one of %v, but got: %v", unmatched, err) + } + } + if len(unmatched) > 0 { + t.Errorf("expected errors %v", unmatched) + } + }) + } +} + func TestCELValidationLimit(t *testing.T) { tests := []struct { name string @@ -1833,7 +2010,7 @@ func TestCELValidationLimit(t *testing.T) { t.Run(validRule, func(t *testing.T) { t.Parallel() s := withRule(*tt.schema, validRule) - celValidator := validator(&s, false, PerCallLimit) + celValidator := validator(&s, false, model.SchemaDeclType(&s, false), PerCallLimit) // test with cost budget exceeded errs, _ := celValidator.Validate(ctx, field.NewPath("root"), &s, tt.obj, nil, 0) @@ -1854,7 +2031,7 @@ func TestCELValidationLimit(t *testing.T) { // test with PerCallLimit exceeded found = false - celValidator = NewValidator(&s, 0) + celValidator = NewValidator(&s, true, 0) if celValidator == nil { t.Fatal("expected non nil validator") } @@ -1901,7 +2078,7 @@ func TestCELValidationContextCancellation(t *testing.T) { t.Run(tt.name, func(t *testing.T) { ctx := context.TODO() s := withRule(*tt.schema, tt.rule) - celValidator := NewValidator(&s, PerCallLimit) + celValidator := NewValidator(&s, true, PerCallLimit) if celValidator == nil { t.Fatal("expected non nil validator") } @@ -1967,7 +2144,7 @@ func TestCELMaxRecursionDepth(t *testing.T) { t.Run(testName, func(t *testing.T) { t.Parallel() s := withRule(*tt.schema, validRule) - celValidator := validator(&s, tt.isRoot, PerCallLimit) + celValidator := validator(&s, tt.isRoot, model.SchemaDeclType(&s, tt.isRoot), PerCallLimit) if celValidator == nil { t.Fatal("expected non nil validator") } @@ -1991,7 +2168,7 @@ func TestCELMaxRecursionDepth(t *testing.T) { } t.Run(testName, func(t *testing.T) { s := withRule(*tt.schema, rule) - celValidator := NewValidator(&s, PerCallLimit) + celValidator := NewValidator(&s, true, PerCallLimit) if celValidator == nil { t.Fatal("expected non nil validator") } @@ -2036,7 +2213,7 @@ func BenchmarkCELValidationWithContext(b *testing.B) { b.Run(tt.name, func(b *testing.B) { ctx := context.TODO() s := withRule(*tt.schema, tt.rule) - celValidator := NewValidator(&s, PerCallLimit) + celValidator := NewValidator(&s, true, PerCallLimit) if celValidator == nil { b.Fatal("expected non nil validator") } @@ -2076,7 +2253,7 @@ func BenchmarkCELValidationWithCancelledContext(b *testing.B) { b.Run(tt.name, func(b *testing.B) { ctx := context.TODO() s := withRule(*tt.schema, tt.rule) - celValidator := NewValidator(&s, PerCallLimit) + celValidator := NewValidator(&s, true, PerCallLimit) if celValidator == nil { b.Fatal("expected non nil validator") } @@ -2130,7 +2307,7 @@ func BenchmarkCELValidationWithAndWithoutOldSelfReference(b *testing.B) { }, }, } - validator := NewValidator(s, PerCallLimit) + validator := NewValidator(s, true, PerCallLimit) if validator == nil { b.Fatal("expected non nil validator") } @@ -2284,6 +2461,20 @@ func withRule(s schema.Structural, rule string) schema.Structural { return s } +func withRulePtr(s *schema.Structural, rule string) *schema.Structural { + s.Extensions.XValidations = apiextensions.ValidationRules{ + { + Rule: rule, + }, + } + return s +} + +func cloneWithRule(s *schema.Structural, rule string) *schema.Structural { + s = s.DeepCopy() + return withRulePtr(s, rule) +} + func withMaxLength(s schema.Structural, maxLength *int64) schema.Structural { if s.ValueValidation == nil { s.ValueValidation = &schema.ValueValidation{} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/validation.go index 40e1218c2d6..f89500b60bd 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/validation.go @@ -70,6 +70,8 @@ func validate(ctx context.Context, pth *field.Path, s *structuralschema.Structur rootSchema = s } + isResourceRoot := s == rootSchema + if s.Default.Object != nil { validator := kubeopenapivalidate.NewSchemaValidator(s.ToKubeOpenAPI(), nil, "", strfmt.Default) @@ -89,7 +91,7 @@ func validate(ctx context.Context, pth *field.Path, s *structuralschema.Structur allErrs = append(allErrs, field.Invalid(pth.Child("default"), s.Default.Object, fmt.Sprintf("must result in valid metadata: %v", errs.ToAggregate()))) } else if errs := apiservervalidation.ValidateCustomResource(pth.Child("default"), s.Default.Object, validator); len(errs) > 0 { allErrs = append(allErrs, errs...) - } else if celValidator := cel.NewValidator(s, cel.PerCallLimit); celValidator != nil { + } else if celValidator := cel.NewValidator(s, isResourceRoot, cel.PerCallLimit); celValidator != nil { celErrs, rmCost := celValidator.Validate(ctx, pth.Child("default"), s, s.Default.Object, s.Default.Object, remainingCost) remainingCost = rmCost allErrs = append(allErrs, celErrs...) @@ -114,7 +116,7 @@ func validate(ctx context.Context, pth *field.Path, s *structuralschema.Structur allErrs = append(allErrs, errs...) } else if errs := apiservervalidation.ValidateCustomResource(pth.Child("default"), s.Default.Object, validator); len(errs) > 0 { allErrs = append(allErrs, errs...) - } else if celValidator := cel.NewValidator(s, cel.PerCallLimit); celValidator != nil { + } else if celValidator := cel.NewValidator(s, isResourceRoot, cel.PerCallLimit); celValidator != nil { celErrs, rmCost := celValidator.Validate(ctx, pth.Child("default"), s, s.Default.Object, s.Default.Object, remainingCost) remainingCost = rmCost allErrs = append(allErrs, celErrs...) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go index d6091bbdcd4..49b1cab79d2 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go @@ -29,6 +29,8 @@ import ( utilpointer "k8s.io/utils/pointer" kjson "sigs.k8s.io/json" + kubeopenapispec "k8s.io/kube-openapi/pkg/validation/spec" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apiextensionsfuzzer "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -40,7 +42,6 @@ import ( "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/util/json" "k8s.io/apimachinery/pkg/util/sets" - kubeopenapispec "k8s.io/kube-openapi/pkg/validation/spec" ) // TestRoundTrip checks the conversion to go-openapi types. @@ -163,6 +164,7 @@ func TestValidateCustomResource(t *testing.T) { }{ {name: "!nullable", schema: apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "field": { Type: "object", @@ -185,6 +187,7 @@ func TestValidateCustomResource(t *testing.T) { }, {name: "nullable", schema: apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "field": { Type: "object", @@ -207,6 +210,7 @@ func TestValidateCustomResource(t *testing.T) { }, {name: "nullable and no type", schema: apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "field": { Nullable: true, @@ -226,6 +230,7 @@ func TestValidateCustomResource(t *testing.T) { }, {name: "x-kubernetes-int-or-string", schema: apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "field": { XIntOrString: true, @@ -247,6 +252,7 @@ func TestValidateCustomResource(t *testing.T) { }, {name: "nullable and x-kubernetes-int-or-string", schema: apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "field": { Nullable: true, @@ -269,6 +275,7 @@ func TestValidateCustomResource(t *testing.T) { }, {name: "nullable, x-kubernetes-int-or-string and user-provided anyOf", schema: apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "field": { Nullable: true, @@ -311,6 +318,7 @@ func TestValidateCustomResource(t *testing.T) { }, {name: "nullable, x-kubernetes-int-or-string and user-provider allOf", schema: apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "field": { Nullable: true, @@ -361,6 +369,7 @@ func TestValidateCustomResource(t *testing.T) { }, {name: "invalid regex", schema: apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "field": { Type: "string", @@ -374,6 +383,7 @@ func TestValidateCustomResource(t *testing.T) { }, {name: "required field", schema: apiextensions.JSONSchemaProps{ + Type: "object", Required: []string{"field"}, Properties: map[string]apiextensions.JSONSchemaProps{ "field": { @@ -392,6 +402,7 @@ func TestValidateCustomResource(t *testing.T) { }, {name: "enum", schema: apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "field": { Type: "object", @@ -424,6 +435,7 @@ func TestValidateCustomResource(t *testing.T) { }, {name: "immutability transition rule", schema: apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "field": { Type: "string", @@ -453,6 +465,7 @@ func TestValidateCustomResource(t *testing.T) { {name: "correlatable transition rule", // Ensures a transition rule under a "listMap" is supported. schema: apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "field": { Type: "array", @@ -503,6 +516,7 @@ func TestValidateCustomResource(t *testing.T) { // does NOT use oldSelf (is not a transition rule), still behaves // as expected under a non-correlatable field. schema: apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "field": { Type: "array", @@ -537,6 +551,7 @@ func TestValidateCustomResource(t *testing.T) { }, {name: "maxProperties", schema: apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "fieldX": { Type: "object", @@ -552,6 +567,7 @@ func TestValidateCustomResource(t *testing.T) { }, {name: "maxItems", schema: apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "fieldX": { Type: "array", @@ -567,6 +583,7 @@ func TestValidateCustomResource(t *testing.T) { }, {name: "maxLength", schema: apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "fieldX": { Type: "string", @@ -591,7 +608,7 @@ func TestValidateCustomResource(t *testing.T) { if err != nil { t.Fatal(err) } - celValidator := cel.NewValidator(structural, cel.PerCallLimit) + celValidator := cel.NewValidator(structural, false, cel.PerCallLimit) for i, obj := range tt.objects { var oldObject interface{} if len(tt.oldObjects) == len(tt.objects) { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go index 3f298d5c3a4..926806af7fc 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go @@ -19,6 +19,8 @@ package customresource import ( "context" + "k8s.io/kube-openapi/pkg/validation/validate" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel" @@ -37,7 +39,6 @@ import ( apiserverstorage "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/names" utilfeature "k8s.io/apiserver/pkg/util/feature" - "k8s.io/kube-openapi/pkg/validation/validate" "sigs.k8s.io/structured-merge-diff/v4/fieldpath" ) @@ -60,7 +61,7 @@ func NewStrategy(typer runtime.ObjectTyper, namespaceScoped bool, kind schema.Gr celValidators := map[string]*cel.Validator{} if utilfeature.DefaultFeatureGate.Enabled(features.CustomResourceValidationExpressions) { for name, s := range structuralSchemas { - v := cel.NewValidator(s, cel.PerCallLimit) // CEL programs are compiled and cached here + v := cel.NewValidator(s, true, cel.PerCallLimit) // CEL programs are compiled and cached here if v != nil { celValidators[name] = v } diff --git a/staging/src/k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model/types.go b/staging/src/k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model/types.go index f5eb7da976a..621b77f41da 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model/types.go +++ b/staging/src/k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model/types.go @@ -26,8 +26,6 @@ import ( exprpb "google.golang.org/genproto/googleapis/api/expr/v1alpha1" "google.golang.org/protobuf/proto" - - "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" ) const ( @@ -81,7 +79,8 @@ func newSimpleType(name string, celType *cel.Type, zeroVal ref.Val) *DeclType { type DeclType struct { fmt.Stringer - name string + name string + // Fields contains a map of escaped CEL identifier field names to field declarations. Fields map[string]*DeclField KeyType *DeclType ElemType *DeclType @@ -295,14 +294,13 @@ func (f *DeclField) EnumValues() []ref.Val { // NewRuleTypes returns an Open API Schema-based type-system which is CEL compatible. func NewRuleTypes(kind string, - schema *schema.Structural, - isResourceRoot bool, + declType *DeclType, res Resolver) (*RuleTypes, error) { // Note, if the schema indicates that it's actually based on another proto // then prefer the proto definition. For expressions in the proto, a new field // annotation will be needed to indicate the expected environment and type of // the expression. - schemaTypes, err := newSchemaTypeProvider(kind, schema, isResourceRoot) + schemaTypes, err := newSchemaTypeProvider(kind, declType) if err != nil { return nil, err } @@ -310,7 +308,6 @@ func NewRuleTypes(kind string, return nil, nil } return &RuleTypes{ - Schema: schema, ruleSchemaDeclTypes: schemaTypes, resolver: res, }, nil @@ -320,7 +317,6 @@ func NewRuleTypes(kind string, // type-system. type RuleTypes struct { ref.TypeProvider - Schema *schema.Structural ruleSchemaDeclTypes *schemaTypeProvider typeAdapter ref.TypeAdapter resolver Resolver @@ -345,7 +341,6 @@ func (rt *RuleTypes) EnvOptions(tp ref.TypeProvider) ([]cel.EnvOption, error) { rtWithTypes := &RuleTypes{ TypeProvider: tp, typeAdapter: ta, - Schema: rt.Schema, ruleSchemaDeclTypes: rt.ruleSchemaDeclTypes, resolver: rt.resolver, } @@ -492,12 +487,11 @@ func (rt *RuleTypes) convertToCustomType(dyn *DynValue, declType *DeclType) *Dyn } } -func newSchemaTypeProvider(kind string, schema *schema.Structural, isResourceRoot bool) (*schemaTypeProvider, error) { - delType := SchemaDeclType(schema, isResourceRoot) - if delType == nil { +func newSchemaTypeProvider(kind string, declType *DeclType) (*schemaTypeProvider, error) { + if declType == nil { return nil, nil } - root := delType.MaybeAssignTypeName(kind) + root := declType.MaybeAssignTypeName(kind) types := FieldTypeMap(kind, root) return &schemaTypeProvider{ root: root, diff --git a/staging/src/k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model/types_test.go b/staging/src/k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model/types_test.go index 207b4befa3d..5edb5e082b6 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model/types_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model/types_test.go @@ -75,7 +75,7 @@ func TestTypes_MapType(t *testing.T) { func TestTypes_RuleTypesFieldMapping(t *testing.T) { stdEnv, _ := cel.NewEnv() reg := NewRegistry(stdEnv) - rt, err := NewRuleTypes("CustomObject", testSchema(), true, reg) + rt, err := NewRuleTypes("CustomObject", SchemaDeclType(testSchema(), true), reg) if err != nil { t.Fatal(err) }