Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CEL runtime cost into CR validation #108482

Merged
merged 1 commit into from Mar 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -946,7 +946,7 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch

structural, err := structuralschema.NewStructural(schema)
if err == nil {
compResults, err := cel.Compile(structural, isRoot)
compResults, err := cel.Compile(structural, isRoot, cel.PerCallLimit)
if err != nil {
allErrs = append(allErrs, field.InternalError(fldPath.Child("x-kubernetes-validations"), err))
} else {
Expand Down
Expand Up @@ -18,6 +18,7 @@ package cel

import (
"fmt"
"math"
"strings"
"time"

Expand All @@ -40,6 +41,14 @@ const (
// OldScopedVarName is the variable name assigned to the existing value of the locally scoped data element of a
// CEL validation expression.
OldScopedVarName = "oldSelf"

// PerCallLimit specify the actual cost limit per CEL validation call
//TODO: pick the number for PerCallLimit
PerCallLimit = uint64(math.MaxInt64)

// RuntimeCELCostBudget is the overall cost budget for runtime CEL validation cost per CustomResource
//TODO: pick the RuntimeCELCostBudget
RuntimeCELCostBudget = math.MaxInt64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can/should be in a follow-up, but we want to make sure that any names or magic numbers that are API-facing (meaning changing them would make existing persisted CRDs fail validation) need to be clearly marked that way and live in a package that triggers API review if changed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing out. I am not quite sure if CustomResource validation related field is also part of API review(ref doc)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's part of our API surface, falls under "validation (pkg/apis/*/validation.go or OpenAPI for CRDs)"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I have moved it under k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation to trigger api review. Please let me know if other place is preferred^-^

Copy link
Member

@liggitt liggitt Mar 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to minimize churn, I'm happy for a no-op move to happen in a follow-up if you want to save the Move RuntimeCELCostBudget under api review commit for the next PR ... this seemed like it was close otherwise

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I will do it in the follow up PR since I still need to sort out some dependency issue in terms of where to put it. It would be great if we could get this one merged first :)

)

// CompilationResult represents the cel compilation result for one rule
Expand All @@ -58,7 +67,8 @@ type CompilationResult struct {
/// - 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
func Compile(s *schema.Structural, isResourceRoot bool) ([]CompilationResult, error) {
// 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) {
if len(s.Extensions.XValidations) == 0 {
return nil, nil
}
Expand Down Expand Up @@ -106,13 +116,13 @@ func Compile(s *schema.Structural, isResourceRoot bool) ([]CompilationResult, er
// 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))
for i, rule := range celRules {
compResults[i] = compileRule(rule, env)
compResults[i] = compileRule(rule, env, perCallLimit)
}

return compResults, nil
}

func compileRule(rule apiextensions.ValidationRule, env *cel.Env) (compilationResult CompilationResult) {
func compileRule(rule apiextensions.ValidationRule, env *cel.Env, perCallLimit 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
Expand Down Expand Up @@ -141,7 +151,8 @@ func compileRule(rule apiextensions.ValidationRule, env *cel.Env) (compilationRe
}
}

prog, err := env.Program(ast, cel.EvalOptions(cel.OptOptimize))
// TODO: Ideally we could configure the per expression limit at validation time and set it to the remaining overall budget, but we would either need a way to pass in a limit at evaluation time or move program creation to validation time
prog, err := env.Program(ast, cel.EvalOptions(cel.OptOptimize, cel.OptTrackCost), cel.CostLimit(perCallLimit))
if err != nil {
compilationResult.Error = &Error{ErrorTypeInvalid, "program instantiation failed: " + err.Error()}
return
Expand Down
Expand Up @@ -637,7 +637,7 @@ func TestCelCompilation(t *testing.T) {

for _, tt := range cases {
t.Run(tt.name, func(t *testing.T) {
compilationResults, err := Compile(&tt.input, false)
compilationResults, err := Compile(&tt.input, false, PerCallLimit)
if err != nil {
t.Errorf("Expected no error, but got: %v", err)
}
Expand Down
Expand Up @@ -18,6 +18,7 @@ package cel

import (
"fmt"
"math"
"strings"

"github.com/google/cel-go/common/types"
Expand Down Expand Up @@ -55,27 +56,28 @@ type Validator struct {
// validators for all items, properties and additionalProperties that transitively contain validator rules.
// Returns nil only if there no validator rules in the Structural schema. May return a validator containing
// only errors.
func NewValidator(s *schema.Structural) *Validator {
return validator(s, true)
// 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 validator(s *schema.Structural, isResourceRoot bool) *Validator {
compiledRules, err := Compile(s, isResourceRoot)
func validator(s *schema.Structural, isResourceRoot bool, perCallLimit uint64) *Validator {
compiledRules, err := Compile(s, isResourceRoot, perCallLimit)
var itemsValidator, additionalPropertiesValidator *Validator
var propertiesValidators map[string]Validator
if s.Items != nil {
itemsValidator = validator(s.Items, s.Items.XEmbeddedResource)
itemsValidator = validator(s.Items, s.Items.XEmbeddedResource, 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); p != nil {
if p := validator(&prop, prop.XEmbeddedResource, perCallLimit); p != nil {
propertiesValidators[k] = *p
}
}
}
if s.AdditionalProperties != nil && s.AdditionalProperties.Structural != nil {
additionalPropertiesValidator = validator(s.AdditionalProperties.Structural, s.AdditionalProperties.Structural.XEmbeddedResource)
additionalPropertiesValidator = validator(s.AdditionalProperties.Structural, s.AdditionalProperties.Structural.XEmbeddedResource, perCallLimit)
}
if len(compiledRules) > 0 || err != nil || itemsValidator != nil || additionalPropertiesValidator != nil || len(propertiesValidators) > 0 {
return &Validator{
Expand All @@ -92,34 +94,51 @@ func validator(s *schema.Structural, isResourceRoot bool) *Validator {
}

// Validate validates all x-kubernetes-validations rules in Validator against obj and returns any errors.
liggitt marked this conversation as resolved.
Show resolved Hide resolved
func (s *Validator) Validate(fldPath *field.Path, sts *schema.Structural, obj interface{}) field.ErrorList {
// If the validation rules exceed the costBudget, subsequent evaluations will be skipped, the list of errs returned will not be empty, and a negative remainingBudget will be returned.
// Most callers can ignore the returned remainingBudget value unless another validate call is going to be made
func (s *Validator) Validate(fldPath *field.Path, sts *schema.Structural, obj interface{}, costBudget int64) (errs field.ErrorList, remainingBudget int64) {
remainingBudget = costBudget
if s == nil || obj == nil {
return nil
return nil, remainingBudget
}

errs := s.validateExpressions(fldPath, sts, obj)
errs, remainingBudget = s.validateExpressions(fldPath, sts, obj, remainingBudget)
if remainingBudget < 0 {
return errs, remainingBudget
liggitt marked this conversation as resolved.
Show resolved Hide resolved
}
switch obj := obj.(type) {
case []interface{}:
return append(errs, s.validateArray(fldPath, sts, obj)...)
var arrayErrs field.ErrorList
arrayErrs, remainingBudget = s.validateArray(fldPath, sts, obj, remainingBudget)
errs = append(errs, arrayErrs...)
return errs, remainingBudget
case map[string]interface{}:
return append(errs, s.validateMap(fldPath, sts, obj)...)
var mapErrs field.ErrorList
mapErrs, remainingBudget = s.validateMap(fldPath, sts, obj, remainingBudget)
errs = append(errs, mapErrs...)
return errs, remainingBudget
}
return errs
return errs, remainingBudget
}

func (s *Validator) validateExpressions(fldPath *field.Path, sts *schema.Structural, obj interface{}) (errs field.ErrorList) {
func (s *Validator) validateExpressions(fldPath *field.Path, sts *schema.Structural, obj interface{}, costBudget int64) (errs field.ErrorList, remainingBudget int64) {
remainingBudget = costBudget
if obj == nil {
// We only validate non-null values. Rules that need to check for the state of a nullable value or the presence of an optional
// field must do so from the surrounding schema. E.g. if an array has nullable string items, a rule on the array
// schema can check if items are null, but a rule on the nullable string schema only validates the non-null strings.
return nil
return nil, remainingBudget
}
if s.compilationErr != nil {
errs = append(errs, field.Invalid(fldPath, obj, fmt.Sprintf("rule compiler initialization error: %v", s.compilationErr)))
return errs
return errs, remainingBudget
}
if len(s.compiledRules) == 0 {
return nil // nothing to do
return nil, remainingBudget // nothing to do
}
if remainingBudget <= 0 {
errs = append(errs, field.Invalid(fldPath, obj, fmt.Sprintf("validation failed due to running out of cost budget, no further validation rules will be run")))
return errs, -1
}
if s.isResourceRoot {
sts = model.WithTypeAndObjectMeta(sts)
Expand All @@ -140,7 +159,23 @@ func (s *Validator) validateExpressions(fldPath *field.Path, sts *schema.Structu
errs = append(errs, field.InternalError(fldPath, fmt.Errorf("oldSelf validation not implemented")))
continue // todo: wire oldObj parameter
}
cici37 marked this conversation as resolved.
Show resolved Hide resolved
evalResult, _, err := compiled.Program.Eval(activation)
evalResult, evalDetails, err := compiled.Program.Eval(activation)
cici37 marked this conversation as resolved.
Show resolved Hide resolved
if evalDetails == nil {
errs = append(errs, field.InternalError(fldPath, fmt.Errorf("runtime cost could not be calculated for validation rule: %v, no further validation rules will be run", ruleErrorString(rule))))
return errs, -1
} else {
rtCost := evalDetails.ActualCost()
if rtCost == nil {
errs = append(errs, field.Invalid(fldPath, obj, fmt.Sprintf("runtime cost could not be calculated for validation rule: %v, no further validation rules will be run", ruleErrorString(rule))))
return errs, -1
} else {
if *rtCost > math.MaxInt64 || int64(*rtCost) > remainingBudget {
errs = append(errs, field.Invalid(fldPath, obj, fmt.Sprintf("validation failed due to running out of cost budget, no further validation rules will be run")))
return errs, -1
}
remainingBudget -= int64(*rtCost)
}
}
if err != nil {
// see types.Err for list of well defined error types
if strings.HasPrefix(err.Error(), "no such overload") {
Expand All @@ -149,12 +184,15 @@ func (s *Validator) validateExpressions(fldPath *field.Path, sts *schema.Structu
// append a more descriptive error message. This error can only occur when static type checking has
// been bypassed. int-or-string is typed as dynamic and so bypasses compiler type checking.
errs = append(errs, field.Invalid(fldPath, obj, fmt.Sprintf("'%v': call arguments did not match a supported operator, function or macro signature for rule: %v", err, ruleErrorString(rule))))
} else if strings.HasPrefix(err.Error(), "operation cancelled: actual cost limit exceeded") {
cici37 marked this conversation as resolved.
Show resolved Hide resolved
errs = append(errs, field.Invalid(fldPath, obj, fmt.Sprintf("'%v': call cost exceeds limit for rule: %v", err, ruleErrorString(rule))))
} else {
// no such key: {key}, index out of bounds: {index}, integer overflow, division by zero, ...
errs = append(errs, field.Invalid(fldPath, obj, fmt.Sprintf("%v evaluating rule: %v", err, ruleErrorString(rule))))
}
continue
liggitt marked this conversation as resolved.
Show resolved Hide resolved
}

if evalResult != types.True {
if len(rule.Message) != 0 {
errs = append(errs, field.Invalid(fldPath, obj, rule.Message))
Expand All @@ -163,7 +201,7 @@ func (s *Validator) validateExpressions(fldPath *field.Path, sts *schema.Structu
}
}
}
return errs
return errs, remainingBudget
}

func ruleErrorString(rule apiextensions.ValidationRule) string {
Expand Down Expand Up @@ -192,37 +230,58 @@ func (a *validationActivation) Parent() interpreter.Activation {
return nil
}

func (s *Validator) validateMap(fldPath *field.Path, sts *schema.Structural, obj map[string]interface{}) (errs field.ErrorList) {
func (s *Validator) validateMap(fldPath *field.Path, sts *schema.Structural, obj map[string]interface{}, costBudget int64) (errs field.ErrorList, remainingBudget int64) {
remainingBudget = costBudget
if remainingBudget < 0 {
return errs, remainingBudget
cici37 marked this conversation as resolved.
Show resolved Hide resolved
}
if s == nil || obj == nil {
return nil
return nil, remainingBudget
}

if s.AdditionalProperties != nil && sts.AdditionalProperties != nil && sts.AdditionalProperties.Structural != nil {
for k, v := range obj {
errs = append(errs, s.AdditionalProperties.Validate(fldPath.Key(k), sts.AdditionalProperties.Structural, v)...)
var err field.ErrorList
err, remainingBudget = s.AdditionalProperties.Validate(fldPath.Key(k), sts.AdditionalProperties.Structural, v, remainingBudget)
errs = append(errs, err...)
if remainingBudget < 0 {
return errs, remainingBudget
}
}
}
if s.Properties != nil && sts.Properties != nil {
for k, v := range obj {
stsProp, stsOk := sts.Properties[k]
sub, ok := s.Properties[k]
if ok && stsOk {
errs = append(errs, sub.Validate(fldPath.Child(k), &stsProp, v)...)
var err field.ErrorList
err, remainingBudget = sub.Validate(fldPath.Child(k), &stsProp, v, remainingBudget)
errs = append(errs, err...)
if remainingBudget < 0 {
return errs, remainingBudget
}
}
}
}

return errs
return errs, remainingBudget
}

func (s *Validator) validateArray(fldPath *field.Path, sts *schema.Structural, obj []interface{}) field.ErrorList {
var errs field.ErrorList

func (s *Validator) validateArray(fldPath *field.Path, sts *schema.Structural, obj []interface{}, costBudget int64) (errs field.ErrorList, remainingBudget int64) {
remainingBudget = costBudget
if remainingBudget < 0 {
return errs, remainingBudget
}
if s.Items != nil && sts.Items != nil {
for i := range obj {
errs = append(errs, s.Items.Validate(fldPath.Index(i), sts.Items, obj[i])...)
var err field.ErrorList
err, remainingBudget = s.Items.Validate(fldPath.Index(i), sts.Items, obj[i], remainingBudget)
errs = append(errs, err...)
if remainingBudget < 0 {
return errs, remainingBudget
}
}
}

return errs
return errs, remainingBudget
}