protovalidate-go icon indicating copy to clipboard operation
protovalidate-go copied to clipboard

[Feature Request] Bitwise operation support in predefined CEL rules

Open KeXiangWang opened this issue 8 months ago • 3 comments

Feature description: Support bitwise operations in predefined CEL rules.

For example,

extend buf.validate.Int32Rules {
  optional bool power_of_two = 100000 [(buf.validate.predefined).cel = {
    id: "int32.power_of_two"
    message: "value must be a power of two"
    expression: "this >= 0 && (this & (this - 1)) == 0"
  }];
}

although the above rules can be successfully complied by protoc, when execute, I got this error:

compilation error: failed to compile standard rule "xxxxx.power_of_two": compilation error: failed to compile expression int32.power_of_two: ERROR: <input>:1:20: Syntax error: token recognition error at: '& '
 | this >= 0 && (this & (this - 1)) == 0

I think CEL itself support bitwise operations? I'm not sure whether protovalidate disable it or there's some reason it doesn't work.

My toolchain version: protoc: libprotoc 3.21.9 validate.proto: https://github.com/bufbuild/protovalidate/blob/51ee9b3a85ed11c19a2b8023aa2520cca82f431e/proto/protovalidate/buf/validate/validate.proto protovalidate-go: buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.6-20250425153114-8976f5be98c1.1

Problem it solves or use case: When predefining some more complicated rules.

Proposed implementation or solution: Although cel-spec hasn't support bitwise operations. They provide go-implenmation for this as an extension. See https://github.com/google/cel-go/blob/f6d3c92171c2c8732a8d0a4b24d6729df4261520/ext/math.go#L95. It should not be hard to support this new operations just like other extension rules.

Contribution: Willing to contribute, if I can get necessary guidance.

Examples or references: None

Additional context: None

KeXiangWang avatar May 08 '25 23:05 KeXiangWang

Just as a note to folks who come across this issue, updating protovalidate-go is blocked on the spec itself being updated (https://github.com/bufbuild/protovalidate/issues/362), and then we will want to land a similar change to all other supported protovalidate implementations at the same time.

nicksnyder avatar May 12 '25 16:05 nicksnyder

Hi @nicksnyder I'm interested in your repo and planning to implement bitwise operations in this repo. @smaye81 told me you would welcome the PR. https://github.com/bufbuild/protovalidate/issues/362#issuecomment-2864869153 However, before kicking off, I still want to confirm one thing: would you consider merging my PR to your main branch? This will break your consistency between different languages.

KeXiangWang avatar May 18 '25 03:05 KeXiangWang

I'm sorry there was some confusion caused by that comment. We can't merge features into this repo (or any Protovalidate repo) until:

  1. The CEL spec itself supports those features, and
  2. We have PRs ready to add support to all protovalidate implementations

We need to maintain consistency across all Protovalidate implementations.

nicksnyder avatar May 19 '25 18:05 nicksnyder