Skip to content

Commit 3d4a5da

Browse files
jpbetzjpbetz
committed
Add cost testing for two variable comprehensions
1 parent b0180a9 commit 3d4a5da

File tree

5 files changed

+162
-10
lines changed

5 files changed

+162
-10
lines changed

‎staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/celcoststability_test.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,15 @@ func TestCelCostStability(t *testing.T) {
381381
"!self.val.exists_one(k, self.val[k] > 0)": 17,
382382
"size(self.val) == 2": 4,
383383
"size(self.val.filter(k, self.val[k] > 1)) == 1": 26,
384+
385+
// two variable comprehensions
386+
"self.val.all(k, v, v > 0)": 13,
387+
"self.val.exists(k, v, v == 2)": 15,
388+
"self.val.existsOne(k, v, v == 2)": 10,
389+
"self.val.transformMap(k, v, v > 1, v + 1).size() == 1": 14,
390+
"self.val.transformMap(k, v, v + 1).size() == 2": 15,
391+
"self.val.transformMapEntry(k, v, v > 1, {k + '2': v + 1}).size() == 1": 45,
392+
"self.val.transformMapEntry(k, v, {k + '2': v + 1}).size() == 2": 77,
384393
},
385394
},
386395
{name: "listMap access",
@@ -426,6 +435,13 @@ func TestCelCostStability(t *testing.T) {
426435
// all() and exists() macros ignore errors from predicates so long as the condition holds for at least one element
427436
"self.listMap.exists(m, m.v2 == 'z')": 24,
428437
"!self.listMap.all(m, m.v2 != 'z')": 22,
438+
439+
// two variable comprehensions
440+
"!self.listMap.all(i, m, has(m.v2) && m.v2 != 'z')": 10,
441+
"self.listMap.exists(i, m, has(m.v2) && m.v2 == 'z')": 21,
442+
"self.listMap.existsOne(i, m, has(m.v2) && m.v2 == 'z')": 12,
443+
"self.listMap.transformList(i, m, has(m.v2) && m.v2 == 'z', m.v2).size() == 1": 25,
444+
"self.listMap.transformList(i, m, m.v).size() == 3": 47,
429445
},
430446
},
431447
{name: "list access",
@@ -447,6 +463,13 @@ func TestCelCostStability(t *testing.T) {
447463
"size(self.array.filter(e, e%2 == 0)) == 3": 68,
448464
"self.array.map(e, e * 20).filter(e, e > 50).exists(e, e == 60)": 194,
449465
"size(self.array) == 8": 4,
466+
467+
// two variable comprehensions
468+
"self.array.all(i, e, e > 0)": 43,
469+
"self.array.exists(i, e, e > 2)": 36,
470+
"self.array.existsOne(i, e, e > 4)": 22,
471+
"self.array.transformList(i, e, e > 2, e * 2).size() > 0": 77,
472+
"self.array.transformList(i, e, e * 2).size() > 0": 117,
450473
},
451474
},
452475
{name: "listSet access",

‎staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,15 @@ func TestValidationExpressions(t *testing.T) {
643643
"size(self.val) == 2",
644644
"self.val.map(k, self.val[k]).exists(v, v == 1)",
645645
"size(self.val.filter(k, self.val[k] > 1)) == 1",
646+
647+
// two variable comprehensions
648+
"self.val.all(k, v, v > 0)",
649+
"self.val.exists(k, v, v == 2)",
650+
"self.val.existsOne(k, v, v == 2)",
651+
"self.val.transformMap(k, v, v > 1, v + 1).size() == 1",
652+
"self.val.transformMap(k, v, v + 1).size() == 2",
653+
"self.val.transformMapEntry(k, v, v > 1, {k + '2': v + 1}).size() == 1",
654+
"self.val.transformMapEntry(k, v, {k + '2': v + 1}).size() == 2",
646655
},
647656
errors: map[string]string{
648657
"self.val['c'] == 1": "no such key: c",
@@ -691,6 +700,13 @@ func TestValidationExpressions(t *testing.T) {
691700
// all() and exists() macros ignore errors from predicates so long as the condition holds for at least one element
692701
"self.listMap.exists(m, m.v2 == 'z')",
693702
"!self.listMap.all(m, m.v2 != 'z')",
703+
704+
// two variable comprehensions
705+
"!self.listMap.all(i, m, has(m.v2) && m.v2 != 'z')",
706+
"self.listMap.exists(i, m, has(m.v2) && m.v2 == 'z')",
707+
"self.listMap.existsOne(i, m, has(m.v2) && m.v2 == 'z')",
708+
"self.listMap.transformList(i, m, has(m.v2) && m.v2 == 'z', m.v2).size() == 1",
709+
"self.listMap.transformList(i, m, m.v).size() == 3",
694710
},
695711
errors: map[string]string{
696712
// test comprehensions where the field used in predicates is unset on all but one of the elements: (error cases)
@@ -727,6 +743,13 @@ func TestValidationExpressions(t *testing.T) {
727743
"size(self.array.filter(e, e%2 == 0)) == 3",
728744
"self.array.map(e, e * 20).filter(e, e > 50).exists(e, e == 60)",
729745
"size(self.array) == 8",
746+
747+
// two variable comprehensions
748+
"self.array.all(i, e, e > 0)",
749+
"self.array.exists(i, e, e > 2)",
750+
"self.array.existsOne(i, e, e > 4)",
751+
"self.array.transformList(i, e, e > 2, e * 2).size() > 0",
752+
"self.array.transformList(i, e, e * 2).size() > 0",
730753
},
731754
errors: map[string]string{
732755
"self.array[100] == 0": "index out of bounds: 100",

‎staging/src/k8s.io/apiserver/pkg/cel/environment/base.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,13 @@ var baseOptsWithoutStrictCost = []VersionedOptions{
169169
library.AuthzSelectors(),
170170
},
171171
},
172+
// Two variable comprehensions
173+
{
174+
IntroducedVersion: version.MajorMinor(1, 32),
175+
EnvOptions: []cel.EnvOption{
176+
ext.TwoVarComprehensions(),
177+
},
178+
},
172179
}
173180

174181
var (

‎staging/src/k8s.io/apiserver/pkg/cel/library/cost.go‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,25 @@ var panicOnUnknown = false
3535

3636
// builtInFunctions is a list of functions used in cost tests that are not handled by CostEstimator.
3737
var knownUnhandledFunctions = map[string]bool{
38-
"uint": true,
39-
"duration": true,
40-
"bytes": true,
41-
"timestamp": true,
42-
"value": true,
43-
"_==_": true,
44-
"_&&_": true,
45-
"_>_": true,
46-
"!_": true,
47-
"strings.quote": true,
38+
"@not_strictly_false": true,
39+
"uint": true,
40+
"duration": true,
41+
"bytes": true,
42+
"cel.@mapInsert": true,
43+
"timestamp": true,
44+
"strings.quote": true,
45+
"value": true,
46+
"_==_": true,
47+
"_&&_": true,
48+
"_||_": true,
49+
"_>_": true,
50+
"_>=_": true,
51+
"_<_": true,
52+
"_<=_": true,
53+
"!_": true,
54+
"_?_:_": true,
55+
"_+_": true,
56+
"_-_": true,
4857
}
4958

5059
// CostEstimator implements CEL's interpretable.ActualCostEstimator and checker.CostEstimator.

‎staging/src/k8s.io/apiserver/pkg/cel/library/cost_test.go‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,6 +1110,95 @@ func TestSetsCost(t *testing.T) {
11101110
}
11111111
}
11121112

1113+
func TestTwoVariableComprehensionCost(t *testing.T) {
1114+
cases := []struct {
1115+
name string
1116+
expr string
1117+
expectEstimatedCost checker.CostEstimate
1118+
expectRuntimeCost uint64
1119+
}{
1120+
{
1121+
name: "map all",
1122+
expr: `{'a': 1, 'b': 2}.all(k, v, v > 0)`,
1123+
expectEstimatedCost: checker.CostEstimate{Min: 37, Max: 41},
1124+
expectRuntimeCost: 41,
1125+
},
1126+
{
1127+
name: "map exists",
1128+
expr: `{'a': 1, 'b': 2}.exists(k, v, v > 0)`,
1129+
expectEstimatedCost: checker.CostEstimate{Min: 39, Max: 43},
1130+
expectRuntimeCost: 40,
1131+
},
1132+
{
1133+
name: "map existsOne",
1134+
expr: `{'a': 1, 'b': 2}.existsOne(k, v, v > 0)`,
1135+
expectEstimatedCost: checker.CostEstimate{Min: 38, Max: 40},
1136+
expectRuntimeCost: 40,
1137+
},
1138+
{
1139+
name: "map transformMap",
1140+
expr: `{'a': 1, 'b': 2}.transformMap(k, v, v + 1)`,
1141+
expectEstimatedCost: checker.CostEstimate{Min: 71, Max: 71},
1142+
expectRuntimeCost: 71,
1143+
},
1144+
{
1145+
name: "map transformMap with filter",
1146+
expr: `{'a': 1, 'b': 2}.transformMap(k, v, v < 5, v + 1)`,
1147+
expectEstimatedCost: checker.CostEstimate{Min: 67, Max: 75},
1148+
expectRuntimeCost: 75,
1149+
},
1150+
{
1151+
name: "map transformMapEntry",
1152+
expr: `{'a': 1, 'b': 2}.transformMapEntry(k, v, {k: v + 1})`,
1153+
expectEstimatedCost: checker.CostEstimate{Min: 131, Max: 131},
1154+
expectRuntimeCost: 131,
1155+
},
1156+
{
1157+
name: "map transformMapEntry with filter",
1158+
expr: `{'a': 1, 'b': 2}.transformMapEntry(k, v, v < 5, {k: v + 1})`,
1159+
expectEstimatedCost: checker.CostEstimate{Min: 67, Max: 135},
1160+
expectRuntimeCost: 135,
1161+
},
1162+
1163+
{
1164+
name: "list all",
1165+
expr: `[1, 2].all(i, v, v > 0)`,
1166+
expectEstimatedCost: checker.CostEstimate{Min: 17, Max: 21},
1167+
expectRuntimeCost: 21,
1168+
},
1169+
{
1170+
name: "list exists",
1171+
expr: `[1, 2].exists(i, v, v > 0)`,
1172+
expectEstimatedCost: checker.CostEstimate{Min: 19, Max: 23},
1173+
expectRuntimeCost: 20,
1174+
},
1175+
{
1176+
name: "list existsOne",
1177+
expr: `[1, 2].existsOne(i, v, v > 0)`,
1178+
expectEstimatedCost: checker.CostEstimate{Min: 18, Max: 20},
1179+
expectRuntimeCost: 20,
1180+
},
1181+
{
1182+
name: "list transformList",
1183+
expr: `[1, 2].transformList(i, v, v + 1)`,
1184+
expectEstimatedCost: checker.CostEstimate{Min: 49, Max: 49},
1185+
expectRuntimeCost: 49,
1186+
},
1187+
{
1188+
name: "list transformList with filter",
1189+
expr: `[1, 2].transformList(i, v, v < 5, v + 1)`,
1190+
expectEstimatedCost: checker.CostEstimate{Min: 27, Max: 53},
1191+
expectRuntimeCost: 53,
1192+
},
1193+
}
1194+
1195+
for _, tc := range cases {
1196+
t.Run(tc.name, func(t *testing.T) {
1197+
testCost(t, tc.expr, tc.expectEstimatedCost, tc.expectRuntimeCost)
1198+
})
1199+
}
1200+
}
1201+
11131202
func testCost(t *testing.T, expr string, expectEsimatedCost checker.CostEstimate, expectRuntimeCost uint64) {
11141203
originalPanicOnUnknown := panicOnUnknown
11151204
panicOnUnknown = true
@@ -1133,6 +1222,7 @@ func testCost(t *testing.T, expr string, expectEsimatedCost checker.CostEstimate
11331222
// cel-go v0.17.7 introduced CostEstimatorOptions.
11341223
// Previous the presence has a cost of 0 but cel fixed it to 1. We still set to 0 here to avoid breaking changes.
11351224
cel.CostEstimatorOptions(checker.PresenceTestHasCost(false)),
1225+
ext.TwoVarComprehensions(),
11361226
)
11371227
if err != nil {
11381228
t.Fatalf("%v", err)

0 commit comments

Comments
 (0)