Change envoy_reloadable_features_http_inspector_use_balsa_parser to default true

changelogs/current.yaml

@@ -686,5 +686,10 @@ new_features:
     RSA public key exchange on behalf of the client. Added a new
     :ref:`downstream_ssl <envoy_v3_api_field_extensions.filters.network.mysql_proxy.v3.MySQLProxy.downstream_ssl>`
     config option with ``DISABLE``, ``REQUIRE``, and ``ALLOW`` modes.
+- area: http_inspector
+  change: |
+    Enabled Balsa parser for HTTP inspector by default. This behavior can be temporarily
+    reverted by setting the runtime guard ``envoy.reloadable_features.http_inspector_use_balsa_parser``
+    to ``false``. This runtime guard will be removed in a future release of Envoy.
 
 deprecated:

source/common/runtime/runtime_features.cc

@@ -65,6 +65,7 @@ RUNTIME_GUARD(envoy_reloadable_features_health_check_after_cluster_warming);
 RUNTIME_GUARD(envoy_reloadable_features_http1_close_connection_on_zombie_stream_complete);
 RUNTIME_GUARD(envoy_reloadable_features_http2_discard_host_header);
 RUNTIME_GUARD(envoy_reloadable_features_http_async_client_retry_respect_buffer_limits);
+RUNTIME_GUARD(envoy_reloadable_features_http_inspector_use_balsa_parser);
 // Delay deprecation and decommission until UHV is enabled.
 RUNTIME_GUARD(envoy_reloadable_features_http_reject_path_with_fragment);
 RUNTIME_GUARD(envoy_reloadable_features_map_http_stream_reset_to_tcp_rst);
