Skip to content
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

matcher: new custom predicate #59

Closed
wants to merge 1 commit into from

Conversation

wbpcode
Copy link

@wbpcode wbpcode commented Apr 24, 2023

New custom predicate of matcher API.

In the previous API, the SinglePredicate could be used to composite various matching conditions. However, it's very flexible but isn't easy to use. Especially if there are some common matching compositions that will be used frequently, for example, the headers matching.

This PR introduced a new CustomPredicate to present an custom matching condition. This new predicate type will take the protocol-specific data as input, and a bool as output. All the input and match logics are defined by the extension config.

This new type could be used when there are some common matching compositions. We can define a custom predicate as a composition of multiple different but releated matching conditions.

In addition, any protocol-specific data matching or non-string data matching could use this new CustomPredicate.

Take the headers match as an example. If there are three headers need to match, then the config in previous API would be:

//                 - predicate:
//                     and_matcher:
//                       predicate:
//                       - single_predicate:
//                           input:
//                             name: header
//                             typed_config:
//                               "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput
//                               header_name: version
//                           value_match:
//                             exact: v1
//                       - single_predicate:
//                           input:
//                             name: header
//                             typed_config:
//                               "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput
//                               header_name: user
//                           value_match:
//                             exact: john
//                       - single_predicate:
//                           input:
//                             name: header
//                             typed_config:
//                               "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput
//                               header_name: id
//                           value_match:
//                             exact: "123"

And with the new API, the API could be:

//                 - predicate:
//                     custom_predicate:
//                       custom_match:
//                         name: multiple-headers
//                         typed_config:
//                           "@type": type.googleapis.com/envoy.type.matcher.v3.HttpHeadersCustomMatcher
//                           version:
//                             exact: v1
//                           user:
//                             exact: john
//                           id:
//                             exact: "123"

Based on the new API, the configuration will be more simple and easy to read or manage.

Signed-off-by: wangbaiping <wbphub@live.com>
@markdroth
Copy link
Contributor

As per discussion in envoyproxy/envoy#26861, I don't think this is what we want. The intent of the generic matcher API was to explicitly decouple inputs and matchers so that the two can be specified independently. I would prefer not to move away from that design principle.

@wbpcode
Copy link
Author

wbpcode commented Apr 25, 2023

cc @markdroth Thanks. And I will close this PR temporarily. See the discussion in the envoyproxy/envoy#26861 for more detail.

@wbpcode wbpcode closed this Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants