-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Add logic for instantiating UdpListenSocket based on SocketAddress.protocol in listener config #5443
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
Merged
mattklein123
merged 12 commits into
envoyproxy:master
from
mpwarres:listener_socket_type
Jan 6, 2019
Merged
Add logic for instantiating UdpListenSocket based on SocketAddress.protocol in listener config #5443
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
580154c
Add Network::Utility::protobufAddressSocketType() function.
mpwarres 5aa7b0e
Add socket_type param to ListenerComponentFactory::createListenSocket().
mpwarres 61c0838
In AddressJson::translateAddress(), set SocketAddress.protocol based …
mpwarres 6efe767
Add ListenerManagerImplWithRealFiltersTest.UdpAddress test.
mpwarres cbd1eb1
Add Network::Socket::socketType() method and implementations.
mpwarres f3bfe1d
Fix formatting.
mpwarres 86ad197
Fix UtilityTest.UnixClusterDns test failure.
mpwarres 96b9649
Address @alyssawilk review comments.
mpwarres feaf4b3
Merge branch 'master' into listener_socket_type
mpwarres 3a3be2b
Revert changes to AddressJson::translateAddress() and update test.
mpwarres f177e57
Forgot to revert config/utility_test.cc when backing out AddressJson:…
mpwarres 393f84b
Address comments from @mattklein123.
mpwarres File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,20 @@ | |
|
|
||
| namespace Envoy { | ||
| namespace Server { | ||
| namespace { | ||
|
|
||
| std::string toString(Network::Address::SocketType socket_type) { | ||
| switch (socket_type) { | ||
| case Network::Address::SocketType::Stream: | ||
| return "SocketType::Stream"; | ||
| case Network::Address::SocketType::Datagram: | ||
| return "SocketType::Datagram"; | ||
| default: | ||
|
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. I think this case is not needed as the compiler will check all enums are handled?
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. Yup, fixed. Thanks! |
||
| return fmt::format("unknown ({})", static_cast<int>(socket_type)); | ||
| } | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| std::vector<Network::FilterFactoryCb> ProdListenerComponentFactory::createNetworkFilterFactoryList_( | ||
| const Protobuf::RepeatedPtrField<envoy::api::v2::listener::Filter>& filters, | ||
|
|
@@ -82,16 +96,24 @@ ProdListenerComponentFactory::createListenerFilterFactoryList_( | |
| return ret; | ||
| } | ||
|
|
||
| Network::SocketSharedPtr | ||
| ProdListenerComponentFactory::createListenSocket(Network::Address::InstanceConstSharedPtr address, | ||
| const Network::Socket::OptionsSharedPtr& options, | ||
| bool bind_to_port) { | ||
| Network::SocketSharedPtr ProdListenerComponentFactory::createListenSocket( | ||
| Network::Address::InstanceConstSharedPtr address, Network::Address::SocketType socket_type, | ||
| const Network::Socket::OptionsSharedPtr& options, bool bind_to_port) { | ||
| ASSERT(address->type() == Network::Address::Type::Ip || | ||
| address->type() == Network::Address::Type::Pipe); | ||
| ASSERT(socket_type == Network::Address::SocketType::Stream || | ||
| socket_type == Network::Address::SocketType::Datagram); | ||
|
|
||
| // For each listener config we share a single socket among all threaded listeners. | ||
| // First we try to get the socket from our parent if applicable. | ||
| if (address->type() == Network::Address::Type::Pipe) { | ||
| if (socket_type != Network::Address::SocketType::Stream) { | ||
| // This could be implemented in the future, since Unix domain sockets | ||
| // support SOCK_DGRAM, but there would need to be a way to specify it in | ||
| // envoy.api.v2.core.Pipe. | ||
| throw EnvoyException( | ||
| fmt::format("socket type {} not supported for pipes", toString(socket_type))); | ||
| } | ||
| const std::string addr = fmt::format("unix://{}", address->asString()); | ||
| const int fd = server_.hotRestart().duplicateParentListenSocket(addr); | ||
| if (fd != -1) { | ||
|
|
@@ -101,13 +123,22 @@ ProdListenerComponentFactory::createListenSocket(Network::Address::InstanceConst | |
| return std::make_shared<Network::UdsListenSocket>(address); | ||
| } | ||
|
|
||
| const std::string addr = fmt::format("tcp://{}", address->asString()); | ||
| const std::string scheme = (socket_type == Network::Address::SocketType::Stream) ? "tcp" : "udp"; | ||
|
mpwarres marked this conversation as resolved.
Outdated
|
||
| const std::string addr = fmt::format("{}://{}", scheme, address->asString()); | ||
| const int fd = server_.hotRestart().duplicateParentListenSocket(addr); | ||
| if (fd != -1) { | ||
| ENVOY_LOG(debug, "obtained socket for address {} from parent", addr); | ||
| return std::make_shared<Network::TcpListenSocket>(fd, address, options); | ||
| if (socket_type == Network::Address::SocketType::Stream) { | ||
| return std::make_shared<Network::TcpListenSocket>(fd, address, options); | ||
| } else { | ||
| return std::make_shared<Network::UdpListenSocket>(fd, address, options); | ||
| } | ||
| } | ||
| if (socket_type == Network::Address::SocketType::Stream) { | ||
| return std::make_shared<Network::TcpListenSocket>(address, options, bind_to_port); | ||
| } else { | ||
| return std::make_shared<Network::UdpListenSocket>(address, options, bind_to_port); | ||
| } | ||
| return std::make_shared<Network::TcpListenSocket>(address, options, bind_to_port); | ||
| } | ||
|
|
||
| DrainManagerPtr | ||
|
|
@@ -119,6 +150,7 @@ ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, const std::st | |
| ListenerManagerImpl& parent, const std::string& name, bool modifiable, | ||
| bool workers_started, uint64_t hash) | ||
| : parent_(parent), address_(Network::Address::resolveProtoAddress(config.address())), | ||
| socket_type_(Network::Utility::protobufAddressSocketType(config.address())), | ||
| global_scope_(parent_.server_.stats().createScope("")), | ||
| listener_scope_( | ||
| parent_.server_.stats().createScope(fmt::format("listener.{}.", address_->asString()))), | ||
|
|
@@ -774,6 +806,7 @@ bool ListenerManagerImpl::addOrUpdateListener(const envoy::api::v2::Listener& co | |
| new_listener->setSocket(draining_listener_socket | ||
| ? draining_listener_socket | ||
| : factory_.createListenSocket(new_listener->address(), | ||
| new_listener->socketType(), | ||
| new_listener->listenSocketOptions(), | ||
| new_listener->bindToPort())); | ||
| if (workers_started_) { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| #include "envoy/common/exception.h" | ||
|
|
||
| #include "common/config/address_json.h" | ||
|
|
||
| #include "gtest/gtest.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Config { | ||
|
|
||
| TEST(AddressJsonTest, TranslateResolvedUrlAddress) { | ||
| { | ||
| envoy::api::v2::core::Address proto_address; | ||
| AddressJson::translateAddress("tcp://1.2.3.4:5678", /*url=*/true, /*resolved=*/true, | ||
| proto_address); | ||
| EXPECT_EQ(envoy::api::v2::core::Address::kSocketAddress, proto_address.address_case()); | ||
| EXPECT_EQ(envoy::api::v2::core::SocketAddress::TCP, proto_address.socket_address().protocol()); | ||
| EXPECT_EQ("1.2.3.4", proto_address.socket_address().address()); | ||
| EXPECT_EQ(5678, proto_address.socket_address().port_value()); | ||
| } | ||
| { | ||
| envoy::api::v2::core::Address proto_address; | ||
| AddressJson::translateAddress("udp://1.2.3.4:5678", /*url=*/true, /*resolved=*/true, | ||
| proto_address); | ||
| EXPECT_EQ(envoy::api::v2::core::Address::kSocketAddress, proto_address.address_case()); | ||
| EXPECT_EQ(envoy::api::v2::core::SocketAddress::UDP, proto_address.socket_address().protocol()); | ||
| EXPECT_EQ("1.2.3.4", proto_address.socket_address().address()); | ||
| EXPECT_EQ(5678, proto_address.socket_address().port_value()); | ||
| } | ||
| { | ||
| envoy::api::v2::core::Address proto_address; | ||
| AddressJson::translateAddress("unix://foo/bar", /*url=*/true, /*resolved=*/true, proto_address); | ||
| EXPECT_EQ(envoy::api::v2::core::Address::kPipe, proto_address.address_case()); | ||
| EXPECT_EQ("foo/bar", proto_address.pipe().path()); | ||
| } | ||
| { | ||
| envoy::api::v2::core::Address proto_address; | ||
| EXPECT_THROW(AddressJson::translateAddress("invalid://1.2.3.4:5678", /*url=*/true, | ||
| /*resolved=*/true, proto_address), | ||
| EnvoyException); | ||
| } | ||
| } | ||
|
|
||
| TEST(AddressJsonTest, TranslateResolvedNonUrlAddress) { | ||
| { | ||
| envoy::api::v2::core::Address proto_address; | ||
| AddressJson::translateAddress("1.2.3.4:5678", /*url=*/false, /*resolved=*/true, proto_address); | ||
| EXPECT_EQ(envoy::api::v2::core::Address::kSocketAddress, proto_address.address_case()); | ||
| EXPECT_EQ(envoy::api::v2::core::SocketAddress::TCP, proto_address.socket_address().protocol()); | ||
| EXPECT_EQ("1.2.3.4", proto_address.socket_address().address()); | ||
| EXPECT_EQ(5678, proto_address.socket_address().port_value()); | ||
| } | ||
| { | ||
| envoy::api::v2::core::Address proto_address; | ||
| EXPECT_THROW(AddressJson::translateAddress("tcp://1.2.3.4:5678", /*url=*/false, | ||
| /*resolved=*/true, proto_address), | ||
| EnvoyException); | ||
| } | ||
| } | ||
|
|
||
| TEST(AddressJsonTest, TranslateUnresolvedUrlAddress) { | ||
| { | ||
| envoy::api::v2::core::Address proto_address; | ||
| AddressJson::translateAddress("tcp://foo.com:5678", /*url=*/true, /*resolved=*/false, | ||
| proto_address); | ||
| EXPECT_EQ(envoy::api::v2::core::Address::kSocketAddress, proto_address.address_case()); | ||
| EXPECT_EQ(envoy::api::v2::core::SocketAddress::TCP, proto_address.socket_address().protocol()); | ||
| EXPECT_EQ("foo.com", proto_address.socket_address().address()); | ||
| EXPECT_EQ(5678, proto_address.socket_address().port_value()); | ||
| } | ||
| { | ||
| envoy::api::v2::core::Address proto_address; | ||
| AddressJson::translateAddress("udp://bar.com:5678", /*url=*/true, /*resolved=*/false, | ||
| proto_address); | ||
| EXPECT_EQ(envoy::api::v2::core::Address::kSocketAddress, proto_address.address_case()); | ||
| EXPECT_EQ(envoy::api::v2::core::SocketAddress::UDP, proto_address.socket_address().protocol()); | ||
| EXPECT_EQ("bar.com", proto_address.socket_address().address()); | ||
| EXPECT_EQ(5678, proto_address.socket_address().port_value()); | ||
| } | ||
| { | ||
| envoy::api::v2::core::Address proto_address; | ||
| EXPECT_THROW(AddressJson::translateAddress("unix://foo/bar", /*url=*/true, /*resolved=*/false, | ||
| proto_address), | ||
| EnvoyException); | ||
| } | ||
| { | ||
| envoy::api::v2::core::Address proto_address; | ||
| EXPECT_THROW(AddressJson::translateAddress("invalid://qux.com:5678", /*url=*/true, | ||
| /*resolved=*/false, proto_address), | ||
| EnvoyException); | ||
| } | ||
| } | ||
|
|
||
| } // namespace Config | ||
| } // namespace Envoy |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.