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 d584770ec1c..94e74f869fd 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 @@ -19,6 +19,7 @@ package validation import ( "context" "fmt" + "math" "reflect" "regexp" "strings" @@ -48,6 +49,11 @@ var ( openapiV3Types = sets.NewString("string", "number", "integer", "boolean", "array", "object") ) +const ( + // StaticEstimatedCostLimit represents the largest-allowed static CEL cost on a per-expression basis. + StaticEstimatedCostLimit = 10000000 +) + // ValidateCustomResourceDefinition statically validates // context is passed for supporting context cancellation during cel validation when validating defaults func ValidateCustomResourceDefinition(ctx context.Context, obj *apiextensions.CustomResourceDefinition) field.ErrorList { @@ -714,7 +720,7 @@ func validateCustomResourceDefinitionValidation(ctx context.Context, customResou requireValidPropertyType: opts.requireValidPropertyType, } - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema, fldPath.Child("openAPIV3Schema"), openAPIV3Schema, true, &opts)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema, fldPath.Child("openAPIV3Schema"), openAPIV3Schema, true, &opts, rootCostInfo())...) if opts.requireStructuralSchema { if ss, err := structuralschema.NewStructural(schema); err != nil { @@ -742,16 +748,101 @@ 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 +} + +// 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{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, + } +} + var metaFields = sets.NewString("metadata", "kind", "apiVersion") // ValidateCustomResourceDefinitionOpenAPISchema statically validates -func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSchemaProps, fldPath *field.Path, ssv specStandardValidator, isRoot bool, opts *validationOptions) field.ErrorList { +func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSchemaProps, fldPath *field.Path, ssv specStandardValidator, isRoot bool, opts *validationOptions, nodeCostInfo costInfo) field.ErrorList { allErrs := field.ErrorList{} if schema == nil { return allErrs } - allErrs = append(allErrs, ssv.validate(schema, fldPath)...) if schema.UniqueItems == true { @@ -780,7 +871,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)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.AdditionalProperties.Schema, fldPath.Child("additionalProperties"), subSsv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) } if len(schema.Properties) != 0 { @@ -798,33 +889,33 @@ 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)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("properties").Key(property), subSsv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) } } - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Not, fldPath.Child("not"), ssv, false, opts)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Not, fldPath.Child("not"), ssv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) if len(schema.AllOf) != 0 { for i, jsonSchema := range schema.AllOf { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("allOf").Index(i), ssv, false, opts)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("allOf").Index(i), ssv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) } } if len(schema.OneOf) != 0 { for i, jsonSchema := range schema.OneOf { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("oneOf").Index(i), ssv, false, opts)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("oneOf").Index(i), ssv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) } } if len(schema.AnyOf) != 0 { for i, jsonSchema := range schema.AnyOf { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("anyOf").Index(i), ssv, false, opts)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("anyOf").Index(i), ssv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) } } if len(schema.Definitions) != 0 { for definition, jsonSchema := range schema.Definitions { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("definitions").Key(definition), ssv, false, opts)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("definitions").Key(definition), ssv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) } } @@ -838,17 +929,17 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch subSsv = subSsv.withForbidOldSelfValidations(fldPath) } - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), subSsv, false, opts)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), subSsv, false, opts, nodeCostInfo.MultiplyByElementCost(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)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("items").Index(i), subSsv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) } } } if schema.Dependencies != nil { for dependency, jsonSchemaPropsOrStringArray := range schema.Dependencies { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(jsonSchemaPropsOrStringArray.Schema, fldPath.Child("dependencies").Key(dependency), ssv, false, opts)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(jsonSchemaPropsOrStringArray.Schema, fldPath.Child("dependencies").Key(dependency), ssv, false, opts, nodeCostInfo.MultiplyByElementCost(schema))...) } } @@ -957,6 +1048,11 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch allErrs = append(allErrs, field.InternalError(fldPath.Child("x-kubernetes-validations"), err)) } else { for i, cr := range compResults { + expressionCost := getExpressionCost(cr, nodeCostInfo) + if expressionCost > StaticEstimatedCostLimit { + costErrorMsg := getCostErrorMessage(expressionCost, StaticEstimatedCostLimit) + allErrs = append(allErrs, field.Forbidden(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), costErrorMsg)) + } 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)) @@ -981,6 +1077,36 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch return allErrs } +// multiplyWithOverflowGuard returns the product of baseCost and cardinality unless that product +// would exceed math.MaxUint, in which case math.MaxUint is returned. +func multiplyWithOverflowGuard(baseCost, cardinality uint64) uint64 { + if baseCost == 0 { + // an empty rule can return 0, so guard for that here + return 0 + } else if math.MaxUint/baseCost < cardinality { + return math.MaxUint + } + return baseCost * cardinality +} + +func getExpressionCost(cr cel.CompilationResult, cardinalityCost costInfo) uint64 { + if cardinalityCost.MaxCardinality != unbounded { + return multiplyWithOverflowGuard(cr.MaxCost, *cardinalityCost.MaxCardinality) + } + return multiplyWithOverflowGuard(cr.MaxCost, cr.MaxCardinality) +} + +func getCostErrorMessage(expressionCost, costLimit uint64) string { + exceedFactor := float64(expressionCost) / float64(StaticEstimatedCostLimit) + if exceedFactor > 100.0 { + // if exceedFactor is greater than 2 orders of magnitude, the rule is likely O(n^2) or worse + // and will probably never validate without some set limits + // also in such cases the cost estimation is generally large enough to not add any value + return fmt.Sprintf("CEL rule exceeded budget by more than 100x (try simplifying the rule, or adding maxItems, maxProperties, and maxLength where arrays, maps, and strings are used)") + } + return fmt.Sprintf("CEL rule exceeded budget by factor of %.1fx (try adding maxItems, maxProperties, and maxLength where arrays, maps, and strings are used)", exceedFactor) +} + var newlineMatcher = regexp.MustCompile(`[\n\r]+`) // valid newline chars in CEL grammar func hasNewlines(s string) bool { return newlineMatcher.MatchString(s) 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 45c356e9dba..d0324f220c7 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 @@ -18,6 +18,7 @@ package validation import ( "context" + "math" "math/rand" "reflect" "strings" @@ -7605,7 +7606,8 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XListType: strPtr("atomic"), Items: &apiextensions.JSONSchemaPropsOrArray{ Schema: &apiextensions.JSONSchemaProps{ - Type: "string", + Type: "string", + MaxLength: int64ptr(10), XValidations: apiextensions.ValidationRules{ {Rule: "self == oldSelf"}, }, @@ -7629,7 +7631,8 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { Type: "array", Items: &apiextensions.JSONSchemaPropsOrArray{ Schema: &apiextensions.JSONSchemaProps{ - Type: "string", + Type: "string", + MaxLength: int64ptr(10), XValidations: apiextensions.ValidationRules{ {Rule: "self == oldSelf"}, }, @@ -7651,6 +7654,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { Properties: map[string]apiextensions.JSONSchemaProps{ "value": { Type: "array", + MaxItems: int64ptr(10), XListType: strPtr("atomic"), Items: &apiextensions.JSONSchemaPropsOrArray{ Schema: &apiextensions.JSONSchemaProps{ @@ -7672,10 +7676,12 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "value": { - Type: "array", + Type: "array", + MaxItems: int64ptr(10), Items: &apiextensions.JSONSchemaPropsOrArray{ Schema: &apiextensions.JSONSchemaProps{ - Type: "string", + Type: "string", + MaxLength: int64ptr(10), }, }, XValidations: apiextensions.ValidationRules{ @@ -7694,10 +7700,12 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { Properties: map[string]apiextensions.JSONSchemaProps{ "value": { Type: "array", + MaxItems: int64ptr(10), XListType: strPtr("set"), Items: &apiextensions.JSONSchemaPropsOrArray{ Schema: &apiextensions.JSONSchemaProps{ - Type: "string", + Type: "string", + MaxLength: int64ptr(10), XValidations: apiextensions.ValidationRules{ {Rule: "self == oldSelf"}, }, @@ -7719,10 +7727,12 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { Properties: map[string]apiextensions.JSONSchemaProps{ "value": { Type: "array", + MaxItems: int64ptr(10), XListType: strPtr("set"), Items: &apiextensions.JSONSchemaPropsOrArray{ Schema: &apiextensions.JSONSchemaProps{ - Type: "string", + Type: "string", + MaxLength: int64ptr(10), }, }, XValidations: apiextensions.ValidationRules{ @@ -7751,7 +7761,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, Required: []string{"name"}, Properties: map[string]apiextensions.JSONSchemaProps{ - "name": {Type: "string"}, + "name": {Type: "string", MaxLength: int64ptr(5)}, }, }, }, @@ -7768,6 +7778,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { Properties: map[string]apiextensions.JSONSchemaProps{ "value": { Type: "array", + MaxItems: int64ptr(10), XListType: strPtr("map"), XListMapKeys: []string{"name"}, Items: &apiextensions.JSONSchemaPropsOrArray{ @@ -7775,7 +7786,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { Type: "object", Required: []string{"name"}, Properties: map[string]apiextensions.JSONSchemaProps{ - "name": {Type: "string"}, + "name": {Type: "string", MaxLength: int64ptr(5)}, }, }, }, @@ -7798,7 +7809,8 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XMapType: strPtr("granular"), Properties: map[string]apiextensions.JSONSchemaProps{ "subfield": { - Type: "string", + Type: "string", + MaxLength: int64ptr(10), XValidations: apiextensions.ValidationRules{ {Rule: "self == oldSelf"}, }, @@ -7820,7 +7832,8 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XMapType: strPtr("future"), Properties: map[string]apiextensions.JSONSchemaProps{ "subfield": { - Type: "string", + Type: "string", + MaxLength: int64ptr(10), XValidations: apiextensions.ValidationRules{ {Rule: "self == oldSelf"}, }, @@ -7845,7 +7858,8 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ "subfield": { - Type: "string", + Type: "string", + MaxLength: int64ptr(10), XValidations: apiextensions.ValidationRules{ {Rule: "self == oldSelf"}, }, @@ -7928,6 +7942,154 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, + { + name: "forbid double-nested rule with no limit set", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "value": { + Type: "array", + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Required: []string{"key"}, + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": {Type: "string"}, + }, + }, + }, + }, + }, + XValidations: apiextensions.ValidationRules{ + {Rule: "self.all(x, x.all(y, x[y].key == x[y].key))"}, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + forbidden("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[0].rule"), + }, + }, + { + name: "forbid double-nested rule with one limit set", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "value": { + Type: "array", + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Required: []string{"key"}, + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": {Type: "string", MaxLength: int64ptr(10)}, + }, + }, + }, + }, + }, + XValidations: apiextensions.ValidationRules{ + {Rule: "self.all(x, x.all(y, x[y].key == x[y].key))"}, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + forbidden("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[0].rule"), + }, + }, + { + name: "allow double-nested rule with three limits set", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "value": { + Type: "array", + MaxItems: int64ptr(10), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + MaxProperties: int64ptr(10), + AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Required: []string{"key"}, + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": {Type: "string", MaxLength: int64ptr(10)}, + }, + }, + }, + }, + }, + XValidations: apiextensions.ValidationRules{ + {Rule: "self.all(x, x.all(y, x[y].key == x[y].key))"}, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{}, + }, + { + name: "allow double-nested rule with one limit set on outermost array", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "value": { + Type: "array", + MaxItems: int64ptr(4), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Required: []string{"key"}, + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": {Type: "number"}, + }, + }, + }, + }, + }, + XValidations: apiextensions.ValidationRules{ + {Rule: "self.all(x, x.all(y, x[y].key == x[y].key))"}, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{}, + }, + { + name: "check for cardinality of 1 under root object", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "value": { + Type: "integer", + XValidations: apiextensions.ValidationRules{ + {Rule: "self < 1024"}, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -8168,3 +8330,139 @@ func Test_validateDeprecationWarning(t *testing.T) { }) } } + +func genMapSchema() *apiextensions.JSONSchemaProps { + return &apiextensions.JSONSchemaProps{ + Type: "object", + AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + }, + }, + } +} + +func withMaxProperties(mapSchema *apiextensions.JSONSchemaProps, maxProps *int64) *apiextensions.JSONSchemaProps { + mapSchema.MaxProperties = maxProps + return mapSchema +} + +func genArraySchema() *apiextensions.JSONSchemaProps { + return &apiextensions.JSONSchemaProps{ + Type: "array", + } +} + +func withMaxItems(arraySchema *apiextensions.JSONSchemaProps, maxItems *int64) *apiextensions.JSONSchemaProps { + arraySchema.MaxItems = maxItems + return arraySchema +} + +func genObjectSchema() *apiextensions.JSONSchemaProps { + return &apiextensions.JSONSchemaProps{ + Type: "object", + } +} + +func TestCostInfo(t *testing.T) { + tests := []struct { + name string + schema []*apiextensions.JSONSchemaProps + expectedMaxCardinality *uint64 + }{ + { + name: "object", + schema: []*apiextensions.JSONSchemaProps{ + genObjectSchema(), + }, + expectedMaxCardinality: uint64ptr(1), + }, + { + name: "array", + schema: []*apiextensions.JSONSchemaProps{ + withMaxItems(genArraySchema(), int64ptr(5)), + }, + expectedMaxCardinality: uint64ptr(5), + }, + { + name: "unbounded array", + schema: []*apiextensions.JSONSchemaProps{genArraySchema()}, + expectedMaxCardinality: nil, + }, + { + name: "map", + schema: []*apiextensions.JSONSchemaProps{withMaxProperties(genMapSchema(), int64ptr(10))}, + expectedMaxCardinality: uint64ptr(10), + }, + { + name: "unbounded map", + schema: []*apiextensions.JSONSchemaProps{ + genMapSchema(), + }, + expectedMaxCardinality: nil, + }, + { + name: "array inside map", + schema: []*apiextensions.JSONSchemaProps{ + withMaxProperties(genMapSchema(), int64ptr(5)), + withMaxItems(genArraySchema(), int64ptr(5)), + }, + expectedMaxCardinality: uint64ptr(25), + }, + { + name: "unbounded array inside bounded map", + schema: []*apiextensions.JSONSchemaProps{ + withMaxProperties(genMapSchema(), int64ptr(5)), + genArraySchema(), + }, + expectedMaxCardinality: nil, + }, + { + name: "object inside array", + schema: []*apiextensions.JSONSchemaProps{ + withMaxItems(genArraySchema(), int64ptr(3)), + genObjectSchema(), + }, + expectedMaxCardinality: uint64ptr(3), + }, + { + name: "map inside object inside array", + schema: []*apiextensions.JSONSchemaProps{ + withMaxItems(genArraySchema(), int64ptr(2)), + genObjectSchema(), + withMaxProperties(genMapSchema(), int64ptr(4)), + }, + expectedMaxCardinality: uint64ptr(8), + }, + { + name: "integer overflow bounds check", + schema: []*apiextensions.JSONSchemaProps{ + withMaxItems(genArraySchema(), int64ptr(math.MaxInt)), + withMaxItems(genArraySchema(), int64ptr(100)), + }, + expectedMaxCardinality: uint64ptr(math.MaxUint), + }, + } + 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) + } + if tt.expectedMaxCardinality == nil && curCostInfo.MaxCardinality == nil { + // unbounded cardinality case, test ran correctly + } else if tt.expectedMaxCardinality == nil && curCostInfo.MaxCardinality != nil { + t.Errorf("expected unbounded cardinality (got %d)", curCostInfo.MaxCardinality) + } else if tt.expectedMaxCardinality != nil && curCostInfo.MaxCardinality == nil { + t.Errorf("expected bounded cardinality of %d but got unbounded cardinality", tt.expectedMaxCardinality) + } else if *tt.expectedMaxCardinality != *curCostInfo.MaxCardinality { + t.Errorf("wrong cardinality (expected %d, got %d)", *tt.expectedMaxCardinality, curCostInfo.MaxCardinality) + } + }) + } +} + +func int64ptr(i int64) *int64 { + return &i +} 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 fae806d3318..d9a5cdb963b 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 @@ -64,6 +64,9 @@ type CompilationResult struct { TransitionRule bool // Represents the worst-case cost of the compiled expression in terms of CEL's cost units, as used by cel.EstimateCost. MaxCost uint64 + // MaxCardinality represents the worse case number of times this validation rule could be invoked if contained under an + // unbounded map or list in an OpenAPIv3 schema. + MaxCardinality uint64 } // Compile compiles all the XValidations rules (without recursing into the schema) and returns a slice containing a @@ -120,14 +123,15 @@ func Compile(s *schema.Structural, isResourceRoot bool, perCallLimit uint64) ([] estimator := newCostEstimator(root) // compResults is the return value which saves a list of compilation results in the same order as x-kubernetes-validations rules. compResults := make([]CompilationResult, len(celRules)) + maxCardinality := celmodel.MaxCardinality(s) for i, rule := range celRules { - compResults[i] = compileRule(rule, env, perCallLimit, estimator) + compResults[i] = compileRule(rule, env, perCallLimit, estimator, maxCardinality) } return compResults, nil } -func compileRule(rule apiextensions.ValidationRule, env *cel.Env, perCallLimit uint64, estimator *library.CostEstimator) (compilationResult CompilationResult) { +func compileRule(rule apiextensions.ValidationRule, env *cel.Env, perCallLimit uint64, estimator *library.CostEstimator, maxCardinality uint64) (compilationResult CompilationResult) { if len(strings.TrimSpace(rule.Rule)) == 0 { // include a compilation result, but leave both program and error nil per documented return semantics of this // function @@ -174,6 +178,7 @@ func compileRule(rule apiextensions.ValidationRule, env *cel.Env, perCallLimit u return } compilationResult.MaxCost = costEst.Max + compilationResult.MaxCardinality = maxCardinality compilationResult.Program = prog return } 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 c0b914dae99..84750cc21a5 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 @@ -18,7 +18,6 @@ package cel import ( "fmt" - "math" "strings" "testing" @@ -1078,8 +1077,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) { - // TODO(DangerOnTheRanger): if perCallLimit in compilation.go changes, this needs to change as well - compilationResults, err := Compile(schema, false, uint64(math.MaxInt64)) + compilationResults, err := Compile(schema, false, PerCallLimit) if err != nil { t.Errorf("Expected no error, got: %v", err) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model/schemas.go b/staging/src/k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model/schemas.go index 51895a64665..b6a92a938e8 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model/schemas.go +++ b/staging/src/k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model/schemas.go @@ -94,7 +94,7 @@ func SchemaDeclType(s *schema.Structural, isResourceRoot bool) *DeclType { itemsType := SchemaDeclType(s.Items, s.Items.XEmbeddedResource) var maxItems int64 if s.ValueValidation != nil && s.ValueValidation.MaxItems != nil { - maxItems = *s.ValueValidation.MaxItems + maxItems = zeroIfNegative(*s.ValueValidation.MaxItems) } else { maxItems = estimateMaxArrayItemsPerRequest(s.Items) } @@ -109,7 +109,7 @@ func SchemaDeclType(s *schema.Structural, isResourceRoot bool) *DeclType { if propsType != nil { var maxProperties int64 if s.ValueValidation != nil && s.ValueValidation.MaxProperties != nil { - maxProperties = *s.ValueValidation.MaxProperties + maxProperties = zeroIfNegative(*s.ValueValidation.MaxProperties) } else { maxProperties = estimateMaxAdditionalPropertiesPerRequest(s.AdditionalProperties.Structural) } @@ -151,7 +151,7 @@ func SchemaDeclType(s *schema.Structural, isResourceRoot bool) *DeclType { case "byte": byteWithMaxLength := newSimpleType("bytes", decls.Bytes, types.Bytes([]byte{})) if s.ValueValidation.MaxLength != nil { - byteWithMaxLength.MaxElements = *s.ValueValidation.MaxLength + byteWithMaxLength.MaxElements = zeroIfNegative(*s.ValueValidation.MaxLength) } else { byteWithMaxLength.MaxElements = estimateMaxStringLengthPerRequest(s) } @@ -172,7 +172,7 @@ func SchemaDeclType(s *schema.Structural, isResourceRoot bool) *DeclType { // we do this because the OpenAPIv3 spec indicates that maxLength is specified in runes/code points, // but we need to reason about length for things like request size, so we use bytes in this code (and an individual // unicode code point can be up to 4 bytes long) - strWithMaxLength.MaxElements = *s.ValueValidation.MaxLength * 4 + strWithMaxLength.MaxElements = zeroIfNegative(*s.ValueValidation.MaxLength) * 4 } else { strWithMaxLength.MaxElements = estimateMaxStringLengthPerRequest(s) } @@ -187,6 +187,13 @@ func SchemaDeclType(s *schema.Structural, isResourceRoot bool) *DeclType { return nil } +func zeroIfNegative(v int64) int64 { + if v < 0 { + return 0 + } + return v +} + // WithTypeAndObjectMeta ensures the kind, apiVersion and // metadata.name and metadata.generateName properties are specified, making a shallow copy of the provided schema if needed. func WithTypeAndObjectMeta(s *schema.Structural) *schema.Structural { @@ -223,6 +230,16 @@ func WithTypeAndObjectMeta(s *schema.Structural) *schema.Structural { return result } +// MaxCardinality returns the maximum number of times data conforming to the schema could possibly exist in +// an object serialized to JSON. For cases where a schema is contained under map or array schemas of unbounded +// size, this can be used as an estimate as the worst case number of times data matching the schema could be repeated. +// Note that this only assumes a single comma between data elements, so if the schema is contained under only maps, +// this estimates a higher cardinality that would be possible. +func MaxCardinality(s *schema.Structural) uint64 { + sz := estimateMinSizeJSON(s) + 1 // assume at least one comma between elements + return uint64(maxRequestSizeBytes / sz) +} + // estimateMinSizeJSON estimates the minimum size in bytes of the given schema when serialized in JSON. // minLength/minProperties/minItems are not currently taken into account, so if these limits are set the // minimum size might be higher than what estimateMinSizeJSON returns. diff --git a/test/integration/apiserver/crd_validation_expressions_test.go b/test/integration/apiserver/crd_validation_expressions_test.go index 40d1be32692..d5db2796eae 100644 --- a/test/integration/apiserver/crd_validation_expressions_test.go +++ b/test/integration/apiserver/crd_validation_expressions_test.go @@ -751,11 +751,13 @@ var structuralSchemaWithValidators = []byte(` }, "assocList": { "type": "array", + "maxItems": 10, "items": { "type": "object", + "maxProperties": 12, "properties": { - "k": { "type": "string" }, - "v": { "type": "string" } + "k": { "type": "string", "maxLength": 3}, + "v": { "type": "string", "maxLength": 3} }, "required": ["k"] }, @@ -1006,12 +1008,13 @@ var structuralSchemaWithDefaultMapKeyTransitionRule = []byte(` "k2" ], "x-kubernetes-list-type": "map", + "maxItems": 100, "items": { "type": "object", "properties": { "k1": { "type": "string" }, "k2": { "type": "string", "default": "DEFAULT" }, - "v": { "type": "string" } + "v": { "type": "string", "maxLength": 200 } }, "required": ["k1"], "x-kubernetes-validations": [