From 510e9f2ddff2e7dceb4b4431ad008bfc38e2b57a Mon Sep 17 00:00:00 2001 From: Cici Huang Date: Tue, 5 Dec 2023 23:27:51 +0000 Subject: [PATCH] Keep presence cost to 0 to ensure backward compatibility. Kubernetes-commit: ed501c1f080c054bae825e2cbdbdf9a8e99378e3 --- .../schema/cel/celcoststability_test.go | 82 ++++++++++--------- 1 file changed, 42 insertions(+), 40 deletions(-) diff --git a/pkg/apiserver/schema/cel/celcoststability_test.go b/pkg/apiserver/schema/cel/celcoststability_test.go index def5e025f..4dcb0d44f 100644 --- a/pkg/apiserver/schema/cel/celcoststability_test.go +++ b/pkg/apiserver/schema/cel/celcoststability_test.go @@ -227,6 +227,8 @@ func TestCelCostStability(t *testing.T) { "!(0 in self.val1)": 6, "self.val1 + self.val2 == [1, 2, 3, 1, 2, 3]": 6, "self.val1 + [4, 5] == [1, 2, 3, 4, 5]": 4, + "has(self.val1)": 1, + "has(self.val1) && has(self.val2)": 2, }, }, {name: "listSets", @@ -1344,9 +1346,9 @@ func TestCelEstimatedCostStability(t *testing.T) { "!(0 in self.val1)": 1572866, "self.val1 + self.val2 == [1, 2, 3, 1, 2, 3]": 16, "self.val1 + [4, 5] == [1, 2, 3, 4, 5]": 24, - "has(self.val1)": 2, - "has(self.val1) && has(self.val2)": 4, - "!has(self.val1)": 3, + "has(self.val1)": 1, + "has(self.val1) && has(self.val2)": 2, + "!has(self.val1)": 2, "self.val1.all(k, size(self.val1) > 0)": 11010044, "self.val1.exists_one(k, self.val1 == [2])": 23592949, "!self.val1.exists_one(k, size(self.val1) > 0)": 9437183, @@ -1365,9 +1367,9 @@ func TestCelEstimatedCostStability(t *testing.T) { "!('x' in self.val1)": 1048578, "self.val1 + self.val2 == ['a', 'b', 'c']": 16, "self.val1 + ['c', 'd'] == ['a', 'b', 'c', 'd']": 24, - "has(self.val1)": 2, - "has(self.val1) && has(self.val2)": 4, - "!has(self.val1)": 3, + "has(self.val1)": 1, + "has(self.val1) && has(self.val2)": 2, + "!has(self.val1)": 2, "self.val1.all(k, size(self.val1) > 0)": 7340028, "self.val1.exists_one(k, self.val1 == ['a'])": 15728629, "!self.val1.exists_one(k, size(self.val1) > 0)": 6291455, @@ -1392,9 +1394,9 @@ func TestCelEstimatedCostStability(t *testing.T) { "self.objs[2] + [self.objs[0][0], self.objs[0][1]] == self.objs[0]": 104883, // concat against a declared list "size(self.objs[0] + [self.objs[3][0]]) == 3": 20, - "has(self.objs)": 2, - "has(self.objs) && has(self.objs)": 4, - "!has(self.objs)": 3, + "has(self.objs)": 1, + "has(self.objs) && has(self.objs)": 2, + "!has(self.objs)": 2, "self.objs[0].all(k, size(self.objs[0]) > 0)": 8388604, "self.objs[0].exists_one(k, size(self.objs[0]) > 0)": 7340030, "!self.objs[0].exists_one(k, size(self.objs[0]) > 0)": 7340031, @@ -1409,9 +1411,9 @@ func TestCelEstimatedCostStability(t *testing.T) { "'k1' in self.val1": 3, "!('k3' in self.val1)": 4, "self.val1 == {'k1': 'a', 'k2': 'b'}": 33, - "has(self.val1)": 2, - "has(self.val1) && has(self.val2)": 4, - "!has(self.val1)": 3, + "has(self.val1)": 1, + "has(self.val1) && has(self.val2)": 2, + "!has(self.val1)": 2, "self.val1.all(k, size(self.val1) > 0)": 2752508, "self.val1.exists_one(k, size(self.val1) > 0)": 2359294, "!self.val1.exists_one(k, size(self.val1) > 0)": 2359295, @@ -1448,13 +1450,13 @@ func TestCelEstimatedCostStability(t *testing.T) { }), // https://github.com/google/cel-spec/blob/master/doc/langdef.md#field-selection expectCost: map[string]uint64{ - "has(self.a.b)": 3, - "has(self.a1.b1.c1)": 4, - "!(has(self.a1.d2) && has(self.a1.d2.e2))": 8, // must check intermediate optional fields (see below no such key error for d2) - "!has(self.a1.d2)": 4, - "has(self.a)": 2, - "has(self.a) && has(self.a1)": 4, - "!has(self.a)": 3, + "has(self.a.b)": 2, + "has(self.a1.b1.c1)": 3, + "!(has(self.a1.d2) && has(self.a1.d2.e2))": 6, // must check intermediate optional fields (see below no such key error for d2) + "!has(self.a1.d2)": 3, + "has(self.a)": 1, + "has(self.a) && has(self.a1)": 2, + "!has(self.a)": 2, }, }, {name: "map access", @@ -1468,10 +1470,10 @@ func TestCelEstimatedCostStability(t *testing.T) { "!('c' in self.val)": 4, "'d' in self.val": 3, // field selection also possible if map key is a valid CEL identifier - "!has(self.val.a)": 4, - "has(self.val.b)": 3, - "!has(self.val.c)": 4, - "has(self.val.d)": 3, + "!has(self.val.a)": 3, + "has(self.val.b)": 2, + "!has(self.val.c)": 3, + "has(self.val.d)": 2, "self.val.all(k, self.val[k] > 0)": 3595115, "self.val.exists_one(k, self.val[k] == 2)": 2696338, "!self.val.exists_one(k, self.val[k] > 0)": 3145728, @@ -1488,9 +1490,9 @@ func TestCelEstimatedCostStability(t *testing.T) { })), }), expectCost: map[string]uint64{ - "has(self.listMap[0].v)": 4, + "has(self.listMap[0].v)": 3, "self.listMap.all(m, m.k.startsWith('a'))": 6291453, - "self.listMap.all(m, !has(m.v2) || m.v2 == 'z')": 9437178, + "self.listMap.all(m, !has(m.v2) || m.v2 == 'z')": 8388603, "self.listMap.exists(m, m.k.endsWith('1'))": 7340028, "self.listMap.exists_one(m, m.k == 'a3')": 5242879, "!self.listMap.all(m, m.k.endsWith('1'))": 6291454, @@ -1503,12 +1505,12 @@ func TestCelEstimatedCostStability(t *testing.T) { // test comprehensions where the field used in predicates is unset on all but one of the elements: // - with has checks: - "self.listMap.exists(m, has(m.v2) && m.v2 == 'z')": 9437178, - "!self.listMap.all(m, has(m.v2) && m.v2 != 'z')": 8388604, - "self.listMap.exists_one(m, has(m.v2) && m.v2 == 'z')": 7340029, - "self.listMap.filter(m, has(m.v2) && m.v2 == 'z').size() == 1": 18874365, + "self.listMap.exists(m, has(m.v2) && m.v2 == 'z')": 8388603, + "!self.listMap.all(m, has(m.v2) && m.v2 != 'z')": 7340029, + "self.listMap.exists_one(m, has(m.v2) && m.v2 == 'z')": 6291454, + "self.listMap.filter(m, has(m.v2) && m.v2 == 'z').size() == 1": 17825790, // undocumented overload of map that takes a filter argument. This is the same as .filter().map() - "self.listMap.map(m, has(m.v2) && m.v2 == 'z', m.v2).size() == 1": 19922940, + "self.listMap.map(m, has(m.v2) && m.v2 == 'z', m.v2).size() == 1": 18874365, "self.listMap.filter(m, has(m.v2) && m.v2 == 'z').map(m, m.v2).size() == 1": uint64(18446744073709551615), // - without has checks: @@ -1639,7 +1641,7 @@ func TestCelEstimatedCostStability(t *testing.T) { "self.embedded.metadata.generateName == 'pickItForMe'": 6, // the object exists - "has(self.embedded)": 2, + "has(self.embedded)": 1, }, }, {name: "string in intOrString", @@ -1691,7 +1693,7 @@ func TestCelEstimatedCostStability(t *testing.T) { "something": withNullable(true, intOrStringType()), }), expectCost: map[string]uint64{ - "!has(self.something)": 3, + "!has(self.something)": 2, }, }, {name: "percent comparison using intOrString", @@ -1753,7 +1755,7 @@ func TestCelEstimatedCostStability(t *testing.T) { }, }), expectCost: map[string]uint64{ - "has(self.withUnknown)": 2, + "has(self.withUnknown)": 1, "self.withUnknownList.size() == 5": 4, // fields that are unknown because they were not specified on the object schema are included in equality checks "self.withUnknownList[0] != self.withUnknownList[1]": 6, @@ -1818,19 +1820,19 @@ func TestCelEstimatedCostStability(t *testing.T) { "setToNullNullableStr": withNullable(true, stringType), }), expectCost: map[string]uint64{ - "!has(self.unsetPlainStr)": 3, - "has(self.unsetDefaultedStr) && self.unsetDefaultedStr == 'default value'": 6, - "!has(self.unsetNullableStr)": 3, + "!has(self.unsetPlainStr)": 2, + "has(self.unsetDefaultedStr) && self.unsetDefaultedStr == 'default value'": 5, + "!has(self.unsetNullableStr)": 2, - "has(self.setPlainStr) && self.setPlainStr == 'v1'": 5, - "has(self.setDefaultedStr) && self.setDefaultedStr == 'v2'": 5, - "has(self.setNullableStr) && self.setNullableStr == 'v3'": 5, + "has(self.setPlainStr) && self.setPlainStr == 'v1'": 4, + "has(self.setDefaultedStr) && self.setDefaultedStr == 'v2'": 4, + "has(self.setNullableStr) && self.setNullableStr == 'v3'": 4, // We treat null fields as absent fields, not as null valued fields. // Note that this is different than how we treat nullable list items or map values. "type(self.setNullableStr) != null_type": 4, // a field that is set to null is treated the same as an absent field in validation rules - "!has(self.setToNullNullableStr)": 3, + "!has(self.setToNullNullableStr)": 2, }, }, {name: "null values in container types",