From 0cb59afc2765a82650661525327dae723b86cf7d Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Wed, 1 Apr 2020 12:16:39 -0400 Subject: [PATCH 01/17] router: adding CONNECT support Signed-off-by: Alyssa Wilk --- api/envoy/api/v2/route/route_components.proto | 16 +- .../config/route/v3/route_components.proto | 19 +- .../envoy/api/v2/route/route_components.proto | 16 +- .../config/route/v3/route_components.proto | 19 +- include/envoy/router/BUILD | 1 + include/envoy/router/router.h | 19 ++ source/common/http/async_client_impl.h | 2 + source/common/router/config_impl.h | 8 + source/common/router/router.cc | 58 +++- source/common/router/upstream_request.cc | 76 ++++++ source/common/router/upstream_request.h | 73 +++++ test/common/router/BUILD | 20 ++ test/common/router/upstream_request_test.cc | 258 ++++++++++++++++++ test/mocks/router/mocks.h | 2 + 14 files changed, 574 insertions(+), 13 deletions(-) create mode 100644 test/common/router/upstream_request_test.cc diff --git a/api/envoy/api/v2/route/route_components.proto b/api/envoy/api/v2/route/route_components.proto index c4ccf2c8c9a18..6b27a569cec15 100644 --- a/api/envoy/api/v2/route/route_components.proto +++ b/api/envoy/api/v2/route/route_components.proto @@ -34,7 +34,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // host header. This allows a single listener to service multiple top level domain path trees. Once // a virtual host is selected based on the domain, the routes are processed in order to see which // upstream cluster to route to or whether to perform a redirect. -// [#next-free-field: 21] +// [#next-free-field: 23] message VirtualHost { enum TlsRequirementType { // No TLS requirement for the virtual host. @@ -49,6 +49,13 @@ message VirtualHost { ALL = 2; } + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when translating HTTP body data to TCP payload. + message ProxyingConfig { + // TODO(alyssawilk) add proxy proto configuration here. + } + reserved 9; // The logical name of the virtual host. This is used when emitting certain @@ -180,6 +187,13 @@ message VirtualHost { // If set and a route-specific limit is not set, the bytes actually buffered will be the minimum // value of this and the listener per_connection_buffer_limit_bytes. google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18; + + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for decapsulation + // of CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request + // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing + // response headers and forwarding the TCP response payload in the HTTP response body. + ProxyingConfig proxying_config = 22; } // A filter-defined action type. diff --git a/api/envoy/config/route/v3/route_components.proto b/api/envoy/config/route/v3/route_components.proto index d5995c1f7b972..13e957906a186 100644 --- a/api/envoy/config/route/v3/route_components.proto +++ b/api/envoy/config/route/v3/route_components.proto @@ -33,7 +33,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // host header. This allows a single listener to service multiple top level domain path trees. Once // a virtual host is selected based on the domain, the routes are processed in order to see which // upstream cluster to route to or whether to perform a redirect. -// [#next-free-field: 21] +// [#next-free-field: 23] message VirtualHost { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.VirtualHost"; @@ -50,6 +50,16 @@ message VirtualHost { ALL = 2; } + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when translating HTTP body data to TCP payload. + message ProxyingConfig { + // TODO(alyssawilk) add proxy proto configuration here. + + option (udpa.annotations.versioning).previous_message_type = + "envoy.api.v2.route.VirtualHost.ProxyingConfig"; + } + reserved 9, 12; reserved "per_filter_config"; @@ -176,6 +186,13 @@ message VirtualHost { // If set and a route-specific limit is not set, the bytes actually buffered will be the minimum // value of this and the listener per_connection_buffer_limit_bytes. google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18; + + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for decapsulation + // of CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request + // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing + // response headers and forwarding the TCP response payload in the HTTP response body. + ProxyingConfig proxying_config = 22; } // A filter-defined action type. diff --git a/generated_api_shadow/envoy/api/v2/route/route_components.proto b/generated_api_shadow/envoy/api/v2/route/route_components.proto index c4ccf2c8c9a18..6b27a569cec15 100644 --- a/generated_api_shadow/envoy/api/v2/route/route_components.proto +++ b/generated_api_shadow/envoy/api/v2/route/route_components.proto @@ -34,7 +34,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // host header. This allows a single listener to service multiple top level domain path trees. Once // a virtual host is selected based on the domain, the routes are processed in order to see which // upstream cluster to route to or whether to perform a redirect. -// [#next-free-field: 21] +// [#next-free-field: 23] message VirtualHost { enum TlsRequirementType { // No TLS requirement for the virtual host. @@ -49,6 +49,13 @@ message VirtualHost { ALL = 2; } + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when translating HTTP body data to TCP payload. + message ProxyingConfig { + // TODO(alyssawilk) add proxy proto configuration here. + } + reserved 9; // The logical name of the virtual host. This is used when emitting certain @@ -180,6 +187,13 @@ message VirtualHost { // If set and a route-specific limit is not set, the bytes actually buffered will be the minimum // value of this and the listener per_connection_buffer_limit_bytes. google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18; + + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for decapsulation + // of CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request + // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing + // response headers and forwarding the TCP response payload in the HTTP response body. + ProxyingConfig proxying_config = 22; } // A filter-defined action type. diff --git a/generated_api_shadow/envoy/config/route/v3/route_components.proto b/generated_api_shadow/envoy/config/route/v3/route_components.proto index 859b3721b77a7..a85879ae9245f 100644 --- a/generated_api_shadow/envoy/config/route/v3/route_components.proto +++ b/generated_api_shadow/envoy/config/route/v3/route_components.proto @@ -33,7 +33,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // host header. This allows a single listener to service multiple top level domain path trees. Once // a virtual host is selected based on the domain, the routes are processed in order to see which // upstream cluster to route to or whether to perform a redirect. -// [#next-free-field: 21] +// [#next-free-field: 23] message VirtualHost { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.VirtualHost"; @@ -50,6 +50,16 @@ message VirtualHost { ALL = 2; } + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when translating HTTP body data to TCP payload. + message ProxyingConfig { + // TODO(alyssawilk) add proxy proto configuration here. + + option (udpa.annotations.versioning).previous_message_type = + "envoy.api.v2.route.VirtualHost.ProxyingConfig"; + } + reserved 9; // The logical name of the virtual host. This is used when emitting certain @@ -182,6 +192,13 @@ message VirtualHost { // If set and a route-specific limit is not set, the bytes actually buffered will be the minimum // value of this and the listener per_connection_buffer_limit_bytes. google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18; + + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for decapsulation + // of CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request + // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing + // response headers and forwarding the TCP response payload in the HTTP response body. + ProxyingConfig proxying_config = 22; } // A filter-defined action type. diff --git a/include/envoy/router/BUILD b/include/envoy/router/BUILD index 6ed49171af71f..b829997d24aa0 100644 --- a/include/envoy/router/BUILD +++ b/include/envoy/router/BUILD @@ -66,6 +66,7 @@ envoy_cc_library( "//source/common/protobuf", "//source/common/protobuf:utility_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", + "@envoy_api//envoy/config/route/v3:pkg_cc_proto", "@envoy_api//envoy/type/v3:pkg_cc_proto", ], ) diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index 13032173d929f..a6ff003ef2d4d 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -11,6 +11,7 @@ #include "envoy/access_log/access_log.h" #include "envoy/common/matchers.h" #include "envoy/config/core/v3/base.pb.h" +#include "envoy/config/route/v3/route_components.pb.h" #include "envoy/config/typed_metadata.h" #include "envoy/http/codec.h" #include "envoy/http/codes.h" @@ -477,6 +478,15 @@ class VirtualHost { */ virtual bool includeAttemptCountInResponse() const PURE; + /** + * If present, this indicates that CONNECT requests to this host should be converted + * to TCP proxying, i.e. the payload of the HTTP body should be sent as raw + * TCP upstream. + * @return configuration for TCP proxying, if present. + */ + using ProxyingConfig = envoy::config::route::v3::VirtualHost::ProxyingConfig; + virtual const absl::optional proxyingConfig() const PURE; + /** * @return uint32_t any route cap on bytes which should be buffered for shadowing or retries. * This is an upper bound so does not necessarily reflect the bytes which will be buffered @@ -819,6 +829,15 @@ class RouteEntry : public ResponseEntry { */ virtual bool includeAttemptCountInResponse() const PURE; + /** + * If present, this indicates that CONNECT requests on this route should be converted + * to TCP proxying, i.e. the payload of the HTTP body should be sent as raw + * TCP upstream. + * @return configuration for TCP proxying, if present. + */ + using ProxyingConfig = envoy::config::route::v3::VirtualHost::ProxyingConfig; + virtual const absl::optional proxyingConfig() const PURE; + using UpgradeMap = std::map; /** * @return a map of route-specific upgrades to their enabled/disabled status. diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index 608b2188dc1bd..42d5334105c7f 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -186,6 +186,7 @@ class AsyncStreamImpl : public AsyncClient::Stream, } bool includeAttemptCountInRequest() const override { return false; } bool includeAttemptCountInResponse() const override { return false; } + const absl::optional proxyingConfig() const override { return absl::nullopt; } uint32_t retryShadowBufferLimit() const override { return std::numeric_limits::max(); } @@ -271,6 +272,7 @@ class AsyncStreamImpl : public AsyncClient::Stream, bool includeAttemptCountInRequest() const override { return false; } bool includeAttemptCountInResponse() const override { return false; } + const absl::optional proxyingConfig() const override { return absl::nullopt; } const Router::RouteEntry::UpgradeMap& upgradeMap() const override { return upgrade_map_; } Router::InternalRedirectAction internalRedirectAction() const override { return Router::InternalRedirectAction::PassThrough; diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index 89add8a903e55..fda91084bd233 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -176,6 +176,7 @@ class VirtualHostImpl : public VirtualHost { const RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override; bool includeAttemptCountInRequest() const override { return include_attempt_count_in_request_; } bool includeAttemptCountInResponse() const override { return include_attempt_count_in_response_; } + const absl::optional proxyingConfig() const override { return proxying_config_; } const absl::optional& retryPolicy() const { return retry_policy_; } @@ -232,6 +233,7 @@ class VirtualHostImpl : public VirtualHost { uint32_t retry_shadow_buffer_limit_{std::numeric_limits::max()}; const bool include_attempt_count_in_request_; const bool include_attempt_count_in_response_; + absl::optional proxying_config_; absl::optional retry_policy_; absl::optional hedge_policy_; const CatchAllVirtualCluster virtual_cluster_catch_all_; @@ -465,6 +467,9 @@ class RouteEntryImplBase : public RouteEntry, bool includeAttemptCountInResponse() const override { return vhost_.includeAttemptCountInResponse(); } + const absl::optional proxyingConfig() const override { + return vhost_.proxyingConfig(); + } const UpgradeMap& upgradeMap() const override { return upgrade_map_; } InternalRedirectAction internalRedirectAction() const override { return internal_redirect_action_; @@ -591,6 +596,9 @@ class RouteEntryImplBase : public RouteEntry, bool includeAttemptCountInResponse() const override { return parent_->includeAttemptCountInResponse(); } + const absl::optional proxyingConfig() const override { + return parent_->proxyingConfig(); + } const UpgradeMap& upgradeMap() const override { return parent_->upgradeMap(); } InternalRedirectAction internalRedirectAction() const override { return parent_->internalRedirectAction(); diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 4171f18440be7..1a19d3a74bfa2 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -113,6 +113,11 @@ bool convertRequestHeadersForInternalRedirect(Http::RequestHeaderMap& downstream constexpr uint64_t TimeoutPrecisionFactor = 100; +bool shouldProxy(Http::RequestHeaderMap& headers, const RouteEntry* route_entry) { + return route_entry->proxyingConfig().has_value() && + headers.Method()->value().getStringView() == Http::Headers::get().MethodValues.Connect; +} + } // namespace // Express percentage as [0, TimeoutPrecisionFactor] because stats do not accept floating point @@ -541,12 +546,30 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, } } - Http::ConnectionPool::Instance* http_pool = getHttpConnPool(); + union ConnPool { + Http::ConnectionPool::Instance* http_pool; + Tcp::ConnectionPool::Instance* tcp_pool; + }; + ConnPool conn_pool; Upstream::HostDescriptionConstSharedPtr host; + bool should_tcp_proxy = shouldProxy(*downstream_headers_, route_entry_); - if (http_pool) { - host = http_pool->host(); + if (!should_tcp_proxy) { + conn_pool.http_pool = getHttpConnPool(); + if (conn_pool.http_pool) { + host = conn_pool.http_pool->host(); + } } else { + transport_socket_options_ = Network::TransportSocketOptionsUtility::fromFilterState( + *callbacks_->streamInfo().filterState()); + conn_pool.tcp_pool = config_.cm_.tcpConnPoolForCluster( + route_entry_->clusterName(), Upstream::ResourcePriority::Default, this); + if (conn_pool.tcp_pool) { + host = conn_pool.tcp_pool->host(); + } + } + + if (!conn_pool.http_pool && !conn_pool.tcp_pool) { sendNoHealthyUpstreamResponse(); return Http::FilterHeadersStatus::StopIteration; } @@ -636,8 +659,14 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, // Hang onto the modify_headers function for later use in handling upstream responses. modify_headers_ = modify_headers; - UpstreamRequestPtr upstream_request = - std::make_unique(*this, std::make_unique(*http_pool)); + UpstreamRequestPtr upstream_request; + if (!should_tcp_proxy) { + upstream_request = std::make_unique( + *this, std::make_unique(*conn_pool.http_pool)); + } else { + upstream_request = + std::make_unique(*this, std::make_unique(conn_pool.tcp_pool)); + } upstream_request->moveIntoList(std::move(upstream_request), upstream_requests_); upstream_requests_.front()->encodeHeaders(end_stream); if (end_stream) { @@ -1438,10 +1467,21 @@ void Filter::doRetry() { pending_retries_--; UpstreamRequestPtr upstream_request; - Http::ConnectionPool::Instance* conn_pool = getHttpConnPool(); - if (conn_pool) { - upstream_request = - std::make_unique(*this, std::make_unique(*conn_pool)); + if (!shouldProxy(*downstream_headers_, route_entry_)) { + Http::ConnectionPool::Instance* conn_pool = getHttpConnPool(); + if (conn_pool) { + upstream_request = + std::make_unique(*this, std::make_unique(*conn_pool)); + } + } else { + transport_socket_options_ = Network::TransportSocketOptionsUtility::fromFilterState( + *callbacks_->streamInfo().filterState()); + Tcp::ConnectionPool::Instance* conn_pool = config_.cm_.tcpConnPoolForCluster( + route_entry_->clusterName(), Upstream::ResourcePriority::Default, this); + if (conn_pool) { + upstream_request = + std::make_unique(*this, std::make_unique(conn_pool)); + } } if (!upstream_request) { diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc index ac048161c93c1..a3e9a88e5b3c8 100644 --- a/source/common/router/upstream_request.cc +++ b/source/common/router/upstream_request.cc @@ -475,6 +475,16 @@ void HttpConnPool::newStream(GenericConnectionPoolCallbacks* callbacks) { } } +void TcpConnPool::onPoolReady(Tcp::ConnectionPool::ConnectionDataPtr&& conn_data, + Upstream::HostDescriptionConstSharedPtr host) { + upstream_handle_ = nullptr; + Network::Connection& latched_conn = conn_data->connection(); + auto upstream = + std::make_unique(callbacks_->upstreamRequest(), std::move(conn_data)); + callbacks_->onPoolReady(std::move(upstream), host, latched_conn.localAddress(), + latched_conn.streamInfo()); +} + bool HttpConnPool::cancelAnyPendingRequest() { if (conn_pool_stream_handle_) { conn_pool_stream_handle_->cancel(); @@ -501,5 +511,71 @@ void HttpConnPool::onPoolReady(Http::RequestEncoder& request_encoder, request_encoder.getStream().connectionLocalAddress(), info); } +TcpUpstream::TcpUpstream(UpstreamRequest* upstream_request, + Tcp::ConnectionPool::ConnectionDataPtr&& upstream) + : upstream_request_(upstream_request), upstream_conn_data_(std::move(upstream)) { + upstream_conn_data_->connection().enableHalfClose(true); + upstream_conn_data_->addUpstreamCallbacks(*this); +} + +void TcpUpstream::encodeData(Buffer::Instance& data, bool end_stream) { + upstream_conn_data_->connection().write(data, end_stream); +} + +void TcpUpstream::encodeHeaders(const Http::RequestHeaderMap&, bool end_stream) { + if (end_stream) { + Buffer::OwnedImpl data; + upstream_conn_data_->connection().write(data, true); + } +} + +void TcpUpstream::encodeTrailers(const Http::RequestTrailerMap&) { + Buffer::OwnedImpl data; + upstream_conn_data_->connection().write(data, true); +} + +void TcpUpstream::readDisable(bool disable) { + if (upstream_conn_data_->connection().state() != Network::Connection::State::Open) { + return; + } + upstream_conn_data_->connection().readDisable(disable); +} + +void TcpUpstream::resetStream() { + upstream_request_ = nullptr; + upstream_conn_data_->connection().close(Network::ConnectionCloseType::NoFlush); +} + +void TcpUpstream::onUpstreamData(Buffer::Instance& data, bool end_stream) { + if (!upstream_request_) { + return; + } + if (!sent_headers_) { + Http::ResponseHeaderMapPtr headers{ + Http::createHeaderMap({{Http::Headers::get().Status, "200"}})}; + upstream_request_->decodeHeaders(std::move(headers), false); + sent_headers_ = true; + } + upstream_request_->decodeData(data, end_stream); +} + +void TcpUpstream::onEvent(Network::ConnectionEvent event) { + if (event != Network::ConnectionEvent::Connected && upstream_request_) { + upstream_request_->onResetStream(Http::StreamResetReason::ConnectionTermination, ""); + } +} + +void TcpUpstream::onAboveWriteBufferHighWatermark() { + if (upstream_request_) { + upstream_request_->disableDataFromDownstreamForFlowControl(); + } +} + +void TcpUpstream::onBelowWriteBufferLowWatermark() { + if (upstream_request_) { + upstream_request_->enableDataFromDownstreamForFlowControl(); + } +} + } // namespace Router } // namespace Envoy diff --git a/source/common/router/upstream_request.h b/source/common/router/upstream_request.h index 0cf36565bed47..d9c1d9bdbf913 100644 --- a/source/common/router/upstream_request.h +++ b/source/common/router/upstream_request.h @@ -200,6 +200,55 @@ class HttpConnPool : public GenericConnPool, public Http::ConnectionPool::Callba GenericConnectionPoolCallbacks* callbacks_{}; }; +class TcpConnPool : public GenericConnPool, public Tcp::ConnectionPool::Callbacks { +public: + TcpConnPool(Tcp::ConnectionPool::Instance* conn_pool) : conn_pool_(conn_pool) {} + + void newStream(GenericConnectionPoolCallbacks* callbacks) override { + callbacks_ = callbacks; + upstream_handle_ = conn_pool_->newConnection(*this); + } + + bool cancelAnyPendingRequest() override { + if (upstream_handle_) { + upstream_handle_->cancel(Tcp::ConnectionPool::CancelPolicy::Default); + upstream_handle_ = nullptr; + return true; + } + return false; + } + absl::optional protocol() const override { return absl::nullopt; } + + static Http::ConnectionPool::PoolFailureReason + convertReason(Tcp::ConnectionPool::PoolFailureReason reason) { + switch (reason) { + case Tcp::ConnectionPool::PoolFailureReason::Overflow: + return Http::ConnectionPool::PoolFailureReason::Overflow; + case Tcp::ConnectionPool::PoolFailureReason::LocalConnectionFailure: + FALLTHRU; + case Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure: + FALLTHRU; + case Tcp::ConnectionPool::PoolFailureReason::Timeout: + return Http::ConnectionPool::PoolFailureReason::ConnectionFailure; + } + } + + // Tcp::ConnectionPool::Callbacks + void onPoolFailure(Tcp::ConnectionPool::PoolFailureReason reason, + Upstream::HostDescriptionConstSharedPtr host) override { + upstream_handle_ = nullptr; + callbacks_->onPoolFailure(convertReason(reason), "", host); + } + + void onPoolReady(Tcp::ConnectionPool::ConnectionDataPtr&& conn_data, + Upstream::HostDescriptionConstSharedPtr host) override; + +private: + Tcp::ConnectionPool::Instance* conn_pool_; + Tcp::ConnectionPool::Cancellable* upstream_handle_{}; + GenericConnectionPoolCallbacks* callbacks_{}; +}; + // A generic API which covers common functionality between HTTP and TCP upstreams. class GenericUpstream { public: @@ -259,5 +308,29 @@ class HttpUpstream : public GenericUpstream, public Http::StreamCallbacks { Http::RequestEncoder* request_encoder_{}; }; +class TcpUpstream : public GenericUpstream, public Tcp::ConnectionPool::UpstreamCallbacks { +public: + TcpUpstream(UpstreamRequest* upstream_request, Tcp::ConnectionPool::ConnectionDataPtr&& upstream); + + // GenericUpstream + void encodeData(Buffer::Instance& data, bool end_stream) override; + void encodeMetadata(const Http::MetadataMapVector&) override {} + void encodeHeaders(const Http::RequestHeaderMap&, bool end_stream) override; + void encodeTrailers(const Http::RequestTrailerMap&) override; + void readDisable(bool disable) override; + void resetStream() override; + + // Tcp::ConnectionPool::UpstreamCallbacks + void onUpstreamData(Buffer::Instance& data, bool end_stream) override; + void onEvent(Network::ConnectionEvent event) override; + void onAboveWriteBufferHighWatermark() override; + void onBelowWriteBufferLowWatermark() override; + +private: + UpstreamRequest* upstream_request_; + Tcp::ConnectionPool::ConnectionDataPtr upstream_conn_data_; + bool sent_headers_{}; +}; + } // namespace Router } // namespace Envoy diff --git a/test/common/router/BUILD b/test/common/router/BUILD index 2ac9c91f26f30..24652f89b50cd 100644 --- a/test/common/router/BUILD +++ b/test/common/router/BUILD @@ -317,6 +317,26 @@ envoy_cc_test( ], ) +envoy_cc_test( + name = "upstream_request_test", + srcs = ["upstream_request_test.cc"], + deps = [ + "//source/common/buffer:buffer_lib", + "//source/common/router:router_lib", + "//source/common/upstream:upstream_includes", + "//source/common/upstream:upstream_lib", + "//test/common/http:common_lib", + "//test/mocks/http:http_mocks", + "//test/mocks/network:network_mocks", + "//test/mocks/router:router_mocks", + "//test/mocks/server:server_mocks", + "//test/mocks/upstream:upstream_mocks", + "//test/test_common:environment_lib", + "//test/test_common:simulated_time_system_lib", + "//test/test_common:utility_lib", + ], +) + envoy_cc_test( name = "header_formatter_test", srcs = ["header_formatter_test.cc"], diff --git a/test/common/router/upstream_request_test.cc b/test/common/router/upstream_request_test.cc new file mode 100644 index 0000000000000..5a445ed93519f --- /dev/null +++ b/test/common/router/upstream_request_test.cc @@ -0,0 +1,258 @@ +#include "common/buffer/buffer_impl.h" +#include "common/router/config_impl.h" +#include "common/router/router.h" +#include "common/router/upstream_request.h" + +#include "test/common/http/common.h" +#include "test/mocks/http/mocks.h" +#include "test/mocks/router/mocks.h" +#include "test/mocks/server/mocks.h" +#include "test/mocks/tcp/mocks.h" +#include "test/test_common/utility.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using testing::_; +using testing::AnyNumber; +using testing::NiceMock; +using testing::Return; +using testing::ReturnRef; + +namespace Envoy { +namespace Router { +namespace { + +class MockGenericConnPool : public GenericConnPool { + MOCK_METHOD(void, newStream, (GenericConnectionPoolCallbacks * request)); + MOCK_METHOD(bool, cancelAnyPendingRequest, ()); + MOCK_METHOD(absl::optional, protocol, (), (const)); +}; + +class MockGenericConnectionPoolCallbacks : public GenericConnectionPoolCallbacks { +public: + MOCK_METHOD(void, onPoolFailure, + (Http::ConnectionPool::PoolFailureReason reason, + absl::string_view transport_failure_reason, + Upstream::HostDescriptionConstSharedPtr host)); + MOCK_METHOD(void, onPoolReady, + (std::unique_ptr && upstream, + Upstream::HostDescriptionConstSharedPtr host, + const Network::Address::InstanceConstSharedPtr& upstream_local_address, + const StreamInfo::StreamInfo& info)); + MOCK_METHOD(UpstreamRequest*, upstreamRequest, ()); +}; + +class MockRouterFilterInterface : public RouterFilterInterface { +public: + MockRouterFilterInterface() + : config_("prefix.", context_, ShadowWriterPtr(new MockShadowWriter()), router_proto) { + auto cluster_info = new NiceMock(); + cluster_info->timeout_budget_stats_ = absl::nullopt; + cluster_info_.reset(cluster_info); + ON_CALL(*this, callbacks()).WillByDefault(Return(&callbacks_)); + ON_CALL(*this, config()).WillByDefault(ReturnRef(config_)); + ON_CALL(*this, cluster()).WillByDefault(Return(cluster_info_)); + ON_CALL(*this, upstreamRequests()).WillByDefault(ReturnRef(requests_)); + EXPECT_CALL(callbacks_.dispatcher_, setTrackedObject(_)).Times(AnyNumber()); + } + + MOCK_METHOD(void, onUpstream100ContinueHeaders, + (Http::ResponseHeaderMapPtr && headers, UpstreamRequest& upstream_request)); + MOCK_METHOD(void, onUpstreamHeaders, + (uint64_t response_code, Http::ResponseHeaderMapPtr&& headers, + UpstreamRequest& upstream_request, bool end_stream)); + MOCK_METHOD(void, onUpstreamData, + (Buffer::Instance & data, UpstreamRequest& upstream_request, bool end_stream)); + MOCK_METHOD(void, onUpstreamTrailers, + (Http::ResponseTrailerMapPtr && trailers, UpstreamRequest& upstream_request)); + MOCK_METHOD(void, onUpstreamMetadata, (Http::MetadataMapPtr && metadata_map)); + MOCK_METHOD(void, onUpstreamReset, + (Http::StreamResetReason reset_reason, absl::string_view transport_failure, + UpstreamRequest& upstream_request)); + MOCK_METHOD(void, onUpstreamHostSelected, (Upstream::HostDescriptionConstSharedPtr host)); + MOCK_METHOD(void, onPerTryTimeout, (UpstreamRequest & upstream_request)); + + MOCK_METHOD(Http::StreamDecoderFilterCallbacks*, callbacks, ()); + MOCK_METHOD(Upstream::ClusterInfoConstSharedPtr, cluster, ()); + MOCK_METHOD(FilterConfig&, config, ()); + MOCK_METHOD(FilterUtility::TimeoutData, timeout, ()); + MOCK_METHOD(Http::RequestHeaderMap*, downstreamHeaders, ()); + MOCK_METHOD(Http::RequestTrailerMap*, downstreamTrailers, ()); + MOCK_METHOD(bool, downstreamResponseStarted, (), (const)); + MOCK_METHOD(bool, downstreamEndStream, (), (const)); + MOCK_METHOD(uint32_t, attemptCount, (), (const)); + MOCK_METHOD(const VirtualCluster*, requestVcluster, (), (const)); + MOCK_METHOD(const RouteEntry*, routeEntry, (), (const)); + MOCK_METHOD(const std::list&, upstreamRequests, (), (const)); + MOCK_METHOD(const UpstreamRequest*, finalUpstreamRequest, (), (const)); + MOCK_METHOD(TimeSource&, timeSource, ()); + + NiceMock callbacks_; + + envoy::extensions::filters::http::router::v3::Router router_proto; + NiceMock context_; + FilterConfig config_; + Upstream::ClusterInfoConstSharedPtr cluster_info_; + std::list requests_; +}; + +class TcpConnPoolTest : public ::testing::Test { +public: + TcpConnPoolTest() + : conn_pool_(&mock_pool_), host_(std::make_shared>()) {} + + TcpConnPool conn_pool_; + Tcp::ConnectionPool::MockInstance mock_pool_; + MockGenericConnectionPoolCallbacks mock_generic_callbacks_; + std::shared_ptr> host_; + NiceMock cancellable_; +}; + +TEST_F(TcpConnPoolTest, Basic) { + NiceMock connection; + + EXPECT_CALL(mock_pool_, newConnection(_)).WillOnce(Return(&cancellable_)); + conn_pool_.newStream(&mock_generic_callbacks_); + + EXPECT_CALL(mock_generic_callbacks_, upstreamRequest()); + EXPECT_CALL(mock_generic_callbacks_, onPoolReady(_, _, _, _)); + auto data = std::make_unique>(); + EXPECT_CALL(*data, connection()).Times(AnyNumber()).WillRepeatedly(ReturnRef(connection)); + conn_pool_.onPoolReady(std::move(data), host_); +} + +TEST_F(TcpConnPoolTest, OnPoolFailure) { + EXPECT_CALL(mock_pool_, newConnection(_)).WillOnce(Return(&cancellable_)); + conn_pool_.newStream(&mock_generic_callbacks_); + + EXPECT_CALL(mock_generic_callbacks_, onPoolFailure(_, _, _)); + conn_pool_.onPoolFailure(Tcp::ConnectionPool::PoolFailureReason::LocalConnectionFailure, host_); + + // Make sure that the pool failure nulled out the pending request. + EXPECT_FALSE(conn_pool_.cancelAnyPendingRequest()); +} + +TEST_F(TcpConnPoolTest, Cancel) { + // Initially cancel should fail as there is no pending request. + EXPECT_FALSE(conn_pool_.cancelAnyPendingRequest()); + + EXPECT_CALL(mock_pool_, newConnection(_)).WillOnce(Return(&cancellable_)); + conn_pool_.newStream(&mock_generic_callbacks_); + + // Canceling should now return true as there was an active request. + EXPECT_TRUE(conn_pool_.cancelAnyPendingRequest()); + + // A second cancel should return false as there is not a pending request. + EXPECT_FALSE(conn_pool_.cancelAnyPendingRequest()); +} + +class TcpUpstreamTest : public ::testing::Test { +public: + TcpUpstreamTest() { + mock_router_filter_.requests_.push_back(std::make_unique( + mock_router_filter_, std::make_unique>())); + auto data = std::make_unique>(); + EXPECT_CALL(*data, connection()).Times(AnyNumber()).WillRepeatedly(ReturnRef(connection_)); + tcp_upstream_ = + std::make_unique(mock_router_filter_.requests_.front().get(), std::move(data)); + } + ~TcpUpstreamTest() { EXPECT_CALL(mock_router_filter_, config()).Times(AnyNumber()); } + +protected: + NiceMock connection_; + NiceMock mock_router_filter_; + Tcp::ConnectionPool::MockConnectionData* mock_connection_data_; + std::unique_ptr tcp_upstream_; + Http::TestRequestHeaderMapImpl request_{{":method", "CONNECT"}, + {":path", "/"}, + {":protocol", "bytestream"}, + {":scheme", "https"}, + {":authority", "host"}}; +}; + +TEST_F(TcpUpstreamTest, Basic) { + // Swallow the headers. + tcp_upstream_->encodeHeaders(request_, false); + + // Proxy the data. + EXPECT_CALL(connection_, write(BufferStringEqual("foo"), false)); + Buffer::OwnedImpl buffer("foo"); + tcp_upstream_->encodeData(buffer, false); + + // Metadata is swallowed. + Http::MetadataMapVector metadata_map_vector; + tcp_upstream_->encodeMetadata(metadata_map_vector); + + // On initial data payload, fake response headers, and forward data. + Buffer::OwnedImpl response1("bar"); + EXPECT_CALL(mock_router_filter_, onUpstreamHeaders(200, _, _, false)); + EXPECT_CALL(mock_router_filter_, onUpstreamData(BufferStringEqual("bar"), _, false)); + tcp_upstream_->onUpstreamData(response1, false); + + // On the next batch of payload there won't be additional headers. + Buffer::OwnedImpl response2("eep"); + EXPECT_CALL(mock_router_filter_, onUpstreamHeaders(_, _, _, _)).Times(0); + EXPECT_CALL(mock_router_filter_, onUpstreamData(BufferStringEqual("eep"), _, false)); + tcp_upstream_->onUpstreamData(response2, false); +} + +TEST_F(TcpUpstreamTest, TrailersEndStream) { + // Swallow the headers. + tcp_upstream_->encodeHeaders(request_, false); + + EXPECT_CALL(connection_, write(BufferStringEqual(""), true)); + Http::TestRequestTrailerMapImpl trailers{{"foo", "bar"}}; + tcp_upstream_->encodeTrailers(trailers); +} + +TEST_F(TcpUpstreamTest, HeaderEndStreamHalfClose) { + EXPECT_CALL(connection_, write(BufferStringEqual(""), true)); + tcp_upstream_->encodeHeaders(request_, true); +} + +TEST_F(TcpUpstreamTest, ReadDisable) { + EXPECT_CALL(connection_, readDisable(true)); + tcp_upstream_->readDisable(true); + + EXPECT_CALL(connection_, readDisable(false)); + tcp_upstream_->readDisable(false); + + // Once the connection is closed, don't touch it. + connection_.state_ = Network::Connection::State::Closed; + EXPECT_CALL(connection_, readDisable(_)).Times(0); + tcp_upstream_->readDisable(true); +} + +TEST_F(TcpUpstreamTest, UpstreamEvent) { + // Make sure upstream disconnects result in stream reset. + EXPECT_CALL(mock_router_filter_, + onUpstreamReset(Http::StreamResetReason::ConnectionTermination, "", _)); + tcp_upstream_->onEvent(Network::ConnectionEvent::RemoteClose); +} + +TEST_F(TcpUpstreamTest, Watermarks) { + EXPECT_CALL(mock_router_filter_, callbacks()).Times(AnyNumber()); + EXPECT_CALL(mock_router_filter_.callbacks_, onDecoderFilterAboveWriteBufferHighWatermark()); + tcp_upstream_->onAboveWriteBufferHighWatermark(); + + EXPECT_CALL(mock_router_filter_.callbacks_, onDecoderFilterBelowWriteBufferLowWatermark()); + tcp_upstream_->onBelowWriteBufferLowWatermark(); +} + +TEST(Errors, ConvertReason) { + EXPECT_EQ(Http::ConnectionPool::PoolFailureReason::Overflow, + TcpConnPool::convertReason(Tcp::ConnectionPool::PoolFailureReason::Overflow)); + EXPECT_EQ( + Http::ConnectionPool::PoolFailureReason::ConnectionFailure, + TcpConnPool::convertReason(Tcp::ConnectionPool::PoolFailureReason::LocalConnectionFailure)); + EXPECT_EQ( + Http::ConnectionPool::PoolFailureReason::ConnectionFailure, + TcpConnPool::convertReason(Tcp::ConnectionPool::PoolFailureReason::RemoteConnectionFailure)); + EXPECT_EQ(Http::ConnectionPool::PoolFailureReason::ConnectionFailure, + TcpConnPool::convertReason(Tcp::ConnectionPool::PoolFailureReason::Timeout)); +} + +} // namespace +} // namespace Router +} // namespace Envoy diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 9ed7b8ead74ba..634fa1df0b861 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -246,6 +246,7 @@ class MockVirtualHost : public VirtualHost { MOCK_METHOD(const RouteSpecificFilterConfig*, perFilterConfig, (const std::string&), (const)); MOCK_METHOD(bool, includeAttemptCountInRequest, (), (const)); MOCK_METHOD(bool, includeAttemptCountInResponse, (), (const)); + MOCK_METHOD(const absl::optional, proxyingConfig, (), (const)); MOCK_METHOD(Upstream::RetryPrioritySharedPtr, retryPriority, ()); MOCK_METHOD(Upstream::RetryHostPredicateSharedPtr, retryHostPredicate, ()); MOCK_METHOD(uint32_t, retryShadowBufferLimit, (), (const)); @@ -353,6 +354,7 @@ class MockRouteEntry : public RouteEntry { MOCK_METHOD(const RouteSpecificFilterConfig*, perFilterConfig, (const std::string&), (const)); MOCK_METHOD(bool, includeAttemptCountInRequest, (), (const)); MOCK_METHOD(bool, includeAttemptCountInResponse, (), (const)); + MOCK_METHOD(const absl::optional, proxyingConfig, (), (const)); MOCK_METHOD(const UpgradeMap&, upgradeMap, (), (const)); MOCK_METHOD(InternalRedirectAction, internalRedirectAction, (), (const)); MOCK_METHOD(uint32_t, maxInternalRedirects, (), (const)); From 7f134d1fe991e86e95d723b54dc641ea8163e118 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Thu, 2 Apr 2020 14:16:09 -0400 Subject: [PATCH 02/17] decapsulating *is* a word Signed-off-by: Alyssa Wilk --- api/envoy/api/v2/route/route_components.proto | 4 ++-- api/envoy/config/route/v3/route_components.proto | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/envoy/api/v2/route/route_components.proto b/api/envoy/api/v2/route/route_components.proto index 6b27a569cec15..8b266592be70c 100644 --- a/api/envoy/api/v2/route/route_components.proto +++ b/api/envoy/api/v2/route/route_components.proto @@ -189,8 +189,8 @@ message VirtualHost { google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18; // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for decapsulation - // of CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request + // Configuration for sending data upstream as a raw data payload. This is used for terminating + // CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing // response headers and forwarding the TCP response payload in the HTTP response body. ProxyingConfig proxying_config = 22; diff --git a/api/envoy/config/route/v3/route_components.proto b/api/envoy/config/route/v3/route_components.proto index 13e957906a186..78a03067a0656 100644 --- a/api/envoy/config/route/v3/route_components.proto +++ b/api/envoy/config/route/v3/route_components.proto @@ -188,8 +188,8 @@ message VirtualHost { google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18; // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for decapsulation - // of CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request + // Configuration for sending data upstream as a raw data payload. This is used for terminating + // CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing // response headers and forwarding the TCP response payload in the HTTP response body. ProxyingConfig proxying_config = 22; From f1fe9553b4010a3d54dbc898d33110c0e5c1cd4b Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Thu, 2 Apr 2020 16:03:46 -0400 Subject: [PATCH 03/17] sigh Signed-off-by: Alyssa Wilk --- .../envoy/api/v2/route/route_components.proto | 4 ++-- .../envoy/config/route/v3/route_components.proto | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/generated_api_shadow/envoy/api/v2/route/route_components.proto b/generated_api_shadow/envoy/api/v2/route/route_components.proto index 6b27a569cec15..8b266592be70c 100644 --- a/generated_api_shadow/envoy/api/v2/route/route_components.proto +++ b/generated_api_shadow/envoy/api/v2/route/route_components.proto @@ -189,8 +189,8 @@ message VirtualHost { google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18; // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for decapsulation - // of CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request + // Configuration for sending data upstream as a raw data payload. This is used for terminating + // CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing // response headers and forwarding the TCP response payload in the HTTP response body. ProxyingConfig proxying_config = 22; diff --git a/generated_api_shadow/envoy/config/route/v3/route_components.proto b/generated_api_shadow/envoy/config/route/v3/route_components.proto index a85879ae9245f..aba2f1f2b1052 100644 --- a/generated_api_shadow/envoy/config/route/v3/route_components.proto +++ b/generated_api_shadow/envoy/config/route/v3/route_components.proto @@ -194,8 +194,8 @@ message VirtualHost { google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18; // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for decapsulation - // of CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request + // Configuration for sending data upstream as a raw data payload. This is used for terminating + // CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing // response headers and forwarding the TCP response payload in the HTTP response body. ProxyingConfig proxying_config = 22; From 6ddcb009c34003a47cd9c6061f6e522c147d62d5 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 6 Apr 2020 10:15:21 -0400 Subject: [PATCH 04/17] reviewer comments Signed-off-by: Alyssa Wilk --- api/envoy/api/v2/route/route_components.proto | 6 +++--- api/envoy/config/route/v3/route_components.proto | 8 ++++---- .../envoy/api/v2/route/route_components.proto | 6 +++--- .../envoy/config/route/v3/route_components.proto | 8 ++++---- include/envoy/router/router.h | 8 ++++---- source/common/http/async_client_impl.cc | 4 ++++ source/common/http/async_client_impl.h | 10 ++++++++-- source/common/router/config_impl.h | 12 +++++------- source/common/router/router.cc | 2 +- test/mocks/router/mocks.cc | 2 ++ test/mocks/router/mocks.h | 6 ++++-- 11 files changed, 42 insertions(+), 30 deletions(-) diff --git a/api/envoy/api/v2/route/route_components.proto b/api/envoy/api/v2/route/route_components.proto index 8b266592be70c..d2b90c3e3d51e 100644 --- a/api/envoy/api/v2/route/route_components.proto +++ b/api/envoy/api/v2/route/route_components.proto @@ -51,8 +51,8 @@ message VirtualHost { // [#not-implemented-hide:] // Configuration for sending data upstream as a raw data payload. This is used for - // CONNECT requests, when translating HTTP body data to TCP payload. - message ProxyingConfig { + // CONNECT requests, when forwarding CONNECT payload as raw TCP. + message ProxyConfig { // TODO(alyssawilk) add proxy proto configuration here. } @@ -193,7 +193,7 @@ message VirtualHost { // CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing // response headers and forwarding the TCP response payload in the HTTP response body. - ProxyingConfig proxying_config = 22; + ProxyConfig proxying_config = 22; } // A filter-defined action type. diff --git a/api/envoy/config/route/v3/route_components.proto b/api/envoy/config/route/v3/route_components.proto index 78a03067a0656..3fa62c799e3f7 100644 --- a/api/envoy/config/route/v3/route_components.proto +++ b/api/envoy/config/route/v3/route_components.proto @@ -52,12 +52,12 @@ message VirtualHost { // [#not-implemented-hide:] // Configuration for sending data upstream as a raw data payload. This is used for - // CONNECT requests, when translating HTTP body data to TCP payload. - message ProxyingConfig { + // CONNECT requests, when forwarding CONNECT payload as raw TCP. + message ProxyConfig { // TODO(alyssawilk) add proxy proto configuration here. option (udpa.annotations.versioning).previous_message_type = - "envoy.api.v2.route.VirtualHost.ProxyingConfig"; + "envoy.api.v2.route.VirtualHost.ProxyConfig"; } reserved 9, 12; @@ -192,7 +192,7 @@ message VirtualHost { // CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing // response headers and forwarding the TCP response payload in the HTTP response body. - ProxyingConfig proxying_config = 22; + ProxyConfig proxying_config = 22; } // A filter-defined action type. diff --git a/generated_api_shadow/envoy/api/v2/route/route_components.proto b/generated_api_shadow/envoy/api/v2/route/route_components.proto index 8b266592be70c..d2b90c3e3d51e 100644 --- a/generated_api_shadow/envoy/api/v2/route/route_components.proto +++ b/generated_api_shadow/envoy/api/v2/route/route_components.proto @@ -51,8 +51,8 @@ message VirtualHost { // [#not-implemented-hide:] // Configuration for sending data upstream as a raw data payload. This is used for - // CONNECT requests, when translating HTTP body data to TCP payload. - message ProxyingConfig { + // CONNECT requests, when forwarding CONNECT payload as raw TCP. + message ProxyConfig { // TODO(alyssawilk) add proxy proto configuration here. } @@ -193,7 +193,7 @@ message VirtualHost { // CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing // response headers and forwarding the TCP response payload in the HTTP response body. - ProxyingConfig proxying_config = 22; + ProxyConfig proxying_config = 22; } // A filter-defined action type. diff --git a/generated_api_shadow/envoy/config/route/v3/route_components.proto b/generated_api_shadow/envoy/config/route/v3/route_components.proto index aba2f1f2b1052..74788f15c8cf7 100644 --- a/generated_api_shadow/envoy/config/route/v3/route_components.proto +++ b/generated_api_shadow/envoy/config/route/v3/route_components.proto @@ -52,12 +52,12 @@ message VirtualHost { // [#not-implemented-hide:] // Configuration for sending data upstream as a raw data payload. This is used for - // CONNECT requests, when translating HTTP body data to TCP payload. - message ProxyingConfig { + // CONNECT requests, when forwarding CONNECT payload as raw TCP. + message ProxyConfig { // TODO(alyssawilk) add proxy proto configuration here. option (udpa.annotations.versioning).previous_message_type = - "envoy.api.v2.route.VirtualHost.ProxyingConfig"; + "envoy.api.v2.route.VirtualHost.ProxyConfig"; } reserved 9; @@ -198,7 +198,7 @@ message VirtualHost { // CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing // response headers and forwarding the TCP response payload in the HTTP response body. - ProxyingConfig proxying_config = 22; + ProxyConfig proxying_config = 22; } // A filter-defined action type. diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index a6ff003ef2d4d..e5f3c6a69165a 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -484,8 +484,8 @@ class VirtualHost { * TCP upstream. * @return configuration for TCP proxying, if present. */ - using ProxyingConfig = envoy::config::route::v3::VirtualHost::ProxyingConfig; - virtual const absl::optional proxyingConfig() const PURE; + using ProxyConfig = envoy::config::route::v3::VirtualHost::ProxyConfig; + virtual const absl::optional& proxyConfig() const PURE; /** * @return uint32_t any route cap on bytes which should be buffered for shadowing or retries. @@ -835,8 +835,8 @@ class RouteEntry : public ResponseEntry { * TCP upstream. * @return configuration for TCP proxying, if present. */ - using ProxyingConfig = envoy::config::route::v3::VirtualHost::ProxyingConfig; - virtual const absl::optional proxyingConfig() const PURE; + using ProxyConfig = envoy::config::route::v3::VirtualHost::ProxyConfig; + virtual const absl::optional& proxyConfig() const PURE; using UpgradeMap = std::map; /** diff --git a/source/common/http/async_client_impl.cc b/source/common/http/async_client_impl.cc index cc5659da7885a..ae6c781a06f2b 100644 --- a/source/common/http/async_client_impl.cc +++ b/source/common/http/async_client_impl.cc @@ -24,12 +24,16 @@ const std::vector AsyncStreamImpl::RouteEntryImpl::shad const AsyncStreamImpl::NullVirtualHost AsyncStreamImpl::RouteEntryImpl::virtual_host_; const AsyncStreamImpl::NullRateLimitPolicy AsyncStreamImpl::NullVirtualHost::rate_limit_policy_; const AsyncStreamImpl::NullConfig AsyncStreamImpl::NullVirtualHost::route_configuration_; +const absl::optional + AsyncStreamImpl::NullVirtualHost::proxy_config_nullopt_; const std::multimap AsyncStreamImpl::RouteEntryImpl::opaque_config_; const envoy::config::core::v3::Metadata AsyncStreamImpl::RouteEntryImpl::metadata_; const Config::TypedMetadataImpl AsyncStreamImpl::RouteEntryImpl::typed_metadata_({}); const AsyncStreamImpl::NullPathMatchCriterion AsyncStreamImpl::RouteEntryImpl::path_match_criterion_; +const absl::optional + AsyncStreamImpl::RouteEntryImpl::proxy_config_nullopt_; const std::list AsyncStreamImpl::NullConfig::internal_only_headers_; AsyncClientImpl::AsyncClientImpl(Upstream::ClusterInfoConstSharedPtr cluster, diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index 42d5334105c7f..965c5aa671195 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -186,12 +186,15 @@ class AsyncStreamImpl : public AsyncClient::Stream, } bool includeAttemptCountInRequest() const override { return false; } bool includeAttemptCountInResponse() const override { return false; } - const absl::optional proxyingConfig() const override { return absl::nullopt; } + const absl::optional& proxyConfig() const override { + return proxy_config_nullopt_; + } uint32_t retryShadowBufferLimit() const override { return std::numeric_limits::max(); } static const NullRateLimitPolicy rate_limit_policy_; static const NullConfig route_configuration_; + static const absl::optional proxy_config_nullopt_; }; struct NullPathMatchCriterion : public Router::PathMatchCriterion { @@ -272,7 +275,9 @@ class AsyncStreamImpl : public AsyncClient::Stream, bool includeAttemptCountInRequest() const override { return false; } bool includeAttemptCountInResponse() const override { return false; } - const absl::optional proxyingConfig() const override { return absl::nullopt; } + const absl::optional& proxyConfig() const override { + return proxy_config_nullopt_; + } const Router::RouteEntry::UpgradeMap& upgradeMap() const override { return upgrade_map_; } Router::InternalRedirectAction internalRedirectAction() const override { return Router::InternalRedirectAction::PassThrough; @@ -294,6 +299,7 @@ class AsyncStreamImpl : public AsyncClient::Stream, Router::RouteEntry::UpgradeMap upgrade_map_; const std::string& cluster_name_; absl::optional timeout_; + static const absl::optional proxy_config_nullopt_; const std::string route_name_; }; diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index fda91084bd233..e478e641a2181 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -176,7 +176,7 @@ class VirtualHostImpl : public VirtualHost { const RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override; bool includeAttemptCountInRequest() const override { return include_attempt_count_in_request_; } bool includeAttemptCountInResponse() const override { return include_attempt_count_in_response_; } - const absl::optional proxyingConfig() const override { return proxying_config_; } + const absl::optional& proxyConfig() const override { return proxying_config_; } const absl::optional& retryPolicy() const { return retry_policy_; } @@ -233,7 +233,7 @@ class VirtualHostImpl : public VirtualHost { uint32_t retry_shadow_buffer_limit_{std::numeric_limits::max()}; const bool include_attempt_count_in_request_; const bool include_attempt_count_in_response_; - absl::optional proxying_config_; + absl::optional proxying_config_; absl::optional retry_policy_; absl::optional hedge_policy_; const CatchAllVirtualCluster virtual_cluster_catch_all_; @@ -467,9 +467,7 @@ class RouteEntryImplBase : public RouteEntry, bool includeAttemptCountInResponse() const override { return vhost_.includeAttemptCountInResponse(); } - const absl::optional proxyingConfig() const override { - return vhost_.proxyingConfig(); - } + const absl::optional& proxyConfig() const override { return vhost_.proxyConfig(); } const UpgradeMap& upgradeMap() const override { return upgrade_map_; } InternalRedirectAction internalRedirectAction() const override { return internal_redirect_action_; @@ -596,8 +594,8 @@ class RouteEntryImplBase : public RouteEntry, bool includeAttemptCountInResponse() const override { return parent_->includeAttemptCountInResponse(); } - const absl::optional proxyingConfig() const override { - return parent_->proxyingConfig(); + const absl::optional& proxyConfig() const override { + return parent_->proxyConfig(); } const UpgradeMap& upgradeMap() const override { return parent_->upgradeMap(); } InternalRedirectAction internalRedirectAction() const override { diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 1a19d3a74bfa2..f8470c4c400de 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -114,7 +114,7 @@ bool convertRequestHeadersForInternalRedirect(Http::RequestHeaderMap& downstream constexpr uint64_t TimeoutPrecisionFactor = 100; bool shouldProxy(Http::RequestHeaderMap& headers, const RouteEntry* route_entry) { - return route_entry->proxyingConfig().has_value() && + return route_entry->proxyConfig().has_value() && headers.Method()->value().getStringView() == Http::Headers::get().MethodValues.Connect; } diff --git a/test/mocks/router/mocks.cc b/test/mocks/router/mocks.cc index 5f4f8d487c1ef..8aee2be24969e 100644 --- a/test/mocks/router/mocks.cc +++ b/test/mocks/router/mocks.cc @@ -60,6 +60,7 @@ MockShadowWriter::~MockShadowWriter() = default; MockVirtualHost::MockVirtualHost() { ON_CALL(*this, name()).WillByDefault(ReturnRef(name_)); ON_CALL(*this, rateLimitPolicy()).WillByDefault(ReturnRef(rate_limit_policy_)); + ON_CALL(*this, proxyConfig()).WillByDefault(ReturnRef(proxy_config_)); } MockVirtualHost::~MockVirtualHost() = default; @@ -97,6 +98,7 @@ MockRouteEntry::MockRouteEntry() { ON_CALL(*this, upgradeMap()).WillByDefault(ReturnRef(upgrade_map_)); ON_CALL(*this, hedgePolicy()).WillByDefault(ReturnRef(hedge_policy_)); ON_CALL(*this, routeName()).WillByDefault(ReturnRef(route_name_)); + ON_CALL(*this, proxyConfig()).WillByDefault(ReturnRef(proxy_config_)); } MockRouteEntry::~MockRouteEntry() = default; diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 634fa1df0b861..0f6c090f604a1 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -246,7 +246,7 @@ class MockVirtualHost : public VirtualHost { MOCK_METHOD(const RouteSpecificFilterConfig*, perFilterConfig, (const std::string&), (const)); MOCK_METHOD(bool, includeAttemptCountInRequest, (), (const)); MOCK_METHOD(bool, includeAttemptCountInResponse, (), (const)); - MOCK_METHOD(const absl::optional, proxyingConfig, (), (const)); + MOCK_METHOD(const absl::optional&, proxyConfig, (), (const)); MOCK_METHOD(Upstream::RetryPrioritySharedPtr, retryPriority, ()); MOCK_METHOD(Upstream::RetryHostPredicateSharedPtr, retryHostPredicate, ()); MOCK_METHOD(uint32_t, retryShadowBufferLimit, (), (const)); @@ -261,6 +261,7 @@ class MockVirtualHost : public VirtualHost { mutable std::unique_ptr stat_name_; testing::NiceMock rate_limit_policy_; TestCorsPolicy cors_policy_; + absl::optional proxy_config_; }; class MockHashPolicy : public Http::HashPolicy { @@ -354,7 +355,7 @@ class MockRouteEntry : public RouteEntry { MOCK_METHOD(const RouteSpecificFilterConfig*, perFilterConfig, (const std::string&), (const)); MOCK_METHOD(bool, includeAttemptCountInRequest, (), (const)); MOCK_METHOD(bool, includeAttemptCountInResponse, (), (const)); - MOCK_METHOD(const absl::optional, proxyingConfig, (), (const)); + MOCK_METHOD(const absl::optional&, proxyConfig, (), (const)); MOCK_METHOD(const UpgradeMap&, upgradeMap, (), (const)); MOCK_METHOD(InternalRedirectAction, internalRedirectAction, (), (const)); MOCK_METHOD(uint32_t, maxInternalRedirects, (), (const)); @@ -376,6 +377,7 @@ class MockRouteEntry : public RouteEntry { testing::NiceMock path_match_criterion_; envoy::config::core::v3::Metadata metadata_; UpgradeMap upgrade_map_; + absl::optional proxy_config_; }; class MockDecorator : public Decorator { From 21986a928ab6fdf569f3c91f435d7dec50e90a09 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Wed, 8 Apr 2020 13:38:44 -0400 Subject: [PATCH 05/17] hidden docs Signed-off-by: Alyssa Wilk --- docs/root/intro/arch_overview/http/http.rst | 2 +- .../http/{websocket.rst => upgrades.rst} | 40 +++++++++++++++---- 2 files changed, 33 insertions(+), 9 deletions(-) rename docs/root/intro/arch_overview/http/{websocket.rst => upgrades.rst} (66%) diff --git a/docs/root/intro/arch_overview/http/http.rst b/docs/root/intro/arch_overview/http/http.rst index f5729560e0f6c..33b7ebffa6fa2 100644 --- a/docs/root/intro/arch_overview/http/http.rst +++ b/docs/root/intro/arch_overview/http/http.rst @@ -7,5 +7,5 @@ HTTP http_connection_management http_filters http_routing - websocket + upgrades http_proxy diff --git a/docs/root/intro/arch_overview/http/websocket.rst b/docs/root/intro/arch_overview/http/upgrades.rst similarity index 66% rename from docs/root/intro/arch_overview/http/websocket.rst rename to docs/root/intro/arch_overview/http/upgrades.rst index fa4e0b1f055d9..11797ddbcf8af 100644 --- a/docs/root/intro/arch_overview/http/websocket.rst +++ b/docs/root/intro/arch_overview/http/upgrades.rst @@ -1,19 +1,19 @@ .. _arch_overview_websocket: -WebSocket and HTTP upgrades +HTTP upgrades =========================== -Envoy Upgrade support is intended mainly for WebSocket but may be used for non-WebSocket -upgrades as well. Upgrades pass both the HTTP headers and the upgrade payload +Envoy Upgrade support is intended mainly for WebSocket and CONNECT support, but may be used for +arbitrary upgrades as well. Upgrades pass both the HTTP headers and the upgrade payload through an HTTP filter chain. One may configure the :ref:`upgrade_configs ` with or without custom filter chains. If only the :ref:`upgrade_type ` -is specified, both the upgrade headers, any request and response body, and WebSocket payload will +is specified, both the upgrade headers, any request and response body, and HTTP data payload will pass through the default HTTP filter chain. To avoid the use of HTTP-only filters for upgrade payload, one can set up custom :ref:`filters ` -for the given upgrade type, up to and including only using the router filter to send the WebSocket +for the given upgrade type, up to and including only using the router filter to send the HTTP data upstream. Upgrades can be enabled or disabled on a :ref:`per-route ` basis. @@ -32,12 +32,12 @@ laid out below, but custom filter chains can only be configured on a per-HttpCon | F | F | F | +-----------------------+-------------------------+-------------------+ -Note that the statistics for upgrades are all bundled together so WebSocket +Note that the statistics for upgrades are all bundled together so WebSocket and other upgrades :ref:`statistics ` are tracked by stats such as downstream_cx_upgrades_total and downstream_cx_upgrades_active -Handling HTTP/2 hops -^^^^^^^^^^^^^^^^^^^^ +Websocket over HTTP/2 hops +^^^^^^^^^^^^^^^^^^^^^^^^^^ While HTTP/2 support for WebSockets is off by default, Envoy does support tunneling WebSockets over HTTP/2 streams for deployments that prefer a uniform HTTP/2 mesh throughout; this enables, for example, @@ -61,3 +61,27 @@ a GET method on the final Envoy-Upstream hop. Note that the HTTP/2 upgrade path has very strict HTTP/1.1 compliance, so will not proxy WebSocket upgrade requests or responses with bodies. + +.. TODO(alyssawilk) unhide this when unhiding config +.. CONNECT support +.. ^^^^^^^^^^^^^^^ + +.. Envoy CONNECT support is off by default (Envoy will send an internall generated 403 in response to +.. CONNECT requests). CONNECT support can be enabled via the upgrade options described above, setting +.. the upgrade value to the special keyword "CONNECT". + +.. Envoy can handle CONNECT in one of two ways, either proxying the CONNECT headers through as if they +.. were any other request, and letting the upstream handle the CONNECT request, or by terminating the +.. CONNECT request, and forwarding the payload as raw TCP data. When CONNECT upgrade configuration is +.. set up, the default behavior is to proxy the CONNECT request. If termination is desired, this can +.. be accomplished by setting +.. the :ref:`per-route proxying_config ` or +.. :ref:`per-virtual-host proxying_config ` +.. If either is present for CONNECT requests, the router filter will strip the request headers, +.. and forward the HTTP payload upstream. On receipt of initial TCP data from upstream, the router +.. will synthesize 200 response headers, and then forward the TCP data as the HTTP response body. + +.. .. warning:: +.. This mode of CONNECT support can create major security holes if configured correctly, as the upstream +.. will be forwarded *unsanitized* headers if they are in the body payload. Please use with caution + From 028603165bf211674b60fbca4af39e72eb3e3bdf Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 13 Apr 2020 15:23:16 -0400 Subject: [PATCH 06/17] Matcher API? Signed-off-by: Alyssa Wilk --- api/envoy/api/v2/route/route_components.proto | 16 +------- .../config/route/v3/route_components.proto | 38 ++++++++++--------- 2 files changed, 21 insertions(+), 33 deletions(-) diff --git a/api/envoy/api/v2/route/route_components.proto b/api/envoy/api/v2/route/route_components.proto index 93dc72750c445..c890134414e55 100644 --- a/api/envoy/api/v2/route/route_components.proto +++ b/api/envoy/api/v2/route/route_components.proto @@ -34,7 +34,7 @@ option (udpa.annotations.file_status).package_version_status = FROZEN; // host header. This allows a single listener to service multiple top level domain path trees. Once // a virtual host is selected based on the domain, the routes are processed in order to see which // upstream cluster to route to or whether to perform a redirect. -// [#next-free-field: 23] +// [#next-free-field: 21] message VirtualHost { enum TlsRequirementType { // No TLS requirement for the virtual host. @@ -49,13 +49,6 @@ message VirtualHost { ALL = 2; } - // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for - // CONNECT requests, when forwarding CONNECT payload as raw TCP. - message ProxyConfig { - // TODO(alyssawilk) add proxy proto configuration here. - } - reserved 9; // The logical name of the virtual host. This is used when emitting certain @@ -187,13 +180,6 @@ message VirtualHost { // If set and a route-specific limit is not set, the bytes actually buffered will be the minimum // value of this and the listener per_connection_buffer_limit_bytes. google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18; - - // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for terminating - // CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request - // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing - // response headers and forwarding the TCP response payload in the HTTP response body. - ProxyConfig proxying_config = 22; } // A filter-defined action type. diff --git a/api/envoy/config/route/v3/route_components.proto b/api/envoy/config/route/v3/route_components.proto index f7f290ef355f4..ab0a0ed3d22bc 100644 --- a/api/envoy/config/route/v3/route_components.proto +++ b/api/envoy/config/route/v3/route_components.proto @@ -33,7 +33,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // host header. This allows a single listener to service multiple top level domain path trees. Once // a virtual host is selected based on the domain, the routes are processed in order to see which // upstream cluster to route to or whether to perform a redirect. -// [#next-free-field: 23] +// [#next-free-field: 21] message VirtualHost { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.VirtualHost"; @@ -50,16 +50,6 @@ message VirtualHost { ALL = 2; } - // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for - // CONNECT requests, when forwarding CONNECT payload as raw TCP. - message ProxyConfig { - // TODO(alyssawilk) add proxy proto configuration here. - - option (udpa.annotations.versioning).previous_message_type = - "envoy.api.v2.route.VirtualHost.ProxyConfig"; - } - reserved 9, 12; reserved "per_filter_config"; @@ -186,13 +176,6 @@ message VirtualHost { // If set and a route-specific limit is not set, the bytes actually buffered will be the minimum // value of this and the listener per_connection_buffer_limit_bytes. google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18; - - // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for terminating - // CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request - // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing - // response headers and forwarding the TCP response payload in the HTTP response body. - ProxyConfig proxying_config = 22; } // A filter-defined action type. @@ -405,6 +388,22 @@ message RouteMatch { google.protobuf.BoolValue validated = 2; } + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when forwarding CONNECT payload as raw TCP. + message ProxyConfig { + // TODO(alyssawilk) add proxy proto configuration here. + } + + // [#not-implemented-hide:] + // Configuration for proxying CONNECT requests. + message ConnectConfig { + // If this is present, CONNECT requests will be decapsulated with the + // CONNECT payload proxied upstream. If this is not present, it will result + // in a CONNECT request being sent upstream. + ProxyConfig proxy_config = 1; + } + reserved 5, 3; reserved "regex"; @@ -420,6 +419,9 @@ message RouteMatch { // exactly match the *:path* header once the query string is removed. string path = 2; + // [#not-implemented-hide:] + ConnectConfig connect_matcher = 3; + // If specified, the route is a regular expression rule meaning that the // regex must match the *:path* header once the query string is removed. The entire path // (without the query string) must match the regex. The rule will not match if only a From 771c1a660c62d214e1e2a3e1fda7c8d0cc549206 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 13 Apr 2020 15:25:46 -0400 Subject: [PATCH 07/17] WIP matcher API use Signed-off-by: Alyssa Wilk --- .../route/v4alpha/route_components.proto | 44 +++++++++++-------- .../envoy/api/v2/route/route_components.proto | 16 +------ .../config/route/v3/route_components.proto | 38 ++++++++-------- .../route/v4alpha/route_components.proto | 44 +++++++++++-------- include/envoy/router/router.h | 18 ++++---- source/common/http/async_client_impl.cc | 6 +-- source/common/http/async_client_impl.h | 12 ++--- source/common/router/config_impl.cc | 30 ++++++++++++- source/common/router/config_impl.h | 8 ++-- test/mocks/router/mocks.cc | 3 +- test/mocks/router/mocks.h | 5 +-- 11 files changed, 123 insertions(+), 101 deletions(-) diff --git a/api/envoy/config/route/v4alpha/route_components.proto b/api/envoy/config/route/v4alpha/route_components.proto index e03b6f69960dd..ce2b4d5a25962 100644 --- a/api/envoy/config/route/v4alpha/route_components.proto +++ b/api/envoy/config/route/v4alpha/route_components.proto @@ -33,7 +33,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // host header. This allows a single listener to service multiple top level domain path trees. Once // a virtual host is selected based on the domain, the routes are processed in order to see which // upstream cluster to route to or whether to perform a redirect. -// [#next-free-field: 23] +// [#next-free-field: 21] message VirtualHost { option (udpa.annotations.versioning).previous_message_type = "envoy.config.route.v3.VirtualHost"; @@ -50,16 +50,6 @@ message VirtualHost { ALL = 2; } - // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for - // CONNECT requests, when forwarding CONNECT payload as raw TCP. - message ProxyConfig { - // TODO(alyssawilk) add proxy proto configuration here. - - option (udpa.annotations.versioning).previous_message_type = - "envoy.config.route.v3.VirtualHost.ProxyConfig"; - } - reserved 9, 12; reserved "per_filter_config"; @@ -186,13 +176,6 @@ message VirtualHost { // If set and a route-specific limit is not set, the bytes actually buffered will be the minimum // value of this and the listener per_connection_buffer_limit_bytes. google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18; - - // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for terminating - // CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request - // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing - // response headers and forwarding the TCP response payload in the HTTP response body. - ProxyConfig proxying_config = 22; } // A filter-defined action type. @@ -406,6 +389,28 @@ message RouteMatch { google.protobuf.BoolValue validated = 2; } + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when forwarding CONNECT payload as raw TCP. + message ProxyConfig { + // TODO(alyssawilk) add proxy proto configuration here. + + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.route.v3.RouteMatch.ProxyConfig"; + } + + // [#not-implemented-hide:] + // Configuration for proxying CONNECT requests. + message ConnectConfig { + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.route.v3.RouteMatch.ConnectConfig"; + + // If this is present, CONNECT requests will be decapsulated with the + // CONNECT payload proxied upstream. If this is not present, it will result + // in a CONNECT request being sent upstream. + ProxyConfig proxy_config = 1; + } + reserved 5, 3; reserved "regex"; @@ -421,6 +426,9 @@ message RouteMatch { // exactly match the *:path* header once the query string is removed. string path = 2; + // [#not-implemented-hide:] + //message ConnectConfig = 3; + // If specified, the route is a regular expression rule meaning that the // regex must match the *:path* header once the query string is removed. The entire path // (without the query string) must match the regex. The rule will not match if only a diff --git a/generated_api_shadow/envoy/api/v2/route/route_components.proto b/generated_api_shadow/envoy/api/v2/route/route_components.proto index 93dc72750c445..c890134414e55 100644 --- a/generated_api_shadow/envoy/api/v2/route/route_components.proto +++ b/generated_api_shadow/envoy/api/v2/route/route_components.proto @@ -34,7 +34,7 @@ option (udpa.annotations.file_status).package_version_status = FROZEN; // host header. This allows a single listener to service multiple top level domain path trees. Once // a virtual host is selected based on the domain, the routes are processed in order to see which // upstream cluster to route to or whether to perform a redirect. -// [#next-free-field: 23] +// [#next-free-field: 21] message VirtualHost { enum TlsRequirementType { // No TLS requirement for the virtual host. @@ -49,13 +49,6 @@ message VirtualHost { ALL = 2; } - // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for - // CONNECT requests, when forwarding CONNECT payload as raw TCP. - message ProxyConfig { - // TODO(alyssawilk) add proxy proto configuration here. - } - reserved 9; // The logical name of the virtual host. This is used when emitting certain @@ -187,13 +180,6 @@ message VirtualHost { // If set and a route-specific limit is not set, the bytes actually buffered will be the minimum // value of this and the listener per_connection_buffer_limit_bytes. google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18; - - // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for terminating - // CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request - // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing - // response headers and forwarding the TCP response payload in the HTTP response body. - ProxyConfig proxying_config = 22; } // A filter-defined action type. diff --git a/generated_api_shadow/envoy/config/route/v3/route_components.proto b/generated_api_shadow/envoy/config/route/v3/route_components.proto index 611b88bf462fb..16b529cf90d85 100644 --- a/generated_api_shadow/envoy/config/route/v3/route_components.proto +++ b/generated_api_shadow/envoy/config/route/v3/route_components.proto @@ -33,7 +33,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // host header. This allows a single listener to service multiple top level domain path trees. Once // a virtual host is selected based on the domain, the routes are processed in order to see which // upstream cluster to route to or whether to perform a redirect. -// [#next-free-field: 23] +// [#next-free-field: 21] message VirtualHost { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.VirtualHost"; @@ -50,16 +50,6 @@ message VirtualHost { ALL = 2; } - // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for - // CONNECT requests, when forwarding CONNECT payload as raw TCP. - message ProxyConfig { - // TODO(alyssawilk) add proxy proto configuration here. - - option (udpa.annotations.versioning).previous_message_type = - "envoy.api.v2.route.VirtualHost.ProxyConfig"; - } - reserved 9; // The logical name of the virtual host. This is used when emitting certain @@ -185,13 +175,6 @@ message VirtualHost { // value of this and the listener per_connection_buffer_limit_bytes. google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18; - // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for terminating - // CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request - // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing - // response headers and forwarding the TCP response payload in the HTTP response body. - ProxyConfig proxying_config = 22; - map hidden_envoy_deprecated_per_filter_config = 12 [deprecated = true]; } @@ -408,6 +391,22 @@ message RouteMatch { google.protobuf.BoolValue validated = 2; } + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when forwarding CONNECT payload as raw TCP. + message ProxyConfig { + // TODO(alyssawilk) add proxy proto configuration here. + } + + // [#not-implemented-hide:] + // Configuration for proxying CONNECT requests. + message ConnectConfig { + // If this is present, CONNECT requests will be decapsulated with the + // CONNECT payload proxied upstream. If this is not present, it will result + // in a CONNECT request being sent upstream. + ProxyConfig proxy_config = 1; + } + reserved 5; // If specified, the route is a prefix rule meaning that the prefix must @@ -418,6 +417,9 @@ message RouteMatch { // exactly match the *:path* header once the query string is removed. core.v3.RuntimeFractionalPercent runtime_fraction = 9; + // [#not-implemented-hide:] + //message ConnectConfig = 3; + // If specified, the route is a regular expression rule meaning that the // regex must match the *:path* header once the query string is removed. The entire path // (without the query string) must match the regex. The rule will not match if only a diff --git a/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto b/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto index e03b6f69960dd..ce2b4d5a25962 100644 --- a/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto +++ b/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto @@ -33,7 +33,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // host header. This allows a single listener to service multiple top level domain path trees. Once // a virtual host is selected based on the domain, the routes are processed in order to see which // upstream cluster to route to or whether to perform a redirect. -// [#next-free-field: 23] +// [#next-free-field: 21] message VirtualHost { option (udpa.annotations.versioning).previous_message_type = "envoy.config.route.v3.VirtualHost"; @@ -50,16 +50,6 @@ message VirtualHost { ALL = 2; } - // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for - // CONNECT requests, when forwarding CONNECT payload as raw TCP. - message ProxyConfig { - // TODO(alyssawilk) add proxy proto configuration here. - - option (udpa.annotations.versioning).previous_message_type = - "envoy.config.route.v3.VirtualHost.ProxyConfig"; - } - reserved 9, 12; reserved "per_filter_config"; @@ -186,13 +176,6 @@ message VirtualHost { // If set and a route-specific limit is not set, the bytes actually buffered will be the minimum // value of this and the listener per_connection_buffer_limit_bytes. google.protobuf.UInt32Value per_request_buffer_limit_bytes = 18; - - // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for terminating - // CONNECT requests. If this message is present, the HTTP body of incoming CONNECT request - // will be proxied upstream as raw TCP. Upstream TCP payload will result in Envoy synthesizing - // response headers and forwarding the TCP response payload in the HTTP response body. - ProxyConfig proxying_config = 22; } // A filter-defined action type. @@ -406,6 +389,28 @@ message RouteMatch { google.protobuf.BoolValue validated = 2; } + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when forwarding CONNECT payload as raw TCP. + message ProxyConfig { + // TODO(alyssawilk) add proxy proto configuration here. + + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.route.v3.RouteMatch.ProxyConfig"; + } + + // [#not-implemented-hide:] + // Configuration for proxying CONNECT requests. + message ConnectConfig { + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.route.v3.RouteMatch.ConnectConfig"; + + // If this is present, CONNECT requests will be decapsulated with the + // CONNECT payload proxied upstream. If this is not present, it will result + // in a CONNECT request being sent upstream. + ProxyConfig proxy_config = 1; + } + reserved 5, 3; reserved "regex"; @@ -421,6 +426,9 @@ message RouteMatch { // exactly match the *:path* header once the query string is removed. string path = 2; + // [#not-implemented-hide:] + //message ConnectConfig = 3; + // If specified, the route is a regular expression rule meaning that the // regex must match the *:path* header once the query string is removed. The entire path // (without the query string) must match the regex. The rule will not match if only a diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index e5f3c6a69165a..d028dcf5582bc 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -484,7 +484,7 @@ class VirtualHost { * TCP upstream. * @return configuration for TCP proxying, if present. */ - using ProxyConfig = envoy::config::route::v3::VirtualHost::ProxyConfig; + using ProxyConfig = envoy::config::route::v3::RouteMatch::ProxyConfig; virtual const absl::optional& proxyConfig() const PURE; /** @@ -829,15 +829,6 @@ class RouteEntry : public ResponseEntry { */ virtual bool includeAttemptCountInResponse() const PURE; - /** - * If present, this indicates that CONNECT requests on this route should be converted - * to TCP proxying, i.e. the payload of the HTTP body should be sent as raw - * TCP upstream. - * @return configuration for TCP proxying, if present. - */ - using ProxyConfig = envoy::config::route::v3::VirtualHost::ProxyConfig; - virtual const absl::optional& proxyConfig() const PURE; - using UpgradeMap = std::map; /** * @return a map of route-specific upgrades to their enabled/disabled status. @@ -965,6 +956,13 @@ class Route { template const Derived* perFilterConfigTyped(const std::string& name) const { return dynamic_cast(perFilterConfig(name)); } + + /** + * If present, this indicates that CONNECT requests are alowed on this route. + */ + using ConnectConfig = envoy::config::route::v3::RouteMatch::ConnectConfig; + using ProxyConfig = envoy::config::route::v3::RouteMatch::ProxyConfig; + virtual const absl::optional& connectConfig() const PURE; }; using RouteConstSharedPtr = std::shared_ptr; diff --git a/source/common/http/async_client_impl.cc b/source/common/http/async_client_impl.cc index ae6c781a06f2b..440aad35bd8c3 100644 --- a/source/common/http/async_client_impl.cc +++ b/source/common/http/async_client_impl.cc @@ -24,16 +24,14 @@ const std::vector AsyncStreamImpl::RouteEntryImpl::shad const AsyncStreamImpl::NullVirtualHost AsyncStreamImpl::RouteEntryImpl::virtual_host_; const AsyncStreamImpl::NullRateLimitPolicy AsyncStreamImpl::NullVirtualHost::rate_limit_policy_; const AsyncStreamImpl::NullConfig AsyncStreamImpl::NullVirtualHost::route_configuration_; -const absl::optional - AsyncStreamImpl::NullVirtualHost::proxy_config_nullopt_; const std::multimap AsyncStreamImpl::RouteEntryImpl::opaque_config_; const envoy::config::core::v3::Metadata AsyncStreamImpl::RouteEntryImpl::metadata_; const Config::TypedMetadataImpl AsyncStreamImpl::RouteEntryImpl::typed_metadata_({}); const AsyncStreamImpl::NullPathMatchCriterion AsyncStreamImpl::RouteEntryImpl::path_match_criterion_; -const absl::optional - AsyncStreamImpl::RouteEntryImpl::proxy_config_nullopt_; +const absl::optional + AsyncStreamImpl::RouteEntryImpl::connect_config_nullopt_; const std::list AsyncStreamImpl::NullConfig::internal_only_headers_; AsyncClientImpl::AsyncClientImpl(Upstream::ClusterInfoConstSharedPtr cluster, diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index 965c5aa671195..143fd8c29541c 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -186,15 +186,11 @@ class AsyncStreamImpl : public AsyncClient::Stream, } bool includeAttemptCountInRequest() const override { return false; } bool includeAttemptCountInResponse() const override { return false; } - const absl::optional& proxyConfig() const override { - return proxy_config_nullopt_; - } uint32_t retryShadowBufferLimit() const override { return std::numeric_limits::max(); } static const NullRateLimitPolicy rate_limit_policy_; static const NullConfig route_configuration_; - static const absl::optional proxy_config_nullopt_; }; struct NullPathMatchCriterion : public Router::PathMatchCriterion { @@ -272,12 +268,12 @@ class AsyncStreamImpl : public AsyncClient::Stream, const Router::RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override { return nullptr; } + const absl::optional& connectConfig() const override { + return connect_config_nullopt_; + } bool includeAttemptCountInRequest() const override { return false; } bool includeAttemptCountInResponse() const override { return false; } - const absl::optional& proxyConfig() const override { - return proxy_config_nullopt_; - } const Router::RouteEntry::UpgradeMap& upgradeMap() const override { return upgrade_map_; } Router::InternalRedirectAction internalRedirectAction() const override { return Router::InternalRedirectAction::PassThrough; @@ -299,7 +295,7 @@ class AsyncStreamImpl : public AsyncClient::Stream, Router::RouteEntry::UpgradeMap upgrade_map_; const std::string& cluster_name_; absl::optional timeout_; - static const absl::optional proxy_config_nullopt_; + static const absl::optional connect_config_nullopt_; const std::string route_name_; }; diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 801cc0e00ad43..70a1396ef7a73 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -59,6 +59,8 @@ convertInternalRedirectAction(const envoy::config::route::v3::RouteAction& route const std::string DEPRECATED_ROUTER_NAME = "envoy.router"; +bool isConnect(const Http::RequestHeaderMap& headers) { + return headers.Method() && headers.Method()->value().getStringView() == Http::Headers::get().MethodValues.Connect; } // namespace std::string SslRedirector::newPath(const Http::RequestHeaderMap& headers) const { @@ -923,6 +925,28 @@ RouteConstSharedPtr RegexRouteEntryImpl::matches(const Http::RequestHeaderMap& h return nullptr; } +ConnectRouteEntryImpl::RegexRouteEntryImpl( + const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route, + Server::Configuration::ServerFactoryContext& factory_context, + ProtobufMessage::ValidationVisitor& validator) + : RouteEntryImplBase(vhost, route, factory_context, validator) { + connect_config_ = route.match().connect_matcher(); +} + +void ConnectRouteEntryImpl::rewritePathHeader(Http::RequestHeaderMap& headers, + bool insert_envoy_original_path) const { + finalizePathHeader(headers, prefix_, insert_envoy_original_path); +} + +RouteConstSharedPtr ConnectRouteEntryImpl::matches(const Http::RequestHeaderMap& headers, + const StreamInfo::StreamInfo& stream_info, + uint64_t random_value) const { + if (isConnect(headers)) { + return clusterEntry(headers, random_value); + } + return nullptr; +} + VirtualHostImpl::VirtualHostImpl(const envoy::config::route::v3::VirtualHost& virtual_host, const ConfigImpl& global_route_config, Server::Configuration::ServerFactoryContext& factory_context, @@ -982,6 +1006,10 @@ VirtualHostImpl::VirtualHostImpl(const envoy::config::route::v3::VirtualHost& vi routes_.emplace_back(new RegexRouteEntryImpl(*this, route, factory_context, validator)); break; } + case envoy::config::route::v3::RouteMatch::PathSpecifierCase::kConnectMatcher: { + // FIXME; + break; + } case envoy::config::route::v3::RouteMatch::PathSpecifierCase::PATH_SPECIFIER_NOT_SET: NOT_REACHED_GCOVR_EXCL_LINE; } @@ -1125,7 +1153,7 @@ RouteConstSharedPtr VirtualHostImpl::getRouteFromEntries(const Http::RequestHead // Check for a route that matches the request. for (const RouteEntryImplBaseConstSharedPtr& route : routes_) { RouteConstSharedPtr route_entry = route->matches(headers, stream_info, random_value); - if (nullptr != route_entry) { + if (nullptr != route_entry && (isConnect(headers) || route_entry->connectConfig.has_value()) { return route_entry; } } diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index e478e641a2181..7d244e98d30ca 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -176,7 +176,6 @@ class VirtualHostImpl : public VirtualHost { const RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override; bool includeAttemptCountInRequest() const override { return include_attempt_count_in_request_; } bool includeAttemptCountInResponse() const override { return include_attempt_count_in_response_; } - const absl::optional& proxyConfig() const override { return proxying_config_; } const absl::optional& retryPolicy() const { return retry_policy_; } @@ -233,7 +232,6 @@ class VirtualHostImpl : public VirtualHost { uint32_t retry_shadow_buffer_limit_{std::numeric_limits::max()}; const bool include_attempt_count_in_request_; const bool include_attempt_count_in_response_; - absl::optional proxying_config_; absl::optional retry_policy_; absl::optional hedge_policy_; const CatchAllVirtualCluster virtual_cluster_catch_all_; @@ -467,7 +465,7 @@ class RouteEntryImplBase : public RouteEntry, bool includeAttemptCountInResponse() const override { return vhost_.includeAttemptCountInResponse(); } - const absl::optional& proxyConfig() const override { return vhost_.proxyConfig(); } + const absl::optional& connectConfig() const override { return connect_config_; } const UpgradeMap& upgradeMap() const override { return upgrade_map_; } InternalRedirectAction internalRedirectAction() const override { return internal_redirect_action_; @@ -497,6 +495,8 @@ class RouteEntryImplBase : public RouteEntry, std::string regex_rewrite_substitution_; const std::string host_rewrite_; bool include_vh_rate_limits_; + absl::optional connect_config_; + RouteConstSharedPtr clusterEntry(const Http::HeaderMap& headers, uint64_t random_value) const; @@ -594,7 +594,7 @@ class RouteEntryImplBase : public RouteEntry, bool includeAttemptCountInResponse() const override { return parent_->includeAttemptCountInResponse(); } - const absl::optional& proxyConfig() const override { + const absl::optional proxyConfig() const override { return parent_->proxyConfig(); } const UpgradeMap& upgradeMap() const override { return parent_->upgradeMap(); } diff --git a/test/mocks/router/mocks.cc b/test/mocks/router/mocks.cc index 8aee2be24969e..32e5ce7ba79eb 100644 --- a/test/mocks/router/mocks.cc +++ b/test/mocks/router/mocks.cc @@ -60,7 +60,6 @@ MockShadowWriter::~MockShadowWriter() = default; MockVirtualHost::MockVirtualHost() { ON_CALL(*this, name()).WillByDefault(ReturnRef(name_)); ON_CALL(*this, rateLimitPolicy()).WillByDefault(ReturnRef(rate_limit_policy_)); - ON_CALL(*this, proxyConfig()).WillByDefault(ReturnRef(proxy_config_)); } MockVirtualHost::~MockVirtualHost() = default; @@ -98,7 +97,7 @@ MockRouteEntry::MockRouteEntry() { ON_CALL(*this, upgradeMap()).WillByDefault(ReturnRef(upgrade_map_)); ON_CALL(*this, hedgePolicy()).WillByDefault(ReturnRef(hedge_policy_)); ON_CALL(*this, routeName()).WillByDefault(ReturnRef(route_name_)); - ON_CALL(*this, proxyConfig()).WillByDefault(ReturnRef(proxy_config_)); + ON_CALL(*this, connectConfig()).WillByDefault(ReturnRef(connect_config_)); } MockRouteEntry::~MockRouteEntry() = default; diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 0f6c090f604a1..6f3d05de10ada 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -246,7 +246,6 @@ class MockVirtualHost : public VirtualHost { MOCK_METHOD(const RouteSpecificFilterConfig*, perFilterConfig, (const std::string&), (const)); MOCK_METHOD(bool, includeAttemptCountInRequest, (), (const)); MOCK_METHOD(bool, includeAttemptCountInResponse, (), (const)); - MOCK_METHOD(const absl::optional&, proxyConfig, (), (const)); MOCK_METHOD(Upstream::RetryPrioritySharedPtr, retryPriority, ()); MOCK_METHOD(Upstream::RetryHostPredicateSharedPtr, retryHostPredicate, ()); MOCK_METHOD(uint32_t, retryShadowBufferLimit, (), (const)); @@ -261,7 +260,7 @@ class MockVirtualHost : public VirtualHost { mutable std::unique_ptr stat_name_; testing::NiceMock rate_limit_policy_; TestCorsPolicy cors_policy_; - absl::optional proxy_config_; + absl::optional connect_config_; }; class MockHashPolicy : public Http::HashPolicy { @@ -355,7 +354,7 @@ class MockRouteEntry : public RouteEntry { MOCK_METHOD(const RouteSpecificFilterConfig*, perFilterConfig, (const std::string&), (const)); MOCK_METHOD(bool, includeAttemptCountInRequest, (), (const)); MOCK_METHOD(bool, includeAttemptCountInResponse, (), (const)); - MOCK_METHOD(const absl::optional&, proxyConfig, (), (const)); + MOCK_METHOD(const absl::optional&, connectConfig, (), (const)); MOCK_METHOD(const UpgradeMap&, upgradeMap, (), (const)); MOCK_METHOD(InternalRedirectAction, internalRedirectAction, (), (const)); MOCK_METHOD(uint32_t, maxInternalRedirects, (), (const)); From b83f9bda342cafdbe27e948d03c15b5688f80f33 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Wed, 15 Apr 2020 17:30:48 -0400 Subject: [PATCH 08/17] API WIP Signed-off-by: Alyssa Wilk --- .../config/route/v3/route_components.proto | 34 ++++++----- .../route/v4alpha/route_components.proto | 43 +++++++------- .../config/route/v3/route_components.proto | 56 ++++++++++--------- .../route/v4alpha/route_components.proto | 43 +++++++------- include/envoy/router/router.h | 25 +++------ source/common/http/async_client_impl.cc | 4 +- source/common/router/config_impl.cc | 19 +++---- source/common/router/config_impl.h | 25 ++++++++- source/common/router/router.cc | 2 +- test/mocks/router/mocks.h | 3 +- 10 files changed, 140 insertions(+), 114 deletions(-) diff --git a/api/envoy/config/route/v3/route_components.proto b/api/envoy/config/route/v3/route_components.proto index ab0a0ed3d22bc..8b60c3e1c4886 100644 --- a/api/envoy/config/route/v3/route_components.proto +++ b/api/envoy/config/route/v3/route_components.proto @@ -366,7 +366,7 @@ message WeightedCluster { string runtime_key_prefix = 2; } -// [#next-free-field: 12] +// [#next-free-field: 13] message RouteMatch { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.RouteMatch"; @@ -389,19 +389,8 @@ message RouteMatch { } // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for - // CONNECT requests, when forwarding CONNECT payload as raw TCP. - message ProxyConfig { - // TODO(alyssawilk) add proxy proto configuration here. - } - - // [#not-implemented-hide:] - // Configuration for proxying CONNECT requests. - message ConnectConfig { - // If this is present, CONNECT requests will be decapsulated with the - // CONNECT payload proxied upstream. If this is not present, it will result - // in a CONNECT request being sent upstream. - ProxyConfig proxy_config = 1; + // An extensible message for matching CONNECT requests. + message ConnectMatcher { } reserved 5, 3; @@ -420,7 +409,11 @@ message RouteMatch { string path = 2; // [#not-implemented-hide:] - ConnectConfig connect_matcher = 3; + // If this is used as the matcher, the matcher will only match CONNECT requests. + // Note that this will not match HTTP/2 upgrade-style CONNECT requests + // (WebSocket and the like) as they are normalized in Envoy HTTP/1.1 style + // upgrades. + ConnectMatcher connect_matcher = 12; // If specified, the route is a regular expression rule meaning that the // regex must match the *:path* header once the query string is removed. The entire path @@ -720,6 +713,15 @@ message RouteAction { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.RouteAction.UpgradeConfig"; + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when forwarding CONNECT payload as raw TCP. + // This may ONLY be used on routes which are selected via a ConnectMatcher + // and is only valid if the upgrade type is "CONNECT" + message ConnectConfig { + // TODO(alyssawilk) add proxy proto configuration here. + } + // The case-insensitive name of this upgrade, e.g. "websocket". // For each upgrade type present in upgrade_configs, requests with // Upgrade: [upgrade_type] will be proxied upstream. @@ -728,6 +730,8 @@ message RouteAction { // Determines if upgrades are available on this route. Defaults to true. google.protobuf.BoolValue enabled = 2; + + ConnectConfig connect_config = 3; } reserved 12, 18, 19, 16, 22, 21, 10; diff --git a/api/envoy/config/route/v4alpha/route_components.proto b/api/envoy/config/route/v4alpha/route_components.proto index ce2b4d5a25962..719d860aea69d 100644 --- a/api/envoy/config/route/v4alpha/route_components.proto +++ b/api/envoy/config/route/v4alpha/route_components.proto @@ -367,7 +367,7 @@ message WeightedCluster { string runtime_key_prefix = 2; } -// [#next-free-field: 12] +// [#next-free-field: 13] message RouteMatch { option (udpa.annotations.versioning).previous_message_type = "envoy.config.route.v3.RouteMatch"; @@ -390,25 +390,10 @@ message RouteMatch { } // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for - // CONNECT requests, when forwarding CONNECT payload as raw TCP. - message ProxyConfig { - // TODO(alyssawilk) add proxy proto configuration here. - + // An extensible message for matching CONNECT requests. + message ConnectMatcher { option (udpa.annotations.versioning).previous_message_type = - "envoy.config.route.v3.RouteMatch.ProxyConfig"; - } - - // [#not-implemented-hide:] - // Configuration for proxying CONNECT requests. - message ConnectConfig { - option (udpa.annotations.versioning).previous_message_type = - "envoy.config.route.v3.RouteMatch.ConnectConfig"; - - // If this is present, CONNECT requests will be decapsulated with the - // CONNECT payload proxied upstream. If this is not present, it will result - // in a CONNECT request being sent upstream. - ProxyConfig proxy_config = 1; + "envoy.config.route.v3.RouteMatch.ConnectMatcher"; } reserved 5, 3; @@ -427,7 +412,11 @@ message RouteMatch { string path = 2; // [#not-implemented-hide:] - //message ConnectConfig = 3; + // If this is used as the matcher, the matcher will only match CONNECT requests. + // Note that this will not match HTTP/2 upgrade-style CONNECT requests + // (WebSocket and the like) as they are normalized in Envoy HTTP/1.1 style + // upgrades. + ConnectMatcher connect_matcher = 12; // If specified, the route is a regular expression rule meaning that the // regex must match the *:path* header once the query string is removed. The entire path @@ -727,6 +716,18 @@ message RouteAction { option (udpa.annotations.versioning).previous_message_type = "envoy.config.route.v3.RouteAction.UpgradeConfig"; + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when forwarding CONNECT payload as raw TCP. + // This may ONLY be used on routes which are selected via a ConnectMatcher + // and is only valid if the upgrade type is "CONNECT" + message ConnectConfig { + // TODO(alyssawilk) add proxy proto configuration here. + + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.route.v3.RouteAction.UpgradeConfig.ConnectConfig"; + } + // The case-insensitive name of this upgrade, e.g. "websocket". // For each upgrade type present in upgrade_configs, requests with // Upgrade: [upgrade_type] will be proxied upstream. @@ -735,6 +736,8 @@ message RouteAction { // Determines if upgrades are available on this route. Defaults to true. google.protobuf.BoolValue enabled = 2; + + ConnectConfig connect_config = 3; } reserved 12, 18, 19, 16, 22, 21, 10; diff --git a/generated_api_shadow/envoy/config/route/v3/route_components.proto b/generated_api_shadow/envoy/config/route/v3/route_components.proto index 16b529cf90d85..7c0ba5beef096 100644 --- a/generated_api_shadow/envoy/config/route/v3/route_components.proto +++ b/generated_api_shadow/envoy/config/route/v3/route_components.proto @@ -369,7 +369,7 @@ message WeightedCluster { string runtime_key_prefix = 2; } -// [#next-free-field: 12] +// [#next-free-field: 13] message RouteMatch { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.RouteMatch"; @@ -392,19 +392,8 @@ message RouteMatch { } // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for - // CONNECT requests, when forwarding CONNECT payload as raw TCP. - message ProxyConfig { - // TODO(alyssawilk) add proxy proto configuration here. - } - - // [#not-implemented-hide:] - // Configuration for proxying CONNECT requests. - message ConnectConfig { - // If this is present, CONNECT requests will be decapsulated with the - // CONNECT payload proxied upstream. If this is not present, it will result - // in a CONNECT request being sent upstream. - ProxyConfig proxy_config = 1; + // An extensible message for matching CONNECT requests. + message ConnectMatcher { } reserved 5; @@ -418,7 +407,11 @@ message RouteMatch { core.v3.RuntimeFractionalPercent runtime_fraction = 9; // [#not-implemented-hide:] - //message ConnectConfig = 3; + // If this is used as the matcher, the matcher will only match CONNECT requests. + // Note that this will not match HTTP/2 upgrade-style CONNECT requests + // (WebSocket and the like) as they are normalized in Envoy HTTP/1.1 style + // upgrades. + repeated HeaderMatcher headers = 6; // If specified, the route is a regular expression rule meaning that the // regex must match the *:path* header once the query string is removed. The entire path @@ -432,11 +425,11 @@ message RouteMatch { // path_specifier entirely and just rely on a set of header matchers which can already match // on :path, etc. The issue with that is it is unclear how to generically deal with query string // stripping. This needs more thought.] - repeated HeaderMatcher headers = 6; + repeated QueryParameterMatcher query_parameters = 7; // Indicates that prefix/path matching should be case insensitive. The default // is true. - repeated QueryParameterMatcher query_parameters = 7; + GrpcRouteMatchOptions grpc = 8; // Indicates that the route should additionally match on a runtime key. Every time the route // is considered for a match, it must also fall under the percentage of matches indicated by @@ -454,29 +447,29 @@ message RouteMatch { // integer with the assumption that the value is an integral percentage out of 100. For // instance, a runtime key lookup returning the value "42" would parse as a FractionalPercent // whose numerator is 42 and denominator is HUNDRED. This preserves legacy semantics. - GrpcRouteMatchOptions grpc = 8; - - // Specifies a set of headers that the route should match on. The router will - // check the request’s headers against all the specified headers in the route - // config. A match will happen if all the headers in the route are present in - // the request with the same values (or based on presence if the value field - // is not in the config). TlsContextMatchOptions tls_context = 11; oneof path_specifier { option (validate.required) = true; + // Specifies a set of headers that the route should match on. The router will + // check the request’s headers against all the specified headers in the route + // config. A match will happen if all the headers in the route are present in + // the request with the same values (or based on presence if the value field + // is not in the config). + string prefix = 1; + // Specifies a set of URL query parameters on which the route should // match. The router will check the query string from the *path* header // against all the specified query parameters. If the number of specified // query parameters is nonzero, they all must match the *path* header's // query string for a match to occur. - string prefix = 1; + string path = 2; // If specified, only gRPC requests will be matched. The router will check // that the content-type header has a application/grpc or one of the various // application/grpc+ values. - string path = 2; + ConnectMatcher connect_matcher = 12; // If specified, the client tls context will be matched against the defined // match options. @@ -731,6 +724,15 @@ message RouteAction { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.RouteAction.UpgradeConfig"; + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when forwarding CONNECT payload as raw TCP. + // This may ONLY be used on routes which are selected via a ConnectMatcher + // and is only valid if the upgrade type is "CONNECT" + message ConnectConfig { + // TODO(alyssawilk) add proxy proto configuration here. + } + // The case-insensitive name of this upgrade, e.g. "websocket". // For each upgrade type present in upgrade_configs, requests with // Upgrade: [upgrade_type] will be proxied upstream. @@ -739,6 +741,8 @@ message RouteAction { // Determines if upgrades are available on this route. Defaults to true. google.protobuf.BoolValue enabled = 2; + + ConnectConfig connect_config = 3; } reserved 12, 18, 19, 16, 22, 21; diff --git a/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto b/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto index ce2b4d5a25962..719d860aea69d 100644 --- a/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto +++ b/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto @@ -367,7 +367,7 @@ message WeightedCluster { string runtime_key_prefix = 2; } -// [#next-free-field: 12] +// [#next-free-field: 13] message RouteMatch { option (udpa.annotations.versioning).previous_message_type = "envoy.config.route.v3.RouteMatch"; @@ -390,25 +390,10 @@ message RouteMatch { } // [#not-implemented-hide:] - // Configuration for sending data upstream as a raw data payload. This is used for - // CONNECT requests, when forwarding CONNECT payload as raw TCP. - message ProxyConfig { - // TODO(alyssawilk) add proxy proto configuration here. - + // An extensible message for matching CONNECT requests. + message ConnectMatcher { option (udpa.annotations.versioning).previous_message_type = - "envoy.config.route.v3.RouteMatch.ProxyConfig"; - } - - // [#not-implemented-hide:] - // Configuration for proxying CONNECT requests. - message ConnectConfig { - option (udpa.annotations.versioning).previous_message_type = - "envoy.config.route.v3.RouteMatch.ConnectConfig"; - - // If this is present, CONNECT requests will be decapsulated with the - // CONNECT payload proxied upstream. If this is not present, it will result - // in a CONNECT request being sent upstream. - ProxyConfig proxy_config = 1; + "envoy.config.route.v3.RouteMatch.ConnectMatcher"; } reserved 5, 3; @@ -427,7 +412,11 @@ message RouteMatch { string path = 2; // [#not-implemented-hide:] - //message ConnectConfig = 3; + // If this is used as the matcher, the matcher will only match CONNECT requests. + // Note that this will not match HTTP/2 upgrade-style CONNECT requests + // (WebSocket and the like) as they are normalized in Envoy HTTP/1.1 style + // upgrades. + ConnectMatcher connect_matcher = 12; // If specified, the route is a regular expression rule meaning that the // regex must match the *:path* header once the query string is removed. The entire path @@ -727,6 +716,18 @@ message RouteAction { option (udpa.annotations.versioning).previous_message_type = "envoy.config.route.v3.RouteAction.UpgradeConfig"; + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when forwarding CONNECT payload as raw TCP. + // This may ONLY be used on routes which are selected via a ConnectMatcher + // and is only valid if the upgrade type is "CONNECT" + message ConnectConfig { + // TODO(alyssawilk) add proxy proto configuration here. + + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.route.v3.RouteAction.UpgradeConfig.ConnectConfig"; + } + // The case-insensitive name of this upgrade, e.g. "websocket". // For each upgrade type present in upgrade_configs, requests with // Upgrade: [upgrade_type] will be proxied upstream. @@ -735,6 +736,8 @@ message RouteAction { // Determines if upgrades are available on this route. Defaults to true. google.protobuf.BoolValue enabled = 2; + + ConnectConfig connect_config = 3; } reserved 12, 18, 19, 16, 22, 21, 10; diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index d028dcf5582bc..696c98c1d0f80 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -478,15 +478,6 @@ class VirtualHost { */ virtual bool includeAttemptCountInResponse() const PURE; - /** - * If present, this indicates that CONNECT requests to this host should be converted - * to TCP proxying, i.e. the payload of the HTTP body should be sent as raw - * TCP upstream. - * @return configuration for TCP proxying, if present. - */ - using ProxyConfig = envoy::config::route::v3::RouteMatch::ProxyConfig; - virtual const absl::optional& proxyConfig() const PURE; - /** * @return uint32_t any route cap on bytes which should be buffered for shadowing or retries. * This is an upper bound so does not necessarily reflect the bytes which will be buffered @@ -835,6 +826,15 @@ class RouteEntry : public ResponseEntry { */ virtual const UpgradeMap& upgradeMap() const PURE; + // TODO(alyssar) rename alias, and interface from proxy to connect. + // TODO(alyssar) validate this is only used on routes with the CONNECT action. + using ConnectConfig = envoy::config::route::v3::RouteAction::UpgradeConfig::ConnectConfig; + + /** + * If present, informs how to handle proxying CONNECT requests on this route. + */ + virtual const absl::optional& connectConfig() const PURE; + /** * @returns the internal redirect action which should be taken on this route. */ @@ -956,13 +956,6 @@ class Route { template const Derived* perFilterConfigTyped(const std::string& name) const { return dynamic_cast(perFilterConfig(name)); } - - /** - * If present, this indicates that CONNECT requests are alowed on this route. - */ - using ConnectConfig = envoy::config::route::v3::RouteMatch::ConnectConfig; - using ProxyConfig = envoy::config::route::v3::RouteMatch::ProxyConfig; - virtual const absl::optional& connectConfig() const PURE; }; using RouteConstSharedPtr = std::shared_ptr; diff --git a/source/common/http/async_client_impl.cc b/source/common/http/async_client_impl.cc index 440aad35bd8c3..625edd1a73fb8 100644 --- a/source/common/http/async_client_impl.cc +++ b/source/common/http/async_client_impl.cc @@ -30,8 +30,8 @@ const Config::TypedMetadataImpl AsyncStreamImpl::RouteEntryImpl::typed_metadata_({}); const AsyncStreamImpl::NullPathMatchCriterion AsyncStreamImpl::RouteEntryImpl::path_match_criterion_; -const absl::optional - AsyncStreamImpl::RouteEntryImpl::connect_config_nullopt_; +const absl::optional + AsyncStreamImpl::RouteEntryImpl::proxy_config_nullopt_; const std::list AsyncStreamImpl::NullConfig::internal_only_headers_; AsyncClientImpl::AsyncClientImpl(Upstream::ClusterInfoConstSharedPtr cluster, diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 70a1396ef7a73..9502eef4d23bb 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -59,8 +59,10 @@ convertInternalRedirectAction(const envoy::config::route::v3::RouteAction& route const std::string DEPRECATED_ROUTER_NAME = "envoy.router"; +// TODO(alyssawilk) use utility. bool isConnect(const Http::RequestHeaderMap& headers) { return headers.Method() && headers.Method()->value().getStringView() == Http::Headers::get().MethodValues.Connect; +} } // namespace std::string SslRedirector::newPath(const Http::RequestHeaderMap& headers) const { @@ -925,21 +927,21 @@ RouteConstSharedPtr RegexRouteEntryImpl::matches(const Http::RequestHeaderMap& h return nullptr; } -ConnectRouteEntryImpl::RegexRouteEntryImpl( +// TODO(alyssawilk) unit tests. +ConnectRouteEntryImpl::ConnectRouteEntryImpl( const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator) : RouteEntryImplBase(vhost, route, factory_context, validator) { - connect_config_ = route.match().connect_matcher(); } -void ConnectRouteEntryImpl::rewritePathHeader(Http::RequestHeaderMap& headers, - bool insert_envoy_original_path) const { - finalizePathHeader(headers, prefix_, insert_envoy_original_path); +void ConnectRouteEntryImpl::rewritePathHeader(Http::RequestHeaderMap&, bool) const { + // TODO(alyssar) handle this for HTTP/2 CONNECT with path? + // finalizePathHeader(headers, prefix_, insert_envoy_original_path); } RouteConstSharedPtr ConnectRouteEntryImpl::matches(const Http::RequestHeaderMap& headers, - const StreamInfo::StreamInfo& stream_info, + const StreamInfo::StreamInfo&, uint64_t random_value) const { if (isConnect(headers)) { return clusterEntry(headers, random_value); @@ -1007,7 +1009,7 @@ VirtualHostImpl::VirtualHostImpl(const envoy::config::route::v3::VirtualHost& vi break; } case envoy::config::route::v3::RouteMatch::PathSpecifierCase::kConnectMatcher: { - // FIXME; + routes_.emplace_back(new ConnectRouteEntryImpl(*this, route, factory_context, validator)); break; } case envoy::config::route::v3::RouteMatch::PathSpecifierCase::PATH_SPECIFIER_NOT_SET: @@ -1153,9 +1155,6 @@ RouteConstSharedPtr VirtualHostImpl::getRouteFromEntries(const Http::RequestHead // Check for a route that matches the request. for (const RouteEntryImplBaseConstSharedPtr& route : routes_) { RouteConstSharedPtr route_entry = route->matches(headers, stream_info, random_value); - if (nullptr != route_entry && (isConnect(headers) || route_entry->connectConfig.has_value()) { - return route_entry; - } } return nullptr; diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index 7d244e98d30ca..bcb520c715e28 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -594,8 +594,8 @@ class RouteEntryImplBase : public RouteEntry, bool includeAttemptCountInResponse() const override { return parent_->includeAttemptCountInResponse(); } - const absl::optional proxyConfig() const override { - return parent_->proxyConfig(); + const absl::optional& connectConfig() const override { + return parent_->connectConfig(); } const UpgradeMap& upgradeMap() const override { return parent_->upgradeMap(); } InternalRedirectAction internalRedirectAction() const override { @@ -830,6 +830,27 @@ class RegexRouteEntryImpl : public RouteEntryImplBase { std::string regex_str_; }; +/** + * Route entry implementation for CONNECT requests. + */ +class ConnectRouteEntryImpl : public RouteEntryImplBase { +public: + ConnectRouteEntryImpl(const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route, + Server::Configuration::ServerFactoryContext& factory_context, + ProtobufMessage::ValidationVisitor& validator); + + // Router::PathMatchCriterion + const std::string& matcher() const override { return EMPTY_STRING; } + PathMatchType matchType() const override { return PathMatchType::None; } + + // Router::Matchable + RouteConstSharedPtr matches(const Http::RequestHeaderMap& headers, + const StreamInfo::StreamInfo& stream_info, + uint64_t random_value) const override; + + // Router::DirectResponseEntry + void rewritePathHeader(Http::RequestHeaderMap&, bool) const override; +}; /** * Wraps the route configuration which matches an incoming request headers to a backend cluster. * This is split out mainly to help with unit testing. diff --git a/source/common/router/router.cc b/source/common/router/router.cc index f8470c4c400de..0de58d7fd6e62 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -114,7 +114,7 @@ bool convertRequestHeadersForInternalRedirect(Http::RequestHeaderMap& downstream constexpr uint64_t TimeoutPrecisionFactor = 100; bool shouldProxy(Http::RequestHeaderMap& headers, const RouteEntry* route_entry) { - return route_entry->proxyConfig().has_value() && + return route_entry->connectConfig().has_value() && headers.Method()->value().getStringView() == Http::Headers::get().MethodValues.Connect; } diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 6f3d05de10ada..3b2ec8ee189a0 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -260,7 +260,6 @@ class MockVirtualHost : public VirtualHost { mutable std::unique_ptr stat_name_; testing::NiceMock rate_limit_policy_; TestCorsPolicy cors_policy_; - absl::optional connect_config_; }; class MockHashPolicy : public Http::HashPolicy { @@ -376,7 +375,7 @@ class MockRouteEntry : public RouteEntry { testing::NiceMock path_match_criterion_; envoy::config::core::v3::Metadata metadata_; UpgradeMap upgrade_map_; - absl::optional proxy_config_; + absl::optional connect_config_; }; class MockDecorator : public Decorator { From f56e2f005be83e8062c1c4a1da28874d8869b76b Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Thu, 16 Apr 2020 10:17:31 -0400 Subject: [PATCH 09/17] cleanup Signed-off-by: Alyssa Wilk --- source/common/http/async_client_impl.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/common/http/async_client_impl.cc b/source/common/http/async_client_impl.cc index 625edd1a73fb8..00998d2b5db2f 100644 --- a/source/common/http/async_client_impl.cc +++ b/source/common/http/async_client_impl.cc @@ -30,8 +30,8 @@ const Config::TypedMetadataImpl AsyncStreamImpl::RouteEntryImpl::typed_metadata_({}); const AsyncStreamImpl::NullPathMatchCriterion AsyncStreamImpl::RouteEntryImpl::path_match_criterion_; -const absl::optional - AsyncStreamImpl::RouteEntryImpl::proxy_config_nullopt_; +const absl::optional + AsyncStreamImpl::RouteEntryImpl::connect_config_nullopt_; const std::list AsyncStreamImpl::NullConfig::internal_only_headers_; AsyncClientImpl::AsyncClientImpl(Upstream::ClusterInfoConstSharedPtr cluster, From 67a753dc4cff10f5be1144ad08417c6abb7ecbae Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Thu, 16 Apr 2020 12:01:12 -0400 Subject: [PATCH 10/17] cleanup and tests of new API Signed-off-by: Alyssa Wilk --- .../config/route/v3/route_components.proto | 22 +++++--- .../route/v4alpha/route_components.proto | 22 +++++--- .../intro/arch_overview/http/upgrades.rst | 9 ++- .../config/route/v3/route_components.proto | 26 +++++---- .../route/v4alpha/route_components.proto | 22 +++++--- include/envoy/router/router.h | 3 - source/common/http/header_utility.cc | 4 ++ source/common/http/header_utility.h | 5 ++ source/common/router/config_impl.cc | 31 ++++++----- source/common/router/config_impl.h | 5 +- source/common/router/router.cc | 42 ++++++++------ test/common/router/BUILD | 20 ------- test/common/router/config_impl_test.cc | 55 ++++++++++++++++++- 13 files changed, 166 insertions(+), 100 deletions(-) diff --git a/api/envoy/config/route/v3/route_components.proto b/api/envoy/config/route/v3/route_components.proto index 8b60c3e1c4886..5a7577970df16 100644 --- a/api/envoy/config/route/v3/route_components.proto +++ b/api/envoy/config/route/v3/route_components.proto @@ -408,13 +408,6 @@ message RouteMatch { // exactly match the *:path* header once the query string is removed. string path = 2; - // [#not-implemented-hide:] - // If this is used as the matcher, the matcher will only match CONNECT requests. - // Note that this will not match HTTP/2 upgrade-style CONNECT requests - // (WebSocket and the like) as they are normalized in Envoy HTTP/1.1 style - // upgrades. - ConnectMatcher connect_matcher = 12; - // If specified, the route is a regular expression rule meaning that the // regex must match the *:path* header once the query string is removed. The entire path // (without the query string) must match the regex. The rule will not match if only a @@ -428,6 +421,16 @@ message RouteMatch { // on :path, etc. The issue with that is it is unclear how to generically deal with query string // stripping. This needs more thought.] type.matcher.v3.RegexMatcher safe_regex = 10 [(validate.rules).message = {required: true}]; + + // [#not-implemented-hide:] + // If this is used as the matcher, the matcher will only match CONNECT requests. + // Note that this will not match HTTP/2 upgrade-style CONNECT requests + // (WebSocket and the like) as they are normalized in Envoy as HTTP/1.1 style + // upgrades. + // This is the only way to match CONNECT requests for HTTP/1.1. For HTTP/2, + // where CONNECT requests may have a path, the path matchers will work if + // there is a path present. + ConnectMatcher connect_matcher = 12; } // Indicates that prefix/path matching should be case insensitive. The default @@ -716,8 +719,6 @@ message RouteAction { // [#not-implemented-hide:] // Configuration for sending data upstream as a raw data payload. This is used for // CONNECT requests, when forwarding CONNECT payload as raw TCP. - // This may ONLY be used on routes which are selected via a ConnectMatcher - // and is only valid if the upgrade type is "CONNECT" message ConnectConfig { // TODO(alyssawilk) add proxy proto configuration here. } @@ -731,6 +732,9 @@ message RouteAction { // Determines if upgrades are available on this route. Defaults to true. google.protobuf.BoolValue enabled = 2; + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when forwarding CONNECT payload as raw TCP. ConnectConfig connect_config = 3; } diff --git a/api/envoy/config/route/v4alpha/route_components.proto b/api/envoy/config/route/v4alpha/route_components.proto index 719d860aea69d..7c335349b3420 100644 --- a/api/envoy/config/route/v4alpha/route_components.proto +++ b/api/envoy/config/route/v4alpha/route_components.proto @@ -411,13 +411,6 @@ message RouteMatch { // exactly match the *:path* header once the query string is removed. string path = 2; - // [#not-implemented-hide:] - // If this is used as the matcher, the matcher will only match CONNECT requests. - // Note that this will not match HTTP/2 upgrade-style CONNECT requests - // (WebSocket and the like) as they are normalized in Envoy HTTP/1.1 style - // upgrades. - ConnectMatcher connect_matcher = 12; - // If specified, the route is a regular expression rule meaning that the // regex must match the *:path* header once the query string is removed. The entire path // (without the query string) must match the regex. The rule will not match if only a @@ -431,6 +424,16 @@ message RouteMatch { // on :path, etc. The issue with that is it is unclear how to generically deal with query string // stripping. This needs more thought.] type.matcher.v3.RegexMatcher safe_regex = 10 [(validate.rules).message = {required: true}]; + + // [#not-implemented-hide:] + // If this is used as the matcher, the matcher will only match CONNECT requests. + // Note that this will not match HTTP/2 upgrade-style CONNECT requests + // (WebSocket and the like) as they are normalized in Envoy as HTTP/1.1 style + // upgrades. + // This is the only way to match CONNECT requests for HTTP/1.1. For HTTP/2, + // where CONNECT requests may have a path, the path matchers will work if + // there is a path present. + ConnectMatcher connect_matcher = 12; } // Indicates that prefix/path matching should be case insensitive. The default @@ -719,8 +722,6 @@ message RouteAction { // [#not-implemented-hide:] // Configuration for sending data upstream as a raw data payload. This is used for // CONNECT requests, when forwarding CONNECT payload as raw TCP. - // This may ONLY be used on routes which are selected via a ConnectMatcher - // and is only valid if the upgrade type is "CONNECT" message ConnectConfig { // TODO(alyssawilk) add proxy proto configuration here. @@ -737,6 +738,9 @@ message RouteAction { // Determines if upgrades are available on this route. Defaults to true. google.protobuf.BoolValue enabled = 2; + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when forwarding CONNECT payload as raw TCP. ConnectConfig connect_config = 3; } diff --git a/docs/root/intro/arch_overview/http/upgrades.rst b/docs/root/intro/arch_overview/http/upgrades.rst index 11797ddbcf8af..dd6bbd9255ca4 100644 --- a/docs/root/intro/arch_overview/http/upgrades.rst +++ b/docs/root/intro/arch_overview/http/upgrades.rst @@ -70,14 +70,17 @@ upgrade requests or responses with bodies. .. CONNECT requests). CONNECT support can be enabled via the upgrade options described above, setting .. the upgrade value to the special keyword "CONNECT". +.. While for HTTP/2, CONNECT request may have a path, in general and for HTTP/1.1 CONNECT requests do +.. not have a path, and can only be matched using a +.. :ref:`connect_matcher ` +.. .. Envoy can handle CONNECT in one of two ways, either proxying the CONNECT headers through as if they .. were any other request, and letting the upstream handle the CONNECT request, or by terminating the .. CONNECT request, and forwarding the payload as raw TCP data. When CONNECT upgrade configuration is .. set up, the default behavior is to proxy the CONNECT request. If termination is desired, this can .. be accomplished by setting -.. the :ref:`per-route proxying_config ` or -.. :ref:`per-virtual-host proxying_config ` -.. If either is present for CONNECT requests, the router filter will strip the request headers, +.. :ref:`connect_config ` +.. If it that message is present for CONNECT requests, the router filter will strip the request headers, .. and forward the HTTP payload upstream. On receipt of initial TCP data from upstream, the router .. will synthesize 200 response headers, and then forward the TCP data as the HTTP response body. diff --git a/generated_api_shadow/envoy/config/route/v3/route_components.proto b/generated_api_shadow/envoy/config/route/v3/route_components.proto index 7c0ba5beef096..97163b4e82016 100644 --- a/generated_api_shadow/envoy/config/route/v3/route_components.proto +++ b/generated_api_shadow/envoy/config/route/v3/route_components.proto @@ -406,13 +406,6 @@ message RouteMatch { // exactly match the *:path* header once the query string is removed. core.v3.RuntimeFractionalPercent runtime_fraction = 9; - // [#not-implemented-hide:] - // If this is used as the matcher, the matcher will only match CONNECT requests. - // Note that this will not match HTTP/2 upgrade-style CONNECT requests - // (WebSocket and the like) as they are normalized in Envoy HTTP/1.1 style - // upgrades. - repeated HeaderMatcher headers = 6; - // If specified, the route is a regular expression rule meaning that the // regex must match the *:path* header once the query string is removed. The entire path // (without the query string) must match the regex. The rule will not match if only a @@ -425,6 +418,16 @@ message RouteMatch { // path_specifier entirely and just rely on a set of header matchers which can already match // on :path, etc. The issue with that is it is unclear how to generically deal with query string // stripping. This needs more thought.] + repeated HeaderMatcher headers = 6; + + // [#not-implemented-hide:] + // If this is used as the matcher, the matcher will only match CONNECT requests. + // Note that this will not match HTTP/2 upgrade-style CONNECT requests + // (WebSocket and the like) as they are normalized in Envoy as HTTP/1.1 style + // upgrades. + // This is the only way to match CONNECT requests for HTTP/1.1. For HTTP/2, + // where CONNECT requests may have a path, the path matchers will work if + // there is a path present. repeated QueryParameterMatcher query_parameters = 7; // Indicates that prefix/path matching should be case insensitive. The default @@ -469,13 +472,13 @@ message RouteMatch { // If specified, only gRPC requests will be matched. The router will check // that the content-type header has a application/grpc or one of the various // application/grpc+ values. - ConnectMatcher connect_matcher = 12; + type.matcher.v3.RegexMatcher safe_regex = 10 [(validate.rules).message = {required: true}]; // If specified, the client tls context will be matched against the defined // match options. // // [#next-major-version: unify with RBAC] - type.matcher.v3.RegexMatcher safe_regex = 10 [(validate.rules).message = {required: true}]; + ConnectMatcher connect_matcher = 12; string hidden_envoy_deprecated_regex = 3 [ deprecated = true, @@ -727,8 +730,6 @@ message RouteAction { // [#not-implemented-hide:] // Configuration for sending data upstream as a raw data payload. This is used for // CONNECT requests, when forwarding CONNECT payload as raw TCP. - // This may ONLY be used on routes which are selected via a ConnectMatcher - // and is only valid if the upgrade type is "CONNECT" message ConnectConfig { // TODO(alyssawilk) add proxy proto configuration here. } @@ -742,6 +743,9 @@ message RouteAction { // Determines if upgrades are available on this route. Defaults to true. google.protobuf.BoolValue enabled = 2; + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when forwarding CONNECT payload as raw TCP. ConnectConfig connect_config = 3; } diff --git a/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto b/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto index 719d860aea69d..7c335349b3420 100644 --- a/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto +++ b/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto @@ -411,13 +411,6 @@ message RouteMatch { // exactly match the *:path* header once the query string is removed. string path = 2; - // [#not-implemented-hide:] - // If this is used as the matcher, the matcher will only match CONNECT requests. - // Note that this will not match HTTP/2 upgrade-style CONNECT requests - // (WebSocket and the like) as they are normalized in Envoy HTTP/1.1 style - // upgrades. - ConnectMatcher connect_matcher = 12; - // If specified, the route is a regular expression rule meaning that the // regex must match the *:path* header once the query string is removed. The entire path // (without the query string) must match the regex. The rule will not match if only a @@ -431,6 +424,16 @@ message RouteMatch { // on :path, etc. The issue with that is it is unclear how to generically deal with query string // stripping. This needs more thought.] type.matcher.v3.RegexMatcher safe_regex = 10 [(validate.rules).message = {required: true}]; + + // [#not-implemented-hide:] + // If this is used as the matcher, the matcher will only match CONNECT requests. + // Note that this will not match HTTP/2 upgrade-style CONNECT requests + // (WebSocket and the like) as they are normalized in Envoy as HTTP/1.1 style + // upgrades. + // This is the only way to match CONNECT requests for HTTP/1.1. For HTTP/2, + // where CONNECT requests may have a path, the path matchers will work if + // there is a path present. + ConnectMatcher connect_matcher = 12; } // Indicates that prefix/path matching should be case insensitive. The default @@ -719,8 +722,6 @@ message RouteAction { // [#not-implemented-hide:] // Configuration for sending data upstream as a raw data payload. This is used for // CONNECT requests, when forwarding CONNECT payload as raw TCP. - // This may ONLY be used on routes which are selected via a ConnectMatcher - // and is only valid if the upgrade type is "CONNECT" message ConnectConfig { // TODO(alyssawilk) add proxy proto configuration here. @@ -737,6 +738,9 @@ message RouteAction { // Determines if upgrades are available on this route. Defaults to true. google.protobuf.BoolValue enabled = 2; + // [#not-implemented-hide:] + // Configuration for sending data upstream as a raw data payload. This is used for + // CONNECT requests, when forwarding CONNECT payload as raw TCP. ConnectConfig connect_config = 3; } diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index 696c98c1d0f80..39bffffa50d11 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -826,10 +826,7 @@ class RouteEntry : public ResponseEntry { */ virtual const UpgradeMap& upgradeMap() const PURE; - // TODO(alyssar) rename alias, and interface from proxy to connect. - // TODO(alyssar) validate this is only used on routes with the CONNECT action. using ConnectConfig = envoy::config::route::v3::RouteAction::UpgradeConfig::ConnectConfig; - /** * If present, informs how to handle proxying CONNECT requests on this route. */ diff --git a/source/common/http/header_utility.cc b/source/common/http/header_utility.cc index 5da6106261a99..83a235de7529e 100644 --- a/source/common/http/header_utility.cc +++ b/source/common/http/header_utility.cc @@ -157,6 +157,10 @@ bool HeaderUtility::authorityIsValid(const absl::string_view header_value) { header_value.size()) != 0; } +bool HeaderUtility::isConnect(const RequestHeaderMap& headers) { + return headers.Method() && headers.Method()->value() == Http::Headers::get().MethodValues.Connect; +} + void HeaderUtility::addHeaders(HeaderMap& headers, const HeaderMap& headers_to_add) { headers_to_add.iterate( [](const HeaderEntry& header, void* context) -> HeaderMap::Iterate { diff --git a/source/common/http/header_utility.h b/source/common/http/header_utility.h index e349a2ee97d0f..3ea94f149900d 100644 --- a/source/common/http/header_utility.h +++ b/source/common/http/header_utility.h @@ -111,6 +111,11 @@ class HeaderUtility { */ static bool authorityIsValid(const absl::string_view authority_value); + /** + * @brief a helper function to determine if the headers represent a CONNECT request. + */ + + static bool isConnect(const RequestHeaderMap& headers); /** * Add headers from one HeaderMap to another * @param headers target where headers will be added diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 9502eef4d23bb..c16f6f58ddcf6 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -59,10 +59,6 @@ convertInternalRedirectAction(const envoy::config::route::v3::RouteAction& route const std::string DEPRECATED_ROUTER_NAME = "envoy.router"; -// TODO(alyssawilk) use utility. -bool isConnect(const Http::RequestHeaderMap& headers) { - return headers.Method() && headers.Method()->value().getStringView() == Http::Headers::get().MethodValues.Connect; -} } // namespace std::string SslRedirector::newPath(const Http::RequestHeaderMap& headers) const { @@ -386,6 +382,12 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, if (!success) { throw EnvoyException(absl::StrCat("Duplicate upgrade ", upgrade_config.upgrade_type())); } + if (upgrade_config.upgrade_type() == Http::Headers::get().MethodValues.Connect) { + connect_config_ = upgrade_config.connect_config(); + } else if (upgrade_config.has_connect_config()) { + throw EnvoyException(absl::StrCat("Non-CONNECT upgrade type ", upgrade_config.upgrade_type(), + " has ConnectConfig")); + } } if (route.route().has_regex_rewrite()) { @@ -927,23 +929,23 @@ RouteConstSharedPtr RegexRouteEntryImpl::matches(const Http::RequestHeaderMap& h return nullptr; } -// TODO(alyssawilk) unit tests. ConnectRouteEntryImpl::ConnectRouteEntryImpl( const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator) - : RouteEntryImplBase(vhost, route, factory_context, validator) { -} + : RouteEntryImplBase(vhost, route, factory_context, validator) {} -void ConnectRouteEntryImpl::rewritePathHeader(Http::RequestHeaderMap&, bool) const { - // TODO(alyssar) handle this for HTTP/2 CONNECT with path? - // finalizePathHeader(headers, prefix_, insert_envoy_original_path); +void ConnectRouteEntryImpl::rewritePathHeader(Http::RequestHeaderMap& headers, + bool insert_envoy_original_path) const { + const absl::string_view path = + Http::PathUtil::removeQueryAndFragment(headers.Path()->value().getStringView()); + finalizePathHeader(headers, path, insert_envoy_original_path); } RouteConstSharedPtr ConnectRouteEntryImpl::matches(const Http::RequestHeaderMap& headers, - const StreamInfo::StreamInfo&, - uint64_t random_value) const { - if (isConnect(headers)) { + const StreamInfo::StreamInfo&, + uint64_t random_value) const { + if (Http::HeaderUtility::isConnect(headers)) { return clusterEntry(headers, random_value); } return nullptr; @@ -1155,6 +1157,9 @@ RouteConstSharedPtr VirtualHostImpl::getRouteFromEntries(const Http::RequestHead // Check for a route that matches the request. for (const RouteEntryImplBaseConstSharedPtr& route : routes_) { RouteConstSharedPtr route_entry = route->matches(headers, stream_info, random_value); + if (nullptr != route_entry) { + return route_entry; + } } return nullptr; diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index bcb520c715e28..f269c0f3560b1 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -497,7 +497,6 @@ class RouteEntryImplBase : public RouteEntry, bool include_vh_rate_limits_; absl::optional connect_config_; - RouteConstSharedPtr clusterEntry(const Http::HeaderMap& headers, uint64_t random_value) const; /** @@ -836,8 +835,8 @@ class RegexRouteEntryImpl : public RouteEntryImplBase { class ConnectRouteEntryImpl : public RouteEntryImplBase { public: ConnectRouteEntryImpl(const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route, - Server::Configuration::ServerFactoryContext& factory_context, - ProtobufMessage::ValidationVisitor& validator); + Server::Configuration::ServerFactoryContext& factory_context, + ProtobufMessage::ValidationVisitor& validator); // Router::PathMatchCriterion const std::string& matcher() const override { return EMPTY_STRING; } diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 0de58d7fd6e62..f43894b8ebb7b 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -113,11 +113,21 @@ bool convertRequestHeadersForInternalRedirect(Http::RequestHeaderMap& downstream constexpr uint64_t TimeoutPrecisionFactor = 100; -bool shouldProxy(Http::RequestHeaderMap& headers, const RouteEntry* route_entry) { +bool shouldTcpProxy(Http::RequestHeaderMap& headers, const RouteEntry* route_entry) { return route_entry->connectConfig().has_value() && headers.Method()->value().getStringView() == Http::Headers::get().MethodValues.Connect; } +Http::ConnectionPool::Instance* +httpPool(absl::variant pool) { + return absl::get(pool); +} + +Tcp::ConnectionPool::Instance* +tcpPool(absl::variant pool) { + return absl::get(pool); +} + } // namespace // Express percentage as [0, TimeoutPrecisionFactor] because stats do not accept floating point @@ -546,30 +556,26 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, } } - union ConnPool { - Http::ConnectionPool::Instance* http_pool; - Tcp::ConnectionPool::Instance* tcp_pool; - }; - ConnPool conn_pool; + absl::variant conn_pool; Upstream::HostDescriptionConstSharedPtr host; - bool should_tcp_proxy = shouldProxy(*downstream_headers_, route_entry_); + bool should_tcp_proxy = shouldTcpProxy(*downstream_headers_, route_entry_); if (!should_tcp_proxy) { - conn_pool.http_pool = getHttpConnPool(); - if (conn_pool.http_pool) { - host = conn_pool.http_pool->host(); + conn_pool = getHttpConnPool(); + if (httpPool(conn_pool)) { + host = httpPool(conn_pool)->host(); } } else { transport_socket_options_ = Network::TransportSocketOptionsUtility::fromFilterState( *callbacks_->streamInfo().filterState()); - conn_pool.tcp_pool = config_.cm_.tcpConnPoolForCluster( - route_entry_->clusterName(), Upstream::ResourcePriority::Default, this); - if (conn_pool.tcp_pool) { - host = conn_pool.tcp_pool->host(); + conn_pool = config_.cm_.tcpConnPoolForCluster(route_entry_->clusterName(), + Upstream::ResourcePriority::Default, this); + if (tcpPool(conn_pool)) { + host = tcpPool(conn_pool)->host(); } } - if (!conn_pool.http_pool && !conn_pool.tcp_pool) { + if (!host) { sendNoHealthyUpstreamResponse(); return Http::FilterHeadersStatus::StopIteration; } @@ -662,10 +668,10 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, UpstreamRequestPtr upstream_request; if (!should_tcp_proxy) { upstream_request = std::make_unique( - *this, std::make_unique(*conn_pool.http_pool)); + *this, std::make_unique(*httpPool(conn_pool))); } else { upstream_request = - std::make_unique(*this, std::make_unique(conn_pool.tcp_pool)); + std::make_unique(*this, std::make_unique(tcpPool(conn_pool))); } upstream_request->moveIntoList(std::move(upstream_request), upstream_requests_); upstream_requests_.front()->encodeHeaders(end_stream); @@ -1467,7 +1473,7 @@ void Filter::doRetry() { pending_retries_--; UpstreamRequestPtr upstream_request; - if (!shouldProxy(*downstream_headers_, route_entry_)) { + if (!shouldTcpProxy(*downstream_headers_, route_entry_)) { Http::ConnectionPool::Instance* conn_pool = getHttpConnPool(); if (conn_pool) { upstream_request = diff --git a/test/common/router/BUILD b/test/common/router/BUILD index 24652f89b50cd..2ac9c91f26f30 100644 --- a/test/common/router/BUILD +++ b/test/common/router/BUILD @@ -317,26 +317,6 @@ envoy_cc_test( ], ) -envoy_cc_test( - name = "upstream_request_test", - srcs = ["upstream_request_test.cc"], - deps = [ - "//source/common/buffer:buffer_lib", - "//source/common/router:router_lib", - "//source/common/upstream:upstream_includes", - "//source/common/upstream:upstream_lib", - "//test/common/http:common_lib", - "//test/mocks/http:http_mocks", - "//test/mocks/network:network_mocks", - "//test/mocks/router:router_mocks", - "//test/mocks/server:server_mocks", - "//test/mocks/upstream:upstream_mocks", - "//test/test_common:environment_lib", - "//test/test_common:simulated_time_system_lib", - "//test/test_common:utility_lib", - ], -) - envoy_cc_test( name = "header_formatter_test", srcs = ["header_formatter_test.cc"], diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index fd61d51ff961a..faf685352c53b 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -271,11 +271,11 @@ most_specific_header_mutations_wins: {0} class RouteMatcherTest : public testing::Test, public ConfigImplTestBase {}; // When removing legacy fields this test can be removed. -TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(TestLegacyRoutes)) { +// TODO(alyssawilk) remove DISABLED once this doesn't break the build. +TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(DISABLED_TestLegacyRoutes)) { const std::string yaml = R"EOF( virtual_hosts: - name: regex - domains: - bat.com routes: - match: @@ -309,6 +309,18 @@ TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(TestLegacyRoutes)) { regex: ".*" route: cluster: regex_default +- name: connect + domains: + - bat3.com + routes: + - match: + connect_matcher: + route: + cluster: connect_match + - match: + regex: ".*" + route: + cluster: connect_fallthrough - name: default domains: - "*" @@ -369,6 +381,12 @@ TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(TestLegacyRoutes)) { EXPECT_EQ("regex_default", config.route(genHeaders("bat2.com", " ", "GET"), 0)->routeEntry()->clusterName()); + // Connect matching + EXPECT_EQ("connect_match", + config.route(genHeaders("bat3.com", " ", "CONNECT"), 0)->routeEntry()->clusterName()); + EXPECT_EQ("connect_fallthrough", + config.route(genHeaders("bat3.com", " ", "GET"), 0)->routeEntry()->clusterName()); + // Regular Expression matching with query string params EXPECT_EQ( "clock", @@ -997,6 +1015,15 @@ TEST_F(RouteMatcherTest, TestRoutes) { EXPECT_EQ("/xx/yy/6472?test=foo", headers.get_(Http::Headers::get().EnvoyOriginalPath)); } + // Prefix rewrite for CONNECT with path (for HTTP/2) + { + Http::TestRequestHeaderMapImpl headers = + genHeaders("bat3.com", "/api/locations?works=true", "CONNECT"); + const RouteEntry* route = config.route(headers, 0)->routeEntry(); + route->finalizeRequestHeaders(headers, stream_info, true); + EXPECT_EQ("/rewrote?works=true", headers.get_(Http::Headers::get().Path)); + } + // Virtual cluster testing. { Http::TestRequestHeaderMapImpl headers = genHeaders("api.lyft.com", "/rides", "GET"); @@ -6698,6 +6725,30 @@ name: RetriableStatusCodes EnvoyException, "Duplicate upgrade WebSocket"); } +TEST_F(RouteConfigurationV2, BadConnectConfig) { + const std::string yaml = R"EOF( +name: RetriableStatusCodes +virtual_hosts: + - name: regex + domains: [idle.lyft.com] + routes: + - match: + safe_regex: + google_re2: {} + regex: "/regex" + route: + cluster: some-cluster + upgrade_configs: + - upgrade_type: Websocket + connect_config: {} + enabled: false + )EOF"; + + EXPECT_THROW_WITH_MESSAGE( + TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true), + EnvoyException, "Non-CONNECT upgrade type Websocket has ConnectConfig"); +} + // Verifies that we're creating a new instance of the retry plugins on each call instead of always // returning the same one. TEST_F(RouteConfigurationV2, RetryPluginsAreNotReused) { From 795231fc922ad26fd1972d1499fa1e472d021989 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Thu, 16 Apr 2020 15:46:22 -0400 Subject: [PATCH 11/17] finally got the config impl test working Signed-off-by: Alyssa Wilk --- test/common/router/config_impl_test.cc | 89 +++++++++++++++++--------- 1 file changed, 59 insertions(+), 30 deletions(-) diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index faf685352c53b..bd0895c05ae94 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -271,11 +271,11 @@ most_specific_header_mutations_wins: {0} class RouteMatcherTest : public testing::Test, public ConfigImplTestBase {}; // When removing legacy fields this test can be removed. -// TODO(alyssawilk) remove DISABLED once this doesn't break the build. -TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(DISABLED_TestLegacyRoutes)) { +TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(TestLegacyRoutes)) { const std::string yaml = R"EOF( virtual_hosts: - name: regex + domains: - bat.com routes: - match: @@ -309,18 +309,6 @@ TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(DISABLED_TestLegacyRoutes)) { regex: ".*" route: cluster: regex_default -- name: connect - domains: - - bat3.com - routes: - - match: - connect_matcher: - route: - cluster: connect_match - - match: - regex: ".*" - route: - cluster: connect_fallthrough - name: default domains: - "*" @@ -381,12 +369,6 @@ TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(DISABLED_TestLegacyRoutes)) { EXPECT_EQ("regex_default", config.route(genHeaders("bat2.com", " ", "GET"), 0)->routeEntry()->clusterName()); - // Connect matching - EXPECT_EQ("connect_match", - config.route(genHeaders("bat3.com", " ", "CONNECT"), 0)->routeEntry()->clusterName()); - EXPECT_EQ("connect_fallthrough", - config.route(genHeaders("bat3.com", " ", "GET"), 0)->routeEntry()->clusterName()); - // Regular Expression matching with query string params EXPECT_EQ( "clock", @@ -450,6 +432,63 @@ TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(DISABLED_TestLegacyRoutes)) { } } +TEST_F(RouteMatcherTest, TestConnectRoutes) { + const std::string yaml = R"EOF( +virtual_hosts: +- name: connect + domains: + - bat3.com + routes: + - match: + connect_matcher: + {} + route: + cluster: connect_match + prefix_rewrite: "/rewrote" + - match: + safe_regex: + google_re2: {} + regex: ".*" + route: + cluster: connect_fallthrough +- name: default + domains: + - "*" + routes: + - match: + prefix: "/" + route: + cluster: instant-server + timeout: 30s + virtual_clusters: + - headers: + - name: ":path" + safe_regex_match: + google_re2: {} + regex: "^/users/\\d+/location$" + - name: ":method" + exact_match: POST + name: ulu + )EOF"; + NiceMock stream_info; + TestConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true); + + // Connect matching + EXPECT_EQ("connect_match", + config.route(genHeaders("bat3.com", " ", "CONNECT"), 0)->routeEntry()->clusterName()); + EXPECT_EQ("connect_fallthrough", + config.route(genHeaders("bat3.com", " ", "GET"), 0)->routeEntry()->clusterName()); + + // Prefix rewrite for CONNECT with path (for HTTP/2) + { + Http::TestRequestHeaderMapImpl headers = + genHeaders("bat3.com", "/api/locations?works=true", "CONNECT"); + const RouteEntry* route = config.route(headers, 0)->routeEntry(); + route->finalizeRequestHeaders(headers, stream_info, true); + EXPECT_EQ("/rewrote?works=true", headers.get_(Http::Headers::get().Path)); + } +} + TEST_F(RouteMatcherTest, TestRoutes) { const std::string yaml = R"EOF( virtual_hosts: @@ -705,7 +744,6 @@ TEST_F(RouteMatcherTest, TestRoutes) { exact_match: POST name: ulu )EOF"; - NiceMock stream_info; TestConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true); @@ -1015,15 +1053,6 @@ TEST_F(RouteMatcherTest, TestRoutes) { EXPECT_EQ("/xx/yy/6472?test=foo", headers.get_(Http::Headers::get().EnvoyOriginalPath)); } - // Prefix rewrite for CONNECT with path (for HTTP/2) - { - Http::TestRequestHeaderMapImpl headers = - genHeaders("bat3.com", "/api/locations?works=true", "CONNECT"); - const RouteEntry* route = config.route(headers, 0)->routeEntry(); - route->finalizeRequestHeaders(headers, stream_info, true); - EXPECT_EQ("/rewrote?works=true", headers.get_(Http::Headers::get().Path)); - } - // Virtual cluster testing. { Http::TestRequestHeaderMapImpl headers = genHeaders("api.lyft.com", "/rides", "GET"); From a847f9161fa3c1955e03543591f8e3450cf3337e Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 20 Apr 2020 08:55:40 -0400 Subject: [PATCH 12/17] default return Signed-off-by: Alyssa Wilk --- source/common/router/upstream_request.h | 1 + 1 file changed, 1 insertion(+) diff --git a/source/common/router/upstream_request.h b/source/common/router/upstream_request.h index d9c1d9bdbf913..3ec3eb3dfb9e7 100644 --- a/source/common/router/upstream_request.h +++ b/source/common/router/upstream_request.h @@ -231,6 +231,7 @@ class TcpConnPool : public GenericConnPool, public Tcp::ConnectionPool::Callback case Tcp::ConnectionPool::PoolFailureReason::Timeout: return Http::ConnectionPool::PoolFailureReason::ConnectionFailure; } + return Http::ConnectionPool::PoolFailureReason::ConnectionFailure; } // Tcp::ConnectionPool::Callbacks From ac38de1f156e65bd091691db7bb580886882559f Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 20 Apr 2020 11:10:00 -0400 Subject: [PATCH 13/17] fixing build rule Signed-off-by: Alyssa Wilk --- test/common/router/BUILD | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/common/router/BUILD b/test/common/router/BUILD index 2ac9c91f26f30..24652f89b50cd 100644 --- a/test/common/router/BUILD +++ b/test/common/router/BUILD @@ -317,6 +317,26 @@ envoy_cc_test( ], ) +envoy_cc_test( + name = "upstream_request_test", + srcs = ["upstream_request_test.cc"], + deps = [ + "//source/common/buffer:buffer_lib", + "//source/common/router:router_lib", + "//source/common/upstream:upstream_includes", + "//source/common/upstream:upstream_lib", + "//test/common/http:common_lib", + "//test/mocks/http:http_mocks", + "//test/mocks/network:network_mocks", + "//test/mocks/router:router_mocks", + "//test/mocks/server:server_mocks", + "//test/mocks/upstream:upstream_mocks", + "//test/test_common:environment_lib", + "//test/test_common:simulated_time_system_lib", + "//test/test_common:utility_lib", + ], +) + envoy_cc_test( name = "header_formatter_test", srcs = ["header_formatter_test.cc"], From 9a82f12bc8429bc0c1dc7751722e3a58ed90fc4a Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 20 Apr 2020 12:19:42 -0400 Subject: [PATCH 14/17] tidier Signed-off-by: Alyssa Wilk --- test/common/router/upstream_request_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/router/upstream_request_test.cc b/test/common/router/upstream_request_test.cc index 5a445ed93519f..a445a4d6e690a 100644 --- a/test/common/router/upstream_request_test.cc +++ b/test/common/router/upstream_request_test.cc @@ -157,7 +157,7 @@ class TcpUpstreamTest : public ::testing::Test { tcp_upstream_ = std::make_unique(mock_router_filter_.requests_.front().get(), std::move(data)); } - ~TcpUpstreamTest() { EXPECT_CALL(mock_router_filter_, config()).Times(AnyNumber()); } + ~TcpUpstreamTest() override { EXPECT_CALL(mock_router_filter_, config()).Times(AnyNumber()); } protected: NiceMock connection_; From 92d833d3bef2dde99792e49fbd3d8c9fd9d9bb2a Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 21 Apr 2020 15:47:46 -0400 Subject: [PATCH 15/17] integration tests! Signed-off-by: Alyssa Wilk --- .../tcp_tunneling_integration_test.cc | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/test/integration/tcp_tunneling_integration_test.cc b/test/integration/tcp_tunneling_integration_test.cc index 2851fd79a4946..3ce0f1cec06a8 100644 --- a/test/integration/tcp_tunneling_integration_test.cc +++ b/test/integration/tcp_tunneling_integration_test.cc @@ -22,12 +22,20 @@ class ConnectTerminationIntegrationTest } void initialize() override { - auto host = config_helper_.createVirtualHost("host", "/"); - // host.mutable_proxying_config(); - config_helper_.addVirtualHost(host); config_helper_.addConfigModifier( [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& - hcm) -> void { + hcm) { + auto* route_config = hcm.mutable_route_config(); + ASSERT_EQ(1, route_config->virtual_hosts_size()); + auto* route = route_config->mutable_virtual_hosts(0)->mutable_routes(0); + auto* match = route->mutable_match(); + match->Clear(); + match->mutable_connect_matcher(); + + auto* upgrade = route->mutable_route()->add_upgrade_configs(); + upgrade->set_upgrade_type("CONNECT"); + upgrade->mutable_connect_config(); + hcm.add_upgrade_configs()->set_upgrade_type("CONNECT"); hcm.mutable_http2_protocol_options()->set_allow_connect(true); @@ -66,7 +74,7 @@ class ConnectTerminationIntegrationTest {":path", "/"}, {":protocol", "bytestream"}, {":scheme", "https"}, - {":authority", "host"}}; + {":authority", "host:80"}}; FakeRawConnectionPtr fake_raw_upstream_connection_; IntegrationStreamDecoderPtr response_; bool enable_timeout_{}; @@ -74,7 +82,7 @@ class ConnectTerminationIntegrationTest // TODO(alyssawilk) make sure that if data is sent with the connect it does not go upstream // until the 200 headers are sent before unhiding ANY config. -TEST_P(ConnectTerminationIntegrationTest, DISABLED_Basic) { +TEST_P(ConnectTerminationIntegrationTest, Basic) { initialize(); setUpConnection(); @@ -92,7 +100,7 @@ TEST_P(ConnectTerminationIntegrationTest, DISABLED_Basic) { ASSERT_FALSE(response_->reset()); } -TEST_P(ConnectTerminationIntegrationTest, DISABLED_DownstreamClose) { +TEST_P(ConnectTerminationIntegrationTest, DownstreamClose) { initialize(); setUpConnection(); @@ -103,7 +111,7 @@ TEST_P(ConnectTerminationIntegrationTest, DISABLED_DownstreamClose) { ASSERT_TRUE(fake_raw_upstream_connection_->waitForHalfClose()); } -TEST_P(ConnectTerminationIntegrationTest, DISABLED_DownstreamReset) { +TEST_P(ConnectTerminationIntegrationTest, DownstreamReset) { initialize(); setUpConnection(); @@ -114,7 +122,7 @@ TEST_P(ConnectTerminationIntegrationTest, DISABLED_DownstreamReset) { ASSERT_TRUE(fake_raw_upstream_connection_->waitForHalfClose()); } -TEST_P(ConnectTerminationIntegrationTest, DISABLED_UpstreamClose) { +TEST_P(ConnectTerminationIntegrationTest, UpstreamClose) { initialize(); setUpConnection(); @@ -125,7 +133,7 @@ TEST_P(ConnectTerminationIntegrationTest, DISABLED_UpstreamClose) { response_->waitForReset(); } -TEST_P(ConnectTerminationIntegrationTest, DISABLED_TestTimeout) { +TEST_P(ConnectTerminationIntegrationTest, TestTimeout) { enable_timeout_ = true; initialize(); @@ -136,7 +144,7 @@ TEST_P(ConnectTerminationIntegrationTest, DISABLED_TestTimeout) { ASSERT_TRUE(fake_raw_upstream_connection_->waitForHalfClose()); } -TEST_P(ConnectTerminationIntegrationTest, DISABLED_BuggyHeaders) { +TEST_P(ConnectTerminationIntegrationTest, BuggyHeaders) { initialize(); // It's possible that the FIN is received before we set half close on the // upstream connection, so allow unexpected disconnects. @@ -150,7 +158,7 @@ TEST_P(ConnectTerminationIntegrationTest, DISABLED_BuggyHeaders) { {":path", "/"}, {":protocol", "bytestream"}, {":scheme", "https"}, - {":authority", "host"}}); + {":authority", "host:80"}}); // If the connection is established (created, set to half close, and then the // FIN arrives), make sure the FIN arrives, and send a FIN from upstream. if (fake_upstreams_[0]->waitForRawConnection(fake_raw_upstream_connection_) && From 8363d5ab94c59d8b584cb8bd7c66891c339c08ea Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Wed, 22 Apr 2020 10:57:06 -0400 Subject: [PATCH 16/17] reviewer comments Signed-off-by: Alyssa Wilk --- source/common/router/router.cc | 87 +++++++++++------------- source/common/router/router.h | 6 ++ source/common/router/upstream_request.cc | 3 - 3 files changed, 44 insertions(+), 52 deletions(-) diff --git a/source/common/router/router.cc b/source/common/router/router.cc index a5dd98728a3fe..45e64d2ff2414 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -112,11 +112,6 @@ bool convertRequestHeadersForInternalRedirect(Http::RequestHeaderMap& downstream constexpr uint64_t TimeoutPrecisionFactor = 100; -bool shouldTcpProxy(Http::RequestHeaderMap& headers, const RouteEntry* route_entry) { - return route_entry->connectConfig().has_value() && - headers.Method()->value().getStringView() == Http::Headers::get().MethodValues.Connect; -} - Http::ConnectionPool::Instance* httpPool(absl::variant pool) { return absl::get(pool); @@ -555,24 +550,8 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, } } - absl::variant conn_pool; Upstream::HostDescriptionConstSharedPtr host; - bool should_tcp_proxy = shouldTcpProxy(*downstream_headers_, route_entry_); - - if (!should_tcp_proxy) { - conn_pool = getHttpConnPool(); - if (httpPool(conn_pool)) { - host = httpPool(conn_pool)->host(); - } - } else { - transport_socket_options_ = Network::TransportSocketOptionsUtility::fromFilterState( - *callbacks_->streamInfo().filterState()); - conn_pool = config_.cm_.tcpConnPoolForCluster(route_entry_->clusterName(), - Upstream::ResourcePriority::Default, this); - if (tcpPool(conn_pool)) { - host = tcpPool(conn_pool)->host(); - } - } + Filter::HttpOrTcpPool conn_pool = createConnPool(host); if (!host) { sendNoHealthyUpstreamResponse(); @@ -664,14 +643,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, // Hang onto the modify_headers function for later use in handling upstream responses. modify_headers_ = modify_headers; - UpstreamRequestPtr upstream_request; - if (!should_tcp_proxy) { - upstream_request = std::make_unique( - *this, std::make_unique(*httpPool(conn_pool))); - } else { - upstream_request = - std::make_unique(*this, std::make_unique(tcpPool(conn_pool))); - } + UpstreamRequestPtr upstream_request = createUpstreamRequest(conn_pool); upstream_request->moveIntoList(std::move(upstream_request), upstream_requests_); upstream_requests_.front()->encodeHeaders(end_stream); if (end_stream) { @@ -681,6 +653,38 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, return Http::FilterHeadersStatus::StopIteration; } +Filter::HttpOrTcpPool Filter::createConnPool(Upstream::HostDescriptionConstSharedPtr& host) { + Filter::HttpOrTcpPool conn_pool; + bool should_tcp_proxy = route_entry_->connectConfig().has_value() && + downstream_headers_->Method()->value().getStringView() == + Http::Headers::get().MethodValues.Connect; + + if (!should_tcp_proxy) { + conn_pool = getHttpConnPool(); + if (httpPool(conn_pool)) { + host = httpPool(conn_pool)->host(); + } + } else { + transport_socket_options_ = Network::TransportSocketOptionsUtility::fromFilterState( + *callbacks_->streamInfo().filterState()); + conn_pool = config_.cm_.tcpConnPoolForCluster(route_entry_->clusterName(), + Upstream::ResourcePriority::Default, this); + if (tcpPool(conn_pool)) { + host = tcpPool(conn_pool)->host(); + } + } + return conn_pool; +} + +UpstreamRequestPtr Filter::createUpstreamRequest(Filter::HttpOrTcpPool conn_pool) { + if (absl::holds_alternative(conn_pool)) { + return std::make_unique(*this, + std::make_unique(*httpPool(conn_pool))); + } + return std::make_unique(*this, + std::make_unique(tcpPool(conn_pool))); +} + Http::ConnectionPool::Instance* Filter::getHttpConnPool() { // Choose protocol based on cluster configuration and downstream connection // Note: Cluster may downgrade HTTP2 to HTTP1 based on runtime configuration. @@ -1479,30 +1483,15 @@ void Filter::doRetry() { attempt_count_++; ASSERT(pending_retries_ > 0); pending_retries_--; - UpstreamRequestPtr upstream_request; - - if (!shouldTcpProxy(*downstream_headers_, route_entry_)) { - Http::ConnectionPool::Instance* conn_pool = getHttpConnPool(); - if (conn_pool) { - upstream_request = - std::make_unique(*this, std::make_unique(*conn_pool)); - } - } else { - transport_socket_options_ = Network::TransportSocketOptionsUtility::fromFilterState( - *callbacks_->streamInfo().filterState()); - Tcp::ConnectionPool::Instance* conn_pool = config_.cm_.tcpConnPoolForCluster( - route_entry_->clusterName(), Upstream::ResourcePriority::Default, this); - if (conn_pool) { - upstream_request = - std::make_unique(*this, std::make_unique(conn_pool)); - } - } - if (!upstream_request) { + Upstream::HostDescriptionConstSharedPtr host; + Filter::HttpOrTcpPool conn_pool = createConnPool(host); + if (!host) { sendNoHealthyUpstreamResponse(); cleanup(); return; } + UpstreamRequestPtr upstream_request = createUpstreamRequest(conn_pool); if (include_attempt_count_in_request_) { downstream_headers_->setEnvoyAttemptCount(attempt_count_); diff --git a/source/common/router/router.h b/source/common/router/router.h index 9f532d08fb5f6..c74a333233773 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -467,6 +467,12 @@ class Filter : Logger::Loggable, const Upstream::ClusterInfo& cluster, const VirtualCluster* vcluster, Runtime::Loader& runtime, Runtime::RandomGenerator& random, Event::Dispatcher& dispatcher, Upstream::ResourcePriority priority) PURE; + + using HttpOrTcpPool = + absl::variant; + HttpOrTcpPool createConnPool(Upstream::HostDescriptionConstSharedPtr& host); + UpstreamRequestPtr createUpstreamRequest(Filter::HttpOrTcpPool conn_pool); + Http::ConnectionPool::Instance* getHttpConnPool(); void maybeDoShadowing(); bool maybeRetryReset(Http::StreamResetReason reset_reason, UpstreamRequest& upstream_request); diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc index 5fcfad41a9d4a..049199a68c0c4 100644 --- a/source/common/router/upstream_request.cc +++ b/source/common/router/upstream_request.cc @@ -552,9 +552,6 @@ void TcpUpstream::resetStream() { } void TcpUpstream::onUpstreamData(Buffer::Instance& data, bool end_stream) { - if (!upstream_request_) { - return; - } if (!sent_headers_) { Http::ResponseHeaderMapPtr headers{ Http::createHeaderMap({{Http::Headers::get().Status, "200"}})}; From 72b292af1576d0cefbff5a3e137a2ba97cc6d717 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Wed, 22 Apr 2020 15:02:27 -0400 Subject: [PATCH 17/17] docs rewording Signed-off-by: Alyssa Wilk --- docs/root/intro/arch_overview/http/upgrades.rst | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/root/intro/arch_overview/http/upgrades.rst b/docs/root/intro/arch_overview/http/upgrades.rst index dd6bbd9255ca4..6f3b88728273f 100644 --- a/docs/root/intro/arch_overview/http/upgrades.rst +++ b/docs/root/intro/arch_overview/http/upgrades.rst @@ -66,7 +66,7 @@ upgrade requests or responses with bodies. .. CONNECT support .. ^^^^^^^^^^^^^^^ -.. Envoy CONNECT support is off by default (Envoy will send an internall generated 403 in response to +.. Envoy CONNECT support is off by default (Envoy will send an internally generated 403 in response to .. CONNECT requests). CONNECT support can be enabled via the upgrade options described above, setting .. the upgrade value to the special keyword "CONNECT". @@ -75,10 +75,11 @@ upgrade requests or responses with bodies. .. :ref:`connect_matcher ` .. .. Envoy can handle CONNECT in one of two ways, either proxying the CONNECT headers through as if they -.. were any other request, and letting the upstream handle the CONNECT request, or by terminating the +.. were any other request, and letting the upstream terminate the CONNECT request, or by terminating the .. CONNECT request, and forwarding the payload as raw TCP data. When CONNECT upgrade configuration is -.. set up, the default behavior is to proxy the CONNECT request. If termination is desired, this can -.. be accomplished by setting +.. set up, the default behavior is to proxy the CONNECT request, treating it like any other request using +.. the upgrade path. +.. If termination is desired, this can be accomplished by setting .. :ref:`connect_config ` .. If it that message is present for CONNECT requests, the router filter will strip the request headers, .. and forward the HTTP payload upstream. On receipt of initial TCP data from upstream, the router