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 2ef5e7a1fc5..e47a30d3a79 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 @@ -1005,6 +1005,7 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch for i, rule := range schema.XValidations { trimmedRule := strings.TrimSpace(rule.Rule) trimmedMsg := strings.TrimSpace(rule.Message) + trimmedMsgExpr := strings.TrimSpace(rule.MessageExpression) if len(trimmedRule) == 0 { allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Required(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), "rule is not specified")) } else if len(rule.Message) > 0 && len(trimmedMsg) == 0 { @@ -1014,6 +1015,9 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch } else if hasNewlines(trimmedRule) && len(trimmedMsg) == 0 { allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Required(fldPath.Child("x-kubernetes-validations").Index(i).Child("message"), "message must be specified if rule contains line breaks")) } + if len(rule.MessageExpression) > 0 && len(trimmedMsgExpr) == 0 { + allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Required(fldPath.Child("x-kubernetes-validations").Index(i).Child("messageExpression"), "messageExpression must be non-empty if specified")) + } } // If any schema related validation errors have been found at this level or deeper, skip CEL expression validation. @@ -1047,6 +1051,19 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch allErrs.CELErrors = append(allErrs.CELErrors, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i], cr.Error.Detail)) } } + if cr.MessageExpressionError != nil { + allErrs.CELErrors = append(allErrs.CELErrors, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("messageExpression"), schema.XValidations[i], cr.MessageExpressionError.Detail)) + } else { + if cr.MessageExpression != nil { + if cr.MessageExpressionMaxCost > StaticEstimatedCostLimit { + costErrorMsg := getCostErrorMessage("estimated messageExpression cost", cr.MessageExpressionMaxCost, StaticEstimatedCostLimit) + allErrs.CELErrors = append(allErrs.CELErrors, field.Forbidden(fldPath.Child("x-kubernetes-validations").Index(i).Child("messageExpression"), costErrorMsg)) + } + if celContext.TotalCost != nil { + celContext.TotalCost.ObserveExpressionCost(fldPath.Child("x-kubernetes-validations").Index(i).Child("messageExpression"), cr.MessageExpressionMaxCost) + } + } + } if cr.TransitionRule { if uncorrelatablePath := ssv.forbidOldSelfValidations(); uncorrelatablePath != nil { allErrs.CELErrors = append(allErrs.CELErrors, 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))) 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 687d238b455..e7d3cc8d410 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 @@ -8558,6 +8558,180 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { invalid("spec.validation.openAPIV3Schema.properties[f@2].x-kubernetes-validations[0].rule"), }, }, + { + name: "x-kubernetes-validations rule with messageExpression", + opts: validationOptions{requireStructuralSchema: true}, + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "f": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self == \"string value\"", + MessageExpression: `self + " should be \"string value\""`, + }, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{}, + }, + { + name: "x-kubernetes-validations rule allows both message and messageExpression", + opts: validationOptions{requireStructuralSchema: true}, + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "f": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self == \"string value\"", + Message: `string should be set to "string value"`, + MessageExpression: `self + " should be \"string value\""`, + }, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{}, + }, + { + name: "x-kubernetes-validations rule invalidated by messageExpression syntax error", + opts: validationOptions{requireStructuralSchema: true}, + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "f": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self == \"string value\"", + MessageExpression: `self + " `, + }, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.properties[f].x-kubernetes-validations[0].messageExpression"), + }, + }, + { + name: "x-kubernetes-validations rule invalidated by messageExpression not returning a string", + opts: validationOptions{requireStructuralSchema: true}, + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "f": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self == \"string value\"", + MessageExpression: `256`, + }, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.properties[f].x-kubernetes-validations[0].messageExpression"), + }, + }, + { + name: "x-kubernetes-validations rule invalidated by messageExpression exceeding per-expression estimated cost limit", + opts: validationOptions{requireStructuralSchema: true}, + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "f": { + Type: "array", + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + }, + }, + XValidations: apiextensions.ValidationRules{ + { + Rule: "true", + MessageExpression: `self[0] + self[1] + self[2] + self[3] + self[4] + self[5] + self[6] + self[7]`, + }, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + // forbidden due to messageExpression exceeding per-expression cost limit + forbidden("spec.validation.openAPIV3Schema.properties[f].x-kubernetes-validations[0].messageExpression"), + }, + }, + { + name: "x-kubernetes-validations rule invalidated by messageExpression exceeding per-CRD estimated cost limit", + opts: validationOptions{requireStructuralSchema: true}, + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "f": { + Type: "array", + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + }, + }, + XValidations: apiextensions.ValidationRules{ + { + Rule: "true", + MessageExpression: `string(self[0]) + string(self[1]) + string(self[2])`, + }, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + // forbidden due to per-CRD cost limit being exceeded + forbidden("spec.validation.openAPIV3Schema"), + // forbidden due to messageExpression exceeding per-expression cost limit + forbidden("spec.validation.openAPIV3Schema.properties[f].x-kubernetes-validations[0].messageExpression"), + // additional message indicated messageExpression's contribution to exceeding the per-CRD cost limit + forbidden("spec.validation.openAPIV3Schema.properties[f].x-kubernetes-validations[0].messageExpression"), + }, + }, + { + name: "x-kubernetes-validations rule invalidated by messageExpression being only empty spaces", + opts: validationOptions{requireStructuralSchema: true}, + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "f": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self == \"string value\"", + MessageExpression: ` `, + }, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + required("spec.validation.openAPIV3Schema.properties[f].x-kubernetes-validations[0].messageExpression"), + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { 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 1f0c9861d1f..35d5d604335 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 @@ -55,6 +55,15 @@ type CompilationResult struct { // 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 + // MessageExpression represents the cel Program that should be evaluated to generate an error message if the rule + // fails to validate. If no MessageExpression was given, or if this expression failed to compile, this will be nil. + MessageExpression cel.Program + // MessageExpressionError represents an error encountered during compilation of MessageExpression. If no error was + // encountered, this will be nil. + MessageExpressionError *apiservercel.Error + // MessageExpressionMaxCost represents the worst-case cost of the compiled MessageExpression in terms of CEL's cost units, + // as used by cel.EstimateCost. + MessageExpressionMaxCost uint64 } var ( @@ -194,6 +203,42 @@ func compileRule(rule apiextensions.ValidationRule, env *cel.Env, perCallLimit u compilationResult.MaxCost = costEst.Max compilationResult.MaxCardinality = maxCardinality compilationResult.Program = prog + if rule.MessageExpression != "" { + ast, issues := env.Compile(rule.MessageExpression) + if issues != nil { + compilationResult.MessageExpressionError = &apiservercel.Error{Type: apiservercel.ErrorTypeInvalid, Detail: "messageExpression compilation failed: " + issues.String()} + return + } + if ast.OutputType() != cel.StringType { + compilationResult.MessageExpressionError = &apiservercel.Error{Type: apiservercel.ErrorTypeInvalid, Detail: "messageExpression must evaluate to a string"} + return + } + + _, err := cel.AstToCheckedExpr(ast) + if err != nil { + compilationResult.MessageExpressionError = &apiservercel.Error{Type: apiservercel.ErrorTypeInternal, Detail: "unexpected messageExpression compilation error: " + err.Error()} + return + } + + msgProg, err := env.Program(ast, + cel.EvalOptions(cel.OptOptimize, cel.OptTrackCost), + cel.CostLimit(perCallLimit), + cel.CostTracking(estimator), + cel.OptimizeRegex(library.ExtensionLibRegexOptimizations...), + cel.InterruptCheckFrequency(celconfig.CheckFrequency), + ) + if err != nil { + compilationResult.MessageExpressionError = &apiservercel.Error{Type: apiservercel.ErrorTypeInvalid, Detail: "messageExpression instantiation failed: " + err.Error()} + return + } + costEst, err := env.EstimateCost(ast, estimator) + if err != nil { + compilationResult.MessageExpressionError = &apiservercel.Error{Type: apiservercel.ErrorTypeInternal, Detail: "cost estimation failed for messageExpression: " + err.Error()} + return + } + compilationResult.MessageExpression = msgProg + compilationResult.MessageExpressionMaxCost = costEst.Max + } 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 0634d463baf..8a8e7599dbb 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 @@ -98,6 +98,22 @@ func (v errorMatcher) String() string { return fmt.Sprintf("has error of type %q containing string %q", v.errorType, v.contains) } +type messageExpressionErrorMatcher struct { + contains string +} + +func messageExpressionError(contains string) validationMatcher { + return messageExpressionErrorMatcher{contains: contains} +} + +func (m messageExpressionErrorMatcher) matches(cr CompilationResult) bool { + return cr.MessageExpressionError != nil && cr.MessageExpressionError.Type == cel.ErrorTypeInvalid && strings.Contains(cr.MessageExpressionError.Error(), m.contains) +} + +func (m messageExpressionErrorMatcher) String() string { + return fmt.Sprintf("has messageExpression error containing string %q", m.contains) +} + type noErrorMatcher struct{} func noError() validationMatcher { @@ -642,6 +658,63 @@ func TestCelCompilation(t *testing.T) { invalidError("must evaluate to a bool"), }, }, + { + name: "messageExpression inclusion", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "self.startsWith('s')", + MessageExpression: `"scoped field should start with 's'"`, + }, + }, + }, + }, + expectedResults: []validationMatcher{ + noError(), + }, + }, + { + name: "messageExpression must evaluate to a string", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "integer", + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "self == 5", + MessageExpression: `42`, + }, + }, + }, + }, + expectedResults: []validationMatcher{ + messageExpressionError("must evaluate to a string"), + }, + }, + { + name: "messageExpression syntax error", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "number", + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "self < 32.0", + MessageExpression: `"abc`, + }, + }, + }, + }, + expectedResults: []validationMatcher{ + messageExpressionError("messageExpression compilation failed"), + }, + }, } for _, tt := range cases { 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 dfa5822245c..2b630e97786 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 @@ -21,9 +21,11 @@ import ( "fmt" "math" "reflect" + "regexp" "strings" "time" + celgo "github.com/google/cel-go/cel" "github.com/google/cel-go/common/types" "github.com/google/cel-go/common/types/ref" "github.com/google/cel-go/interpreter" @@ -34,6 +36,9 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/cel" "k8s.io/apiserver/pkg/cel/metrics" + "k8s.io/klog/v2" + + celconfig "k8s.io/apiserver/pkg/apis/cel" ) // Validator parallels the structure of schema.Structural and includes the compiled CEL programs @@ -252,16 +257,102 @@ func (s *Validator) validateExpressions(ctx context.Context, fldPath *field.Path continue } if evalResult != types.True { - if len(rule.Message) != 0 { - errs = append(errs, field.Invalid(fldPath, sts.Type, rule.Message)) + if compiled.MessageExpression != nil { + messageExpression, newRemainingBudget, msgErr := evalMessageExpression(ctx, compiled.MessageExpression, rule.MessageExpression, activation, remainingBudget) + if msgErr != nil { + if msgErr.Type == cel.ErrorTypeInternal { + errs = append(errs, field.InternalError(fldPath, msgErr)) + return errs, -1 + } else if msgErr.Type == cel.ErrorTypeInvalid { + errs = append(errs, field.Invalid(fldPath, sts.Type, msgErr.Error())) + return errs, -1 + } else { + klog.V(2).ErrorS(msgErr, "messageExpression evaluation failed") + errs = append(errs, field.Invalid(fldPath, sts.Type, ruleMessageOrDefault(rule))) + remainingBudget = newRemainingBudget + } + } else { + errs = append(errs, field.Invalid(fldPath, sts.Type, messageExpression)) + remainingBudget = newRemainingBudget + } } else { - errs = append(errs, field.Invalid(fldPath, sts.Type, fmt.Sprintf("failed rule: %s", ruleErrorString(rule)))) + errs = append(errs, field.Invalid(fldPath, sts.Type, ruleMessageOrDefault(rule))) } } } return errs, remainingBudget } +// evalMessageExpression evaluates the given message expression and returns the evaluated string form and the remaining budget, or an error if one +// occurred during evaluation. +func evalMessageExpression(ctx context.Context, expr celgo.Program, exprSrc string, activation interpreter.Activation, remainingBudget int64) (string, int64, *cel.Error) { + evalResult, evalDetails, err := expr.ContextEval(ctx, activation) + if evalDetails == nil { + return "", -1, &cel.Error{ + Type: cel.ErrorTypeInternal, + Detail: fmt.Sprintf("runtime cost could not be calculated for messageExpression: %q", exprSrc), + } + } + rtCost := evalDetails.ActualCost() + if rtCost == nil { + return "", -1, &cel.Error{ + Type: cel.ErrorTypeInternal, + Detail: fmt.Sprintf("runtime cost could not be calculated for messageExpression: %q", exprSrc), + } + } else if *rtCost > math.MaxInt64 || int64(*rtCost) > remainingBudget { + return "", -1, &cel.Error{ + Type: cel.ErrorTypeInvalid, + Detail: "messageExpression evaluation failed due to running out of cost budget, no further validation rules will be run", + } + } + if err != nil { + if strings.HasPrefix(err.Error(), "operation cancelled: actual cost limit exceeded") { + return "", -1, &cel.Error{ + Type: cel.ErrorTypeInvalid, + Detail: fmt.Sprintf("no further validation rules will be run due to call cost exceeds limit for messageExpression: %q", exprSrc), + } + } + return "", remainingBudget - int64(*rtCost), &cel.Error{ + Detail: fmt.Sprintf("messageExpression evaluation failed due to: %v", err.Error()), + } + } + messageStr, ok := evalResult.Value().(string) + if !ok { + return "", remainingBudget - int64(*rtCost), &cel.Error{ + Detail: "messageExpression failed to convert to string", + } + } + trimmedMsgStr := strings.TrimSpace(messageStr) + if len(trimmedMsgStr) > celconfig.MaxEvaluatedMessageExpressionSizeBytes { + return "", remainingBudget - int64(*rtCost), &cel.Error{ + Detail: fmt.Sprintf("messageExpression beyond allowable length of %d", celconfig.MaxEvaluatedMessageExpressionSizeBytes), + } + } else if hasNewlines(trimmedMsgStr) { + return "", remainingBudget - int64(*rtCost), &cel.Error{ + Detail: "messageExpression should not contain line breaks", + } + } else if len(trimmedMsgStr) == 0 { + return "", remainingBudget - int64(*rtCost), &cel.Error{ + Detail: "messageExpression should evaluate to a non-empty string", + } + } + return trimmedMsgStr, remainingBudget - int64(*rtCost), nil +} + +var newlineMatcher = regexp.MustCompile(`[\n]+`) + +func hasNewlines(s string) bool { + return newlineMatcher.MatchString(s) +} + +func ruleMessageOrDefault(rule apiextensions.ValidationRule) string { + if len(rule.Message) == 0 { + return fmt.Sprintf("failed rule: %s", ruleErrorString(rule)) + } else { + return strings.TrimSpace(rule.Message) + } +} + func ruleErrorString(rule apiextensions.ValidationRule) string { if len(rule.Message) > 0 { return strings.TrimSpace(rule.Message) 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 e1c885d9b7e..86a9313e410 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 @@ -18,12 +18,14 @@ package cel import ( "context" + "flag" "fmt" "math" "strings" "testing" "time" + "k8s.io/klog/v2" "k8s.io/kube-openapi/pkg/validation/strfmt" apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -2260,6 +2262,186 @@ func TestCELMaxRecursionDepth(t *testing.T) { } } +func TestMessageExpression(t *testing.T) { + klog.LogToStderr(false) + klog.InitFlags(nil) + setDefaultVerbosity(2) + defer klog.LogToStderr(true) + tests := []struct { + name string + costBudget int64 + perCallLimit uint64 + message string + messageExpression string + expectedLogErr string + expectedValidationErr string + expectedRemainingBudget int64 + }{ + { + name: "no cost error expected", + messageExpression: `"static string"`, + expectedValidationErr: "static string", + costBudget: 300, + expectedRemainingBudget: 300, + }, + { + name: "messageExpression takes precedence over message", + message: "invisible", + messageExpression: `"this is messageExpression"`, + costBudget: celconfig.RuntimeCELCostBudget, + expectedValidationErr: "this is messageExpression", + }, + { + name: "default rule message used if messageExpression does not eval to string", + messageExpression: `true`, + costBudget: celconfig.RuntimeCELCostBudget, + expectedValidationErr: "failed rule", + expectedRemainingBudget: celconfig.RuntimeCELCostBudget, + }, + { + name: "limit exceeded", + messageExpression: `"string 1" + "string 2" + "string 3"`, + costBudget: 1, + expectedValidationErr: "messageExpression evaluation failed due to running out of cost budget", + expectedRemainingBudget: -1, + }, + { + name: "messageExpression budget (str concat)", + messageExpression: `"str1 " + self.str`, + costBudget: 50, + expectedValidationErr: "str1 a string", + expectedRemainingBudget: 46, + }, + { + name: "runtime cost preserved if messageExpression fails during evaluation", + message: "message not messageExpression", + messageExpression: `"str1 " + ["a", "b", "c", "d"][4]`, + costBudget: 50, + expectedLogErr: "messageExpression evaluation failed due to: index '4' out of range in list size '4'", + expectedValidationErr: "message not messageExpression", + expectedRemainingBudget: 47, + }, + { + name: "runtime cost preserved if messageExpression fails during evaluation (no message set)", + messageExpression: `"str1 " + ["a", "b", "c", "d"][4]`, + costBudget: 50, + expectedLogErr: "messageExpression evaluation failed due to: index '4' out of range in list size '4'", + expectedValidationErr: "failed rule", + expectedRemainingBudget: 47, + }, + { + name: "per-call limit exceeded during messageExpression execution", + messageExpression: `"string 1" + "string 2" + "string 3"`, + costBudget: celconfig.RuntimeCELCostBudget, + perCallLimit: 1, + expectedValidationErr: "call cost exceeds limit for messageExpression", + expectedRemainingBudget: -1, + }, + { + name: "messageExpression is not allowed to generate a string with newlines", + message: "message not messageExpression", + messageExpression: `"str with \na newline"`, + costBudget: celconfig.RuntimeCELCostBudget, + expectedLogErr: "messageExpression should not contain line breaks", + expectedValidationErr: "message not messageExpression", + }, + { + name: "messageExpression is not allowed to generate messages >5000 characters", + message: "message not messageExpression", + messageExpression: fmt.Sprintf(`"%s"`, genString(5121, 'a')), + costBudget: celconfig.RuntimeCELCostBudget, + expectedLogErr: "messageExpression beyond allowable length of 5120", + expectedValidationErr: "message not messageExpression", + }, + { + name: "messageExpression is not allowed to generate an empty string", + message: "message not messageExpression", + messageExpression: `string("")`, + costBudget: celconfig.RuntimeCELCostBudget, + expectedLogErr: "messageExpression should evaluate to a non-empty string", + expectedValidationErr: "message not messageExpression", + }, + { + name: "messageExpression is not allowed to generate a string with only spaces", + message: "message not messageExpression", + messageExpression: `string(" ")`, + costBudget: celconfig.RuntimeCELCostBudget, + expectedLogErr: "messageExpression should evaluate to a non-empty string", + expectedValidationErr: "message not messageExpression", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + outputBuffer := strings.Builder{} + klog.SetOutput(&outputBuffer) + + ctx := context.TODO() + var s schema.Structural + if tt.message != "" { + s = withRuleMessageAndMessageExpression(objectType(map[string]schema.Structural{ + "str": stringType}), "false", tt.message, tt.messageExpression) + } else { + s = withRuleAndMessageExpression(objectType(map[string]schema.Structural{ + "str": stringType}), "false", tt.messageExpression) + } + obj := map[string]interface{}{ + "str": "a string", + } + + callLimit := uint64(celconfig.PerCallLimit) + if tt.perCallLimit != 0 { + callLimit = tt.perCallLimit + } + celValidator := NewValidator(&s, false, callLimit) + if celValidator == nil { + t.Fatal("expected non nil validator") + } + errs, remainingBudget := celValidator.Validate(ctx, field.NewPath("root"), &s, obj, nil, tt.costBudget) + klog.Flush() + + if len(errs) != 1 { + t.Fatalf("expected 1 error, got %d", len(errs)) + } + + if tt.expectedLogErr != "" { + if !strings.Contains(outputBuffer.String(), tt.expectedLogErr) { + t.Fatalf("did not find expected log error message: %q\n%q", tt.expectedLogErr, outputBuffer.String()) + } + } else if tt.expectedLogErr == "" && outputBuffer.String() != "" { + t.Fatalf("expected no log output, got: %q", outputBuffer.String()) + } + + if tt.expectedValidationErr != "" { + if !strings.Contains(errs[0].Error(), tt.expectedValidationErr) { + t.Fatalf("did not find expected validation error message: %q", tt.expectedValidationErr) + } + } + + if tt.expectedRemainingBudget != 0 { + if tt.expectedRemainingBudget != remainingBudget { + t.Fatalf("expected %d cost left, got %d", tt.expectedRemainingBudget, remainingBudget) + } + } + }) + } +} + +func genString(n int, c rune) string { + b := strings.Builder{} + for i := 0; i < n; i++ { + _, err := b.WriteRune(c) + if err != nil { + panic(err) + } + } + return b.String() +} + +func setDefaultVerbosity(v int) { + f := flag.CommandLine.Lookup("v") + _ = f.Value.Set(fmt.Sprintf("%d", v)) +} + func BenchmarkCELValidationWithContext(b *testing.B) { items := make([]interface{}, 1000) for i := int64(0); i < 1000; i++ { @@ -2534,6 +2716,27 @@ func withRule(s schema.Structural, rule string) schema.Structural { return s } +func withRuleMessageAndMessageExpression(s schema.Structural, rule, message, messageExpression string) schema.Structural { + s.Extensions.XValidations = apiextensions.ValidationRules{ + { + Rule: rule, + Message: message, + MessageExpression: messageExpression, + }, + } + return s +} + +func withRuleAndMessageExpression(s schema.Structural, rule, messageExpression string) schema.Structural { + s.Extensions.XValidations = apiextensions.ValidationRules{ + { + Rule: rule, + MessageExpression: messageExpression, + }, + } + return s +} + func withRulePtr(s *schema.Structural, rule string) *schema.Structural { s.Extensions.XValidations = apiextensions.ValidationRules{ { diff --git a/staging/src/k8s.io/apiserver/pkg/apis/cel/config.go b/staging/src/k8s.io/apiserver/pkg/apis/cel/config.go index 4e66868c14e..d05821899de 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/cel/config.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/cel/config.go @@ -33,4 +33,8 @@ const ( // TODO(DangerOnTheRanger): wire in MaxRequestBodyBytes from apiserver/pkg/server/options/server_run_options.go to make this configurable // Note that even if server_run_options.go becomes configurable in the future, this cost constant should be fixed and it should be the max allowed request size for the server MaxRequestSizeBytes = int64(3 * 1024 * 1024) + + // MaxEvaluatedMessageExpressionSizeBytes represents the largest-allowable string generated + // by a messageExpression field + MaxEvaluatedMessageExpressionSizeBytes = 5 * 1024 ) diff --git a/test/integration/apiserver/crd_validation_expressions_test.go b/test/integration/apiserver/crd_validation_expressions_test.go index 5824f2f3a69..9d1848e0969 100644 --- a/test/integration/apiserver/crd_validation_expressions_test.go +++ b/test/integration/apiserver/crd_validation_expressions_test.go @@ -523,8 +523,8 @@ func TestCustomResourceValidatorsWithBlockingErrors(t *testing.T) { if err == nil { t.Fatal("Expected create of invalid custom resource to fail") } else { - if !strings.Contains(err.Error(), "failed rule: self.spec.x + self.spec.y") { - t.Fatalf("Expected error to contain %s but got %v", "failed rule: self.spec.x + self.spec.y", err.Error()) + if !strings.Contains(err.Error(), "self.spec.x + self.spec.y must be greater than or equal to 0") { + t.Fatalf("Expected error to contain %s but got %v", "self.spec.x + self.spec.y must be greater than or equal to 0", err.Error()) } } }) @@ -837,7 +837,8 @@ var structuralSchemaWithBlockingErr = []byte(` "type": "object", "x-kubernetes-validations": [ { - "rule": "self.spec.x + self.spec.y >= (has(self.status) ? self.status.z : 0)" + "rule": "self.spec.x + self.spec.y >= (has(self.status) ? self.status.z : 0)", + "messageExpression": "\"self.spec.x + self.spec.y must be greater than or equal to 0\"" } ], "properties": {