Merge pull request #129749 from pohly/dra-cel-cost-estimate-during-validation

DRA:  CEL cost estimate during validation
This commit is contained in:
Kubernetes Prow Robot 2025-01-23 15:53:27 -08:00 committed by GitHub
commit 9635a253da
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 26 additions and 10 deletions

View File

@ -43,6 +43,8 @@ func NewCache(maxCacheEntries int) *Cache {
// GetOrCompile checks whether the cache already has a compilation result
// and returns that if available. Otherwise it compiles, stores successful
// results and returns the new result.
//
// Cost estimation is disabled.
func (c *Cache) GetOrCompile(expression string) CompilationResult {
// Compiling a CEL expression is expensive enough that it is cheaper
// to lock a mutex than doing it several times in parallel.
@ -55,7 +57,7 @@ func (c *Cache) GetOrCompile(expression string) CompilationResult {
return *cached
}
expr := GetCompiler().CompileCELExpression(expression, Options{})
expr := GetCompiler().CompileCELExpression(expression, Options{DisableCostEstimation: true})
if expr.Error == nil {
c.add(expression, &expr)
}

View File

@ -18,6 +18,7 @@ package cel
import (
"fmt"
"math"
"sync"
"testing"
@ -73,6 +74,11 @@ func TestCacheSemantic(t *testing.T) {
if resultFalse == resultFalseAgain {
t.Fatal("result of compiling `false` should have been evicted from the cache")
}
// Cost estimation must be off (not needed by scheduler).
if resultFalseAgain.MaxCost != math.MaxUint64 {
t.Error("cost estimation should have been disabled, was enabled")
}
}
func TestCacheConcurrency(t *testing.T) {

View File

@ -20,6 +20,7 @@ import (
"context"
"errors"
"fmt"
"math"
"reflect"
"strings"
"sync"
@ -120,6 +121,10 @@ type Options struct {
// CostLimit allows overriding the default runtime cost limit [resourceapi.CELSelectorExpressionMaxCost].
CostLimit *uint64
// DisableCostEstimation can be set to skip estimating the worst-case CEL cost.
// If disabled or after an error, [CompilationResult.MaxCost] will be set to [math.Uint64].
DisableCostEstimation bool
}
// CompileCELExpression returns a compiled CEL expression. It evaluates to bool.
@ -133,6 +138,7 @@ func (c compiler) CompileCELExpression(expression string, options Options) Compi
Detail: errorString,
},
Expression: expression,
MaxCost: math.MaxUint64,
}
}
@ -141,10 +147,6 @@ func (c compiler) CompileCELExpression(expression string, options Options) Compi
return resultError(fmt.Sprintf("unexpected error loading CEL environment: %v", err), apiservercel.ErrorTypeInternal)
}
// We don't have a SizeEstimator. The potential size of the input (= a
// device) is already declared in the definition of the environment.
estimator := c.newCostEstimator()
ast, issues := env.Compile(expression)
if issues != nil {
return resultError("compilation failed: "+issues.String(), apiservercel.ErrorTypeInvalid)
@ -176,15 +178,21 @@ func (c compiler) CompileCELExpression(expression string, options Options) Compi
OutputType: ast.OutputType(),
Environment: env,
emptyMapVal: env.CELTypeAdapter().NativeToValue(map[string]any{}),
MaxCost: math.MaxUint64,
}
costEst, err := env.EstimateCost(ast, estimator)
if err != nil {
compilationResult.Error = &apiservercel.Error{Type: apiservercel.ErrorTypeInternal, Detail: "cost estimation failed: " + err.Error()}
return compilationResult
if !options.DisableCostEstimation {
// We don't have a SizeEstimator. The potential size of the input (= a
// device) is already declared in the definition of the environment.
estimator := c.newCostEstimator()
costEst, err := env.EstimateCost(ast, estimator)
if err != nil {
compilationResult.Error = &apiservercel.Error{Type: apiservercel.ErrorTypeInternal, Detail: "cost estimation failed: " + err.Error()}
return compilationResult
}
compilationResult.MaxCost = costEst.Max
}
compilationResult.MaxCost = costEst.Max
return compilationResult
}