From 3fb679016423e80b87cf3e540d296471223460e6 Mon Sep 17 00:00:00 2001 From: Cici Huang Date: Tue, 5 Dec 2023 23:26:13 +0000 Subject: [PATCH 1/2] Update env version, Add cost for previous func, add tests, etc. --- .../schema/cel/celcoststability_test.go | 39 +++++++++++++++++++ .../apiserver/schema/cel/validation_test.go | 17 ++++++++ .../apiserver/pkg/cel/environment/base.go | 2 +- .../k8s.io/apiserver/pkg/cel/library/cost.go | 15 +++++++ 4 files changed, 72 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/celcoststability_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/celcoststability_test.go index 4dcb0d44f98..b451756b3ff 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/celcoststability_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/celcoststability_test.go @@ -1130,6 +1130,26 @@ func TestCelCostStability(t *testing.T) { "optional.ofNonZeroValue(1).hasValue()": 2, }, }, + {name: "quantity", + obj: objs("20", "200M"), + schema: schemas(stringType, stringType), + expectCost: map[string]int64{ + `isQuantity(self.val1)`: 3, + `isQuantity(self.val2)`: 3, + `isQuantity("200M")`: 1, + `isQuantity("20Mi")`: 1, + `quantity("200M") == quantity("0.2G") && quantity("0.2G") == quantity("200M")`: 6, + `quantity("2M") == quantity("0.002G") && quantity("2000k") == quantity("2M") && quantity("0.002G") == quantity("2000k")`: 9, + `quantity(self.val1).isLessThan(quantity(self.val2))`: 7, + `quantity("50M").isLessThan(quantity("100M"))`: 3, + `quantity("50Mi").isGreaterThan(quantity("50M"))`: 3, + `quantity("200M").compareTo(quantity("0.2G")) == 0`: 4, + `quantity("50k").add(quantity("20")) == quantity("50.02k")`: 5, + `quantity("50k").sub(20) == quantity("49980")`: 4, + `quantity("50").isInteger()`: 2, + `quantity(self.val1).isInteger()`: 4, + }, + }, } for _, tt := range cases { @@ -1939,6 +1959,25 @@ func TestCelEstimatedCostStability(t *testing.T) { "optional.ofNonZeroValue(1).hasValue()": 2, }, }, + {name: "quantity", + schema: schemas(stringType, stringType), + expectCost: map[string]uint64{ + `isQuantity(self.val1)`: 314575, + `isQuantity(self.val2)`: 314575, + `isQuantity("200M")`: 1, + `isQuantity("20Mi")`: 1, + `quantity("200M") == quantity("0.2G") && quantity("0.2G") == quantity("200M")`: uint64(3689348814741910532), + `quantity("2M") == quantity("0.002G") && quantity("2000k") == quantity("2M") && quantity("0.002G") == quantity("2000k")`: uint64(5534023222112865798), + `quantity(self.val1).isLessThan(quantity(self.val2))`: 629151, + `quantity("50M").isLessThan(quantity("100M"))`: 3, + `quantity("50Mi").isGreaterThan(quantity("50M"))`: 3, + `quantity("200M").compareTo(quantity("0.2G")) == 0`: 4, + `quantity("50k").add(quantity("20")) == quantity("50.02k")`: uint64(1844674407370955268), + `quantity("50k").sub(20) == quantity("49980")`: uint64(1844674407370955267), + `quantity("50").isInteger()`: 2, + `quantity(self.val1).isInteger()`: 314576, + }, + }, } for _, tt := range cases { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go index 63119ebaa22..3e6291dd8e9 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go @@ -1952,6 +1952,23 @@ func TestValidationExpressions(t *testing.T) { "self.absentObj.?absentStr == optional.none()": "no such key: absentObj", // missing ?. operator on first deref is an error }, }, + {name: "quantity", + obj: objs("20", "200M"), + schema: schemas(stringType, stringType), + valid: []string{ + "isQuantity(self.val1)", + "isQuantity(self.val2)", + `isQuantity("20Mi")`, + `quantity(self.val2) == quantity("0.2G") && quantity("0.2G") == quantity("200M")`, + `quantity("2M") == quantity("0.002G") && quantity("2000k") == quantity("2M") && quantity("0.002G") == quantity("2000k")`, + `quantity(self.val1).isLessThan(quantity("100M"))`, + `quantity(self.val2).isGreaterThan(quantity("50M"))`, + `quantity(self.val2).compareTo(quantity("0.2G")) == 0`, + `quantity("50k").add(quantity(self.val1)) == quantity("50.02k")`, + `quantity("50k").sub(quantity(self.val1)) == quantity("49980")`, + `quantity(self.val1).isInteger()`, + }, + }, } for i := range tests { diff --git a/staging/src/k8s.io/apiserver/pkg/cel/environment/base.go b/staging/src/k8s.io/apiserver/pkg/cel/environment/base.go index 4dce93a7925..c108bdd644f 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/environment/base.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/environment/base.go @@ -43,7 +43,7 @@ import ( // desirable because it means that CEL expressions are portable across a wider range // of Kubernetes versions. func DefaultCompatibilityVersion() *version.Version { - return version.MajorMinor(1, 28) + return version.MajorMinor(1, 29) } var baseOpts = []VersionedOptions{ diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/cost.go b/staging/src/k8s.io/apiserver/pkg/cel/library/cost.go index a723937f19f..e3bde017bea 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/cost.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/cost.go @@ -147,6 +147,14 @@ func (l *CostEstimator) CallCost(function, overloadId string, args []ref.Val, re return &cost } + case "quantity", "isQuantity": + if len(args) >= 1 { + cost := uint64(math.Ceil(float64(actualSize(args[0])) * common.StringTraversalCostFactor)) + return &cost + } + case "sign", "asInteger", "isInteger", "asApproximateFloat", "isGreaterThan", "isLessThan", "compareTo", "add", "sub": + cost := uint64(1) + return &cost } return nil } @@ -360,6 +368,13 @@ func (l *CostEstimator) EstimateCallCost(function, overloadId string, target *ch return &checker.CallEstimate{CostEstimate: ipCompCost} } + case "quantity", "isQuantity": + if target != nil { + sz := l.sizeEstimate(args[0]) + return &checker.CallEstimate{CostEstimate: sz.MultiplyByCostFactor(common.StringTraversalCostFactor)} + } + case "sign", "asInteger", "isInteger", "asApproximateFloat", "isGreaterThan", "isLessThan", "compareTo", "add", "sub": + return &checker.CallEstimate{CostEstimate: checker.CostEstimate{Min: 1, Max: 1}} } return nil } From eafa9bd24b298187bf6ac96127368b827fcd21ae Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Tue, 23 Jan 2024 11:35:08 -0500 Subject: [PATCH 2/2] Add validation tests --- .../apiserver/schema/cel/validation_test.go | 51 ++++++++++++++++--- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go index 3e6291dd8e9..2170fb9eef6 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go @@ -29,6 +29,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "k8s.io/klog/v2" "k8s.io/kube-openapi/pkg/validation/strfmt" "k8s.io/utils/ptr" @@ -325,9 +326,9 @@ func TestValidationExpressions(t *testing.T) { "type(self.val1) == google.protobuf.Duration", }, errors: map[string]string{ - "duration('1')": "invalid duration argument", - "duration('1d')": "invalid duration argument", - "duration('1us') < duration('1nns')": "invalid duration argument", + "duration('1')": "compilation failed: ERROR: :1:10: invalid duration argument", + "duration('1d')": "compilation failed: ERROR: :1:10: invalid duration argument", + "duration('1us') < duration('1nns')": "compilation failed: ERROR: :1:28: invalid duration argument", }, }, {name: "date format", @@ -358,9 +359,9 @@ func TestValidationExpressions(t *testing.T) { "type(self.val1) == google.protobuf.Timestamp", }, errors: map[string]string{ - "timestamp('1000-00-00T00:00:00Z')": "invalid timestamp", - "timestamp('1000-01-01T00:00:00ZZ')": "invalid timestamp", - "timestamp(-62135596801)": "invalid timestamp", + "timestamp('1000-00-00T00:00:00Z')": "compilation failed: ERROR: :1:11: invalid timestamp", + "timestamp('1000-01-01T00:00:00ZZ')": "compilation failed: ERROR: :1:11: invalid timestamp", + "timestamp(-62135596801)": "compilation failed: ERROR: :1:11: invalid timestamp", }, }, {name: "enums", @@ -422,7 +423,7 @@ func TestValidationExpressions(t *testing.T) { }, errors: map[string]string{ // Mixed type lists are not allowed since we have HomogeneousAggregateLiterals enabled - "[1, 'a', false].filter(x, string(x) == 'a')": "expected type 'int' but found 'string'", + "[1, 'a', false].filter(x, string(x) == 'a')": "compilation failed: ERROR: :1:5: expected type 'int' but found 'string'", }, }, {name: "string lists", @@ -454,6 +455,42 @@ func TestValidationExpressions(t *testing.T) { "['a', 'b', 'c'].join('-') == 'a-b-c'", "self.val1.join() == 'abc'", "['a', 'b', 'c'].join() == 'abc'", + + // CEL sets functions + "sets.contains(['a', 'b'], [])", + "sets.contains(['a', 'b'], ['b'])", + "!sets.contains(['a', 'b'], ['c'])", + "sets.equivalent([], [])", + "sets.equivalent(['c', 'b', 'a'], ['b', 'c', 'a'])", + "!sets.equivalent(['a', 'b'], ['b', 'c'])", + "sets.intersects(['a', 'b'], ['b', 'c'])", + "!sets.intersects([], [])", + "!sets.intersects(['a', 'b'], [])", + "!sets.intersects(['a', 'b'], ['c', 'd'])", + + "sets.contains([1, 2], [2])", + "sets.contains([true, false], [false])", + "sets.contains([1.25, 1.5], [1.5])", + "sets.contains([{'a': 1}, {'b': 2}], [{'b': 2}])", + "sets.contains([[1, 2], [3, 4]], [[3, 4]])", + "sets.contains([timestamp('2000-01-01T00:00:00.000+01:00'), timestamp('2012-01-01T00:00:00.000+01:00')], [timestamp('2012-01-01T00:00:00.000+01:00')])", + "sets.contains([duration('1h'), duration('2h')], [duration('2h')])", + + "sets.equivalent([1, 2], [1, 2])", + "sets.equivalent([true, false], [true, false])", + "sets.equivalent([1.25, 1.5], [1.25, 1.5])", + "sets.equivalent([{'a': 1}, {'b': 2}], [{'a': 1}, {'b': 2}])", + "sets.equivalent([[1, 2], [3, 4]], [[1, 2], [3, 4]])", + "sets.equivalent([timestamp('2012-01-01T00:00:00.000+01:00')], [timestamp('2012-01-01T00:00:00.000+01:00')])", + "sets.equivalent([duration('1h'), duration('2h')], [duration('1h'), duration('2h')])", + + "sets.intersects([1, 2], [2])", + "sets.intersects([true, false], [false])", + "sets.intersects([1.25, 1.5], [1.5])", + "sets.intersects([{'a': 1}, {'b': 2}], [{'b': 2}])", + "sets.intersects([[1, 2], [3, 4]], [[3, 4]])", + "sets.intersects([timestamp('2000-01-01T00:00:00.000+01:00'), timestamp('2012-01-01T00:00:00.000+01:00')], [timestamp('2012-01-01T00:00:00.000+01:00')])", + "sets.intersects([duration('1h'), duration('2h')], [duration('2h')])", }, }, {name: "listMaps",