-
Notifications
You must be signed in to change notification settings - Fork 5.3k
test: creating a test listener filter and moving the xds test over to it #20571
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| #include "test/integration/filters/test_listener_filter.h" | ||
|
|
||
| namespace Envoy { | ||
|
|
||
| /** | ||
| * Config registration for the test filter. | ||
| */ | ||
| class TestInspectorConfigFactory : public Server::Configuration::NamedListenerFilterConfigFactory { | ||
| public: | ||
| // NamedListenerFilterConfigFactory | ||
| Network::ListenerFilterFactoryCb createListenerFilterFactoryFromProto( | ||
| const Protobuf::Message& /*message*/, | ||
| const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher, | ||
| Server::Configuration::ListenerFactoryContext& /*context*/) override { | ||
| return [listener_filter_matcher](Network::ListenerFilterManager& filter_manager) -> void { | ||
| filter_manager.addAcceptFilter(listener_filter_matcher, | ||
| std::make_unique<TestListenerFilter>()); | ||
| }; | ||
| } | ||
|
|
||
| ProtobufTypes::MessagePtr createEmptyConfigProto() override { | ||
| return ProtobufTypes::MessagePtr{new Envoy::ProtobufWkt::Struct()}; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean it's not needed here, so would it make sense to add it as a follow up over in one of those PRs?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh whoops, I was looking at the second and not the first. :-/
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, we have #20476 for general fix-ups. |
||
| } | ||
|
|
||
| std::string name() const override { return "envoy.filters.listener.test"; } | ||
| }; | ||
|
|
||
| absl::Mutex TestListenerFilter::alpn_lock_; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another way is to set the alpn as a parameter for this filter config, then we can get per-instance alpn value. But yes, it needs more code probably.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that doesn't work as we want one filter which returns different alpn values for different connections. But anyway the goal is to just have those tests pass without a filter some folks don't want to include so I think simple (with crummy locks) is fine :-) |
||
| std::string TestListenerFilter::alpn_; | ||
|
|
||
| REGISTER_FACTORY(TestInspectorConfigFactory, | ||
| Server::Configuration::NamedListenerFilterConfigFactory){"envoy.listener.test"}; | ||
|
|
||
| } // namespace Envoy | ||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,35 @@ | ||||
| #include "envoy/registry/registry.h" | ||||
| #include "envoy/server/filter_config.h" | ||||
|
|
||||
| namespace Envoy { | ||||
| /** | ||||
| * Test listener filter which sets the ALPN to a manually configured string. | ||||
| */ | ||||
| class TestListenerFilter : public Network::ListenerFilter { | ||||
| public: | ||||
| TestListenerFilter() = default; | ||||
|
|
||||
| // Network::ListenerFilter | ||||
| Network::FilterStatus onAccept(Network::ListenerFilterCallbacks& cb) override { | ||||
| absl::MutexLock m(&alpn_lock_); | ||||
| ASSERT(!alpn_.empty()); | ||||
| cb.socket().setRequestedApplicationProtocols({alpn_}); | ||||
| alpn_.clear(); | ||||
| return Network::FilterStatus::Continue; | ||||
| } | ||||
| Network::FilterStatus onData(Network::ListenerFilterBuffer&) override { | ||||
| return Network::FilterStatus::Continue; | ||||
| } | ||||
| size_t maxReadBytes() const override { return 0; } | ||||
|
|
||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We got a new method for ListenerFilter Line 341 in 08c2e5a
Just need a empty implement of onData
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh thanks for the heads up - github hadn't picked up what would have broken CI :-) |
||||
| static void setAlpn(std::string alpn) { | ||||
| absl::MutexLock m(&alpn_lock_); | ||||
| alpn_ = alpn; | ||||
| } | ||||
|
|
||||
| private: | ||||
| static absl::Mutex alpn_lock_; | ||||
| static std::string alpn_; | ||||
| }; | ||||
|
|
||||
| } // namespace Envoy | ||||
Uh oh!
There was an error while loading. Please reload this page.