http: unifying client types

include/envoy/http/codec.h

@@ -19,6 +19,8 @@
 namespace Envoy {
 namespace Http {
 
+enum class CodecType { HTTP1, HTTP2, HTTP3 };
+
 namespace Http1 {
 struct CodecStats;
 }

source/common/http/codec_client.cc

@@ -20,12 +20,12 @@
 namespace Envoy {
 namespace Http {
 
-CodecClient::CodecClient(Type type, Network::ClientConnectionPtr&& connection,
+CodecClient::CodecClient(CodecType type, Network::ClientConnectionPtr&& connection,
                          Upstream::HostDescriptionConstSharedPtr host,
                          Event::Dispatcher& dispatcher)
     : type_(type), host_(host), connection_(std::move(connection)),
       idle_timeout_(host_->cluster().idleTimeout()) {
-  if (type_ != Type::HTTP3) {
+  if (type_ != CodecType::HTTP3) {
     // Make sure upstream connections process data and then the FIN, rather than processing
     // TCP disconnects immediately. (see https://github.com/envoyproxy/envoy/issues/1679 for
     // details)
@@ -95,7 +95,7 @@ void CodecClient::onEvent(Network::ConnectionEvent event) {
   }
 
   // HTTP/1 can signal end of response by disconnecting. We need to handle that case.
-  if (type_ == Type::HTTP1 && event == Network::ConnectionEvent::RemoteClose &&
+  if (type_ == CodecType::HTTP1 && event == Network::ConnectionEvent::RemoteClose &&
       !active_requests_.empty()) {
     Buffer::OwnedImpl empty;
     onData(empty);
@@ -169,26 +169,26 @@ void CodecClient::onData(Buffer::Instance& data) {
          absl::StrCat("extraneous bytes after response complete: ", data.length()));
 }
 
-CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& connection,
+CodecClientProd::CodecClientProd(CodecType type, Network::ClientConnectionPtr&& connection,
                                  Upstream::HostDescriptionConstSharedPtr host,
                                  Event::Dispatcher& dispatcher,
                                  Random::RandomGenerator& random_generator)
     : CodecClient(type, std::move(connection), host, dispatcher) {
   switch (type) {
-  case Type::HTTP1: {
+  case CodecType::HTTP1: {
     codec_ = std::make_unique<Http1::ClientConnectionImpl>(
         *connection_, host->cluster().http1CodecStats(), *this, host->cluster().http1Settings(),
         host->cluster().maxResponseHeadersCount());
     break;
   }
-  case Type::HTTP2: {
+  case CodecType::HTTP2: {
     codec_ = std::make_unique<Http2::ClientConnectionImpl>(
         *connection_, *this, host->cluster().http2CodecStats(), random_generator,
         host->cluster().http2Options(), Http::DEFAULT_MAX_REQUEST_HEADERS_KB,
         host->cluster().maxResponseHeadersCount(), Http2::ProdNghttp2SessionFactory::get());
     break;
   }
-  case Type::HTTP3: {
+  case CodecType::HTTP3: {
 #ifdef ENVOY_ENABLE_QUIC
     auto& quic_session = dynamic_cast<Quic::EnvoyQuicClientSession&>(*connection_);
     codec_ = std::make_unique<Quic::QuicHttpClientConnectionImpl>(

source/common/http/codec_client.h

@@ -55,7 +55,8 @@ class CodecClient : protected Logger::Loggable<Logger::Id::client>,
   /**
    * Type of HTTP codec to use.
    */
-  enum class Type { HTTP1, HTTP2, HTTP3 };
+  // This is a legacy alias.
+  using Type = Envoy::Http::CodecType;
 
   ~CodecClient() override;
 
@@ -125,7 +126,7 @@ class CodecClient : protected Logger::Loggable<Logger::Id::client>,
 
   bool remoteClosed() const { return remote_closed_; }
 
-  Type type() const { return type_; }
+  CodecType type() const { return type_; }
 
   // Note this is the L4 stream info, not L7.
   const StreamInfo::StreamInfo& streamInfo() { return connection_->streamInfo(); }
@@ -137,7 +138,7 @@ class CodecClient : protected Logger::Loggable<Logger::Id::client>,
    * @param connection supplies the connection to communicate on.
    * @param host supplies the owning host.
    */
-  CodecClient(Type type, Network::ClientConnectionPtr&& connection,
+  CodecClient(CodecType type, Network::ClientConnectionPtr&& connection,
               Upstream::HostDescriptionConstSharedPtr host, Event::Dispatcher& dispatcher);
 
   /**
@@ -175,7 +176,7 @@ class CodecClient : protected Logger::Loggable<Logger::Id::client>,
     }
   }
 
-  const Type type_;
+  const CodecType type_;
   // The order of host_, connection_, and codec_ matter as during destruction each can refer to
   // the previous, at least in tests.
   Upstream::HostDescriptionConstSharedPtr host_;
@@ -273,7 +274,7 @@ using CodecClientPtr = std::unique_ptr<CodecClient>;
  */
 class CodecClientProd : public CodecClient {
 public:
-  CodecClientProd(Type type, Network::ClientConnectionPtr&& connection,
+  CodecClientProd(CodecType type, Network::ClientConnectionPtr&& connection,
                   Upstream::HostDescriptionConstSharedPtr host, Event::Dispatcher& dispatcher,
                   Random::RandomGenerator& random_generator);
 };

source/common/http/http1/conn_pool.cc

@@ -108,9 +108,9 @@ allocateConnPool(Event::Dispatcher& dispatcher, Random::RandomGenerator& random_
       random_generator, state,
       [](HttpConnPoolImplBase* pool) { return std::make_unique<ActiveClient>(*pool); },
       [](Upstream::Host::CreateConnectionData& data, HttpConnPoolImplBase* pool) {
-        CodecClientPtr codec{new CodecClientProd(
-            CodecClient::Type::HTTP1, std::move(data.connection_), data.host_description_,
-            pool->dispatcher(), pool->randomGenerator())};
+        CodecClientPtr codec{new CodecClientProd(CodecType::HTTP1, std::move(data.connection_),
+                                                 data.host_description_, pool->dispatcher(),
+                                                 pool->randomGenerator())};
         return codec;
       },
       std::vector<Protocol>{Protocol::Http11});

source/common/http/http2/conn_pool.cc

@@ -33,9 +33,9 @@ allocateConnPool(Event::Dispatcher& dispatcher, Random::RandomGenerator& random_
       host, priority, dispatcher, options, transport_socket_options, random_generator, state,
       [](HttpConnPoolImplBase* pool) { return std::make_unique<ActiveClient>(*pool); },
       [](Upstream::Host::CreateConnectionData& data, HttpConnPoolImplBase* pool) {
-        CodecClientPtr codec{new CodecClientProd(
-            CodecClient::Type::HTTP2, std::move(data.connection_), data.host_description_,
-            pool->dispatcher(), pool->randomGenerator())};
+        CodecClientPtr codec{new CodecClientProd(CodecType::HTTP2, std::move(data.connection_),
+                                                 data.host_description_, pool->dispatcher(),
+                                                 pool->randomGenerator())};
         return codec;
       },
       std::vector<Protocol>{Protocol::Http2});

source/common/http/http3/conn_pool.cc

@@ -81,9 +81,9 @@ allocateConnPool(Event::Dispatcher& dispatcher, Random::RandomGenerator& random_
         return std::make_unique<ActiveClient>(*pool, data);
       },
       [](Upstream::Host::CreateConnectionData& data, HttpConnPoolImplBase* pool) {
-        CodecClientPtr codec{new CodecClientProd(
-            CodecClient::Type::HTTP3, std::move(data.connection_), data.host_description_,
-            pool->dispatcher(), pool->randomGenerator())};
+        CodecClientPtr codec{new CodecClientProd(CodecType::HTTP3, std::move(data.connection_),
+                                                 data.host_description_, pool->dispatcher(),
+                                                 pool->randomGenerator())};
         return codec;
       },
       std::vector<Protocol>{Protocol::Http3}, time_source);

source/common/http/mixed_conn_pool.cc

@@ -16,8 +16,7 @@ Envoy::ConnectionPool::ActiveClientPtr HttpConnPoolImplMixed::instantiateActiveC
 
 CodecClientPtr
 HttpConnPoolImplMixed::createCodecClient(Upstream::Host::CreateConnectionData& data) {
-  auto protocol =
-      protocol_ == Protocol::Http11 ? CodecClient::Type::HTTP1 : CodecClient::Type::HTTP2;
+  auto protocol = protocol_ == Protocol::Http11 ? CodecType::HTTP1 : CodecType::HTTP2;
   CodecClientPtr codec{new CodecClientProd(protocol, std::move(data.connection_),
                                            data.host_description_, dispatcher_, random_generator_)};
   return codec;

source/common/tcp_proxy/upstream.cc

@@ -193,7 +193,7 @@ void TcpConnPool::onPoolReady(Tcp::ConnectionPool::ConnectionDataPtr&& conn_data
 HttpConnPool::HttpConnPool(Upstream::ThreadLocalCluster& thread_local_cluster,
                            Upstream::LoadBalancerContext* context, const TunnelingConfig& config,
                            Tcp::ConnectionPool::UpstreamCallbacks& upstream_callbacks,
-                           Http::CodecClient::Type type)
+                           Http::CodecType type)
     : config_(config), type_(type), upstream_callbacks_(upstream_callbacks) {
   conn_pool_ = thread_local_cluster.httpConnPool(Upstream::ResourcePriority::Default, absl::nullopt,
                                                  context);
@@ -209,7 +209,7 @@ HttpConnPool::~HttpConnPool() {
 
 void HttpConnPool::newStream(GenericConnectionPoolCallbacks& callbacks) {
   callbacks_ = &callbacks;
-  if (type_ == Http::CodecClient::Type::HTTP1) {
+  if (type_ == Http::CodecType::HTTP1) {
     upstream_ = std::make_unique<Http1Upstream>(upstream_callbacks_, config_);
   } else {
     upstream_ = std::make_unique<Http2Upstream>(upstream_callbacks_, config_);

source/common/tcp_proxy/upstream.h

@@ -48,12 +48,11 @@ class HttpConnPool : public GenericConnPool, public Http::ConnectionPool::Callba
 
   HttpConnPool(Upstream::ThreadLocalCluster& thread_local_cluster,
                Upstream::LoadBalancerContext* context, const TunnelingConfig& config,
-               Tcp::ConnectionPool::UpstreamCallbacks& upstream_callbacks,
-               Http::CodecClient::Type type);
+               Tcp::ConnectionPool::UpstreamCallbacks& upstream_callbacks, Http::CodecType type);
   ~HttpConnPool() override;
 
   // HTTP/3 upstreams are not supported at the moment.
-  bool valid() const { return conn_pool_ != nullptr && type_ <= Http::CodecClient::Type::HTTP2; }
+  bool valid() const { return conn_pool_ != nullptr && type_ <= Http::CodecType::HTTP2; }
 
   // GenericConnPool
   void newStream(GenericConnectionPoolCallbacks& callbacks) override;
@@ -97,7 +96,7 @@ class HttpConnPool : public GenericConnPool, public Http::ConnectionPool::Callba
                           const Network::Address::InstanceConstSharedPtr& local_address,
                           Ssl::ConnectionInfoConstSharedPtr ssl_info);
   const TunnelingConfig config_;
-  Http::CodecClient::Type type_;
+  Http::CodecType type_;
   Http::ConnectionPool::Instance* conn_pool_{};
   Http::ConnectionPool::Cancellable* upstream_handle_{};
   GenericConnectionPoolCallbacks* callbacks_{};

source/common/upstream/health_checker_impl.cc

@@ -197,13 +197,13 @@ bool HttpHealthCheckerImpl::HttpStatusChecker::inRange(uint64_t http_status) con
   return false;
 }
 
-Http::Protocol codecClientTypeToProtocol(Http::CodecClient::Type codec_client_type) {
+Http::Protocol codecClientTypeToProtocol(Http::CodecType codec_client_type) {
   switch (codec_client_type) {
-  case Http::CodecClient::Type::HTTP1:
+  case Http::CodecType::HTTP1:
     return Http::Protocol::Http11;
-  case Http::CodecClient::Type::HTTP2:
+  case Http::CodecType::HTTP2:
     return Http::Protocol::Http2;
-  case Http::CodecClient::Type::HTTP3:
+  case Http::CodecType::HTTP3:
     return Http::Protocol::Http3;
   default:
     NOT_REACHED_GCOVR_EXCL_LINE;
@@ -422,15 +422,15 @@ void HttpHealthCheckerImpl::HttpActiveHealthCheckSession::onTimeout() {
   }
 }
 
-Http::CodecClient::Type
+Http::CodecType
 HttpHealthCheckerImpl::codecClientType(const envoy::type::v3::CodecClientType& type) {
   switch (type) {
   case envoy::type::v3::HTTP3:
-    return Http::CodecClient::Type::HTTP3;
+    return Http::CodecType::HTTP3;
   case envoy::type::v3::HTTP2:
-    return Http::CodecClient::Type::HTTP2;
+    return Http::CodecType::HTTP2;
   case envoy::type::v3::HTTP1:
-    return Http::CodecClient::Type::HTTP1;
+    return Http::CodecType::HTTP1;
   default:
     NOT_REACHED_GCOVR_EXCL_LINE;
   }
@@ -892,8 +892,8 @@ void GrpcHealthCheckerImpl::GrpcActiveHealthCheckSession::logHealthCheckStatus(
 Http::CodecClientPtr
 ProdGrpcHealthCheckerImpl::createCodecClient(Upstream::Host::CreateConnectionData& data) {
   return std::make_unique<Http::CodecClientProd>(
-      Http::CodecClient::Type::HTTP2, std::move(data.connection_), data.host_description_,
-      dispatcher_, random_generator_);
+      Http::CodecType::HTTP2, std::move(data.connection_), data.host_description_, dispatcher_,
+      random_generator_);
 }
 
 std::ostream& operator<<(std::ostream& out, HealthState state) {

source/common/upstream/health_checker_impl.h

@@ -159,7 +159,7 @@ class HttpHealthCheckerImpl : public HealthCheckerImplBase {
     return envoy::data::core::v3::HTTP;
   }
 
-  Http::CodecClient::Type codecClientType(const envoy::type::v3::CodecClientType& type);
+  Http::CodecType codecClientType(const envoy::type::v3::CodecClientType& type);
 
   const std::string path_;
   const std::string host_value_;
@@ -168,7 +168,7 @@ class HttpHealthCheckerImpl : public HealthCheckerImplBase {
   const HttpStatusChecker http_status_checker_;
 
 protected:
-  const Http::CodecClient::Type codec_client_type_;
+  const Http::CodecType codec_client_type_;
   Random::RandomGenerator& random_generator_;
 };
 

source/extensions/upstreams/tcp/generic/config.cc

@@ -18,8 +18,8 @@ TcpProxy::GenericConnPoolPtr GenericConnPoolFactory::createGenericConnPool(
   if (config.has_value()) {
     auto pool_type =
         ((thread_local_cluster.info()->features() & Upstream::ClusterInfo::Features::HTTP2) != 0)
-            ? Http::CodecClient::Type::HTTP2
-            : Http::CodecClient::Type::HTTP1;
+            ? Http::CodecType::HTTP2
+            : Http::CodecType::HTTP1;
     auto ret = std::make_unique<TcpProxy::HttpConnPool>(
         thread_local_cluster, context, config.value(), upstream_callbacks, pool_type);
     return (ret->valid() ? std::move(ret) : nullptr);

test/common/grpc/grpc_client_integration_test_harness.h

@@ -246,7 +246,7 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest {
   virtual void initialize() {
     if (fake_upstream_ == nullptr) {
       FakeUpstreamConfig config(test_time_.timeSystem());
-      config.upstream_protocol_ = FakeHttpConnection::Type::HTTP2;
+      config.upstream_protocol_ = Http::CodecType::HTTP2;
       fake_upstream_ = std::make_unique<FakeUpstream>(0, ipVersion(), config);
     }
     switch (clientType()) {
@@ -524,7 +524,7 @@ class GrpcSslClientIntegrationTest : public GrpcClientIntegrationTest {
     async_client_transport_socket_ =
         mock_host_description_->socket_factory_->createTransportSocket(nullptr);
     FakeUpstreamConfig config(test_time_.timeSystem());
-    config.upstream_protocol_ = FakeHttpConnection::Type::HTTP2;
+    config.upstream_protocol_ = Http::CodecType::HTTP2;
     fake_upstream_ =
         std::make_unique<FakeUpstream>(createUpstreamSslContext(), 0, ipVersion(), config);
 

test/common/http/codec_client_test.cc

@@ -56,8 +56,8 @@ class CodecClientTest : public Event::TestUsingSimulatedTime, public testing::Te
 
     Network::ClientConnectionPtr connection{connection_};
     EXPECT_CALL(dispatcher_, createTimer_(_));
-    client_ = std::make_unique<CodecClientForTest>(CodecClient::Type::HTTP1, std::move(connection),
-                                                   codec_, nullptr, host_, dispatcher_);
+    client_ = std::make_unique<CodecClientForTest>(CodecType::HTTP1, std::move(connection), codec_,
+                                                   nullptr, host_, dispatcher_);
     ON_CALL(*connection_, streamInfo()).WillByDefault(ReturnRef(stream_info_));
   }
 
@@ -87,8 +87,8 @@ TEST_F(CodecClientTest, NotCallDetectEarlyCloseWhenReadDiabledUsingHttp3) {
   auto codec = new Http::MockClientConnection();
 
   EXPECT_CALL(dispatcher_, createTimer_(_));
-  client_ = std::make_unique<CodecClientForTest>(CodecClient::Type::HTTP3, std::move(connection),
-                                                 codec, nullptr, host_, dispatcher_);
+  client_ = std::make_unique<CodecClientForTest>(CodecType::HTTP3, std::move(connection), codec,
+                                                 nullptr, host_, dispatcher_);
 }
 
 TEST_F(CodecClientTest, BasicHeaderOnlyResponse) {
@@ -308,9 +308,8 @@ class CodecNetworkTest : public Event::TestUsingSimulatedTime,
     client_connection_->addConnectionCallbacks(client_callbacks_);
 
     codec_ = new Http::MockClientConnection();
-    client_ =
-        std::make_unique<CodecClientForTest>(CodecClient::Type::HTTP1, std::move(client_connection),
-                                             codec_, nullptr, host_, *dispatcher_);
+    client_ = std::make_unique<CodecClientForTest>(CodecType::HTTP1, std::move(client_connection),
+                                                   codec_, nullptr, host_, *dispatcher_);
 
     int expected_callbacks = 2;
     EXPECT_CALL(listener_callbacks_, onAccept_(_))

test/common/http/common.h

@@ -17,7 +17,7 @@ namespace Envoy {
 class CodecClientForTest : public Http::CodecClient {
 public:
   using DestroyCb = std::function<void(CodecClient*)>;
-  CodecClientForTest(CodecClient::Type type, Network::ClientConnectionPtr&& connection,
+  CodecClientForTest(Http::CodecType type, Network::ClientConnectionPtr&& connection,
                      Http::ClientConnection* codec, DestroyCb destroy_cb,
                      Upstream::HostDescriptionConstSharedPtr host, Event::Dispatcher& dispatcher)
       : CodecClient(type, std::move(connection), host, dispatcher), destroy_cb_(destroy_cb) {

test/common/http/http1/conn_pool_test.cc

@@ -106,7 +106,7 @@ class ConnPoolImplForTest : public Event::TestUsingSimulatedTime, public FixedHt
     test_client.client_dispatcher_ = api_->allocateDispatcher("test_thread");
     Network::ClientConnectionPtr connection{test_client.connection_};
     test_client.codec_client_ = new CodecClientForTest(
-        CodecClient::Type::HTTP1, std::move(connection), test_client.codec_,
+        CodecType::HTTP1, std::move(connection), test_client.codec_,
         [this](CodecClient* codec_client) -> void {
           for (auto i = test_clients_.begin(); i != test_clients_.end(); i++) {
             if (i->codec_client_ == codec_client) {

test/common/http/http2/conn_pool_test.cc

@@ -131,7 +131,7 @@ class Http2ConnPoolImplTest : public Event::TestUsingSimulatedTime, public testi
       auto cluster = std::make_shared<NiceMock<Upstream::MockClusterInfo>>();
       Network::ClientConnectionPtr connection{test_client.connection_};
       test_client.codec_client_ = new CodecClientForTest(
-          CodecClient::Type::HTTP1, std::move(connection), test_client.codec_,
+          CodecType::HTTP1, std::move(connection), test_client.codec_,
           [this](CodecClient*) -> void { onClientDestroy(); },
           Upstream::makeTestHost(cluster, "tcp://127.0.0.1:9000", simTime()),
           *test_client.client_dispatcher_);

test/common/upstream/health_check_fuzz.cc

@@ -343,9 +343,9 @@ void GrpcHealthCheckFuzz::initialize(test::common::upstream::HealthCheckTestCase
             Event::MockDispatcher dispatcher_;
             auto time_source = std::make_unique<NiceMock<MockTimeSystem>>();
             test_session.codec_client_ = new CodecClientForTest(
-                Http::CodecClient::Type::HTTP1, std::move(conn_data.connection_),
-                test_session.codec_, nullptr,
-                Upstream::makeTestHost(cluster, "tcp://127.0.0.1:9000", *time_source), dispatcher_);
+                Http::CodecType::HTTP1, std::move(conn_data.connection_), test_session.codec_,
+                nullptr, Upstream::makeTestHost(cluster, "tcp://127.0.0.1:9000", *time_source),
+                dispatcher_);
             return test_session.codec_client_;
           }));
   expectStreamCreate();