Revert "[http] Initial codec splitting with test parametrization (env…

…oyproxy#10591)" (envoyproxy#12142) This reverts commit 568b759. Signed-off-by: Lizan Zhou <[email protected]>
veshij · Jul 17, 2020 · 6f17ae3 · 6f17ae3
1 parent 7498c33
commit 6f17ae3
Show file tree

Hide file tree

Showing 43 changed files with 358 additions and 7,201 deletions.
diff --git a/bazel/BUILD b/bazel/BUILD
@@ -199,11 +199,6 @@ config_setting(
     values = {"define": "path_normalization_by_default=true"},
 )
 
-config_setting(
-    name = "enable_legacy_codecs_in_integration_tests",
-    values = {"define": "use_legacy_codecs_in_integration_tests=true"},
-)
-
 cc_proto_library(
     name = "grpc_health_proto",
     deps = ["@com_github_grpc_grpc//src/proto/grpc/health/v1:_health_proto_only"],

diff --git a/bazel/envoy_build_system.bzl b/bazel/envoy_build_system.bzl
@@ -18,7 +18,6 @@ load(
     _envoy_select_boringssl = "envoy_select_boringssl",
     _envoy_select_google_grpc = "envoy_select_google_grpc",
     _envoy_select_hot_restart = "envoy_select_hot_restart",
-    _envoy_select_legacy_codecs_in_integration_tests = "envoy_select_legacy_codecs_in_integration_tests",
 )
 load(
     ":envoy_test.bzl",
@@ -169,7 +168,6 @@ def envoy_google_grpc_external_deps():
 envoy_select_boringssl = _envoy_select_boringssl
 envoy_select_google_grpc = _envoy_select_google_grpc
 envoy_select_hot_restart = _envoy_select_hot_restart
-envoy_select_legacy_codecs_in_integration_tests = _envoy_select_legacy_codecs_in_integration_tests
 
 # Binary wrappers (from envoy_binary.bzl)
 envoy_cc_binary = _envoy_cc_binary

diff --git a/bazel/envoy_select.bzl b/bazel/envoy_select.bzl
@@ -31,10 +31,3 @@ def envoy_select_hot_restart(xs, repository = ""):
         repository + "//bazel:disable_hot_restart_or_apple": [],
         "//conditions:default": xs,
     })
-
-# Select the given values if use legacy codecs in test is on in the current build.
-def envoy_select_legacy_codecs_in_integration_tests(xs, repository = ""):
-    return select({
-        repository + "//bazel:enable_legacy_codecs_in_integration_tests": xs,
-        "//conditions:default": [],
-    })
diff --git a/ci/do_ci.sh b/ci/do_ci.sh
@@ -220,7 +220,6 @@ elif [[ "$CI_TARGET" == "bazel.compile_time_options" ]]; then
     --define quiche=enabled \
     --define path_normalization_by_default=true \
     --define deprecated_features=disabled \
-    --define use_legacy_codecs_in_integration_tests=true \
     --define --cxxopt=-std=c++14 \
   "
   ENVOY_STDLIB="${ENVOY_STDLIB:-libstdc++}"
@@ -238,10 +237,6 @@ elif [[ "$CI_TARGET" == "bazel.compile_time_options" ]]; then
   echo "Building and testing ${TEST_TARGETS}"
   bazel test ${BAZEL_BUILD_OPTIONS} ${COMPILE_TIME_OPTIONS} -c dbg ${TEST_TARGETS} --test_tag_filters=-nofips --build_tests_only
 
-  # Legacy codecs "--define legacy_codecs_in_integration_tests=true" should also be tested in
-  # integration tests with asan.
-  bazel test ${BAZEL_BUILD_OPTIONS} ${COMPILE_TIME_OPTIONS} -c dbg @envoy//test/integration/... --config=clang-asan --build_tests_only
-
   # "--define log_debug_assert_in_release=enabled" must be tested with a release build, so run only
   # these tests under "-c opt" to save time in CI.
   bazel test ${BAZEL_BUILD_OPTIONS} ${COMPILE_TIME_OPTIONS} -c opt @envoy//test/common/common:assert_test @envoy//test/server:server_test

diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
@@ -36,7 +36,6 @@ New Features
 
 * ext_authz filter: added support for emitting dynamic metadata for both :ref:`HTTP <config_http_filters_ext_authz_dynamic_metadata>` and :ref:`network <config_network_filters_ext_authz_dynamic_metadata>` filters.
 * grpc-json: support specifying `response_body` field in for `google.api.HttpBody` message.
-* http: introduced new HTTP/1 and HTTP/2 codec implementations that will remove the use of exceptions for control flow due to high risk factors and instead use error statuses. The old behavior is deprecated, but can be used during the removal period by setting the runtime feature `envoy.reloadable_features.new_codec_behavior` to false. The removal period will be one month.
 * load balancer: added a :ref:`configuration<envoy_v3_api_msg_config.cluster.v3.Cluster.LeastRequestLbConfig>` option to specify the active request bias used by the least request load balancer.
 * tap: added :ref:`generic body matcher<envoy_v3_api_msg_config.tap.v3.HttpGenericBodyMatch>` to scan http requests and responses for text or hex patterns.
 

