cel: add raw string expression support #118
Conversation
Signed-off-by: wangbaiping(wbpcode) <wangbaiping@bytedance.com>
|
|
||
| // Unparsed expression in string form. | ||
| // | ||
| // If set, takes precedence over ``cel_expr_parsed`` and ``cel_expr_checked``. |
There was a problem hiding this comment.
Please add a description or link to what is a valid string value that is expected here.
There was a problem hiding this comment.
An example and the spec link is added.
There was a problem hiding this comment.
Thanks!
I'm going to approve this for now, but it's not clear to me where is "request.headers" defined? Is there a "response.body"?
I would assume this is all written somewhere (doc), and there should be a link here to that doc.
There was a problem hiding this comment.
Most CEL attributes are host-dependent. That's say, the example requires the host to support request.headers attributes. And you can see all attributes supported by envoy at here: https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes
There was a problem hiding this comment.
So I guess that's the link that could've been added here, no?
There was a problem hiding this comment.
No sure because xds repo is also be used by other projects. So, I only put an example and spec link here.
Signed-off-by: wangbaiping(wbpcode) <wangbaiping@bytedance.com>
|
|
||
| // Unparsed expression in string form. | ||
| // | ||
| // If set, takes precedence over ``cel_expr_parsed`` and ``cel_expr_checked``. |
There was a problem hiding this comment.
Thanks!
I'm going to approve this for now, but it's not clear to me where is "request.headers" defined? Is there a "response.body"?
I would assume this is all written somewhere (doc), and there should be a link here to that doc.
|
What's the motivation for this? In general, I don't think we want to support sending raw CEL expressions to the data plane. It makes more sense for the control plane to pre-compile the expression before sending it to the data plane, for both performance and validation reasons. Also, note that gRPC C++ will basically never be able to support this, because CEL C++ depends on protobuf, and gRPC can't take a dependency on protobuf internally. |
Here's the context:
I think this should be called out in the field's comments, highlighting the benefits of using the other fields. |
No description provided.