tap: factor out the TAP filter matcher for later reuse in other filters#12429
tap: factor out the TAP filter matcher for later reuse in other filters#12429mattklein123 merged 3 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Yangmin Zhu <ymzhu@google.com>
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
Thanks for taking this on. This looks like it's on the right track. A few comments to get started.
/wait
| std::string out; | ||
| ASSERT(proto_config.match_config().SerializeToString(&out)); | ||
| ASSERT(match.ParseFromString(out)); |
There was a problem hiding this comment.
Is this effectively a wire cast? I don't think this will always be safe as we add new fields, right? Or I guess it will as long as the new proto stays backwards compatible? Can you add comments here? Also see wireCast in the version conversion code and other utilities that we have in protobuf/utility.h. I think we do this elsewhere already.
Also, as neither will be required this part needs extra error handling to make sure one of these is set.
There was a problem hiding this comment.
As long as the MatchPredicate is wire compatible today, and we don't add a new field to the old one and different new field to the new one, we should be good with wireCast.
There was a problem hiding this comment.
yes, this is doing a wire cast, I changed to use VersionConverter::upgrade and also check at least one of them are set since I removed the validation in the proto.
This should work since the whole proto_config could have been converted from v2alpha to v3 before calling to this function. Here we do one more conversion from the v3.MatchPredicate to the refactored the factored out v3.MatchPredicate. We will need to maintain the backward compatibility in the factored out MatchPreficate just like any other API.
Signed-off-by: Yangmin Zhu <ymzhu@google.com>
|
Please check CI. /wait |
The option 'envoy.config.tap.v3.TapConfig.match_config' has been marked deprecated by the commit 520389e ("tap: factor out the TAP filter matcher for later reuse in other filters (#12429)"). Update the option to 'envoy.config.tap.v3.TapConfig.match'. And copy the configuration out of rst and into yaml. Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
The option 'envoy.config.tap.v3.TapConfig.match_config' has been marked deprecated by the commit 520389e ("tap: factor out the TAP filter matcher for later reuse in other filters (envoyproxy#12429)"). Update the option to 'envoy.config.tap.v3.TapConfig.match'. And copy the configuration out of rst and into yaml. Signed-off-by: Haiyue Wang <haiyue.wang@intel.com> Signed-off-by: Ryan Eskin <ryan.eskin89@protonmail.com>
3 participants
This is the 1st PR for #11832 that factors out the TAP filter matcher to prepare for reuse in other filters.
Signed-off-by: Yangmin Zhu ymzhu@google.com
For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md
Commit Message:
Additional Description:
Risk Level: Low
Testing: Covered by existing tests
Docs Changes: N/A
Release Notes: N/A
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]