From 745132d311e75427c92230eae6eb690247862c5e Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 6 Apr 2021 13:36:38 -0400 Subject: [PATCH 1/5] Adding QUIC-to-TCP failover Signed-off-by: Alyssa Wilk --- .../http/v3/http_protocol_options.proto | 7 ++ .../http/v4alpha/http_protocol_options.proto | 7 ++ .../http/v3/http_protocol_options.proto | 7 ++ .../http/v4alpha/http_protocol_options.proto | 7 ++ include/envoy/upstream/host_description.h | 6 +- source/common/http/http3/conn_pool.cc | 2 +- source/common/upstream/BUILD | 1 + .../common/upstream/cluster_manager_impl.cc | 10 ++- source/common/upstream/logical_host.cc | 2 +- source/common/upstream/logical_host.h | 4 +- source/common/upstream/upstream_impl.cc | 17 +++- source/common/upstream/upstream_impl.h | 21 ++++- source/extensions/upstreams/http/config.cc | 4 + test/common/upstream/BUILD | 1 + test/common/upstream/upstream_impl_test.cc | 79 +++++++++++++++++++ test/config/utility.cc | 21 ++++- test/integration/base_integration_test.h | 1 - .../multiplexed_upstream_integration_test.cc | 38 +++++++++ .../multiplexed_upstream_integration_test.h | 7 +- test/mocks/upstream/host.cc | 4 +- test/mocks/upstream/host.h | 4 +- 21 files changed, 226 insertions(+), 24 deletions(-) diff --git a/api/envoy/extensions/upstreams/http/v3/http_protocol_options.proto b/api/envoy/extensions/upstreams/http/v3/http_protocol_options.proto index 00cac9d27336e..2ce22fe6c0a79 100644 --- a/api/envoy/extensions/upstreams/http/v3/http_protocol_options.proto +++ b/api/envoy/extensions/upstreams/http/v3/http_protocol_options.proto @@ -97,6 +97,13 @@ message HttpProtocolOptions { config.core.v3.Http1ProtocolOptions http_protocol_options = 1; config.core.v3.Http2ProtocolOptions http2_protocol_options = 2; + + // [#not-implemented-hide:] + // Unlike HTTP/1 and HTTP/2, HTTP/3 will not be configured unless it is + // present. If HTTP/3 is present, attempts to connect will first be made + // via HTTP/3, and if the HTTP/3 connection fails, TCP (HTTP/1 or HTTP/2 + // based on ALPN) will be used instead. + config.core.v3.Http3ProtocolOptions http3_protocol_options = 3; } // This contains options common across HTTP/1 and HTTP/2 diff --git a/api/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto b/api/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto index e3cf4476983a9..2011abc5a5a79 100644 --- a/api/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto +++ b/api/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto @@ -110,6 +110,13 @@ message HttpProtocolOptions { config.core.v4alpha.Http1ProtocolOptions http_protocol_options = 1; config.core.v4alpha.Http2ProtocolOptions http2_protocol_options = 2; + + // [#not-implemented-hide:] + // Unlike HTTP/1 and HTTP/2, HTTP/3 will not be configured unless it is + // present. If HTTP/3 is present, attempts to connect will first be made + // via HTTP/3, and if the HTTP/3 connection fails, TCP (HTTP/1 or HTTP/2 + // based on ALPN) will be used instead. + config.core.v4alpha.Http3ProtocolOptions http3_protocol_options = 3; } // This contains options common across HTTP/1 and HTTP/2 diff --git a/generated_api_shadow/envoy/extensions/upstreams/http/v3/http_protocol_options.proto b/generated_api_shadow/envoy/extensions/upstreams/http/v3/http_protocol_options.proto index 00cac9d27336e..2ce22fe6c0a79 100644 --- a/generated_api_shadow/envoy/extensions/upstreams/http/v3/http_protocol_options.proto +++ b/generated_api_shadow/envoy/extensions/upstreams/http/v3/http_protocol_options.proto @@ -97,6 +97,13 @@ message HttpProtocolOptions { config.core.v3.Http1ProtocolOptions http_protocol_options = 1; config.core.v3.Http2ProtocolOptions http2_protocol_options = 2; + + // [#not-implemented-hide:] + // Unlike HTTP/1 and HTTP/2, HTTP/3 will not be configured unless it is + // present. If HTTP/3 is present, attempts to connect will first be made + // via HTTP/3, and if the HTTP/3 connection fails, TCP (HTTP/1 or HTTP/2 + // based on ALPN) will be used instead. + config.core.v3.Http3ProtocolOptions http3_protocol_options = 3; } // This contains options common across HTTP/1 and HTTP/2 diff --git a/generated_api_shadow/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto b/generated_api_shadow/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto index e3cf4476983a9..2011abc5a5a79 100644 --- a/generated_api_shadow/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto +++ b/generated_api_shadow/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto @@ -110,6 +110,13 @@ message HttpProtocolOptions { config.core.v4alpha.Http1ProtocolOptions http_protocol_options = 1; config.core.v4alpha.Http2ProtocolOptions http2_protocol_options = 2; + + // [#not-implemented-hide:] + // Unlike HTTP/1 and HTTP/2, HTTP/3 will not be configured unless it is + // present. If HTTP/3 is present, attempts to connect will first be made + // via HTTP/3, and if the HTTP/3 connection fails, TCP (HTTP/1 or HTTP/2 + // based on ALPN) will be used instead. + config.core.v4alpha.Http3ProtocolOptions http3_protocol_options = 3; } // This contains options common across HTTP/1 and HTTP/2 diff --git a/include/envoy/upstream/host_description.h b/include/envoy/upstream/host_description.h index 71f91cde11dc9..efac203343c2d 100644 --- a/include/envoy/upstream/host_description.h +++ b/include/envoy/upstream/host_description.h @@ -111,8 +111,12 @@ class HostDescription { /** * @return the transport socket factory responsible for this host. + * + * In the case that a host supports both TCP and HTTP/3 for auto-failover, + * getTransportSocketFactory will return the TCP socket factory by default, + * and return the HTTP/3 socket factory if http3 is explicitly specified. */ - virtual Network::TransportSocketFactory& transportSocketFactory() const PURE; + virtual Network::TransportSocketFactory& transportSocketFactory(bool http3 = false) const PURE; /** * @return the address used to connect to the host. diff --git a/source/common/http/http3/conn_pool.cc b/source/common/http/http3/conn_pool.cc index 1184618ef01a3..4eecdf204202a 100644 --- a/source/common/http/http3/conn_pool.cc +++ b/source/common/http/http3/conn_pool.cc @@ -40,7 +40,7 @@ class Http3ConnPoolImpl : public FixedHttpConnPoolImpl { auto host_address = host->address(); source_address = Network::Utility::getLocalAddress(host_address->ip()->version()); } - Network::TransportSocketFactory& transport_socket_factory = host->transportSocketFactory(); + Network::TransportSocketFactory& transport_socket_factory = host->transportSocketFactory(true); quic_info_ = std::make_unique( dispatcher, transport_socket_factory, host->cluster().statsScope(), time_source, source_address); diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index b970eacd34199..4b8992d17d5b5 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -74,6 +74,7 @@ envoy_cc_library( "//source/common/config:xds_resource_lib", "//source/common/grpc:async_client_manager_lib", "//source/common/http:async_client_lib", + "//source/common/http:conn_pool_grid", "//source/common/http:mixed_conn_pool", "//source/common/http/http1:conn_pool_lib", "//source/common/http/http2:conn_pool_lib", diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 35a0aefeef5ae..8c1bf7d2afdc9 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -26,6 +26,7 @@ #include "common/config/xds_resource.h" #include "common/grpc/async_client_manager_impl.h" #include "common/http/async_client_impl.h" +#include "common/http/conn_pool_grid.h" #include "common/http/http1/conn_pool.h" #include "common/http/http2/conn_pool.h" #include "common/http/mixed_conn_pool.h" @@ -1505,14 +1506,19 @@ Http::ConnectionPool::InstancePtr ProdClusterManagerFactory::allocateConnPool( const Network::ConnectionSocket::OptionsSharedPtr& options, const Network::TransportSocketOptionsSharedPtr& transport_socket_options, TimeSource& source, ClusterConnectivityState& state) { - if (protocols.size() == 2) { + if (protocols.size() == 3 && runtime_.snapshot().featureEnabled("upstream.use_http3", 100)) { + Envoy::Http::ConnectivityGrid::ConnectivityOptions coptions{protocols}; + return std::make_unique(dispatcher, api_.randomGenerator(), host, + priority, options, transport_socket_options, + state, source, coptions); + } + if (protocols.size() >= 2) { ASSERT((protocols[0] == Http::Protocol::Http2 && protocols[1] == Http::Protocol::Http11) || (protocols[1] == Http::Protocol::Http2 && protocols[0] == Http::Protocol::Http11)); return std::make_unique(dispatcher, api_.randomGenerator(), host, priority, options, transport_socket_options, state); } - if (protocols.size() == 1 && protocols[0] == Http::Protocol::Http2 && runtime_.snapshot().featureEnabled("upstream.use_http2", 100)) { return Http::Http2::allocateConnPool(dispatcher, api_.randomGenerator(), host, priority, diff --git a/source/common/upstream/logical_host.cc b/source/common/upstream/logical_host.cc index 302b125cc18e7..fde1405ff05a8 100644 --- a/source/common/upstream/logical_host.cc +++ b/source/common/upstream/logical_host.cc @@ -8,7 +8,7 @@ Upstream::Host::CreateConnectionData LogicalHost::createConnection( Network::TransportSocketOptionsSharedPtr transport_socket_options) const { const auto current_address = address(); return {HostImpl::createConnection( - dispatcher, cluster(), current_address, transportSocketFactory(), options, + dispatcher, cluster(), current_address, transportSocketFactory(false), options, override_transport_socket_options_ != nullptr ? override_transport_socket_options_ : transport_socket_options), std::make_shared(current_address, shared_from_this())}; diff --git a/source/common/upstream/logical_host.h b/source/common/upstream/logical_host.h index 574728fe92bb0..b06348d391825 100644 --- a/source/common/upstream/logical_host.h +++ b/source/common/upstream/logical_host.h @@ -81,8 +81,8 @@ class RealHostDescription : public HostDescription { MetadataConstSharedPtr metadata() const override { return logical_host_->metadata(); } void metadata(MetadataConstSharedPtr) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } - Network::TransportSocketFactory& transportSocketFactory() const override { - return logical_host_->transportSocketFactory(); + Network::TransportSocketFactory& transportSocketFactory(bool http3) const override { + return logical_host_->transportSocketFactory(http3); } const ClusterInfo& cluster() const override { return logical_host_->cluster(); } HealthCheckHostMonitor& healthChecker() const override { return logical_host_->healthChecker(); } diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 5d70b2f90fabf..1248806cd3010 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -926,8 +926,11 @@ ClusterInfoImpl::upstreamHttpProtocol(absl::optional downstream_ features_ & Upstream::ClusterInfo::Features::USE_DOWNSTREAM_PROTOCOL) { return {downstream_protocol.value()}; } else if (features_ & Upstream::ClusterInfo::Features::USE_ALPN) { - ASSERT(!(features_ & Upstream::ClusterInfo::Features::HTTP3)); - return {Http::Protocol::Http2, Http::Protocol::Http11}; + if (!(features_ & Upstream::ClusterInfo::Features::HTTP3)) { + return {Http::Protocol::Http2, Http::Protocol::Http11}; + } else { + return {Http::Protocol::Http3, Http::Protocol::Http2, Http::Protocol::Http11}; + } } else { if (features_ & Upstream::ClusterInfo::Features::HTTP3) { return {Http::Protocol::Http3}; @@ -974,11 +977,19 @@ ClusterImplBase::ClusterImplBase( if (info_->features() & ClusterInfoImpl::Features::HTTP3) { #if defined(ENVOY_ENABLE_QUIC) - if (cluster.transport_socket().name() != "envoy.transport_sockets.quic") { + auto metadata = http3Metadata(); + if (!(info_->features() & ClusterInfoImpl::Features::USE_ALPN) && + cluster.transport_socket().name() != "envoy.transport_sockets.quic") { throw EnvoyException( fmt::format("HTTP3 requires a QuicUpstreamTransport transport socket: {}", cluster.name(), cluster.DebugString())); } + if (info_->features() & ClusterInfoImpl::Features::USE_ALPN && + info_->transportSocketMatcher().resolve(&metadata).name_ != "use_quic") { + throw EnvoyException( + fmt::format("HTTP3 with ALPN requires a socket match with 'use_quic': {}", cluster.name(), + cluster.DebugString())); + } #else throw EnvoyException("HTTP3 configured but not enabled in the build."); #endif diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index ec98ca4146d25..dcf6bdd7515d3 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -74,6 +74,19 @@ class HealthCheckHostMonitorNullImpl : public HealthCheckHostMonitor { void setUnhealthy(UnhealthyType) override {} }; +static envoy::config::core::v3::Metadata http3Metadata() { + // Functionally, in yaml: + // filter_metadata: + // envoy.transport_socket_match: { use_quic: "true" } + envoy::config::core::v3::Metadata metadata; + ProtobufWkt::Struct match; + ProtobufWkt::Value v; + v.set_string_value("true"); + match.mutable_fields()->insert({"use_quic", v}); + (*metadata.mutable_filter_metadata())["envoy.transport_socket_match"] = match; + return metadata; +} + /** * Implementation of Upstream::HostDescription. */ @@ -87,8 +100,12 @@ class HostDescriptionImpl : virtual public HostDescription, const envoy::config::endpoint::v3::Endpoint::HealthCheckConfig& health_check_config, uint32_t priority, TimeSource& time_source); - Network::TransportSocketFactory& transportSocketFactory() const override { - return socket_factory_; + Network::TransportSocketFactory& transportSocketFactory(bool http3) const override { + if (!http3) { + return socket_factory_; + } + auto metadata = http3Metadata(); + return resolveTransportSocketFactory(address(), &metadata); } bool canary() const override { return canary_; } diff --git a/source/extensions/upstreams/http/config.cc b/source/extensions/upstreams/http/config.cc index 9822271413b30..5cb2acc4187f3 100644 --- a/source/extensions/upstreams/http/config.cc +++ b/source/extensions/upstreams/http/config.cc @@ -48,6 +48,9 @@ getHttp3Options(const envoy::extensions::upstreams::http::v3::HttpProtocolOption options.use_downstream_protocol_config().has_http3_protocol_options()) { return options.use_downstream_protocol_config().http3_protocol_options(); } + if (options.has_auto_config()) { + return options.auto_config().http3_protocol_options(); + } return options.explicit_http_config().http3_protocol_options(); } @@ -107,6 +110,7 @@ ProtocolOptionsConfigImpl::ProtocolOptionsConfigImpl( if (options.has_auto_config()) { use_http2_ = true; use_alpn_ = true; + use_http3_ = options.auto_config().has_http3_protocol_options(); } } diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index e4b4b0948812f..c4964177d10fb 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -608,6 +608,7 @@ envoy_cc_test( "//source/common/upstream:static_cluster_lib", "//source/common/upstream:strict_dns_cluster_lib", "//source/extensions/transport_sockets/raw_buffer:config", + "//source/extensions/transport_sockets/tls:config", "//source/extensions/upstreams/http:config", "//source/server:transport_socket_config_lib", "//test/common/stats:stat_test_utility_lib", diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index fdcf38fda1e7a..6bab6c4625773 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -3250,6 +3250,85 @@ TEST_F(ClusterInfoImplTest, Http3BadConfig) { EXPECT_THROW_WITH_REGEX(makeCluster(yaml), EnvoyException, "HTTP3 requires a QuicUpstreamTransport transport socket: name.*"); } + +TEST_F(ClusterInfoImplTest, Http3Alpn) { + const std::string yaml = TestEnvironment::substitute(R"EOF( + name: name + connect_timeout: 0.25s + type: STRICT_DNS + lb_policy: MAGLEV + load_assignment: + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: foo.bar.com + port_value: 443 + transport_socket: + name: envoy.transport_sockets.tls + typed_config: + "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext + common_tls_context: + tls_certificates: + - certificate_chain: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_cert.pem" + private_key: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_key.pem" + validation_context: + trusted_ca: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" + match_subject_alt_names: + - exact: localhost + - exact: 127.0.0.1 + transport_socket_matches: + - name: use_quic + match: + use_quic: true + transport_socket: + name: envoy.transport_sockets.quic + typed_config: + "@type": type.googleapis.com/envoy.extensions.transport_sockets.quic.v3.QuicUpstreamTransport + upstream_tls_context: + common_tls_context: + tls_certificates: + - certificate_chain: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_cert.pem" + private_key: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_key.pem" + validation_context: + trusted_ca: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" + match_subject_alt_names: + - exact: localhost + - exact: 127.0.0.1 + )EOF", + Network::Address::IpVersion::v4); + + auto cluster1 = makeCluster(yaml); + ASSERT_TRUE(cluster1->info()->idleTimeout().has_value()); + EXPECT_EQ(std::chrono::hours(1), cluster1->info()->idleTimeout().value()); + + const std::string explicit_http3 = R"EOF( + typed_extension_protocol_options: + envoy.extensions.upstreams.http.v3.HttpProtocolOptions: + "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions + auto_config: + http3_protocol_options: + quic_protocol_options: + max_concurrent_streams: 2 + common_http_protocol_options: + idle_timeout: 1s + )EOF"; + + auto explicit_h3 = makeCluster(yaml + explicit_http3); + EXPECT_EQ(Http::Protocol::Http3, + explicit_h3->info()->upstreamHttpProtocol({Http::Protocol::Http10})[0]); + EXPECT_EQ( + explicit_h3->info()->http3Options().quic_protocol_options().max_concurrent_streams().value(), + 2); +} + #else TEST_F(ClusterInfoImplTest, Http3BadConfig) { const std::string yaml = TestEnvironment::substitute(R"EOF( diff --git a/test/config/utility.cc b/test/config/utility.cc index 7dfbbceba7765..e5e7e168ada09 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -713,6 +713,10 @@ void ConfigHelper::configureUpstreamTls(bool use_alpn, bool http3) { new_protocol_options.mutable_auto_config()->mutable_http2_protocol_options()->MergeFrom( old_protocol_options.explicit_http_config().http2_protocol_options()); } + if (http3 || old_protocol_options.explicit_http_config().has_http3_protocol_options()) { + new_protocol_options.mutable_auto_config()->mutable_http3_protocol_options()->MergeFrom( + old_protocol_options.explicit_http_config().http3_protocol_options()); + } (*cluster->mutable_typed_extension_protocol_options()) ["envoy.extensions.upstreams.http.v3.HttpProtocolOptions"] .PackFrom(new_protocol_options); @@ -725,10 +729,23 @@ void ConfigHelper::configureUpstreamTls(bool use_alpn, bool http3) { // The test certs are for *.lyft.com, so make sure SNI matches. tls_context.set_sni("foo.lyft.com"); if (http3) { - cluster->mutable_transport_socket()->set_name("envoy.transport_sockets.quic"); envoy::extensions::transport_sockets::quic::v3::QuicUpstreamTransport quic_context; quic_context.mutable_upstream_tls_context()->CopyFrom(tls_context); - cluster->mutable_transport_socket()->mutable_typed_config()->PackFrom(quic_context); + if (use_alpn) { + // Configure both HTTP/2 and HTTP/3 + cluster->mutable_transport_socket()->set_name("envoy.transport_sockets.tls"); + cluster->mutable_transport_socket()->mutable_typed_config()->PackFrom(tls_context); + auto* match = cluster->add_transport_socket_matches(); + ProtobufWkt::Value v; + v.set_string_value("true"); + match->mutable_match()->mutable_fields()->insert({"use_quic", v}); + match->set_name("use_quic"); + match->mutable_transport_socket()->set_name("envoy.transport_sockets.quic"); + match->mutable_transport_socket()->mutable_typed_config()->PackFrom(quic_context); + } else { + cluster->mutable_transport_socket()->set_name("envoy.transport_sockets.quic"); + cluster->mutable_transport_socket()->mutable_typed_config()->PackFrom(quic_context); + } } else { cluster->mutable_transport_socket()->set_name("envoy.transport_sockets.tls"); cluster->mutable_transport_socket()->mutable_typed_config()->PackFrom(tls_context); diff --git a/test/integration/base_integration_test.h b/test/integration/base_integration_test.h index cc3008b249f27..9f378ec5f19c2 100644 --- a/test/integration/base_integration_test.h +++ b/test/integration/base_integration_test.h @@ -468,7 +468,6 @@ class BaseIntegrationTest : protected Logger::Loggable { // Use a v2 bootstrap. bool v2_bootstrap_{false}; -private: // Configuration for the fake upstream. FakeUpstreamConfig upstream_config_{time_system_}; // True if initialized() has been called. diff --git a/test/integration/multiplexed_upstream_integration_test.cc b/test/integration/multiplexed_upstream_integration_test.cc index c9d98d42fdcb6..39b9a9001033e 100644 --- a/test/integration/multiplexed_upstream_integration_test.cc +++ b/test/integration/multiplexed_upstream_integration_test.cc @@ -262,6 +262,7 @@ TEST_P(Http2UpstreamIntegrationTest, SimultaneousRequestAlpn) { } TEST_P(Http2UpstreamIntegrationTest, LargeSimultaneousRequestWithBufferLimitsAlpn) { + EXCLUDE_UPSTREAM_HTTP3; // No H3 support yet. use_alpn_ = true; config_helper_.setBufferLimits(1024, 1024); // Set buffer limits upstream and downstream. simultaneousRequest(1024 * 20, 1024 * 14 + 2, 1024 * 10 + 5, 1024 * 16); @@ -619,4 +620,41 @@ TEST_P(Http2UpstreamIntegrationTest, UpstreamGoaway) { cleanupUpstreamAndDownstream(); } +#ifdef ENVOY_ENABLE_QUIC + +class MixedUpstreamIntegrationTest : public Http2UpstreamIntegrationTest { +protected: + void initialize() override { + use_alpn_ = true; + Http2UpstreamIntegrationTest::initialize(); + } + void createUpstreams() override { + ASSERT_EQ(upstreamProtocol(), FakeHttpConnection::Type::HTTP3); + if (use_http2_) { + upstream_config_.udp_fake_upstream_ = absl::nullopt; + upstream_config_.upstream_protocol_ = FakeHttpConnection::Type::HTTP2; + } + Http2UpstreamIntegrationTest::createUpstreams(); + upstream_config_.upstream_protocol_ = FakeHttpConnection::Type::HTTP3; + } + + bool use_http2_{false}; +}; + +TEST_P(MixedUpstreamIntegrationTest, SimultaneousRequestAutoWithHttp3) { + testRouterRequestAndResponseWithBody(0, 0, false); +} + +TEST_P(MixedUpstreamIntegrationTest, SimultaneousRequestAutoWithHttp2) { + use_http2_ = true; + testRouterRequestAndResponseWithBody(0, 0, false); +} + +INSTANTIATE_TEST_SUITE_P(Protocols, MixedUpstreamIntegrationTest, + testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( + {Http::CodecClient::Type::HTTP2}, {FakeHttpConnection::Type::HTTP3})), + HttpProtocolIntegrationTest::protocolTestParamsToString); + +#endif + } // namespace Envoy diff --git a/test/integration/multiplexed_upstream_integration_test.h b/test/integration/multiplexed_upstream_integration_test.h index 09d51ecf5fcf8..45e35a83b42e6 100644 --- a/test/integration/multiplexed_upstream_integration_test.h +++ b/test/integration/multiplexed_upstream_integration_test.h @@ -7,16 +7,13 @@ namespace Envoy { class Http2UpstreamIntegrationTest : public HttpProtocolIntegrationTest { public: - void SetUp() override { - HttpProtocolIntegrationTest::SetUp(); - + void initialize() override { upstream_tls_ = true; config_helper_.configureUpstreamTls(use_alpn_, upstreamProtocol() == FakeHttpConnection::Type::HTTP3); + HttpProtocolIntegrationTest::initialize(); } - void initialize() override { HttpIntegrationTest::initialize(); } - void bidirectionalStreaming(uint32_t bytes); void simultaneousRequest(uint32_t request1_bytes, uint32_t request2_bytes, uint32_t response1_bytes, uint32_t response2_bytes); diff --git a/test/mocks/upstream/host.cc b/test/mocks/upstream/host.cc index 0670abadd5c01..4e01c26af437e 100644 --- a/test/mocks/upstream/host.cc +++ b/test/mocks/upstream/host.cc @@ -40,7 +40,7 @@ MockHostDescription::MockHostDescription() ON_CALL(*this, locality()).WillByDefault(ReturnRef(locality_)); ON_CALL(*this, cluster()).WillByDefault(ReturnRef(cluster_)); ON_CALL(*this, healthChecker()).WillByDefault(ReturnRef(health_checker_)); - ON_CALL(*this, transportSocketFactory()).WillByDefault(ReturnRef(*socket_factory_)); + ON_CALL(*this, transportSocketFactory(_)).WillByDefault(ReturnRef(*socket_factory_)); } MockHostDescription::~MockHostDescription() = default; @@ -50,7 +50,7 @@ MockHost::MockHost() : socket_factory_(new testing::NiceMock Date: Mon, 12 Apr 2021 16:07:43 -0400 Subject: [PATCH 2/5] Removing duplicate TLS config Signed-off-by: Alyssa Wilk --- include/envoy/upstream/host_description.h | 6 +- source/common/http/conn_pool_grid.cc | 16 +----- source/common/http/http3/conn_pool.cc | 2 +- source/common/quic/BUILD | 1 + .../quic/quic_transport_socket_factory.cc | 14 ++++- .../quic/quic_transport_socket_factory.h | 39 +++++++++++-- .../common/upstream/cluster_manager_impl.cc | 20 ++++++- source/common/upstream/logical_host.cc | 2 +- source/common/upstream/logical_host.h | 4 +- source/common/upstream/upstream_impl.cc | 30 ++++------ source/common/upstream/upstream_impl.h | 21 +------ .../transport_sockets/tls/ssl_socket.h | 2 + test/common/upstream/upstream_impl_test.cc | 55 ++++++------------- test/config/utility.cc | 17 +----- test/integration/base_integration_test.h | 3 + .../multiplexed_upstream_integration_test.cc | 4 ++ test/mocks/server/BUILD | 1 + .../transport_socket_factory_context.cc | 2 + .../server/transport_socket_factory_context.h | 4 ++ test/mocks/upstream/host.cc | 4 +- test/mocks/upstream/host.h | 4 +- 21 files changed, 124 insertions(+), 127 deletions(-) diff --git a/include/envoy/upstream/host_description.h b/include/envoy/upstream/host_description.h index efac203343c2d..71f91cde11dc9 100644 --- a/include/envoy/upstream/host_description.h +++ b/include/envoy/upstream/host_description.h @@ -111,12 +111,8 @@ class HostDescription { /** * @return the transport socket factory responsible for this host. - * - * In the case that a host supports both TCP and HTTP/3 for auto-failover, - * getTransportSocketFactory will return the TCP socket factory by default, - * and return the HTTP/3 socket factory if http3 is explicitly specified. */ - virtual Network::TransportSocketFactory& transportSocketFactory(bool http3 = false) const PURE; + virtual Network::TransportSocketFactory& transportSocketFactory() const PURE; /** * @return the address used to connect to the host. diff --git a/source/common/http/conn_pool_grid.cc b/source/common/http/conn_pool_grid.cc index a6b03d56f406e..9c917727908b9 100644 --- a/source/common/http/conn_pool_grid.cc +++ b/source/common/http/conn_pool_grid.cc @@ -6,18 +6,6 @@ namespace Envoy { namespace Http { -// Helper function to make sure each protocol in expected_protocols is present -// in protocols (only used for an ASSERT in debug builds) -bool contains(const std::vector& protocols, - const std::vector& expected_protocols) { - for (auto protocol : expected_protocols) { - if (std::find(protocols.begin(), protocols.end(), protocol) == protocols.end()) { - return false; - } - } - return true; -} - absl::string_view describePool(const ConnectionPool::Instance& pool) { return pool.protocolDescription(); } @@ -129,10 +117,10 @@ ConnectivityGrid::ConnectivityGrid( : dispatcher_(dispatcher), random_generator_(random_generator), host_(host), priority_(priority), options_(options), transport_socket_options_(transport_socket_options), state_(state), next_attempt_duration_(next_attempt_duration), time_source_(time_source) { + // ProdClusterManagerFactory::allocateConnPool verifies the protocols are HTTP/1, HTTP/2 and + // HTTP/3. // TODO(#15649) support v6/v4, WiFi/cellular. ASSERT(connectivity_options.protocols_.size() == 3); - ASSERT(contains(connectivity_options.protocols_, - {Http::Protocol::Http11, Http::Protocol::Http2, Http::Protocol::Http3})); } ConnectivityGrid::~ConnectivityGrid() { diff --git a/source/common/http/http3/conn_pool.cc b/source/common/http/http3/conn_pool.cc index 4eecdf204202a..1184618ef01a3 100644 --- a/source/common/http/http3/conn_pool.cc +++ b/source/common/http/http3/conn_pool.cc @@ -40,7 +40,7 @@ class Http3ConnPoolImpl : public FixedHttpConnPoolImpl { auto host_address = host->address(); source_address = Network::Utility::getLocalAddress(host_address->ip()->version()); } - Network::TransportSocketFactory& transport_socket_factory = host->transportSocketFactory(true); + Network::TransportSocketFactory& transport_socket_factory = host->transportSocketFactory(); quic_info_ = std::make_unique( dispatcher, transport_socket_factory, host->cluster().statsScope(), time_source, source_address); diff --git a/source/common/quic/BUILD b/source/common/quic/BUILD index 506ee80b4294f..d0f3b48491c6b 100644 --- a/source/common/quic/BUILD +++ b/source/common/quic/BUILD @@ -371,6 +371,7 @@ envoy_cc_library( "//source/common/common:assert_lib", "//source/extensions/transport_sockets:well_known_names", "//source/extensions/transport_sockets/tls:context_config_lib", + "//source/extensions/transport_sockets/tls:ssl_socket_lib", "@envoy_api//envoy/extensions/transport_sockets/quic/v3:pkg_cc_proto", ], ) diff --git a/source/common/quic/quic_transport_socket_factory.cc b/source/common/quic/quic_transport_socket_factory.cc index 4e87a90606721..2930329f62958 100644 --- a/source/common/quic/quic_transport_socket_factory.cc +++ b/source/common/quic/quic_transport_socket_factory.cc @@ -1,8 +1,7 @@ #include "common/quic/quic_transport_socket_factory.h" -// #include "envoy/extensions/transport_sockets/tls/v3/tls.pb.validate.h" -#include "envoy/extensions/transport_sockets/quic/v3/quic_transport.pb.h" #include "envoy/extensions/transport_sockets/quic/v3/quic_transport.pb.validate.h" + #include "extensions/transport_sockets/tls/context_config_impl.h" namespace Envoy { @@ -34,7 +33,16 @@ QuicClientTransportSocketConfigFactory::createTransportSocketFactory( config, context.messageValidationVisitor()); auto client_config = std::make_unique( quic_transport.upstream_tls_context(), context); - return std::make_unique(std::move(client_config)); + return std::make_unique(std::move(client_config), context); +} + +void QuicClientTransportSocketFactory::maybeCreateFallbackFactory() { + if (fallback_factory_) { + return; + } + + fallback_factory_ = std::make_unique( + std::move(config_), manager_, scope_); } ProtobufTypes::MessagePtr QuicClientTransportSocketConfigFactory::createEmptyConfigProto() { diff --git a/source/common/quic/quic_transport_socket_factory.h b/source/common/quic/quic_transport_socket_factory.h index fdcdd557c91bb..5cacc17941567 100644 --- a/source/common/quic/quic_transport_socket_factory.h +++ b/source/common/quic/quic_transport_socket_factory.h @@ -1,11 +1,13 @@ #pragma once +#include "envoy/extensions/transport_sockets/quic/v3/quic_transport.pb.h" #include "envoy/network/transport_socket.h" #include "envoy/server/transport_socket_config.h" #include "envoy/ssl/context_config.h" #include "common/common/assert.h" +#include "extensions/transport_sockets/tls/ssl_socket.h" #include "extensions/transport_sockets/well_known_names.h" namespace Envoy { @@ -25,6 +27,7 @@ class QuicTransportSocketFactoryBase : public Network::TransportSocketFactory { } bool implementsSecureTransport() const override { return true; } bool usesProxyProtocolOptions() const override { return false; } + bool supportsAlpn() const override { return true; } }; // TODO(danzh): when implement ProofSource, examine of it's necessary to @@ -42,13 +45,41 @@ class QuicServerTransportSocketFactory : public QuicTransportSocketFactoryBase { class QuicClientTransportSocketFactory : public QuicTransportSocketFactoryBase { public: - QuicClientTransportSocketFactory(Envoy::Ssl::ClientContextConfigPtr config) - : config_(std::move(config)) {} + QuicClientTransportSocketFactory( + Ssl::ClientContextConfigPtr config, + Server::Configuration::TransportSocketFactoryContext& factory_context) + : config_(std::move(config)), manager_(factory_context.sslContextManager()), + scope_(factory_context.scope()) {} + + // As documented above for QuicTransportSocketFactoryBase, the actual HTTP/3 + // code does not create transport sockets. + // QuicClientTransportSocketFactory::createTransportSocket is called by the + // connection grid when uptream HTTP/3 fails over to TCP, and a raw SSL socket + // is needed. In this case the QuicClientTransportSocketFactory falls over to + // using the fallback factory. + Network::TransportSocketPtr + createTransportSocket(Network::TransportSocketOptionsSharedPtr options) const override { + // TODO(#15649) make createTransportSocket non-const to avoid the const cast. + const_cast(this)->maybeCreateFallbackFactory(); + return fallback_factory_->createTransportSocket(options); + } - const Ssl::ClientContextConfig& clientContextConfig() const { return *config_; } + // Creates fallback_factory_ if it does not already exist. + void maybeCreateFallbackFactory(); + + // Generally the QuicClientTransportSocketFactory owns its own client context + // config, but in the case of failover to TCP, it hands it off to the fallback_factory_ + const Ssl::ClientContextConfig& clientContextConfig() const { + return fallback_factory_ ? fallback_factory_->config() : *config_; + } private: - std::unique_ptr config_; + Envoy::Ssl::ClientContextConfigPtr config_; + Envoy::Ssl::ContextManager& manager_; + Stats::Scope& scope_; + envoy::extensions::transport_sockets::quic::v3::QuicUpstreamTransport quic_config_; + // The QUIC client transport socket can create TLS sockets for fallback to TCP. + std::unique_ptr fallback_factory_; }; // Base class to create above QuicTransportSocketFactory for server and client diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 8c1bf7d2afdc9..c21cf85ac06e2 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -60,6 +60,18 @@ void addOptionsIfNotNull(Network::Socket::OptionsSharedPtr& options, } } +// Helper function to make sure each protocol in expected_protocols is present +// in protocols (only used for an ASSERT in debug builds) +bool contains(const std::vector& protocols, + const std::vector& expected_protocols) { + for (auto protocol : expected_protocols) { + if (std::find(protocols.begin(), protocols.end(), protocol) == protocols.end()) { + return false; + } + } + return true; +} + } // namespace void ClusterManagerInitHelper::addCluster(ClusterManagerCluster& cm_cluster) { @@ -1507,10 +1519,12 @@ Http::ConnectionPool::InstancePtr ProdClusterManagerFactory::allocateConnPool( const Network::TransportSocketOptionsSharedPtr& transport_socket_options, TimeSource& source, ClusterConnectivityState& state) { if (protocols.size() == 3 && runtime_.snapshot().featureEnabled("upstream.use_http3", 100)) { + ASSERT(contains(protocols, + {Http::Protocol::Http11, Http::Protocol::Http2, Http::Protocol::Http3})); Envoy::Http::ConnectivityGrid::ConnectivityOptions coptions{protocols}; - return std::make_unique(dispatcher, api_.randomGenerator(), host, - priority, options, transport_socket_options, - state, source, coptions); + return std::make_unique( + dispatcher, api_.randomGenerator(), host, priority, options, transport_socket_options, + state, source, std::chrono::milliseconds(300), coptions); } if (protocols.size() >= 2) { ASSERT((protocols[0] == Http::Protocol::Http2 && protocols[1] == Http::Protocol::Http11) || diff --git a/source/common/upstream/logical_host.cc b/source/common/upstream/logical_host.cc index fde1405ff05a8..302b125cc18e7 100644 --- a/source/common/upstream/logical_host.cc +++ b/source/common/upstream/logical_host.cc @@ -8,7 +8,7 @@ Upstream::Host::CreateConnectionData LogicalHost::createConnection( Network::TransportSocketOptionsSharedPtr transport_socket_options) const { const auto current_address = address(); return {HostImpl::createConnection( - dispatcher, cluster(), current_address, transportSocketFactory(false), options, + dispatcher, cluster(), current_address, transportSocketFactory(), options, override_transport_socket_options_ != nullptr ? override_transport_socket_options_ : transport_socket_options), std::make_shared(current_address, shared_from_this())}; diff --git a/source/common/upstream/logical_host.h b/source/common/upstream/logical_host.h index b06348d391825..574728fe92bb0 100644 --- a/source/common/upstream/logical_host.h +++ b/source/common/upstream/logical_host.h @@ -81,8 +81,8 @@ class RealHostDescription : public HostDescription { MetadataConstSharedPtr metadata() const override { return logical_host_->metadata(); } void metadata(MetadataConstSharedPtr) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } - Network::TransportSocketFactory& transportSocketFactory(bool http3) const override { - return logical_host_->transportSocketFactory(http3); + Network::TransportSocketFactory& transportSocketFactory() const override { + return logical_host_->transportSocketFactory(); } const ClusterInfo& cluster() const override { return logical_host_->cluster(); } HealthCheckHostMonitor& healthChecker() const override { return logical_host_->healthChecker(); } diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 1248806cd3010..070063f768589 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -925,19 +925,21 @@ ClusterInfoImpl::upstreamHttpProtocol(absl::optional downstream_ if (downstream_protocol.has_value() && features_ & Upstream::ClusterInfo::Features::USE_DOWNSTREAM_PROTOCOL) { return {downstream_protocol.value()}; - } else if (features_ & Upstream::ClusterInfo::Features::USE_ALPN) { + } + + if (features_ & Upstream::ClusterInfo::Features::USE_ALPN) { if (!(features_ & Upstream::ClusterInfo::Features::HTTP3)) { return {Http::Protocol::Http2, Http::Protocol::Http11}; - } else { - return {Http::Protocol::Http3, Http::Protocol::Http2, Http::Protocol::Http11}; } - } else { - if (features_ & Upstream::ClusterInfo::Features::HTTP3) { - return {Http::Protocol::Http3}; - } - return {(features_ & Upstream::ClusterInfo::Features::HTTP2) ? Http::Protocol::Http2 - : Http::Protocol::Http11}; + return {Http::Protocol::Http3, Http::Protocol::Http2, Http::Protocol::Http11}; } + + if (features_ & Upstream::ClusterInfo::Features::HTTP3) { + return {Http::Protocol::Http3}; + } + + return {(features_ & Upstream::ClusterInfo::Features::HTTP2) ? Http::Protocol::Http2 + : Http::Protocol::Http11}; } ClusterImplBase::ClusterImplBase( @@ -977,19 +979,11 @@ ClusterImplBase::ClusterImplBase( if (info_->features() & ClusterInfoImpl::Features::HTTP3) { #if defined(ENVOY_ENABLE_QUIC) - auto metadata = http3Metadata(); - if (!(info_->features() & ClusterInfoImpl::Features::USE_ALPN) && - cluster.transport_socket().name() != "envoy.transport_sockets.quic") { + if (cluster.transport_socket().name() != "envoy.transport_sockets.quic") { throw EnvoyException( fmt::format("HTTP3 requires a QuicUpstreamTransport transport socket: {}", cluster.name(), cluster.DebugString())); } - if (info_->features() & ClusterInfoImpl::Features::USE_ALPN && - info_->transportSocketMatcher().resolve(&metadata).name_ != "use_quic") { - throw EnvoyException( - fmt::format("HTTP3 with ALPN requires a socket match with 'use_quic': {}", cluster.name(), - cluster.DebugString())); - } #else throw EnvoyException("HTTP3 configured but not enabled in the build."); #endif diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index dcf6bdd7515d3..ec98ca4146d25 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -74,19 +74,6 @@ class HealthCheckHostMonitorNullImpl : public HealthCheckHostMonitor { void setUnhealthy(UnhealthyType) override {} }; -static envoy::config::core::v3::Metadata http3Metadata() { - // Functionally, in yaml: - // filter_metadata: - // envoy.transport_socket_match: { use_quic: "true" } - envoy::config::core::v3::Metadata metadata; - ProtobufWkt::Struct match; - ProtobufWkt::Value v; - v.set_string_value("true"); - match.mutable_fields()->insert({"use_quic", v}); - (*metadata.mutable_filter_metadata())["envoy.transport_socket_match"] = match; - return metadata; -} - /** * Implementation of Upstream::HostDescription. */ @@ -100,12 +87,8 @@ class HostDescriptionImpl : virtual public HostDescription, const envoy::config::endpoint::v3::Endpoint::HealthCheckConfig& health_check_config, uint32_t priority, TimeSource& time_source); - Network::TransportSocketFactory& transportSocketFactory(bool http3) const override { - if (!http3) { - return socket_factory_; - } - auto metadata = http3Metadata(); - return resolveTransportSocketFactory(address(), &metadata); + Network::TransportSocketFactory& transportSocketFactory() const override { + return socket_factory_; } bool canary() const override { return canary_; } diff --git a/source/extensions/transport_sockets/tls/ssl_socket.h b/source/extensions/transport_sockets/tls/ssl_socket.h index 534dcc29fc472..5b9239d266053 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.h +++ b/source/extensions/transport_sockets/tls/ssl_socket.h @@ -116,6 +116,8 @@ class ClientSslSocketFactory : public Network::TransportSocketFactory, // Secret::SecretCallbacks void onAddOrUpdateSecret() override; + const Ssl::ClientContextConfig& config() const { return *config_; } + private: Envoy::Ssl::ContextManager& manager_; Stats::Scope& stats_scope_; diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index 6bab6c4625773..bab8b51cc6e67 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -62,7 +62,7 @@ class UpstreamImplTestBase { UpstreamImplTestBase() : api_(Api::createApiForTest(stats_, random_)) {} NiceMock admin_; - Ssl::MockContextManager ssl_context_manager_; + NiceMock ssl_context_manager_; NiceMock cm_; NiceMock local_info_; NiceMock dispatcher_; @@ -2218,7 +2218,7 @@ class ClusterInfoImplTest : public testing::Test { }; Stats::TestUtil::TestStore stats_; - Ssl::MockContextManager ssl_context_manager_; + NiceMock ssl_context_manager_; std::shared_ptr dns_resolver_{new NiceMock()}; NiceMock dispatcher_; NiceMock runtime_; @@ -3165,7 +3165,6 @@ TEST_F(ClusterInfoImplTest, Http3) { - exact: 127.0.0.1 )EOF", Network::Address::IpVersion::v4); - auto cluster1 = makeCluster(yaml); ASSERT_TRUE(cluster1->info()->idleTimeout().has_value()); EXPECT_EQ(std::chrono::hours(1), cluster1->info()->idleTimeout().value()); @@ -3266,42 +3265,22 @@ TEST_F(ClusterInfoImplTest, Http3Alpn) { address: foo.bar.com port_value: 443 transport_socket: - name: envoy.transport_sockets.tls + name: envoy.transport_sockets.quic typed_config: - "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext - common_tls_context: - tls_certificates: - - certificate_chain: - filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_cert.pem" - private_key: - filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_key.pem" - validation_context: - trusted_ca: - filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" - match_subject_alt_names: - - exact: localhost - - exact: 127.0.0.1 - transport_socket_matches: - - name: use_quic - match: - use_quic: true - transport_socket: - name: envoy.transport_sockets.quic - typed_config: - "@type": type.googleapis.com/envoy.extensions.transport_sockets.quic.v3.QuicUpstreamTransport - upstream_tls_context: - common_tls_context: - tls_certificates: - - certificate_chain: - filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_cert.pem" - private_key: - filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_key.pem" - validation_context: - trusted_ca: - filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" - match_subject_alt_names: - - exact: localhost - - exact: 127.0.0.1 + "@type": type.googleapis.com/envoy.extensions.transport_sockets.quic.v3.QuicUpstreamTransport + upstream_tls_context: + common_tls_context: + tls_certificates: + - certificate_chain: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_cert.pem" + private_key: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_uri_key.pem" + validation_context: + trusted_ca: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem" + match_subject_alt_names: + - exact: localhost + - exact: 127.0.0.1 )EOF", Network::Address::IpVersion::v4); diff --git a/test/config/utility.cc b/test/config/utility.cc index e5e7e168ada09..8da3ea9c1bffc 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -731,21 +731,8 @@ void ConfigHelper::configureUpstreamTls(bool use_alpn, bool http3) { if (http3) { envoy::extensions::transport_sockets::quic::v3::QuicUpstreamTransport quic_context; quic_context.mutable_upstream_tls_context()->CopyFrom(tls_context); - if (use_alpn) { - // Configure both HTTP/2 and HTTP/3 - cluster->mutable_transport_socket()->set_name("envoy.transport_sockets.tls"); - cluster->mutable_transport_socket()->mutable_typed_config()->PackFrom(tls_context); - auto* match = cluster->add_transport_socket_matches(); - ProtobufWkt::Value v; - v.set_string_value("true"); - match->mutable_match()->mutable_fields()->insert({"use_quic", v}); - match->set_name("use_quic"); - match->mutable_transport_socket()->set_name("envoy.transport_sockets.quic"); - match->mutable_transport_socket()->mutable_typed_config()->PackFrom(quic_context); - } else { - cluster->mutable_transport_socket()->set_name("envoy.transport_sockets.quic"); - cluster->mutable_transport_socket()->mutable_typed_config()->PackFrom(quic_context); - } + cluster->mutable_transport_socket()->set_name("envoy.transport_sockets.quic"); + cluster->mutable_transport_socket()->mutable_typed_config()->PackFrom(quic_context); } else { cluster->mutable_transport_socket()->set_name("envoy.transport_sockets.tls"); cluster->mutable_transport_socket()->mutable_typed_config()->PackFrom(tls_context); diff --git a/test/integration/base_integration_test.h b/test/integration/base_integration_test.h index 9f378ec5f19c2..1baab383b20e8 100644 --- a/test/integration/base_integration_test.h +++ b/test/integration/base_integration_test.h @@ -468,6 +468,9 @@ class BaseIntegrationTest : protected Logger::Loggable { // Use a v2 bootstrap. bool v2_bootstrap_{false}; +private: + friend class MixedUpstreamIntegrationTest; + // Configuration for the fake upstream. FakeUpstreamConfig upstream_config_{time_system_}; // True if initialized() has been called. diff --git a/test/integration/multiplexed_upstream_integration_test.cc b/test/integration/multiplexed_upstream_integration_test.cc index 39b9a9001033e..87d0657d8e797 100644 --- a/test/integration/multiplexed_upstream_integration_test.cc +++ b/test/integration/multiplexed_upstream_integration_test.cc @@ -631,6 +631,10 @@ class MixedUpstreamIntegrationTest : public Http2UpstreamIntegrationTest { void createUpstreams() override { ASSERT_EQ(upstreamProtocol(), FakeHttpConnection::Type::HTTP3); if (use_http2_) { + // Generally we always want to set these fields via accessors, which + // changes both the upstreams and Envoy's configuration at the same time. + // In this particular case, we want to change the upstreams without + // touching config, so edit the raw members directly. upstream_config_.udp_fake_upstream_ = absl::nullopt; upstream_config_.upstream_protocol_ = FakeHttpConnection::Type::HTTP2; } diff --git a/test/mocks/server/BUILD b/test/mocks/server/BUILD index 1ab093a3b698d..7f4408de326d1 100644 --- a/test/mocks/server/BUILD +++ b/test/mocks/server/BUILD @@ -242,6 +242,7 @@ envoy_cc_mock( "//source/common/secret:secret_manager_impl_lib", "//test/mocks/api:api_mocks", "//test/mocks/server:config_tracker_mocks", + "//test/mocks/ssl:ssl_mocks", "//test/mocks/upstream:cluster_manager_mocks", ], ) diff --git a/test/mocks/server/transport_socket_factory_context.cc b/test/mocks/server/transport_socket_factory_context.cc index 0e4e50231a7d7..88ea41bd20fab 100644 --- a/test/mocks/server/transport_socket_factory_context.cc +++ b/test/mocks/server/transport_socket_factory_context.cc @@ -17,6 +17,8 @@ MockTransportSocketFactoryContext::MockTransportSocketFactoryContext() ON_CALL(*this, api()).WillByDefault(ReturnRef(api_)); ON_CALL(*this, messageValidationVisitor()) .WillByDefault(ReturnRef(ProtobufMessage::getStrictValidationVisitor())); + ON_CALL(*this, sslContextManager()).WillByDefault(ReturnRef(context_manager_)); + ON_CALL(*this, scope()).WillByDefault(ReturnRef(store_)); } MockTransportSocketFactoryContext::~MockTransportSocketFactoryContext() = default; diff --git a/test/mocks/server/transport_socket_factory_context.h b/test/mocks/server/transport_socket_factory_context.h index 2e346518cf935..f14bdd0ac9fe1 100644 --- a/test/mocks/server/transport_socket_factory_context.h +++ b/test/mocks/server/transport_socket_factory_context.h @@ -5,6 +5,8 @@ #include "common/secret/secret_manager_impl.h" #include "test/mocks/api/mocks.h" +#include "test/mocks/ssl/mocks.h" +#include "test/mocks/stats/mocks.h" #include "test/mocks/upstream/cluster_manager.h" #include "config_tracker.h" @@ -38,6 +40,8 @@ class MockTransportSocketFactoryContext : public TransportSocketFactoryContext { testing::NiceMock cluster_manager_; testing::NiceMock api_; testing::NiceMock config_tracker_; + testing::NiceMock context_manager_; + testing::NiceMock store_; std::unique_ptr secret_manager_; }; } // namespace Configuration diff --git a/test/mocks/upstream/host.cc b/test/mocks/upstream/host.cc index 4e01c26af437e..0670abadd5c01 100644 --- a/test/mocks/upstream/host.cc +++ b/test/mocks/upstream/host.cc @@ -40,7 +40,7 @@ MockHostDescription::MockHostDescription() ON_CALL(*this, locality()).WillByDefault(ReturnRef(locality_)); ON_CALL(*this, cluster()).WillByDefault(ReturnRef(cluster_)); ON_CALL(*this, healthChecker()).WillByDefault(ReturnRef(health_checker_)); - ON_CALL(*this, transportSocketFactory(_)).WillByDefault(ReturnRef(*socket_factory_)); + ON_CALL(*this, transportSocketFactory()).WillByDefault(ReturnRef(*socket_factory_)); } MockHostDescription::~MockHostDescription() = default; @@ -50,7 +50,7 @@ MockHost::MockHost() : socket_factory_(new testing::NiceMock Date: Tue, 13 Apr 2021 13:50:24 -0400 Subject: [PATCH 3/5] spelling Signed-off-by: Alyssa Wilk --- source/common/quic/quic_transport_socket_factory.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/quic/quic_transport_socket_factory.h b/source/common/quic/quic_transport_socket_factory.h index 5cacc17941567..96391bb3db92e 100644 --- a/source/common/quic/quic_transport_socket_factory.h +++ b/source/common/quic/quic_transport_socket_factory.h @@ -54,7 +54,7 @@ class QuicClientTransportSocketFactory : public QuicTransportSocketFactoryBase { // As documented above for QuicTransportSocketFactoryBase, the actual HTTP/3 // code does not create transport sockets. // QuicClientTransportSocketFactory::createTransportSocket is called by the - // connection grid when uptream HTTP/3 fails over to TCP, and a raw SSL socket + // connection grid when upstream HTTP/3 fails over to TCP, and a raw SSL socket // is needed. In this case the QuicClientTransportSocketFactory falls over to // using the fallback factory. Network::TransportSocketPtr From c3f7e0abef7dba4065960a480cf44e64229a8400 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Wed, 14 Apr 2021 09:35:30 -0400 Subject: [PATCH 4/5] fixing ~http3 build Signed-off-by: Alyssa Wilk --- source/common/upstream/BUILD | 2 +- source/common/upstream/cluster_manager_impl.cc | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index 4b8992d17d5b5..efcb951029fcc 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -74,7 +74,6 @@ envoy_cc_library( "//source/common/config:xds_resource_lib", "//source/common/grpc:async_client_manager_lib", "//source/common/http:async_client_lib", - "//source/common/http:conn_pool_grid", "//source/common/http:mixed_conn_pool", "//source/common/http/http1:conn_pool_lib", "//source/common/http/http2:conn_pool_lib", @@ -93,6 +92,7 @@ envoy_cc_library( "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ] + envoy_select_enable_http3([ "//source/common/http/http3:conn_pool_lib", + "//source/common/http:conn_pool_grid", ]), ) diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index c21cf85ac06e2..b739d2e24b0e6 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -26,7 +26,6 @@ #include "common/config/xds_resource.h" #include "common/grpc/async_client_manager_impl.h" #include "common/http/async_client_impl.h" -#include "common/http/conn_pool_grid.h" #include "common/http/http1/conn_pool.h" #include "common/http/http2/conn_pool.h" #include "common/http/mixed_conn_pool.h" @@ -46,6 +45,7 @@ #include "common/upstream/subset_lb.h" #ifdef ENVOY_ENABLE_QUIC +#include "common/http/conn_pool_grid.h" #include "common/http/http3/conn_pool.h" #endif @@ -1521,10 +1521,15 @@ Http::ConnectionPool::InstancePtr ProdClusterManagerFactory::allocateConnPool( if (protocols.size() == 3 && runtime_.snapshot().featureEnabled("upstream.use_http3", 100)) { ASSERT(contains(protocols, {Http::Protocol::Http11, Http::Protocol::Http2, Http::Protocol::Http3})); +#ifdef ENVOY_ENABLE_QUIC Envoy::Http::ConnectivityGrid::ConnectivityOptions coptions{protocols}; return std::make_unique( dispatcher, api_.randomGenerator(), host, priority, options, transport_socket_options, state, source, std::chrono::milliseconds(300), coptions); +#else + // Should be blocked by configuration checking at an earlier point. + NOT_REACHED_GCOVR_EXCL_LINE; +#endif } if (protocols.size() >= 2) { ASSERT((protocols[0] == Http::Protocol::Http2 && protocols[1] == Http::Protocol::Http11) || From 7cdb07c0a178b4d3fb65bed1532247a0dc62e0f8 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Thu, 15 Apr 2021 21:36:31 -0400 Subject: [PATCH 5/5] comments Signed-off-by: Alyssa Wilk --- .../quic/quic_transport_socket_factory.cc | 13 +++++-------- .../quic/quic_transport_socket_factory.h | 18 +++--------------- source/common/upstream/cluster_manager_impl.cc | 3 +-- test/common/upstream/upstream_impl_test.cc | 11 +++++------ 4 files changed, 14 insertions(+), 31 deletions(-) diff --git a/source/common/quic/quic_transport_socket_factory.cc b/source/common/quic/quic_transport_socket_factory.cc index 2930329f62958..8611477d4cf27 100644 --- a/source/common/quic/quic_transport_socket_factory.cc +++ b/source/common/quic/quic_transport_socket_factory.cc @@ -36,14 +36,11 @@ QuicClientTransportSocketConfigFactory::createTransportSocketFactory( return std::make_unique(std::move(client_config), context); } -void QuicClientTransportSocketFactory::maybeCreateFallbackFactory() { - if (fallback_factory_) { - return; - } - - fallback_factory_ = std::make_unique( - std::move(config_), manager_, scope_); -} +QuicClientTransportSocketFactory::QuicClientTransportSocketFactory( + Ssl::ClientContextConfigPtr config, + Server::Configuration::TransportSocketFactoryContext& factory_context) + : fallback_factory_(std::make_unique( + std::move(config), factory_context.sslContextManager(), factory_context.scope())) {} ProtobufTypes::MessagePtr QuicClientTransportSocketConfigFactory::createEmptyConfigProto() { return std::make_unique(); diff --git a/source/common/quic/quic_transport_socket_factory.h b/source/common/quic/quic_transport_socket_factory.h index 96391bb3db92e..381d77f8acb15 100644 --- a/source/common/quic/quic_transport_socket_factory.h +++ b/source/common/quic/quic_transport_socket_factory.h @@ -47,9 +47,7 @@ class QuicClientTransportSocketFactory : public QuicTransportSocketFactoryBase { public: QuicClientTransportSocketFactory( Ssl::ClientContextConfigPtr config, - Server::Configuration::TransportSocketFactoryContext& factory_context) - : config_(std::move(config)), manager_(factory_context.sslContextManager()), - scope_(factory_context.scope()) {} + Server::Configuration::TransportSocketFactoryContext& factory_context); // As documented above for QuicTransportSocketFactoryBase, the actual HTTP/3 // code does not create transport sockets. @@ -59,25 +57,15 @@ class QuicClientTransportSocketFactory : public QuicTransportSocketFactoryBase { // using the fallback factory. Network::TransportSocketPtr createTransportSocket(Network::TransportSocketOptionsSharedPtr options) const override { - // TODO(#15649) make createTransportSocket non-const to avoid the const cast. - const_cast(this)->maybeCreateFallbackFactory(); return fallback_factory_->createTransportSocket(options); } - // Creates fallback_factory_ if it does not already exist. - void maybeCreateFallbackFactory(); - - // Generally the QuicClientTransportSocketFactory owns its own client context - // config, but in the case of failover to TCP, it hands it off to the fallback_factory_ + // TODO(14829) make sure that clientContextConfig() is safe when secrets are updated. const Ssl::ClientContextConfig& clientContextConfig() const { - return fallback_factory_ ? fallback_factory_->config() : *config_; + return fallback_factory_->config(); } private: - Envoy::Ssl::ClientContextConfigPtr config_; - Envoy::Ssl::ContextManager& manager_; - Stats::Scope& scope_; - envoy::extensions::transport_sockets::quic::v3::QuicUpstreamTransport quic_config_; // The QUIC client transport socket can create TLS sockets for fallback to TCP. std::unique_ptr fallback_factory_; }; diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index b739d2e24b0e6..63eaac2820e46 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -1532,8 +1532,7 @@ Http::ConnectionPool::InstancePtr ProdClusterManagerFactory::allocateConnPool( #endif } if (protocols.size() >= 2) { - ASSERT((protocols[0] == Http::Protocol::Http2 && protocols[1] == Http::Protocol::Http11) || - (protocols[1] == Http::Protocol::Http2 && protocols[0] == Http::Protocol::Http11)); + ASSERT(contains(protocols, {Http::Protocol::Http11, Http::Protocol::Http2})); return std::make_unique(dispatcher, api_.randomGenerator(), host, priority, options, transport_socket_options, state); diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index bab8b51cc6e67..a0ce86ae68ac1 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -3250,7 +3250,7 @@ TEST_F(ClusterInfoImplTest, Http3BadConfig) { "HTTP3 requires a QuicUpstreamTransport transport socket: name.*"); } -TEST_F(ClusterInfoImplTest, Http3Alpn) { +TEST_F(ClusterInfoImplTest, Http3Auto) { const std::string yaml = TestEnvironment::substitute(R"EOF( name: name connect_timeout: 0.25s @@ -3288,7 +3288,7 @@ TEST_F(ClusterInfoImplTest, Http3Alpn) { ASSERT_TRUE(cluster1->info()->idleTimeout().has_value()); EXPECT_EQ(std::chrono::hours(1), cluster1->info()->idleTimeout().value()); - const std::string explicit_http3 = R"EOF( + const std::string auto_http3 = R"EOF( typed_extension_protocol_options: envoy.extensions.upstreams.http.v3.HttpProtocolOptions: "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions @@ -3300,12 +3300,11 @@ TEST_F(ClusterInfoImplTest, Http3Alpn) { idle_timeout: 1s )EOF"; - auto explicit_h3 = makeCluster(yaml + explicit_http3); + auto auto_h3 = makeCluster(yaml + auto_http3); EXPECT_EQ(Http::Protocol::Http3, - explicit_h3->info()->upstreamHttpProtocol({Http::Protocol::Http10})[0]); + auto_h3->info()->upstreamHttpProtocol({Http::Protocol::Http10})[0]); EXPECT_EQ( - explicit_h3->info()->http3Options().quic_protocol_options().max_concurrent_streams().value(), - 2); + auto_h3->info()->http3Options().quic_protocol_options().max_concurrent_streams().value(), 2); } #else