thrift: filter integration tests #3679
Conversation
Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
| import os | ||
| import argparse | ||
| import io | ||
| import sys |
| proxy supports (or will eventually support): | ||
|
|
||
| Transports: framed, unframed, header | ||
| Protocols: binary, compact, json, ttwitter/finagle |
There was a problem hiding this comment.
That's what they call it: https://twitter.github.io/finagle/guide/Protocols.html?highlight=TTwitter#thrift%20user%20guide
| sys.exit("invalid unix domain socket: {}".format(cfg.addr)) | ||
| socket = TSocket.TSocket(unix_socket=cfg.addr) | ||
| else: | ||
| (host, port) = cfg.addr.split(":") |
There was a problem hiding this comment.
this will raise ValueError if cfg.addr has no :
Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
There was a problem hiding this comment.
@htuch @alyssawilk FYI this might create important issues for you. Do you want to check this real quick? Presumably you can just not-build/delete all of this code on import?
|
I believe with the new extension logic we should be able to not-build fairly easily. I'm also not sure we've tested that. @mrice32 @PiotrSikora (current and next import rotation) |
|
Do you want me to hold off merging until you've had a chance to check it? |
|
I'd say if @tschroed (swapped rotation with our-Matt) doesn't respond in the next 15 minutes you're good to go |
|
I'm already in the pain cave trying to reconcile some weird bazel versus blaze dysfunction. What's one more log on the fire? :) |
Provides a set of python scripts that allows an integration test to run a Thrift client and server. The scripts handle all the Thrift transports and protocols we expect Envoy to eventually handle.
Adds envoy_extension_cc_test_library to test/extensions/extensions_build_system.bzl. I believe this is necessary to successfully exclude the thrift_proxy from builds. Adds more descriptive names to parameterized tests.
Implements an integration test for the Thrift sniffing filter, which uncovered some compact protocol bugs, which are also fixed in this PR along with matching unit test coverage.
Risk Level: Low - introduces some new dependencies, but they are not part of any build path beyond the thrift_proxy
Testing: integration tests
Docs Changes: n/a
Relates to: #2247