diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 8d147d071a455..f385f6bc51499 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -38,6 +38,9 @@ behavior_changes: minor_behavior_changes: # *Changes that may cause incompatibilities for some users, but should not for most* +- area: logging + change: | + changed the ``UPSTREAM_REMOTE_ADDRESS``, ``UPSTREAM_REMOTE_ADDRESS_WITHOUT_PORT``, and ``UPSTREAM_REMOTE_PORT`` fields to log based on the actual upstream connection rather than the upstream host. This fixes a bug where the address components were not consistently correct for Happy Eyeballs connections and proxied connections, but also means in cases where the host was selected but a connection was not established, the fields will be absent. This change can be temporarily reverted by setting the runtime guard ``envoy.reloadable_features.correct_remote_address`` to false. - area: resource_monitors change: | changed behavior of the fixed heap monitor to count pages allocated to TCMalloc as free memory if it's not used by Envoy. This change can be reverted temporarily by setting the runtime guard ``envoy.reloadable_features.do_not_count_mapped_pages_as_free`` to true. @@ -59,6 +62,8 @@ minor_behavior_changes: - area: http change: | changed the filter callback interfaces to make sure that downstream-only functionality is explicit. + change: | + the upstream remote address is now available to downstream filters via the ``upstreamRemoteAddress`` function. - area: stats change: | Default tag extraction rules were changed for ``worker_id`` extraction. Previously, ``worker_`` was removed from the original name during the extraction. This diff --git a/envoy/http/codec.h b/envoy/http/codec.h index 2cfb9ef5a5616..6679d6dff95c6 100644 --- a/envoy/http/codec.h +++ b/envoy/http/codec.h @@ -341,10 +341,10 @@ class Stream : public StreamResetHandler { virtual absl::string_view responseDetails() { return ""; } /** - * @return const Address::InstanceConstSharedPtr& the local address of the connection associated - * with the stream. + * @return const Network::ConnectionInfoProvider& the adderess provider of the connection + * associated with the stream. */ - virtual const Network::Address::InstanceConstSharedPtr& connectionLocalAddress() PURE; + virtual const Network::ConnectionInfoProvider& connectionInfoProvider() PURE; /** * Set the flush timeout for the stream. At the codec level this is used to bound the amount of diff --git a/envoy/router/router.h b/envoy/router/router.h index c1b79c36c69aa..eb1f504c5db13 100644 --- a/envoy/router/router.h +++ b/envoy/router/router.h @@ -1400,13 +1400,13 @@ class GenericConnectionPoolCallbacks { * @param upstream supplies the generic upstream for the stream. * @param host supplies the description of the host that will carry the request. For logical * connection pools the description may be different each time this is called. - * @param upstream_local_address supplies the local address of the upstream connection. + * @param connection_info_provider, supplies the address provider of the upstream connection. * @param info supplies the stream info object associated with the upstream connection. * @param protocol supplies the protocol associated with the upstream connection. */ virtual void onPoolReady(std::unique_ptr&& upstream, Upstream::HostDescriptionConstSharedPtr host, - const Network::Address::InstanceConstSharedPtr& upstream_local_address, + const Network::ConnectionInfoProvider& connection_info_provider, StreamInfo::StreamInfo& info, absl::optional protocol) PURE; diff --git a/envoy/stream_info/stream_info.h b/envoy/stream_info/stream_info.h index 69f10e6a90d38..3bec7cc93fc28 100644 --- a/envoy/stream_info/stream_info.h +++ b/envoy/stream_info/stream_info.h @@ -402,6 +402,17 @@ class UpstreamInfo { */ virtual const Network::Address::InstanceConstSharedPtr& upstreamLocalAddress() const PURE; + /** + * @param upstream_remote_address sets the remote address of the upstream connection. + */ + virtual void setUpstreamRemoteAddress( + const Network::Address::InstanceConstSharedPtr& upstream_remote_address) PURE; + + /** + * @return the upstream remote address. + */ + virtual const Network::Address::InstanceConstSharedPtr& upstreamRemoteAddress() const PURE; + /** * @param failure_reason the upstream transport failure reason. */ diff --git a/envoy/tcp/upstream.h b/envoy/tcp/upstream.h index be6170c8ae7d7..8c44bdb852400 100644 --- a/envoy/tcp/upstream.h +++ b/envoy/tcp/upstream.h @@ -67,14 +67,14 @@ class GenericConnectionPoolCallbacks { * @param info supplies the stream info object associated with the upstream connection. * @param upstream supplies the generic upstream for the stream. * @param host supplies the description of the host that will carry the request. - * @param upstream_local_address supplies the local address of the upstream connection. + * @param address_provider supplies the address provider of the upstream connection. * @param ssl_info supplies the ssl information of the upstream connection. */ - virtual void - onGenericPoolReady(StreamInfo::StreamInfo* info, std::unique_ptr&& upstream, - Upstream::HostDescriptionConstSharedPtr& host, - const Network::Address::InstanceConstSharedPtr& upstream_local_address, - Ssl::ConnectionInfoConstSharedPtr ssl_info) PURE; + virtual void onGenericPoolReady(StreamInfo::StreamInfo* info, + std::unique_ptr&& upstream, + Upstream::HostDescriptionConstSharedPtr& host, + const Network::ConnectionInfoProvider& address_provider, + Ssl::ConnectionInfoConstSharedPtr ssl_info) PURE; /** * Called to indicate a failure for GenericConnPool::newStream to establish a stream. diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index 1fb8cf36740c6..69fd5877bd32a 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/source/common/http/http1/codec_impl.cc @@ -375,8 +375,8 @@ void StreamEncoderImpl::readDisable(bool disable) { uint32_t StreamEncoderImpl::bufferLimit() const { return connection_.bufferLimit(); } -const Network::Address::InstanceConstSharedPtr& StreamEncoderImpl::connectionLocalAddress() { - return connection_.connection().connectionInfoProvider().localAddress(); +const Network::ConnectionInfoProvider& StreamEncoderImpl::connectionInfoProvider() { + return connection_.connection().connectionInfoProvider(); } static constexpr absl::string_view RESPONSE_PREFIX = "HTTP/1.1 "; diff --git a/source/common/http/http1/codec_impl.h b/source/common/http/http1/codec_impl.h index 439351c75c6da..60d6c98f11d15 100644 --- a/source/common/http/http1/codec_impl.h +++ b/source/common/http/http1/codec_impl.h @@ -62,7 +62,7 @@ class StreamEncoderImpl : public virtual StreamEncoder, void readDisable(bool disable) override; uint32_t bufferLimit() const override; absl::string_view responseDetails() override { return details_; } - const Network::Address::InstanceConstSharedPtr& connectionLocalAddress() override; + const Network::ConnectionInfoProvider& connectionInfoProvider() override; void setFlushTimeout(std::chrono::milliseconds) override { // HTTP/1 has one stream per connection, thus any data encoded is immediately written to the // connection, invoking any watermarks as necessary. There is no internal buffering that would diff --git a/source/common/http/http2/codec_impl.h b/source/common/http/http2/codec_impl.h index 754e375c9177c..929a6cf9ff283 100644 --- a/source/common/http/http2/codec_impl.h +++ b/source/common/http/http2/codec_impl.h @@ -266,8 +266,8 @@ class ConnectionImpl : public virtual Connection, void resetStream(StreamResetReason reason) override; void readDisable(bool disable) override; uint32_t bufferLimit() const override { return pending_recv_data_->highWatermark(); } - const Network::Address::InstanceConstSharedPtr& connectionLocalAddress() override { - return parent_.connection_.connectionInfoProvider().localAddress(); + const Network::ConnectionInfoProvider& connectionInfoProvider() override { + return parent_.connection_.connectionInfoProvider(); } absl::string_view responseDetails() override { return details_; } void setAccount(Buffer::BufferMemoryAccountSharedPtr account) override; diff --git a/source/common/quic/envoy_quic_stream.h b/source/common/quic/envoy_quic_stream.h index 95a6a905ad5ff..7474e2c35df81 100644 --- a/source/common/quic/envoy_quic_stream.h +++ b/source/common/quic/envoy_quic_stream.h @@ -78,8 +78,8 @@ class EnvoyQuicStream : public virtual Http::StreamEncoder, removeCallbacksHelper(callbacks); } uint32_t bufferLimit() const override { return send_buffer_simulation_.highWatermark(); } - const Network::Address::InstanceConstSharedPtr& connectionLocalAddress() override { - return connection()->connectionInfoProvider().localAddress(); + const Network::ConnectionInfoProvider& connectionInfoProvider() override { + return connection()->connectionInfoProvider(); } void setAccount(Buffer::BufferMemoryAccountSharedPtr account) override { diff --git a/source/common/router/header_formatter.cc b/source/common/router/header_formatter.cc index 67e1bc215aa44..9a198038150d2 100644 --- a/source/common/router/header_formatter.cc +++ b/source/common/router/header_formatter.cc @@ -354,6 +354,13 @@ StreamInfoHeaderFormatter::StreamInfoHeaderFormatter(absl::string_view field_nam field_extractor_ = parseSubstitutionFormatField(field_name, formatter_map_); } else if (field_name == "UPSTREAM_LOCAL_ADDRESS") { field_extractor_ = [](const Envoy::StreamInfo::StreamInfo& stream_info) -> std::string { + if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.correct_remote_address")) { + if (stream_info.upstreamInfo().has_value() && + stream_info.upstreamInfo()->upstreamHost()->address()) { + return stream_info.upstreamInfo()->upstreamHost()->address()->asString(); + } + return ""; + } if (stream_info.upstreamInfo().has_value() && stream_info.upstreamInfo()->upstreamLocalAddress()) { return stream_info.upstreamInfo()->upstreamLocalAddress()->asString(); @@ -362,6 +369,14 @@ StreamInfoHeaderFormatter::StreamInfoHeaderFormatter(absl::string_view field_nam }; } else if (field_name == "UPSTREAM_LOCAL_ADDRESS_WITHOUT_PORT") { field_extractor_ = [](const Envoy::StreamInfo::StreamInfo& stream_info) -> std::string { + if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.correct_remote_address")) { + if (stream_info.upstreamInfo().has_value() && + stream_info.upstreamInfo()->upstreamHost()->address()) { + return StreamInfo::Utility::formatDownstreamAddressNoPort( + *stream_info.upstreamInfo()->upstreamHost()->address()); + } + return ""; + } if (stream_info.upstreamInfo().has_value() && stream_info.upstreamInfo()->upstreamLocalAddress()) { return StreamInfo::Utility::formatDownstreamAddressNoPort( @@ -371,6 +386,14 @@ StreamInfoHeaderFormatter::StreamInfoHeaderFormatter(absl::string_view field_nam }; } else if (field_name == "UPSTREAM_LOCAL_PORT") { field_extractor_ = [](const Envoy::StreamInfo::StreamInfo& stream_info) -> std::string { + if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.correct_remote_address")) { + if (stream_info.upstreamInfo().has_value() && + stream_info.upstreamInfo()->upstreamHost()->address()) { + return StreamInfo::Utility::formatDownstreamAddressJustPort( + *stream_info.upstreamInfo()->upstreamHost()->address()); + } + return ""; + } if (stream_info.upstreamInfo().has_value() && stream_info.upstreamInfo()->upstreamLocalAddress()) { return StreamInfo::Utility::formatDownstreamAddressJustPort( @@ -380,16 +403,16 @@ StreamInfoHeaderFormatter::StreamInfoHeaderFormatter(absl::string_view field_nam }; } else if (field_name == "UPSTREAM_REMOTE_ADDRESS") { field_extractor_ = [](const Envoy::StreamInfo::StreamInfo& stream_info) -> std::string { - if (stream_info.upstreamInfo() && stream_info.upstreamInfo()->upstreamHost()) { - return stream_info.upstreamInfo()->upstreamHost()->address()->asString(); + if (stream_info.upstreamInfo() && stream_info.upstreamInfo()->upstreamRemoteAddress()) { + return stream_info.upstreamInfo()->upstreamRemoteAddress()->asString(); } return ""; }; } else if (field_name == "UPSTREAM_REMOTE_ADDRESS_WITHOUT_PORT") { field_extractor_ = [](const Envoy::StreamInfo::StreamInfo& stream_info) -> std::string { - if (stream_info.upstreamInfo() && stream_info.upstreamInfo()->upstreamHost()) { + if (stream_info.upstreamInfo() && stream_info.upstreamInfo()->upstreamRemoteAddress()) { return StreamInfo::Utility::formatDownstreamAddressNoPort( - *stream_info.upstreamInfo()->upstreamHost()->address()); + *stream_info.upstreamInfo()->upstreamRemoteAddress()); } return ""; }; diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc index 8d3cc149e150b..bbf32a0a169eb 100644 --- a/source/common/router/upstream_request.cc +++ b/source/common/router/upstream_request.cc @@ -710,10 +710,11 @@ void UpstreamRequest::onPoolFailure(ConnectionPool::PoolFailureReason reason, onResetStream(reset_reason, transport_failure_reason); } -void UpstreamRequest::onPoolReady( - std::unique_ptr&& upstream, Upstream::HostDescriptionConstSharedPtr host, - const Network::Address::InstanceConstSharedPtr& upstream_local_address, - StreamInfo::StreamInfo& info, absl::optional protocol) { +void UpstreamRequest::onPoolReady(std::unique_ptr&& upstream, + Upstream::HostDescriptionConstSharedPtr host, + const Network::ConnectionInfoProvider& address_provider, + StreamInfo::StreamInfo& info, + absl::optional protocol) { // This may be called under an existing ScopeTrackerScopeState but it will unwind correctly. ScopeTrackerScopeState scope(&parent_.callbacks()->scope(), parent_.callbacks()->dispatcher()); ENVOY_STREAM_LOG(debug, "pool ready", *parent_.callbacks()); @@ -763,7 +764,8 @@ void UpstreamRequest::onPoolReady( } else { upstream_info.setUpstreamFilterState(filter_state); } - upstream_info.setUpstreamLocalAddress(upstream_local_address); + upstream_info.setUpstreamLocalAddress(address_provider.localAddress()); + upstream_info.setUpstreamRemoteAddress(address_provider.remoteAddress()); upstream_info.setUpstreamSslConnection(info.downstreamAddressProvider().sslConnection()); if (info.downstreamAddressProvider().connectionID().has_value()) { diff --git a/source/common/router/upstream_request.h b/source/common/router/upstream_request.h index 6dd823ec3821f..1232dd340bbfc 100644 --- a/source/common/router/upstream_request.h +++ b/source/common/router/upstream_request.h @@ -137,7 +137,7 @@ class UpstreamRequest : public Logger::Loggable, Upstream::HostDescriptionConstSharedPtr host) override; void onPoolReady(std::unique_ptr&& upstream, Upstream::HostDescriptionConstSharedPtr host, - const Network::Address::InstanceConstSharedPtr& upstream_local_address, + const Network::ConnectionInfoProvider& address_provider, StreamInfo::StreamInfo& info, absl::optional protocol) override; UpstreamToDownstream& upstreamToDownstream() override; diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index f0f9d1380a561..442f1b8d66387 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -41,6 +41,7 @@ RUNTIME_GUARD(envoy_reloadable_features_cares_accept_nodata); RUNTIME_GUARD(envoy_reloadable_features_combine_sds_requests); RUNTIME_GUARD(envoy_reloadable_features_conn_pool_delete_when_idle); RUNTIME_GUARD(envoy_reloadable_features_conn_pool_new_stream_with_early_data_and_http3); +RUNTIME_GUARD(envoy_reloadable_features_correct_remote_address); RUNTIME_GUARD(envoy_reloadable_features_deprecate_global_ints); RUNTIME_GUARD(envoy_reloadable_features_do_not_await_headers_on_upstream_timeout_to_emit_stats); RUNTIME_GUARD(envoy_reloadable_features_do_not_count_mapped_pages_as_free); diff --git a/source/common/stream_info/stream_info_impl.h b/source/common/stream_info/stream_info_impl.h index b674aaa122dd4..6a3e39130a805 100644 --- a/source/common/stream_info/stream_info_impl.h +++ b/source/common/stream_info/stream_info_impl.h @@ -49,10 +49,17 @@ struct UpstreamInfoImpl : public UpstreamInfo { const Network::Address::InstanceConstSharedPtr& upstreamLocalAddress() const override { return upstream_local_address_; } + const Network::Address::InstanceConstSharedPtr& upstreamRemoteAddress() const override { + return upstream_remote_address_; + } void setUpstreamLocalAddress( const Network::Address::InstanceConstSharedPtr& upstream_local_address) override { upstream_local_address_ = upstream_local_address; } + void setUpstreamRemoteAddress( + const Network::Address::InstanceConstSharedPtr& upstream_remote_address) override { + upstream_remote_address_ = upstream_remote_address; + } void setUpstreamTransportFailureReason(absl::string_view failure_reason) override { upstream_transport_failure_reason_ = std::string(failure_reason); } @@ -84,6 +91,7 @@ struct UpstreamInfoImpl : public UpstreamInfo { Upstream::HostDescriptionConstSharedPtr upstream_host_{}; Network::Address::InstanceConstSharedPtr upstream_local_address_; + Network::Address::InstanceConstSharedPtr upstream_remote_address_; UpstreamTiming upstream_timing_; Ssl::ConnectionInfoConstSharedPtr upstream_ssl_info_; absl::optional upstream_connection_id_; diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index f0c1c5d42fb9a..5979f237a21a1 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -506,14 +506,15 @@ void Filter::onGenericPoolFailure(ConnectionPool::PoolFailureReason reason, void Filter::onGenericPoolReady(StreamInfo::StreamInfo* info, std::unique_ptr&& upstream, Upstream::HostDescriptionConstSharedPtr& host, - const Network::Address::InstanceConstSharedPtr& local_address, + const Network::ConnectionInfoProvider& address_provider, Ssl::ConnectionInfoConstSharedPtr ssl_info) { upstream_ = std::move(upstream); generic_conn_pool_.reset(); read_callbacks_->upstreamHost(host); StreamInfo::UpstreamInfo& upstream_info = *getStreamInfo().upstreamInfo(); upstream_info.setUpstreamHost(host); - upstream_info.setUpstreamLocalAddress(local_address); + upstream_info.setUpstreamLocalAddress(address_provider.localAddress()); + upstream_info.setUpstreamRemoteAddress(address_provider.remoteAddress()); upstream_info.setUpstreamSslConnection(ssl_info); onUpstreamConnection(); read_callbacks_->continueReading(); diff --git a/source/common/tcp_proxy/tcp_proxy.h b/source/common/tcp_proxy/tcp_proxy.h index 0f3705c9d2e95..a03376ed1ee4d 100644 --- a/source/common/tcp_proxy/tcp_proxy.h +++ b/source/common/tcp_proxy/tcp_proxy.h @@ -341,7 +341,7 @@ class Filter : public Network::ReadFilter, // GenericConnectionPoolCallbacks void onGenericPoolReady(StreamInfo::StreamInfo* info, std::unique_ptr&& upstream, Upstream::HostDescriptionConstSharedPtr& host, - const Network::Address::InstanceConstSharedPtr& local_address, + const Network::ConnectionInfoProvider& address_provider, Ssl::ConnectionInfoConstSharedPtr ssl_info) override; void onGenericPoolFailure(ConnectionPool::PoolFailureReason reason, absl::string_view failure_reason, diff --git a/source/common/tcp_proxy/upstream.cc b/source/common/tcp_proxy/upstream.cc index 53dbff2a2e593..439c8253e5822 100644 --- a/source/common/tcp_proxy/upstream.cc +++ b/source/common/tcp_proxy/upstream.cc @@ -189,7 +189,7 @@ void TcpConnPool::onPoolReady(Tcp::ConnectionPool::ConnectionDataPtr&& conn_data auto upstream = std::make_unique(std::move(conn_data), upstream_callbacks_); callbacks_->onGenericPoolReady( &connection.streamInfo(), std::move(upstream), host, - latched_data->connection().connectionInfoProvider().localAddress(), + latched_data->connection().connectionInfoProvider(), latched_data->connection().streamInfo().downstreamAddressProvider().sslConnection()); } @@ -252,9 +252,9 @@ void HttpConnPool::onPoolReady(Http::RequestEncoder& request_encoder, } void HttpConnPool::onGenericPoolReady(Upstream::HostDescriptionConstSharedPtr& host, - const Network::Address::InstanceConstSharedPtr& local_address, + const Network::ConnectionInfoProvider& address_provider, Ssl::ConnectionInfoConstSharedPtr ssl_info) { - callbacks_->onGenericPoolReady(nullptr, std::move(upstream_), host, local_address, ssl_info); + callbacks_->onGenericPoolReady(nullptr, std::move(upstream_), host, address_provider, ssl_info); } Http2Upstream::Http2Upstream(Tcp::ConnectionPool::UpstreamCallbacks& callbacks, diff --git a/source/common/tcp_proxy/upstream.h b/source/common/tcp_proxy/upstream.h index 2597804939d3e..1d4afd41ef868 100644 --- a/source/common/tcp_proxy/upstream.h +++ b/source/common/tcp_proxy/upstream.h @@ -71,7 +71,7 @@ class HttpConnPool : public GenericConnPool, public Http::ConnectionPool::Callba virtual ~Callbacks() = default; virtual void onSuccess(Http::RequestEncoder& request_encoder) { ASSERT(conn_pool_ != nullptr); - conn_pool_->onGenericPoolReady(host_, request_encoder.getStream().connectionLocalAddress(), + conn_pool_->onGenericPoolReady(host_, request_encoder.getStream().connectionInfoProvider(), ssl_info_); } virtual void onFailure() { @@ -91,7 +91,7 @@ class HttpConnPool : public GenericConnPool, public Http::ConnectionPool::Callba private: void onGenericPoolReady(Upstream::HostDescriptionConstSharedPtr& host, - const Network::Address::InstanceConstSharedPtr& local_address, + const Network::ConnectionInfoProvider& address_provider, Ssl::ConnectionInfoConstSharedPtr ssl_info); const TunnelingConfigHelper& config_; Http::CodecType type_; diff --git a/source/extensions/upstreams/http/http/upstream_request.cc b/source/extensions/upstreams/http/http/upstream_request.cc index 7923d554ac4a1..616918e8d7d5d 100644 --- a/source/extensions/upstreams/http/http/upstream_request.cc +++ b/source/extensions/upstreams/http/http/upstream_request.cc @@ -63,7 +63,7 @@ void HttpConnPool::onPoolReady(Envoy::Http::RequestEncoder& request_encoder, auto upstream = std::make_unique(callbacks_->upstreamToDownstream(), &request_encoder); callbacks_->onPoolReady(std::move(upstream), host, - request_encoder.getStream().connectionLocalAddress(), info, protocol); + request_encoder.getStream().connectionInfoProvider(), info, protocol); } } // namespace Http diff --git a/source/extensions/upstreams/http/tcp/upstream_request.cc b/source/extensions/upstreams/http/tcp/upstream_request.cc index 5f4263074f976..8d36cc00175fa 100644 --- a/source/extensions/upstreams/http/tcp/upstream_request.cc +++ b/source/extensions/upstreams/http/tcp/upstream_request.cc @@ -27,8 +27,7 @@ void TcpConnPool::onPoolReady(Envoy::Tcp::ConnectionPool::ConnectionDataPtr&& co Network::Connection& latched_conn = conn_data->connection(); auto upstream = std::make_unique(&callbacks_->upstreamToDownstream(), std::move(conn_data)); - callbacks_->onPoolReady(std::move(upstream), host, - latched_conn.connectionInfoProvider().localAddress(), + callbacks_->onPoolReady(std::move(upstream), host, latched_conn.connectionInfoProvider(), latched_conn.streamInfo(), {}); } diff --git a/test/common/router/BUILD b/test/common/router/BUILD index 427b6ca79c293..217fb94c13991 100644 --- a/test/common/router/BUILD +++ b/test/common/router/BUILD @@ -429,6 +429,7 @@ envoy_cc_test( "//test/mocks/ssl:ssl_mocks", "//test/mocks/stream_info:stream_info_mocks", "//test/mocks/upstream:host_mocks", + "//test/test_common:test_runtime_lib", "//test/test_common:threadsafe_singleton_injector_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", diff --git a/test/common/router/header_formatter_test.cc b/test/common/router/header_formatter_test.cc index 72b00c9b0e94e..d59b2f3eaccc8 100644 --- a/test/common/router/header_formatter_test.cc +++ b/test/common/router/header_formatter_test.cc @@ -21,6 +21,7 @@ #include "test/mocks/ssl/mocks.h" #include "test/mocks/stream_info/mocks.h" #include "test/mocks/upstream/host.h" +#include "test/test_common/test_runtime.h" #include "test/test_common/threadsafe_singleton_injector.h" #include "test/test_common/utility.h" @@ -53,6 +54,12 @@ class StreamInfoHeaderFormatterTest : public testing::Test { const std::string formatted_string = f.format(stream_info); EXPECT_EQ(expected_output, formatted_string); } + if (test_with_and_without_runtime_) { + TestScopedRuntime runtime_; + auto f = StreamInfoHeaderFormatter(variable); + const std::string formatted_string = f.format(stream_info); + EXPECT_EQ(expected_output, formatted_string); + } } void testFormatting(const std::string& variable, const std::string& expected_output) { @@ -64,6 +71,7 @@ class StreamInfoHeaderFormatterTest : public testing::Test { EXPECT_THROW_WITH_MESSAGE(StreamInfoHeaderFormatter{variable}, EnvoyException, fmt::format("field '{}' not supported as custom header", variable)); } + bool test_with_and_without_runtime_ = false; }; TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamRemoteAddressVariable) { @@ -165,26 +173,41 @@ TEST_F(StreamInfoHeaderFormatterTest, } TEST_F(StreamInfoHeaderFormatterTest, TestformatWithUpstreamRemoteAddressVariable) { + test_with_and_without_runtime_ = true; testFormatting("UPSTREAM_REMOTE_ADDRESS", "10.0.0.1:443"); NiceMock stream_info; stream_info.upstreamInfo()->setUpstreamHost(nullptr); + Network::Address::InstanceConstSharedPtr nullptr_address; + EXPECT_CALL(*dynamic_cast(stream_info.upstream_info_.get()), + upstreamRemoteAddress()) + .WillRepeatedly(ReturnRef(nullptr_address)); testFormatting(stream_info, "UPSTREAM_REMOTE_ADDRESS", ""); } TEST_F(StreamInfoHeaderFormatterTest, TestformatWithUpstreamRemotePortVariable) { + test_with_and_without_runtime_ = true; testFormatting("UPSTREAM_REMOTE_PORT", "443"); NiceMock stream_info; stream_info.upstreamInfo()->setUpstreamHost(nullptr); + Network::Address::InstanceConstSharedPtr nullptr_address; + EXPECT_CALL(*dynamic_cast(stream_info.upstream_info_.get()), + upstreamRemoteAddress()) + .WillRepeatedly(ReturnRef(nullptr_address)); testFormatting(stream_info, "UPSTREAM_REMOTE_PORT", ""); } TEST_F(StreamInfoHeaderFormatterTest, TestformatWithUpstreamRemoteAddressWithoutPortVariable) { + test_with_and_without_runtime_ = true; testFormatting("UPSTREAM_REMOTE_ADDRESS_WITHOUT_PORT", "10.0.0.1"); NiceMock stream_info; stream_info.upstreamInfo()->setUpstreamHost(nullptr); + Network::Address::InstanceConstSharedPtr nullptr_address; + EXPECT_CALL(*dynamic_cast(stream_info.upstream_info_.get()), + upstreamRemoteAddress()) + .WillRepeatedly(ReturnRef(nullptr_address)); testFormatting(stream_info, "UPSTREAM_REMOTE_ADDRESS_WITHOUT_PORT", ""); } diff --git a/test/common/router/router_upstream_log_test.cc b/test/common/router/router_upstream_log_test.cc index af6c93adda96f..192eec21e31e0 100644 --- a/test/common/router/router_upstream_log_test.cc +++ b/test/common/router/router_upstream_log_test.cc @@ -156,8 +156,8 @@ class RouterUpstreamLogTest : public testing::Test { const Http::ConnectionPool::Instance::StreamOptions&) -> Http::ConnectionPool::Cancellable* { response_decoder = &decoder; - EXPECT_CALL(encoder.stream_, connectionLocalAddress()) - .WillRepeatedly(ReturnRef(upstream_local_address1_)); + EXPECT_CALL(encoder.stream_, connectionInfoProvider()) + .WillRepeatedly(ReturnRef(connection_info1_)); callbacks.onPoolReady(encoder, context_.cluster_manager_.thread_local_cluster_.conn_pool_.host_, stream_info_, Http::Protocol::Http10); @@ -198,8 +198,8 @@ class RouterUpstreamLogTest : public testing::Test { const Http::ConnectionPool::Instance::StreamOptions&) -> Http::ConnectionPool::Cancellable* { response_decoder = &decoder; - EXPECT_CALL(encoder1.stream_, connectionLocalAddress()) - .WillRepeatedly(ReturnRef(upstream_local_address1_)); + EXPECT_CALL(encoder1.stream_, connectionInfoProvider()) + .WillRepeatedly(ReturnRef(connection_info1_)); callbacks.onPoolReady(encoder1, context_.cluster_manager_.thread_local_cluster_.conn_pool_.host_, stream_info_, Http::Protocol::Http10); @@ -230,8 +230,8 @@ class RouterUpstreamLogTest : public testing::Test { EXPECT_CALL( context_.cluster_manager_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putResult(Upstream::Outlier::Result::LocalOriginConnectSuccess, _)); - EXPECT_CALL(encoder2.stream_, connectionLocalAddress()) - .WillRepeatedly(ReturnRef(upstream_local_address2_)); + EXPECT_CALL(encoder2.stream_, connectionInfoProvider()) + .WillRepeatedly(ReturnRef(connection_info2_)); callbacks.onPoolReady(encoder2, context_.cluster_manager_.thread_local_cluster_.conn_pool_.host_, stream_info_, Http::Protocol::Http10); @@ -263,6 +263,10 @@ class RouterUpstreamLogTest : public testing::Test { Network::Utility::resolveUrl("tcp://10.0.0.5:10211")}; Network::Address::InstanceConstSharedPtr upstream_local_address2_{ Network::Utility::resolveUrl("tcp://10.0.0.5:10212")}; + Network::ConnectionInfoSetterImpl connection_info1_{upstream_local_address1_, + upstream_local_address1_}; + Network::ConnectionInfoSetterImpl connection_info2_{upstream_local_address2_, + upstream_local_address2_}; Event::MockTimer* response_timeout_{}; Event::MockTimer* per_try_timeout_{}; diff --git a/test/integration/filters/stream_info_to_headers_filter.cc b/test/integration/filters/stream_info_to_headers_filter.cc index b6de41d00b4dc..6d93bc4f29edd 100644 --- a/test/integration/filters/stream_info_to_headers_filter.cc +++ b/test/integration/filters/stream_info_to_headers_filter.cc @@ -50,6 +50,11 @@ class StreamInfoToHeadersFilter : public Http::PassThroughFilter { Http::LowerCaseString("alpn"), decoder_callbacks_->streamInfo().upstreamInfo()->upstreamSslConnection()->alpn()); } + if (decoder_callbacks_->streamInfo().upstreamInfo()->upstreamRemoteAddress()) { + headers.addCopy( + Http::LowerCaseString("remote_address"), + decoder_callbacks_->streamInfo().upstreamInfo()->upstreamRemoteAddress()->asString()); + } headers.addCopy(Http::LowerCaseString("num_streams"), decoder_callbacks_->streamInfo().upstreamInfo()->upstreamNumStreams()); diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index fb9634077d1a0..274a2e516e19b 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -314,7 +314,7 @@ HttpIntegrationTest::HttpIntegrationTest(Http::CodecType downstream_protocol, downstream_protocol, [version](int) { return Network::Utility::parseInternetAddress( - Network::Test::getAnyAddressString(version), 0); + Network::Test::getLoopbackAddressString(version), 0); }, version, config) {} diff --git a/test/integration/multiplexed_upstream_integration_test.cc b/test/integration/multiplexed_upstream_integration_test.cc index 75a3019aa3bbb..783462aa472e8 100644 --- a/test/integration/multiplexed_upstream_integration_test.cc +++ b/test/integration/multiplexed_upstream_integration_test.cc @@ -161,6 +161,9 @@ void MultiplexedUpstreamIntegrationTest::bidirectionalStreaming(uint32_t bytes) EXPECT_EQ( "1", response->headers().get(Http::LowerCaseString("num_streams"))[0]->value().getStringView()); + EXPECT_EQ( + fake_upstreams_[0]->localAddress()->asString(), + response->headers().get(Http::LowerCaseString("remote_address"))[0]->value().getStringView()); } TEST_P(MultiplexedUpstreamIntegrationTest, BidirectionalStreaming) { bidirectionalStreaming(1024); } diff --git a/test/integration/upstreams/per_host_upstream_config.h b/test/integration/upstreams/per_host_upstream_config.h index d16fcb38120f6..829d4b871baeb 100644 --- a/test/integration/upstreams/per_host_upstream_config.h +++ b/test/integration/upstreams/per_host_upstream_config.h @@ -83,7 +83,7 @@ class PerHostHttpConnPool : public Extensions::Upstreams::Http::Http::HttpConnPo auto upstream = std::make_unique(callbacks_->upstreamToDownstream(), &callbacks_encoder, host); callbacks_->onPoolReady(std::move(upstream), host, - callbacks_encoder.getStream().connectionLocalAddress(), info, protocol); + callbacks_encoder.getStream().connectionInfoProvider(), info, protocol); } }; diff --git a/test/mocks/http/BUILD b/test/mocks/http/BUILD index bbbd6fb68f632..5ac43d98b9ea2 100644 --- a/test/mocks/http/BUILD +++ b/test/mocks/http/BUILD @@ -85,6 +85,7 @@ envoy_cc_mock( hdrs = ["stream.h"], deps = [ "//envoy/http:codec_interface", + "//source/common/network:socket_lib", ], ) diff --git a/test/mocks/http/stream.cc b/test/mocks/http/stream.cc index 9410afdf2774c..2ce4ce232960d 100644 --- a/test/mocks/http/stream.cc +++ b/test/mocks/http/stream.cc @@ -7,7 +7,7 @@ using testing::ReturnRef; namespace Envoy { namespace Http { -MockStream::MockStream() { +MockStream::MockStream() : connection_info_provider_(nullptr, nullptr) { ON_CALL(*this, addCallbacks(_)).WillByDefault(Invoke([this](StreamCallbacks& callbacks) -> void { callbacks_.push_back(&callbacks); })); @@ -31,7 +31,7 @@ MockStream::MockStream() { } })); - ON_CALL(*this, connectionLocalAddress()).WillByDefault(ReturnRef(connection_local_address_)); + ON_CALL(*this, connectionInfoProvider()).WillByDefault(ReturnRef(connection_info_provider_)); ON_CALL(*this, setAccount(_)) .WillByDefault(Invoke( diff --git a/test/mocks/http/stream.h b/test/mocks/http/stream.h index 2d0356abf812d..59b603e8b28f0 100644 --- a/test/mocks/http/stream.h +++ b/test/mocks/http/stream.h @@ -2,6 +2,8 @@ #include "envoy/http/codec.h" +#include "source/common/network/socket_impl.h" + #include "gmock/gmock.h" namespace Envoy { @@ -19,14 +21,14 @@ class MockStream : public Stream { MOCK_METHOD(void, readDisable, (bool disable)); MOCK_METHOD(void, setWriteBufferWatermarks, (uint32_t)); MOCK_METHOD(uint32_t, bufferLimit, (), (const)); - MOCK_METHOD(const Network::Address::InstanceConstSharedPtr&, connectionLocalAddress, ()); + MOCK_METHOD(const Network::ConnectionInfoProvider&, connectionInfoProvider, ()); MOCK_METHOD(void, setFlushTimeout, (std::chrono::milliseconds timeout)); MOCK_METHOD(void, setAccount, (Buffer::BufferMemoryAccountSharedPtr)); // Use the same underlying structure as StreamCallbackHelper to insure iteration stability // if we remove callbacks during iteration. absl::InlinedVector callbacks_; - Network::Address::InstanceConstSharedPtr connection_local_address_; + Network::ConnectionInfoSetterImpl connection_info_provider_; Buffer::BufferMemoryAccountSharedPtr account_; void runHighWatermarkCallbacks() { diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index ba3b848d11efc..a6507a9b74f66 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -627,8 +627,8 @@ class MockGenericConnectionPoolCallbacks : public GenericConnectionPoolCallbacks MOCK_METHOD(void, onPoolReady, (std::unique_ptr && upstream, Upstream::HostDescriptionConstSharedPtr host, - const Network::Address::InstanceConstSharedPtr& upstream_local_address, - StreamInfo::StreamInfo& info, absl::optional protocol)); + const Network::ConnectionInfoProvider& info_provider, StreamInfo::StreamInfo& info, + absl::optional protocol)); MOCK_METHOD(UpstreamToDownstream&, upstreamToDownstream, ()); NiceMock upstream_to_downstream_; diff --git a/test/mocks/stream_info/mocks.cc b/test/mocks/stream_info/mocks.cc index d2d0bb1a051c4..0f6aa8b7d93fb 100644 --- a/test/mocks/stream_info/mocks.cc +++ b/test/mocks/stream_info/mocks.cc @@ -19,6 +19,7 @@ namespace StreamInfo { MockUpstreamInfo::MockUpstreamInfo() : upstream_local_address_(new Network::Address::Ipv4Instance("127.1.2.3", 58443)), + upstream_remote_address_(new Network::Address::Ipv4Instance("10.0.0.1", 443)), upstream_host_(new testing::NiceMock()) { ON_CALL(*this, dumpState(_, _)).WillByDefault(Invoke([](std::ostream& os, int indent_level) { os << "MockUpstreamInfo test dumpState with indent: " << indent_level << std::endl; @@ -66,6 +67,7 @@ MockUpstreamInfo::MockUpstreamInfo() upstream_protocol_ = protocol; })); ON_CALL(*this, upstreamProtocol()).WillByDefault(ReturnPointee(&upstream_protocol_)); + ON_CALL(*this, upstreamRemoteAddress()).WillByDefault(ReturnRef(upstream_remote_address_)); } MockUpstreamInfo::~MockUpstreamInfo() = default; diff --git a/test/mocks/stream_info/mocks.h b/test/mocks/stream_info/mocks.h index 65eb03bdf81eb..3b90f4c66fb8c 100644 --- a/test/mocks/stream_info/mocks.h +++ b/test/mocks/stream_info/mocks.h @@ -34,6 +34,9 @@ class MockUpstreamInfo : public UpstreamInfo { MOCK_METHOD(void, setUpstreamLocalAddress, (const Network::Address::InstanceConstSharedPtr& upstream_local_address)); MOCK_METHOD(const Network::Address::InstanceConstSharedPtr&, upstreamLocalAddress, (), (const)); + MOCK_METHOD(void, setUpstreamRemoteAddress, + (const Network::Address::InstanceConstSharedPtr& upstream_remote_address)); + MOCK_METHOD(const Network::Address::InstanceConstSharedPtr&, upstreamRemoteAddress, (), (const)); MOCK_METHOD(void, setUpstreamTransportFailureReason, (absl::string_view failure_reason)); MOCK_METHOD(const std::string&, upstreamTransportFailureReason, (), (const)); MOCK_METHOD(void, setUpstreamHost, (Upstream::HostDescriptionConstSharedPtr host)); @@ -50,6 +53,7 @@ class MockUpstreamInfo : public UpstreamInfo { Ssl::ConnectionInfoConstSharedPtr ssl_connection_info_; UpstreamTiming upstream_timing_; Network::Address::InstanceConstSharedPtr upstream_local_address_; + Network::Address::InstanceConstSharedPtr upstream_remote_address_; std::string failure_reason_; Upstream::HostDescriptionConstSharedPtr upstream_host_; FilterStateSharedPtr filter_state_;