Bump cel string lib to v2, add tests

This commit is contained in:
Joe Betz 2023-08-22 13:05:09 -04:00
parent 640a2ec332
commit 3fb14cf4e7
6 changed files with 35 additions and 21 deletions

View File

@ -259,13 +259,15 @@ func TestValidationExpressions(t *testing.T) {
"self.val1.substring(4, 10).trim() == 'takes'", "self.val1.substring(4, 10).trim() == 'takes'",
"self.val1.upperAscii() == 'ROOK TAKES 👑'", "self.val1.upperAscii() == 'ROOK TAKES 👑'",
"self.val1.lowerAscii() == 'rook takes 👑'", "self.val1.lowerAscii() == 'rook takes 👑'",
"'%d %s %f %s %s'.format([1, 'abc', 1.0, duration('1m'), timestamp('2000-01-01T00:00:00.000Z')]) == '1 abc 1.000000 60s 2000-01-01T00:00:00Z'",
"'%e'.format([3.14]) == '3.140000×10⁰⁰'",
"'%o %o %o'.format([7, 8, 9]) == '7 10 11'",
"'%b %b %b'.format([7, 8, 9]) == '111 1000 1001'",
}, },
errors: map[string]string{ errors: map[string]string{
// Invalid regex with a string constant regex pattern is compile time error // Invalid regex with a string constant regex pattern is compile time error
"self.val1.matches(')')": "compile error: program instantiation failed: error parsing regexp: unexpected ): `)`", "self.val1.matches(')')": "compile error: program instantiation failed: error parsing regexp: unexpected ): `)`",
// strings version 0 does not have format or join
// TODO: Replace this error test with valid tests when the string version is bumped.
"'%s %i'.format('a', 1) == 'a 1'": "undeclared reference to 'format'",
}, },
}, },
{name: "escaped strings", {name: "escaped strings",

View File

@ -41,7 +41,7 @@ import (
// desirable because it means that CEL expressions are portable across a wider range // desirable because it means that CEL expressions are portable across a wider range
// of Kubernetes versions. // of Kubernetes versions.
func DefaultCompatibilityVersion() *version.Version { func DefaultCompatibilityVersion() *version.Version {
return version.MajorMinor(1, 27) return version.MajorMinor(1, 28)
} }
var baseOpts = []VersionedOptions{ var baseOpts = []VersionedOptions{
@ -57,7 +57,6 @@ var baseOpts = []VersionedOptions{
cel.EagerlyValidateDeclarations(true), cel.EagerlyValidateDeclarations(true),
cel.DefaultUTCTimeZone(true), cel.DefaultUTCTimeZone(true),
ext.Strings(ext.StringsVersion(0)),
library.URLs(), library.URLs(),
library.Regex(), library.Regex(),
library.Lists(), library.Lists(),
@ -67,6 +66,13 @@ var baseOpts = []VersionedOptions{
cel.CostLimit(celconfig.PerCallLimit), 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), IntroducedVersion: version.MajorMinor(1, 27),
EnvOptions: []cel.EnvOption{ EnvOptions: []cel.EnvOption{
@ -81,7 +87,12 @@ var baseOpts = []VersionedOptions{
library.Quantity(), 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 // MustBaseEnvSet returns the common CEL base environments for Kubernetes for Version, or panics

View File

@ -35,7 +35,7 @@ var _ traits.Mapper = (*MapValue)(nil)
// MapValue is a map that lazily evaluate its value when a field is first accessed. // 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. // The map value is not designed to be thread-safe.
type MapValue struct { type MapValue struct {
typeValue *types.TypeValue typeValue *types.Type
// values are previously evaluated values obtained from callbacks // values are previously evaluated values obtained from callbacks
values map[string]ref.Val values map[string]ref.Val

View File

@ -102,7 +102,7 @@ func (l *CostEstimator) EstimateCallCost(function, overloadId string, target *ch
// of estimating the additional comparison cost. // of estimating the additional comparison cost.
if elNode := l.listElementNode(*target); elNode != nil { if elNode := l.listElementNode(*target); elNode != nil {
k := elNode.Type().Kind() k := elNode.Type().Kind()
if k == types.StructKind || k == types.BytesKind { if k == types.StringKind || k == types.BytesKind {
sz := l.sizeEstimate(elNode) sz := l.sizeEstimate(elNode)
elCost = elCost.Add(sz.MultiplyByCostFactor(common.StringTraversalCostFactor)) elCost = elCost.Add(sz.MultiplyByCostFactor(common.StringTraversalCostFactor))
} }

View File

@ -24,7 +24,7 @@ import (
"github.com/google/cel-go/cel" "github.com/google/cel-go/cel"
"github.com/google/cel-go/checker" "github.com/google/cel-go/checker"
"github.com/google/cel-go/ext" "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" "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) { func testCost(t *testing.T, expr string, expectEsimatedCost checker.CostEstimate, expectRuntimeCost uint64) {
est := &CostEstimator{SizeEstimator: &testCostEstimator{}} est := &CostEstimator{SizeEstimator: &testCostEstimator{}}
env, err := cel.NewEnv( env, err := cel.NewEnv(
ext.Strings(), ext.Strings(ext.StringsVersion(2)),
URLs(), URLs(),
Regex(), Regex(),
Lists(), Lists(),
@ -554,14 +554,15 @@ type testCostEstimator struct {
} }
func (t *testCostEstimator) EstimateSize(element checker.AstNode) *checker.SizeEstimate { func (t *testCostEstimator) EstimateSize(element checker.AstNode) *checker.SizeEstimate {
switch t := element.Type().TypeKind.(type) { expr, err := cel.TypeToExprType(element.Type())
case *expr.Type_Primitive: if err != nil {
switch t.Primitive { return nil
case expr.Type_STRING: }
return &checker.SizeEstimate{Min: 0, Max: 12} switch expr.GetPrimitive() {
case expr.Type_BYTES: case exprpb.Type_STRING:
return &checker.SizeEstimate{Min: 0, Max: 12} return &checker.SizeEstimate{Min: 0, Max: 12}
} case exprpb.Type_BYTES:
return &checker.SizeEstimate{Min: 0, Max: 12}
} }
return nil return nil
} }

View File

@ -21,11 +21,11 @@ import (
"testing" "testing"
"github.com/google/cel-go/cel" "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"
"github.com/google/cel-go/common/types/ref" "github.com/google/cel-go/common/types/ref"
"github.com/google/cel-go/ext" "github.com/google/cel-go/ext"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
apiservercel "k8s.io/apiserver/pkg/cel" apiservercel "k8s.io/apiserver/pkg/cel"
@ -66,10 +66,10 @@ func testQuantity(t *testing.T, expr string, expectResult ref.Val, expectRuntime
if !didMatch { if !didMatch {
missingCompileErrs = append(missingCompileErrs, expectedCompileErr) missingCompileErrs = append(missingCompileErrs, expectedCompileErr)
} else if len(matchedCompileErrs) != len(issues.Errors()) { } else if len(matchedCompileErrs) != len(issues.Errors()) {
unmatchedErrs := []common.Error{} unmatchedErrs := []cel.Error{}
for i, issue := range issues.Errors() { for i, issue := range issues.Errors() {
if !matchedCompileErrs.Has(i) { if !matchedCompileErrs.Has(i) {
unmatchedErrs = append(unmatchedErrs, issue) unmatchedErrs = append(unmatchedErrs, *issue)
} }
} }
require.Empty(t, unmatchedErrs, "unexpected compilation errors") require.Empty(t, unmatchedErrs, "unexpected compilation errors")