runtime: remove HCM stream error runtime override (envoyproxy#16040)

* Remove HCM stream error runtime override. Remove envoy.reloadable_features.hcm_stream_error_on_invalid_message now deprecated

Signed-off-by: Alex Konradi <akonradi@google.com>
Signed-off-by: Gokul Nair <gnair@twitter.com>

docs/root/version_history/current.rst

@@ -27,6 +27,7 @@ Removed Config or Runtime
 -------------------------
 *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`
 
+* http: removed `envoy.reloadable_features.hcm_stream_error_on_invalid_message` for disabling closing HTTP/1.1 connections on error. Connection-closing can still be disabled by setting the HTTP/1 configuration :ref:`override_stream_error_on_invalid_http_message <envoy_v3_api_field_config.core.v3.Http1ProtocolOptions.override_stream_error_on_invalid_http_message>`.
 * http: removed `envoy.reloadable_features.overload_manager_disable_keepalive_drain_http2`; Envoy will now always send GOAWAY to HTTP2 downstreams when the :ref:`disable_keepalive <config_overload_manager_overload_actions>` overload action is active.
 * tls: removed `envoy.reloadable_features.tls_use_io_handle_bio` runtime guard and legacy code path.
 

source/common/http/conn_manager_impl.cc

@@ -1312,9 +1312,7 @@ absl::optional<Router::ConfigConstSharedPtr> ConnectionManagerImpl::ActiveStream
 
 void ConnectionManagerImpl::ActiveStream::onLocalReply(Code code) {
   // The BadRequest error code indicates there has been a messaging error.
-  if (Runtime::runtimeFeatureEnabled(
-          "envoy.reloadable_features.hcm_stream_error_on_invalid_message") &&
-      code == Http::Code::BadRequest && connection_manager_.codec_->protocol() < Protocol::Http2 &&
+  if (code == Http::Code::BadRequest && connection_manager_.codec_->protocol() < Protocol::Http2 &&
       !response_encoder_->streamErrorOnInvalidHttpMessage()) {
     state_.saw_connection_close_ = true;
   }

source/common/http/utility.cc

@@ -128,9 +128,7 @@ initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions
                              bool hcm_stream_error_set,
                              const Protobuf::BoolValue& hcm_stream_error) {
   auto ret = initializeAndValidateOptions(options);
-  if (Runtime::runtimeFeatureEnabled(
-          "envoy.reloadable_features.hcm_stream_error_on_invalid_message") &&
-      !options.has_override_stream_error_on_invalid_http_message() && hcm_stream_error_set) {
+  if (!options.has_override_stream_error_on_invalid_http_message() && hcm_stream_error_set) {
     ret.mutable_override_stream_error_on_invalid_http_message()->set_value(
         hcm_stream_error.value());
   }
@@ -211,9 +209,7 @@ initializeAndValidateOptions(const envoy::config::core::v3::Http3ProtocolOptions
     return options;
   }
   envoy::config::core::v3::Http3ProtocolOptions options_clone(options);
-  if (Runtime::runtimeFeatureEnabled(
-          "envoy.reloadable_features.hcm_stream_error_on_invalid_message") &&
-      hcm_stream_error_set) {
+  if (hcm_stream_error_set) {
     options_clone.mutable_override_stream_error_on_invalid_http_message()->set_value(
         hcm_stream_error.value());
   } else {

source/common/runtime/runtime_features.cc

@@ -70,7 +70,6 @@ constexpr const char* runtime_features[] = {
     "envoy.reloadable_features.grpc_web_fix_non_proto_encoded_response_handling",
     "envoy.reloadable_features.grpc_json_transcoder_adhere_to_buffer_limits",
     "envoy.reloadable_features.hash_multiple_header_values",
-    "envoy.reloadable_features.hcm_stream_error_on_invalid_message",
     "envoy.reloadable_features.health_check.graceful_goaway_handling",
     "envoy.reloadable_features.health_check.immediate_failure_exclude_from_cluster",
     "envoy.reloadable_features.http_match_on_all_headers",

test/common/http/utility_test.cc

@@ -14,8 +14,8 @@
 #include "common/network/address_impl.h"
 
 #include "test/mocks/http/mocks.h"
+#include "test/mocks/protobuf/mocks.h"
 #include "test/test_common/printers.h"
-#include "test/test_common/test_runtime.h"
 #include "test/test_common/utility.h"
 
 #include "gtest/gtest.h"
@@ -409,16 +409,6 @@ TEST(HttpUtility, ValidateStreamErrorsWithHcm) {
   EXPECT_TRUE(Envoy::Http2::Utility::initializeAndValidateOptions(http2_options, true, hcm_value)
                   .override_stream_error_on_invalid_http_message()
                   .value());
-
-  {
-    // With runtime flipped, override is ignored.
-    TestScopedRuntime scoped_runtime;
-    Runtime::LoaderSingleton::getExisting()->mergeValues(
-        {{"envoy.reloadable_features.hcm_stream_error_on_invalid_message", "false"}});
-    EXPECT_TRUE(Envoy::Http2::Utility::initializeAndValidateOptions(http2_options, true, hcm_value)
-                    .override_stream_error_on_invalid_http_message()
-                    .value());
-  }
 }
 
 TEST(HttpUtility, ValidateStreamErrorConfigurationForHttp1) {

test/integration/integration_test.cc

@@ -964,9 +964,9 @@ TEST_P(IntegrationTest, PipelineWithTrailers) {
 // an inline sendLocalReply to make sure the "kick" works under the call stack
 // of dispatch as well as when a response is proxied from upstream.
 TEST_P(IntegrationTest, PipelineInline) {
-  // When deprecating this flag, set hcm.mutable_stream_error_on_invalid_http_message true.
-  config_helper_.addRuntimeOverride("envoy.reloadable_features.hcm_stream_error_on_invalid_message",
-                                    "false");
+  config_helper_.addConfigModifier(
+      [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
+             hcm) { hcm.mutable_stream_error_on_invalid_http_message()->set_value(true); });
 
   autonomous_upstream_ = true;
   initialize();