From 1c0cf91f5c7eec50373b9bfc09db5bbf91f555a6 Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Mon, 13 Jun 2022 16:40:31 -0700 Subject: [PATCH 01/12] add headers Signed-off-by: Kuat Yessenov --- envoy/network/transport_socket.h | 5 +++-- envoy/stream_info/filter_state.h | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/envoy/network/transport_socket.h b/envoy/network/transport_socket.h index ca892ed079d53..26e59f5f88387 100644 --- a/envoy/network/transport_socket.h +++ b/envoy/network/transport_socket.h @@ -231,9 +231,10 @@ class TransportSocketOptions { virtual absl::optional proxyProtocolOptions() const PURE; /** - * @return filter state from the downstream request or connection. + * @return filter state objects from the downstream request or connection + * that are marked as shared with the upstream. */ - virtual const StreamInfo::FilterStateSharedPtr& filterState() const PURE; + virtual const StreamInfo::FilterState::Objects& filterStateObjects() const PURE; }; using TransportSocketOptionsConstSharedPtr = std::shared_ptr; diff --git a/envoy/stream_info/filter_state.h b/envoy/stream_info/filter_state.h index a8bd8587c8195..48538e6a34d92 100644 --- a/envoy/stream_info/filter_state.h +++ b/envoy/stream_info/filter_state.h @@ -45,6 +45,16 @@ class FilterState { // // Note that order matters in this enum because it's assumed that life span grows as enum number // grows. + // + // Note that for more accurate book-keeping it is recommended to subscribe to + // the stream callbacks instead of relying on the destruction of the filter + // state. + // + // As a special case, objects that are marked as shared with the upstream + // become bound to the upstream connection life span, regardless of the + // original life span. That means, for example, that a shared request span + // object may outlive the original request when it is shared, because it may + // be captured by an upstream connection for the original downstream request. enum LifeSpan { FilterChain, Request, Connection, TopSpan = Connection }; class Object { @@ -66,6 +76,9 @@ class FilterState { virtual absl::optional serializeAsString() const { return absl::nullopt; } }; + using Objects = std::vector>; + using ObjectsPtr = std::unique_ptr; + virtual ~FilterState() = default; /** @@ -156,6 +169,20 @@ class FilterState { * either the top LifeSpan or the parent is not yet created. */ virtual FilterStateSharedPtr parent() const PURE; + + /** + * Mark a filter state object as shared with the upstream. Shared filter + * state objects are copied by reference from the downstream requests and + * connections to the upstream connection filter state, and also participate + * in the hashing decisions in the transport socket options when + * connections are re-used between downstream requests. + **/ + virtual void markAsSharedWithUpstream(absl::string_view data_name) PURE; + + /** + * @return objects that are shared with the upstream connection. + **/ + virtual Objects objectsSharedWithUpstream() const PURE; }; } // namespace StreamInfo From 52bd5c72415f0b8e8d9d9bf2fcb57e84210e36d1 Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Tue, 14 Jun 2022 10:26:47 -0700 Subject: [PATCH 02/12] draft Signed-off-by: Kuat Yessenov --- envoy/event/dispatcher.h | 12 +++--- envoy/network/client_connection_factory.h | 12 +++--- envoy/stream_info/filter_state.h | 40 +++++++++++++------ source/common/event/dispatcher_impl.cc | 13 +++--- source/common/event/dispatcher_impl.h | 11 ++--- source/common/network/BUILD | 1 + source/common/network/connection_impl.cc | 17 ++++++-- source/common/network/connection_impl.h | 7 +++- .../default_client_connection_factory.cc | 7 ++-- .../default_client_connection_factory.h | 12 +++--- .../network/happy_eyeballs_connection_impl.cc | 3 +- .../network/transport_socket_options_impl.cc | 13 +++++- .../network/transport_socket_options_impl.h | 16 +++++--- .../common/stream_info/filter_state_impl.cc | 17 ++++++-- source/common/stream_info/filter_state_impl.h | 6 +-- source/common/upstream/upstream_impl.cc | 15 ++++--- 16 files changed, 129 insertions(+), 73 deletions(-) diff --git a/envoy/event/dispatcher.h b/envoy/event/dispatcher.h index ab1974f72499a..5cfc3f87b8f8c 100644 --- a/envoy/event/dispatcher.h +++ b/envoy/event/dispatcher.h @@ -210,13 +210,15 @@ class Dispatcher : public DispatcherBase, public ScopeTracker { * @param transport_socket supplies a transport socket to be used by the connection. * @param options the socket options to be set on the underlying socket before anything is sent * on the socket. + * @param transport socket options used to create the transport socket. * @return Network::ClientConnectionPtr a client connection that is owned by the caller. */ - virtual Network::ClientConnectionPtr - createClientConnection(Network::Address::InstanceConstSharedPtr address, - Network::Address::InstanceConstSharedPtr source_address, - Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options) PURE; + virtual Network::ClientConnectionPtr createClientConnection( + Network::Address::InstanceConstSharedPtr address, + Network::Address::InstanceConstSharedPtr source_address, + Network::TransportSocketPtr&& transport_socket, + const Network::ConnectionSocket::OptionsSharedPtr& options, + const Network::TransportSocketOptionsConstSharedPtr& transport_options = nullptr) PURE; /** * @return Filesystem::WatcherPtr a filesystem watcher owned by the caller. diff --git a/envoy/network/client_connection_factory.h b/envoy/network/client_connection_factory.h index c9a69b2ca4f43..c6e89740a3fd5 100644 --- a/envoy/network/client_connection_factory.h +++ b/envoy/network/client_connection_factory.h @@ -26,12 +26,12 @@ class ClientConnectionFactory : public Config::UntypedFactory { * @return Network::ClientConnectionPtr The created connection. It's never nullptr but the return * connection may be closed upon return. */ - virtual Network::ClientConnectionPtr - createClientConnection(Event::Dispatcher& dispatcher, - Network::Address::InstanceConstSharedPtr address, - Network::Address::InstanceConstSharedPtr source_address, - Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options) PURE; + virtual Network::ClientConnectionPtr createClientConnection( + Event::Dispatcher& dispatcher, Network::Address::InstanceConstSharedPtr address, + Network::Address::InstanceConstSharedPtr source_address, + Network::TransportSocketPtr&& transport_socket, + const Network::ConnectionSocket::OptionsSharedPtr& options, + const Network::TransportSocketOptionsConstSharedPtr& transport_options = nullptr) PURE; }; } // namespace Network diff --git a/envoy/stream_info/filter_state.h b/envoy/stream_info/filter_state.h index 48538e6a34d92..c8e8b6f2ba286 100644 --- a/envoy/stream_info/filter_state.h +++ b/envoy/stream_info/filter_state.h @@ -26,7 +26,24 @@ using FilterStateSharedPtr = std::shared_ptr; */ class FilterState { public: - enum class StateType { ReadOnly, Mutable }; + enum StateType { + // Readonly objects must be set once. + ReadOnly = 0x1, + + // Mutable objects can be overwritten with the same state type, life span, and name. + Mutable = 0, + + // Mark a filter state object as shared with the upstream connection. + // Shared filter state objects are copied by reference from the downstream + // requests and connections to the upstream connection filter state, and + // also participate in the hashing decisions in the transport socket + // options when connections are re-used between downstream requests. + SharedWithUpstreamConnection = 0x2, + + // ATTENTION: MAKE SURE THIS REMAINS EQUAL TO THE LAST FLAG. + LastFlag = SharedWithUpstreamConnection, + }; + // Objects stored in the FilterState may have different life span. Life span is what controls // how long an object stored in FilterState lives. Implementation of this interface actually // stores objects in a (reverse) tree manner - multiple FilterStateImpl with shorter life span may @@ -76,7 +93,13 @@ class FilterState { virtual absl::optional serializeAsString() const { return absl::nullopt; } }; - using Objects = std::vector>; + struct FilterObject { + std::shared_ptr data_; + FilterState::StateType state_type_; + std::string name_{""}; + }; + + using Objects = std::vector; using ObjectsPtr = std::unique_ptr; virtual ~FilterState() = default; @@ -171,18 +194,9 @@ class FilterState { virtual FilterStateSharedPtr parent() const PURE; /** - * Mark a filter state object as shared with the upstream. Shared filter - * state objects are copied by reference from the downstream requests and - * connections to the upstream connection filter state, and also participate - * in the hashing decisions in the transport socket options when - * connections are re-used between downstream requests. - **/ - virtual void markAsSharedWithUpstream(absl::string_view data_name) PURE; - - /** - * @return objects that are shared with the upstream connection. + * @return filter objects that are shared with the upstream connection. **/ - virtual Objects objectsSharedWithUpstream() const PURE; + virtual ObjectsPtr objectsSharedWithUpstreamConnection() const PURE; }; } // namespace StreamInfo diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index b066e3ae6bb9c..104d72afc979e 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -155,11 +155,12 @@ DispatcherImpl::createServerConnection(Network::ConnectionSocketPtr&& socket, *this, std::move(socket), std::move(transport_socket), stream_info, true); } -Network::ClientConnectionPtr -DispatcherImpl::createClientConnection(Network::Address::InstanceConstSharedPtr address, - Network::Address::InstanceConstSharedPtr source_address, - Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options) { +Network::ClientConnectionPtr DispatcherImpl::createClientConnection( + Network::Address::InstanceConstSharedPtr address, + Network::Address::InstanceConstSharedPtr source_address, + Network::TransportSocketPtr&& transport_socket, + const Network::ConnectionSocket::OptionsSharedPtr& options, + const Network::TransportSocketOptionsConstSharedPtr& transport_options) { ASSERT(isThreadSafe()); auto* factory = Config::Utility::getFactoryByName( @@ -170,7 +171,7 @@ DispatcherImpl::createClientConnection(Network::Address::InstanceConstSharedPtr // expects a non-null connection as of today so we cannot gracefully handle unsupported address // type. return factory->createClientConnection(*this, address, source_address, - std::move(transport_socket), options); + std::move(transport_socket), options, transport_options); } FileEventPtr DispatcherImpl::createFileEvent(os_fd_t fd, FileReadyCb cb, FileTriggerType trigger, diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index b98ecaa400a5a..ffef3427923c3 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -66,11 +66,12 @@ class DispatcherImpl : Logger::Loggable, createServerConnection(Network::ConnectionSocketPtr&& socket, Network::TransportSocketPtr&& transport_socket, StreamInfo::StreamInfo& stream_info) override; - Network::ClientConnectionPtr - createClientConnection(Network::Address::InstanceConstSharedPtr address, - Network::Address::InstanceConstSharedPtr source_address, - Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options) override; + Network::ClientConnectionPtr createClientConnection( + Network::Address::InstanceConstSharedPtr address, + Network::Address::InstanceConstSharedPtr source_address, + Network::TransportSocketPtr&& transport_socket, + const Network::ConnectionSocket::OptionsSharedPtr& options, + const Network::TransportSocketOptionsConstSharedPtr& transport_options = nullptr) override; FileEventPtr createFileEvent(os_fd_t fd, FileReadyCb cb, FileTriggerType trigger, uint32_t events) override; Filesystem::WatcherPtr createFilesystemWatcher() override; diff --git a/source/common/network/BUILD b/source/common/network/BUILD index eb2976555c631..9c541477e94b5 100644 --- a/source/common/network/BUILD +++ b/source/common/network/BUILD @@ -442,6 +442,7 @@ envoy_cc_library( ":proxy_protocol_filter_state_lib", ":upstream_server_name_lib", ":upstream_subject_alt_names_lib", + "//envoy/common:hashable_interface", "//envoy/network:proxy_protocol_options_lib", "//envoy/network:transport_socket_interface", "//envoy/stream_info:filter_state_interface", diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc index f8bfa2ef03e10..0887877f4ac29 100644 --- a/source/common/network/connection_impl.cc +++ b/source/common/network/connection_impl.cc @@ -855,15 +855,18 @@ ClientConnectionImpl::ClientConnectionImpl( Event::Dispatcher& dispatcher, const Address::InstanceConstSharedPtr& remote_address, const Network::Address::InstanceConstSharedPtr& source_address, Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options) + const Network::ConnectionSocket::OptionsSharedPtr& options, + const Network::TransportSocketOptionsConstSharedPtr& transport_options) : ClientConnectionImpl(dispatcher, std::make_unique(remote_address, options), - source_address, std::move(transport_socket), options) {} + source_address, std::move(transport_socket), options, + transport_options) {} ClientConnectionImpl::ClientConnectionImpl( Event::Dispatcher& dispatcher, std::unique_ptr socket, const Address::InstanceConstSharedPtr& source_address, Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options) + const Network::ConnectionSocket::OptionsSharedPtr& options, + const Network::TransportSocketOptionsConstSharedPtr& transport_options) : ConnectionImpl(dispatcher, std::move(socket), std::move(transport_socket), stream_info_, false), stream_info_(dispatcher_.timeSource(), socket_->connectionInfoProviderSharedPtr()) { @@ -905,6 +908,14 @@ ClientConnectionImpl::ClientConnectionImpl( ioHandle().activateFileEvents(Event::FileReadyType::Write); } } + + if (transport_options) { + for (const auto& object : transport_options->filterStateObjects()) { + // TODO: handle exception + stream_info_.filterState()->setData(object.name_, object.data_, object.state_type_, + StreamInfo::FilterState::LifeSpan::Connection); + } + } } void ClientConnectionImpl::connect() { diff --git a/source/common/network/connection_impl.h b/source/common/network/connection_impl.h index cfdb2482a23c4..fe06344a5a4b7 100644 --- a/source/common/network/connection_impl.h +++ b/source/common/network/connection_impl.h @@ -262,11 +262,14 @@ class ClientConnectionImpl : public ConnectionImpl, virtual public ClientConnect const Address::InstanceConstSharedPtr& remote_address, const Address::InstanceConstSharedPtr& source_address, Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options); + const Network::ConnectionSocket::OptionsSharedPtr& options, + const Network::TransportSocketOptionsConstSharedPtr& transport_options); + ClientConnectionImpl(Event::Dispatcher& dispatcher, std::unique_ptr socket, const Address::InstanceConstSharedPtr& source_address, Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options); + const Network::ConnectionSocket::OptionsSharedPtr& options, + const Network::TransportSocketOptionsConstSharedPtr& transport_options); // Network::ClientConnection void connect() override; diff --git a/source/common/network/default_client_connection_factory.cc b/source/common/network/default_client_connection_factory.cc index e1a65a0e81579..31b3f6e03cb85 100644 --- a/source/common/network/default_client_connection_factory.cc +++ b/source/common/network/default_client_connection_factory.cc @@ -13,10 +13,11 @@ Network::ClientConnectionPtr DefaultClientConnectionFactory::createClientConnect Event::Dispatcher& dispatcher, Network::Address::InstanceConstSharedPtr address, Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options) { + const Network::ConnectionSocket::OptionsSharedPtr& options, + const Network::TransportSocketOptionsConstSharedPtr& transport_options) { ASSERT(address->ip() || address->pipe()); - return std::make_unique(dispatcher, address, source_address, - std::move(transport_socket), options); + return std::make_unique( + dispatcher, address, source_address, std::move(transport_socket), options, transport_options); } REGISTER_FACTORY(DefaultClientConnectionFactory, Network::ClientConnectionFactory); diff --git a/source/common/network/default_client_connection_factory.h b/source/common/network/default_client_connection_factory.h index 6658e2d3093c0..a30ca8ed2d76b 100644 --- a/source/common/network/default_client_connection_factory.h +++ b/source/common/network/default_client_connection_factory.h @@ -20,12 +20,12 @@ class DefaultClientConnectionFactory : public ClientConnectionFactory { std::string name() const override { return "default"; } // Network::ClientConnectionFactory - Network::ClientConnectionPtr - createClientConnection(Event::Dispatcher& dispatcher, - Network::Address::InstanceConstSharedPtr address, - Network::Address::InstanceConstSharedPtr source_address, - Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options) override; + Network::ClientConnectionPtr createClientConnection( + Event::Dispatcher& dispatcher, Network::Address::InstanceConstSharedPtr address, + Network::Address::InstanceConstSharedPtr source_address, + Network::TransportSocketPtr&& transport_socket, + const Network::ConnectionSocket::OptionsSharedPtr& options, + const Network::TransportSocketOptionsConstSharedPtr& transport_options = nullptr) override; }; } // namespace Network diff --git a/source/common/network/happy_eyeballs_connection_impl.cc b/source/common/network/happy_eyeballs_connection_impl.cc index c6dc14da14cbb..9a8069bc0e4af 100644 --- a/source/common/network/happy_eyeballs_connection_impl.cc +++ b/source/common/network/happy_eyeballs_connection_impl.cc @@ -433,7 +433,8 @@ ClientConnectionPtr HappyEyeballsConnectionImpl::createNextConnection() { address_list_[next_address_++], connection_construction_state_.source_address_, connection_construction_state_.socket_factory_.createTransportSocket( connection_construction_state_.transport_socket_options_), - connection_construction_state_.options_); + connection_construction_state_.options_, + connection_construction_state_.transport_socket_options_); ENVOY_LOG_EVENT(debug, "happy_eyeballs_cx_attempt", "C[{}] address={}", id_, next_address_); callbacks_wrappers_.push_back(std::make_unique(*this, *connection)); connection->addConnectionCallbacks(*callbacks_wrappers_.back()); diff --git a/source/common/network/transport_socket_options_impl.cc b/source/common/network/transport_socket_options_impl.cc index d841dff98a328..5287196492a1b 100644 --- a/source/common/network/transport_socket_options_impl.cc +++ b/source/common/network/transport_socket_options_impl.cc @@ -6,6 +6,8 @@ #include #include +#include "envoy/common/hashable.h" + #include "source/common/common/scalar_to_byte_vector.h" #include "source/common/common/utility.h" #include "source/common/network/application_protocol.h" @@ -40,6 +42,14 @@ void CommonTransportSocketFactory::hashKey(std::vector& key, for (const auto& protocol : alpn_fallback) { pushScalarToByteVector(StringUtil::CaseInsensitiveHash()(protocol), key); } + + for (const auto& object : options->filterStateObjects()) { + if (auto hashable = dynamic_cast(object.data_.get()); hashable != nullptr) { + if (auto hash = hashable->hash(); hash) { + pushScalarToByteVector(hash.value(), key); + } + } + } } TransportSocketOptionsConstSharedPtr TransportSocketOptionsUtility::fromFilterState( @@ -79,7 +89,8 @@ TransportSocketOptionsConstSharedPtr TransportSocketOptionsUtility::fromFilterSt return std::make_shared( server_name, std::move(subject_alt_names), std::move(application_protocols), - std::move(alpn_fallback), proxy_protocol_options, filter_state); + std::move(alpn_fallback), proxy_protocol_options, + filter_state->objectsSharedWithUpstreamConnection()); } } // namespace Network diff --git a/source/common/network/transport_socket_options_impl.h b/source/common/network/transport_socket_options_impl.h index 34029dcbe7e46..6e77534593da6 100644 --- a/source/common/network/transport_socket_options_impl.h +++ b/source/common/network/transport_socket_options_impl.h @@ -29,8 +29,8 @@ class AlpnDecoratingTransportSocketOptions : public TransportSocketOptions { absl::optional proxyProtocolOptions() const override { return inner_options_->proxyProtocolOptions(); } - const StreamInfo::FilterStateSharedPtr& filterState() const override { - return inner_options_->filterState(); + const StreamInfo::FilterState::Objects& filterStateObjects() const override { + return inner_options_->filterStateObjects(); } private: @@ -45,13 +45,15 @@ class TransportSocketOptionsImpl : public TransportSocketOptions { std::vector&& override_verify_san_list = {}, std::vector&& override_alpn = {}, std::vector&& fallback_alpn = {}, absl::optional proxy_proto_options = absl::nullopt, - const StreamInfo::FilterStateSharedPtr filter_state = nullptr) + StreamInfo::FilterState::ObjectsPtr filter_state_objects = + std::make_unique()) : override_server_name_(override_server_name.empty() ? absl::nullopt : absl::optional(override_server_name)), override_verify_san_list_{std::move(override_verify_san_list)}, override_alpn_list_{std::move(override_alpn)}, alpn_fallback_{std::move(fallback_alpn)}, - proxy_protocol_options_(proxy_proto_options), filter_state_(filter_state) {} + proxy_protocol_options_(proxy_proto_options), + filter_state_objects_(std::move(filter_state_objects)) {} // Network::TransportSocketOptions const absl::optional& serverNameOverride() const override { @@ -69,7 +71,9 @@ class TransportSocketOptionsImpl : public TransportSocketOptions { absl::optional proxyProtocolOptions() const override { return proxy_protocol_options_; } - const StreamInfo::FilterStateSharedPtr& filterState() const override { return filter_state_; } + const StreamInfo::FilterState::Objects& filterStateObjects() const override { + return *filter_state_objects_; + } private: const absl::optional override_server_name_; @@ -77,7 +81,7 @@ class TransportSocketOptionsImpl : public TransportSocketOptions { const std::vector override_alpn_list_; const std::vector alpn_fallback_; const absl::optional proxy_protocol_options_; - const StreamInfo::FilterStateSharedPtr filter_state_; + const StreamInfo::FilterState::ObjectsPtr filter_state_objects_; }; class TransportSocketOptionsUtility { diff --git a/source/common/stream_info/filter_state_impl.cc b/source/common/stream_info/filter_state_impl.cc index b973881019c60..e87e972a6bc8f 100644 --- a/source/common/stream_info/filter_state_impl.cc +++ b/source/common/stream_info/filter_state_impl.cc @@ -23,10 +23,10 @@ void FilterStateImpl::setData(absl::string_view data_name, std::shared_ptrsecond.get(); - if (current->state_type_ == FilterState::StateType::ReadOnly) { + if (current->state_type_ & FilterState::StateType::ReadOnly) { throw EnvoyException("FilterState::setData called twice on same ReadOnly state."); } @@ -76,7 +76,7 @@ FilterStateImpl::getDataSharedMutableGeneric(absl::string_view data_name) { } FilterStateImpl::FilterObject* current = it->second.get(); - if (current->state_type_ == FilterState::StateType::ReadOnly) { + if (current->state_type_ & FilterState::StateType::ReadOnly) { throw EnvoyException("FilterState tried to access immutable data as mutable."); } @@ -90,6 +90,17 @@ bool FilterStateImpl::hasDataAtOrAboveLifeSpan(FilterState::LifeSpan life_span) return !data_storage_.empty() || (parent_ && parent_->hasDataAtOrAboveLifeSpan(life_span)); } +FilterState::ObjectsPtr FilterStateImpl::objectsSharedWithUpstreamConnection() const { + auto objects = parent_ ? parent_->objectsSharedWithUpstreamConnection() + : std::make_unique(); + for (const auto& [name, object] : data_storage_) { + if (object->state_type_ & StateType::SharedWithUpstreamConnection) { + objects->push_back({object->data_, object->state_type_, name}); + } + } + return objects; +} + bool FilterStateImpl::hasDataWithNameInternally(absl::string_view data_name) const { return data_storage_.count(data_name) > 0; } diff --git a/source/common/stream_info/filter_state_impl.h b/source/common/stream_info/filter_state_impl.h index da08ceadaa100..60a285e8aa4e6 100644 --- a/source/common/stream_info/filter_state_impl.h +++ b/source/common/stream_info/filter_state_impl.h @@ -48,6 +48,7 @@ class FilterStateImpl : public FilterState { Object* getDataMutableGeneric(absl::string_view data_name) override; std::shared_ptr getDataSharedMutableGeneric(absl::string_view data_name) override; bool hasDataAtOrAboveLifeSpan(FilterState::LifeSpan life_span) const override; + FilterState::ObjectsPtr objectsSharedWithUpstreamConnection() const override; FilterState::LifeSpan lifeSpan() const override { return life_span_; } FilterStateSharedPtr parent() const override { return parent_; } @@ -58,11 +59,6 @@ class FilterStateImpl : public FilterState { enum class ParentAccessMode { ReadOnly, ReadWrite }; void maybeCreateParent(ParentAccessMode parent_access_mode); - struct FilterObject { - std::shared_ptr data_; - FilterState::StateType state_type_; - }; - absl::variant ancestor_; FilterStateSharedPtr parent_; const FilterState::LifeSpan life_span_; diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index cedb77f32d8ec..51cff56b73a45 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -340,14 +340,13 @@ Network::ClientConnectionPtr HostImpl::createConnection( Runtime::runtimeFeatureEnabled("envoy.reloadable_features.internal_address")); Network::ClientConnectionPtr connection = - address_list.size() > 1 - ? std::make_unique( - dispatcher, address_list, cluster.sourceAddress(), socket_factory, - transport_socket_options, connection_options) - : dispatcher.createClientConnection( - address, cluster.sourceAddress(), - socket_factory.createTransportSocket(std::move(transport_socket_options)), - connection_options); + address_list.size() > 1 ? std::make_unique( + dispatcher, address_list, cluster.sourceAddress(), + socket_factory, transport_socket_options, connection_options) + : dispatcher.createClientConnection( + address, cluster.sourceAddress(), + socket_factory.createTransportSocket(transport_socket_options), + connection_options, transport_socket_options); connection->connectionInfoSetter().enableSettingInterfaceName( cluster.setLocalInterfaceNameOnUpstreamConnections()); From 83a8469366035130dfb8cf57e58785d729ab9de2 Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Tue, 14 Jun 2022 12:05:39 -0700 Subject: [PATCH 03/12] fix release Signed-off-by: Kuat Yessenov --- envoy/event/dispatcher.h | 2 +- envoy/network/client_connection_factory.h | 2 +- source/common/event/dispatcher_impl.cc | 5 +++-- source/common/event/dispatcher_impl.h | 2 +- source/common/network/default_client_connection_factory.cc | 2 +- source/common/network/default_client_connection_factory.h | 2 +- .../internal_listener/client_connection_factory.cc | 5 +++-- .../bootstrap/internal_listener/client_connection_factory.h | 3 ++- source/server/config_validation/connection.h | 6 ++++-- source/server/config_validation/dispatcher.cc | 6 ++++-- source/server/config_validation/dispatcher.h | 3 ++- 11 files changed, 23 insertions(+), 15 deletions(-) diff --git a/envoy/event/dispatcher.h b/envoy/event/dispatcher.h index 5cfc3f87b8f8c..2b30550cc2598 100644 --- a/envoy/event/dispatcher.h +++ b/envoy/event/dispatcher.h @@ -218,7 +218,7 @@ class Dispatcher : public DispatcherBase, public ScopeTracker { Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - const Network::TransportSocketOptionsConstSharedPtr& transport_options = nullptr) PURE; + Network::TransportSocketOptionsConstSharedPtr transport_options = nullptr) PURE; /** * @return Filesystem::WatcherPtr a filesystem watcher owned by the caller. diff --git a/envoy/network/client_connection_factory.h b/envoy/network/client_connection_factory.h index c6e89740a3fd5..ff39034793cf1 100644 --- a/envoy/network/client_connection_factory.h +++ b/envoy/network/client_connection_factory.h @@ -31,7 +31,7 @@ class ClientConnectionFactory : public Config::UntypedFactory { Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - const Network::TransportSocketOptionsConstSharedPtr& transport_options = nullptr) PURE; + Network::TransportSocketOptionsConstSharedPtr transport_options = nullptr) PURE; }; } // namespace Network diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index 104d72afc979e..633e5b08b3ecf 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -160,7 +160,7 @@ Network::ClientConnectionPtr DispatcherImpl::createClientConnection( Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - const Network::TransportSocketOptionsConstSharedPtr& transport_options) { + Network::TransportSocketOptionsConstSharedPtr transport_options) { ASSERT(isThreadSafe()); auto* factory = Config::Utility::getFactoryByName( @@ -171,7 +171,8 @@ Network::ClientConnectionPtr DispatcherImpl::createClientConnection( // expects a non-null connection as of today so we cannot gracefully handle unsupported address // type. return factory->createClientConnection(*this, address, source_address, - std::move(transport_socket), options, transport_options); + std::move(transport_socket), options, + std::move(transport_options)); } FileEventPtr DispatcherImpl::createFileEvent(os_fd_t fd, FileReadyCb cb, FileTriggerType trigger, diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index ffef3427923c3..62b1bdd95cdae 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -71,7 +71,7 @@ class DispatcherImpl : Logger::Loggable, Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - const Network::TransportSocketOptionsConstSharedPtr& transport_options = nullptr) override; + Network::TransportSocketOptionsConstSharedPtr transport_options = nullptr) override; FileEventPtr createFileEvent(os_fd_t fd, FileReadyCb cb, FileTriggerType trigger, uint32_t events) override; Filesystem::WatcherPtr createFilesystemWatcher() override; diff --git a/source/common/network/default_client_connection_factory.cc b/source/common/network/default_client_connection_factory.cc index 31b3f6e03cb85..53351579ff493 100644 --- a/source/common/network/default_client_connection_factory.cc +++ b/source/common/network/default_client_connection_factory.cc @@ -14,7 +14,7 @@ Network::ClientConnectionPtr DefaultClientConnectionFactory::createClientConnect Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - const Network::TransportSocketOptionsConstSharedPtr& transport_options) { + Network::TransportSocketOptionsConstSharedPtr transport_options) { ASSERT(address->ip() || address->pipe()); return std::make_unique( dispatcher, address, source_address, std::move(transport_socket), options, transport_options); diff --git a/source/common/network/default_client_connection_factory.h b/source/common/network/default_client_connection_factory.h index a30ca8ed2d76b..547881aec4da5 100644 --- a/source/common/network/default_client_connection_factory.h +++ b/source/common/network/default_client_connection_factory.h @@ -25,7 +25,7 @@ class DefaultClientConnectionFactory : public ClientConnectionFactory { Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - const Network::TransportSocketOptionsConstSharedPtr& transport_options = nullptr) override; + Network::TransportSocketOptionsConstSharedPtr transport_options = nullptr) override; }; } // namespace Network diff --git a/source/extensions/bootstrap/internal_listener/client_connection_factory.cc b/source/extensions/bootstrap/internal_listener/client_connection_factory.cc index 59e2c8b956fdb..ce098fcc8c5d9 100644 --- a/source/extensions/bootstrap/internal_listener/client_connection_factory.cc +++ b/source/extensions/bootstrap/internal_listener/client_connection_factory.cc @@ -20,7 +20,8 @@ Network::ClientConnectionPtr InternalClientConnectionFactory::createClientConnec Event::Dispatcher& dispatcher, Network::Address::InstanceConstSharedPtr address, Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options) { + const Network::ConnectionSocket::OptionsSharedPtr& options, + Network::TransportSocketOptionsConstSharedPtr transport_options) { // OS does not fill the address automatically so a pivotal address is populated. // TODO(lambdai): provide option to fill the downstream remote address here. if (source_address == nullptr) { @@ -38,7 +39,7 @@ Network::ClientConnectionPtr InternalClientConnectionFactory::createClientConnec dispatcher, std::make_unique(std::move(io_handle_client), source_address, address), - source_address, std::move(transport_socket), options); + source_address, std::move(transport_socket), options, transport_options); if (registry_tls_slot_ == nullptr || !registry_tls_slot_->get().has_value()) { ENVOY_LOG_MISC(debug, diff --git a/source/extensions/bootstrap/internal_listener/client_connection_factory.h b/source/extensions/bootstrap/internal_listener/client_connection_factory.h index c751f11c28a3a..e05e9aba501f4 100644 --- a/source/extensions/bootstrap/internal_listener/client_connection_factory.h +++ b/source/extensions/bootstrap/internal_listener/client_connection_factory.h @@ -25,7 +25,8 @@ class InternalClientConnectionFactory : public Network::ClientConnectionFactory, Network::Address::InstanceConstSharedPtr address, Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options) override; + const Network::ConnectionSocket::OptionsSharedPtr& options, + Network::TransportSocketOptionsConstSharedPtr transport_options) override; // The slot is owned by the internal listener registry extension. Once that extension is // initialized, this slot is available. The ClientConnectionFactory has two potential user cases. // 1. The per worker thread connection handler populates the per worker listener registry. diff --git a/source/server/config_validation/connection.h b/source/server/config_validation/connection.h index c7b2dfc9f8b4b..53cbeb7f0e0a5 100644 --- a/source/server/config_validation/connection.h +++ b/source/server/config_validation/connection.h @@ -17,9 +17,11 @@ class ConfigValidateConnection : public Network::ClientConnectionImpl { Network::Address::InstanceConstSharedPtr remote_address, Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options) + const Network::ConnectionSocket::OptionsSharedPtr& options, + Network::TransportSocketOptionsConstSharedPtr transport_options) : Network::ClientConnectionImpl(dispatcher, remote_address, source_address, - std::move(transport_socket), options) {} + std::move(transport_socket), options, + std::move(transport_options)) {} // Unit tests may instantiate it without proper event machine and leave opened sockets. // Do some cleanup before invoking base class's destructor. diff --git a/source/server/config_validation/dispatcher.cc b/source/server/config_validation/dispatcher.cc index 0049576f34508..a8b3ac146425b 100644 --- a/source/server/config_validation/dispatcher.cc +++ b/source/server/config_validation/dispatcher.cc @@ -10,9 +10,11 @@ Network::ClientConnectionPtr ValidationDispatcher::createClientConnection( Network::Address::InstanceConstSharedPtr remote_address, Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options) { + const Network::ConnectionSocket::OptionsSharedPtr& options, + Network::TransportSocketOptionsConstSharedPtr transport_options) { return std::make_unique(*this, remote_address, source_address, - std::move(transport_socket), options); + std::move(transport_socket), options, + std::move(transport_options)); } Network::ListenerPtr ValidationDispatcher::createListener(Network::SocketSharedPtr&&, diff --git a/source/server/config_validation/dispatcher.h b/source/server/config_validation/dispatcher.h index 3733cb0fdda0e..b8f568bc54e1e 100644 --- a/source/server/config_validation/dispatcher.h +++ b/source/server/config_validation/dispatcher.h @@ -22,7 +22,8 @@ class ValidationDispatcher : public DispatcherImpl { Network::ClientConnectionPtr createClientConnection(Network::Address::InstanceConstSharedPtr, Network::Address::InstanceConstSharedPtr, Network::TransportSocketPtr&&, - const Network::ConnectionSocket::OptionsSharedPtr& options) override; + const Network::ConnectionSocket::OptionsSharedPtr& options, + Network::TransportSocketOptionsConstSharedPtr transport_options) override; Network::ListenerPtr createListener(Network::SocketSharedPtr&&, Network::TcpListenerCallbacks&, Runtime::Loader& runtime, bool bind_to_port, bool ignore_global_conn_limit) override; From 218f97dac420ab2ae0961cd0edac797c8f41833f Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Mon, 27 Jun 2022 21:39:55 -0700 Subject: [PATCH 04/12] changes Signed-off-by: Kuat Yessenov --- .../v3/internal_upstream.proto | 30 +----- envoy/network/transport_socket.h | 2 +- envoy/stream_info/filter_state.h | 40 ++++---- source/common/network/connection_impl.cc | 4 +- .../network/transport_socket_options_impl.cc | 37 +++++--- .../network/transport_socket_options_impl.h | 2 +- source/common/router/router.cc | 2 +- .../common/stream_info/filter_state_impl.cc | 12 ++- source/common/stream_info/filter_state_impl.h | 8 +- source/common/tcp_proxy/tcp_proxy.cc | 3 +- .../io_socket/user_space/io_handle.h | 5 +- .../io_socket/user_space/io_handle_impl.cc | 23 +++-- .../io_socket/user_space/io_handle_impl.h | 4 +- .../internal_upstream/config.cc | 56 +---------- .../internal_upstream/config.h | 12 +-- .../internal_upstream/internal_upstream.cc | 14 ++- .../internal_upstream/internal_upstream.h | 4 +- .../grpc_client_integration_test_harness.h | 2 +- test/common/network/connection_impl_test.cc | 2 +- .../transport_socket_options_impl_test.cc | 45 +++++---- .../user_space/io_handle_impl_test.cc | 17 ++-- .../internal_upstream_integration_test.cc | 4 +- .../internal_upstream_test.cc | 95 ++++--------------- .../filters/header_to_filter_state.cc | 13 ++- .../filters/header_to_filter_state.proto | 1 + test/mocks/event/mocks.h | 3 +- test/mocks/event/wrapped_dispatcher.h | 13 +-- 27 files changed, 168 insertions(+), 285 deletions(-) diff --git a/api/envoy/extensions/transport_sockets/internal_upstream/v3/internal_upstream.proto b/api/envoy/extensions/transport_sockets/internal_upstream/v3/internal_upstream.proto index 97a196a4980e2..4ef02295ccbff 100644 --- a/api/envoy/extensions/transport_sockets/internal_upstream/v3/internal_upstream.proto +++ b/api/envoy/extensions/transport_sockets/internal_upstream/v3/internal_upstream.proto @@ -22,20 +22,9 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // extension allows passing additional structured state across the user space // socket in addition to the regular byte stream. The purpose is to facilitate // communication between filters on the downstream and the upstream internal -// connections. -// -// Because the passthrough state is transferred once per the upstream -// connection before the bytes payload, every passthrough filter state object -// is included in the hash key used to select an upstream connection if it -// implements a hashing interface. -// -// .. note:: -// -// Using internal upstream transport affects load balancing decisions if the -// passthrough state is derived from the downstream connection attributes. As -// an example, using the downstream source IP in the passthrough state will -// prevent sharing of an upstream internal connection between different source -// IPs. +// connections. All filter state objects that are shared with the upstream +// connection are also shared with the downstream internal connection using +// this transport socket. message InternalUpstreamTransport { // Describes the location of the imported metadata value. // If the metadata with the given name is not present at the source location, @@ -48,12 +37,6 @@ message InternalUpstreamTransport { string name = 2 [(validate.rules).string = {min_len: 1}]; } - // Describes the location of the imported filter state object from the downstream connection. - message FilterStateSource { - // Name is the imported filter state object name. - string name = 1 [(validate.rules).string = {min_len: 1}]; - } - // Specifies the metadata namespaces and values to insert into the downstream // internal connection dynamic metadata when an internal address is used as a // host. If the destination name is repeated across two metadata source @@ -61,13 +44,6 @@ message InternalUpstreamTransport { // then the latter in the list overrides the former. repeated MetadataValueSource passthrough_metadata = 1; - // Specifies the list of the filter state object names to insert into the - // server internal connection from the downstream connection when an internal - // address is used as a host. The filter state objects must be mutable. These - // objects participate in the connection hashing decisions if they implement a - // hashing function. - repeated FilterStateSource passthrough_filter_state_objects = 2; - // The underlying transport socket being wrapped. config.core.v3.TransportSocket transport_socket = 3 [(validate.rules).message = {required: true}]; } diff --git a/envoy/network/transport_socket.h b/envoy/network/transport_socket.h index c699ce20eb12e..a198a6c8f5eb0 100644 --- a/envoy/network/transport_socket.h +++ b/envoy/network/transport_socket.h @@ -236,7 +236,7 @@ class TransportSocketOptions { /** * @return filter state objects from the downstream request or connection - * that are marked as shared with the upstream. + * that are marked as shared with the upstream connection. */ virtual const StreamInfo::FilterState::Objects& filterStateObjects() const PURE; }; diff --git a/envoy/stream_info/filter_state.h b/envoy/stream_info/filter_state.h index c8e8b6f2ba286..8ca72768b988f 100644 --- a/envoy/stream_info/filter_state.h +++ b/envoy/stream_info/filter_state.h @@ -26,23 +26,7 @@ using FilterStateSharedPtr = std::shared_ptr; */ class FilterState { public: - enum StateType { - // Readonly objects must be set once. - ReadOnly = 0x1, - - // Mutable objects can be overwritten with the same state type, life span, and name. - Mutable = 0, - - // Mark a filter state object as shared with the upstream connection. - // Shared filter state objects are copied by reference from the downstream - // requests and connections to the upstream connection filter state, and - // also participate in the hashing decisions in the transport socket - // options when connections are re-used between downstream requests. - SharedWithUpstreamConnection = 0x2, - - // ATTENTION: MAKE SURE THIS REMAINS EQUAL TO THE LAST FLAG. - LastFlag = SharedWithUpstreamConnection, - }; + enum class StateType { ReadOnly, Mutable }; // Objects stored in the FilterState may have different life span. Life span is what controls // how long an object stored in FilterState lives. Implementation of this interface actually @@ -74,6 +58,22 @@ class FilterState { // be captured by an upstream connection for the original downstream request. enum LifeSpan { FilterChain, Request, Connection, TopSpan = Connection }; + // Objects stored in the filter state can optionally be shared between the + // upstrean and downstream filter state. + enum class StreamSharing { + // None implies the object is exclusive to the stream. + None, + + // Mark a filter state object as shared with the upstream connection. + // Shared filter state objects are copied by reference from the downstream + // requests and connections to the upstream connection filter state. When + // upstream connections are re-used between streams, the downstream objects + // are captured for the first, initiating stream. To force distinct + // upstream connections and prevent sharing, the shared filter state object + // must implement the hashing interface. + SharedWithUpstreamConnection, + }; + class Object { public: virtual ~Object() = default; @@ -95,7 +95,8 @@ class FilterState { struct FilterObject { std::shared_ptr data_; - FilterState::StateType state_type_; + StateType state_type_; + StreamSharing stream_sharing_{StreamSharing::None}; std::string name_{""}; }; @@ -119,7 +120,8 @@ class FilterState { * data stored in FilterState. */ virtual void setData(absl::string_view data_name, std::shared_ptr data, - StateType state_type, LifeSpan life_span = LifeSpan::FilterChain) PURE; + StateType state_type, LifeSpan life_span = LifeSpan::FilterChain, + StreamSharing stream_sharing = StreamSharing::None) PURE; /** * @param data_name the name of the data being looked up (mutable/readonly). diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc index 0887877f4ac29..36f27e11a0d29 100644 --- a/source/common/network/connection_impl.cc +++ b/source/common/network/connection_impl.cc @@ -911,9 +911,9 @@ ClientConnectionImpl::ClientConnectionImpl( if (transport_options) { for (const auto& object : transport_options->filterStateObjects()) { - // TODO: handle exception stream_info_.filterState()->setData(object.name_, object.data_, object.state_type_, - StreamInfo::FilterState::LifeSpan::Connection); + StreamInfo::FilterState::LifeSpan::Connection, + object.stream_sharing_); } } } diff --git a/source/common/network/transport_socket_options_impl.cc b/source/common/network/transport_socket_options_impl.cc index a116218ad0dea..75a962aae361a 100644 --- a/source/common/network/transport_socket_options_impl.cc +++ b/source/common/network/transport_socket_options_impl.cc @@ -52,45 +52,54 @@ void CommonUpstreamTransportSocketFactory::hashKey( } } -TransportSocketOptionsConstSharedPtr TransportSocketOptionsUtility::fromFilterState( - const StreamInfo::FilterStateSharedPtr& filter_state) { - if (!filter_state) { - return nullptr; - } +TransportSocketOptionsConstSharedPtr +TransportSocketOptionsUtility::fromFilterState(const StreamInfo::FilterState& filter_state) { absl::string_view server_name; std::vector application_protocols; std::vector subject_alt_names; std::vector alpn_fallback; absl::optional proxy_protocol_options; - if (auto typed_data = - filter_state->getDataReadOnly(UpstreamServerName::key()); + bool needs_transport_socket_options = false; + if (auto typed_data = filter_state.getDataReadOnly(UpstreamServerName::key()); typed_data != nullptr) { server_name = typed_data->value(); + needs_transport_socket_options = true; } - if (auto typed_data = filter_state->getDataReadOnly( + if (auto typed_data = filter_state.getDataReadOnly( Network::ApplicationProtocols::key()); typed_data != nullptr) { application_protocols = typed_data->value(); + needs_transport_socket_options = true; } if (auto typed_data = - filter_state->getDataReadOnly(UpstreamSubjectAltNames::key()); + filter_state.getDataReadOnly(UpstreamSubjectAltNames::key()); typed_data != nullptr) { subject_alt_names = typed_data->value(); + needs_transport_socket_options = true; } if (auto typed_data = - filter_state->getDataReadOnly(ProxyProtocolFilterState::key()); + filter_state.getDataReadOnly(ProxyProtocolFilterState::key()); typed_data != nullptr) { proxy_protocol_options.emplace(typed_data->value()); + needs_transport_socket_options = true; } - return std::make_shared( - server_name, std::move(subject_alt_names), std::move(application_protocols), - std::move(alpn_fallback), proxy_protocol_options, - filter_state->objectsSharedWithUpstreamConnection()); + StreamInfo::FilterState::ObjectsPtr objects = filter_state.objectsSharedWithUpstreamConnection(); + if (objects->size() > 0) { + needs_transport_socket_options = true; + } + + if (needs_transport_socket_options) { + return std::make_shared( + server_name, std::move(subject_alt_names), std::move(application_protocols), + std::move(alpn_fallback), proxy_protocol_options, std::move(objects)); + } else { + return nullptr; + } } } // namespace Network diff --git a/source/common/network/transport_socket_options_impl.h b/source/common/network/transport_socket_options_impl.h index 0333e54de062f..1b84e2c3e059a 100644 --- a/source/common/network/transport_socket_options_impl.h +++ b/source/common/network/transport_socket_options_impl.h @@ -93,7 +93,7 @@ class TransportSocketOptionsUtility { * nullptr if nothing is in the filter state. */ static TransportSocketOptionsConstSharedPtr - fromFilterState(const StreamInfo::FilterStateSharedPtr& stream_info); + fromFilterState(const StreamInfo::FilterState& stream_info); }; class CommonUpstreamTransportSocketFactory : public UpstreamTransportSocketFactory { diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 7ecd5e35c8c72..3e6e2a83e99be 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -557,7 +557,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, } transport_socket_options_ = Network::TransportSocketOptionsUtility::fromFilterState( - callbacks_->streamInfo().filterState()); + *callbacks_->streamInfo().filterState()); if (auto downstream_connection = downstreamConnection(); downstream_connection != nullptr) { if (auto typed_state = downstream_connection->streamInfo() diff --git a/source/common/stream_info/filter_state_impl.cc b/source/common/stream_info/filter_state_impl.cc index e87e972a6bc8f..e54178a6a6ec7 100644 --- a/source/common/stream_info/filter_state_impl.cc +++ b/source/common/stream_info/filter_state_impl.cc @@ -6,7 +6,8 @@ namespace Envoy { namespace StreamInfo { void FilterStateImpl::setData(absl::string_view data_name, std::shared_ptr data, - FilterState::StateType state_type, FilterState::LifeSpan life_span) { + FilterState::StateType state_type, FilterState::LifeSpan life_span, + FilterState::StreamSharing stream_sharing) { if (life_span > life_span_) { if (hasDataWithNameInternally(data_name)) { throw EnvoyException( @@ -26,7 +27,7 @@ void FilterStateImpl::setData(absl::string_view data_name, std::shared_ptrsecond.get(); - if (current->state_type_ & FilterState::StateType::ReadOnly) { + if (current->state_type_ == FilterState::StateType::ReadOnly) { throw EnvoyException("FilterState::setData called twice on same ReadOnly state."); } @@ -38,6 +39,7 @@ void FilterStateImpl::setData(absl::string_view data_name, std::shared_ptr filter_object(new FilterStateImpl::FilterObject()); filter_object->data_ = data; filter_object->state_type_ = state_type; + filter_object->stream_sharing_ = stream_sharing; data_storage_[data_name] = std::move(filter_object); } @@ -76,7 +78,7 @@ FilterStateImpl::getDataSharedMutableGeneric(absl::string_view data_name) { } FilterStateImpl::FilterObject* current = it->second.get(); - if (current->state_type_ & FilterState::StateType::ReadOnly) { + if (current->state_type_ == FilterState::StateType::ReadOnly) { throw EnvoyException("FilterState tried to access immutable data as mutable."); } @@ -94,8 +96,8 @@ FilterState::ObjectsPtr FilterStateImpl::objectsSharedWithUpstreamConnection() c auto objects = parent_ ? parent_->objectsSharedWithUpstreamConnection() : std::make_unique(); for (const auto& [name, object] : data_storage_) { - if (object->state_type_ & StateType::SharedWithUpstreamConnection) { - objects->push_back({object->data_, object->state_type_, name}); + if (object->stream_sharing_ == StreamSharing::SharedWithUpstreamConnection) { + objects->push_back({object->data_, object->state_type_, object->stream_sharing_, name}); } } return objects; diff --git a/source/common/stream_info/filter_state_impl.h b/source/common/stream_info/filter_state_impl.h index 60a285e8aa4e6..e62a4cedea47c 100644 --- a/source/common/stream_info/filter_state_impl.h +++ b/source/common/stream_info/filter_state_impl.h @@ -40,9 +40,11 @@ class FilterStateImpl : public FilterState { } // FilterState - void setData(absl::string_view data_name, std::shared_ptr data, - FilterState::StateType state_type, - FilterState::LifeSpan life_span = FilterState::LifeSpan::FilterChain) override; + void + setData(absl::string_view data_name, std::shared_ptr data, + FilterState::StateType state_type, + FilterState::LifeSpan life_span = FilterState::LifeSpan::FilterChain, + FilterState::StreamSharing stream_sharing = FilterState::StreamSharing::None) override; bool hasDataWithName(absl::string_view) const override; const Object* getDataReadOnlyGeneric(absl::string_view data_name) const override; Object* getDataMutableGeneric(absl::string_view data_name) override; diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index b388c17c20966..7d3a2ed901237 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -400,7 +400,8 @@ Network::FilterStatus Filter::establishUpstreamConnection() { StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::Connection); } - transport_socket_options_ = Network::TransportSocketOptionsUtility::fromFilterState(filter_state); + transport_socket_options_ = + Network::TransportSocketOptionsUtility::fromFilterState(*filter_state); if (auto typed_state = filter_state->getDataReadOnly( Network::UpstreamSocketOptionsFilterState::key()); diff --git a/source/extensions/io_socket/user_space/io_handle.h b/source/extensions/io_socket/user_space/io_handle.h index 6019e3e459e6b..927d062a6b6df 100644 --- a/source/extensions/io_socket/user_space/io_handle.h +++ b/source/extensions/io_socket/user_space/io_handle.h @@ -11,9 +11,6 @@ namespace Extensions { namespace IoSocket { namespace UserSpace { -using FilterStateObjects = - std::vector>>; - // Shared state between peering user space IO handles. class PassthroughState { public: @@ -24,7 +21,7 @@ class PassthroughState { * called exactly once before `mergeInto`. */ virtual void initialize(std::unique_ptr metadata, - std::unique_ptr filter_state_objects) PURE; + const StreamInfo::FilterState::Objects& filter_state_objects) PURE; /** * Merge the passthrough state into a recipient stream metadata and its diff --git a/source/extensions/io_socket/user_space/io_handle_impl.cc b/source/extensions/io_socket/user_space/io_handle_impl.cc index 003f42910aaff..4687cfd633f78 100644 --- a/source/extensions/io_socket/user_space/io_handle_impl.cc +++ b/source/extensions/io_socket/user_space/io_handle_impl.cc @@ -378,11 +378,12 @@ Api::SysCallIntResult IoHandleImpl::shutdown(int how) { return {0, 0}; } -void PassthroughStateImpl::initialize(std::unique_ptr metadata, - std::unique_ptr filter_state_objects) { +void PassthroughStateImpl::initialize( + std::unique_ptr metadata, + const StreamInfo::FilterState::Objects& filter_state_objects) { ASSERT(state_ == State::Created); metadata_ = std::move(metadata); - filter_state_objects_ = std::move(filter_state_objects); + filter_state_objects_ = filter_state_objects; state_ = State::Initialized; } void PassthroughStateImpl::mergeInto(envoy::config::core::v3::Metadata& metadata, @@ -391,18 +392,16 @@ void PassthroughStateImpl::mergeInto(envoy::config::core::v3::Metadata& metadata if (metadata_) { metadata.MergeFrom(*metadata_); } - if (filter_state_objects_) { - for (const auto& [name, object] : *filter_state_objects_) { - try { - filter_state.setData(name, object, StreamInfo::FilterState::StateType::Mutable, - StreamInfo::FilterState::LifeSpan::Connection); - } catch (const EnvoyException& e) { - ENVOY_LOG(trace, "Failed to set data for '{}': {}", name, e.what()); - } + for (const auto& object : filter_state_objects_) { + try { + filter_state.setData(object.name_, object.data_, object.state_type_, + StreamInfo::FilterState::LifeSpan::Connection, object.stream_sharing_); + } catch (const EnvoyException& e) { + ENVOY_LOG(trace, "Failed to set data for '{}': {}", object.name_, e.what()); } } metadata_ = nullptr; - filter_state_objects_ = nullptr; + filter_state_objects_.clear(); state_ = State::Done; } } // namespace UserSpace diff --git a/source/extensions/io_socket/user_space/io_handle_impl.h b/source/extensions/io_socket/user_space/io_handle_impl.h index 65bfe0084d019..e1ff27ae48ad7 100644 --- a/source/extensions/io_socket/user_space/io_handle_impl.h +++ b/source/extensions/io_socket/user_space/io_handle_impl.h @@ -185,7 +185,7 @@ class IoHandleImpl final : public Network::IoHandle, class PassthroughStateImpl : public PassthroughState, public Logger::Loggable { public: void initialize(std::unique_ptr metadata, - std::unique_ptr filter_state_objects) override; + const StreamInfo::FilterState::Objects& filter_state_objects) override; void mergeInto(envoy::config::core::v3::Metadata& metadata, StreamInfo::FilterState& filter_state) override; @@ -193,7 +193,7 @@ class PassthroughStateImpl : public PassthroughState, public Logger::Loggable metadata_; - std::unique_ptr filter_state_objects_; + StreamInfo::FilterState::Objects filter_state_objects_; }; using IoHandleImplPtr = std::unique_ptr; diff --git a/source/extensions/transport_sockets/internal_upstream/config.cc b/source/extensions/transport_sockets/internal_upstream/config.cc index 984769e9c5609..e7793f646d62f 100644 --- a/source/extensions/transport_sockets/internal_upstream/config.cc +++ b/source/extensions/transport_sockets/internal_upstream/config.cc @@ -67,10 +67,8 @@ Config::Config( } metadata_sources_.push_back(MetadataSource(kind, metadata.name())); } - for (const auto& filter_state_object : config_proto.passthrough_filter_state_objects()) { - filter_state_names_.push_back(filter_state_object.name()); - } } + std::unique_ptr Config::extractMetadata(const Upstream::HostDescriptionConstSharedPtr& host) const { if (metadata_sources_.empty()) { @@ -101,40 +99,6 @@ Config::extractMetadata(const Upstream::HostDescriptionConstSharedPtr& host) con return metadata; } -std::unique_ptr -Config::extractFilterState(const StreamInfo::FilterStateSharedPtr& filter_state) const { - if (filter_state_names_.empty()) { - return nullptr; - } - auto filter_state_objects = std::make_unique(); - for (const auto& name : filter_state_names_) { - try { - auto object = filter_state->getDataSharedMutableGeneric(name); - if (object == nullptr) { - ENVOY_LOG(trace, "Internal upstream missing filter state: {}", name); - stats_.no_filter_state_.inc(); - continue; - } - filter_state_objects->emplace_back(name, object); - } catch (const EnvoyException& e) { - ENVOY_LOG(trace, "Internal upstream filter state error: {}. Error: {}", name, e.what()); - stats_.filter_state_error_.inc(); - } - } - return filter_state_objects; -} - -void Config::hashKey(std::vector& key, - const StreamInfo::FilterStateSharedPtr& filter_state) const { - for (const auto& name : filter_state_names_) { - if (auto object = filter_state->getDataReadOnly(name); object != nullptr) { - if (auto hash = object->hash(); hash) { - pushScalarToByteVector(hash.value(), key); - } - } - } -} - InternalSocketFactory::InternalSocketFactory( Server::Configuration::TransportSocketFactoryContext& context, const envoy::extensions::transport_sockets::internal_upstream::v3::InternalUpstreamTransport& @@ -153,23 +117,9 @@ InternalSocketFactory::createTransportSocket(Network::TransportSocketOptionsCons if (host) { extracted_metadata = config_.extractMetadata(host); } - std::unique_ptr extracted_filter_state; - if (options && options->filterState()) { - extracted_filter_state = config_.extractFilterState(options->filterState()); - } return std::make_unique(std::move(inner_socket), std::move(extracted_metadata), - std::move(extracted_filter_state)); -} - -void InternalSocketFactory::hashKey(std::vector& key, - Network::TransportSocketOptionsConstSharedPtr options) const { - PassthroughFactory::hashKey(key, options); - // Filter state should be included in the hash since it can originate from - // the downstream request, but it is only applied once per upstream connection in - // the internal listener. - if (options && options->filterState()) { - config_.hashKey(key, options->filterState()); - } + options ? options->filterStateObjects() + : StreamInfo::FilterState::Objects()); } REGISTER_FACTORY(InternalUpstreamConfigFactory, diff --git a/source/extensions/transport_sockets/internal_upstream/config.h b/source/extensions/transport_sockets/internal_upstream/config.h index 1c983a5d60179..a4e57106cbb47 100644 --- a/source/extensions/transport_sockets/internal_upstream/config.h +++ b/source/extensions/transport_sockets/internal_upstream/config.h @@ -13,10 +13,7 @@ namespace Extensions { namespace TransportSockets { namespace InternalUpstream { -#define ALL_INTERNAL_UPSTREAM_STATS(COUNTER) \ - COUNTER(no_metadata) \ - COUNTER(no_filter_state) \ - COUNTER(filter_state_error) +#define ALL_INTERNAL_UPSTREAM_STATS(COUNTER) COUNTER(no_metadata) /** * Struct definition for all internal transport socket stats. @see stats_macros.h @@ -33,10 +30,6 @@ class Config : public Logger::Loggable { Stats::Scope& scope); std::unique_ptr extractMetadata(const Upstream::HostDescriptionConstSharedPtr& host) const; - std::unique_ptr - extractFilterState(const StreamInfo::FilterStateSharedPtr& filter_state) const; - void hashKey(std::vector& key, - const StreamInfo::FilterStateSharedPtr& filter_state) const; private: enum class MetadataKind { Host, Cluster }; @@ -47,7 +40,6 @@ class Config : public Logger::Loggable { }; InternalUpstreamStats stats_; std::vector metadata_sources_; - std::vector filter_state_names_; }; class InternalSocketFactory : public PassthroughFactory { @@ -61,8 +53,6 @@ class InternalSocketFactory : public PassthroughFactory { Network::TransportSocketPtr createTransportSocket(Network::TransportSocketOptionsConstSharedPtr options, Upstream::HostDescriptionConstSharedPtr host) const override; - void hashKey(std::vector& key, - Network::TransportSocketOptionsConstSharedPtr options) const override; private: const Config config_; diff --git a/source/extensions/transport_sockets/internal_upstream/internal_upstream.cc b/source/extensions/transport_sockets/internal_upstream/internal_upstream.cc index 62b021c10826c..d872eeccbc0d3 100644 --- a/source/extensions/transport_sockets/internal_upstream/internal_upstream.cc +++ b/source/extensions/transport_sockets/internal_upstream/internal_upstream.cc @@ -5,22 +5,20 @@ namespace Extensions { namespace TransportSockets { namespace InternalUpstream { -InternalSocket::InternalSocket( - Network::TransportSocketPtr inner_socket, - std::unique_ptr metadata, - std::unique_ptr filter_state_objects) +InternalSocket::InternalSocket(Network::TransportSocketPtr inner_socket, + std::unique_ptr metadata, + const StreamInfo::FilterState::Objects& filter_state_objects) : PassthroughSocket(std::move(inner_socket)), metadata_(std::move(metadata)), - filter_state_objects_(std::move(filter_state_objects)) {} + filter_state_objects_(filter_state_objects) {} void InternalSocket::setTransportSocketCallbacks(Network::TransportSocketCallbacks& callbacks) { transport_socket_->setTransportSocketCallbacks(callbacks); auto* io_handle = dynamic_cast(&callbacks.ioHandle()); if (io_handle != nullptr && io_handle->passthroughState()) { - io_handle->passthroughState()->initialize(std::move(metadata_), - std::move(filter_state_objects_)); + io_handle->passthroughState()->initialize(std::move(metadata_), filter_state_objects_); } metadata_ = nullptr; - filter_state_objects_ = nullptr; + filter_state_objects_.clear(); } } // namespace InternalUpstream diff --git a/source/extensions/transport_sockets/internal_upstream/internal_upstream.h b/source/extensions/transport_sockets/internal_upstream/internal_upstream.h index aad825a68a042..92e36a04cc83e 100644 --- a/source/extensions/transport_sockets/internal_upstream/internal_upstream.h +++ b/source/extensions/transport_sockets/internal_upstream/internal_upstream.h @@ -14,14 +14,14 @@ class InternalSocket : public TransportSockets::PassthroughSocket { public: InternalSocket(Network::TransportSocketPtr inner_socket, std::unique_ptr metadata, - std::unique_ptr filter_state_objects); + const StreamInfo::FilterState::Objects& filter_state_objects); // Network::TransportSocket void setTransportSocketCallbacks(Network::TransportSocketCallbacks& callbacks) override; private: std::unique_ptr metadata_; - std::unique_ptr filter_state_objects_; + StreamInfo::FilterState::Objects filter_state_objects_; }; } // namespace InternalUpstream diff --git a/test/common/grpc/grpc_client_integration_test_harness.h b/test/common/grpc/grpc_client_integration_test_harness.h index 30deec6a01862..dd340e15b4372 100644 --- a/test/common/grpc/grpc_client_integration_test_harness.h +++ b/test/common/grpc/grpc_client_integration_test_harness.h @@ -294,7 +294,7 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { RawAsyncClientPtr createAsyncClientImpl() { client_connection_ = std::make_unique( *dispatcher_, fake_upstream_->localAddress(), nullptr, - std::move(async_client_transport_socket_), nullptr); + std::move(async_client_transport_socket_), nullptr, nullptr); ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, connectTimeout()) .WillByDefault(Return(std::chrono::milliseconds(10000))); cm_.initializeThreadLocalClusters({"fake_cluster"}); diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index ebf333db7764b..eed07b6301bd4 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -161,7 +161,7 @@ class ConnectionImplTest : public testing::TestWithParam { listener_ = dispatcher_->createListener(socket_, listener_callbacks_, runtime_, true, false); client_connection_ = std::make_unique( *dispatcher_, socket_->connectionInfoProvider().localAddress(), source_address_, - createTransportSocket(), socket_options_); + createTransportSocket(), socket_options_, nullptr); client_connection_->addConnectionCallbacks(client_callbacks_); EXPECT_EQ(nullptr, client_connection_->ssl()); const Network::ClientConnection& const_connection = *client_connection_; diff --git a/test/common/network/transport_socket_options_impl_test.cc b/test/common/network/transport_socket_options_impl_test.cc index 7044054d982fe..912d00cc5a4c5 100644 --- a/test/common/network/transport_socket_options_impl_test.cc +++ b/test/common/network/transport_socket_options_impl_test.cc @@ -15,34 +15,43 @@ namespace { class TransportSocketOptionsImplTest : public testing::Test { public: TransportSocketOptionsImplTest() - : filter_state_(std::make_shared( - StreamInfo::FilterState::LifeSpan::FilterChain)) {} + : filter_state_(StreamInfo::FilterState::LifeSpan::FilterChain) {} protected: - StreamInfo::FilterStateSharedPtr filter_state_; + StreamInfo::FilterStateImpl filter_state_; }; TEST_F(TransportSocketOptionsImplTest, Nullptr) { - EXPECT_EQ(nullptr, TransportSocketOptionsUtility::fromFilterState(nullptr)); - filter_state_->setData( + EXPECT_EQ(nullptr, TransportSocketOptionsUtility::fromFilterState(filter_state_)); + filter_state_.setData( "random_key_has_no_effect", std::make_unique("www.example.com"), StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::FilterChain); + EXPECT_EQ(nullptr, TransportSocketOptionsUtility::fromFilterState(filter_state_)); +} + +TEST_F(TransportSocketOptionsImplTest, SharedFilterState) { + filter_state_.setData( + "random_key_has_effect", std::make_unique("www.example.com"), + StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::FilterChain, + StreamInfo::FilterState::StreamSharing::SharedWithUpstreamConnection); auto transport_socket_options = TransportSocketOptionsUtility::fromFilterState(filter_state_); - EXPECT_TRUE(transport_socket_options->filterState()->hasDataWithName("random_key_has_no_effect")); + auto objects = transport_socket_options->filterStateObjects(); + EXPECT_EQ(1, objects.size()); + EXPECT_EQ("random_key_has_effect", objects.at(0).name_); } TEST_F(TransportSocketOptionsImplTest, UpstreamServer) { - filter_state_->setData( + filter_state_.setData( UpstreamServerName::key(), std::make_unique("www.example.com"), StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::FilterChain); - filter_state_->setData(ProxyProtocolFilterState::key(), - std::make_unique(Network::ProxyProtocolData{ - Network::Address::InstanceConstSharedPtr( - new Network::Address::Ipv4Instance("202.168.0.13", 52000)), - Network::Address::InstanceConstSharedPtr( - new Network::Address::Ipv4Instance("174.2.2.222", 80))}), - StreamInfo::FilterState::StateType::ReadOnly, - StreamInfo::FilterState::LifeSpan::FilterChain); + filter_state_.setData(ProxyProtocolFilterState::key(), + std::make_unique(Network::ProxyProtocolData{ + Network::Address::InstanceConstSharedPtr( + new Network::Address::Ipv4Instance("202.168.0.13", 52000)), + Network::Address::InstanceConstSharedPtr( + new Network::Address::Ipv4Instance("174.2.2.222", 80))}), + StreamInfo::FilterState::StateType::ReadOnly, + StreamInfo::FilterState::LifeSpan::FilterChain); auto transport_socket_options = TransportSocketOptionsUtility::fromFilterState(filter_state_); EXPECT_EQ(absl::make_optional("www.example.com"), transport_socket_options->serverNameOverride()); @@ -54,7 +63,7 @@ TEST_F(TransportSocketOptionsImplTest, UpstreamServer) { TEST_F(TransportSocketOptionsImplTest, ApplicationProtocols) { std::vector http_alpns{Http::Utility::AlpnNames::get().Http2, Http::Utility::AlpnNames::get().Http11}; - filter_state_->setData( + filter_state_.setData( ApplicationProtocols::key(), std::make_unique(http_alpns), StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::FilterChain); auto transport_socket_options = TransportSocketOptionsUtility::fromFilterState(filter_state_); @@ -65,10 +74,10 @@ TEST_F(TransportSocketOptionsImplTest, ApplicationProtocols) { TEST_F(TransportSocketOptionsImplTest, Both) { std::vector http_alpns{Http::Utility::AlpnNames::get().Http2, Http::Utility::AlpnNames::get().Http11}; - filter_state_->setData( + filter_state_.setData( UpstreamServerName::key(), std::make_unique("www.example.com"), StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::FilterChain); - filter_state_->setData( + filter_state_.setData( ApplicationProtocols::key(), std::make_unique(http_alpns), StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::FilterChain); auto transport_socket_options = TransportSocketOptionsUtility::fromFilterState(filter_state_); diff --git a/test/extensions/io_socket/user_space/io_handle_impl_test.cc b/test/extensions/io_socket/user_space/io_handle_impl_test.cc index ed638b965d2ff..a083f7c492ab7 100644 --- a/test/extensions/io_socket/user_space/io_handle_impl_test.cc +++ b/test/extensions/io_socket/user_space/io_handle_impl_test.cc @@ -1131,12 +1131,13 @@ TEST_F(IoHandleImplTest, PassthroughState) { ProtobufWkt::Value val; val.set_string_value("val"); (*map.mutable_fields())["key"] = val; - auto source_filter_state = std::make_unique(); + StreamInfo::FilterState::Objects source_filter_state; auto object = std::make_shared(1000); - source_filter_state->emplace_back("object_key", object); + source_filter_state.push_back( + {object, StreamInfo::FilterState::StateType::ReadOnly, + StreamInfo::FilterState::StreamSharing::SharedWithUpstreamConnection, "object_key"}); ASSERT_NE(nullptr, io_handle_->passthroughState()); - io_handle_->passthroughState()->initialize(std::move(source_metadata), - std::move(source_filter_state)); + io_handle_->passthroughState()->initialize(std::move(source_metadata), source_filter_state); StreamInfo::FilterStateImpl dest_filter_state(StreamInfo::FilterState::LifeSpan::Connection); envoy::config::core::v3::Metadata dest_metadata; @@ -1150,10 +1151,12 @@ TEST_F(IoHandleImplTest, PassthroughState) { } TEST_F(IoHandleImplTest, PassthroughStateReadOnlyObject) { - auto source_filter_state = std::make_unique(); + StreamInfo::FilterState::Objects source_filter_state; auto object = std::make_shared(1000); - source_filter_state->emplace_back("object_key", object); - io_handle_->passthroughState()->initialize(nullptr, std::move(source_filter_state)); + source_filter_state.push_back( + {object, StreamInfo::FilterState::StateType::ReadOnly, + StreamInfo::FilterState::StreamSharing::SharedWithUpstreamConnection, "object_key"}); + io_handle_->passthroughState()->initialize(nullptr, source_filter_state); StreamInfo::FilterStateImpl dest_filter_state(StreamInfo::FilterState::LifeSpan::Connection); auto read_only = std::make_shared(1); dest_filter_state.setData("object_key", read_only, StreamInfo::FilterState::StateType::ReadOnly, diff --git a/test/extensions/transport_sockets/internal_upstream/internal_upstream_integration_test.cc b/test/extensions/transport_sockets/internal_upstream/internal_upstream_integration_test.cc index 079721fceddc8..0b0a20c542c34 100644 --- a/test/extensions/transport_sockets/internal_upstream/internal_upstream_integration_test.cc +++ b/test/extensions/transport_sockets/internal_upstream/internal_upstream_integration_test.cc @@ -39,8 +39,6 @@ class InternalUpstreamIntegrationTest : public testing::Test, public HttpIntegra kind: { host: {}} - name: cluster_metadata kind: { cluster: {}} - passthrough_filter_state_objects: - - name: internal_state transport_socket: name: envoy.transport_sockets.raw_buffer typed_config: @@ -113,7 +111,7 @@ class InternalUpstreamIntegrationTest : public testing::Test, public HttpIntegra "@type": type.googleapis.com/test.integration.filters.HeaderToFilterStateFilterConfig header_name: internal-header state_name: internal_state - read_only: false + shared: true )EOF"); HttpIntegrationTest::initialize(); } diff --git a/test/extensions/transport_sockets/internal_upstream/internal_upstream_test.cc b/test/extensions/transport_sockets/internal_upstream/internal_upstream_test.cc index 5652ae269d95c..d4963d4aa9c26 100644 --- a/test/extensions/transport_sockets/internal_upstream/internal_upstream_test.cc +++ b/test/extensions/transport_sockets/internal_upstream/internal_upstream_test.cc @@ -21,14 +21,10 @@ namespace InternalUpstream { namespace { -using IoSocket::UserSpace::FilterStateObjects; using IoSocket::UserSpace::IoHandle; using IoSocket::UserSpace::PassthroughState; -class NonHashable : public StreamInfo::FilterState::Object {}; -class HashableObj : public StreamInfo::FilterState::Object, public Hashable { - absl::optional hash() const override { return 12345; }; -}; +class TestObject : public StreamInfo::FilterState::Object {}; class MockUserSpaceIoHandle : public Network::MockIoHandle, public IoHandle { public: @@ -48,7 +44,7 @@ class MockPassthroughState : public PassthroughState { public: MOCK_METHOD(void, initialize, (std::unique_ptr metadata, - std::unique_ptr filter_state_objects)); + const StreamInfo::FilterState::Objects& filter_state_objects)); MOCK_METHOD(void, mergeInto, (envoy::config::core::v3::Metadata & metadata, StreamInfo::FilterState& filter_state)); @@ -56,22 +52,20 @@ class MockPassthroughState : public PassthroughState { class InternalSocketTest : public testing::Test { public: - InternalSocketTest() - : metadata_(std::make_unique()), - filter_state_objects_(std::make_unique()) {} + InternalSocketTest() : metadata_(std::make_unique()) {} void initialize(Network::IoHandle& io_handle) { auto inner_socket = std::make_unique>(); inner_socket_ = inner_socket.get(); ON_CALL(transport_callbacks_, ioHandle()).WillByDefault(testing::ReturnRef(io_handle)); socket_ = std::make_unique(std::move(inner_socket), std::move(metadata_), - std::move(filter_state_objects_)); + filter_state_objects_); EXPECT_CALL(*inner_socket_, setTransportSocketCallbacks(_)); socket_->setTransportSocketCallbacks(transport_callbacks_); } std::unique_ptr metadata_; - std::unique_ptr filter_state_objects_; + StreamInfo::FilterState::Objects filter_state_objects_; NiceMock* inner_socket_; std::unique_ptr socket_; NiceMock transport_callbacks_; @@ -85,8 +79,10 @@ TEST_F(InternalSocketTest, NativeSocket) { // Test that internal transport socket updates the passthrough state for the user space sockets. TEST_F(InternalSocketTest, PassthroughStateInjected) { - auto filter_state_object = std::make_shared(); - filter_state_objects_->emplace_back("test.object", filter_state_object); + auto filter_state_object = std::make_shared(); + filter_state_objects_.push_back( + {filter_state_object, StreamInfo::FilterState::StateType::ReadOnly, + StreamInfo::FilterState::StreamSharing::SharedWithUpstreamConnection, "test.object"}); ProtobufWkt::Struct& map = (*metadata_->mutable_filter_metadata())["envoy.test"]; ProtobufWkt::Value val; val.set_string_value("val"); @@ -97,28 +93,27 @@ TEST_F(InternalSocketTest, PassthroughStateInjected) { EXPECT_CALL(io_handle, passthroughState()).WillRepeatedly(testing::Return(state)); EXPECT_CALL(*state, initialize(_, _)) .WillOnce(Invoke([&](std::unique_ptr metadata, - std::unique_ptr filter_state_objects) -> void { + const StreamInfo::FilterState::Objects& filter_state_objects) -> void { ASSERT_EQ("val", metadata->filter_metadata().at("envoy.test").fields().at("key").string_value()); - ASSERT_EQ(1, filter_state_objects->size()); - const auto& [name, object] = filter_state_objects->at(0); - ASSERT_EQ("test.object", name); - ASSERT_EQ(filter_state_object.get(), object.get()); + ASSERT_EQ(1, filter_state_objects.size()); + const auto& object = filter_state_objects.at(0); + ASSERT_EQ("test.object", object.name_); + ASSERT_EQ(filter_state_object.get(), object.data_.get()); })); initialize(io_handle); } TEST_F(InternalSocketTest, EmptyPassthroughState) { metadata_ = nullptr; - filter_state_objects_ = nullptr; auto state = std::make_shared>(); NiceMock io_handle; EXPECT_CALL(io_handle, passthroughState()).WillRepeatedly(testing::Return(state)); EXPECT_CALL(*state, initialize(_, _)) .WillOnce(Invoke([&](std::unique_ptr metadata, - std::unique_ptr filter_state_objects) -> void { + const StreamInfo::FilterState::Objects& filter_state_objects) -> void { ASSERT_EQ(nullptr, metadata); - ASSERT_EQ(nullptr, filter_state_objects); + ASSERT_EQ(0, filter_state_objects.size()); })); initialize(io_handle); } @@ -127,9 +122,7 @@ class ConfigTest : public testing::Test { public: ConfigTest() : host_(std::make_shared>()), - host_metadata_(std::make_shared()), - filter_state_(std::make_shared( - StreamInfo::FilterState::LifeSpan::FilterChain)) { + host_metadata_(std::make_shared()) { ON_CALL(*host_, metadata()).WillByDefault(testing::Return(host_metadata_)); } @@ -141,7 +134,6 @@ class ConfigTest : public testing::Test { std::unique_ptr config_; std::shared_ptr> host_; std::shared_ptr host_metadata_; - std::shared_ptr filter_state_; }; constexpr absl::string_view SampleConfig = R"EOF( @@ -150,9 +142,6 @@ constexpr absl::string_view SampleConfig = R"EOF( name: host.metadata - kind: { cluster: {}} name: cluster.metadata - passthrough_filter_state_objects: - - name: filter_state_key - - name: readonly_filter_state transport_socket: name: raw_buffer typed_config: @@ -191,33 +180,7 @@ TEST_F(ConfigTest, Basic) { expected); EXPECT_THAT(*metadata, ProtoEq(expected)); - auto filter_state_object = std::make_shared(); - filter_state_->setData("filter_state_key", filter_state_object, - StreamInfo::FilterState::StateType::Mutable); - auto filter_state_objects = config_->extractFilterState(filter_state_); - const auto& [name, object] = filter_state_objects->at(0); - EXPECT_EQ("filter_state_key", name); - EXPECT_EQ(filter_state_object.get(), object.get()); - - std::vector keys; - config_->hashKey(keys, filter_state_); - EXPECT_GT(keys.size(), 0); - EXPECT_EQ(0, stats_store_.counter("internal_upstream.no_metadata").value()); - EXPECT_EQ(1, stats_store_.counter("internal_upstream.no_filter_state").value()); - EXPECT_EQ(0, stats_store_.counter("internal_upstream.filter_state_error").value()); -} - -TEST_F(ConfigTest, FilterStateError) { - TestUtility::loadFromYaml(std::string(SampleConfig), config_proto_); - initialize(); - auto filter_state_object = std::make_shared(); - filter_state_->setData("filter_state_key", filter_state_object, - StreamInfo::FilterState::StateType::ReadOnly); - auto filter_state_objects = config_->extractFilterState(filter_state_); - EXPECT_EQ(0, filter_state_objects->size()); - EXPECT_EQ(1, stats_store_.counter("internal_upstream.no_filter_state").value()); - EXPECT_EQ(1, stats_store_.counter("internal_upstream.filter_state_error").value()); } TEST_F(ConfigTest, UnsupportedMetadata) { @@ -235,42 +198,18 @@ TEST_F(ConfigTest, UnsupportedMetadata) { "metadata type is not supported: route {\n}\n"); } -TEST_F(ConfigTest, NonHashable) { - TestUtility::loadFromYaml(std::string(SampleConfig), config_proto_); - initialize(); - auto filter_state_object = std::make_shared(); - filter_state_->setData("filter_state_key", filter_state_object, - StreamInfo::FilterState::StateType::Mutable); - std::vector keys; - config_->hashKey(keys, filter_state_); - EXPECT_EQ(keys.size(), 0); -} - TEST_F(ConfigTest, MissingState) { TestUtility::loadFromYaml(std::string(SampleConfig), config_proto_); initialize(); auto metadata = config_->extractMetadata(host_); EXPECT_EQ(0, metadata->filter_metadata().size()); - auto filter_state_objects = config_->extractFilterState(filter_state_); - EXPECT_EQ(0, filter_state_objects->size()); - std::vector keys; - config_->hashKey(keys, filter_state_); - EXPECT_EQ(keys.size(), 0); EXPECT_EQ(2, stats_store_.counter("internal_upstream.no_metadata").value()); - EXPECT_EQ(2, stats_store_.counter("internal_upstream.no_filter_state").value()); - EXPECT_EQ(0, stats_store_.counter("internal_upstream.filter_state_error").value()); } TEST_F(ConfigTest, Empty) { initialize(); EXPECT_EQ(nullptr, config_->extractMetadata(host_)); - EXPECT_EQ(nullptr, config_->extractFilterState(filter_state_)); - std::vector keys; - config_->hashKey(keys, filter_state_); - EXPECT_EQ(keys.size(), 0); EXPECT_EQ(0, stats_store_.counter("internal_upstream.no_metadata").value()); - EXPECT_EQ(0, stats_store_.counter("internal_upstream.no_filter_state").value()); - EXPECT_EQ(0, stats_store_.counter("internal_upstream.filter_state_error").value()); } } // namespace diff --git a/test/integration/filters/header_to_filter_state.cc b/test/integration/filters/header_to_filter_state.cc index eefd81bd3f080..4573f81b980c4 100644 --- a/test/integration/filters/header_to_filter_state.cc +++ b/test/integration/filters/header_to_filter_state.cc @@ -17,8 +17,9 @@ namespace Envoy { // save it into FilterState as name "state" as read-only Router::StringAccessor. class HeaderToFilterStateFilter : public Http::PassThroughDecoderFilter { public: - HeaderToFilterStateFilter(const std::string& header, const std::string& state, bool read_only) - : header_(header), state_(state), read_only_(read_only) {} + HeaderToFilterStateFilter(const std::string& header, const std::string& state, bool read_only, + bool shared) + : header_(header), state_(state), read_only_(read_only), shared_(shared) {} Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, bool) override { const auto entry = headers.get(header_); @@ -27,7 +28,9 @@ class HeaderToFilterStateFilter : public Http::PassThroughDecoderFilter { state_, std::make_unique(entry[0]->value().getStringView()), read_only_ ? StreamInfo::FilterState::StateType::ReadOnly : StreamInfo::FilterState::StateType::Mutable, - StreamInfo::FilterState::LifeSpan::FilterChain); + StreamInfo::FilterState::LifeSpan::FilterChain, + shared_ ? StreamInfo::FilterState::StreamSharing::SharedWithUpstreamConnection + : StreamInfo::FilterState::StreamSharing::None); } return Http::FilterHeadersStatus::Continue; } @@ -36,6 +39,7 @@ class HeaderToFilterStateFilter : public Http::PassThroughDecoderFilter { Http::LowerCaseString header_; std::string state_; bool read_only_; + bool shared_; }; class HeaderToFilterStateFilterFactory @@ -50,7 +54,8 @@ class HeaderToFilterStateFilterFactory const std::string&, Server::Configuration::FactoryContext&) override { return [=](Http::FilterChainFactoryCallbacks& callbacks) -> void { callbacks.addStreamDecoderFilter(std::make_shared( - proto_config.header_name(), proto_config.state_name(), proto_config.read_only())); + proto_config.header_name(), proto_config.state_name(), proto_config.read_only(), + proto_config.shared())); }; } }; diff --git a/test/integration/filters/header_to_filter_state.proto b/test/integration/filters/header_to_filter_state.proto index 5ae44014960db..2405804ab9756 100644 --- a/test/integration/filters/header_to_filter_state.proto +++ b/test/integration/filters/header_to_filter_state.proto @@ -6,4 +6,5 @@ message HeaderToFilterStateFilterConfig { string header_name = 1; string state_name = 2; bool read_only = 3; + bool shared = 4; } diff --git a/test/mocks/event/mocks.h b/test/mocks/event/mocks.h index 4ffcfa7ab1323..539190a215c5d 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -55,7 +55,8 @@ class MockDispatcher : public Dispatcher { createClientConnection(Network::Address::InstanceConstSharedPtr address, Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options) override { + const Network::ConnectionSocket::OptionsSharedPtr& options, + Network::TransportSocketOptionsConstSharedPtr = nullptr) override { return Network::ClientConnectionPtr{ createClientConnection_(address, source_address, transport_socket, options)}; } diff --git a/test/mocks/event/wrapped_dispatcher.h b/test/mocks/event/wrapped_dispatcher.h index f2ccd6db54589..1ce08a5225358 100644 --- a/test/mocks/event/wrapped_dispatcher.h +++ b/test/mocks/event/wrapped_dispatcher.h @@ -41,13 +41,14 @@ class WrappedDispatcher : public Dispatcher { stream_info); } - Network::ClientConnectionPtr - createClientConnection(Network::Address::InstanceConstSharedPtr address, - Network::Address::InstanceConstSharedPtr source_address, - Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options) override { + Network::ClientConnectionPtr createClientConnection( + Network::Address::InstanceConstSharedPtr address, + Network::Address::InstanceConstSharedPtr source_address, + Network::TransportSocketPtr&& transport_socket, + const Network::ConnectionSocket::OptionsSharedPtr& options, + Network::TransportSocketOptionsConstSharedPtr transport_options = nullptr) override { return impl_.createClientConnection(std::move(address), std::move(source_address), - std::move(transport_socket), options); + std::move(transport_socket), options, transport_options); } FileEventPtr createFileEvent(os_fd_t fd, FileReadyCb cb, FileTriggerType trigger, From eaca9dbbaa11ffd69690c54503fd3f92fd9490d1 Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Tue, 28 Jun 2022 09:51:24 -0700 Subject: [PATCH 05/12] wip Signed-off-by: Kuat Yessenov --- changelogs/current.yaml | 4 ++++ .../other_features/internal_listener.rst | 2 -- envoy/event/dispatcher.h | 2 +- envoy/network/client_connection_factory.h | 3 ++- source/common/event/dispatcher_impl.cc | 5 ++--- source/common/event/dispatcher_impl.h | 2 +- source/common/network/connection_impl.cc | 1 + .../network/default_client_connection_factory.cc | 2 +- .../network/default_client_connection_factory.h | 2 +- .../internal_listener/client_connection_factory.cc | 2 +- .../internal_listener/client_connection_factory.h | 13 ++++++------- source/server/config_validation/connection.h | 5 ++--- source/server/config_validation/dispatcher.cc | 4 ++-- source/server/config_validation/dispatcher.h | 9 ++++----- .../user_space/connection_compatbility_test.cc | 6 +++--- .../starttls/starttls_integration_test.cc | 2 +- test/mocks/event/mocks.h | 2 +- test/mocks/event/wrapped_dispatcher.h | 2 +- 18 files changed, 34 insertions(+), 34 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 34b1f5ebc6c9b..bafe6af908333 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -116,6 +116,10 @@ minor_behavior_changes: - area: logging change: | changed category name for access log filter extensions to ``envoy.access_loggers.extension_filters``. +- area: filter state + change: | + revert to respecting the life time of the filter state objects to be bound to the original stream and make sharing + filter state objects with the upstream explicit via a flag. bug_fixes: - area: http diff --git a/docs/root/configuration/other_features/internal_listener.rst b/docs/root/configuration/other_features/internal_listener.rst index 8259c016fff5d..199033b331dac 100644 --- a/docs/root/configuration/other_features/internal_listener.rst +++ b/docs/root/configuration/other_features/internal_listener.rst @@ -38,8 +38,6 @@ This extension emits the following statistics: :widths: 1, 1, 2 no_metadata, Counter, Metadata key is absent from the import location. - no_filter_state, Counter, Filter state object key is absent from the downstream filter state. - filter_state_error, Counter, Unable to copy the filter state object (e.g. it is read only). Example config -------------- diff --git a/envoy/event/dispatcher.h b/envoy/event/dispatcher.h index 2b30550cc2598..5cfc3f87b8f8c 100644 --- a/envoy/event/dispatcher.h +++ b/envoy/event/dispatcher.h @@ -218,7 +218,7 @@ class Dispatcher : public DispatcherBase, public ScopeTracker { Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - Network::TransportSocketOptionsConstSharedPtr transport_options = nullptr) PURE; + const Network::TransportSocketOptionsConstSharedPtr& transport_options = nullptr) PURE; /** * @return Filesystem::WatcherPtr a filesystem watcher owned by the caller. diff --git a/envoy/network/client_connection_factory.h b/envoy/network/client_connection_factory.h index ff39034793cf1..ed06666c9eeeb 100644 --- a/envoy/network/client_connection_factory.h +++ b/envoy/network/client_connection_factory.h @@ -23,6 +23,7 @@ class ClientConnectionFactory : public Config::UntypedFactory { * @param source_address Optional source address. * @param transport_socket The transport socket which supports this client connection. * @param options The optional socket options. + * @param transport socket options used to create the transport socket. * @return Network::ClientConnectionPtr The created connection. It's never nullptr but the return * connection may be closed upon return. */ @@ -31,7 +32,7 @@ class ClientConnectionFactory : public Config::UntypedFactory { Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - Network::TransportSocketOptionsConstSharedPtr transport_options = nullptr) PURE; + const Network::TransportSocketOptionsConstSharedPtr& transport_options) PURE; }; } // namespace Network diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index 633e5b08b3ecf..104d72afc979e 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -160,7 +160,7 @@ Network::ClientConnectionPtr DispatcherImpl::createClientConnection( Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - Network::TransportSocketOptionsConstSharedPtr transport_options) { + const Network::TransportSocketOptionsConstSharedPtr& transport_options) { ASSERT(isThreadSafe()); auto* factory = Config::Utility::getFactoryByName( @@ -171,8 +171,7 @@ Network::ClientConnectionPtr DispatcherImpl::createClientConnection( // expects a non-null connection as of today so we cannot gracefully handle unsupported address // type. return factory->createClientConnection(*this, address, source_address, - std::move(transport_socket), options, - std::move(transport_options)); + std::move(transport_socket), options, transport_options); } FileEventPtr DispatcherImpl::createFileEvent(os_fd_t fd, FileReadyCb cb, FileTriggerType trigger, diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index 62b1bdd95cdae..ffef3427923c3 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -71,7 +71,7 @@ class DispatcherImpl : Logger::Loggable, Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - Network::TransportSocketOptionsConstSharedPtr transport_options = nullptr) override; + const Network::TransportSocketOptionsConstSharedPtr& transport_options = nullptr) override; FileEventPtr createFileEvent(os_fd_t fd, FileReadyCb cb, FileTriggerType trigger, uint32_t events) override; Filesystem::WatcherPtr createFilesystemWatcher() override; diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc index 36f27e11a0d29..45cd17451fa6f 100644 --- a/source/common/network/connection_impl.cc +++ b/source/common/network/connection_impl.cc @@ -911,6 +911,7 @@ ClientConnectionImpl::ClientConnectionImpl( if (transport_options) { for (const auto& object : transport_options->filterStateObjects()) { + // This does not throw as all objects are distinctly named and the stream info is empty. stream_info_.filterState()->setData(object.name_, object.data_, object.state_type_, StreamInfo::FilterState::LifeSpan::Connection, object.stream_sharing_); diff --git a/source/common/network/default_client_connection_factory.cc b/source/common/network/default_client_connection_factory.cc index 53351579ff493..31b3f6e03cb85 100644 --- a/source/common/network/default_client_connection_factory.cc +++ b/source/common/network/default_client_connection_factory.cc @@ -14,7 +14,7 @@ Network::ClientConnectionPtr DefaultClientConnectionFactory::createClientConnect Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - Network::TransportSocketOptionsConstSharedPtr transport_options) { + const Network::TransportSocketOptionsConstSharedPtr& transport_options) { ASSERT(address->ip() || address->pipe()); return std::make_unique( dispatcher, address, source_address, std::move(transport_socket), options, transport_options); diff --git a/source/common/network/default_client_connection_factory.h b/source/common/network/default_client_connection_factory.h index 547881aec4da5..e82ad46a7b8d3 100644 --- a/source/common/network/default_client_connection_factory.h +++ b/source/common/network/default_client_connection_factory.h @@ -25,7 +25,7 @@ class DefaultClientConnectionFactory : public ClientConnectionFactory { Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - Network::TransportSocketOptionsConstSharedPtr transport_options = nullptr) override; + const Network::TransportSocketOptionsConstSharedPtr& transport_options) override; }; } // namespace Network diff --git a/source/extensions/bootstrap/internal_listener/client_connection_factory.cc b/source/extensions/bootstrap/internal_listener/client_connection_factory.cc index ce098fcc8c5d9..014e2ea534ed6 100644 --- a/source/extensions/bootstrap/internal_listener/client_connection_factory.cc +++ b/source/extensions/bootstrap/internal_listener/client_connection_factory.cc @@ -21,7 +21,7 @@ Network::ClientConnectionPtr InternalClientConnectionFactory::createClientConnec Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - Network::TransportSocketOptionsConstSharedPtr transport_options) { + const Network::TransportSocketOptionsConstSharedPtr& transport_options) { // OS does not fill the address automatically so a pivotal address is populated. // TODO(lambdai): provide option to fill the downstream remote address here. if (source_address == nullptr) { diff --git a/source/extensions/bootstrap/internal_listener/client_connection_factory.h b/source/extensions/bootstrap/internal_listener/client_connection_factory.h index e05e9aba501f4..2a712f337966e 100644 --- a/source/extensions/bootstrap/internal_listener/client_connection_factory.h +++ b/source/extensions/bootstrap/internal_listener/client_connection_factory.h @@ -20,13 +20,12 @@ class InternalClientConnectionFactory : public Network::ClientConnectionFactory, Logger::Loggable { public: std::string name() const override { return "envoy_internal"; } - Network::ClientConnectionPtr - createClientConnection(Event::Dispatcher& dispatcher, - Network::Address::InstanceConstSharedPtr address, - Network::Address::InstanceConstSharedPtr source_address, - Network::TransportSocketPtr&& transport_socket, - const Network::ConnectionSocket::OptionsSharedPtr& options, - Network::TransportSocketOptionsConstSharedPtr transport_options) override; + Network::ClientConnectionPtr createClientConnection( + Event::Dispatcher& dispatcher, Network::Address::InstanceConstSharedPtr address, + Network::Address::InstanceConstSharedPtr source_address, + Network::TransportSocketPtr&& transport_socket, + const Network::ConnectionSocket::OptionsSharedPtr& options, + const Network::TransportSocketOptionsConstSharedPtr& transport_options) override; // The slot is owned by the internal listener registry extension. Once that extension is // initialized, this slot is available. The ClientConnectionFactory has two potential user cases. // 1. The per worker thread connection handler populates the per worker listener registry. diff --git a/source/server/config_validation/connection.h b/source/server/config_validation/connection.h index 53cbeb7f0e0a5..575a350c94ab2 100644 --- a/source/server/config_validation/connection.h +++ b/source/server/config_validation/connection.h @@ -18,10 +18,9 @@ class ConfigValidateConnection : public Network::ClientConnectionImpl { Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - Network::TransportSocketOptionsConstSharedPtr transport_options) + const Network::TransportSocketOptionsConstSharedPtr& transport_options) : Network::ClientConnectionImpl(dispatcher, remote_address, source_address, - std::move(transport_socket), options, - std::move(transport_options)) {} + std::move(transport_socket), options, transport_options) {} // Unit tests may instantiate it without proper event machine and leave opened sockets. // Do some cleanup before invoking base class's destructor. diff --git a/source/server/config_validation/dispatcher.cc b/source/server/config_validation/dispatcher.cc index a8b3ac146425b..9e88c3975476e 100644 --- a/source/server/config_validation/dispatcher.cc +++ b/source/server/config_validation/dispatcher.cc @@ -11,10 +11,10 @@ Network::ClientConnectionPtr ValidationDispatcher::createClientConnection( Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - Network::TransportSocketOptionsConstSharedPtr transport_options) { + const Network::TransportSocketOptionsConstSharedPtr& transport_options) { return std::make_unique(*this, remote_address, source_address, std::move(transport_socket), options, - std::move(transport_options)); + transport_options); } Network::ListenerPtr ValidationDispatcher::createListener(Network::SocketSharedPtr&&, diff --git a/source/server/config_validation/dispatcher.h b/source/server/config_validation/dispatcher.h index b8f568bc54e1e..a4aa824a00c11 100644 --- a/source/server/config_validation/dispatcher.h +++ b/source/server/config_validation/dispatcher.h @@ -19,11 +19,10 @@ class ValidationDispatcher : public DispatcherImpl { ValidationDispatcher(const std::string& name, Api::Api& api, Event::TimeSystem& time_system) : DispatcherImpl(name, api, time_system) {} - Network::ClientConnectionPtr - createClientConnection(Network::Address::InstanceConstSharedPtr, - Network::Address::InstanceConstSharedPtr, Network::TransportSocketPtr&&, - const Network::ConnectionSocket::OptionsSharedPtr& options, - Network::TransportSocketOptionsConstSharedPtr transport_options) override; + Network::ClientConnectionPtr createClientConnection( + Network::Address::InstanceConstSharedPtr, Network::Address::InstanceConstSharedPtr, + Network::TransportSocketPtr&&, const Network::ConnectionSocket::OptionsSharedPtr& options, + const Network::TransportSocketOptionsConstSharedPtr& transport_options) override; Network::ListenerPtr createListener(Network::SocketSharedPtr&&, Network::TcpListenerCallbacks&, Runtime::Loader& runtime, bool bind_to_port, bool ignore_global_conn_limit) override; diff --git a/test/extensions/io_socket/user_space/connection_compatbility_test.cc b/test/extensions/io_socket/user_space/connection_compatbility_test.cc index e7410aa7bb3dc..c5044fae25f8a 100644 --- a/test/extensions/io_socket/user_space/connection_compatbility_test.cc +++ b/test/extensions/io_socket/user_space/connection_compatbility_test.cc @@ -49,7 +49,7 @@ TEST_F(InternalClientConnectionImplTest, Basic) { *dispatcher_, std::make_unique(std::move(io_handle_), local_addr_, remote_addr_), - nullptr, std::make_unique(), nullptr); + nullptr, std::make_unique(), nullptr, nullptr); client_->connect(); client_->noDelay(true); dispatcher_->run(Event::Dispatcher::RunType::Block); @@ -62,7 +62,7 @@ TEST_F(InternalClientConnectionImplTest, ConnectCallbacksAreInvoked) { *dispatcher_, std::make_unique(std::move(io_handle_), local_addr_, remote_addr_), - nullptr, std::make_unique(), nullptr); + nullptr, std::make_unique(), nullptr, nullptr); client_->addConnectionCallbacks(connection_callbacks); client_->connect(); client_->noDelay(true); @@ -82,7 +82,7 @@ TEST_F(InternalClientConnectionImplTest, ConnectFailed) { *dispatcher_, std::make_unique(std::move(io_handle_), local_addr_, remote_addr_), - nullptr, std::make_unique(), nullptr); + nullptr, std::make_unique(), nullptr, nullptr); client_->addConnectionCallbacks(connection_callbacks); client_->connect(); client_->noDelay(true); diff --git a/test/extensions/transport_sockets/starttls/starttls_integration_test.cc b/test/extensions/transport_sockets/starttls/starttls_integration_test.cc index d57b130930a97..c4c02be078ad3 100644 --- a/test/extensions/transport_sockets/starttls/starttls_integration_test.cc +++ b/test/extensions/transport_sockets/starttls/starttls_integration_test.cc @@ -136,7 +136,7 @@ class ClientTestConnection : public Network::ClientConnectionImpl { Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options) : ClientConnectionImpl(dispatcher, remote_address, source_address, - std::move(transport_socket), options) {} + std::move(transport_socket), options, nullptr) {} void setTransportSocket(Network::TransportSocketPtr&& transport_socket) { transport_socket_ = std::move(transport_socket); diff --git a/test/mocks/event/mocks.h b/test/mocks/event/mocks.h index 539190a215c5d..80cee9064a5c0 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -56,7 +56,7 @@ class MockDispatcher : public Dispatcher { Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - Network::TransportSocketOptionsConstSharedPtr = nullptr) override { + const Network::TransportSocketOptionsConstSharedPtr& = nullptr) override { return Network::ClientConnectionPtr{ createClientConnection_(address, source_address, transport_socket, options)}; } diff --git a/test/mocks/event/wrapped_dispatcher.h b/test/mocks/event/wrapped_dispatcher.h index 1ce08a5225358..66b22b0994fa3 100644 --- a/test/mocks/event/wrapped_dispatcher.h +++ b/test/mocks/event/wrapped_dispatcher.h @@ -46,7 +46,7 @@ class WrappedDispatcher : public Dispatcher { Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - Network::TransportSocketOptionsConstSharedPtr transport_options = nullptr) override { + const Network::TransportSocketOptionsConstSharedPtr& transport_options = nullptr) override { return impl_.createClientConnection(std::move(address), std::move(source_address), std::move(transport_socket), options, transport_options); } From 0c3b11f2f43932bda04e7eb211f916560eed36fd Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Tue, 28 Jun 2022 11:34:50 -0700 Subject: [PATCH 06/12] tests Signed-off-by: Kuat Yessenov --- .../common/stream_info/filter_state_impl.cc | 2 +- test/common/network/connection_impl_test.cc | 13 ++++++- .../transport_socket_options_impl_test.cc | 39 +++++++++++++++++++ .../stream_info/filter_state_impl_test.cc | 28 +++++++++++++ 4 files changed, 80 insertions(+), 2 deletions(-) diff --git a/source/common/stream_info/filter_state_impl.cc b/source/common/stream_info/filter_state_impl.cc index e54178a6a6ec7..97a64dd7f4611 100644 --- a/source/common/stream_info/filter_state_impl.cc +++ b/source/common/stream_info/filter_state_impl.cc @@ -14,7 +14,7 @@ void FilterStateImpl::setData(absl::string_view data_name, std::shared_ptr called twice with conflicting life_span on the same data_name."); } maybeCreateParent(ParentAccessMode::ReadWrite); - parent_->setData(data_name, data, state_type, life_span); + parent_->setData(data_name, data, state_type, life_span, stream_sharing); return; } if (parent_ && parent_->hasDataWithName(data_name)) { diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index eed07b6301bd4..a470a9787484a 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -161,7 +161,7 @@ class ConnectionImplTest : public testing::TestWithParam { listener_ = dispatcher_->createListener(socket_, listener_callbacks_, runtime_, true, false); client_connection_ = std::make_unique( *dispatcher_, socket_->connectionInfoProvider().localAddress(), source_address_, - createTransportSocket(), socket_options_, nullptr); + createTransportSocket(), socket_options_, transport_socket_options_); client_connection_->addConnectionCallbacks(client_callbacks_); EXPECT_EQ(nullptr, client_connection_->ssl()); const Network::ClientConnection& const_connection = *client_connection_; @@ -293,6 +293,7 @@ class ConnectionImplTest : public testing::TestWithParam { Address::InstanceConstSharedPtr source_address_; Socket::OptionsSharedPtr socket_options_; StreamInfo::StreamInfoImpl stream_info_; + Network::TransportSocketOptionsConstSharedPtr transport_socket_options_ = nullptr; }; INSTANTIATE_TEST_SUITE_P(IpVersions, ConnectionImplTest, @@ -3085,10 +3086,20 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, ClientConnectionWithCustomRawBufferSocketTe testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), TestUtility::ipTestParamsToString); +class TestObject : public StreamInfo::FilterState::Object {}; + TEST_P(ClientConnectionWithCustomRawBufferSocketTest, TransportSocketCallbacks) { + StreamInfo::FilterStateImpl filter_state(StreamInfo::FilterState::LifeSpan::Connection); + auto filter_state_object = std::make_shared(); + filter_state.setData("test-filter-state", filter_state_object, + StreamInfo::FilterState::StateType::ReadOnly, + StreamInfo::FilterState::LifeSpan::Connection, + StreamInfo::FilterState::StreamSharing::SharedWithUpstreamConnection); + transport_socket_options_ = TransportSocketOptionsUtility::fromFilterState(filter_state); setUpBasicConnection(); EXPECT_TRUE(getTransportSocket()->compareCallbacks(testClientConnection())); + EXPECT_TRUE(client_connection_->streamInfo().filterState()->hasDataWithName("test-filter-state")); disconnect(false); } diff --git a/test/common/network/transport_socket_options_impl_test.cc b/test/common/network/transport_socket_options_impl_test.cc index 912d00cc5a4c5..837cb4bf8c819 100644 --- a/test/common/network/transport_socket_options_impl_test.cc +++ b/test/common/network/transport_socket_options_impl_test.cc @@ -86,6 +86,45 @@ TEST_F(TransportSocketOptionsImplTest, Both) { EXPECT_EQ(http_alpns, transport_socket_options->applicationProtocolListOverride()); } +class TestTransportSocketFactory : public CommonUpstreamTransportSocketFactory { +public: + TransportSocketPtr + createTransportSocket(TransportSocketOptionsConstSharedPtr, + std::shared_ptr) const override { + return nullptr; + } + absl::string_view defaultServerNameIndication() const override { return ""; } +}; + +class NonHashableObj : public StreamInfo::FilterState::Object {}; +class HashableObj : public StreamInfo::FilterState::Object, public Hashable { + absl::optional hash() const override { return 12345; }; +}; + +TEST_F(TransportSocketOptionsImplTest, FilterStateHashable) { + filter_state_.setData("hashable", std::make_shared(), + StreamInfo::FilterState::StateType::ReadOnly, + StreamInfo::FilterState::LifeSpan::FilterChain, + StreamInfo::FilterState::StreamSharing::SharedWithUpstreamConnection); + auto transport_socket_options = TransportSocketOptionsUtility::fromFilterState(filter_state_); + TestTransportSocketFactory factory; + std::vector keys; + factory.hashKey(keys, transport_socket_options); + EXPECT_GT(keys.size(), 0); +} + +TEST_F(TransportSocketOptionsImplTest, FilterStateNonHashable) { + filter_state_.setData("non-hashable", std::make_shared(), + StreamInfo::FilterState::StateType::ReadOnly, + StreamInfo::FilterState::LifeSpan::FilterChain, + StreamInfo::FilterState::StreamSharing::SharedWithUpstreamConnection); + auto transport_socket_options = TransportSocketOptionsUtility::fromFilterState(filter_state_); + TestTransportSocketFactory factory; + std::vector keys; + factory.hashKey(keys, transport_socket_options); + EXPECT_EQ(keys.size(), 0); +} + } // namespace } // namespace Network } // namespace Envoy diff --git a/test/common/stream_info/filter_state_impl_test.cc b/test/common/stream_info/filter_state_impl_test.cc index ae69c438f2300..7915f06d63864 100644 --- a/test/common/stream_info/filter_state_impl_test.cc +++ b/test/common/stream_info/filter_state_impl_test.cc @@ -348,6 +348,34 @@ TEST_F(FilterStateImplTest, LifeSpanInitFromNonParent) { EXPECT_FALSE(new_filter_state.hasDataWithName("test_6")); } +TEST_F(FilterStateImplTest, SharedWithUpstream) { + auto shared = std::make_shared(1); + filter_state().setData("shared_1", shared, FilterState::StateType::ReadOnly, + FilterState::LifeSpan::FilterChain, + FilterState::StreamSharing::SharedWithUpstreamConnection); + filter_state().setData("test_2", std::make_shared(2), FilterState::StateType::Mutable, + FilterState::LifeSpan::FilterChain); + filter_state().setData("test_3", std::make_shared(3), + FilterState::StateType::ReadOnly, FilterState::LifeSpan::Request); + filter_state().setData("shared_4", std::make_shared(4), + FilterState::StateType::Mutable, FilterState::LifeSpan::Request, + FilterState::StreamSharing::SharedWithUpstreamConnection); + filter_state().setData("shared_5", std::make_shared(5), + FilterState::StateType::ReadOnly, FilterState::LifeSpan::Connection, + FilterState::StreamSharing::SharedWithUpstreamConnection); + filter_state().setData("test_6", std::make_shared(6), FilterState::StateType::Mutable, + FilterState::LifeSpan::Connection); + auto objects = filter_state().objectsSharedWithUpstreamConnection(); + EXPECT_EQ(objects->size(), 3); + EXPECT_EQ(objects->at(0).name_, "shared_5"); + EXPECT_EQ(objects->at(0).state_type_, FilterState::StateType::ReadOnly); + EXPECT_EQ(objects->at(1).name_, "shared_4"); + EXPECT_EQ(objects->at(1).state_type_, FilterState::StateType::Mutable); + EXPECT_EQ(objects->at(2).name_, "shared_1"); + EXPECT_EQ(objects->at(2).state_type_, FilterState::StateType::ReadOnly); + EXPECT_EQ(objects->at(2).data_.get(), shared.get()); +} + TEST_F(FilterStateImplTest, HasDataAtOrAboveLifeSpan) { filter_state().setData("test_1", std::make_unique(1), FilterState::StateType::ReadOnly, FilterState::LifeSpan::FilterChain); From 9e5a187f5920156368b6b016a4ebbe89d6008bc5 Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Tue, 28 Jun 2022 12:08:07 -0700 Subject: [PATCH 07/12] fix Signed-off-by: Kuat Yessenov --- test/common/network/transport_socket_options_impl_test.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/common/network/transport_socket_options_impl_test.cc b/test/common/network/transport_socket_options_impl_test.cc index 837cb4bf8c819..347d6d18c582c 100644 --- a/test/common/network/transport_socket_options_impl_test.cc +++ b/test/common/network/transport_socket_options_impl_test.cc @@ -1,3 +1,5 @@ +#include "envoy/common/hashable.h" + #include "source/common/http/utility.h" #include "source/common/network/address_impl.h" #include "source/common/network/application_protocol.h" @@ -94,6 +96,7 @@ class TestTransportSocketFactory : public CommonUpstreamTransportSocketFactory { return nullptr; } absl::string_view defaultServerNameIndication() const override { return ""; } + bool implementsSecureTransport() const override { return false; } }; class NonHashableObj : public StreamInfo::FilterState::Object {}; From cd1f7c53a41c15981a0b542cc240b36d2531188d Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Tue, 28 Jun 2022 15:11:11 -0700 Subject: [PATCH 08/12] remove default argument Signed-off-by: Kuat Yessenov --- envoy/event/dispatcher.h | 2 +- source/common/event/dispatcher_impl.h | 2 +- test/common/event/dispatcher_impl_test.cc | 4 +-- test/common/http/codec_client_test.cc | 2 +- test/common/network/connection_impl_test.cc | 33 ++++++++--------- test/common/network/listener_impl_test.cc | 34 +++++++++--------- .../client_connection_factory_test.cc | 12 ++++--- .../proxy_protocol_regression_test.cc | 3 +- .../common/fuzz/listener_filter_fuzzer.cc | 6 ++-- .../proxy_protocol/proxy_protocol_test.cc | 9 ++--- .../tls_inspector_integration_test.cc | 5 +-- .../proxy_filter_integration_test.cc | 2 +- .../alts/alts_integration_test.cc | 3 +- .../default_validator_integration_test.cc | 2 +- .../spiffe_validator_integration_test.cc | 2 +- .../tls/integration/ssl_integration_test.cc | 2 +- .../transport_sockets/tls/ssl_socket_test.cc | 36 +++++++++---------- test/integration/base_integration_test.cc | 3 +- test/integration/integration_tcp_client.cc | 2 +- test/integration/utility.cc | 12 +++---- test/mocks/event/mocks.h | 2 +- test/mocks/event/wrapped_dispatcher.h | 2 +- .../config_validation/dispatcher_test.cc | 2 +- 23 files changed, 96 insertions(+), 86 deletions(-) diff --git a/envoy/event/dispatcher.h b/envoy/event/dispatcher.h index 5cfc3f87b8f8c..c3741bc15faf4 100644 --- a/envoy/event/dispatcher.h +++ b/envoy/event/dispatcher.h @@ -218,7 +218,7 @@ class Dispatcher : public DispatcherBase, public ScopeTracker { Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - const Network::TransportSocketOptionsConstSharedPtr& transport_options = nullptr) PURE; + const Network::TransportSocketOptionsConstSharedPtr& transport_options) PURE; /** * @return Filesystem::WatcherPtr a filesystem watcher owned by the caller. diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index ffef3427923c3..a55738a02e278 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -71,7 +71,7 @@ class DispatcherImpl : Logger::Loggable, Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - const Network::TransportSocketOptionsConstSharedPtr& transport_options = nullptr) override; + const Network::TransportSocketOptionsConstSharedPtr& transport_options) override; FileEventPtr createFileEvent(os_fd_t fd, FileReadyCb cb, FileTriggerType trigger, uint32_t events) override; Filesystem::WatcherPtr createFilesystemWatcher() override; diff --git a/test/common/event/dispatcher_impl_test.cc b/test/common/event/dispatcher_impl_test.cc index 291b39038a317..4c1c87869ac9b 100644 --- a/test/common/event/dispatcher_impl_test.cc +++ b/test/common/event/dispatcher_impl_test.cc @@ -1568,7 +1568,7 @@ TEST_F(DispatcherConnectionTest, CreateTcpConnection) { fmt::format("{}:{}", Network::Test::getLoopbackAddressUrlString(ip_version), 10911)); auto client_conn = dispatcher_->createClientConnection( client_addr_port, Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); EXPECT_NE(nullptr, client_conn); client_conn->close(Network::ConnectionCloseType::NoFlush); } @@ -1581,7 +1581,7 @@ TEST_F(DispatcherConnectionTest, CreateEnvoyInternalConnectionWhenFactoryNotExis dispatcher_->createClientConnection( std::make_shared("listener_internal_address"), Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), - nullptr), + nullptr, nullptr), ""); } diff --git a/test/common/http/codec_client_test.cc b/test/common/http/codec_client_test.cc index bd49443748bc8..444893d5be0d4 100644 --- a/test/common/http/codec_client_test.cc +++ b/test/common/http/codec_client_test.cc @@ -292,7 +292,7 @@ class CodecNetworkTest : public Event::TestUsingSimulatedTime, Network::Test::getCanonicalLoopbackAddress(GetParam())); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), source_address_, - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); upstream_listener_ = dispatcher_->createListener(std::move(socket), listener_callbacks_, runtime_, true, false); client_connection_ = client_connection.get(); diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index a470a9787484a..67112035a98cb 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -433,7 +433,7 @@ TEST_P(ConnectionImplTest, ImmediateConnectError) { } client_connection_ = dispatcher_->createClientConnection( - broadcast_address, source_address_, Network::Test::createRawBufferSocket(), nullptr); + broadcast_address, source_address_, Network::Test::createRawBufferSocket(), nullptr, nullptr); client_connection_->addConnectionCallbacks(client_callbacks_); client_connection_->connect(); @@ -551,7 +551,7 @@ TEST_P(ConnectionImplTest, SocketOptions) { upstream_connection_ = dispatcher_->createClientConnection( socket_->connectionInfoProvider().localAddress(), source_address_, - Network::Test::createRawBufferSocket(), server_connection_->socketOptions()); + Network::Test::createRawBufferSocket(), server_connection_->socketOptions(), nullptr); })); EXPECT_CALL(server_callbacks_, onEvent(ConnectionEvent::RemoteClose)) @@ -600,7 +600,7 @@ TEST_P(ConnectionImplTest, SocketOptionsFailureTest) { upstream_connection_ = dispatcher_->createClientConnection( socket_->connectionInfoProvider().localAddress(), source_address_, - Network::Test::createRawBufferSocket(), server_connection_->socketOptions()); + Network::Test::createRawBufferSocket(), server_connection_->socketOptions(), nullptr); upstream_connection_->addConnectionCallbacks(upstream_callbacks_); })); @@ -1407,7 +1407,7 @@ TEST_P(ConnectionImplTest, BindFailureTest) { client_connection_ = dispatcher_->createClientConnection( socket_->connectionInfoProvider().localAddress(), source_address_, - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); MockConnectionStats connection_stats; client_connection_->setConnectionStats(connection_stats.toBufferStats()); @@ -2879,10 +2879,10 @@ class ReadBufferLimitTest : public ConnectionImplTest { Network::Test::getCanonicalLoopbackAddress(GetParam())); listener_ = dispatcher_->createListener(socket_, listener_callbacks_, runtime_, true, false); - client_connection_ = - dispatcher_->createClientConnection(socket_->connectionInfoProvider().localAddress(), - Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + client_connection_ = dispatcher_->createClientConnection( + socket_->connectionInfoProvider().localAddress(), + Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), nullptr, + nullptr); client_connection_->addConnectionCallbacks(client_callbacks_); client_connection_->connect(); @@ -2965,7 +2965,7 @@ TEST_P(TcpClientConnectionImplTest, BadConnectNotConnRefused) { } ClientConnectionPtr connection = dispatcher_->createClientConnection(address, Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); connection->connect(); connection->noDelay(true); connection->close(ConnectionCloseType::NoFlush); @@ -2978,7 +2978,8 @@ TEST_P(TcpClientConnectionImplTest, BadConnectConnRefused) { ClientConnectionPtr connection = dispatcher_->createClientConnection( Utility::resolveUrl( fmt::format("tcp://{}:1", Network::Test::getLoopbackAddressUrlString(GetParam()))), - Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), nullptr); + Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), nullptr, + nullptr); connection->connect(); connection->noDelay(true); dispatcher_->run(Event::Dispatcher::RunType::Block); @@ -2993,7 +2994,7 @@ TEST_P(TcpClientConnectionImplTest, BadConnectConnRefusedWithTransportError) { ClientConnectionPtr connection = dispatcher_->createClientConnection( Utility::resolveUrl( fmt::format("tcp://{}:1", Network::Test::getLoopbackAddressUrlString(GetParam()))), - Network::Address::InstanceConstSharedPtr(), std::move(transport_socket), nullptr); + Network::Address::InstanceConstSharedPtr(), std::move(transport_socket), nullptr, nullptr); connection->connect(); connection->noDelay(true); dispatcher_->run(Event::Dispatcher::RunType::Block); @@ -3019,7 +3020,7 @@ TEST_F(PipeClientConnectionImplTest, SkipSocketOptions) { options->emplace_back(option); ClientConnectionPtr connection = dispatcher_->createClientConnection( Utility::resolveUrl("unix://" + path_), Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), options); + Network::Test::createRawBufferSocket(), options, nullptr); connection->close(ConnectionCloseType::NoFlush); } @@ -3027,7 +3028,7 @@ TEST_F(PipeClientConnectionImplTest, SkipSocketOptions) { TEST_F(PipeClientConnectionImplTest, SkipSourceAddress) { ClientConnectionPtr connection = dispatcher_->createClientConnection( Utility::resolveUrl("unix://" + path_), Utility::resolveUrl("tcp://1.2.3.4:5"), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); connection->close(ConnectionCloseType::NoFlush); } @@ -3056,9 +3057,9 @@ TEST_F(InternalClientConnectionImplTest, ASSERT_DEATH( { - ClientConnectionPtr connection = - dispatcher_->createClientConnection(address, Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + ClientConnectionPtr connection = dispatcher_->createClientConnection( + address, Network::Address::InstanceConstSharedPtr(), + Network::Test::createRawBufferSocket(), nullptr, nullptr); }, ""); } diff --git a/test/common/network/listener_impl_test.cc b/test/common/network/listener_impl_test.cc index f66202e75002e..252ac9af95dc3 100644 --- a/test/common/network/listener_impl_test.cc +++ b/test/common/network/listener_impl_test.cc @@ -42,7 +42,7 @@ static void errorCallbackTest(Address::IpVersion version) { Network::ClientConnectionPtr client_connection = dispatcher->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); client_connection->connect(); StreamInfo::StreamInfoImpl stream_info(dispatcher->timeSource(), nullptr); @@ -99,7 +99,7 @@ TEST_P(TcpListenerImplTest, UseActualDst) { Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); client_connection->connect(); EXPECT_CALL(listener, getLocalAddress(_)).Times(0); @@ -143,10 +143,10 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitEnforcement) { auto initiate_connections = [&](const int count) { for (int i = 0; i < count; ++i) { - client_connections.emplace_back( - dispatcher_->createClientConnection(socket->connectionInfoProvider().localAddress(), - Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr)); + client_connections.emplace_back(dispatcher_->createClientConnection( + socket->connectionInfoProvider().localAddress(), + Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), + nullptr, nullptr)); client_connections.back()->connect(); } }; @@ -208,10 +208,10 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitListenerOptOut) { auto initiate_connections = [&](const int count) { for (int i = 0; i < count; ++i) { - client_connections.emplace_back( - dispatcher_->createClientConnection(socket->connectionInfoProvider().localAddress(), - Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr)); + client_connections.emplace_back(dispatcher_->createClientConnection( + socket->connectionInfoProvider().localAddress(), + Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), + nullptr, nullptr)); client_connections.back()->connect(); } }; @@ -247,7 +247,7 @@ TEST_P(TcpListenerImplTest, WildcardListenerUseActualDst) { socket->connectionInfoProvider().localAddress()->ip()->port()); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( local_dst_address, Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); client_connection->connect(); StreamInfo::StreamInfoImpl stream_info(dispatcher_->timeSource(), nullptr); @@ -295,7 +295,7 @@ TEST_P(TcpListenerImplTest, WildcardListenerIpv4Compat) { socket->connectionInfoProvider().localAddress()->ip()->port()); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( local_dst_address, Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); client_connection->connect(); StreamInfo::StreamInfoImpl stream_info(dispatcher_->timeSource(), nullptr); @@ -333,7 +333,7 @@ TEST_P(TcpListenerImplTest, DisableAndEnableListener) { ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); client_connection->addConnectionCallbacks(connection_callbacks); client_connection->connect(); @@ -382,7 +382,7 @@ TEST_P(TcpListenerImplTest, SetListenerRejectFractionZero) { ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); client_connection->addConnectionCallbacks(connection_callbacks); client_connection->connect(); dispatcher_->run(Event::Dispatcher::RunType::Block); @@ -422,7 +422,7 @@ TEST_P(TcpListenerImplTest, SetListenerRejectFractionIntermediate) { { ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); client_connection->addConnectionCallbacks(connection_callbacks); client_connection->connect(); dispatcher_->run(Event::Dispatcher::RunType::Block); @@ -445,7 +445,7 @@ TEST_P(TcpListenerImplTest, SetListenerRejectFractionIntermediate) { { ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); client_connection->addConnectionCallbacks(connection_callbacks); client_connection->connect(); dispatcher_->run(Event::Dispatcher::RunType::Block); @@ -484,7 +484,7 @@ TEST_P(TcpListenerImplTest, SetListenerRejectFractionAll) { ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); client_connection->addConnectionCallbacks(connection_callbacks); client_connection->connect(); dispatcher_->run(Event::Dispatcher::RunType::Block); diff --git a/test/extensions/bootstrap/internal_listener/client_connection_factory_test.cc b/test/extensions/bootstrap/internal_listener/client_connection_factory_test.cc index 07c7da7958e06..4a1c850994a8c 100644 --- a/test/extensions/bootstrap/internal_listener/client_connection_factory_test.cc +++ b/test/extensions/bootstrap/internal_listener/client_connection_factory_test.cc @@ -90,7 +90,8 @@ TEST_F(ClientConnectionFactoryTest, tls_slot_.get(); auto client_conn = dispatcher_->createClientConnection( std::make_shared(listener_addr), - Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), nullptr); + Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), nullptr, + nullptr); EXPECT_NE(nullptr, client_conn); EXPECT_TRUE(client_conn->connecting()); client_conn->connect(); @@ -103,7 +104,8 @@ TEST_F(ClientConnectionFactoryTest, ConnectFailsIfInternalConnectionManagerNotEx setupTlsSlot(); auto client_conn = dispatcher_->createClientConnection( std::make_shared(listener_addr), - Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), nullptr); + Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), nullptr, + nullptr); EXPECT_NE(nullptr, client_conn); EXPECT_TRUE(client_conn->connecting()); client_conn->connect(); @@ -121,7 +123,8 @@ TEST_F(ClientConnectionFactoryTest, ConnectFailsIfInternalListenerNotExist) { auto client_conn = dispatcher_->createClientConnection( std::make_shared(listener_addr), - Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), nullptr); + Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), nullptr, + nullptr); EXPECT_NE(nullptr, client_conn); EXPECT_TRUE(client_conn->connecting()); @@ -148,7 +151,8 @@ TEST_F(ClientConnectionFactoryTest, ConnectSucceeds) { auto client_conn = dispatcher_->createClientConnection( std::make_shared(listener_addr), - Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), nullptr); + Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), nullptr, + nullptr); EXPECT_NE(nullptr, server_socket); diff --git a/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc b/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc index e44ddab6a6801..d9fc63860ba0c 100644 --- a/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc +++ b/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc @@ -59,7 +59,8 @@ class ProxyProtocolRegressionTest : public testing::TestWithParamaddListener(absl::nullopt, *this, runtime_); conn_ = dispatcher_->createClientConnection(socket_->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, + nullptr); conn_->addConnectionCallbacks(connection_callbacks_); } diff --git a/test/extensions/filters/listener/common/fuzz/listener_filter_fuzzer.cc b/test/extensions/filters/listener/common/fuzz/listener_filter_fuzzer.cc index 73e2e8aa91fe9..ce28253506102 100644 --- a/test/extensions/filters/listener/common/fuzz/listener_filter_fuzzer.cc +++ b/test/extensions/filters/listener/common/fuzz/listener_filter_fuzzer.cc @@ -46,9 +46,9 @@ ListenerFilterWithDataFuzzer::ListenerFilterWithDataFuzzer() getListenSocket(_)) .WillOnce(Return(socket_)); connection_handler_->addListener(absl::nullopt, *this, runtime_); - conn_ = dispatcher_->createClientConnection(socket_->connectionInfoProvider().localAddress(), - Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + conn_ = dispatcher_->createClientConnection( + socket_->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), + Network::Test::createRawBufferSocket(), nullptr, nullptr); conn_->addConnectionCallbacks(connection_callbacks_); } diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 0aba05e58a98e..16754daebcfe4 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -76,7 +76,8 @@ class ProxyProtocolTest : public testing::TestWithParamaddListener(absl::nullopt, *this, runtime_); conn_ = dispatcher_->createClientConnection(socket_->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, + nullptr); conn_->addConnectionCallbacks(connection_callbacks_); } @@ -1767,9 +1768,9 @@ class WildcardProxyProtocolTest : public testing::TestWithParamaddListener(absl::nullopt, *this, runtime_); - conn_ = dispatcher_->createClientConnection(local_dst_address_, - Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + conn_ = dispatcher_->createClientConnection( + local_dst_address_, Network::Address::InstanceConstSharedPtr(), + Network::Test::createRawBufferSocket(), nullptr, nullptr); conn_->addConnectionCallbacks(connection_callbacks_); EXPECT_CALL(factory_, createListenerFilterChain(_)) diff --git a/test/extensions/filters/listener/tls_inspector/tls_inspector_integration_test.cc b/test/extensions/filters/listener/tls_inspector/tls_inspector_integration_test.cc index 1efefc8314227..292e6f5f8bda5 100644 --- a/test/extensions/filters/listener/tls_inspector/tls_inspector_integration_test.cc +++ b/test/extensions/filters/listener/tls_inspector/tls_inspector_integration_test.cc @@ -116,8 +116,9 @@ class TlsInspectorIntegrationTest : public testing::TestWithParam(); transport_socket = transport_socket_factory->createTransportSocket(nullptr, nullptr); } - client_ = dispatcher_->createClientConnection( - address, Network::Address::InstanceConstSharedPtr(), std::move(transport_socket), nullptr); + client_ = + dispatcher_->createClientConnection(address, Network::Address::InstanceConstSharedPtr(), + std::move(transport_socket), nullptr, nullptr); client_->addConnectionCallbacks(connect_callbacks_); client_->connect(); while (!connect_callbacks_.connected() && !connect_callbacks_.closed()) { diff --git a/test/extensions/filters/network/sni_dynamic_forward_proxy/proxy_filter_integration_test.cc b/test/extensions/filters/network/sni_dynamic_forward_proxy/proxy_filter_integration_test.cc index 0b238a069b90b..343cd65bf3e55 100644 --- a/test/extensions/filters/network/sni_dynamic_forward_proxy/proxy_filter_integration_test.cc +++ b/test/extensions/filters/network/sni_dynamic_forward_proxy/proxy_filter_integration_test.cc @@ -100,7 +100,7 @@ name: envoy.clusters.dynamic_forward_proxy Ssl::createClientSslTransportSocketFactory(options, context_manager_, *api_); return dispatcher_->createClientConnection( address, Network::Address::InstanceConstSharedPtr(), - client_transport_socket_factory_ptr->createTransportSocket({}, nullptr), nullptr); + client_transport_socket_factory_ptr->createTransportSocket({}, nullptr), nullptr, nullptr); } std::string upstream_cert_name_{"server"}; diff --git a/test/extensions/transport_sockets/alts/alts_integration_test.cc b/test/extensions/transport_sockets/alts/alts_integration_test.cc index acb4b9b2d5dc1..570646820764f 100644 --- a/test/extensions/transport_sockets/alts/alts_integration_test.cc +++ b/test/extensions/transport_sockets/alts/alts_integration_test.cc @@ -196,7 +196,8 @@ class AltsIntegrationTestBase : public Event::TestUsingSimulatedTime, auto client_transport_socket = makeAltsTransportSocket(); Network::Address::InstanceConstSharedPtr address = getAddress(version_, lookupPort("http")); return dispatcher_->createClientConnection(address, Network::Address::InstanceConstSharedPtr(), - std::move(client_transport_socket), nullptr); + std::move(client_transport_socket), nullptr, + nullptr); } std::string fakeHandshakerServerAddress(bool connect_to_handshaker) { diff --git a/test/extensions/transport_sockets/tls/cert_validator/default_validator_integration_test.cc b/test/extensions/transport_sockets/tls/cert_validator/default_validator_integration_test.cc index b874b4d3c20f8..80dfbd018dbc0 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/default_validator_integration_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/default_validator_integration_test.cc @@ -38,7 +38,7 @@ SslCertValidatorIntegrationTest::makeSslClientConnection(const ClientSslTranspor createClientSslTransportSocketFactory(modified_options, *context_manager_, *api_); return dispatcher_->createClientConnection( address, Network::Address::InstanceConstSharedPtr(), - client_transport_socket_factory_ptr->createTransportSocket({}, nullptr), nullptr); + client_transport_socket_factory_ptr->createTransportSocket({}, nullptr), nullptr, nullptr); } INSTANTIATE_TEST_SUITE_P( diff --git a/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.cc b/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.cc index 0fe9cab9283e2..6b07060331a9a 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.cc @@ -43,7 +43,7 @@ Network::ClientConnectionPtr SslSPIFFECertValidatorIntegrationTest::makeSslClien createClientSslTransportSocketFactory(modified_options, *context_manager_, *api_); return dispatcher_->createClientConnection( address, Network::Address::InstanceConstSharedPtr(), - client_transport_socket_factory_ptr->createTransportSocket({}, nullptr), nullptr); + client_transport_socket_factory_ptr->createTransportSocket({}, nullptr), nullptr, nullptr); } void SslSPIFFECertValidatorIntegrationTest::checkVerifyErrorCouter(uint64_t value) { diff --git a/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc b/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc index 7551f5fbe9f75..26ef40a54478d 100644 --- a/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc +++ b/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc @@ -81,7 +81,7 @@ SslIntegrationTestBase::makeSslClientConnection(const ClientSslTransportOptions& createClientSslTransportSocketFactory(options, *context_manager_, *api_); return dispatcher_->createClientConnection( address, Network::Address::InstanceConstSharedPtr(), - client_transport_socket_factory_ptr->createTransportSocket({}, nullptr), nullptr); + client_transport_socket_factory_ptr->createTransportSocket({}, nullptr), nullptr, nullptr); } void SslIntegrationTestBase::checkStats() { diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 152d3381b714e..691ab02dccf9f 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -372,7 +372,7 @@ void testUtil(const TestUtilOptions& options) { client_stats_store); Network::ClientConnectionPtr client_connection = dispatcher->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - client_ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr); + client_ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr, nullptr); Network::ConnectionPtr server_connection; Network::MockConnectionCallbacks server_connection_callbacks; NiceMock stream_info; @@ -711,7 +711,7 @@ void testUtilV2(const TestUtilOptionsV2& options) { Network::ClientConnectionPtr client_connection = dispatcher->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), client_ssl_socket_factory.createTransportSocket(options.transportSocketOptions(), nullptr), - nullptr); + nullptr, nullptr); if (!options.clientSession().empty()) { const SslHandshakerImpl* ssl_socket = @@ -2611,7 +2611,7 @@ TEST_P(SslSocketTest, FlushCloseDuringHandshake) { Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); client_connection->connect(); Network::MockConnectionCallbacks client_connection_callbacks; client_connection->addConnectionCallbacks(client_connection_callbacks); @@ -2679,7 +2679,7 @@ TEST_P(SslSocketTest, HalfClose) { client_stats_store); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - client_ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr); + client_ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr, nullptr); client_connection->enableHalfClose(true); client_connection->addReadFilter(client_read_filter); client_connection->connect(); @@ -2761,7 +2761,7 @@ TEST_P(SslSocketTest, ShutdownWithCloseNotify) { client_stats_store); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - client_ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr); + client_ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr, nullptr); Network::MockConnectionCallbacks client_connection_callbacks; client_connection->enableHalfClose(true); client_connection->addReadFilter(client_read_filter); @@ -2849,7 +2849,7 @@ TEST_P(SslSocketTest, ShutdownWithoutCloseNotify) { client_stats_store); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - client_ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr); + client_ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr, nullptr); Network::MockConnectionCallbacks client_connection_callbacks; client_connection->enableHalfClose(true); client_connection->addReadFilter(client_read_filter); @@ -2955,7 +2955,7 @@ TEST_P(SslSocketTest, ClientAuthMultipleCAs) { ClientSslSocketFactory ssl_socket_factory(std::move(client_cfg), manager, client_stats_store); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr); + ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr, nullptr); // Verify that server sent list with 2 acceptable client certificate CA names. const SslHandshakerImpl* ssl_socket = @@ -3054,7 +3054,7 @@ void testTicketSessionResumption(const std::string& server_ctx_yaml1, ClientSslSocketFactory ssl_socket_factory(std::move(client_cfg), manager, client_stats_store); Network::ClientConnectionPtr client_connection = dispatcher->createClientConnection( socket1->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr); + ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr, nullptr); Network::MockConnectionCallbacks client_connection_callbacks; client_connection->addConnectionCallbacks(client_connection_callbacks); @@ -3097,7 +3097,7 @@ void testTicketSessionResumption(const std::string& server_ctx_yaml1, client_connection = dispatcher->createClientConnection( socket2->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr); + ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr, nullptr); client_connection->addConnectionCallbacks(client_connection_callbacks); const SslHandshakerImpl* ssl_socket = dynamic_cast(client_connection->ssl().get()); @@ -3195,7 +3195,7 @@ void testSupportForStatelessSessionResumption(const std::string& server_ctx_yaml Network::ClientConnectionPtr client_connection = dispatcher->createClientConnection( tcp_socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr); + ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr, nullptr); Network::MockConnectionCallbacks client_connection_callbacks; client_connection->addConnectionCallbacks(client_connection_callbacks); @@ -3799,7 +3799,7 @@ TEST_P(SslSocketTest, ClientAuthCrossListenerSessionResumption) { ClientSslSocketFactory ssl_socket_factory(std::move(client_cfg), manager, client_stats_store); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr); + ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr, nullptr); Network::MockConnectionCallbacks client_connection_callbacks; client_connection->addConnectionCallbacks(client_connection_callbacks); @@ -3841,7 +3841,7 @@ TEST_P(SslSocketTest, ClientAuthCrossListenerSessionResumption) { client_connection = dispatcher_->createClientConnection( socket2->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr); + ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr, nullptr); client_connection->addConnectionCallbacks(client_connection_callbacks); const SslHandshakerImpl* ssl_socket = dynamic_cast(client_connection->ssl().get()); @@ -3919,7 +3919,7 @@ void SslSocketTest::testClientSessionResumption(const std::string& server_ctx_ya client_stats_store); Network::ClientConnectionPtr client_connection = dispatcher->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - client_ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr); + client_ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr, nullptr); Network::MockConnectionCallbacks client_connection_callbacks; client_connection->addConnectionCallbacks(client_connection_callbacks); @@ -3981,7 +3981,7 @@ void SslSocketTest::testClientSessionResumption(const std::string& server_ctx_ya client_connection = dispatcher->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - client_ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr); + client_ssl_socket_factory.createTransportSocket(nullptr, nullptr), nullptr, nullptr); client_connection->addConnectionCallbacks(client_connection_callbacks); client_connection->connect(); @@ -4163,7 +4163,7 @@ TEST_P(SslSocketTest, SslError) { Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, nullptr); client_connection->connect(); Buffer::OwnedImpl bad_data("bad_handshake_data"); client_connection->write(bad_data, false); @@ -5263,9 +5263,9 @@ class SslReadBufferLimitTest : public SslSocketTest { std::move(client_cfg), *manager_, client_stats_store_); auto transport_socket = client_ssl_socket_factory_->createTransportSocket(nullptr, nullptr); client_transport_socket_ = transport_socket.get(); - client_connection_ = - dispatcher_->createClientConnection(socket_->connectionInfoProvider().localAddress(), - source_address_, std::move(transport_socket), nullptr); + client_connection_ = dispatcher_->createClientConnection( + socket_->connectionInfoProvider().localAddress(), source_address_, + std::move(transport_socket), nullptr, nullptr); client_connection_->addConnectionCallbacks(client_callbacks_); client_connection_->connect(); read_filter_ = std::make_shared(); diff --git a/test/integration/base_integration_test.cc b/test/integration/base_integration_test.cc index 2534b6dfedf91..f794e8af5d84c 100644 --- a/test/integration/base_integration_test.cc +++ b/test/integration/base_integration_test.cc @@ -89,7 +89,8 @@ Network::ClientConnectionPtr BaseIntegrationTest::makeClientConnectionWithOption Network::ClientConnectionPtr connection(dispatcher_->createClientConnection( Network::Utility::resolveUrl( fmt::format("tcp://{}:{}", Network::Test::getLoopbackAddressUrlString(version_), port)), - Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), options)); + Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), options, + nullptr)); connection->enableHalfClose(enableHalfClose()); return connection; diff --git a/test/integration/integration_tcp_client.cc b/test/integration/integration_tcp_client.cc index 0e145ae12c838..4f2ce7831947f 100644 --- a/test/integration/integration_tcp_client.cc +++ b/test/integration/integration_tcp_client.cc @@ -57,7 +57,7 @@ IntegrationTcpClient::IntegrationTcpClient( connection_ = dispatcher.createClientConnection( Network::Utility::resolveUrl( fmt::format("tcp://{}:{}", Network::Test::getLoopbackAddressUrlString(version), port)), - source_address, Network::Test::createRawBufferSocket(), options); + source_address, Network::Test::createRawBufferSocket(), options, nullptr); ON_CALL(*client_write_buffer_, drain(_)) .WillByDefault(Invoke(client_write_buffer_, &MockWatermarkBuffer::trackDrains)); diff --git a/test/integration/utility.cc b/test/integration/utility.cc index b7cf9b5d60f59..faf211a082578 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -200,11 +200,11 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt time_system)}; if (type <= Http::CodecType::HTTP2) { - Http::CodecClientProd client( - type, - dispatcher->createClientConnection(addr, Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr), - host_description, *dispatcher, random); + Http::CodecClientProd client(type, + dispatcher->createClientConnection( + addr, Network::Address::InstanceConstSharedPtr(), + Network::Test::createRawBufferSocket(), nullptr, nullptr), + host_description, *dispatcher, random); return sendRequestAndWaitForResponse(*dispatcher, method, url, body, host, content_type, client); } @@ -283,7 +283,7 @@ RawConnectionDriver::RawConnectionDriver(uint32_t port, DoWriteCallback write_re client_ = dispatcher_.createClientConnection( Network::Utility::resolveUrl( fmt::format("tcp://{}:{}", Network::Test::getLoopbackAddressUrlString(version), port)), - Network::Address::InstanceConstSharedPtr(), std::move(transport_socket), nullptr); + Network::Address::InstanceConstSharedPtr(), std::move(transport_socket), nullptr, nullptr); // ConnectionCallbacks will call write_request_callback from the connect and low-watermark // callbacks. Set a small buffer limit so high-watermark is triggered after every write and // low-watermark is triggered every time the buffer is drained. diff --git a/test/mocks/event/mocks.h b/test/mocks/event/mocks.h index 80cee9064a5c0..2ef2063026570 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -56,7 +56,7 @@ class MockDispatcher : public Dispatcher { Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - const Network::TransportSocketOptionsConstSharedPtr& = nullptr) override { + const Network::TransportSocketOptionsConstSharedPtr&) override { return Network::ClientConnectionPtr{ createClientConnection_(address, source_address, transport_socket, options)}; } diff --git a/test/mocks/event/wrapped_dispatcher.h b/test/mocks/event/wrapped_dispatcher.h index 66b22b0994fa3..03e403fe5f69a 100644 --- a/test/mocks/event/wrapped_dispatcher.h +++ b/test/mocks/event/wrapped_dispatcher.h @@ -46,7 +46,7 @@ class WrappedDispatcher : public Dispatcher { Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options, - const Network::TransportSocketOptionsConstSharedPtr& transport_options = nullptr) override { + const Network::TransportSocketOptionsConstSharedPtr& transport_options) override { return impl_.createClientConnection(std::move(address), std::move(source_address), std::move(transport_socket), options, transport_options); } diff --git a/test/server/config_validation/dispatcher_test.cc b/test/server/config_validation/dispatcher_test.cc index 58bbea8c0ee0f..8058b915f7c52 100644 --- a/test/server/config_validation/dispatcher_test.cc +++ b/test/server/config_validation/dispatcher_test.cc @@ -47,7 +47,7 @@ TEST_P(ConfigValidation, CreateConnection) { Network::Address::InstanceConstSharedPtr address( Network::Test::getCanonicalLoopbackAddress(GetParam())); dispatcher_->createClientConnection(address, address, Network::Test::createRawBufferSocket(), - nullptr); + nullptr, nullptr); SUCCEED(); } From 7ff007019302ac66c71807dc92dfca4238a067f9 Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Tue, 28 Jun 2022 22:21:39 -0700 Subject: [PATCH 09/12] fix tests Signed-off-by: Kuat Yessenov --- test/integration/overload_integration_test.cc | 2 +- test/integration/sds_dynamic_integration_test.cc | 2 +- test/integration/sds_static_integration_test.cc | 2 +- test/integration/socket_interface_integration_test.cc | 7 ++++--- test/integration/tcp_proxy_integration_test.cc | 2 +- test/integration/uds_integration_test.cc | 2 +- test/integration/xds_integration_test.cc | 2 +- test/integration/xfcc_integration_test.cc | 5 +++-- 8 files changed, 13 insertions(+), 11 deletions(-) diff --git a/test/integration/overload_integration_test.cc b/test/integration/overload_integration_test.cc index 467fa4ad3660b..c6dcd173f40af 100644 --- a/test/integration/overload_integration_test.cc +++ b/test/integration/overload_integration_test.cc @@ -337,7 +337,7 @@ TEST_P(OverloadScaledTimerIntegrationTest, TlsHandshakeTimeout) { Ssl::getSslAddress(version_, lookupPort("http")); auto bad_ssl_client = dispatcher_->createClientConnection(address, Network::Address::InstanceConstSharedPtr(), - std::move(bad_transport_socket), nullptr); + std::move(bad_transport_socket), nullptr, nullptr); bad_ssl_client->addConnectionCallbacks(connect_callbacks); bad_ssl_client->enableHalfClose(true); bad_ssl_client->connect(); diff --git a/test/integration/sds_dynamic_integration_test.cc b/test/integration/sds_dynamic_integration_test.cc index cda67a208358a..452befab80b50 100644 --- a/test/integration/sds_dynamic_integration_test.cc +++ b/test/integration/sds_dynamic_integration_test.cc @@ -309,7 +309,7 @@ version_info: "0" Network::Address::InstanceConstSharedPtr address = getSslAddress(version_, port); return dispatcher_->createClientConnection( address, Network::Address::InstanceConstSharedPtr(), - client_ssl_ctx_->createTransportSocket(nullptr, nullptr), nullptr); + client_ssl_ctx_->createTransportSocket(nullptr, nullptr), nullptr, nullptr); } return makeClientConnectionWithOptions(port, nullptr); } diff --git a/test/integration/sds_static_integration_test.cc b/test/integration/sds_static_integration_test.cc index 9de5d0dc0aef1..e179f00a25e15 100644 --- a/test/integration/sds_static_integration_test.cc +++ b/test/integration/sds_static_integration_test.cc @@ -85,7 +85,7 @@ class SdsStaticDownstreamIntegrationTest Network::Address::InstanceConstSharedPtr address = getSslAddress(version_, lookupPort("http")); return dispatcher_->createClientConnection( address, Network::Address::InstanceConstSharedPtr(), - client_ssl_ctx_->createTransportSocket(nullptr, nullptr), nullptr); + client_ssl_ctx_->createTransportSocket(nullptr, nullptr), nullptr, nullptr); } private: diff --git a/test/integration/socket_interface_integration_test.cc b/test/integration/socket_interface_integration_test.cc index dc6d0b186bb0d..1379aacec886a 100644 --- a/test/integration/socket_interface_integration_test.cc +++ b/test/integration/socket_interface_integration_test.cc @@ -77,8 +77,9 @@ TEST_P(SocketInterfaceIntegrationTest, AddressWithSocketInterface) { Network::Test::getLoopbackAddressUrlString(Network::Address::IpVersion::v4), lookupPort("listener_0"), sock_interface); - client_ = dispatcher_->createClientConnection(address, Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + client_ = + dispatcher_->createClientConnection(address, Network::Address::InstanceConstSharedPtr(), + Network::Test::createRawBufferSocket(), nullptr, nullptr); client_->addConnectionCallbacks(connect_callbacks_); client_->connect(); @@ -103,7 +104,7 @@ TEST_P(SocketInterfaceIntegrationTest, InternalAddressWithSocketInterface) { ASSERT_DEATH(client_ = dispatcher_->createClientConnection( address, Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr), + Network::Test::createRawBufferSocket(), nullptr, nullptr), "" /* Nullptr dereference */); } diff --git a/test/integration/tcp_proxy_integration_test.cc b/test/integration/tcp_proxy_integration_test.cc index 446d8930100df..0f3f9459ee6a6 100644 --- a/test/integration/tcp_proxy_integration_test.cc +++ b/test/integration/tcp_proxy_integration_test.cc @@ -1316,7 +1316,7 @@ void TcpProxySslIntegrationTest::setupConnections() { context_ = Ssl::createClientSslTransportSocketFactory({}, *context_manager_, *api_); ssl_client_ = dispatcher_->createClientConnection( address, Network::Address::InstanceConstSharedPtr(), - context_->createTransportSocket(nullptr, nullptr), nullptr); + context_->createTransportSocket(nullptr, nullptr), nullptr, nullptr); // Perform the SSL handshake. Loopback is allowlisted in tcp_proxy.json for the ssl_auth // filter so there will be no pause waiting on auth data. diff --git a/test/integration/uds_integration_test.cc b/test/integration/uds_integration_test.cc index 3fb1ecfc70e11..8300371122621 100644 --- a/test/integration/uds_integration_test.cc +++ b/test/integration/uds_integration_test.cc @@ -85,7 +85,7 @@ HttpIntegrationTest::ConnectionCreationFunction UdsListenerIntegrationTest::crea return [&]() -> Network::ClientConnectionPtr { Network::ClientConnectionPtr conn(dispatcher_->createClientConnection( Network::Utility::resolveUrl(fmt::format("unix://{}", getListenerSocketName())), - Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), + Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), nullptr, nullptr)); conn->enableHalfClose(enableHalfClose()); return conn; diff --git a/test/integration/xds_integration_test.cc b/test/integration/xds_integration_test.cc index 901c56c77d3ce..b571a70851be2 100644 --- a/test/integration/xds_integration_test.cc +++ b/test/integration/xds_integration_test.cc @@ -472,7 +472,7 @@ class LdsInplaceUpdateHttpIntegrationTest std::make_shared( absl::string_view(""), std::vector(), std::vector{alpn}), nullptr), - nullptr); + nullptr, nullptr); return makeHttpConnection(std::move(ssl_conn)); } diff --git a/test/integration/xfcc_integration_test.cc b/test/integration/xfcc_integration_test.cc index 737c7e429f8cb..a6f53db7fa536 100644 --- a/test/integration/xfcc_integration_test.cc +++ b/test/integration/xfcc_integration_test.cc @@ -123,7 +123,8 @@ Network::ClientConnectionPtr XfccIntegrationTest::makeTcpClientConnection() { Network::Utility::resolveUrl("tcp://" + Network::Test::getLoopbackAddressUrlString(version_) + ":" + std::to_string(lookupPort("http"))); return dispatcher_->createClientConnection(address, Network::Address::InstanceConstSharedPtr(), - Network::Test::createRawBufferSocket(), nullptr); + Network::Test::createRawBufferSocket(), nullptr, + nullptr); } Network::ClientConnectionPtr XfccIntegrationTest::makeMtlsClientConnection() { @@ -132,7 +133,7 @@ Network::ClientConnectionPtr XfccIntegrationTest::makeMtlsClientConnection() { ":" + std::to_string(lookupPort("http"))); return dispatcher_->createClientConnection( address, Network::Address::InstanceConstSharedPtr(), - client_mtls_ssl_ctx_->createTransportSocket(nullptr, nullptr), nullptr); + client_mtls_ssl_ctx_->createTransportSocket(nullptr, nullptr), nullptr, nullptr); } void XfccIntegrationTest::createUpstreams() { From 1948a0a4b6c225102d77e0efc211f9a9164dc52b Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Wed, 29 Jun 2022 13:33:45 -0700 Subject: [PATCH 10/12] review Signed-off-by: Kuat Yessenov --- changelogs/current.yaml | 2 +- envoy/network/transport_socket.h | 2 +- envoy/stream_info/filter_state.h | 17 ++++++++++------- source/common/network/connection_impl.cc | 2 +- .../network/transport_socket_options_impl.cc | 4 ++-- .../network/transport_socket_options_impl.h | 6 +++--- .../io_socket/user_space/io_handle_impl.cc | 9 +++------ .../internal_upstream/config.cc | 2 +- .../transport_socket_options_impl_test.cc | 2 +- .../io_socket/user_space/io_handle_impl_test.cc | 17 ----------------- 10 files changed, 23 insertions(+), 40 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index bafe6af908333..d03de66849571 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -119,7 +119,7 @@ minor_behavior_changes: - area: filter state change: | revert to respecting the life time of the filter state objects to be bound to the original stream and make sharing - filter state objects with the upstream explicit via a flag. + filter state objects with the upstream info explicit via an extra flag in ``setData``. bug_fixes: - area: http diff --git a/envoy/network/transport_socket.h b/envoy/network/transport_socket.h index a198a6c8f5eb0..d471e7eee88b2 100644 --- a/envoy/network/transport_socket.h +++ b/envoy/network/transport_socket.h @@ -238,7 +238,7 @@ class TransportSocketOptions { * @return filter state objects from the downstream request or connection * that are marked as shared with the upstream connection. */ - virtual const StreamInfo::FilterState::Objects& filterStateObjects() const PURE; + virtual const StreamInfo::FilterState::Objects& downstreamSharedFilterStateObjects() const PURE; }; using TransportSocketOptionsConstSharedPtr = std::shared_ptr; diff --git a/envoy/stream_info/filter_state.h b/envoy/stream_info/filter_state.h index 8ca72768b988f..17ef69161e1a7 100644 --- a/envoy/stream_info/filter_state.h +++ b/envoy/stream_info/filter_state.h @@ -52,10 +52,11 @@ class FilterState { // state. // // As a special case, objects that are marked as shared with the upstream - // become bound to the upstream connection life span, regardless of the - // original life span. That means, for example, that a shared request span - // object may outlive the original request when it is shared, because it may - // be captured by an upstream connection for the original downstream request. + // become bound to the upstream connection life span in addition to the + // original stream life span. That means, for example, that a shared request + // span object may outlive the original request when it is shared, because it + // may be captured by an upstream connection for the original downstream + // request, which remains open after the downstream request completes. enum LifeSpan { FilterChain, Request, Connection, TopSpan = Connection }; // Objects stored in the filter state can optionally be shared between the @@ -69,8 +70,10 @@ class FilterState { // requests and connections to the upstream connection filter state. When // upstream connections are re-used between streams, the downstream objects // are captured for the first, initiating stream. To force distinct - // upstream connections and prevent sharing, the shared filter state object - // must implement the hashing interface. + // upstream connections, the shared filter state object must implement the + // hashing interface. Shared objects with distinct hashes will use distinct + // upstream connections. Note that this affects connection pooling, + // preventing any re-use of the upstream connections in the worst case. SharedWithUpstreamConnection, }; @@ -97,7 +100,7 @@ class FilterState { std::shared_ptr data_; StateType state_type_; StreamSharing stream_sharing_{StreamSharing::None}; - std::string name_{""}; + std::string name_; }; using Objects = std::vector; diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc index 45cd17451fa6f..796fa59922f14 100644 --- a/source/common/network/connection_impl.cc +++ b/source/common/network/connection_impl.cc @@ -910,7 +910,7 @@ ClientConnectionImpl::ClientConnectionImpl( } if (transport_options) { - for (const auto& object : transport_options->filterStateObjects()) { + for (const auto& object : transport_options->downstreamSharedFilterStateObjects()) { // This does not throw as all objects are distinctly named and the stream info is empty. stream_info_.filterState()->setData(object.name_, object.data_, object.state_type_, StreamInfo::FilterState::LifeSpan::Connection, diff --git a/source/common/network/transport_socket_options_impl.cc b/source/common/network/transport_socket_options_impl.cc index 75a962aae361a..7494093ecfa17 100644 --- a/source/common/network/transport_socket_options_impl.cc +++ b/source/common/network/transport_socket_options_impl.cc @@ -43,7 +43,7 @@ void CommonUpstreamTransportSocketFactory::hashKey( pushScalarToByteVector(StringUtil::CaseInsensitiveHash()(protocol), key); } - for (const auto& object : options->filterStateObjects()) { + for (const auto& object : options->downstreamSharedFilterStateObjects()) { if (auto hashable = dynamic_cast(object.data_.get()); hashable != nullptr) { if (auto hash = hashable->hash(); hash) { pushScalarToByteVector(hash.value(), key); @@ -89,7 +89,7 @@ TransportSocketOptionsUtility::fromFilterState(const StreamInfo::FilterState& fi } StreamInfo::FilterState::ObjectsPtr objects = filter_state.objectsSharedWithUpstreamConnection(); - if (objects->size() > 0) { + if (!objects->empty()) { needs_transport_socket_options = true; } diff --git a/source/common/network/transport_socket_options_impl.h b/source/common/network/transport_socket_options_impl.h index 1b84e2c3e059a..fb5ff4f2bac73 100644 --- a/source/common/network/transport_socket_options_impl.h +++ b/source/common/network/transport_socket_options_impl.h @@ -29,8 +29,8 @@ class AlpnDecoratingTransportSocketOptions : public TransportSocketOptions { absl::optional proxyProtocolOptions() const override { return inner_options_->proxyProtocolOptions(); } - const StreamInfo::FilterState::Objects& filterStateObjects() const override { - return inner_options_->filterStateObjects(); + const StreamInfo::FilterState::Objects& downstreamSharedFilterStateObjects() const override { + return inner_options_->downstreamSharedFilterStateObjects(); } private: @@ -71,7 +71,7 @@ class TransportSocketOptionsImpl : public TransportSocketOptions { absl::optional proxyProtocolOptions() const override { return proxy_protocol_options_; } - const StreamInfo::FilterState::Objects& filterStateObjects() const override { + const StreamInfo::FilterState::Objects& downstreamSharedFilterStateObjects() const override { return *filter_state_objects_; } diff --git a/source/extensions/io_socket/user_space/io_handle_impl.cc b/source/extensions/io_socket/user_space/io_handle_impl.cc index 4687cfd633f78..41dd4901fc853 100644 --- a/source/extensions/io_socket/user_space/io_handle_impl.cc +++ b/source/extensions/io_socket/user_space/io_handle_impl.cc @@ -393,12 +393,9 @@ void PassthroughStateImpl::mergeInto(envoy::config::core::v3::Metadata& metadata metadata.MergeFrom(*metadata_); } for (const auto& object : filter_state_objects_) { - try { - filter_state.setData(object.name_, object.data_, object.state_type_, - StreamInfo::FilterState::LifeSpan::Connection, object.stream_sharing_); - } catch (const EnvoyException& e) { - ENVOY_LOG(trace, "Failed to set data for '{}': {}", object.name_, e.what()); - } + // This should not throw as stream info is new and filter objects are uniquely named. + filter_state.setData(object.name_, object.data_, object.state_type_, + StreamInfo::FilterState::LifeSpan::Connection, object.stream_sharing_); } metadata_ = nullptr; filter_state_objects_.clear(); diff --git a/source/extensions/transport_sockets/internal_upstream/config.cc b/source/extensions/transport_sockets/internal_upstream/config.cc index e7793f646d62f..2993e34d28154 100644 --- a/source/extensions/transport_sockets/internal_upstream/config.cc +++ b/source/extensions/transport_sockets/internal_upstream/config.cc @@ -118,7 +118,7 @@ InternalSocketFactory::createTransportSocket(Network::TransportSocketOptionsCons extracted_metadata = config_.extractMetadata(host); } return std::make_unique(std::move(inner_socket), std::move(extracted_metadata), - options ? options->filterStateObjects() + options ? options->downstreamSharedFilterStateObjects() : StreamInfo::FilterState::Objects()); } diff --git a/test/common/network/transport_socket_options_impl_test.cc b/test/common/network/transport_socket_options_impl_test.cc index 347d6d18c582c..3688d6a9053b9 100644 --- a/test/common/network/transport_socket_options_impl_test.cc +++ b/test/common/network/transport_socket_options_impl_test.cc @@ -37,7 +37,7 @@ TEST_F(TransportSocketOptionsImplTest, SharedFilterState) { StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::FilterChain, StreamInfo::FilterState::StreamSharing::SharedWithUpstreamConnection); auto transport_socket_options = TransportSocketOptionsUtility::fromFilterState(filter_state_); - auto objects = transport_socket_options->filterStateObjects(); + auto objects = transport_socket_options->downstreamSharedFilterStateObjects(); EXPECT_EQ(1, objects.size()); EXPECT_EQ("random_key_has_effect", objects.at(0).name_); } diff --git a/test/extensions/io_socket/user_space/io_handle_impl_test.cc b/test/extensions/io_socket/user_space/io_handle_impl_test.cc index a083f7c492ab7..d393ecac78064 100644 --- a/test/extensions/io_socket/user_space/io_handle_impl_test.cc +++ b/test/extensions/io_socket/user_space/io_handle_impl_test.cc @@ -1150,23 +1150,6 @@ TEST_F(IoHandleImplTest, PassthroughState) { ASSERT_EQ(object->value_, dest_object->value_); } -TEST_F(IoHandleImplTest, PassthroughStateReadOnlyObject) { - StreamInfo::FilterState::Objects source_filter_state; - auto object = std::make_shared(1000); - source_filter_state.push_back( - {object, StreamInfo::FilterState::StateType::ReadOnly, - StreamInfo::FilterState::StreamSharing::SharedWithUpstreamConnection, "object_key"}); - io_handle_->passthroughState()->initialize(nullptr, source_filter_state); - StreamInfo::FilterStateImpl dest_filter_state(StreamInfo::FilterState::LifeSpan::Connection); - auto read_only = std::make_shared(1); - dest_filter_state.setData("object_key", read_only, StreamInfo::FilterState::StateType::ReadOnly, - StreamInfo::FilterState::LifeSpan::Connection); - envoy::config::core::v3::Metadata dest_metadata; - io_handle_peer_->passthroughState()->mergeInto(dest_metadata, dest_filter_state); - auto dest_object = dest_filter_state.getDataReadOnly("object_key"); - ASSERT_EQ(read_only->value_, dest_object->value_); -} - class IoHandleImplNotImplementedTest : public testing::Test { public: IoHandleImplNotImplementedTest() { From a16242b350052d39a106607c73602496ff41dc6d Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Wed, 29 Jun 2022 15:54:07 -0700 Subject: [PATCH 11/12] default initialize Signed-off-by: Kuat Yessenov --- envoy/stream_info/filter_state.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/envoy/stream_info/filter_state.h b/envoy/stream_info/filter_state.h index 17ef69161e1a7..b5cfb9d32b7b7 100644 --- a/envoy/stream_info/filter_state.h +++ b/envoy/stream_info/filter_state.h @@ -98,7 +98,7 @@ class FilterState { struct FilterObject { std::shared_ptr data_; - StateType state_type_; + StateType state_type_{StateType::ReadOnly}; StreamSharing stream_sharing_{StreamSharing::None}; std::string name_; }; From 30250ba458aed4987f24c61b791b1333c32690aa Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Fri, 1 Jul 2022 14:40:08 -0700 Subject: [PATCH 12/12] merge fix Signed-off-by: Kuat Yessenov --- source/common/network/happy_eyeballs_connection_impl.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source/common/network/happy_eyeballs_connection_impl.cc b/source/common/network/happy_eyeballs_connection_impl.cc index 9af0d4dfbae4d..42ca576a62898 100644 --- a/source/common/network/happy_eyeballs_connection_impl.cc +++ b/source/common/network/happy_eyeballs_connection_impl.cc @@ -25,7 +25,8 @@ ClientConnectionPtr HappyEyeballsConnectionProvider::createNextConnection(const address_list_[next_address_]); return dispatcher_.createClientConnection( address_list_[next_address_++], source_address_, - socket_factory_.createTransportSocket(transport_socket_options_, host_), options_); + socket_factory_.createTransportSocket(transport_socket_options_, host_), options_, + transport_socket_options_); } size_t HappyEyeballsConnectionProvider::nextConnection() { return next_address_; }