From f0fbaa3fab12183d26aafa858b7fc236c33ebe47 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Tue, 22 Aug 2023 13:05:09 -0400 Subject: [PATCH] Bump cel string lib to v2, add tests Kubernetes-commit: 3fb14cf4e7a0230d57f579b86262d9df6997e5e3 --- pkg/cel/environment/base.go | 17 ++++++++++++++--- pkg/cel/lazy/lazy.go | 2 +- pkg/cel/library/cost.go | 2 +- pkg/cel/library/cost_test.go | 21 +++++++++++---------- pkg/cel/library/quantity_test.go | 6 +++--- 5 files changed, 30 insertions(+), 18 deletions(-) diff --git a/pkg/cel/environment/base.go b/pkg/cel/environment/base.go index ed0d34041..bf3144ab4 100644 --- a/pkg/cel/environment/base.go +++ b/pkg/cel/environment/base.go @@ -41,7 +41,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, 27) + return version.MajorMinor(1, 28) } var baseOpts = []VersionedOptions{ @@ -57,7 +57,6 @@ var baseOpts = []VersionedOptions{ cel.EagerlyValidateDeclarations(true), cel.DefaultUTCTimeZone(true), - ext.Strings(ext.StringsVersion(0)), library.URLs(), library.Regex(), library.Lists(), @@ -67,6 +66,13 @@ var baseOpts = []VersionedOptions{ cel.CostLimit(celconfig.PerCallLimit), }, }, + { + IntroducedVersion: version.MajorMinor(1, 0), + RemovedVersion: version.MajorMinor(1, 29), + EnvOptions: []cel.EnvOption{ + ext.Strings(ext.StringsVersion(0)), + }, + }, { IntroducedVersion: version.MajorMinor(1, 27), EnvOptions: []cel.EnvOption{ @@ -81,7 +87,12 @@ var baseOpts = []VersionedOptions{ library.Quantity(), }, }, - // TODO: switch to ext.Strings version 2 once format() is fixed to work with HomogeneousAggregateLiterals. + { + IntroducedVersion: version.MajorMinor(1, 29), + EnvOptions: []cel.EnvOption{ + ext.Strings(ext.StringsVersion(2)), + }, + }, } // MustBaseEnvSet returns the common CEL base environments for Kubernetes for Version, or panics diff --git a/pkg/cel/lazy/lazy.go b/pkg/cel/lazy/lazy.go index 1742deb0a..16183050d 100644 --- a/pkg/cel/lazy/lazy.go +++ b/pkg/cel/lazy/lazy.go @@ -35,7 +35,7 @@ var _ traits.Mapper = (*MapValue)(nil) // MapValue is a map that lazily evaluate its value when a field is first accessed. // The map value is not designed to be thread-safe. type MapValue struct { - typeValue *types.TypeValue + typeValue *types.Type // values are previously evaluated values obtained from callbacks values map[string]ref.Val diff --git a/pkg/cel/library/cost.go b/pkg/cel/library/cost.go index c3cacf155..d18c138ec 100644 --- a/pkg/cel/library/cost.go +++ b/pkg/cel/library/cost.go @@ -102,7 +102,7 @@ func (l *CostEstimator) EstimateCallCost(function, overloadId string, target *ch // of estimating the additional comparison cost. if elNode := l.listElementNode(*target); elNode != nil { k := elNode.Type().Kind() - if k == types.StructKind || k == types.BytesKind { + if k == types.StringKind || k == types.BytesKind { sz := l.sizeEstimate(elNode) elCost = elCost.Add(sz.MultiplyByCostFactor(common.StringTraversalCostFactor)) } diff --git a/pkg/cel/library/cost_test.go b/pkg/cel/library/cost_test.go index 006e2719d..fbe7511fc 100644 --- a/pkg/cel/library/cost_test.go +++ b/pkg/cel/library/cost_test.go @@ -24,7 +24,7 @@ import ( "github.com/google/cel-go/cel" "github.com/google/cel-go/checker" "github.com/google/cel-go/ext" - expr "google.golang.org/genproto/googleapis/api/expr/v1alpha1" + exprpb "google.golang.org/genproto/googleapis/api/expr/v1alpha1" "k8s.io/apiserver/pkg/authorization/authorizer" ) @@ -411,7 +411,7 @@ func TestAuthzLibrary(t *testing.T) { func testCost(t *testing.T, expr string, expectEsimatedCost checker.CostEstimate, expectRuntimeCost uint64) { est := &CostEstimator{SizeEstimator: &testCostEstimator{}} env, err := cel.NewEnv( - ext.Strings(), + ext.Strings(ext.StringsVersion(2)), URLs(), Regex(), Lists(), @@ -554,14 +554,15 @@ type testCostEstimator struct { } func (t *testCostEstimator) EstimateSize(element checker.AstNode) *checker.SizeEstimate { - switch t := element.Type().TypeKind.(type) { - case *expr.Type_Primitive: - switch t.Primitive { - case expr.Type_STRING: - return &checker.SizeEstimate{Min: 0, Max: 12} - case expr.Type_BYTES: - return &checker.SizeEstimate{Min: 0, Max: 12} - } + expr, err := cel.TypeToExprType(element.Type()) + if err != nil { + return nil + } + switch expr.GetPrimitive() { + case exprpb.Type_STRING: + return &checker.SizeEstimate{Min: 0, Max: 12} + case exprpb.Type_BYTES: + return &checker.SizeEstimate{Min: 0, Max: 12} } return nil } diff --git a/pkg/cel/library/quantity_test.go b/pkg/cel/library/quantity_test.go index 3eda89446..d42a6ecb8 100644 --- a/pkg/cel/library/quantity_test.go +++ b/pkg/cel/library/quantity_test.go @@ -21,11 +21,11 @@ import ( "testing" "github.com/google/cel-go/cel" - "github.com/google/cel-go/common" "github.com/google/cel-go/common/types" "github.com/google/cel-go/common/types/ref" "github.com/google/cel-go/ext" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/sets" apiservercel "k8s.io/apiserver/pkg/cel" @@ -66,10 +66,10 @@ func testQuantity(t *testing.T, expr string, expectResult ref.Val, expectRuntime if !didMatch { missingCompileErrs = append(missingCompileErrs, expectedCompileErr) } else if len(matchedCompileErrs) != len(issues.Errors()) { - unmatchedErrs := []common.Error{} + unmatchedErrs := []cel.Error{} for i, issue := range issues.Errors() { if !matchedCompileErrs.Has(i) { - unmatchedErrs = append(unmatchedErrs, issue) + unmatchedErrs = append(unmatchedErrs, *issue) } } require.Empty(t, unmatchedErrs, "unexpected compilation errors")