From ca087cccffd69ea9a55edac38f5e91e948d0a2bd Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Tue, 22 Apr 2025 01:01:42 -0400 Subject: [PATCH 1/3] Implement getField CEL function I think it has to be a global overload, a member overload would require a generic on the receiver which doesn't seem possible. I'm proposing this as an eventual replacement (before 1.0) of our hack around the fact that the `in` identifier is reserved in CEL. This is especially urgent for protovalidate-cc which is currently carrying patches to the CEL implementation in order to enable it, since cel-cpp doesn't allow this sort of functionality to be added in at runtime. --- cel/library.go | 24 ++++++++++++++++++++++ cel/library_test.go | 49 ++++++++++++++++++++++++++++++++++++++------- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/cel/library.go b/cel/library.go index 7c4bd6ec..7c4c93e5 100644 --- a/cel/library.go +++ b/cel/library.go @@ -30,6 +30,7 @@ import ( "github.com/google/cel-go/common/types/ref" "github.com/google/cel-go/common/types/traits" "github.com/google/cel-go/ext" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/reflect/protoregistry" "google.golang.org/protobuf/types/dynamicpb" @@ -73,6 +74,29 @@ func (l library) CompileOptions() []cel.EnvOption { //nolint:funlen,gocyclo l.uniqueMemberOverload(cel.StringType, l.uniqueScalar), l.uniqueMemberOverload(cel.BytesType, l.uniqueBytes), ), + cel.Function("getField", + cel.Overload( + "get_field_any_string", + []*cel.Type{cel.AnyType, cel.StringType}, + cel.AnyType, + cel.FunctionBinding(func(values ...ref.Val) ref.Val { + message, ok := values[0].Value().(proto.Message) + if !ok { + return types.UnsupportedRefValConversionErr(values[0]) + } + fieldName, ok := values[1].Value().(string) + if !ok { + return types.UnsupportedRefValConversionErr(values[1]) + } + descriptor := message.ProtoReflect().Descriptor() + fieldDescriptor := descriptor.Fields().ByName(protoreflect.Name(fieldName)) + if fieldDescriptor == nil { + return types.NewErr("no such field: %s", fieldName) + } + return ProtoFieldToValue(fieldDescriptor, message.ProtoReflect().Get(fieldDescriptor), false) + }), + ), + ), cel.Function("isNan", cel.MemberOverload( "double_is_nan_bool", diff --git a/cel/library_test.go b/cel/library_test.go index 834dae19..b6d71e5a 100644 --- a/cel/library_test.go +++ b/cel/library_test.go @@ -17,7 +17,10 @@ package cel import ( "testing" + "github.com/bufbuild/protovalidate-go/internal/gen/buf/validate/conformance/cases" "github.com/google/cel-go/cel" + "github.com/google/cel-go/common/types" + "github.com/google/cel-go/common/types/ref" "github.com/google/cel-go/interpreter" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -26,7 +29,23 @@ import ( func TestCELLib(t *testing.T) { t.Parallel() - env, err := cel.NewEnv(cel.Lib(library{})) + testValue := cases.StringConst_builder{Val: "test_string"}.Build() + + activation, err := interpreter.NewActivation(map[string]any{ + "test": testValue, + }) + require.NoError(t, err) + + env, err := cel.NewEnv( + cel.Lib(library{}), + cel.Variable( + "test", + cel.ObjectType( + string(testValue.ProtoReflect().Descriptor().FullName()), + ), + ), + ) + require.NoError(t, err) t.Run("ext", func(t *testing.T) { @@ -34,7 +53,7 @@ func TestCELLib(t *testing.T) { tests := []struct { expr string - ex bool + ex any }{ {"0.0.isInf()", false}, {"0.0.isNan()", false}, @@ -197,6 +216,18 @@ func TestCELLib(t *testing.T) { "'foo@example.com '.isEmail()", false, }, + { + "getField(test, 'val')", + "test_string", + }, + { + "getField(test, 'lav')", + types.NewErrFromString("no such field: lav"), + }, + { + "getField(0, 'val')", + types.NewErrFromString("unsupported conversion"), + }, } for _, tc := range tests { @@ -204,11 +235,15 @@ func TestCELLib(t *testing.T) { t.Run(test.expr, func(t *testing.T) { t.Parallel() prog := buildTestProgram(t, env, test.expr) - val, _, err := prog.Eval(interpreter.EmptyActivation()) - require.NoError(t, err) - isUnique, ok := val.Value().(bool) - require.True(t, ok) - assert.Equal(t, test.ex, isUnique) + val, _, err := prog.Eval(activation) + if refEx, ok := test.ex.(ref.Val); ok && types.IsError(refEx) { + refErr, ok := refEx.Value().(error) + require.True(t, ok) + assert.ErrorContains(t, err, refErr.Error()) + } else { + require.NoError(t, err) + assert.Equal(t, test.ex, val.Value()) + } }) } }) From 40dc82df184280df1f1f3e8b11700103d3dbfd03 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Tue, 22 Apr 2025 12:42:41 -0400 Subject: [PATCH 2/3] Use the built-in Get function instead of doing it manually --- cel/library.go | 10 ++-------- cel/library_test.go | 2 +- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/cel/library.go b/cel/library.go index 7c4c93e5..f77c700d 100644 --- a/cel/library.go +++ b/cel/library.go @@ -30,7 +30,6 @@ import ( "github.com/google/cel-go/common/types/ref" "github.com/google/cel-go/common/types/traits" "github.com/google/cel-go/ext" - "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/reflect/protoregistry" "google.golang.org/protobuf/types/dynamicpb" @@ -80,7 +79,7 @@ func (l library) CompileOptions() []cel.EnvOption { //nolint:funlen,gocyclo []*cel.Type{cel.AnyType, cel.StringType}, cel.AnyType, cel.FunctionBinding(func(values ...ref.Val) ref.Val { - message, ok := values[0].Value().(proto.Message) + message, ok := values[0].(interface{ Get(index ref.Val) ref.Val }) if !ok { return types.UnsupportedRefValConversionErr(values[0]) } @@ -88,12 +87,7 @@ func (l library) CompileOptions() []cel.EnvOption { //nolint:funlen,gocyclo if !ok { return types.UnsupportedRefValConversionErr(values[1]) } - descriptor := message.ProtoReflect().Descriptor() - fieldDescriptor := descriptor.Fields().ByName(protoreflect.Name(fieldName)) - if fieldDescriptor == nil { - return types.NewErr("no such field: %s", fieldName) - } - return ProtoFieldToValue(fieldDescriptor, message.ProtoReflect().Get(fieldDescriptor), false) + return message.Get(types.String(fieldName)) }), ), ), diff --git a/cel/library_test.go b/cel/library_test.go index b6d71e5a..93f4009d 100644 --- a/cel/library_test.go +++ b/cel/library_test.go @@ -222,7 +222,7 @@ func TestCELLib(t *testing.T) { }, { "getField(test, 'lav')", - types.NewErrFromString("no such field: lav"), + types.NewErrFromString("no such field"), }, { "getField(0, 'val')", From 992ecd73a5677dadb68144e22c5daff3cdf531b9 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Tue, 22 Apr 2025 19:50:46 -0400 Subject: [PATCH 3/3] Oops, use traits.Indexer not ad-hoc interface lol --- cel/library.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cel/library.go b/cel/library.go index f77c700d..dc746ffd 100644 --- a/cel/library.go +++ b/cel/library.go @@ -79,7 +79,7 @@ func (l library) CompileOptions() []cel.EnvOption { //nolint:funlen,gocyclo []*cel.Type{cel.AnyType, cel.StringType}, cel.AnyType, cel.FunctionBinding(func(values ...ref.Val) ref.Val { - message, ok := values[0].(interface{ Get(index ref.Val) ref.Val }) + message, ok := values[0].(traits.Indexer) if !ok { return types.UnsupportedRefValConversionErr(values[0]) }