Merge pull request #128101 from pohly/dra-api-cel-cost-limit

DRA API: implement CEL cost limit
This commit is contained in:
Kubernetes Prow Robot 2024-10-26 20:18:52 +01:00 committed by GitHub
commit 3690cb7f9a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 432 additions and 163 deletions

View File

@ -15259,7 +15259,7 @@
"description": "CELDeviceSelector contains a CEL expression for selecting a device.",
"properties": {
"expression": {
"description": "Expression is a CEL expression which evaluates a single device. It must evaluate to true when the device under consideration satisfies the desired criteria, and false when it does not. Any other result is an error and causes allocation of devices to abort.\n\nThe expression's input is an object named \"device\", which carries the following properties:\n - driver (string): the name of the driver which defines this device.\n - attributes (map[string]object): the device's attributes, grouped by prefix\n (e.g. device.attributes[\"dra.example.com\"] evaluates to an object with all\n of the attributes which were prefixed by \"dra.example.com\".\n - capacity (map[string]object): the device's capacities, grouped by prefix.\n\nExample: Consider a device with driver=\"dra.example.com\", which exposes two attributes named \"model\" and \"ext.example.com/family\" and which exposes one capacity named \"modules\". This input to this expression would have the following fields:\n\n device.driver\n device.attributes[\"dra.example.com\"].model\n device.attributes[\"ext.example.com\"].family\n device.capacity[\"dra.example.com\"].modules\n\nThe device.driver field can be used to check for a specific driver, either as a high-level precondition (i.e. you only want to consider devices from this driver) or as part of a multi-clause expression that is meant to consider devices from different drivers.\n\nThe value type of each attribute is defined by the device definition, and users who write these expressions must consult the documentation for their specific drivers. The value type of each capacity is Quantity.\n\nIf an unknown prefix is used as a lookup in either device.attributes or device.capacity, an empty map will be returned. Any reference to an unknown field will cause an evaluation error and allocation to abort.\n\nA robust expression should check for the existence of attributes before referencing them.\n\nFor ease of use, the cel.bind() function is enabled, and can be used to simplify expressions that access multiple attributes with the same domain. For example:\n\n cel.bind(dra, device.attributes[\"dra.example.com\"], dra.someBool && dra.anotherBool)",
"description": "Expression is a CEL expression which evaluates a single device. It must evaluate to true when the device under consideration satisfies the desired criteria, and false when it does not. Any other result is an error and causes allocation of devices to abort.\n\nThe expression's input is an object named \"device\", which carries the following properties:\n - driver (string): the name of the driver which defines this device.\n - attributes (map[string]object): the device's attributes, grouped by prefix\n (e.g. device.attributes[\"dra.example.com\"] evaluates to an object with all\n of the attributes which were prefixed by \"dra.example.com\".\n - capacity (map[string]object): the device's capacities, grouped by prefix.\n\nExample: Consider a device with driver=\"dra.example.com\", which exposes two attributes named \"model\" and \"ext.example.com/family\" and which exposes one capacity named \"modules\". This input to this expression would have the following fields:\n\n device.driver\n device.attributes[\"dra.example.com\"].model\n device.attributes[\"ext.example.com\"].family\n device.capacity[\"dra.example.com\"].modules\n\nThe device.driver field can be used to check for a specific driver, either as a high-level precondition (i.e. you only want to consider devices from this driver) or as part of a multi-clause expression that is meant to consider devices from different drivers.\n\nThe value type of each attribute is defined by the device definition, and users who write these expressions must consult the documentation for their specific drivers. The value type of each capacity is Quantity.\n\nIf an unknown prefix is used as a lookup in either device.attributes or device.capacity, an empty map will be returned. Any reference to an unknown field will cause an evaluation error and allocation to abort.\n\nA robust expression should check for the existence of attributes before referencing them.\n\nFor ease of use, the cel.bind() function is enabled, and can be used to simplify expressions that access multiple attributes with the same domain. For example:\n\n cel.bind(dra, device.attributes[\"dra.example.com\"], dra.someBool && dra.anotherBool)\n\nThe length of the expression must be smaller or equal to 10 Ki. The cost of evaluating it is also limited based on the estimated number of logical steps.",
"type": "string"
}
},

View File