diff --git a/source/common/http/BUILD b/source/common/http/BUILD
@@ -57,21 +57,16 @@ envoy_cc_library(
         "//include/envoy/http:codec_interface",
         "//include/envoy/network:connection_interface",
         "//include/envoy/network:filter_interface",
-        "//include/envoy/runtime:runtime_interface",
         "//source/common/common:assert_lib",
         "//source/common/common:enum_to_int",
         "//source/common/common:linked_object",
         "//source/common/common:minimal_logger_lib",
         "//source/common/config:utility_lib",
-        "//source/common/http/http1:codec_legacy_lib",
         "//source/common/http/http1:codec_lib",
-        "//source/common/http/http2:codec_legacy_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",
-        "//source/common/runtime:runtime_features_lib",
-        "//source/common/runtime:runtime_lib",
     ],
 )
 
@@ -212,9 +207,7 @@ envoy_cc_library(
         "//source/common/common:scope_tracker",
         "//source/common/common:utility_lib",
         "//source/common/config:utility_lib",
-        "//source/common/http/http1:codec_legacy_lib",
         "//source/common/http/http1:codec_lib",
-        "//source/common/http/http2:codec_legacy_lib",
         "//source/common/http/http2:codec_lib",
         "//source/common/http/http3:quic_codec_factory_lib",
         "//source/common/http/http3:well_known_names",

diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc
@@ -9,15 +9,11 @@
 #include "common/config/utility.h"
 #include "common/http/exception.h"
 #include "common/http/http1/codec_impl.h"
-#include "common/http/http1/codec_impl_legacy.h"
 #include "common/http/http2/codec_impl.h"
-#include "common/http/http2/codec_impl_legacy.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"
-#include "common/runtime/runtime_features.h"
-#include "common/runtime/runtime_impl.h"
 
 namespace Envoy {
 namespace Http {
@@ -154,37 +150,23 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne
 
   switch (type) {
   case Type::HTTP1: {
-    if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.new_codec_behavior")) {
-      codec_ = std::make_unique<Http1::ClientConnectionImpl>(
-          *connection_, host->cluster().http1CodecStats(), *this, host->cluster().http1Settings(),
-          host->cluster().maxResponseHeadersCount());
-    } else {
-      codec_ = std::make_unique<Legacy::Http1::ClientConnectionImpl>(
-          *connection_, host->cluster().http1CodecStats(), *this, host->cluster().http1Settings(),
-          host->cluster().maxResponseHeadersCount());
-    }
+    codec_ = std::make_unique<Http1::ClientConnectionImpl>(
+        *connection_, host->cluster().http1CodecStats(), *this, host->cluster().http1Settings(),
+        host->cluster().maxResponseHeadersCount());
     break;
   }
   case Type::HTTP2: {
-    if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.new_codec_behavior")) {
-      codec_ = std::make_unique<Http2::ClientConnectionImpl>(
-          *connection_, *this, host->cluster().http2CodecStats(), host->cluster().http2Options(),
-          Http::DEFAULT_MAX_REQUEST_HEADERS_KB, host->cluster().maxResponseHeadersCount(),
-          Http2::ProdNghttp2SessionFactory::get());
-    } else {
-      codec_ = std::make_unique<Http2::ClientConnectionImpl>(
-          *connection_, *this, host->cluster().http2CodecStats(), host->cluster().http2Options(),
-          Http::DEFAULT_MAX_REQUEST_HEADERS_KB, host->cluster().maxResponseHeadersCount(),
-          Http2::ProdNghttp2SessionFactory::get());
-    }
+    codec_ = std::make_unique<Http2::ClientConnectionImpl>(
+        *connection_, *this, host->cluster().http2CodecStats(), host->cluster().http2Options(),
+        Http::DEFAULT_MAX_REQUEST_HEADERS_KB, host->cluster().maxResponseHeadersCount(),
+        Http2::ProdNghttp2SessionFactory::get());
     break;
   }
   case Type::HTTP3: {
     codec_ = std::unique_ptr<ClientConnection>(
         Config::Utility::getAndCheckFactoryByName<Http::QuicHttpClientConnectionFactory>(
             Http::QuicCodecNames::get().Quiche)
             .createQuicClientConnection(*connection_, *this));
-    break;
   }
   }
 }

diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc
@@ -11,9 +11,7 @@
 #include "common/http/header_utility.h"
 #include "common/http/headers.h"
 #include "common/http/http1/codec_impl.h"
-#include "common/http/http1/codec_impl_legacy.h"
 #include "common/http/http2/codec_impl.h"
