-
Notifications
You must be signed in to change notification settings - Fork 13
Rename blocking module to filter and define additional evaluate methods #89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
* {@link FilterEvaluator} evaluates given request/RPC and the result is used to block further | ||
* processing of the request. | ||
*/ | ||
public interface FilterEvaluator { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davexroth @jcchavezs please have a look at this API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davexroth could you please paste link to the Envoy filter interface?
@mohit-a21 could you please also have a look?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This interface will need to be used by multiple languages, some of which may not have operator overloading. Can you separate out the methods into evaluateHeaders
and evaluateBody
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will all languages follow the same API? I would say no, in each language we will have a different API. The impl will then forward calls to the webasm.
Signed-off-by: Pavol Loffay <[email protected]>
* @param headers are used for blocking evaluation. | ||
* @return filter result | ||
*/ | ||
FilterResult evaluate(Map<String, String> headers); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all the methods are missing the information whether the passed arguments are from request or response. We could have request/reponse combination for all of them or maybe pass enum to distinguish the request/response/db?
* @param body request body | ||
* @return filter result | ||
*/ | ||
FilterResult evaluate(String body); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In which case, blocking will happen based on the body only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think we will need:
evaluate(headers)
evaluate(body)
evaluate(headers, body)
and have this for request and response since the evaluation might differ.
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
The additional blocking methods will be used to block on request/RPC bodies