@ -139,7 +139,7 @@
"properties": {
"expression": {
"default": "",
"description": "Expression is a CEL expression which evaluates a single device. It must evaluate to true when the device under consideration satisfies the desired criteria, and false when it does not. Any other result is an error and causes allocation of devices to abort.\n\nThe expression's input is an object named \"device\", which carries the following properties:\n - driver (string): the name of the driver which defines this device.\n - attributes (map[string]object): the device's attributes, grouped by prefix\n (e.g. device.attributes[\"dra.example.com\"] evaluates to an object with all\n of the attributes which were prefixed by \"dra.example.com\".\n - capacity (map[string]object): the device's capacities, grouped by prefix.\n\nExample: Consider a device with driver=\"dra.example.com\", which exposes two attributes named \"model\" and \"ext.example.com/family\" and which exposes one capacity named \"modules\". This input to this expression would have the following fields:\n\n device.driver\n device.attributes[\"dra.example.com\"].model\n device.attributes[\"ext.example.com\"].family\n device.capacity[\"dra.example.com\"].modules\n\nThe device.driver field can be used to check for a specific driver, either as a high-level precondition (i.e. you only want to consider devices from this driver) or as part of a multi-clause expression that is meant to consider devices from different drivers.\n\nThe value type of each attribute is defined by the device definition, and users who write these expressions must consult the documentation for their specific drivers. The value type of each capacity is Quantity.\n\nIf an unknown prefix is used as a lookup in either device.attributes or device.capacity, an empty map will be returned. Any reference to an unknown field will cause an evaluation error and allocation to abort.\n\nA robust expression should check for the existence of attributes before referencing them.\n\nFor ease of use, the cel.bind() function is enabled, and can be used to simplify expressions that access multiple attributes with the same domain. For example:\n\n cel.bind(dra, device.attributes[\"dra.example.com\"], dra.someBool && dra.anotherBool)",
"description": "Expression is a CEL expression which evaluates a single device. It must evaluate to true when the device under consideration satisfies the desired criteria, and false when it does not. Any other result is an error and causes allocation of devices to abort.\n\nThe expression's input is an object named \"device\", which carries the following properties:\n - driver (string): the name of the driver which defines this device.\n - attributes (map[string]object): the device's attributes, grouped by prefix\n (e.g. device.attributes[\"dra.example.com\"] evaluates to an object with all\n of the attributes which were prefixed by \"dra.example.com\".\n - capacity (map[string]object): the device's capacities, grouped by prefix.\n\nExample: Consider a device with driver=\"dra.example.com\", which exposes two attributes named \"model\" and \"ext.example.com/family\" and which exposes one capacity named \"modules\". This input to this expression would have the following fields:\n\n device.driver\n device.attributes[\"dra.example.com\"].model\n device.attributes[\"ext.example.com\"].family\n device.capacity[\"dra.example.com\"].modules\n\nThe device.driver field can be used to check for a specific driver, either as a high-level precondition (i.e. you only want to consider devices from this driver) or as part of a multi-clause expression that is meant to consider devices from different drivers.\n\nThe value type of each attribute is defined by the device definition, and users who write these expressions must consult the documentation for their specific drivers. The value type of each capacity is Quantity.\n\nIf an unknown prefix is used as a lookup in either device.attributes or device.capacity, an empty map will be returned. Any reference to an unknown field will cause an evaluation error and allocation to abort.\n\nA robust expression should check for the existence of attributes before referencing them.\n\nFor ease of use, the cel.bind() function is enabled, and can be used to simplify expressions that access multiple attributes with the same domain. For example:\n\n cel.bind(dra, device.attributes[\"dra.example.com\"], dra.someBool && dra.anotherBool)\n\nThe length of the expression must be smaller or equal to 10 Ki. The cost of evaluating it is also limited based on the estimated number of logical steps.",
"type": "string"
}
},

View File

@ -517,10 +517,42 @@ type CELDeviceSelector struct {
//
// cel.bind(dra, device.attributes["dra.example.com"], dra.someBool && dra.anotherBool)
//
// The length of the expression must be smaller or equal to 10 Ki. The
// cost of evaluating it is also limited based on the estimated number
// of logical steps.
//
// +required
Expression string
}
// CELSelectorExpressionMaxCost specifies the cost limit for a single CEL selector
// evaluation.
//
// There is no overall budget for selecting a device, so the actual time
// required for that is proportional to the number of CEL selectors and how
// often they need to be evaluated, which can vary depending on several factors
// (number of devices, cluster utilization, additional constraints).
//
// Validation against this limit and [CELSelectorExpressionMaxLength] happens
// only when setting an expression for the first time or when changing it. If
// the limits are changed in a future Kubernetes release, existing users are
// guaranteed that existing expressions will continue to be valid.
//
// However, the kube-scheduler also applies this cost limit at runtime, so it
// could happen that a valid expression fails at runtime after an up- or
// downgrade. This can also happen without version skew when the cost estimate
// underestimated the actual cost. That this might happen is the reason why
// kube-scheduler enforces the runtime limit instead of relying on validation.
//
// According to
// https://github.com/kubernetes/kubernetes/blob/4aeaf1e99e82da8334c0d6dddd848a194cd44b4f/staging/src/k8s.io/apiserver/pkg/apis/cel/config.go#L20-L22,
// this gives roughly 0.1 second for each expression evaluation.
// However, this depends on how fast the machine is.
const CELSelectorExpressionMaxCost = 1000000
// CELSelectorExpressionMaxLength is the maximum length of a CEL selector expression string.
const CELSelectorExpressionMaxLength = 10 * 1024
// DeviceConstraint must have exactly one field set besides Requests.
type DeviceConstraint struct {
// Requests is a list of the one or more requests in this claim which

View File

@ -167,10 +167,19 @@ func validateCELSelector(celSelector resource.CELDeviceSelector, fldPath *field.
if stored {
envType = environment.StoredExpressions
}
result := dracel.GetCompiler().CompileCELExpression(celSelector.Expression, envType)
if len(celSelector.Expression) > resource.CELSelectorExpressionMaxLength {
allErrs = append(allErrs, field.TooLongMaxLength(fldPath.Child("expression"), "<value omitted>", resource.CELSelectorExpressionMaxLength))
// Don't bother compiling too long expressions.
return allErrs
}
result := dracel.GetCompiler().CompileCELExpression(celSelector.Expression, dracel.Options{EnvType: &envType})
if result.Error != nil {
allErrs = append(allErrs, convertCELErrorToValidationError(fldPath.Child("expression"), celSelector.Expression, result.Error))
} else if result.MaxCost > resource.CELSelectorExpressionMaxCost {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("expression"), "too complex, exceeds cost limit"))
}
return allErrs
}

View File