-#include "common/http/http2/codec_impl_legacy.h"
 #include "common/http/path_utility.h"
 #include "common/http/utility.h"
 #include "common/network/utility.h"
@@ -53,26 +51,14 @@ ServerConnectionPtr ConnectionManagerUtility::autoCreateCodec(
         headers_with_underscores_action) {
   if (determineNextProtocol(connection, data) == Utility::AlpnNames::get().Http2) {
     Http2::CodecStats& stats = Http2::CodecStats::atomicGet(http2_codec_stats, scope);
-    if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.new_codec_behavior")) {
-      return std::make_unique<Http2::ServerConnectionImpl>(
-          connection, callbacks, stats, http2_options, max_request_headers_kb,
-          max_request_headers_count, headers_with_underscores_action);
-    } else {
-      return std::make_unique<Legacy::Http2::ServerConnectionImpl>(
-          connection, callbacks, stats, http2_options, max_request_headers_kb,
-          max_request_headers_count, headers_with_underscores_action);
-    }
+    return std::make_unique<Http2::ServerConnectionImpl>(
+        connection, callbacks, stats, http2_options, max_request_headers_kb,
+        max_request_headers_count, headers_with_underscores_action);
   } else {
     Http1::CodecStats& stats = Http1::CodecStats::atomicGet(http1_codec_stats, scope);
-    if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.new_codec_behavior")) {
-      return std::make_unique<Http1::ServerConnectionImpl>(
-          connection, stats, callbacks, http1_settings, max_request_headers_kb,
-          max_request_headers_count, headers_with_underscores_action);
-    } else {
-      return std::make_unique<Legacy::Http1::ServerConnectionImpl>(
-          connection, stats, callbacks, http1_settings, max_request_headers_kb,
-          max_request_headers_count, headers_with_underscores_action);
-    }
+    return std::make_unique<Http1::ServerConnectionImpl>(
+        connection, stats, callbacks, http1_settings, max_request_headers_kb,
+        max_request_headers_count, headers_with_underscores_action);
   }
 }
 

diff --git a/source/common/http/http1/BUILD b/source/common/http/http1/BUILD
@@ -24,46 +24,36 @@ envoy_cc_library(
     ],
 )
 
-CODEC_LIB_DEPS = [
-    ":codec_stats_lib",
-    ":header_formatter_lib",
-    "//include/envoy/buffer:buffer_interface",
-    "//include/envoy/http:codec_interface",
-    "//include/envoy/http:header_map_interface",
-    "//include/envoy/network:connection_interface",
-    "//source/common/buffer:buffer_lib",
-    "//source/common/buffer:watermark_buffer_lib",
-    "//source/common/common:assert_lib",
-    "//source/common/common:statusor_lib",
-    "//source/common/common:utility_lib",
-    "//source/common/grpc:common_lib",
-    "//source/common/http:codec_helper_lib",
-    "//source/common/http:codes_lib",
-    "//source/common/http:exception_lib",
-    "//source/common/http:header_map_lib",
-    "//source/common/http:header_utility_lib",
-    "//source/common/http:headers_lib",
-    "//source/common/http:status_lib",
-    "//source/common/http:url_utility_lib",
-    "//source/common/http:utility_lib",
-    "//source/common/runtime:runtime_features_lib",
-    "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
-]
-
 envoy_cc_library(
     name = "codec_lib",
     srcs = ["codec_impl.cc"],
     hdrs = ["codec_impl.h"],
     external_deps = ["http_parser"],
-    deps = CODEC_LIB_DEPS,
-)
-
-envoy_cc_library(
-    name = "codec_legacy_lib",
-    srcs = ["codec_impl_legacy.cc"],
-    hdrs = ["codec_impl_legacy.h"],
-    external_deps = ["http_parser"],
-    deps = CODEC_LIB_DEPS,
+    deps = [
+        ":codec_stats_lib",
+        ":header_formatter_lib",
+        "//include/envoy/buffer:buffer_interface",
+        "//include/envoy/http:codec_interface",
+        "//include/envoy/http:header_map_interface",
+        "//include/envoy/network:connection_interface",
+        "//source/common/buffer:buffer_lib",
+        "//source/common/buffer:watermark_buffer_lib",
+        "//source/common/common:assert_lib",
+        "//source/common/common:statusor_lib",
+        "//source/common/common:utility_lib",
+        "//source/common/grpc:common_lib",
+        "//source/common/http:codec_helper_lib",
+        "//source/common/http:codes_lib",
+        "//source/common/http:exception_lib",
+        "//source/common/http:header_map_lib",
+        "//source/common/http:header_utility_lib",
+        "//source/common/http:headers_lib",
+        "//source/common/http:status_lib",
+        "//source/common/http:url_utility_lib",
+        "//source/common/http:utility_lib",
+        "//source/common/runtime:runtime_features_lib",
+        "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
+    ],
 )
 
 envoy_cc_library(