Skip to content

Commit 7b0071d

Browse files
committed
DRA CEL: disable runtime cost check
In DRA, the cost check is done only at validation time. At runtime, any expression that passed validation gets executed without interrupting it. The advantage is that it becomes easier to change the limit because stored expression do not suddenly fail after an up- or downgrade. The limit could even become a configuration parameter of the apiserver because that is the only place where the limit gets checked
1 parent 39f2592 commit 7b0071d

File tree

2 files changed

+22
-9
lines changed

2 files changed

+22
-9
lines changed

staging/src/k8s.io/dynamic-resource-allocation/cel/compile.go

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23+
"math"
2324
"reflect"
2425
"strings"
2526
"sync"
@@ -130,12 +131,25 @@ func (c compiler) CompileCELExpression(expression string, envType environment.Ty
130131
return resultError("unexpected compilation error: "+err.Error(), apiservercel.ErrorTypeInternal)
131132
}
132133
prog, err := env.Program(ast,
133-
// cel.CostLimit is also set to the VAP PerCallLimit as part of
134-
// the base environment.
134+
// The Kubernetes CEL base environment sets the VAP cost limit.
135135
//
136-
// This call here should override that. In practice it shouldn't
137-
// matter because the limits are the same.
138-
cel.CostLimit(resourceapi.CELSelectorExpressionMaxCost),
136+
// In DRA, the cost check is done only at validation time. At
137+
// runtime, any expression that passed validation gets executed
138+
// without interrupting it. The advantage is that it becomes
139+
// easier to change the limit because stored expression do not
140+
// suddenly fail after an up- or downgrade. The limit could
141+
// even become a configuration parameter of the apiserver
142+
// because that is the only place where the limit gets checked
143+
//
144+
// We cannot unset the cost limit
145+
// (https://github.com/kubernetes/kubernetes/blob/9568a2ac145cf9be930e3da835f86c1e61f7f7c1/vendor/github.com/google/cel-go/cel/options.go#L512-L518). But
146+
// we can set a high value that then never triggers
147+
// (https://github.com/kubernetes/kubernetes/blob/9568a2ac145cf9be930e3da835f86c1e61f7f7c1/vendor/github.com/google/cel-go/interpreter/runtimecost.go#L104-L106).
148+
//
149+
// Even better would be to enable cost tracking only during
150+
// validation, but for that k8s.io/apiserver/pkg/cel/environment would
151+
// have to be changed to not set the limit.
152+
cel.CostLimit(math.MaxInt64),
139153
cel.InterruptCheckFrequency(celconfig.CheckFrequency),
140154
)
141155
if err != nil {

staging/src/k8s.io/dynamic-resource-allocation/cel/compile_test.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -227,10 +227,9 @@ device.attributes["dra.example.com"]["version"].isGreaterThan(semver("0.0.1"))
227227
}
228228
return attributes
229229
}(),
230-
expression: `device.attributes["dra.example.com"].map(s, s.lowerAscii()).map(s, s.size()).sum() == 0`,
231-
driver: "dra.example.com",
232-
expectMatchError: "actual cost limit exceeded",
233-
expectCost: 18446744073709551615, // Exceeds limit!
230+
expression: `device.attributes["dra.example.com"].map(s, s.lowerAscii()).map(s, s.size()).sum() == 0`,
231+
driver: "dra.example.com",
232+
expectCost: 18446744073709551615, // Exceeds limit!
234233
},
235234
} {
236235
t.Run(name, func(t *testing.T) {

0 commit comments

Comments
 (0)