Allow passing a custom env for a validator

[simon0191]

Quick and dirty PR aiming for some feedback :)

---

Allow passing a custom env for a validator so that the caller can include custom CEL functions.

For example:

env, err := celext.DefaultEnv(true, celext.WithEnvOptions(
   cel.Function("isUUID",
			cel.MemberOverload(
				"string_is_uuid_bool",
				[]*cel.Type{cel.StringType},
				cel.BoolType,
				cel.FunctionBinding(func(value ref.Val) ref.Val {
                                          ....
				}),
			),
		),
))
assert.NoError(err)
validator := protovalidate.New(protovalidate.WithEnv(env))

[CLAassistant]

All committers have signed the CLA.

[rodaine]

Hey @simon0191! We want to ensure that any rules present on a proto are portable to any consumer of that message. If custom CEL expressions are used, they would need to be present everywhere. This is why we currently don't allow access to the underlying CEL environment in Go or any other supported language and will likely keep it this way to preserve this invariant of the project.

We are, however, exploring ways of making custom CEL expressions portable so that they can be applied easily to multiple fields without repeating the expression logic in N places. (The design there is not likely to be what it ultimately looks like, but feel free to give feedback there and stay tuned for feature updates!)

[rodaine]

I'm assuming your example above is just demonstrative, but just in case it's not, there is a standard constraint for UUIDs already.

[birdayz]

@rodaine we have a use case where we want to run certain validations only for Update calls in gRPC, but not for create calls. in this case, we cannot use standard constraints, because they would apply to all validations, as the field is present in both Update and Create calls.

so we add this as CEL expression to UpdateXYZ request, so it only applies to the Update call.

[rodaine]

Ah, that makes sense! We still want to make sure that everything remains self-contained within the proto files + the agreed upon functions required by the protovalidate spec, so we still likely won't accept this change.

If it's an option, another strategy you could use for this is have create/update request messages only include fields that can actually be mutated by the caller and only return the resource's message in responses. In my experience, this makes the semantics of the types clearer (there's no ambiguity about what a user can set vs what's computed by the server).

That said, I can see a feature where instead of just skipped at the field level, we can support skipped_fields which takes a FieldMask and can specify which fields not to evaluate. I think that'd work in your usecase. Then, you can have the uuid constraint on the field normally, and disable it for circumstances where you don't want to. This means the message describes its constraints accurately (the uuid field must be a uuid), but loosens it as an exception.

[simon0191]

We are, however, bufbuild/protovalidate#51 so that they can be applied easily to multiple fields without repeating the expression logic in N places.

This would be really useful.

another strategy you could use for this is have create/update request messages only include fields that can actually be mutated by the caller and only return the resource's message in responses. In my experience, this makes the semantics of the types clearer (there's no ambiguity about what a user can set vs what's computed by the server).

Good alternative. We'll give this a try.

I can see a feature where instead of just skipped at the field level, we can support skipped_fields which takes a FieldMask and can specify which fields not to evaluate. I think that'd work in your usecase. Then, you can have the uuid constraint on the field normally, and disable it for circumstances where you don't want to. This means the message describes its constraints accurately (the uuid field must be a uuid), but loosens it as an exception.

This also sounds like a good addition

---

Thanks for the feedback @rodaine