From 53f1d6fa51bf34d133e2712733b2e5b6e2dbd29f Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Fri, 26 Mar 2021 22:58:35 +0000 Subject: [PATCH 1/8] http3: further cleanup and extension removal Fixes https://github.com/envoyproxy/envoy/issues/12829 Signed-off-by: Matt Klein --- source/common/http/BUILD | 9 +++-- source/common/http/codec_client.cc | 17 +++++---- source/common/http/http3/BUILD | 23 +----------- source/common/http/http3/conn_pool.cc | 22 ++++++------ .../http3/quic_client_connection_factory.h | 30 ---------------- source/common/http/http3/quic_codec_factory.h | 35 ------------------- source/common/http/http3/well_known_names.h | 20 ----------- source/common/quic/BUILD | 4 --- .../quic/client_connection_factory_impl.cc | 26 +++++++++----- .../quic/client_connection_factory_impl.h | 29 +++------------ source/common/quic/codec_impl.cc | 18 ---------- source/common/quic/codec_impl.h | 25 ------------- source/common/upstream/BUILD | 6 ++-- .../common/upstream/cluster_manager_impl.cc | 11 +++++- .../network/http_connection_manager/BUILD | 5 ++- .../network/http_connection_manager/config.cc | 21 +++++------ test/integration/BUILD | 4 +-- test/integration/fake_upstream.cc | 16 +++++---- test/integration/fake_upstream.h | 2 -- test/integration/http_integration.cc | 25 ++++++++----- test/integration/utility.cc | 18 ++++++---- test/server/BUILD | 1 - 22 files changed, 116 insertions(+), 251 deletions(-) delete mode 100644 source/common/http/http3/quic_codec_factory.h delete mode 100644 source/common/http/http3/well_known_names.h diff --git a/source/common/http/BUILD b/source/common/http/BUILD index afcf183e5be8..46a7830cb82b 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -2,6 +2,7 @@ load( "//bazel:envoy_build_system.bzl", "envoy_cc_library", "envoy_package", + "envoy_select_enable_http3", ) licenses(["notice"]) # Apache 2 @@ -65,10 +66,10 @@ envoy_cc_library( "//source/common/config:utility_lib", "//source/common/http/http1:codec_lib", "//source/common/http/http2:codec_lib", - "//source/common/http/http3:quic_codec_factory_lib", - "//source/common/http/http3:well_known_names", "//source/common/network:filter_lib", - ], + ] + envoy_select_enable_http3([ + "//source/common/quic:codec_lib", + ]), ) envoy_cc_library( @@ -249,8 +250,6 @@ envoy_cc_library( "//source/common/config:utility_lib", "//source/common/http/http1:codec_lib", "//source/common/http/http2:codec_lib", - "//source/common/http/http3:quic_codec_factory_lib", - "//source/common/http/http3:well_known_names", "//source/common/http/match_wrapper:config", "//source/common/network:utility_lib", "//source/common/router:config_lib", diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc index f6d457403cb7..d6c2d43d4fad 100644 --- a/source/common/http/codec_client.cc +++ b/source/common/http/codec_client.cc @@ -10,11 +10,13 @@ #include "common/http/exception.h" #include "common/http/http1/codec_impl.h" #include "common/http/http2/codec_impl.h" -#include "common/http/http3/quic_codec_factory.h" -#include "common/http/http3/well_known_names.h" #include "common/http/status.h" #include "common/http/utility.h" +#ifdef ENVOY_ENABLE_QUIC +#include "common/quic/codec_impl.h" +#endif + namespace Envoy { namespace Http { @@ -182,11 +184,14 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne break; } case Type::HTTP3: { - codec_ = std::unique_ptr( - Config::Utility::getAndCheckFactoryByName( - Http::QuicCodecNames::get().Quiche) - .createQuicClientConnection(*connection_, *this)); +#ifdef ENVOY_ENABLE_QUIC + codec_ = std::make_unique( + dynamic_cast(*connection_), *this); break; +#else + // Should be blocked by configuration checking at an earlier point. + NOT_REACHED_GCOVR_EXCL_LINE; +#endif } } } diff --git a/source/common/http/http3/BUILD b/source/common/http/http3/BUILD index 112ca100d629..a89c441ad794 100644 --- a/source/common/http/http3/BUILD +++ b/source/common/http/http3/BUILD @@ -18,32 +18,11 @@ envoy_cc_library( "//include/envoy/upstream:upstream_interface", "//source/common/http:codec_client_lib", "//source/common/http:conn_pool_base_lib", - ], -) - -envoy_cc_library( - name = "quic_codec_factory_lib", - hdrs = ["quic_codec_factory.h"], - deps = [ - "//include/envoy/config:typed_config_interface", - "//include/envoy/http:codec_interface", - "//include/envoy/network:connection_interface", + "//source/common/quic:client_connection_factory_lib", ], ) envoy_cc_library( name = "quic_client_connection_factory_lib", hdrs = ["quic_client_connection_factory.h"], - deps = [ - "//include/envoy/config:typed_config_interface", - "//include/envoy/network:connection_interface", - "//include/envoy/ssl:context_config_interface", - "@envoy_api//envoy/config/core/v3:pkg_cc_proto", - ], -) - -envoy_cc_library( - name = "well_known_names", - hdrs = ["well_known_names.h"], - deps = ["//source/common/singleton:const_singleton"], ) diff --git a/source/common/http/http3/conn_pool.cc b/source/common/http/http3/conn_pool.cc index a052b91170c4..1184618ef01a 100644 --- a/source/common/http/http3/conn_pool.cc +++ b/source/common/http/http3/conn_pool.cc @@ -7,12 +7,17 @@ #include "common/config/utility.h" #include "common/http/http3/quic_client_connection_factory.h" -#include "common/http/http3/well_known_names.h" #include "common/http/utility.h" #include "common/network/address_impl.h" #include "common/network/utility.h" #include "common/runtime/runtime_features.h" +#ifdef ENVOY_ENABLE_QUIC +#include "common/quic/client_connection_factory_impl.h" +#else +#error "http3 conn pool should not be built with QUIC disabled" +#endif + namespace Envoy { namespace Http { namespace Http3 { @@ -36,11 +41,9 @@ class Http3ConnPoolImpl : public FixedHttpConnPoolImpl { source_address = Network::Utility::getLocalAddress(host_address->ip()->version()); } Network::TransportSocketFactory& transport_socket_factory = host->transportSocketFactory(); - quic_info_ = - Config::Utility::getAndCheckFactoryByName( - Http::QuicCodecNames::get().Quiche) - .createNetworkConnectionInfo(dispatcher, transport_socket_factory, - host->cluster().statsScope(), time_source, source_address); + quic_info_ = std::make_unique( + dispatcher, transport_socket_factory, host->cluster().statsScope(), time_source, + source_address); } // Make sure all connections are torn down before quic_info_ is deleted. @@ -68,11 +71,8 @@ allocateConnPool(Event::Dispatcher& dispatcher, Random::RandomGenerator& random_ if (!source_address.get()) { source_address = Network::Utility::getLocalAddress(host_address->ip()->version()); } - data.connection_ = - Config::Utility::getAndCheckFactoryByName( - Http::QuicCodecNames::get().Quiche) - .createQuicNetworkConnection(*h3_pool->quic_info_, pool->dispatcher(), host_address, - source_address); + data.connection_ = Quic::createQuicNetworkConnection( + *h3_pool->quic_info_, pool->dispatcher(), host_address, source_address); return std::make_unique(*pool, data); }, [](Upstream::Host::CreateConnectionData& data, HttpConnPoolImplBase* pool) { diff --git a/source/common/http/http3/quic_client_connection_factory.h b/source/common/http/http3/quic_client_connection_factory.h index 7a27a1a26365..bfc639d47cfb 100644 --- a/source/common/http/http3/quic_client_connection_factory.h +++ b/source/common/http/http3/quic_client_connection_factory.h @@ -1,12 +1,5 @@ #pragma once -#include - -#include "envoy/config/core/v3/protocol.pb.h" -#include "envoy/config/typed_config.h" -#include "envoy/network/connection.h" -#include "envoy/ssl/context_config.h" - namespace Envoy { namespace Http { @@ -16,28 +9,5 @@ struct PersistentQuicInfo { virtual ~PersistentQuicInfo() = default; }; -// A factory to create EnvoyQuicClientSession and EnvoyQuicClientConnection for QUIC -class QuicClientConnectionFactory : public Config::UntypedFactory { -public: - ~QuicClientConnectionFactory() override = default; - - // Allows thread-local creation of PersistentQuicInfo. - // Each (thread local) connection pool can call createNetworkConnectionInfo to create a - // PersistentQuicInfo, then use that PersistentQuicInfo. when calling createQuicNetworkConnection - // for all the connections in that pool. - virtual std::unique_ptr - createNetworkConnectionInfo(Event::Dispatcher& dispatcher, - Network::TransportSocketFactory& transport_socket_factory, - Stats::Scope& stats_scope, TimeSource& time_source, - Network::Address::InstanceConstSharedPtr server_addr) PURE; - - virtual std::unique_ptr - createQuicNetworkConnection(PersistentQuicInfo& info, Event::Dispatcher& dispatcher, - Network::Address::InstanceConstSharedPtr server_addr, - Network::Address::InstanceConstSharedPtr local_addr) PURE; - - std::string category() const override { return "envoy.quic_connection"; } -}; - } // namespace Http } // namespace Envoy diff --git a/source/common/http/http3/quic_codec_factory.h b/source/common/http/http3/quic_codec_factory.h deleted file mode 100644 index 0b4a72404200..000000000000 --- a/source/common/http/http3/quic_codec_factory.h +++ /dev/null @@ -1,35 +0,0 @@ -#pragma once - -#include - -#include "envoy/config/typed_config.h" -#include "envoy/http/codec.h" -#include "envoy/network/connection.h" - -namespace Envoy { -namespace Http { - -// A factory to create Http::ServerConnection instance for QUIC. -class QuicHttpServerConnectionFactory : public Config::UntypedFactory { -public: - ~QuicHttpServerConnectionFactory() override = default; - - virtual std::unique_ptr - createQuicServerConnection(Network::Connection& connection, ConnectionCallbacks& callbacks) PURE; - - std::string category() const override { return "envoy.quic_client_codec"; } -}; - -// A factory to create Http::ClientConnection instance for QUIC. -class QuicHttpClientConnectionFactory : public Config::UntypedFactory { -public: - ~QuicHttpClientConnectionFactory() override = default; - - virtual std::unique_ptr - createQuicClientConnection(Network::Connection& connection, ConnectionCallbacks& callbacks) PURE; - - std::string category() const override { return "envoy.quic_server_codec"; } -}; - -} // namespace Http -} // namespace Envoy diff --git a/source/common/http/http3/well_known_names.h b/source/common/http/http3/well_known_names.h deleted file mode 100644 index 80ac862b5f78..000000000000 --- a/source/common/http/http3/well_known_names.h +++ /dev/null @@ -1,20 +0,0 @@ -#pragma once - -#include - -#include "common/singleton/const_singleton.h" - -namespace Envoy { -namespace Http { - -// TODO(mattklein123): Remove this as part of #12829 -class QuicCodecNameValues { -public: - // QUICHE is the only QUIC implementation for now. - const std::string Quiche = "quiche"; -}; - -using QuicCodecNames = ConstSingleton; - -} // namespace Http -} // namespace Envoy diff --git a/source/common/quic/BUILD b/source/common/quic/BUILD index 5fd73e26b9a9..efa12ec9004d 100644 --- a/source/common/quic/BUILD +++ b/source/common/quic/BUILD @@ -151,8 +151,6 @@ envoy_cc_library( "//include/envoy/http:codec_interface", "//include/envoy/registry", "//source/common/http/http3:quic_client_connection_factory_lib", - "//source/common/http/http3:quic_codec_factory_lib", - "//source/common/http/http3:well_known_names", "//source/extensions/transport_sockets/tls:ssl_socket_lib", "@com_googlesource_quiche//:quic_core_http_spdy_session_lib", ], @@ -169,8 +167,6 @@ envoy_cc_library( ":envoy_quic_utils_lib", "//include/envoy/http:codec_interface", "//include/envoy/registry", - "//source/common/http/http3:quic_codec_factory_lib", - "//source/common/http/http3:well_known_names", "@com_googlesource_quiche//:quic_core_http_spdy_session_lib", ], ) diff --git a/source/common/quic/client_connection_factory_impl.cc b/source/common/quic/client_connection_factory_impl.cc index c83ba3532957..c109d50d7ced 100644 --- a/source/common/quic/client_connection_factory_impl.cc +++ b/source/common/quic/client_connection_factory_impl.cc @@ -24,11 +24,21 @@ PersistentQuicInfoImpl::PersistentQuicInfoImpl( std::make_unique(std::make_unique( stats_scope, getConfig(transport_socket_factory), time_source))) {} +namespace { +// TODO(mattklein123): This is mutable static info that is required for the QUICHE code. This was +// preexisting but should be removed as we shouldn't be using global mutable static data. +struct StaticInfo { + quic::QuicConfig quic_config_; + quic::QuicClientPushPromiseIndex push_promise_index_; + + static StaticInfo& get() { MUTABLE_CONSTRUCT_ON_FIRST_USE(StaticInfo); } +}; +} // namespace + std::unique_ptr -QuicClientConnectionFactoryImpl::createQuicNetworkConnection( - Http::PersistentQuicInfo& info, Event::Dispatcher& dispatcher, - Network::Address::InstanceConstSharedPtr server_addr, - Network::Address::InstanceConstSharedPtr local_addr) { +createQuicNetworkConnection(Http::PersistentQuicInfo& info, Event::Dispatcher& dispatcher, + Network::Address::InstanceConstSharedPtr server_addr, + Network::Address::InstanceConstSharedPtr local_addr) { // This flag fix a QUICHE issue which may crash Envoy during connection close. SetQuicReloadableFlag(quic_single_ack_in_packet2, true); PersistentQuicInfoImpl* info_impl = reinterpret_cast(&info); @@ -37,14 +47,14 @@ QuicClientConnectionFactoryImpl::createQuicNetworkConnection( quic::QuicUtils::CreateRandomConnectionId(), server_addr, info_impl->conn_helper_, info_impl->alarm_factory_, quic::ParsedQuicVersionVector{info_impl->supported_versions_[0]}, local_addr, dispatcher, nullptr); + auto& static_info = StaticInfo::get(); auto ret = std::make_unique( - quic_config_, info_impl->supported_versions_, std::move(connection), info_impl->server_id_, - info_impl->crypto_config_.get(), &push_promise_index_, dispatcher, 0); + static_info.quic_config_, info_impl->supported_versions_, std::move(connection), + info_impl->server_id_, info_impl->crypto_config_.get(), &static_info.push_promise_index_, + dispatcher, 0); ret->Initialize(); return ret; } -REGISTER_FACTORY(QuicClientConnectionFactoryImpl, Http::QuicClientConnectionFactory); - } // namespace Quic } // namespace Envoy diff --git a/source/common/quic/client_connection_factory_impl.h b/source/common/quic/client_connection_factory_impl.h index 75b20a60ecef..df52e80beb01 100644 --- a/source/common/quic/client_connection_factory_impl.h +++ b/source/common/quic/client_connection_factory_impl.h @@ -1,7 +1,6 @@ #pragma once #include "common/http/http3/quic_client_connection_factory.h" -#include "common/http/http3/well_known_names.h" #include "common/quic/envoy_quic_alarm_factory.h" #include "common/quic/envoy_quic_client_session.h" #include "common/quic/envoy_quic_connection_helper.h" @@ -32,30 +31,10 @@ struct PersistentQuicInfoImpl : public Http::PersistentQuicInfo { std::unique_ptr crypto_config_; }; -// A factory to create EnvoyQuicClientConnection instance for QUIC -class QuicClientConnectionFactoryImpl : public Http::QuicClientConnectionFactory { -public: - std::unique_ptr - createNetworkConnectionInfo(Event::Dispatcher& dispatcher, - Network::TransportSocketFactory& transport_socket_factory, - Stats::Scope& stats_scope, TimeSource& time_source, - Network::Address::InstanceConstSharedPtr server_addr) override { - return std::make_unique(dispatcher, transport_socket_factory, - stats_scope, time_source, server_addr); - } - - std::unique_ptr - createQuicNetworkConnection(Http::PersistentQuicInfo& info, Event::Dispatcher& dispatcher, - Network::Address::InstanceConstSharedPtr server_addr, - Network::Address::InstanceConstSharedPtr local_addr) override; - - std::string name() const override { return Http::QuicCodecNames::get().Quiche; } - - quic::QuicConfig quic_config_; - quic::QuicClientPushPromiseIndex push_promise_index_; -}; - -DECLARE_FACTORY(QuicClientConnectionFactoryImpl); +std::unique_ptr +createQuicNetworkConnection(Http::PersistentQuicInfo& info, Event::Dispatcher& dispatcher, + Network::Address::InstanceConstSharedPtr server_addr, + Network::Address::InstanceConstSharedPtr local_addr); } // namespace Quic } // namespace Envoy diff --git a/source/common/quic/codec_impl.cc b/source/common/quic/codec_impl.cc index bb6de974e9ac..86ded99aa179 100644 --- a/source/common/quic/codec_impl.cc +++ b/source/common/quic/codec_impl.cc @@ -93,23 +93,5 @@ void QuicHttpClientConnectionImpl::onUnderlyingConnectionBelowWriteBufferLowWate }); } -std::unique_ptr -QuicHttpClientConnectionFactoryImpl::createQuicClientConnection( - Network::Connection& connection, Http::ConnectionCallbacks& callbacks) { - return std::make_unique( - dynamic_cast(connection), callbacks); -} - -std::unique_ptr -QuicHttpServerConnectionFactoryImpl::createQuicServerConnection( - Network::Connection& connection, Http::ConnectionCallbacks& callbacks) { - return std::make_unique( - dynamic_cast(connection), - dynamic_cast(callbacks)); -} - -REGISTER_FACTORY(QuicHttpClientConnectionFactoryImpl, Http::QuicHttpClientConnectionFactory); -REGISTER_FACTORY(QuicHttpServerConnectionFactoryImpl, Http::QuicHttpServerConnectionFactory); - } // namespace Quic } // namespace Envoy diff --git a/source/common/quic/codec_impl.h b/source/common/quic/codec_impl.h index 5ce40d49ffdc..240e7e7dd29e 100644 --- a/source/common/quic/codec_impl.h +++ b/source/common/quic/codec_impl.h @@ -5,8 +5,6 @@ #include "common/common/assert.h" #include "common/common/logger.h" -#include "common/http/http3/quic_codec_factory.h" -#include "common/http/http3/well_known_names.h" #include "common/quic/envoy_quic_client_session.h" #include "common/quic/envoy_quic_server_session.h" @@ -71,28 +69,5 @@ class QuicHttpClientConnectionImpl : public QuicHttpConnectionImplBase, EnvoyQuicClientSession& quic_client_session_; }; -// A factory to create QuicHttpClientConnection. -class QuicHttpClientConnectionFactoryImpl : public Http::QuicHttpClientConnectionFactory { -public: - std::unique_ptr - createQuicClientConnection(Network::Connection& connection, - Http::ConnectionCallbacks& callbacks) override; - - std::string name() const override { return Http::QuicCodecNames::get().Quiche; } -}; - -// A factory to create QuicHttpServerConnection. -class QuicHttpServerConnectionFactoryImpl : public Http::QuicHttpServerConnectionFactory { -public: - std::unique_ptr - createQuicServerConnection(Network::Connection& connection, - Http::ConnectionCallbacks& callbacks) override; - - std::string name() const override { return Http::QuicCodecNames::get().Quiche; } -}; - -DECLARE_FACTORY(QuicHttpClientConnectionFactoryImpl); -DECLARE_FACTORY(QuicHttpServerConnectionFactoryImpl); - } // namespace Quic } // namespace Envoy diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index 4fa77043d8b3..9eb6e2f11c9f 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -2,6 +2,7 @@ load( "//bazel:envoy_build_system.bzl", "envoy_cc_library", "envoy_package", + "envoy_select_enable_http3", ) licenses(["notice"]) # Apache 2 @@ -62,7 +63,6 @@ envoy_cc_library( "//source/common/http:mixed_conn_pool", "//source/common/http/http1:conn_pool_lib", "//source/common/http/http2:conn_pool_lib", - "//source/common/http/http3:conn_pool_lib", "//source/common/network:resolver_lib", "//source/common/network:utility_lib", "//source/common/protobuf:utility_lib", @@ -76,7 +76,9 @@ envoy_cc_library( "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", - ], + ] + envoy_select_enable_http3([ + "//source/common/http/http3:conn_pool_lib", + ]), ) envoy_cc_library( diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index b846080eb524..a2f97ff9afc1 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -28,7 +28,6 @@ #include "common/http/async_client_impl.h" #include "common/http/http1/conn_pool.h" #include "common/http/http2/conn_pool.h" -#include "common/http/http3/conn_pool.h" #include "common/http/mixed_conn_pool.h" #include "common/network/resolver_impl.h" #include "common/network/utility.h" @@ -45,6 +44,10 @@ #include "common/upstream/ring_hash_lb.h" #include "common/upstream/subset_lb.h" +#ifdef ENVOY_ENABLE_QUIC +#include "common/http/http3/conn_pool.h" +#endif + namespace Envoy { namespace Upstream { namespace { @@ -1517,8 +1520,14 @@ Http::ConnectionPool::InstancePtr ProdClusterManagerFactory::allocateConnPool( } if (protocols.size() == 1 && protocols[0] == Http::Protocol::Http3 && runtime_.snapshot().featureEnabled("upstream.use_http3", 100)) { +#ifdef ENVOY_ENABLE_QUIC return Http::Http3::allocateConnPool(dispatcher, api_.randomGenerator(), host, priority, options, transport_socket_options, state, source); +#else + UNREFERENCED_PARAMETER(source); + // Should be blocked by configuration checking at an earlier point. + NOT_REACHED_GCOVR_EXCL_LINE; +#endif } ASSERT(protocols.size() == 1 && protocols[0] == Http::Protocol::Http11); return Http::Http1::allocateConnPool(dispatcher, api_.randomGenerator(), host, priority, options, diff --git a/source/extensions/filters/network/http_connection_manager/BUILD b/source/extensions/filters/network/http_connection_manager/BUILD index 0868898bc13c..aa55666bb72e 100644 --- a/source/extensions/filters/network/http_connection_manager/BUILD +++ b/source/extensions/filters/network/http_connection_manager/BUILD @@ -3,6 +3,7 @@ load( "envoy_cc_extension", "envoy_cc_library", "envoy_extension_package", + "envoy_select_enable_http3", ) licenses(["notice"]) # Apache 2 @@ -60,7 +61,9 @@ envoy_cc_extension( "@envoy_api//envoy/extensions/request_id/uuid/v3:pkg_cc_proto", "@envoy_api//envoy/type/tracing/v3:pkg_cc_proto", "@envoy_api//envoy/type/v3:pkg_cc_proto", - ], + ] + envoy_select_enable_http3([ + "//source/common/quic:codec_lib", + ]), ) envoy_cc_library( diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 37c69d72aa55..d29a58960587 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -26,8 +26,6 @@ #include "common/http/http1/codec_impl.h" #include "common/http/http1/settings.h" #include "common/http/http2/codec_impl.h" -#include "common/http/http3/quic_codec_factory.h" -#include "common/http/http3/well_known_names.h" #include "common/http/request_id_extension_impl.h" #include "common/http/utility.h" #include "common/local_reply/local_reply.h" @@ -38,6 +36,10 @@ #include "common/tracing/http_tracer_config_impl.h" #include "common/tracing/http_tracer_manager_impl.h" +#ifdef ENVOY_ENABLE_QUIC +#include "common/quic/codec_impl.h" +#endif + #include "extensions/filters/http/common/pass_through_filter.h" namespace Envoy { @@ -577,14 +579,13 @@ HttpConnectionManagerConfig::createCodec(Network::Connection& connection, maxRequestHeadersCount(), headersWithUnderscoresAction()); } case CodecType::HTTP3: - // Hard code Quiche factory name here to instantiate a QUIC codec implemented. - // TODO(danzh) Add support to get the factory name from config, possibly - // from HttpConnectionManager protobuf. This is not essential till there are multiple - // implementations of QUIC. - return std::unique_ptr( - Config::Utility::getAndCheckFactoryByName( - Http::QuicCodecNames::get().Quiche) - .createQuicServerConnection(connection, callbacks)); +#ifdef ENVOY_ENABLE_QUIC + return std::make_unique( + dynamic_cast(connection), callbacks); +#else + // Should be blocked by configuration checking at an earlier point. + NOT_REACHED_GCOVR_EXCL_LINE; +#endif case CodecType::AUTO: return Http::ConnectionManagerUtility::autoCreateCodec( connection, data, callbacks, context_.scope(), context_.api().randomGenerator(), diff --git a/test/integration/BUILD b/test/integration/BUILD index 456df05e5f84..d0b2981777d8 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -662,8 +662,6 @@ envoy_cc_test_library( "//source/common/grpc:common_lib", "//source/common/http/http1:codec_lib", "//source/common/http/http2:codec_lib", - "//source/common/http/http3:quic_codec_factory_lib", - "//source/common/http/http3:well_known_names", "//source/common/network:connection_balancer_lib", "//source/common/network:filter_lib", "//source/common/network:listen_socket_lib", @@ -680,6 +678,7 @@ envoy_cc_test_library( "@envoy_api//envoy/config/listener/v3:pkg_cc_proto", ] + envoy_select_enable_http3([ "//source/common/quic:active_quic_listener_lib", + "//source/common/quic:codec_lib", ]), ) @@ -766,7 +765,6 @@ envoy_cc_test_library( "//source/common/http/http3:quic_client_connection_factory_lib", "//source/common/json:json_loader_lib", "//source/common/network:utility_lib", - "//source/common/quic:quic_transport_socket_factory_lib", "//source/common/stats:allocator_lib", "//source/common/stats:isolated_store_lib", "//source/common/thread_local:thread_local_lib", diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index c4ba2f423f70..941b7b1b59e3 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -10,12 +10,15 @@ #include "common/http/header_map_impl.h" #include "common/http/http1/codec_impl.h" #include "common/http/http2/codec_impl.h" -#include "common/http/http3/well_known_names.h" #include "common/network/address_impl.h" #include "common/network/listen_socket_impl.h" #include "common/network/socket_option_factory.h" #include "common/network/utility.h" +#ifdef ENVOY_ENABLE_QUIC +#include "common/quic/codec_impl.h" +#endif + #include "server/connection_handler_impl.h" #include "test/test_common/network_utility.h" @@ -339,11 +342,12 @@ FakeHttpConnection::FakeHttpConnection( max_request_headers_kb, max_request_headers_count, headers_with_underscores_action); } else { ASSERT(type == Type::HTTP3); - envoy::config::core::v3::Http3ProtocolOptions http3_options; - codec_ = std::unique_ptr( - Config::Utility::getAndCheckFactoryByName( - Http::QuicCodecNames::get().Quiche) - .createQuicServerConnection(shared_connection_.connection(), *this)); +#ifdef ENVOY_ENABLE_QUIC + codec_ = std::make_unique( + dynamic_cast(shared_connection_.connection()), *this); +#else + ASSERT(false, "running a QUIC integration test without compiling QUIC"); +#endif } shared_connection_.connection().addReadFilter( Network::ReadFilterSharedPtr{new ReadFilter(*this)}); diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index d7dbe2b65e6e..73d9f93ddb73 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -27,8 +27,6 @@ #include "common/grpc/common.h" #include "common/http/http1/codec_impl.h" #include "common/http/http2/codec_impl.h" -#include "common/http/http3/quic_codec_factory.h" -#include "common/http/http3/well_known_names.h" #include "common/network/connection_balancer_impl.h" #include "common/network/filter_impl.h" #include "common/network/listen_socket_impl.h" diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 25a9bf65b7b5..ef0759c806ae 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -27,6 +27,10 @@ #include "common/runtime/runtime_impl.h" #include "common/upstream/upstream_impl.h" +#ifdef ENVOY_ENABLE_QUIC +#include "common/quic/client_connection_factory_impl.h" +#endif + #include "extensions/transport_sockets/tls/context_config_impl.h" #include "extensions/transport_sockets/tls/context_impl.h" #include "extensions/transport_sockets/tls/ssl_socket.h" @@ -220,16 +224,18 @@ Network::ClientConnectionPtr HttpIntegrationTest::makeClientConnectionWithOption if (downstream_protocol_ <= Http::CodecClient::Type::HTTP2) { return BaseIntegrationTest::makeClientConnectionWithOptions(port, options); } +#ifdef ENVOY_ENABLE_QUIC // Setting socket options is not supported for HTTP3. ASSERT(!options); Network::Address::InstanceConstSharedPtr server_addr = Network::Utility::resolveUrl( fmt::format("udp://{}:{}", Network::Test::getLoopbackAddressUrlString(version_), port)); Network::Address::InstanceConstSharedPtr local_addr = Network::Test::getCanonicalLoopbackAddress(version_); - return Config::Utility::getAndCheckFactoryByName( - Http::QuicCodecNames::get().Quiche) - .createQuicNetworkConnection(*quic_connection_persistent_info_, *dispatcher_, server_addr, - local_addr); + return Quic::createQuicNetworkConnection(*quic_connection_persistent_info_, *dispatcher_, + server_addr, local_addr); +#else + ASSERT(false, "running a QUIC integration test without compiling QUIC"); +#endif } IntegrationCodecClientPtr HttpIntegrationTest::makeHttpConnection(uint32_t port) { @@ -310,6 +316,7 @@ void HttpIntegrationTest::initialize() { if (downstream_protocol_ != Http::CodecClient::Type::HTTP3) { return BaseIntegrationTest::initialize(); } +#ifdef ENVOY_ENABLE_QUIC // Needs to be instantiated before base class calls initialize() which starts a QUIC listener // according to the config. quic_transport_socket_factory_ = @@ -325,11 +332,11 @@ void HttpIntegrationTest::initialize() { Network::Address::InstanceConstSharedPtr server_addr = Network::Utility::resolveUrl(fmt::format( "udp://{}:{}", Network::Test::getLoopbackAddressUrlString(version_), lookupPort("http"))); // Needs to outlive all QUIC connections. - quic_connection_persistent_info_ = - Config::Utility::getAndCheckFactoryByName( - Http::QuicCodecNames::get().Quiche) - .createNetworkConnectionInfo(*dispatcher_, *quic_transport_socket_factory_, stats_store_, - timeSystem(), server_addr); + quic_connection_persistent_info_ = std::make_unique( + *dispatcher_, *quic_transport_socket_factory_, stats_store_, timeSystem(), server_addr); +#else + ASSERT(false, "running a QUIC integration test without compiling QUIC"); +#endif } void HttpIntegrationTest::setDownstreamProtocol(Http::CodecClient::Type downstream_protocol) { diff --git a/test/integration/utility.cc b/test/integration/utility.cc index d461516e038b..628069846782 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -17,11 +17,14 @@ #include "common/http/header_map_impl.h" #include "common/http/headers.h" #include "common/http/http3/quic_client_connection_factory.h" -#include "common/http/http3/well_known_names.h" #include "common/network/address_impl.h" #include "common/network/utility.h" #include "common/upstream/upstream_impl.h" +#ifdef ENVOY_ENABLE_QUIC +#include "common/quic/client_connection_factory_impl.h" +#endif + #include "test/common/upstream/utility.h" #include "test/integration/ssl_utility.h" #include "test/mocks/common.h" @@ -199,13 +202,11 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt client); } +#ifdef ENVOY_ENABLE_QUIC Network::TransportSocketFactoryPtr transport_socket_factory = createQuicUpstreamTransportSocketFactory(api, "spiffe://lyft.com/backend-team"); std::unique_ptr persistent_info; - Http::QuicClientConnectionFactory& connection_factory = - Config::Utility::getAndCheckFactoryByName( - Http::QuicCodecNames::get().Quiche); - persistent_info = connection_factory.createNetworkConnectionInfo( + persistent_info = std::make_unique( *dispatcher, *transport_socket_factory, mock_stats_store, time_system, addr); Network::Address::InstanceConstSharedPtr local_address; @@ -215,13 +216,16 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt // Docker only works with loopback v6 address. local_address = std::make_shared("::1"); } - Network::ClientConnectionPtr connection = connection_factory.createQuicNetworkConnection( - *persistent_info, *dispatcher, addr, local_address); + Network::ClientConnectionPtr connection = + Quic::createQuicNetworkConnection(*persistent_info, *dispatcher, addr, local_address); connection->addConnectionCallbacks(connection_callbacks); Http::CodecClientProd client(type, std::move(connection), host_description, *dispatcher, random); // Quic connection needs to finish handshake. dispatcher->run(Event::Dispatcher::RunType::Block); return sendRequestAndWaitForResponse(*dispatcher, method, url, body, host, content_type, client); +#else + ASSERT(false, "running a QUIC integration test without compiling QUIC"); +#endif } BufferingStreamDecoderPtr diff --git a/test/server/BUILD b/test/server/BUILD index edcb9f3fd5ff..555e84507092 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -300,7 +300,6 @@ envoy_cc_test( deps = [ ":listener_manager_impl_test_lib", ":utility_lib", - "//source/common/quic:quic_factory_lib", "//source/extensions/transport_sockets/raw_buffer:config", "//source/extensions/transport_sockets/tls:config", "//test/test_common:threadsafe_singleton_injector_lib", From 9bea38bab3dc5f0411db2d6751659bd448f98e57 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Mon, 29 Mar 2021 23:53:38 +0000 Subject: [PATCH 2/8] fixes Signed-off-by: Matt Klein --- .../quic/v3/quic_transport.proto | 1 - .../quic/v4alpha/quic_transport.proto | 1 - .../quic/v3/quic_transport.proto | 1 - .../quic/v4alpha/quic_transport.proto | 1 - source/common/upstream/upstream_impl.cc | 16 ++++++--- source/extensions/extensions_build_config.bzl | 2 +- .../network/http_connection_manager/config.cc | 4 +++ test/common/quic/BUILD | 23 +++++++++++++ test/common/quic/quic_upstream_test.cc | 9 ++++- .../http_connection_manager/config_test.cc | 34 +++++++++++++++++++ 10 files changed, 81 insertions(+), 11 deletions(-) diff --git a/api/envoy/extensions/transport_sockets/quic/v3/quic_transport.proto b/api/envoy/extensions/transport_sockets/quic/v3/quic_transport.proto index 7f713c0f194f..25122b09c597 100644 --- a/api/envoy/extensions/transport_sockets/quic/v3/quic_transport.proto +++ b/api/envoy/extensions/transport_sockets/quic/v3/quic_transport.proto @@ -14,7 +14,6 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // [#protodoc-title: quic transport] // [#comment:#extension: envoy.transport_sockets.quic] -// [#comment:TODO(#12829): Remove this as an extension point.] // Configuration for Downstream QUIC transport socket. This provides Google's implementation of Google QUIC and IETF QUIC to Envoy. message QuicDownstreamTransport { diff --git a/api/envoy/extensions/transport_sockets/quic/v4alpha/quic_transport.proto b/api/envoy/extensions/transport_sockets/quic/v4alpha/quic_transport.proto index 5f5f7d39daa7..9a5f096f56c7 100644 --- a/api/envoy/extensions/transport_sockets/quic/v4alpha/quic_transport.proto +++ b/api/envoy/extensions/transport_sockets/quic/v4alpha/quic_transport.proto @@ -15,7 +15,6 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // [#protodoc-title: quic transport] // [#comment:#extension: envoy.transport_sockets.quic] -// [#comment:TODO(#12829): Remove this as an extension point.] // Configuration for Downstream QUIC transport socket. This provides Google's implementation of Google QUIC and IETF QUIC to Envoy. message QuicDownstreamTransport { diff --git a/generated_api_shadow/envoy/extensions/transport_sockets/quic/v3/quic_transport.proto b/generated_api_shadow/envoy/extensions/transport_sockets/quic/v3/quic_transport.proto index 7f713c0f194f..25122b09c597 100644 --- a/generated_api_shadow/envoy/extensions/transport_sockets/quic/v3/quic_transport.proto +++ b/generated_api_shadow/envoy/extensions/transport_sockets/quic/v3/quic_transport.proto @@ -14,7 +14,6 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // [#protodoc-title: quic transport] // [#comment:#extension: envoy.transport_sockets.quic] -// [#comment:TODO(#12829): Remove this as an extension point.] // Configuration for Downstream QUIC transport socket. This provides Google's implementation of Google QUIC and IETF QUIC to Envoy. message QuicDownstreamTransport { diff --git a/generated_api_shadow/envoy/extensions/transport_sockets/quic/v4alpha/quic_transport.proto b/generated_api_shadow/envoy/extensions/transport_sockets/quic/v4alpha/quic_transport.proto index 5f5f7d39daa7..9a5f096f56c7 100644 --- a/generated_api_shadow/envoy/extensions/transport_sockets/quic/v4alpha/quic_transport.proto +++ b/generated_api_shadow/envoy/extensions/transport_sockets/quic/v4alpha/quic_transport.proto @@ -15,7 +15,6 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // [#protodoc-title: quic transport] // [#comment:#extension: envoy.transport_sockets.quic] -// [#comment:TODO(#12829): Remove this as an extension point.] // Configuration for Downstream QUIC transport socket. This provides Google's implementation of Google QUIC and IETF QUIC to Envoy. message QuicDownstreamTransport { diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 8d80b69346ab..6fb0dbf95044 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -971,11 +971,17 @@ ClusterImplBase::ClusterImplBase( fmt::format("ALPN configured for cluster {} which has a non-ALPN transport socket: {}", cluster.name(), cluster.DebugString())); } - // TODO(#12829) clean up (e.g. move tests out of extensions) once QUIC is no longer an extension. - if ((info_->features() & ClusterInfoImpl::Features::HTTP3) && - (cluster.transport_socket().name() != "envoy.transport_sockets.quic")) { - throw EnvoyException(fmt::format("HTTP3 requires a QuicUpstreamTransport tranport socket: {}", - cluster.name(), cluster.DebugString())); + + if (info_->features() & ClusterInfoImpl::Features::HTTP3) { +#if defined(ENVOY_ENABLE_QUIC) + if (cluster.transport_socket().name() != "envoy.transport_sockets.quic") { + throw EnvoyException( + fmt::format("HTTP3 requires a QuicUpstreamTransport transport socket: {}", cluster.name(), + cluster.DebugString())); + } +#else + throw EnvoyException("HTTP3 configured but not enabled in the build."); +#endif } // Create the default (empty) priority set before registering callbacks to diff --git a/source/extensions/extensions_build_config.bzl b/source/extensions/extensions_build_config.bzl index 9db4a2467a0c..ade157cf6f2f 100644 --- a/source/extensions/extensions_build_config.bzl +++ b/source/extensions/extensions_build_config.bzl @@ -262,7 +262,7 @@ EXTENSIONS = { # HTTP header formatters # - "envoy.http.stateful_header_formatters.preserve_case": "//source/extensions/http/header_formatters/preserve_case:preserve_case_formatter" + "envoy.http.stateful_header_formatters.preserve_case": "//source/extensions/http/header_formatters/preserve_case:preserve_case_formatter", } # These can be changed to ["//visibility:public"], for downstream builds which diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index d29a58960587..fe95f635e57e 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -460,7 +460,11 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( break; case envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager:: HTTP3: +#ifdef ENVOY_ENABLE_QUIC codec_type_ = CodecType::HTTP3; +#else + throw EnvoyException("HTTP3 configured but not enabled in the build."); +#endif break; default: NOT_REACHED_GCOVR_EXCL_LINE; diff --git a/test/common/quic/BUILD b/test/common/quic/BUILD index 567e93adce0e..c0b202ac3a02 100644 --- a/test/common/quic/BUILD +++ b/test/common/quic/BUILD @@ -281,3 +281,26 @@ envoy_cc_test_library( "@com_googlesource_quiche//:quic_test_tools_qpack_qpack_encoder_test_utils_lib", ], ) + +envoy_cc_test( + name = "quic_upstream_test", + srcs = ["quic_upstream_test.cc"], + data = ["//test/extensions/transport_sockets/tls/test_data:certs"], + tags = ["nofips"], + deps = [ + "//source/common/quic:quic_transport_socket_factory_lib", + "//source/common/singleton:manager_impl_lib", + "//source/common/upstream:static_cluster_lib", + "//source/common/upstream:strict_dns_cluster_lib", + "//test/common/upstream:utility_lib", + "//test/mocks/local_info:local_info_mocks", + "//test/mocks/network:network_mocks", + "//test/mocks/protobuf:protobuf_mocks", + "//test/mocks/server:admin_mocks", + "//test/mocks/server:instance_mocks", + "//test/mocks/ssl:ssl_mocks", + "//test/mocks/upstream:health_checker_mocks", + "//test/test_common:registry_lib", + "//test/test_common:test_runtime_lib", + ], +) diff --git a/test/common/quic/quic_upstream_test.cc b/test/common/quic/quic_upstream_test.cc index 5c2ed95a8ce7..3090dc1c0b37 100644 --- a/test/common/quic/quic_upstream_test.cc +++ b/test/common/quic/quic_upstream_test.cc @@ -89,6 +89,7 @@ class ClusterInfoImplTest : public testing::Test { Server::MockOptions options_; }; +#ifdef ENVOY_ENABLE_QUIC TEST_F(ClusterInfoImplTest, Http3) { const std::string yaml = TestEnvironment::substitute(R"EOF( name: name @@ -162,6 +163,7 @@ TEST_F(ClusterInfoImplTest, Http3) { EXPECT_FALSE( downstream_h3->info()->http3Options().quic_protocol_options().has_max_concurrent_streams()); } +#endif TEST_F(ClusterInfoImplTest, Http3BadConfig) { const std::string yaml = TestEnvironment::substitute(R"EOF( @@ -204,8 +206,13 @@ TEST_F(ClusterInfoImplTest, Http3BadConfig) { )EOF", Network::Address::IpVersion::v4); +#ifdef ENVOY_ENABLE_QUIC EXPECT_THROW_WITH_REGEX(makeCluster(yaml), EnvoyException, - "HTTP3 requires a QuicUpstreamTransport tranport socket: name.*"); + "HTTP3 requires a QuicUpstreamTransport transport socket: name.*"); +#else + EXPECT_THROW_WITH_REGEX(makeCluster(yaml), EnvoyException, + "HTTP3 configured but not enabled in the build."); +#endif } } // namespace diff --git a/test/extensions/filters/network/http_connection_manager/config_test.cc b/test/extensions/filters/network/http_connection_manager/config_test.cc index 7db4e20d4e8c..40ec2c38e4f9 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -176,6 +176,40 @@ stat_prefix: router EXPECT_EQ(5 * 60 * 1000, config.streamIdleTimeout().count()); } +TEST_F(HttpConnectionManagerConfigTest, Http3Configured) { + const std::string yaml_string = R"EOF( +codec_type: http3 +server_name: foo +stat_prefix: router +route_config: + virtual_hosts: + - name: service + domains: + - "*" + routes: + - match: + prefix: "/" + route: + cluster: cluster +http_filters: +- name: envoy.filters.http.router + )EOF"; + +#ifdef ENVOY_ENABLE_QUIC + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromYaml(yaml_string), context_, + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_, http_tracer_manager_, + filter_config_provider_manager_); +#else + EXPECT_THROW_WITH_MESSAGE( + HttpConnectionManagerConfig(parseHttpConnectionManagerFromYaml(yaml_string), context_, + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_, http_tracer_manager_, + filter_config_provider_manager_), + EnvoyException, "HTTP3 configured but not enabled in the build."); +#endif +} + TEST_F(HttpConnectionManagerConfigTest, TracingNotEnabledAndNoTracingConfigInBootstrap) { const std::string yaml_string = R"EOF( codec_type: http1 From c7b1a51e798ff44f235d126b994eac021dca4c0c Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Tue, 30 Mar 2021 18:11:03 +0000 Subject: [PATCH 3/8] fix Signed-off-by: Matt Klein --- test/common/quic/BUILD | 23 --- test/common/quic/quic_upstream_test.cc | 220 --------------------- test/common/upstream/upstream_impl_test.cc | 50 +++++ test/integration/utility.cc | 1 + 4 files changed, 51 insertions(+), 243 deletions(-) delete mode 100644 test/common/quic/quic_upstream_test.cc diff --git a/test/common/quic/BUILD b/test/common/quic/BUILD index c0b202ac3a02..567e93adce0e 100644 --- a/test/common/quic/BUILD +++ b/test/common/quic/BUILD @@ -281,26 +281,3 @@ envoy_cc_test_library( "@com_googlesource_quiche//:quic_test_tools_qpack_qpack_encoder_test_utils_lib", ], ) - -envoy_cc_test( - name = "quic_upstream_test", - srcs = ["quic_upstream_test.cc"], - data = ["//test/extensions/transport_sockets/tls/test_data:certs"], - tags = ["nofips"], - deps = [ - "//source/common/quic:quic_transport_socket_factory_lib", - "//source/common/singleton:manager_impl_lib", - "//source/common/upstream:static_cluster_lib", - "//source/common/upstream:strict_dns_cluster_lib", - "//test/common/upstream:utility_lib", - "//test/mocks/local_info:local_info_mocks", - "//test/mocks/network:network_mocks", - "//test/mocks/protobuf:protobuf_mocks", - "//test/mocks/server:admin_mocks", - "//test/mocks/server:instance_mocks", - "//test/mocks/ssl:ssl_mocks", - "//test/mocks/upstream:health_checker_mocks", - "//test/test_common:registry_lib", - "//test/test_common:test_runtime_lib", - ], -) diff --git a/test/common/quic/quic_upstream_test.cc b/test/common/quic/quic_upstream_test.cc deleted file mode 100644 index 3090dc1c0b37..000000000000 --- a/test/common/quic/quic_upstream_test.cc +++ /dev/null @@ -1,220 +0,0 @@ -#include -#include -#include -#include -#include -#include - -#include "envoy/api/api.h" - -#include "common/network/utility.h" -#include "common/singleton/manager_impl.h" -#include "common/upstream/static_cluster.h" -#include "common/upstream/strict_dns_cluster.h" - -#include "server/transport_socket_config_impl.h" - -#include "test/common/stats/stat_test_utility.h" -#include "test/common/upstream/utility.h" -#include "test/mocks/common.h" -#include "test/mocks/local_info/mocks.h" -#include "test/mocks/network/mocks.h" -#include "test/mocks/protobuf/mocks.h" -#include "test/mocks/runtime/mocks.h" -#include "test/mocks/server/admin.h" -#include "test/mocks/server/instance.h" -#include "test/mocks/server/options.h" -#include "test/mocks/ssl/mocks.h" -#include "test/mocks/upstream/cluster_info.h" -#include "test/mocks/upstream/cluster_manager.h" -#include "test/mocks/upstream/health_checker.h" -#include "test/mocks/upstream/priority_set.h" -#include "test/test_common/environment.h" -#include "test/test_common/registry.h" -#include "test/test_common/test_runtime.h" -#include "test/test_common/utility.h" - -#include "gmock/gmock.h" -#include "gtest/gtest.h" - -using testing::NiceMock; - -namespace Envoy { -namespace Upstream { -namespace { - -class ClusterInfoImplTest : public testing::Test { -public: - ClusterInfoImplTest() : api_(Api::createApiForTest(stats_, random_)) {} - - std::unique_ptr makeCluster(const std::string& yaml, - bool avoid_boosting = true) { - cluster_config_ = parseClusterFromV3Yaml(yaml, avoid_boosting); - scope_ = stats_.createScope(fmt::format("cluster.{}.", cluster_config_.alt_stat_name().empty() - ? cluster_config_.name() - : cluster_config_.alt_stat_name())); - factory_context_ = std::make_unique( - admin_, ssl_context_manager_, *scope_, cm_, local_info_, dispatcher_, stats_, - singleton_manager_, tls_, validation_visitor_, *api_, options_); - - return std::make_unique(cluster_config_, runtime_, dns_resolver_, - *factory_context_, std::move(scope_), false); - } - - class RetryBudgetTestClusterInfo : public ClusterInfoImpl { - public: - static std::pair, absl::optional> getRetryBudgetParams( - const envoy::config::cluster::v3::CircuitBreakers::Thresholds& thresholds) { - return ClusterInfoImpl::getRetryBudgetParams(thresholds); - } - }; - - Stats::TestUtil::TestStore stats_; - Ssl::MockContextManager ssl_context_manager_; - std::shared_ptr dns_resolver_{new NiceMock()}; - NiceMock dispatcher_; - NiceMock runtime_; - NiceMock cm_; - NiceMock local_info_; - NiceMock random_; - NiceMock admin_; - Singleton::ManagerImpl singleton_manager_{Thread::threadFactoryForTest()}; - NiceMock tls_; - ReadyWatcher initialized_; - envoy::config::cluster::v3::Cluster cluster_config_; - Envoy::Stats::ScopePtr scope_; - std::unique_ptr factory_context_; - NiceMock validation_visitor_; - Api::ApiPtr api_; - Server::MockOptions options_; -}; - -#ifdef ENVOY_ENABLE_QUIC -TEST_F(ClusterInfoImplTest, Http3) { - 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.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 - explicit_http_config: - http3_protocol_options: - quic_protocol_options: - max_concurrent_streams: 2 - common_http_protocol_options: - idle_timeout: 1s - )EOF"; - - const std::string downstream_http3 = R"EOF( - typed_extension_protocol_options: - envoy.extensions.upstreams.http.v3.HttpProtocolOptions: - "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions - use_downstream_protocol_config: - http3_protocol_options: {} - 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); - - auto downstream_h3 = makeCluster(yaml + downstream_http3); - EXPECT_EQ(Http::Protocol::Http3, - downstream_h3->info()->upstreamHttpProtocol({Http::Protocol::Http3})[0]); - EXPECT_FALSE( - downstream_h3->info()->http3Options().quic_protocol_options().has_max_concurrent_streams()); -} -#endif - -TEST_F(ClusterInfoImplTest, Http3BadConfig) { - 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.not_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 - typed_extension_protocol_options: - envoy.extensions.upstreams.http.v3.HttpProtocolOptions: - "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions - use_downstream_protocol_config: - http3_protocol_options: {} - common_http_protocol_options: - idle_timeout: 1s - )EOF", - Network::Address::IpVersion::v4); - -#ifdef ENVOY_ENABLE_QUIC - EXPECT_THROW_WITH_REGEX(makeCluster(yaml), EnvoyException, - "HTTP3 requires a QuicUpstreamTransport transport socket: name.*"); -#else - EXPECT_THROW_WITH_REGEX(makeCluster(yaml), EnvoyException, - "HTTP3 configured but not enabled in the build."); -#endif -} - -} // namespace -} // namespace Upstream -} // namespace Envoy diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index d25f8e230a3f..3184d27e70b9 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -3207,6 +3207,56 @@ TEST_F(ClusterInfoImplTest, Http3) { } #endif // ENVOY_ENABLE_QUIC +TEST_F(ClusterInfoImplTest, Http3BadConfig) { + 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.not_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 + typed_extension_protocol_options: + envoy.extensions.upstreams.http.v3.HttpProtocolOptions: + "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions + use_downstream_protocol_config: + http3_protocol_options: {} + common_http_protocol_options: + idle_timeout: 1s + )EOF", + Network::Address::IpVersion::v4); + +#ifdef ENVOY_ENABLE_QUIC + EXPECT_THROW_WITH_REGEX(makeCluster(yaml), EnvoyException, + "HTTP3 requires a QuicUpstreamTransport transport socket: name.*"); +#else + EXPECT_THROW_WITH_REGEX(makeCluster(yaml), EnvoyException, + "HTTP3 configured but not enabled in the build."); +#endif +} + // Validate empty singleton for HostsPerLocalityImpl. TEST(HostsPerLocalityImpl, Empty) { EXPECT_FALSE(HostsPerLocalityImpl::empty()->hasLocalLocality()); diff --git a/test/integration/utility.cc b/test/integration/utility.cc index 628069846782..37286f756112 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -225,6 +225,7 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt return sendRequestAndWaitForResponse(*dispatcher, method, url, body, host, content_type, client); #else ASSERT(false, "running a QUIC integration test without compiling QUIC"); + return nullptr; #endif } From abfafa06d7f2e2f46a16e0b2b9e2f28a03514238 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Tue, 30 Mar 2021 21:36:14 +0000 Subject: [PATCH 4/8] fix Signed-off-by: Matt Klein --- bazel/envoy_select.bzl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bazel/envoy_select.bzl b/bazel/envoy_select.bzl index 886296d320a1..5f530d00e267 100644 --- a/bazel/envoy_select.bzl +++ b/bazel/envoy_select.bzl @@ -25,10 +25,12 @@ def envoy_select_google_grpc(xs, repository = ""): "//conditions:default": xs, }) -# Selects the given values if http3 is enabled in the current build. +# Selects the given values if http3 is enabled in the current build. Also disable if fips is +# enabled until the fips build supports http3. def envoy_select_enable_http3(xs, repository = ""): return select({ repository + "//bazel:disable_http3": [], + repository + "//bazel:boringssl_fips": [], "//conditions:default": xs, }) From aafca1848a8c71fee384c91ac9a05f55779dfd61 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Tue, 30 Mar 2021 22:49:25 +0000 Subject: [PATCH 5/8] fix Signed-off-by: Matt Klein --- test/common/http/BUILD | 6 ++-- test/common/upstream/upstream_impl_test.cc | 29 +++++++++++++++++-- .../listener_manager_impl_quic_only_test.cc | 2 ++ 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/test/common/http/BUILD b/test/common/http/BUILD index 1b2d87bb38ff..5313d06697c5 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -415,10 +415,11 @@ envoy_cc_test( ], ) +# TODO(mattklein123): Use compatible with when we move to Bazel 4.0. envoy_cc_test( name = "conn_pool_grid_test", - srcs = ["conn_pool_grid_test.cc"], - deps = [ + srcs = envoy_select_enable_http3(["conn_pool_grid_test.cc"]), + deps = envoy_select_enable_http3([ ":common_lib", "//source/common/http:conn_pool_grid", "//test/common/upstream:utility_lib", @@ -431,7 +432,6 @@ envoy_cc_test( "//test/mocks/router:router_mocks", "//test/mocks/runtime:runtime_mocks", "//test/mocks/stats:stats_mocks", - ] + envoy_select_enable_http3([ "//source/common/quic:quic_factory_lib", "//source/common/quic:quic_transport_socket_factory_lib", "//source/common/quic:client_connection_factory_lib", diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index 3184d27e70b9..fdcf38fda1e7 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -3205,7 +3205,6 @@ TEST_F(ClusterInfoImplTest, Http3) { EXPECT_FALSE( downstream_h3->info()->http3Options().quic_protocol_options().has_max_concurrent_streams()); } -#endif // ENVOY_ENABLE_QUIC TEST_F(ClusterInfoImplTest, Http3BadConfig) { const std::string yaml = TestEnvironment::substitute(R"EOF( @@ -3248,14 +3247,38 @@ TEST_F(ClusterInfoImplTest, Http3BadConfig) { )EOF", Network::Address::IpVersion::v4); -#ifdef ENVOY_ENABLE_QUIC EXPECT_THROW_WITH_REGEX(makeCluster(yaml), EnvoyException, "HTTP3 requires a QuicUpstreamTransport transport socket: name.*"); +} #else +TEST_F(ClusterInfoImplTest, Http3BadConfig) { + 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 + typed_extension_protocol_options: + envoy.extensions.upstreams.http.v3.HttpProtocolOptions: + "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions + use_downstream_protocol_config: + http3_protocol_options: {} + common_http_protocol_options: + idle_timeout: 1s + )EOF", + Network::Address::IpVersion::v4); + EXPECT_THROW_WITH_REGEX(makeCluster(yaml), EnvoyException, "HTTP3 configured but not enabled in the build."); -#endif } +#endif // ENVOY_ENABLE_QUIC // Validate empty singleton for HostsPerLocalityImpl. TEST(HostsPerLocalityImpl, Empty) { diff --git a/test/server/listener_manager_impl_quic_only_test.cc b/test/server/listener_manager_impl_quic_only_test.cc index 33212c62e153..d20ac9c7d42d 100644 --- a/test/server/listener_manager_impl_quic_only_test.cc +++ b/test/server/listener_manager_impl_quic_only_test.cc @@ -1,7 +1,9 @@ #include "envoy/config/core/v3/base.pb.h" #include "envoy/config/listener/v3/listener.pb.h" +#if defined(ENVOY_ENABLE_QUIC) #include "common/quic/quic_transport_socket_factory.h" +#endif #include "test/server/listener_manager_impl_test.h" #include "test/server/utility.h" From 23620c7e29eec04ba877b84f7066d02d85a09a0c Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Wed, 31 Mar 2021 04:14:25 +0000 Subject: [PATCH 6/8] fix Signed-off-by: Matt Klein --- bazel/BUILD | 8 ++++++++ bazel/envoy_select.bzl | 6 ++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/bazel/BUILD b/bazel/BUILD index 2cb2c433004e..5df844e1a902 100644 --- a/bazel/BUILD +++ b/bazel/BUILD @@ -299,6 +299,14 @@ config_setting( }, ) +selects.config_setting_group( + name = "disable_http3_all_reasons", + match_any = [ + ":disable_http3", + "boringssl_fips", + ], +) + config_setting( name = "disable_hot_restart", values = {"define": "hot_restart=disabled"}, diff --git a/bazel/envoy_select.bzl b/bazel/envoy_select.bzl index 5f530d00e267..328869ad2a48 100644 --- a/bazel/envoy_select.bzl +++ b/bazel/envoy_select.bzl @@ -25,12 +25,10 @@ def envoy_select_google_grpc(xs, repository = ""): "//conditions:default": xs, }) -# Selects the given values if http3 is enabled in the current build. Also disable if fips is -# enabled until the fips build supports http3. +# Selects the given values if http3 is enabled in the current build. def envoy_select_enable_http3(xs, repository = ""): return select({ - repository + "//bazel:disable_http3": [], - repository + "//bazel:boringssl_fips": [], + repository + "//bazel:disable_http3_all_reasons": [], "//conditions:default": xs, }) From 92a72f45010cad2e6a7b4ddffa98ba550ed27ffa Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Wed, 31 Mar 2021 04:18:56 +0000 Subject: [PATCH 7/8] fix Signed-off-by: Matt Klein --- bazel/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bazel/BUILD b/bazel/BUILD index 5df844e1a902..1073b685d023 100644 --- a/bazel/BUILD +++ b/bazel/BUILD @@ -303,7 +303,7 @@ selects.config_setting_group( name = "disable_http3_all_reasons", match_any = [ ":disable_http3", - "boringssl_fips", + ":boringssl_fips", ], ) From a8eec4df3cb1dee4a2fd27e534474fa966f245e7 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Wed, 31 Mar 2021 17:29:15 +0000 Subject: [PATCH 8/8] comments Signed-off-by: Matt Klein --- source/common/quic/client_connection_factory_impl.cc | 5 +++-- test/common/http/BUILD | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/source/common/quic/client_connection_factory_impl.cc b/source/common/quic/client_connection_factory_impl.cc index c109d50d7ced..8034c74e4741 100644 --- a/source/common/quic/client_connection_factory_impl.cc +++ b/source/common/quic/client_connection_factory_impl.cc @@ -25,8 +25,9 @@ PersistentQuicInfoImpl::PersistentQuicInfoImpl( stats_scope, getConfig(transport_socket_factory), time_source))) {} namespace { -// TODO(mattklein123): This is mutable static info that is required for the QUICHE code. This was -// preexisting but should be removed as we shouldn't be using global mutable static data. +// TODO(alyssawilk, danzh2010): This is mutable static info that is required for the QUICHE code. +// This was preexisting but should either be removed or potentially moved inside +// PersistentQuicInfoImpl. struct StaticInfo { quic::QuicConfig quic_config_; quic::QuicClientPushPromiseIndex push_promise_index_; diff --git a/test/common/http/BUILD b/test/common/http/BUILD index 5313d06697c5..523d5aba4035 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -415,7 +415,8 @@ envoy_cc_test( ], ) -# TODO(mattklein123): Use compatible with when we move to Bazel 4.0. +# TODO(alyssawilk): The conn pool grid should work correctly without H3. Circle back and figure +# out the right way of handling the lack of H3 in the build. envoy_cc_test( name = "conn_pool_grid_test", srcs = envoy_select_enable_http3(["conn_pool_grid_test.cc"]),