From 580154c6bd7c9f5f08bad4208bcef1e9aa22c307 Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Sun, 30 Dec 2018 23:46:19 -0500 Subject: [PATCH 01/11] Add Network::Utility::protobufAddressSocketType() function. Also add Server::ListenerImpl::socket_type_ field and accessor. Signed-off-by: Michael Warres --- source/common/network/utility.cc | 21 +++++++++++++++++++ source/common/network/utility.h | 9 ++++++++ source/server/listener_manager_impl.cc | 1 + source/server/listener_manager_impl.h | 2 ++ test/common/network/utility_test.cc | 29 ++++++++++++++++++++++++++ 5 files changed, 62 insertions(+) diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc index 1fcb8fb1b92f2..7c39168014dc8 100644 --- a/source/common/network/utility.cc +++ b/source/common/network/utility.cc @@ -501,5 +501,26 @@ void Utility::addressToProtobufAddress(const Address::Instance& address, } } +Address::SocketType +Utility::protobufAddressSocketType(const envoy::api::v2::core::Address& proto_address) { + switch (proto_address.address_case()) { + case envoy::api::v2::core::Address::kSocketAddress: { + auto p = proto_address.socket_address().protocol(); + switch (p) { + case envoy::api::v2::core::SocketAddress::TCP: + return Address::SocketType::Stream; + case envoy::api::v2::core::SocketAddress::UDP: + return Address::SocketType::Datagram; + default: + throw EnvoyException(fmt::format("unknown protocol value: {}", p)); + } + } + case envoy::api::v2::core::Address::kPipe: + return Address::SocketType::Stream; + default: + NOT_REACHED_GCOVR_EXCL_LINE; + } +} + } // namespace Network } // namespace Envoy diff --git a/source/common/network/utility.h b/source/common/network/utility.h index 4a732b0ef87be..68f56d7ec5491 100644 --- a/source/common/network/utility.h +++ b/source/common/network/utility.h @@ -239,6 +239,15 @@ class Utility { static void addressToProtobufAddress(const Address::Instance& address, envoy::api::v2::core::Address& proto_address); + /** + * Returns socket type corresponding to SocketAddress.protocol value of the + * given address, or SocketType::Stream if the address is a pipe address. + * @param proto_address the address protobuf + * @return socket type + */ + static Address::SocketType + protobufAddressSocketType(const envoy::api::v2::core::Address& proto_address); + private: static void throwWithMalformedIp(const std::string& ip_address); diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index b294e981523ee..3b3175f8c4788 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -119,6 +119,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()))), diff --git a/source/server/listener_manager_impl.h b/source/server/listener_manager_impl.h index 745691e144bd3..f59f6bed1a612 100644 --- a/source/server/listener_manager_impl.h +++ b/source/server/listener_manager_impl.h @@ -227,6 +227,7 @@ class ListenerImpl : public Network::ListenerConfig, } Network::Address::InstanceConstSharedPtr address() const { return address_; } + Network::Address::SocketType socketType() const { return socket_type_; } const envoy::api::v2::Listener& config() { return config_; } const Network::SocketSharedPtr& getSocket() const { return socket_; } void debugLog(const std::string& message); @@ -386,6 +387,7 @@ class ListenerImpl : public Network::ListenerConfig, ListenerManagerImpl& parent_; Network::Address::InstanceConstSharedPtr address_; + Network::Address::SocketType socket_type_; Network::SocketSharedPtr socket_; Stats::ScopePtr global_scope_; // Stats with global named scope, but needed for LDS cleanup. Stats::ScopePtr listener_scope_; // Stats with listener named scope. diff --git a/test/common/network/utility_test.cc b/test/common/network/utility_test.cc index fcac051d36333..3f4245377c59e 100644 --- a/test/common/network/utility_test.cc +++ b/test/common/network/utility_test.cc @@ -320,6 +320,35 @@ TEST(NetworkUtility, AddressToProtobufAddress) { } } +TEST(NetworkUtility, ProtobufAddressSocketType) { + { + envoy::api::v2::core::Address proto_address; + proto_address.mutable_socket_address(); + EXPECT_EQ(Address::SocketType::Stream, Utility::protobufAddressSocketType(proto_address)); + } + { + envoy::api::v2::core::Address proto_address; + proto_address.mutable_socket_address()->set_protocol(envoy::api::v2::core::SocketAddress::TCP); + EXPECT_EQ(Address::SocketType::Stream, Utility::protobufAddressSocketType(proto_address)); + } + { + envoy::api::v2::core::Address proto_address; + proto_address.mutable_socket_address()->set_protocol(envoy::api::v2::core::SocketAddress::UDP); + EXPECT_EQ(Address::SocketType::Datagram, Utility::protobufAddressSocketType(proto_address)); + } + { + const auto kInvalidValue = static_cast(123); + envoy::api::v2::core::Address proto_address; + proto_address.mutable_socket_address()->set_protocol(kInvalidValue); + EXPECT_THROW(Utility::protobufAddressSocketType(proto_address), EnvoyException); + } + { + envoy::api::v2::core::Address proto_address; + proto_address.mutable_pipe(); + EXPECT_EQ(Address::SocketType::Stream, Utility::protobufAddressSocketType(proto_address)); + } +} + TEST(PortRangeListTest, Errors) { { std::string port_range_str = "a1"; From 5aa7b0ef5359955a1c806bffdfefdf24648e97d9 Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Sun, 30 Dec 2018 23:48:15 -0500 Subject: [PATCH 02/11] Add socket_type param to ListenerComponentFactory::createListenSocket(). Signed-off-by: Michael Warres --- include/envoy/server/listener_manager.h | 2 + source/server/config_validation/server.h | 1 + source/server/listener_manager_impl.cc | 40 +++++++- source/server/listener_manager_impl.h | 1 + test/mocks/server/mocks.cc | 3 +- test/mocks/server/mocks.h | 3 +- test/server/listener_manager_impl_test.cc | 114 +++++++++++----------- 7 files changed, 104 insertions(+), 60 deletions(-) diff --git a/include/envoy/server/listener_manager.h b/include/envoy/server/listener_manager.h index b00ba6da93176..fd2808cd6da76 100644 --- a/include/envoy/server/listener_manager.h +++ b/include/envoy/server/listener_manager.h @@ -44,12 +44,14 @@ class ListenerComponentFactory { /** * Creates a socket. * @param address supplies the socket's address. + * @param socket_type the type of socket (stream or datagram) to create. * @param options to be set on the created socket just before calling 'bind()'. * @param bind_to_port supplies whether to actually bind the socket. * @return Network::SocketSharedPtr an initialized and potentially bound socket. */ virtual Network::SocketSharedPtr createListenSocket(Network::Address::InstanceConstSharedPtr address, + Network::Address::SocketType socket_type, const Network::Socket::OptionsSharedPtr& options, bool bind_to_port) PURE; /** diff --git a/source/server/config_validation/server.h b/source/server/config_validation/server.h index 222c863e46e78..df320e9bc0a1e 100644 --- a/source/server/config_validation/server.h +++ b/source/server/config_validation/server.h @@ -114,6 +114,7 @@ class ValidationInstance : Logger::Loggable, return ProdListenerComponentFactory::createListenerFilterFactoryList_(filters, context); } Network::SocketSharedPtr createListenSocket(Network::Address::InstanceConstSharedPtr, + Network::Address::SocketType, const Network::Socket::OptionsSharedPtr&, bool) override { // Returned sockets are not currently used so we can return nothing here safely vs. a diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index 3b3175f8c4788..8bb5025dd7239 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -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: + return fmt::format("unknown ({})", static_cast(socket_type)); + } +} + +} // namespace std::vector ProdListenerComponentFactory::createNetworkFilterFactoryList_( const Protobuf::RepeatedPtrField& filters, @@ -84,14 +98,24 @@ ProdListenerComponentFactory::createListenerFilterFactoryList_( 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 +125,22 @@ ProdListenerComponentFactory::createListenSocket(Network::Address::InstanceConst return std::make_shared(address); } - const std::string addr = fmt::format("tcp://{}", address->asString()); + const std::string scheme = (socket_type == Network::Address::SocketType::Stream) ? "tcp" : "udp"; + 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(fd, address, options); + if (socket_type == Network::Address::SocketType::Stream) { + return std::make_shared(fd, address, options); + } else { + return std::make_shared(fd, address, options); + } + } + if (socket_type == Network::Address::SocketType::Stream) { + return std::make_shared(address, options, bind_to_port); + } else { + return std::make_shared(address, options, bind_to_port); } - return std::make_shared(address, options, bind_to_port); } DrainManagerPtr @@ -775,6 +808,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_) { diff --git a/source/server/listener_manager_impl.h b/source/server/listener_manager_impl.h index f59f6bed1a612..bf860ad9baa31 100644 --- a/source/server/listener_manager_impl.h +++ b/source/server/listener_manager_impl.h @@ -61,6 +61,7 @@ class ProdListenerComponentFactory : public ListenerComponentFactory, return createListenerFilterFactoryList_(filters, context); } Network::SocketSharedPtr createListenSocket(Network::Address::InstanceConstSharedPtr address, + Network::Address::SocketType socket_type, const Network::Socket::OptionsSharedPtr& options, bool bind_to_port) override; DrainManagerPtr createDrainManager(envoy::api::v2::Listener::DrainType drain_type) override; diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index dc194d3f58102..d4ea5bcecdbd7 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -83,8 +83,9 @@ MockOverloadManager::~MockOverloadManager() {} MockListenerComponentFactory::MockListenerComponentFactory() : socket_(std::make_shared>()) { - ON_CALL(*this, createListenSocket(_, _, _)) + ON_CALL(*this, createListenSocket(_, _, _, _)) .WillByDefault(Invoke([&](Network::Address::InstanceConstSharedPtr, + Network::Address::SocketType, const Network::Socket::OptionsSharedPtr& options, bool) -> Network::SocketSharedPtr { if (!Network::Socket::applyOptions(options, *socket_, diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index 8c9f9c4454454..9e929c826bbd8 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -220,8 +220,9 @@ class MockListenerComponentFactory : public ListenerComponentFactory { std::vector( const Protobuf::RepeatedPtrField&, Configuration::ListenerFactoryContext& context)); - MOCK_METHOD3(createListenSocket, + MOCK_METHOD4(createListenSocket, Network::SocketSharedPtr(Network::Address::InstanceConstSharedPtr address, + Network::Address::SocketType socket_type, const Network::Socket::OptionsSharedPtr& options, bool bind_to_port)); MOCK_METHOD1(createDrainManager_, DrainManager*(envoy::api::v2::Listener::DrainType drain_type)); diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index 87a02dc9b7b64..03e129940bd82 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -231,7 +231,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, EmptyFilter) { )EOF"; EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromJson(json), "", true); EXPECT_EQ(1U, manager_->listeners().size()); EXPECT_EQ(std::chrono::milliseconds(15000), @@ -246,7 +246,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, DefaultListenerPerConnectionBuffe } )EOF"; - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromJson(json), "", true); EXPECT_EQ(1024 * 1024U, manager_->listeners().back().get().perConnectionBufferLimitBytes()); } @@ -260,7 +260,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SetListenerPerConnectionBufferLim } )EOF"; - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromJson(json), "", true); EXPECT_EQ(8192U, manager_->listeners().back().get().perConnectionBufferLimitBytes()); } @@ -283,7 +283,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SslContext) { )EOF", Network::Address::IpVersion::v4); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromJson(json), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -370,7 +370,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, StatsScopeTest) { } )EOF"; - EXPECT_CALL(listener_factory_, createListenSocket(_, _, false)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, false)); manager_->addOrUpdateListener(parseListenerFromJson(json), "", true); manager_->listeners().front().get().listenerScope().counter("foo").inc(); @@ -388,7 +388,7 @@ TEST_F(ListenerManagerImplTest, NotDefaultListenerFiltersTimeout) { listener_filters_timeout: 0s )EOF"; - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(json), "", true)); EXPECT_EQ(std::chrono::milliseconds(), manager_->listeners().front().get().listenerFiltersTimeout()); @@ -403,7 +403,7 @@ TEST_F(ListenerManagerImplTest, ReversedWriteFilterOrder) { - filters: )EOF"; - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(json), "", true)); EXPECT_TRUE(manager_->listeners().front().get().reverseWriteFilterOrder()); } @@ -423,7 +423,7 @@ TEST_F(ListenerManagerImplTest, ModifyOnlyDrainType) { ListenerHandle* listener_foo = expectListenerCreate(false, envoy::api::v2::Listener_DrainType_MODIFY_ONLY); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_yaml), "", true)); checkStats(1, 0, 0, 0, 1, 0); @@ -444,7 +444,7 @@ TEST_F(ListenerManagerImplTest, AddListenerAddressNotMatching) { )EOF"; ListenerHandle* listener_foo = expectListenerCreate(false); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); checkStats(1, 0, 0, 0, 1, 0); @@ -494,7 +494,7 @@ TEST_F(ListenerManagerImplTest, AddListenerOnIpv4OnlySetups) { EXPECT_CALL(os_sys_calls, socket(AF_INET, _, 0)).WillOnce(Return(Api::SysCallIntResult{5, 0})); EXPECT_CALL(os_sys_calls, socket(AF_INET6, _, 0)).WillOnce(Return(Api::SysCallIntResult{-1, 0})); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); checkStats(1, 0, 0, 0, 1, 0); @@ -524,7 +524,7 @@ TEST_F(ListenerManagerImplTest, AddListenerOnIpv6OnlySetups) { EXPECT_CALL(os_sys_calls, socket(AF_INET, _, 0)).WillOnce(Return(Api::SysCallIntResult{-1, 0})); EXPECT_CALL(os_sys_calls, socket(AF_INET6, _, 0)).WillOnce(Return(Api::SysCallIntResult{5, 0})); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); checkStats(1, 0, 0, 0, 1, 0); @@ -547,7 +547,7 @@ TEST_F(ListenerManagerImplTest, UpdateRemoveNotModifiableListener) { )EOF"; ListenerHandle* listener_foo = expectListenerCreate(false); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", false)); checkStats(1, 0, 0, 0, 1, 0); checkConfigDump(R"EOF( @@ -618,7 +618,7 @@ filter_chains: {} )EOF"; ListenerHandle* listener_foo = expectListenerCreate(false); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE( manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_yaml), "version1", true)); checkStats(1, 0, 0, 0, 1, 0); @@ -760,7 +760,7 @@ filter_chains: {} )EOF"; ListenerHandle* listener_bar = expectListenerCreate(false); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_TRUE( manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_bar_yaml), "version4", true)); @@ -781,7 +781,7 @@ filter_chains: {} )EOF"; ListenerHandle* listener_baz = expectListenerCreate(true); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(listener_baz->target_, initialize(_)); EXPECT_TRUE( manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_baz_yaml), "version5", true)); @@ -887,7 +887,7 @@ TEST_F(ListenerManagerImplTest, AddDrainingListener) { ON_CALL(*listener_factory_.socket_, localAddress()).WillByDefault(ReturnRef(local_address)); ListenerHandle* listener_foo = expectListenerCreate(false); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); worker_->callAddCompletion(true); @@ -931,7 +931,7 @@ TEST_F(ListenerManagerImplTest, CantBindSocket) { )EOF"; ListenerHandle* listener_foo = expectListenerCreate(true); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)) + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)) .WillOnce(Throw(EnvoyException("can't bind"))); EXPECT_CALL(*listener_foo, onDestroy()); EXPECT_THROW(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true), @@ -953,7 +953,7 @@ TEST_F(ListenerManagerImplTest, ListenerDraining) { )EOF"; ListenerHandle* listener_foo = expectListenerCreate(false); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); worker_->callAddCompletion(true); @@ -1005,7 +1005,7 @@ TEST_F(ListenerManagerImplTest, RemoveListener) { )EOF"; ListenerHandle* listener_foo = expectListenerCreate(true); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(listener_foo->target_, initialize(_)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); EXPECT_EQ(0UL, manager_->listeners().size()); @@ -1019,7 +1019,7 @@ TEST_F(ListenerManagerImplTest, RemoveListener) { // Add foo again and initialize it. listener_foo = expectListenerCreate(true); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(listener_foo->target_, initialize(_)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); checkStats(2, 0, 1, 1, 0, 0); @@ -1078,7 +1078,7 @@ TEST_F(ListenerManagerImplTest, AddListenerFailure) { )EOF"; ListenerHandle* listener_foo = expectListenerCreate(false); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); @@ -1127,7 +1127,7 @@ TEST_F(ListenerManagerImplTest, DuplicateAddressDontBind) { )EOF"; ListenerHandle* listener_foo = expectListenerCreate(true); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, false)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, false)); EXPECT_CALL(listener_foo->target_, initialize(_)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); @@ -1190,7 +1190,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithDestinationP Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -1235,7 +1235,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithDestinationI Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -1280,7 +1280,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithServerNamesM Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -1325,7 +1325,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithTransportPro Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -1366,7 +1366,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithApplicationP Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -1407,7 +1407,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithSourceTypeMa Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -1474,7 +1474,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainWithSourceType Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -1549,7 +1549,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithDestinati Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -1629,7 +1629,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithDestinati Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -1718,7 +1718,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithServerNam Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -1787,7 +1787,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithTransport Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -1830,7 +1830,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithApplicati Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -1875,7 +1875,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithMultipleR Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -1940,7 +1940,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithDifferent Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); } @@ -1975,7 +1975,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); } @@ -2050,7 +2050,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, TlsFilterChainWithoutTlsInspector Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -2077,7 +2077,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SniFilterChainWithoutTlsInspector Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -2104,7 +2104,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, AlpnFilterChainWithoutTlsInspecto Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -2132,7 +2132,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, CustomTransportProtocolWithSniWit Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -2168,7 +2168,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, TlsCertificateInline) { )EOF"); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); } @@ -2190,7 +2190,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, TlsCertificateChainInlinePrivateK Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); } @@ -2330,7 +2330,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, OriginalDstFilter) { Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -2416,7 +2416,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, OriginalDstTestFilter) { Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -2486,7 +2486,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, OriginalDstTestFilterOptionFail) )EOF", Network::Address::IpVersion::v4); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_THROW_WITH_MESSAGE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true), EnvoyException, @@ -2553,7 +2553,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, OriginalDstTestFilterIPv6) { Network::Address::IpVersion::v6); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); @@ -2623,7 +2623,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, OriginalDstTestFilterOptionFailIP )EOF", Network::Address::IpVersion::v6); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_THROW_WITH_MESSAGE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true), EnvoyException, @@ -2642,8 +2642,9 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, TransparentFreebindListenerDisabl - filters: )EOF", Network::Address::IpVersion::v4); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)) + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)) .WillOnce(Invoke([&](Network::Address::InstanceConstSharedPtr, + Network::Address::SocketType, const Network::Socket::OptionsSharedPtr& options, bool) -> Network::SocketSharedPtr { EXPECT_EQ(options, nullptr); @@ -2673,8 +2674,9 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, TransparentListenerEnabled) { )EOF", Network::Address::IpVersion::v4); if (ENVOY_SOCKET_IP_TRANSPARENT.has_value()) { - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)) + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)) .WillOnce(Invoke([this](Network::Address::InstanceConstSharedPtr, + Network::Address::SocketType, const Network::Socket::OptionsSharedPtr& options, bool) -> Network::SocketSharedPtr { EXPECT_NE(options.get(), nullptr); @@ -2724,8 +2726,9 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, FreebindListenerEnabled) { )EOF", Network::Address::IpVersion::v4); if (ENVOY_SOCKET_IP_FREEBIND.has_value()) { - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)) + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)) .WillOnce(Invoke([this](Network::Address::InstanceConstSharedPtr, + Network::Address::SocketType, const Network::Socket::OptionsSharedPtr& options, bool) -> Network::SocketSharedPtr { EXPECT_NE(options.get(), nullptr); @@ -2771,8 +2774,9 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, LiteralSockoptListenerEnabled) { ] )EOF", Network::Address::IpVersion::v4); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)) + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)) .WillOnce(Invoke([this](Network::Address::InstanceConstSharedPtr, + Network::Address::SocketType, const Network::Socket::OptionsSharedPtr& options, bool) -> Network::SocketSharedPtr { EXPECT_NE(options.get(), nullptr); @@ -2814,7 +2818,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, AddressResolver) { Registry::InjectFactory register_resolver(mock_resolver); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); } @@ -2836,7 +2840,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, CRLFilename) { Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); } @@ -2861,7 +2865,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, CRLInline) { Network::Address::IpVersion::v4); EXPECT_CALL(server_.random_, uuid()); - EXPECT_CALL(listener_factory_, createListenSocket(_, _, true)); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); } From 61c083826dd02812d537b7ce78f721ed34060dde Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Mon, 31 Dec 2018 14:17:40 -0500 Subject: [PATCH 03/11] In AddressJson::translateAddress(), set SocketAddress.protocol based on URL scheme. Signed-off-by: Michael Warres --- source/common/config/address_json.cc | 26 +++++-- test/common/config/BUILD | 8 +++ test/common/config/address_json_test.cc | 94 +++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 5 deletions(-) create mode 100644 test/common/config/address_json_test.cc diff --git a/source/common/config/address_json.cc b/source/common/config/address_json.cc index 7206e7a3d8799..efa075bd2c6b1 100644 --- a/source/common/config/address_json.cc +++ b/source/common/config/address_json.cc @@ -16,6 +16,13 @@ void AddressJson::translateAddress(const std::string& json_address, bool url, bo if (instance->type() == Network::Address::Type::Ip) { address.mutable_socket_address()->set_address(instance->ip()->addressAsString()); address.mutable_socket_address()->set_port_value(instance->ip()->port()); + if (url) { + if (Network::Utility::urlIsTcpScheme(json_address)) { + address.mutable_socket_address()->set_protocol(envoy::api::v2::core::SocketAddress::TCP); + } else if (Network::Utility::urlIsUdpScheme(json_address)) { + address.mutable_socket_address()->set_protocol(envoy::api::v2::core::SocketAddress::UDP); + } + } } else { ASSERT(instance->type() == Network::Address::Type::Pipe); address.mutable_pipe()->set_path(instance->asString()); @@ -25,12 +32,21 @@ void AddressJson::translateAddress(const std::string& json_address, bool url, bo // We don't have v1 JSON with unresolved addresses in non-URL form. ASSERT(url); - // Non-TCP scheme (e.g. Unix scheme) is not supported with unresolved address. - if (!Network::Utility::urlIsTcpScheme(json_address)) { - throw EnvoyException(fmt::format("unresolved URL must be TCP scheme, got: {}", json_address)); + if (Network::Utility::urlIsTcpScheme(json_address)) { + address.mutable_socket_address()->set_address(Network::Utility::hostFromTcpUrl(json_address)); + address.mutable_socket_address()->set_port_value( + Network::Utility::portFromTcpUrl(json_address)); + address.mutable_socket_address()->set_protocol(envoy::api::v2::core::SocketAddress::TCP); + } else if (Network::Utility::urlIsUdpScheme(json_address)) { + address.mutable_socket_address()->set_address(Network::Utility::hostFromUdpUrl(json_address)); + address.mutable_socket_address()->set_port_value( + Network::Utility::portFromUdpUrl(json_address)); + address.mutable_socket_address()->set_protocol(envoy::api::v2::core::SocketAddress::UDP); + } else { + // Non-TCP/UDP scheme (e.g. Unix scheme) is not supported with unresolved address. + throw EnvoyException( + fmt::format("unresolved URL must be TCP or UDP scheme, got: {}", json_address)); } - address.mutable_socket_address()->set_address(Network::Utility::hostFromTcpUrl(json_address)); - address.mutable_socket_address()->set_port_value(Network::Utility::portFromTcpUrl(json_address)); } void AddressJson::translateCidrRangeList( diff --git a/test/common/config/BUILD b/test/common/config/BUILD index 918e263d850a9..db236dcaf97d5 100644 --- a/test/common/config/BUILD +++ b/test/common/config/BUILD @@ -9,6 +9,14 @@ load( envoy_package() +envoy_cc_test( + name = "address_json_test", + srcs = ["address_json_test.cc"], + deps = [ + "//source/common/config:address_json_lib", + ], +) + envoy_cc_test( name = "filesystem_subscription_impl_test", srcs = ["filesystem_subscription_impl_test.cc"], diff --git a/test/common/config/address_json_test.cc b/test/common/config/address_json_test.cc new file mode 100644 index 0000000000000..1a6c3411f69ed --- /dev/null +++ b/test/common/config/address_json_test.cc @@ -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 From 6efe767474b9f75b451f16bc94e0e1dbea238352 Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Mon, 31 Dec 2018 14:34:33 -0500 Subject: [PATCH 04/11] Add ListenerManagerImplWithRealFiltersTest.UdpAddress test. Signed-off-by: Michael Warres --- test/server/listener_manager_impl_test.cc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index 03e129940bd82..483d5edb3e20c 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -293,6 +293,21 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SslContext) { EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); } +TEST_F(ListenerManagerImplWithRealFiltersTest, UdpAddress) { + const std::string json = R"EOF( + { + "address": "udp://127.0.0.1:1234", + "filters": [] + } + )EOF"; + + EXPECT_CALL(server_.random_, uuid()); + EXPECT_CALL(listener_factory_, + createListenSocket(_, Network::Address::SocketType::Datagram, _, true)); + manager_->addOrUpdateListener(parseListenerFromJson(json), "", true); + EXPECT_EQ(1U, manager_->listeners().size()); +} + TEST_F(ListenerManagerImplWithRealFiltersTest, BadListenerConfig) { const std::string json = R"EOF( { From cbd1eb1f912a64457e604e47266c16ff1f54862e Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Mon, 31 Dec 2018 17:16:16 -0500 Subject: [PATCH 05/11] Add Network::Socket::socketType() method and implementations. Signed-off-by: Michael Warres --- include/envoy/network/listen_socket.h | 5 +++++ source/common/network/listen_socket_impl.h | 6 ++++++ test/common/network/listen_socket_impl_test.cc | 3 +++ test/mocks/network/mocks.h | 2 ++ 4 files changed, 16 insertions(+) diff --git a/include/envoy/network/listen_socket.h b/include/envoy/network/listen_socket.h index 6b475c318e13c..fd5e897131b0d 100644 --- a/include/envoy/network/listen_socket.h +++ b/include/envoy/network/listen_socket.h @@ -35,6 +35,11 @@ class Socket { */ virtual int fd() const PURE; + /** + * @return the type (stream or datagram) of the socket. + */ + virtual Address::SocketType socketType() const PURE; + /** * Close the underlying socket. */ diff --git a/source/common/network/listen_socket_impl.h b/source/common/network/listen_socket_impl.h index 51b86449e6dc8..86d1cf81bf545 100644 --- a/source/common/network/listen_socket_impl.h +++ b/source/common/network/listen_socket_impl.h @@ -92,6 +92,8 @@ template class NetworkListenSocket : public ListenSocketImpl { setListenSocketOptions(options); } + Address::SocketType socketType() const override { return T::type; } + protected: void setPrebindSocketOptions(); }; @@ -106,6 +108,7 @@ class UdsListenSocket : public ListenSocketImpl { public: UdsListenSocket(const Address::InstanceConstSharedPtr& address); UdsListenSocket(int fd, const Address::InstanceConstSharedPtr& address); + Address::SocketType socketType() const override { return Address::SocketType::Stream; } }; class ConnectionSocketImpl : public SocketImpl, public ConnectionSocket { @@ -114,6 +117,9 @@ class ConnectionSocketImpl : public SocketImpl, public ConnectionSocket { const Address::InstanceConstSharedPtr& remote_address) : SocketImpl(fd, local_address), remote_address_(remote_address) {} + // Network::Socket + Address::SocketType socketType() const override { return Address::SocketType::Stream; } + // Network::ConnectionSocket const Address::InstanceConstSharedPtr& remoteAddress() const override { return remote_address_; } void setLocalAddress(const Address::InstanceConstSharedPtr& local_address, diff --git a/test/common/network/listen_socket_impl_test.cc b/test/common/network/listen_socket_impl_test.cc index 5995e93cf6443..cc9bb42e05319 100644 --- a/test/common/network/listen_socket_impl_test.cc +++ b/test/common/network/listen_socket_impl_test.cc @@ -70,6 +70,7 @@ class ListenSocketImplTest : public testing::TestWithParam { } continue; } + // TODO (conqerAtapple): This is unfortunate. We should be able to templatize this // instead of if block. if (NetworkSocketTrait::type == Address::SocketType::Stream) { @@ -78,6 +79,7 @@ class ListenSocketImplTest : public testing::TestWithParam { EXPECT_EQ(addr->ip()->port(), socket1->localAddress()->ip()->port()); EXPECT_EQ(addr->ip()->addressAsString(), socket1->localAddress()->ip()->addressAsString()); + EXPECT_EQ(Type, socket1->socketType()); auto option2 = std::make_unique(); auto options2 = std::make_shared>(); @@ -103,6 +105,7 @@ class ListenSocketImplTest : public testing::TestWithParam { EXPECT_EQ(version_, socket->localAddress()->ip()->version()); EXPECT_EQ(loopback->ip()->addressAsString(), socket->localAddress()->ip()->addressAsString()); EXPECT_GT(socket->localAddress()->ip()->port(), 0U); + EXPECT_EQ(Type, socket->socketType()); } }; diff --git a/test/mocks/network/mocks.h b/test/mocks/network/mocks.h index 46352cfdca111..e8bd7c70eb093 100644 --- a/test/mocks/network/mocks.h +++ b/test/mocks/network/mocks.h @@ -193,6 +193,7 @@ class MockListenSocket : public Socket { MOCK_CONST_METHOD0(localAddress, const Address::InstanceConstSharedPtr&()); MOCK_CONST_METHOD0(fd, int()); + MOCK_CONST_METHOD0(socketType, Address::SocketType()); MOCK_METHOD0(close, void()); MOCK_METHOD1(addOption_, void(const Socket::OptionConstSharedPtr& option)); MOCK_METHOD1(addOptions_, void(const Socket::OptionsSharedPtr& options)); @@ -238,6 +239,7 @@ class MockConnectionSocket : public ConnectionSocket { MOCK_METHOD1(addOptions_, void(const Socket::OptionsSharedPtr&)); MOCK_CONST_METHOD0(options, const Network::ConnectionSocket::OptionsSharedPtr&()); MOCK_CONST_METHOD0(fd, int()); + MOCK_CONST_METHOD0(socketType, Address::SocketType()); MOCK_METHOD0(close, void()); Address::InstanceConstSharedPtr local_address_; From f3bfe1db7d3e8b7a5f2c3c0ebc8c600bec98957c Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Mon, 31 Dec 2018 17:20:46 -0500 Subject: [PATCH 06/11] Fix formatting. Signed-off-by: Michael Warres --- source/server/listener_manager_impl.cc | 8 +-- test/mocks/server/mocks.cc | 19 +++---- test/server/listener_manager_impl_test.cc | 69 +++++++++++------------ 3 files changed, 46 insertions(+), 50 deletions(-) diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index 8bb5025dd7239..087d8fc72e454 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -96,11 +96,9 @@ ProdListenerComponentFactory::createListenerFilterFactoryList_( return ret; } -Network::SocketSharedPtr -ProdListenerComponentFactory::createListenSocket(Network::Address::InstanceConstSharedPtr address, - Network::Address::SocketType socket_type, - 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 || diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index d4ea5bcecdbd7..5729d1608a3e6 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -84,16 +84,15 @@ MockOverloadManager::~MockOverloadManager() {} MockListenerComponentFactory::MockListenerComponentFactory() : socket_(std::make_shared>()) { ON_CALL(*this, createListenSocket(_, _, _, _)) - .WillByDefault(Invoke([&](Network::Address::InstanceConstSharedPtr, - Network::Address::SocketType, - const Network::Socket::OptionsSharedPtr& options, - bool) -> Network::SocketSharedPtr { - if (!Network::Socket::applyOptions(options, *socket_, - envoy::api::v2::core::SocketOption::STATE_PREBIND)) { - throw EnvoyException("MockListenerComponentFactory: Setting socket options failed"); - } - return socket_; - })); + .WillByDefault(Invoke( + [&](Network::Address::InstanceConstSharedPtr, Network::Address::SocketType, + const Network::Socket::OptionsSharedPtr& options, bool) -> Network::SocketSharedPtr { + if (!Network::Socket::applyOptions(options, *socket_, + envoy::api::v2::core::SocketOption::STATE_PREBIND)) { + throw EnvoyException("MockListenerComponentFactory: Setting socket options failed"); + } + return socket_; + })); } MockListenerComponentFactory::~MockListenerComponentFactory() {} diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index 483d5edb3e20c..a2134f90f5045 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -2658,8 +2658,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, TransparentFreebindListenerDisabl )EOF", Network::Address::IpVersion::v4); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)) - .WillOnce(Invoke([&](Network::Address::InstanceConstSharedPtr, - Network::Address::SocketType, + .WillOnce(Invoke([&](Network::Address::InstanceConstSharedPtr, Network::Address::SocketType, const Network::Socket::OptionsSharedPtr& options, bool) -> Network::SocketSharedPtr { EXPECT_EQ(options, nullptr); @@ -2690,17 +2689,17 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, TransparentListenerEnabled) { Network::Address::IpVersion::v4); if (ENVOY_SOCKET_IP_TRANSPARENT.has_value()) { EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)) - .WillOnce(Invoke([this](Network::Address::InstanceConstSharedPtr, - Network::Address::SocketType, - const Network::Socket::OptionsSharedPtr& options, - bool) -> Network::SocketSharedPtr { - EXPECT_NE(options.get(), nullptr); - EXPECT_EQ(options->size(), 2); - EXPECT_TRUE( - Network::Socket::applyOptions(options, *listener_factory_.socket_, - envoy::api::v2::core::SocketOption::STATE_PREBIND)); - return listener_factory_.socket_; - })); + .WillOnce( + Invoke([this](Network::Address::InstanceConstSharedPtr, Network::Address::SocketType, + const Network::Socket::OptionsSharedPtr& options, + bool) -> Network::SocketSharedPtr { + EXPECT_NE(options.get(), nullptr); + EXPECT_EQ(options->size(), 2); + EXPECT_TRUE( + Network::Socket::applyOptions(options, *listener_factory_.socket_, + envoy::api::v2::core::SocketOption::STATE_PREBIND)); + return listener_factory_.socket_; + })); // Expecting the socket option to bet set twice, once pre-bind, once post-bind. EXPECT_CALL(os_sys_calls, setsockopt_(_, ENVOY_SOCKET_IP_TRANSPARENT.value().first, @@ -2742,17 +2741,17 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, FreebindListenerEnabled) { Network::Address::IpVersion::v4); if (ENVOY_SOCKET_IP_FREEBIND.has_value()) { EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)) - .WillOnce(Invoke([this](Network::Address::InstanceConstSharedPtr, - Network::Address::SocketType, - const Network::Socket::OptionsSharedPtr& options, - bool) -> Network::SocketSharedPtr { - EXPECT_NE(options.get(), nullptr); - EXPECT_EQ(options->size(), 1); - EXPECT_TRUE( - Network::Socket::applyOptions(options, *listener_factory_.socket_, - envoy::api::v2::core::SocketOption::STATE_PREBIND)); - return listener_factory_.socket_; - })); + .WillOnce( + Invoke([this](Network::Address::InstanceConstSharedPtr, Network::Address::SocketType, + const Network::Socket::OptionsSharedPtr& options, + bool) -> Network::SocketSharedPtr { + EXPECT_NE(options.get(), nullptr); + EXPECT_EQ(options->size(), 1); + EXPECT_TRUE( + Network::Socket::applyOptions(options, *listener_factory_.socket_, + envoy::api::v2::core::SocketOption::STATE_PREBIND)); + return listener_factory_.socket_; + })); EXPECT_CALL(os_sys_calls, setsockopt_(_, ENVOY_SOCKET_IP_FREEBIND.value().first, ENVOY_SOCKET_IP_FREEBIND.value().second, _, sizeof(int))) .WillOnce(Invoke([](int, int, int, const void* optval, socklen_t) -> int { @@ -2790,17 +2789,17 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, LiteralSockoptListenerEnabled) { )EOF", Network::Address::IpVersion::v4); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)) - .WillOnce(Invoke([this](Network::Address::InstanceConstSharedPtr, - Network::Address::SocketType, - const Network::Socket::OptionsSharedPtr& options, - bool) -> Network::SocketSharedPtr { - EXPECT_NE(options.get(), nullptr); - EXPECT_EQ(options->size(), 3); - EXPECT_TRUE( - Network::Socket::applyOptions(options, *listener_factory_.socket_, - envoy::api::v2::core::SocketOption::STATE_PREBIND)); - return listener_factory_.socket_; - })); + .WillOnce( + Invoke([this](Network::Address::InstanceConstSharedPtr, Network::Address::SocketType, + const Network::Socket::OptionsSharedPtr& options, + bool) -> Network::SocketSharedPtr { + EXPECT_NE(options.get(), nullptr); + EXPECT_EQ(options->size(), 3); + EXPECT_TRUE( + Network::Socket::applyOptions(options, *listener_factory_.socket_, + envoy::api::v2::core::SocketOption::STATE_PREBIND)); + return listener_factory_.socket_; + })); EXPECT_CALL(os_sys_calls, setsockopt_(_, 1, 2, _, sizeof(int))) .WillOnce(Invoke([](int, int, int, const void* optval, socklen_t) -> int { EXPECT_EQ(3, *static_cast(optval)); From 86ad19733b44b2c81f5e181535ce3a65eb8fe725 Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Wed, 2 Jan 2019 11:08:36 -0500 Subject: [PATCH 07/11] Fix UtilityTest.UnixClusterDns test failure. Signed-off-by: Michael Warres --- test/common/config/utility_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/config/utility_test.cc b/test/common/config/utility_test.cc index d049e5a2b8c52..da1be2665c407 100644 --- a/test/common/config/utility_test.cc +++ b/test/common/config/utility_test.cc @@ -188,7 +188,7 @@ TEST(UtilityTest, UnixClusterDns) { Stats::StatsOptionsImpl stats_options; EXPECT_THROW_WITH_MESSAGE( Config::CdsJson::translateCluster(*json_object_ptr, eds_config, cluster, stats_options), - EnvoyException, "unresolved URL must be TCP scheme, got: unix:///test.sock"); + EnvoyException, "unresolved URL must be TCP or UDP scheme, got: unix:///test.sock"); } TEST(UtilityTest, UnixClusterStatic) { From 96b9649f416fffa7a38b7d4d69aed3a4e686bbe7 Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Wed, 2 Jan 2019 15:03:01 -0500 Subject: [PATCH 08/11] Address @alyssawilk review comments. Signed-off-by: Michael Warres --- source/common/network/utility.cc | 6 +++--- source/server/listener_manager_impl.cc | 7 +++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc index 7c39168014dc8..e13453ceff744 100644 --- a/source/common/network/utility.cc +++ b/source/common/network/utility.cc @@ -505,14 +505,14 @@ Address::SocketType Utility::protobufAddressSocketType(const envoy::api::v2::core::Address& proto_address) { switch (proto_address.address_case()) { case envoy::api::v2::core::Address::kSocketAddress: { - auto p = proto_address.socket_address().protocol(); - switch (p) { + auto protocol = proto_address.socket_address().protocol(); + switch (protocol) { case envoy::api::v2::core::SocketAddress::TCP: return Address::SocketType::Stream; case envoy::api::v2::core::SocketAddress::UDP: return Address::SocketType::Datagram; default: - throw EnvoyException(fmt::format("unknown protocol value: {}", p)); + throw EnvoyException(fmt::format("unknown protocol value: {}", protocol)); } } case envoy::api::v2::core::Address::kPipe: diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index 087d8fc72e454..ea601cc6ee8c9 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -25,6 +25,7 @@ #include "extensions/transport_sockets/well_known_names.h" #include "absl/strings/match.h" +#include "absl/strings/str_cat.h" namespace Envoy { namespace Server { @@ -123,8 +124,10 @@ Network::SocketSharedPtr ProdListenerComponentFactory::createListenSocket( return std::make_shared(address); } - const std::string scheme = (socket_type == Network::Address::SocketType::Stream) ? "tcp" : "udp"; - const std::string addr = fmt::format("{}://{}", scheme, address->asString()); + const std::string scheme = (socket_type == Network::Address::SocketType::Stream) + ? Network::Utility::TCP_SCHEME + : Network::Utility::UDP_SCHEME; + const std::string addr = absl::StrCat(scheme, address->asString()); const int fd = server_.hotRestart().duplicateParentListenSocket(addr); if (fd != -1) { ENVOY_LOG(debug, "obtained socket for address {} from parent", addr); From 3a3be2b00635a73839668e52d634dbeec429cbe2 Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Thu, 3 Jan 2019 18:22:01 -0500 Subject: [PATCH 09/11] Revert changes to AddressJson::translateAddress() and update test. Modify ListenerManagerImplWithRealFiltersTest.UdpAddress to specify listener protobuf directly via protobuf text format, rather than deprecated JSON format. Signed-off-by: Michael Warres --- source/common/config/address_json.cc | 26 ++----- test/common/config/BUILD | 8 -- test/common/config/address_json_test.cc | 94 ----------------------- test/server/BUILD | 1 + test/server/listener_manager_impl_test.cc | 19 +++-- 5 files changed, 19 insertions(+), 129 deletions(-) delete mode 100644 test/common/config/address_json_test.cc diff --git a/source/common/config/address_json.cc b/source/common/config/address_json.cc index efa075bd2c6b1..7206e7a3d8799 100644 --- a/source/common/config/address_json.cc +++ b/source/common/config/address_json.cc @@ -16,13 +16,6 @@ void AddressJson::translateAddress(const std::string& json_address, bool url, bo if (instance->type() == Network::Address::Type::Ip) { address.mutable_socket_address()->set_address(instance->ip()->addressAsString()); address.mutable_socket_address()->set_port_value(instance->ip()->port()); - if (url) { - if (Network::Utility::urlIsTcpScheme(json_address)) { - address.mutable_socket_address()->set_protocol(envoy::api::v2::core::SocketAddress::TCP); - } else if (Network::Utility::urlIsUdpScheme(json_address)) { - address.mutable_socket_address()->set_protocol(envoy::api::v2::core::SocketAddress::UDP); - } - } } else { ASSERT(instance->type() == Network::Address::Type::Pipe); address.mutable_pipe()->set_path(instance->asString()); @@ -32,21 +25,12 @@ void AddressJson::translateAddress(const std::string& json_address, bool url, bo // We don't have v1 JSON with unresolved addresses in non-URL form. ASSERT(url); - if (Network::Utility::urlIsTcpScheme(json_address)) { - address.mutable_socket_address()->set_address(Network::Utility::hostFromTcpUrl(json_address)); - address.mutable_socket_address()->set_port_value( - Network::Utility::portFromTcpUrl(json_address)); - address.mutable_socket_address()->set_protocol(envoy::api::v2::core::SocketAddress::TCP); - } else if (Network::Utility::urlIsUdpScheme(json_address)) { - address.mutable_socket_address()->set_address(Network::Utility::hostFromUdpUrl(json_address)); - address.mutable_socket_address()->set_port_value( - Network::Utility::portFromUdpUrl(json_address)); - address.mutable_socket_address()->set_protocol(envoy::api::v2::core::SocketAddress::UDP); - } else { - // Non-TCP/UDP scheme (e.g. Unix scheme) is not supported with unresolved address. - throw EnvoyException( - fmt::format("unresolved URL must be TCP or UDP scheme, got: {}", json_address)); + // Non-TCP scheme (e.g. Unix scheme) is not supported with unresolved address. + if (!Network::Utility::urlIsTcpScheme(json_address)) { + throw EnvoyException(fmt::format("unresolved URL must be TCP scheme, got: {}", json_address)); } + address.mutable_socket_address()->set_address(Network::Utility::hostFromTcpUrl(json_address)); + address.mutable_socket_address()->set_port_value(Network::Utility::portFromTcpUrl(json_address)); } void AddressJson::translateCidrRangeList( diff --git a/test/common/config/BUILD b/test/common/config/BUILD index 99064e7e881b3..d80c5b07dd781 100644 --- a/test/common/config/BUILD +++ b/test/common/config/BUILD @@ -10,14 +10,6 @@ load( envoy_package() -envoy_cc_test( - name = "address_json_test", - srcs = ["address_json_test.cc"], - deps = [ - "//source/common/config:address_json_lib", - ], -) - envoy_cc_test( name = "filesystem_subscription_impl_test", srcs = ["filesystem_subscription_impl_test.cc"], diff --git a/test/common/config/address_json_test.cc b/test/common/config/address_json_test.cc deleted file mode 100644 index 1a6c3411f69ed..0000000000000 --- a/test/common/config/address_json_test.cc +++ /dev/null @@ -1,94 +0,0 @@ -#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 diff --git a/test/server/BUILD b/test/server/BUILD index b48f39d3c6bc8..5054dea51c9f8 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -160,6 +160,7 @@ envoy_cc_test( "//source/common/network:listen_socket_lib", "//source/common/network:socket_option_lib", "//source/common/network:utility_lib", + "//source/common/protobuf", "//source/common/ssl:ssl_socket_lib", "//source/extensions/filters/listener/original_dst:config", "//source/extensions/filters/listener/tls_inspector:config", diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index a2134f90f5045..cc12cdd052e29 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -10,6 +10,7 @@ #include "common/network/listen_socket_impl.h" #include "common/network/socket_option_impl.h" #include "common/network/utility.h" +#include "common/protobuf/protobuf.h" #include "common/ssl/ssl_socket.h" #include "server/configuration_impl.h" @@ -294,17 +295,23 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SslContext) { } TEST_F(ListenerManagerImplWithRealFiltersTest, UdpAddress) { - const std::string json = R"EOF( - { - "address": "udp://127.0.0.1:1234", - "filters": [] - } + const std::string proto_text = R"EOF( + address: { + socket_address: { + protocol: UDP + address: "127.0.0.1" + port_value: 1234 + } + } + filter_chains: {} )EOF"; + envoy::api::v2::Listener listener_proto; + EXPECT_TRUE(Protobuf::TextFormat::ParseFromString(proto_text, &listener_proto)); EXPECT_CALL(server_.random_, uuid()); EXPECT_CALL(listener_factory_, createListenSocket(_, Network::Address::SocketType::Datagram, _, true)); - manager_->addOrUpdateListener(parseListenerFromJson(json), "", true); + manager_->addOrUpdateListener(listener_proto, "", true); EXPECT_EQ(1U, manager_->listeners().size()); } From f177e572a7ae8091f9e330999764b42ba517686a Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Fri, 4 Jan 2019 16:34:55 -0500 Subject: [PATCH 10/11] Forgot to revert config/utility_test.cc when backing out AddressJson::translateAddress() changes. Signed-off-by: Michael Warres --- test/common/config/utility_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/config/utility_test.cc b/test/common/config/utility_test.cc index da1be2665c407..d049e5a2b8c52 100644 --- a/test/common/config/utility_test.cc +++ b/test/common/config/utility_test.cc @@ -188,7 +188,7 @@ TEST(UtilityTest, UnixClusterDns) { Stats::StatsOptionsImpl stats_options; EXPECT_THROW_WITH_MESSAGE( Config::CdsJson::translateCluster(*json_object_ptr, eds_config, cluster, stats_options), - EnvoyException, "unresolved URL must be TCP or UDP scheme, got: unix:///test.sock"); + EnvoyException, "unresolved URL must be TCP scheme, got: unix:///test.sock"); } TEST(UtilityTest, UnixClusterStatic) { From 393f84b23915ed078237012ed4bfa43349f69338 Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Sat, 5 Jan 2019 13:09:44 -0500 Subject: [PATCH 11/11] Address comments from @mattklein123. Signed-off-by: Michael Warres --- source/common/network/utility.cc | 2 +- source/server/listener_manager_impl.cc | 2 +- test/common/network/utility_test.cc | 6 ------ 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc index e13453ceff744..4d53ce3668bac 100644 --- a/source/common/network/utility.cc +++ b/source/common/network/utility.cc @@ -512,7 +512,7 @@ Utility::protobufAddressSocketType(const envoy::api::v2::core::Address& proto_ad case envoy::api::v2::core::SocketAddress::UDP: return Address::SocketType::Datagram; default: - throw EnvoyException(fmt::format("unknown protocol value: {}", protocol)); + NOT_REACHED_GCOVR_EXCL_LINE; } } case envoy::api::v2::core::Address::kPipe: diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index ea601cc6ee8c9..06b3a659331b0 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -38,7 +38,7 @@ std::string toString(Network::Address::SocketType socket_type) { case Network::Address::SocketType::Datagram: return "SocketType::Datagram"; default: - return fmt::format("unknown ({})", static_cast(socket_type)); + NOT_REACHED_GCOVR_EXCL_LINE; } } diff --git a/test/common/network/utility_test.cc b/test/common/network/utility_test.cc index 3f4245377c59e..910a8a2d0d9ff 100644 --- a/test/common/network/utility_test.cc +++ b/test/common/network/utility_test.cc @@ -336,12 +336,6 @@ TEST(NetworkUtility, ProtobufAddressSocketType) { proto_address.mutable_socket_address()->set_protocol(envoy::api::v2::core::SocketAddress::UDP); EXPECT_EQ(Address::SocketType::Datagram, Utility::protobufAddressSocketType(proto_address)); } - { - const auto kInvalidValue = static_cast(123); - envoy::api::v2::core::Address proto_address; - proto_address.mutable_socket_address()->set_protocol(kInvalidValue); - EXPECT_THROW(Utility::protobufAddressSocketType(proto_address), EnvoyException); - } { envoy::api::v2::core::Address proto_address; proto_address.mutable_pipe();