@@ -114,13 +115,11 @@ RUNTIME_GUARD(envoy_reloadable_features_websocket_allow_4xx_5xx_through_filter_c
 RUNTIME_GUARD(envoy_reloadable_features_websocket_enable_timeout_on_upgrade_response);
 RUNTIME_GUARD(envoy_reloadable_features_xds_failover_to_primary_enabled);
 RUNTIME_GUARD(envoy_reloadable_features_xds_legacy_delta_skip_subsequent_node);
-
 RUNTIME_GUARD(envoy_restart_features_move_locality_schedulers_to_lb);
 RUNTIME_GUARD(envoy_restart_features_raise_file_limits);
 RUNTIME_GUARD(envoy_restart_features_use_eds_cache_for_ads);
 RUNTIME_GUARD(envoy_restart_features_validate_http3_pseudo_headers);
 RUNTIME_GUARD(envoy_restart_features_worker_threads_watchdog_fix);
-
 // Begin false flags. Most of them should come with a TODO to flip true.
 
 // Sentinel and test flag.
@@ -158,8 +157,6 @@ FALSE_RUNTIME_GUARD(envoy_restart_features_xds_failover_support);
 FALSE_RUNTIME_GUARD(envoy_reloadable_features_dns_cache_set_ip_version_to_remove);
 // TODO(fredyw): evaluate and either make this a config knob or remove.
 FALSE_RUNTIME_GUARD(envoy_reloadable_features_quic_no_tcp_delay);
-// Adding runtime flag to use balsa_parser for http_inspector.
-FALSE_RUNTIME_GUARD(envoy_reloadable_features_http_inspector_use_balsa_parser);
 // TODO(danzh) re-enable it when the issue of preferring TCP over v6 rather than QUIC over v4 is
 // fixed.
 FALSE_RUNTIME_GUARD(envoy_reloadable_features_http3_happy_eyeballs);

test/extensions/filters/listener/http_inspector/BUILD

@@ -90,6 +90,7 @@ envoy_extension_cc_test(
         "//source/extensions/transport_sockets/raw_buffer:config",
         "//test/integration:base_integration_test_lib",
         "//test/integration:common_extensions_lib",
+        "//test/test_common:test_runtime_lib",
         "//test/test_common:utility_lib",
         "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",
         "@envoy_api//envoy/extensions/access_loggers/file/v3:pkg_cc_proto",

test/extensions/filters/listener/http_inspector/http_inspector_integration_test.cc

@@ -9,6 +9,7 @@
 #include "test/integration/fake_upstream.h"
 #include "test/integration/server.h"
 #include "test/test_common/printers.h"
+#include "test/test_common/test_runtime.h"
 #include "test/test_common/utility.h"
 
 #include "gtest/gtest.h"
@@ -23,30 +24,62 @@ void insertHttpInspectorConfigModifier(envoy::config::bootstrap::v3::Bootstrap&
   ppv_filter->set_name("http_inspector");
   ppv_filter->mutable_typed_config()->PackFrom(http_inspector);
 }
+
+std::string testParamToString(
+    const ::testing::TestParamInfo<std::tuple<Network::Address::IpVersion, Http1ParserImpl>>&
+        info) {
+  ::testing::TestParamInfo<Network::Address::IpVersion> ip_info(std::get<0>(info.param),
+                                                                info.index);
+  return TestUtility::ipTestParamsToString(ip_info) + "_" +
+         TestUtility::http1ParserImplToString(std::get<1>(info.param));
+}
+
 } // namespace
 
-class HttpInspectorTcpIntegrationTest : public testing::TestWithParam<Network::Address::IpVersion>,
-                                        public BaseIntegrationTest {
+class HttpInspectorTcpIntegrationTest
+    : public testing::TestWithParam<std::tuple<Network::Address::IpVersion, Http1ParserImpl>>,
+      public BaseIntegrationTest {
 public:
   HttpInspectorTcpIntegrationTest()
-      : BaseIntegrationTest(GetParam(), ConfigHelper::tcpProxyConfig()) {
+      : BaseIntegrationTest(std::get<0>(GetParam()), ConfigHelper::tcpProxyConfig()),
+        parser_impl_(std::get<1>(GetParam())) {
     config_helper_.addConfigModifier(insertHttpInspectorConfigModifier);
     config_helper_.renameListener("tcp_proxy");
+
+    if (parser_impl_ == Http1ParserImpl::BalsaParser) {
+      scoped_runtime_.mergeValues(
+          {{"envoy.reloadable_features.http_inspector_use_balsa_parser", "true"}});
+    } else {
+      scoped_runtime_.mergeValues(
+          {{"envoy.reloadable_features.http_inspector_use_balsa_parser", "false"}});
+    }
   }
+
+  const Http1ParserImpl parser_impl_;
+  TestScopedRuntime scoped_runtime_;
 };
 
-INSTANTIATE_TEST_SUITE_P(IpVersions, HttpInspectorTcpIntegrationTest,
-                         testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
-                         TestUtility::ipTestParamsToString);
+INSTANTIATE_TEST_SUITE_P(
+    ParsersAndIp, HttpInspectorTcpIntegrationTest,
+    testing::Combine(testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
+                     testing::Values(Http1ParserImpl::HttpParser, Http1ParserImpl::BalsaParser)),
+    testParamToString);
 
 TEST_P(HttpInspectorTcpIntegrationTest, DetectNoHttp) {
   initialize();
 
+  std::string data = "hello";
+  size_t expected_bytes = 5;
+  if (parser_impl_ == Http1ParserImpl::BalsaParser) {
+    data = "hello\r\n";
+    expected_bytes = 7;
+  }
+
   IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy"));
-  ASSERT_TRUE(tcp_client->write("hello", false));
+  ASSERT_TRUE(tcp_client->write(data, false));
   FakeRawConnectionPtr fake_upstream_connection;
   ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection));
-  ASSERT_TRUE(fake_upstream_connection->waitForData(5));
+  ASSERT_TRUE(fake_upstream_connection->waitForData(expected_bytes));
   ASSERT_TRUE(fake_upstream_connection->close());
   tcp_client->close();
   ASSERT_TRUE(fake_upstream_connection->waitForDisconnect());