From 8dc20a0f62d44af1d584efbbc6bf918b93c4e3d8 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Wed, 9 Jul 2025 12:14:28 +0100 Subject: [PATCH 1/2] Fix IntOrString cost estimation when schema has a MaxLength constraint --- .../apiserver/pkg/cel/common/schemas.go | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/cel/common/schemas.go b/staging/src/k8s.io/apiserver/pkg/cel/common/schemas.go index 19392babeb2..909284166ab 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/common/schemas.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/common/schemas.go @@ -55,8 +55,15 @@ func SchemaDeclType(s Schema, isResourceRoot bool) *apiservercel.DeclType { // `type(intOrStringField) == int ? intOrStringField < 5 : double(intOrStringField.replace('%', '')) < 0.5 // dyn := apiservercel.NewSimpleTypeWithMinSize("dyn", cel.DynType, nil, 1) // smallest value for a serialized x-kubernetes-int-or-string is 0 - // handle x-kubernetes-int-or-string by returning the max length/min serialized size of the largest possible string - dyn.MaxElements = maxRequestSizeBytes - 2 + + // If the schema has a maxlength constraint, bound the max elements based on the max length. + // Otherwise, fallback to the max request size. + if s.MaxLength() != nil { + dyn.MaxElements = estimateMaxElementsFromMaxLength(s) + } else { + dyn.MaxElements = estimateMaxStringLengthPerRequest(s) + } + return dyn } @@ -159,11 +166,7 @@ func SchemaDeclType(s Schema, isResourceRoot bool) *apiservercel.DeclType { strWithMaxLength := apiservercel.NewSimpleTypeWithMinSize("string", cel.StringType, types.String(""), apiservercel.MinStringSize) if s.MaxLength() != nil { - // multiply the user-provided max length by 4 in the case of an otherwise-untyped string - // 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 = zeroIfNegative(*s.MaxLength()) * 4 + strWithMaxLength.MaxElements = estimateMaxElementsFromMaxLength(s) } else { if len(s.Enum()) > 0 { strWithMaxLength.MaxElements = estimateMaxStringEnumLength(s) @@ -228,6 +231,7 @@ func WithTypeAndObjectMeta(s *spec.Schema) *spec.Schema { // must only be called on schemas of type "string" or x-kubernetes-int-or-string: true func estimateMaxStringLengthPerRequest(s Schema) int64 { if s.IsXIntOrString() { + // handle x-kubernetes-int-or-string by returning the max length/min serialized size of the largest possible string return maxRequestSizeBytes - 2 } switch s.Format() { @@ -272,3 +276,13 @@ func estimateMaxAdditionalPropertiesFromMinSize(minSize int64) int64 { // subtract 2 to account for { and } return (maxRequestSizeBytes - 2) / keyValuePairSize } + +// estimateMaxElementsFromMaxLength estimates the maximum number of elements for a string schema +// that is bound with a maxLength constraint. +func estimateMaxElementsFromMaxLength(s Schema) int64 { + // multiply the user-provided max length by 4 in the case of an otherwise-untyped string + // 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) + return zeroIfNegative(*s.MaxLength()) * 4 +} From b8d74e75c7bfd9c6479b89976ebcda84bd152fe3 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Fri, 18 Jul 2025 12:15:28 +0100 Subject: [PATCH 2/2] Add test case to prove MaxElements correctly set on IntOrString --- .../apiserver/schema/cel/compilation_test.go | 12 ++++++ .../schema/cel/model/schemas_test.go | 41 ++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) 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 690f831af36..70aa2ea266d 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 @@ -1872,6 +1872,18 @@ func TestCostEstimation(t *testing.T) { setMaxElements: 1000, expectedSetCost: 401, }, + { + name: "IntOrString type with quantity rule", + schemaGenerator: func(max *int64) *schema.Structural { + intOrString := intOrStringType() + intOrString = withRule(intOrString, "isQuantity(self)") + intOrString = withMaxLength(intOrString, max) + return &intOrString + }, + expectedCalcCost: 314574, + setMaxElements: 20, + expectedSetCost: 9, + }, } for _, testCase := range cases { t.Run(testCase.name, func(t *testing.T) { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/model/schemas_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/model/schemas_test.go index 53a97f52681..f0331c3c3c1 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/model/schemas_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/model/schemas_test.go @@ -35,8 +35,8 @@ func TestSchemaDeclType(t *testing.T) { if cust.TypeName() != "object" { t.Errorf("incorrect type name, got %v, wanted object", cust.TypeName()) } - if len(cust.Fields) != 4 { - t.Errorf("incorrect number of fields, got %d, wanted 4", len(cust.Fields)) + if len(cust.Fields) != 5 { + t.Errorf("incorrect number of fields, got %d, wanted 5", len(cust.Fields)) } for _, f := range cust.Fields { prop, found := ts.Properties[f.Name] @@ -71,6 +71,13 @@ func TestSchemaDeclType(t *testing.T) { } } } + if prop.ValueValidation != nil && prop.ValueValidation.MaxLength != nil { + if f.Type.MaxElements != 4*(*prop.ValueValidation.MaxLength) { + // When converting maxLength to maxElements, it's based on the number of bytes.] + // Worst case is that one rune is 4 bytes, so maxElements should be 4x maxLength. + t.Errorf("field maxElements does not match property 4x maxLength. field: %s, maxElements: %d, maxLength: %d", f.Name, f.Type.MaxElements, *prop.ValueValidation.MaxLength) + } + } } if ts.ValueValidation != nil { for _, name := range ts.ValueValidation.Required { @@ -138,6 +145,7 @@ func testSchema() *schema.Structural { // properties: // name: // type: string + // maxLength: 256 // nested: // type: object // properties: @@ -167,6 +175,12 @@ func testSchema() *schema.Structural { // format: int64 // default: 1 // enum: [1,2,3] + // intOrString: + // x-kubernetes-int-or-string: true + // anyOf: + // - type: "integer" + // - type: "string" + // maxLength: 20 ts := &schema.Structural{ Generic: schema.Generic{ Type: "object", @@ -176,6 +190,9 @@ func testSchema() *schema.Structural { Generic: schema.Generic{ Type: "string", }, + ValueValidation: &schema.ValueValidation{ + MaxLength: ptr.To[int64](256), + }, }, "value": { Generic: schema.Generic{ @@ -246,6 +263,26 @@ func testSchema() *schema.Structural { }, }, }, + "intOrString": { + Extensions: schema.Extensions{ + XIntOrString: true, + }, + ValueValidation: &schema.ValueValidation{ + MaxLength: ptr.To[int64](20), + AnyOf: []schema.NestedValueValidation{ + { + ForbiddenGenerics: schema.Generic{ + Type: "integer", + }, + }, + { + ForbiddenGenerics: schema.Generic{ + Type: "string", + }, + }, + }, + }, + }, }, } return ts