@ -18,6 +18,7 @@ package validation
import (
"fmt"
"strings"
"testing"
"github.com/stretchr/testify/assert"
@ -316,6 +317,48 @@ func TestValidateClaim(t *testing.T) {
return claim
}(),
},
"CEL-length": {
wantFailures: field.ErrorList{
field.TooLongMaxLength(field.NewPath("spec", "devices", "requests").Index(1).Child("selectors").Index(1).Child("cel", "expression"), "<value omitted>", resource.CELSelectorExpressionMaxLength),
},
claim: func() *resource.ResourceClaim {
claim := testClaim(goodName, goodNS, validClaimSpec)
claim.Spec.Devices.Requests = append(claim.Spec.Devices.Requests, claim.Spec.Devices.Requests[0])
claim.Spec.Devices.Requests[1].Name += "-2"
expression := `device.driver == ""`
claim.Spec.Devices.Requests[1].Selectors = []resource.DeviceSelector{
{
// Good selector.
CEL: &resource.CELDeviceSelector{
Expression: strings.ReplaceAll(expression, `""`, `"`+strings.Repeat("x", resource.CELSelectorExpressionMaxLength-len(expression))+`"`),
},
},
{
// Too long by one selector.
CEL: &resource.CELDeviceSelector{
Expression: strings.ReplaceAll(expression, `""`, `"`+strings.Repeat("x", resource.CELSelectorExpressionMaxLength-len(expression)+1)+`"`),
},
},
}
return claim
}(),
},
"CEL-cost": {
wantFailures: field.ErrorList{
field.Forbidden(field.NewPath("spec", "devices", "requests").Index(0).Child("selectors").Index(0).Child("cel", "expression"), "too complex, exceeds cost limit"),
},
claim: func() *resource.ResourceClaim {
claim := testClaim(goodName, goodNS, validClaimSpec)
claim.Spec.Devices.Requests[0].Selectors = []resource.DeviceSelector{
{
CEL: &resource.CELDeviceSelector{
Expression: `device.attributes["dra.example.com"].map(s, s.lowerAscii()).map(s, s.size()).sum() == 0`,
},
},
}
return claim
}(),
},
}
for name, scenario := range scenarios {

View File

@ -45916,7 +45916,7 @@ func schema_k8sio_api_resource_v1alpha3_CELDeviceSelector(ref common.ReferenceCa
Properties: map[string]spec.Schema{
"expression": {
SchemaProps: spec.SchemaProps{
Description: "Expression is a CEL expression which evaluates a single device. It must evaluate to true when the device under consideration satisfies the desired criteria, and false when it does not. Any other result is an error and causes allocation of devices to abort.\n\nThe expression's input is an object named \"device\", which carries the following properties:\n - driver (string): the name of the driver which defines this device.\n - attributes (map[string]object): the device's attributes, grouped by prefix\n (e.g. device.attributes[\"dra.example.com\"] evaluates to an object with all\n of the attributes which were prefixed by \"dra.example.com\".\n - capacity (map[string]object): the device's capacities, grouped by prefix.\n\nExample: Consider a device with driver=\"dra.example.com\", which exposes two attributes named \"model\" and \"ext.example.com/family\" and which exposes one capacity named \"modules\". This input to this expression would have the following fields:\n\n device.driver\n device.attributes[\"dra.example.com\"].model\n device.attributes[\"ext.example.com\"].family\n device.capacity[\"dra.example.com\"].modules\n\nThe device.driver field can be used to check for a specific driver, either as a high-level precondition (i.e. you only want to consider devices from this driver) or as part of a multi-clause expression that is meant to consider devices from different drivers.\n\nThe value type of each attribute is defined by the device definition, and users who write these expressions must consult the documentation for their specific drivers. The value type of each capacity is Quantity.\n\nIf an unknown prefix is used as a lookup in either device.attributes or device.capacity, an empty map will be returned. Any reference to an unknown field will cause an evaluation error and allocation to abort.\n\nA robust expression should check for the existence of attributes before referencing them.\n\nFor ease of use, the cel.bind() function is enabled, and can be used to simplify expressions that access multiple attributes with the same domain. For example:\n\n cel.bind(dra, device.attributes[\"dra.example.com\"], dra.someBool && dra.anotherBool)",
Description: "Expression is a CEL expression which evaluates a single device. It must evaluate to true when the device under consideration satisfies the desired criteria, and false when it does not. Any other result is an error and causes allocation of devices to abort.\n\nThe expression's input is an object named \"device\", which carries the following properties:\n - driver (string): the name of the driver which defines this device.\n - attributes (map[string]object): the device's attributes, grouped by prefix\n (e.g. device.attributes[\"dra.example.com\"] evaluates to an object with all\n of the attributes which were prefixed by \"dra.example.com\".\n - capacity (map[string]object): the device's capacities, grouped by prefix.\n\nExample: Consider a device with driver=\"dra.example.com\", which exposes two attributes named \"model\" and \"ext.example.com/family\" and which exposes one capacity named \"modules\". This input to this expression would have the following fields:\n\n device.driver\n device.attributes[\"dra.example.com\"].model\n device.attributes[\"ext.example.com\"].family\n device.capacity[\"dra.example.com\"].modules\n\nThe device.driver field can be used to check for a specific driver, either as a high-level precondition (i.e. you only want to consider devices from this driver) or as part of a multi-clause expression that is meant to consider devices from different drivers.\n\nThe value type of each attribute is defined by the device definition, and users who write these expressions must consult the documentation for their specific drivers. The value type of each capacity is Quantity.\n\nIf an unknown prefix is used as a lookup in either device.attributes or device.capacity, an empty map will be returned. Any reference to an unknown field will cause an evaluation error and allocation to abort.\n\nA robust expression should check for the existence of attributes before referencing them.\n\nFor ease of use, the cel.bind() function is enabled, and can be used to simplify expressions that access multiple attributes with the same domain. For example:\n\n cel.bind(dra, device.attributes[\"dra.example.com\"], dra.someBool && dra.anotherBool)\n\nThe length of the expression must be smaller or equal to 10 Ki. The cost of evaluating it is also limited based on the estimated number of logical steps.",
Default: "",
Type: []string{"string"},
Format: "",

View File

@ -112,6 +112,10 @@ message CELDeviceSelector {
//
// cel.bind(dra, device.attributes["dra.example.com"], dra.someBool && dra.anotherBool)
//
// The length of the expression must be smaller or equal to 10 Ki. The
// cost of evaluating it is also limited based on the estimated number
// of logical steps.
//
// +required
optional string expression = 1;
}

View File

@ -523,10 +523,42 @@ type CELDeviceSelector struct {
//
// cel.bind(dra, device.attributes["dra.example.com"], dra.someBool && dra.anotherBool)
//
// The length of the expression must be smaller or equal to 10 Ki. The
// cost of evaluating it is also limited based on the estimated number
// of logical steps.
//
// +required
Expression string `json:"expression" protobuf:"bytes,1,name=expression"`
}
// CELSelectorExpressionMaxCost specifies the cost limit for a single CEL selector
// evaluation.
//
// There is no overall budget for selecting a device, so the actual time
// required for that is proportional to the number of CEL selectors and how
// often they need to be evaluated, which can vary depending on several factors
// (number of devices, cluster utilization, additional constraints).
//
// Validation against this limit and [CELSelectorExpressionMaxLength] happens
// only when setting an expression for the first time or when changing it. If
// the limits are changed in a future Kubernetes release, existing users are
// guaranteed that existing expressions will continue to be valid.
//
// However, the kube-scheduler also applies this cost limit at runtime, so it
// could happen that a valid expression fails at runtime after an up- or
// downgrade. This can also happen without version skew when the cost estimate
// underestimated the actual cost. That this might happen is the reason why
// kube-scheduler enforces the runtime limit instead of relying on validation.
//
// According to
// https://github.com/kubernetes/kubernetes/blob/4aeaf1e99e82da8334c0d6dddd848a194cd44b4f/staging/src/k8s.io/apiserver/pkg/apis/cel/config.go#L20-L22,
// this gives roughly 0.1 second for each expression evaluation.
// However, this depends on how fast the machine is.
const CELSelectorExpressionMaxCost = 1000000
// CELSelectorExpressionMaxLength is the maximum length of a CEL selector expression string.
const CELSelectorExpressionMaxLength = 10 * 1024
// DeviceConstraint must have exactly one field set besides Requests.
type DeviceConstraint struct {
// Requests is a list of the one or more requests in this claim which

View File

@ -49,7 +49,7 @@ func (BasicDevice) SwaggerDoc() map[string]string {
var map_CELDeviceSelector = map[string]string{
"": "CELDeviceSelector contains a CEL expression for selecting a device.",
"expression": "Expression is a CEL expression which evaluates a single device. It must evaluate to true when the device under consideration satisfies the desired criteria, and false when it does not. Any other result is an error and causes allocation of devices to abort.\n\nThe expression's input is an object named \"device\", which carries the following properties:\n - driver (string): the name of the driver which defines this device.\n - attributes (map[string]object): the device's attributes, grouped by prefix\n (e.g. device.attributes[\"dra.example.com\"] evaluates to an object with all\n of the attributes which were prefixed by \"dra.example.com\".\n - capacity (map[string]object): the device's capacities, grouped by prefix.\n\nExample: Consider a device with driver=\"dra.example.com\", which exposes two attributes named \"model\" and \"ext.example.com/family\" and which exposes one capacity named \"modules\". This input to this expression would have the following fields:\n\n device.driver\n device.attributes[\"dra.example.com\"].model\n device.attributes[\"ext.example.com\"].family\n device.capacity[\"dra.example.com\"].modules\n\nThe device.driver field can be used to check for a specific driver, either as a high-level precondition (i.e. you only want to consider devices from this driver) or as part of a multi-clause expression that is meant to consider devices from different drivers.\n\nThe value type of each attribute is defined by the device definition, and users who write these expressions must consult the documentation for their specific drivers. The value type of each capacity is Quantity.\n\nIf an unknown prefix is used as a lookup in either device.attributes or device.capacity, an empty map will be returned. Any reference to an unknown field will cause an evaluation error and allocation to abort.\n\nA robust expression should check for the existence of attributes before referencing them.\n\nFor ease of use, the cel.bind() function is enabled, and can be used to simplify expressions that access multiple attributes with the same domain. For example:\n\n cel.bind(dra, device.attributes[\"dra.example.com\"], dra.someBool && dra.anotherBool)",
"expression": "Expression is a CEL expression which evaluates a single device. It must evaluate to true when the device under consideration satisfies the desired criteria, and false when it does not. Any other result is an error and causes allocation of devices to abort.\n\nThe expression's input is an object named \"device\", which carries the following properties:\n - driver (string): the name of the driver which defines this device.\n - attributes (map[string]object): the device's attributes, grouped by prefix\n (e.g. device.attributes[\"dra.example.com\"] evaluates to an object with all\n of the attributes which were prefixed by \"dra.example.com\".\n - capacity (map[string]object): the device's capacities, grouped by prefix.\n\nExample: Consider a device with driver=\"dra.example.com\", which exposes two attributes named \"model\" and \"ext.example.com/family\" and which exposes one capacity named \"modules\". This input to this expression would have the following fields:\n\n device.driver\n device.attributes[\"dra.example.com\"].model\n device.attributes[\"ext.example.com\"].family\n device.capacity[\"dra.example.com\"].modules\n\nThe device.driver field can be used to check for a specific driver, either as a high-level precondition (i.e. you only want to consider devices from this driver) or as part of a multi-clause expression that is meant to consider devices from different drivers.\n\nThe value type of each attribute is defined by the device definition, and users who write these expressions must consult the documentation for their specific drivers. The value type of each capacity is Quantity.\n\nIf an unknown prefix is used as a lookup in either device.attributes or device.capacity, an empty map will be returned. Any reference to an unknown field will cause an evaluation error and allocation to abort.\n\nA robust expression should check for the existence of attributes before referencing them.\n\nFor ease of use, the cel.bind() function is enabled, and can be used to simplify expressions that access multiple attributes with the same domain. For example:\n\n cel.bind(dra, device.attributes[\"dra.example.com\"], dra.someBool && dra.anotherBool)\n\nThe length of the expression must be smaller or equal to 10 Ki. The cost of evaluating it is also limited based on the estimated number of logical steps.",
}
func (CELDeviceSelector) SwaggerDoc() map[string]string {

View File

@ -38,6 +38,7 @@ import (
apiservercel "k8s.io/apiserver/pkg/cel"
"k8s.io/apiserver/pkg/cel/environment"
"k8s.io/apiserver/pkg/cel/library"
"k8s.io/utils/ptr"
)
const (
@ -67,6 +68,10 @@ type CompilationResult struct {
OutputType *cel.Type
Environment *cel.Env
// MaxCost represents the worst-case cost of the compiled MessageExpression in terms of CEL's cost units,
// as used by cel.EstimateCost.
MaxCost uint64
emptyMapVal ref.Val
}
@ -88,10 +93,21 @@ func newCompiler() *compiler {
return &compiler{envset: mustBuildEnv()}
}
// Options contains several additional parameters
// for [CompileCELExpression]. All of them have reasonable
// defaults.
type Options struct {
// EnvType allows to override the default environment type [environment.StoredExpressions].
EnvType *environment.Type
// CostLimit allows overriding the default runtime cost limit [resourceapi.CELSelectorExpressionMaxCost].
CostLimit *uint64
}
// CompileCELExpression returns a compiled CEL expression. It evaluates to bool.
//
// TODO (https://github.com/kubernetes/kubernetes/issues/125826): validate AST to detect invalid attribute names.
func (c compiler) CompileCELExpression(expression string, envType environment.Type) CompilationResult {
func (c compiler) CompileCELExpression(expression string, options Options) CompilationResult {
resultError := func(errorString string, errType apiservercel.ErrorType) CompilationResult {
return CompilationResult{
Error: &apiservercel.Error{
@ -102,11 +118,15 @@ func (c compiler) CompileCELExpression(expression string, envType environment.Ty
}
}
env, err := c.envset.Env(envType)
env, err := c.envset.Env(ptr.Deref(options.EnvType, environment.StoredExpressions))
if err != nil {
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 := &library.CostEstimator{}
ast, issues := env.Compile(expression)
if issues != nil {
return resultError("compilation failed: "+issues.String(), apiservercel.ErrorTypeInvalid)
@ -122,18 +142,32 @@ func (c compiler) CompileCELExpression(expression string, envType environment.Ty
return resultError("unexpected compilation error: "+err.Error(), apiservercel.ErrorTypeInternal)
}
prog, err := env.Program(ast,
// The Kubernetes CEL base environment sets the VAP limit as runtime cost limit.
// DRA has its own default cost limit and also allows the caller to change that
// limit.
cel.CostLimit(ptr.Deref(options.CostLimit, resourceapi.CELSelectorExpressionMaxCost)),
cel.InterruptCheckFrequency(celconfig.CheckFrequency),
)
if err != nil {
return resultError("program instantiation failed: "+err.Error(), apiservercel.ErrorTypeInternal)
}
return CompilationResult{
compilationResult := CompilationResult{
Program: prog,
Expression: expression,
OutputType: ast.OutputType(),
Environment: env,
emptyMapVal: env.CELTypeAdapter().NativeToValue(map[string]any{}),
}
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
return compilationResult
}
// getAttributeValue returns the native representation of the one value that
@ -160,14 +194,14 @@ func getAttributeValue(attr resourceapi.DeviceAttribute) (any, error) {
var boolType = reflect.TypeOf(true)
func (c CompilationResult) DeviceMatches(ctx context.Context, input Device) (bool, error) {
func (c CompilationResult) DeviceMatches(ctx context.Context, input Device) (bool, *cel.EvalDetails, error) {
// TODO (future): avoid building these maps and instead use a proxy
// which wraps the underlying maps and directly looks up values.
attributes := make(map[string]any)
for name, attr := range input.Attributes {
value, err := getAttributeValue(attr)
if err != nil {
return false, fmt.Errorf("attribute %s: %w", name, err)
return false, nil, fmt.Errorf("attribute %s: %w", name, err)
}
domain, id := parseQualifiedName(name, input.Driver)
if attributes[domain] == nil {
@ -193,23 +227,23 @@ func (c CompilationResult) DeviceMatches(ctx context.Context, input Device) (boo
},
}
result, _, err := c.Program.ContextEval(ctx, variables)
result, details, err := c.Program.ContextEval(ctx, variables)
if err != nil {
return false, err
return false, details, err
}
resultAny, err := result.ConvertToNative(boolType)
if err != nil {
return false, fmt.Errorf("CEL result of type %s could not be converted to bool: %w", result.Type().TypeName(), err)
return false, details, fmt.Errorf("CEL result of type %s could not be converted to bool: %w", result.Type().TypeName(), err)
}
resultBool, ok := resultAny.(bool)
if !ok {
return false, fmt.Errorf("CEL native result value should have been a bool, got instead: %T", resultAny)
return false, details, fmt.Errorf("CEL native result value should have been a bool, got instead: %T", resultAny)
}
return resultBool, nil
return resultBool, details, nil
}
func mustBuildEnv() *environment.EnvSet {
envset := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), false /* strictCost */)
envset := environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true /* strictCost */)
field := func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField {
return apiservercel.NewDeclField(name, declType, required, nil, nil)
}

View File

@ -17,134 +17,157 @@ limitations under the License.
package cel
import (
"fmt"
"strings"
"testing"
resourceapi "k8s.io/api/resource/v1alpha3"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apiserver/pkg/cel/environment"
"k8s.io/klog/v2/ktesting"
"k8s.io/utils/ptr"
)
func TestCompile(t *testing.T) {
for name, scenario := range map[string]struct {
expression string
driver string
attributes map[resourceapi.QualifiedName]resourceapi.DeviceAttribute
capacity map[resourceapi.QualifiedName]resource.Quantity
expectCompileError string
expectMatchError string
expectMatch bool
}{
"true": {
expression: "true",
expectMatch: true,
},
"false": {
expression: "false",
expectMatch: false,
},
"syntax-error": {
expression: "?!",
expectCompileError: "Syntax error",
},
"type-error": {
expression: `1`,
expectCompileError: "must evaluate to bool or the unknown type, not int",
},
"runtime-error-lookup-identifier": {
expression: `device.attributes["no-such-domain"].noSuchAttr.isGreaterThan(quantity("0"))`,
expectMatchError: "no such key: noSuchAttr",
},
"runtime-error-lookup-map": {
expression: `device.attributes["no-such-domain"]["noSuchAttr"].isGreaterThan(quantity("0"))`,
expectMatchError: "no such key: noSuchAttr",
},
"domain-check-negative": {
expression: `"no-such-domain" in device.attributes`,
expectMatch: false,
},
"domain-check-positive": {
expression: `"dra.example.com" in device.attributes`,
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"dra.example.com/something": {BoolValue: ptr.To(true)}},
expectMatch: true,
},
"empty-driver-name": {
expression: `device.driver == ""`,
expectMatch: true,
},
"real-driver-name": {
expression: `device.driver == "dra.example.com"`,
driver: "dra.example.com",
expectMatch: true,
},
"driver-name-qualifier": {
expression: `device.attributes["dra.example.com"].name`,
driver: "dra.example.com",
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"name": {BoolValue: ptr.To(true)}},
expectMatch: true,
},
"driver-name-qualifier-map-lookup": {
expression: `device.attributes["dra.example.com"]["name"]`,
driver: "dra.example.com",
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"name": {BoolValue: ptr.To(true)}},
expectMatch: true,
},
"bind": {
expression: `cel.bind(dra, device.attributes["dra.example.com"], dra.name)`,
driver: "dra.example.com",
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"name": {BoolValue: ptr.To(true)}},
expectMatch: true,
},
"qualified-attribute-name": {
expression: `device.attributes["other.example.com"].name`,
driver: "dra.example.com",
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"other.example.com/name": {BoolValue: ptr.To(true)}},
expectMatch: true,
},
"bool": {
expression: `device.attributes["dra.example.com"].name`,
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"name": {BoolValue: ptr.To(true)}},
driver: "dra.example.com",
expectMatch: true,
},
"int": {
expression: `device.attributes["dra.example.com"].name > 0`,
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"name": {IntValue: ptr.To(int64(1))}},
driver: "dra.example.com",
expectMatch: true,
},
"string": {
expression: `device.attributes["dra.example.com"].name == "fish"`,
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"name": {StringValue: ptr.To("fish")}},
driver: "dra.example.com",
expectMatch: true,
},
"version": {
expression: `device.attributes["dra.example.com"].name.isGreaterThan(semver("0.0.1"))`,
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"name": {VersionValue: ptr.To("1.0.0")}},
driver: "dra.example.com",
expectMatch: true,
},
"quantity": {
expression: `device.capacity["dra.example.com"].name.isGreaterThan(quantity("1Ki"))`,
capacity: map[resourceapi.QualifiedName]resource.Quantity{"name": resource.MustParse("1Mi")},
driver: "dra.example.com",
expectMatch: true,
},
"check-positive": {
expression: `"name" in device.capacity["dra.example.com"] && device.capacity["dra.example.com"].name.isGreaterThan(quantity("1Ki"))`,
capacity: map[resourceapi.QualifiedName]resource.Quantity{"name": resource.MustParse("1Mi")},
driver: "dra.example.com",
expectMatch: true,
},
"check-negative": {
expression: `!("name" in device.capacity["dra.example.com"]) || device.capacity["dra.example.com"].name.isGreaterThan(quantity("1Ki"))`,
expectMatch: true,
},
"all": {
expression: `
var testcases = map[string]struct {
expression string
driver string
attributes map[resourceapi.QualifiedName]resourceapi.DeviceAttribute
capacity map[resourceapi.QualifiedName]resource.Quantity
expectCompileError string
expectMatchError string
expectMatch bool
// There's no good way to verify that the cost of an expression
// really is what it should be other than eye-balling it. The
// cost should not change in the future unless the expression
// gets changed, so this provides some protection against
// regressions.
expectCost uint64
}{
"true": {
expression: "true",
expectMatch: true,
},
"false": {
expression: "false",
expectMatch: false,
},
"syntax-error": {
expression: "?!",
expectCompileError: "Syntax error",
},
"type-error": {
expression: `1`,
expectCompileError: "must evaluate to bool or the unknown type, not int",
},
"runtime-error-lookup-identifier": {
expression: `device.attributes["no-such-domain"].noSuchAttr.isGreaterThan(quantity("0"))`,
expectMatchError: "no such key: noSuchAttr",
expectCost: 6,
},
"runtime-error-lookup-map": {
expression: `device.attributes["no-such-domain"]["noSuchAttr"].isGreaterThan(quantity("0"))`,
expectMatchError: "no such key: noSuchAttr",
expectCost: 6,
},
"domain-check-negative": {
expression: `"no-such-domain" in device.attributes`,
expectMatch: false,
expectCost: 3,
},
"domain-check-positive": {
expression: `"dra.example.com" in device.attributes`,
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"dra.example.com/something": {BoolValue: ptr.To(true)}},
expectMatch: true,
expectCost: 3,
},
"empty-driver-name": {
expression: `device.driver == ""`,
expectMatch: true,
expectCost: 2,
},
"real-driver-name": {
expression: `device.driver == "dra.example.com"`,
driver: "dra.example.com",
expectMatch: true,
expectCost: 4,
},
"driver-name-qualifier": {
expression: `device.attributes["dra.example.com"].name`,
driver: "dra.example.com",
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"name": {BoolValue: ptr.To(true)}},
expectMatch: true,
expectCost: 4,
},
"driver-name-qualifier-map-lookup": {
expression: `device.attributes["dra.example.com"]["name"]`,
driver: "dra.example.com",
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"name": {BoolValue: ptr.To(true)}},
expectMatch: true,
expectCost: 4,
},
"bind": {
expression: `cel.bind(dra, device.attributes["dra.example.com"], dra.name)`,
driver: "dra.example.com",
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"name": {BoolValue: ptr.To(true)}},
expectMatch: true,
expectCost: 15,
},
"qualified-attribute-name": {
expression: `device.attributes["other.example.com"].name`,
driver: "dra.example.com",
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"other.example.com/name": {BoolValue: ptr.To(true)}},
expectMatch: true,
expectCost: 4,
},
"bool": {
expression: `device.attributes["dra.example.com"].name`,
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"name": {BoolValue: ptr.To(true)}},
driver: "dra.example.com",
expectMatch: true,
expectCost: 4,
},
"int": {
expression: `device.attributes["dra.example.com"].name > 0`,
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"name": {IntValue: ptr.To(int64(1))}},
driver: "dra.example.com",
expectMatch: true,
expectCost: 5,
},
"string": {
expression: `device.attributes["dra.example.com"].name == "fish"`,
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"name": {StringValue: ptr.To("fish")}},
driver: "dra.example.com",
expectMatch: true,
expectCost: 5,
},
"version": {
expression: `device.attributes["dra.example.com"].name.isGreaterThan(semver("0.0.1"))`,
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{"name": {VersionValue: ptr.To("1.0.0")}},
driver: "dra.example.com",
expectMatch: true,
expectCost: 6,
},
"quantity": {
expression: `device.capacity["dra.example.com"].name.isGreaterThan(quantity("1Ki"))`,
capacity: map[resourceapi.QualifiedName]resource.Quantity{"name": resource.MustParse("1Mi")},
driver: "dra.example.com",
expectMatch: true,
expectCost: 6,
},
"check-positive": {
expression: `"name" in device.capacity["dra.example.com"] && device.capacity["dra.example.com"].name.isGreaterThan(quantity("1Ki"))`,
capacity: map[resourceapi.QualifiedName]resource.Quantity{"name": resource.MustParse("1Mi")},
driver: "dra.example.com",
expectMatch: true,
expectCost: 10,
},
"check-negative": {
expression: `!("name" in device.capacity["dra.example.com"]) || device.capacity["dra.example.com"].name.isGreaterThan(quantity("1Ki"))`,
expectMatch: true,
expectCost: 11,
},
"all": {
expression: `
device.capacity["dra.example.com"].quantity.isGreaterThan(quantity("1Ki")) &&
device.attributes["dra.example.com"].bool &&
device.attributes["dra.example.com"].int > 0 &&
@ -156,32 +179,64 @@ device.attributes["dra.example.com"]["int"] > 0 &&
device.attributes["dra.example.com"]["string"] == "fish" &&
device.attributes["dra.example.com"]["version"].isGreaterThan(semver("0.0.1"))
`,
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{
"bool": {BoolValue: ptr.To(true)},
"int": {IntValue: ptr.To(int64(1))},
"string": {StringValue: ptr.To("fish")},
"version": {VersionValue: ptr.To("1.0.0")},
},
capacity: map[resourceapi.QualifiedName]resource.Quantity{
"quantity": resource.MustParse("1Mi"),
},
driver: "dra.example.com",
expectMatch: true,
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{
"bool": {BoolValue: ptr.To(true)},
"int": {IntValue: ptr.To(int64(1))},
"string": {StringValue: ptr.To("fish")},
"version": {VersionValue: ptr.To("1.0.0")},
},
"many": {
expression: `device.attributes["dra.example.com"].a && device.attributes["dra.example.com"].b && device.attributes["dra.example.com"].c`,
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{
"a": {BoolValue: ptr.To(true)},
"b": {BoolValue: ptr.To(true)},
"c": {BoolValue: ptr.To(true)},
},
driver: "dra.example.com",
expectMatch: true,
capacity: map[resourceapi.QualifiedName]resource.Quantity{
"quantity": resource.MustParse("1Mi"),
},
} {
driver: "dra.example.com",
expectMatch: true,
expectCost: 52,
},
"many": {
expression: `device.attributes["dra.example.com"].a && device.attributes["dra.example.com"].b && device.attributes["dra.example.com"].c`,
attributes: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{
"a": {BoolValue: ptr.To(true)},
"b": {BoolValue: ptr.To(true)},
"c": {BoolValue: ptr.To(true)},
},
driver: "dra.example.com",
expectMatch: true,
expectCost: 12,
},
"expensive": {
// The worst-case is based on the maximum number of
// attributes and the maximum attribute name length.
// To actually reach that expected cost at runtime, we must
// have many attributes.
attributes: func() map[resourceapi.QualifiedName]resourceapi.DeviceAttribute {
attributes := make(map[resourceapi.QualifiedName]resourceapi.DeviceAttribute)
prefix := "dra.example.com/"
attribute := resourceapi.DeviceAttribute{
StringValue: ptr.To("abc"),
}
// If the cost estimate was accurate, using exactly as many attributes
// as allowed at most should exceed the limit. In practice, the estimate
// is an upper bound and significantly more attributes are needed before
// the runtime cost becomes too large.
for i := 0; i < 1000*resourceapi.ResourceSliceMaxAttributesAndCapacitiesPerDevice; i++ {
suffix := fmt.Sprintf("-%d", i)
name := prefix + strings.Repeat("x", resourceapi.DeviceMaxIDLength-len(suffix)) + suffix
attributes[resourceapi.QualifiedName(name)] = attribute
}
return attributes
}(),
expression: `device.attributes["dra.example.com"].map(s, s.lowerAscii()).map(s, s.size()).sum() == 0`,
driver: "dra.example.com",
expectMatchError: "actual cost limit exceeded",
expectCost: 18446744073709551615, // Exceeds limit!
},
}
func TestCEL(t *testing.T) {
for name, scenario := range testcases {
t.Run(name, func(t *testing.T) {
_, ctx := ktesting.NewTestContext(t)
result := GetCompiler().CompileCELExpression(scenario.expression, environment.StoredExpressions)
result := GetCompiler().CompileCELExpression(scenario.expression, Options{})
if scenario.expectCompileError != "" && result.Error == nil {
t.Fatalf("expected compile error %q, got none", scenario.expectCompileError)
}
@ -194,7 +249,25 @@ device.attributes["dra.example.com"]["version"].isGreaterThan(semver("0.0.1"))
}
return
}
match, err := result.DeviceMatches(ctx, Device{Attributes: scenario.attributes, Capacity: scenario.capacity, Driver: scenario.driver})
if scenario.expectCompileError != "" {
t.Fatalf("expected compile error %q, got none", scenario.expectCompileError)
}
if expect, actual := scenario.expectCost, result.MaxCost; expect != actual {
t.Errorf("expected CEL cost %d, got %d instead", expect, actual)
}
match, details, err := result.DeviceMatches(ctx, Device{Attributes: scenario.attributes, Capacity: scenario.capacity, Driver: scenario.driver})
// details.ActualCost can be called for nil details, no need to check.
actualCost := ptr.Deref(details.ActualCost(), 0)
if scenario.expectCost > 0 {
t.Logf("actual cost %d, %d%% of worst-case estimate", actualCost, actualCost*100/scenario.expectCost)
} else {
t.Logf("actual cost %d, expected zero costs", actualCost)
if actualCost > 0 {
t.Errorf("expected zero costs for (presumably) constant expression %q, got instead %d", scenario.expression, actualCost)
}
}
if err != nil {
if scenario.expectMatchError == "" {
t.Fatalf("unexpected evaluation error: %v", err)
@ -204,9 +277,51 @@ device.attributes["dra.example.com"]["version"].isGreaterThan(semver("0.0.1"))
}
return
}
if scenario.expectMatchError != "" {
t.Fatalf("expected match error %q, got none", scenario.expectMatchError)
}
if match != scenario.expectMatch {
t.Fatalf("expected result %v, got %v", scenario.expectMatch, match)
}
})
}
}
func BenchmarkDeviceMatches(b *testing.B) {
for name, scenario := range testcases {
if scenario.expectCompileError != "" {
continue
}
b.Run(name, func(b *testing.B) {
_, ctx := ktesting.NewTestContext(b)
result := GetCompiler().CompileCELExpression(scenario.expression, Options{})
if result.Error != nil {
b.Fatalf("unexpected compile error: %s", result.Error.Error())
}
for i := 0; i < b.N; i++ {
// It would be nice to measure
// time/actual_cost, but the time as observed
// here also includes additional preparations
// in result.DeviceMatches and thus cannot be
// used.
match, _, err := result.DeviceMatches(ctx, Device{Attributes: scenario.attributes, Capacity: scenario.capacity, Driver: scenario.driver})
if err != nil {
if scenario.expectMatchError == "" {
b.Fatalf("unexpected evaluation error: %v", err)
}
if !strings.Contains(err.Error(), scenario.expectMatchError) {
b.Fatalf("expected evaluation error to contain %q, but got instead: %v", scenario.expectMatchError, err)
}
return
}
if scenario.expectMatchError != "" {
b.Fatalf("expected match error %q, got none", scenario.expectMatchError)
}
if match != scenario.expectMatch {
b.Fatalf("expected result %v, got %v", scenario.expectMatch, match)
}
}
})
}
}

View File

@ -26,10 +26,10 @@ import (
v1 "k8s.io/api/core/v1"
resourceapi "k8s.io/api/resource/v1alpha3"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apiserver/pkg/cel/environment"
resourcelisters "k8s.io/client-go/listers/resource/v1alpha3"
"k8s.io/dynamic-resource-allocation/cel"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"
)
// ClaimLister returns a subset of the claims that a
@ -658,7 +658,7 @@ func (alloc *allocator) isSelectable(r requestIndices, slice *resourceapi.Resour
func (alloc *allocator) selectorsMatch(r requestIndices, device *resourceapi.BasicDevice, deviceID DeviceID, class *resourceapi.DeviceClass, selectors []resourceapi.DeviceSelector) (bool, error) {
for i, selector := range selectors {
expr := cel.GetCompiler().CompileCELExpression(selector.CEL.Expression, environment.StoredExpressions)
expr := cel.GetCompiler().CompileCELExpression(selector.CEL.Expression, cel.Options{})
if expr.Error != nil {
// Could happen if some future apiserver accepted some
// future expression and then got downgraded. Normally
@ -671,11 +671,11 @@ func (alloc *allocator) selectorsMatch(r requestIndices, device *resourceapi.Bas
return false, fmt.Errorf("claim %s: selector #%d: CEL compile error: %w", klog.KObj(alloc.claimsToAllocate[r.claimIndex]), i, expr.Error)
}
matches, err := expr.DeviceMatches(alloc.ctx, cel.Device{Driver: deviceID.Driver, Attributes: device.Attributes, Capacity: device.Capacity})
matches, details, err := expr.DeviceMatches(alloc.ctx, cel.Device{Driver: deviceID.Driver, Attributes: device.Attributes, Capacity: device.Capacity})
if class != nil {
alloc.logger.V(7).Info("CEL result", "device", deviceID, "class", klog.KObj(class), "selector", i, "expression", selector.CEL.Expression, "matches", matches, "err", err)
alloc.logger.V(7).Info("CEL result", "device", deviceID, "class", klog.KObj(class), "selector", i, "expression", selector.CEL.Expression, "matches", matches, "actualCost", ptr.Deref(details.ActualCost(), 0), "err", err)
} else {
alloc.logger.V(7).Info("CEL result", "device", deviceID, "claim", klog.KObj(alloc.claimsToAllocate[r.claimIndex]), "selector", i, "expression", selector.CEL.Expression, "matches", matches, "err", err)
alloc.logger.V(7).Info("CEL result", "device", deviceID, "claim", klog.KObj(alloc.claimsToAllocate[r.claimIndex]), "selector", i, "expression", selector.CEL.Expression, "actualCost", ptr.Deref(details.ActualCost(), 0), "matches", matches, "err", err)
}
if err != nil {