Remove runtime flag envoy_reloadable_features_immediate_response_use_…

…filter_mutation_rule (envoyproxy#35381)

Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: inline
Fixes: envoyproxy#31977

Description:

envoyproxy#27925 (Applying same ext_proc
filter checker rule for immediate response.) introduced a feature
guarded by:
envoy_reloadable_features_immediate_response_use_filter_mutation_rule.
It has been 6 months since the code has been exercised by default, so
it's time to remove the old code path and deprecate this runtime flag.

---------

Signed-off-by: Yanjun Xiang <[email protected]>

changelogs/current.yaml

@@ -31,6 +31,10 @@ removed_config_or_runtime:
 - area: upstream
   change: |
     Removed runtime flag ``envoy.reloadable_features.avoid_zombie_streams`` and legacy code paths.
+- area: ext_proc
+  change: |
+    Removed runtime flag ``envoy_reloadable_features_immediate_response_use_filter_mutation_rule`` and legacy code
+    path.
 - area: ext_proc
   change: |
     Removed runtime flag ``envoy_reloadable_features_send_header_raw_value`` and legacy code path.

source/common/runtime/runtime_features.cc

@@ -62,7 +62,6 @@ RUNTIME_GUARD(envoy_reloadable_features_http_filter_avoid_reentrant_local_reply)
 // Delay deprecation and decommission until UHV is enabled.
 RUNTIME_GUARD(envoy_reloadable_features_http_reject_path_with_fragment);
 RUNTIME_GUARD(envoy_reloadable_features_http_route_connect_proxy_by_default);
-RUNTIME_GUARD(envoy_reloadable_features_immediate_response_use_filter_mutation_rule);
 RUNTIME_GUARD(envoy_reloadable_features_jwt_authn_remove_jwt_from_query_params);
 RUNTIME_GUARD(envoy_reloadable_features_jwt_authn_validate_uri);
 RUNTIME_GUARD(envoy_reloadable_features_lua_flow_control_while_http_call);

source/extensions/filters/http/ext_proc/ext_proc.cc

@@ -1251,17 +1251,9 @@ void Filter::sendImmediateResponse(const ImmediateResponse& response) {
           : absl::nullopt;
   const auto mutate_headers = [this, &response](Http::ResponseHeaderMap& headers) {
     if (response.has_headers()) {
-      absl::Status mut_status;
-      if (Runtime::runtimeFeatureEnabled(
-              "envoy.reloadable_features.immediate_response_use_filter_mutation_rule")) {
-        mut_status = MutationUtils::applyHeaderMutations(response.headers(), headers, false,
-                                                         config().mutationChecker(),
-                                                         stats_.rejected_header_mutations_);
-      } else {
-        mut_status = MutationUtils::applyHeaderMutations(
-            response.headers(), headers, false, config().immediateMutationChecker().checker(),
-            stats_.rejected_header_mutations_);
-      }
+      const absl::Status mut_status = MutationUtils::applyHeaderMutations(
+          response.headers(), headers, false, config().mutationChecker(),
+          stats_.rejected_header_mutations_);
       if (!mut_status.ok()) {
         ENVOY_LOG_EVERY_POW_2(error, "Immediate response mutations failed with {}",
                               mut_status.message());

test/extensions/filters/http/ext_proc/BUILD

@@ -23,7 +23,6 @@ envoy_extension_cc_test(
     deps = [
         "//source/extensions/filters/http/ext_proc:config",
         "//test/mocks/server:factory_context_mocks",
-        "//test/test_common:test_runtime_lib",
     ],
 )
 

test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc

@@ -97,10 +97,6 @@ class ExtProcIntegrationTest : public HttpIntegrationTest,
         [&total_cluster_endpoints](const auto& item) { total_cluster_endpoints += item.second; });
     ASSERT_EQ(total_cluster_endpoints, grpc_upstream_count_);
 
-    scoped_runtime_.mergeValues(
-        {{"envoy.reloadable_features.immediate_response_use_filter_mutation_rule",
-          filter_mutation_rule_}});
-
     config_helper_.addConfigModifier([this, cluster_endpoints, config_option](
                                          envoy::config::bootstrap::v3::Bootstrap& bootstrap) {
       // Ensure "HTTP2 with no prior knowledge." Necessary for gRPC and for headers
@@ -680,7 +676,6 @@ class ExtProcIntegrationTest : public HttpIntegrationTest,
   FakeHttpConnectionPtr processor_connection_;
   FakeStreamPtr processor_stream_;
   TestScopedRuntime scoped_runtime_;
-  std::string filter_mutation_rule_{"false"};
   // Number of grpc upstreams in the test.
   int grpc_upstream_count_ = 2;
 };
@@ -2150,7 +2145,6 @@ TEST_P(ExtProcIntegrationTest, GetAndRespondImmediatelyWithBadStatus) {
 // Test the filter using an ext_proc server that responds to the request_header message
 // by sending back an immediate_response message with system header mutation.
 TEST_P(ExtProcIntegrationTest, GetAndRespondImmediatelyWithSystemHeaderMutation) {
-  filter_mutation_rule_ = "true";
   proto_config_.mutable_mutation_rules()->mutable_disallow_is_error()->set_value(true);
   // Disallow system header in the mutation rule config.
   proto_config_.mutable_mutation_rules()->mutable_disallow_system()->set_value(true);
@@ -2172,7 +2166,6 @@ TEST_P(ExtProcIntegrationTest, GetAndRespondImmediatelyWithSystemHeaderMutation)
 // Test the filter using an ext_proc server that responds to the request_header message
 // by sending back an immediate_response message with x-envoy header mutation.
 TEST_P(ExtProcIntegrationTest, GetAndRespondImmediatelyWithEnvoyHeaderMutation) {
-  filter_mutation_rule_ = "true";
   proto_config_.mutable_mutation_rules()->mutable_disallow_is_error()->set_value(true);
   proto_config_.mutable_mutation_rules()->mutable_allow_envoy()->set_value(false);
   initializeConfig();
@@ -2190,7 +2183,6 @@ TEST_P(ExtProcIntegrationTest, GetAndRespondImmediatelyWithEnvoyHeaderMutation)
 }
 
 TEST_P(ExtProcIntegrationTest, GetAndImmediateRespondMutationAllowEnvoy) {
-  filter_mutation_rule_ = "true";
   proto_config_.mutable_mutation_rules()->mutable_allow_envoy()->set_value(true);
   proto_config_.mutable_mutation_rules()->mutable_allow_all_routing()->set_value(true);
 
@@ -2212,28 +2204,6 @@ TEST_P(ExtProcIntegrationTest, GetAndImmediateRespondMutationAllowEnvoy) {
   EXPECT_THAT(response->headers(), SingleHeaderValueIs("x-envoy-foo", "bar"));
 }
 
-// Test the filter using an ext_proc server that responds to the request_header message
-// by sending back an immediate_response message with x-envoy header mutation.
-// The deprecated default checker allows x-envoy headers to be mutated and should
-// override config-level checkers if the runtime guard is disabled.
-TEST_P(ExtProcIntegrationTest, GetAndRespondImmediatelyWithDefaultHeaderMutationChecker) {
-  // this is default, but setting explicitly for test clarity
-  filter_mutation_rule_ = "false";
-  proto_config_.mutable_mutation_rules()->mutable_allow_envoy()->set_value(false);
-  initializeConfig();
-  HttpIntegrationTest::initialize();
-  auto response = sendDownstreamRequest(absl::nullopt);
-  processAndRespondImmediately(*grpc_upstreams_[0], true, [](ImmediateResponse& immediate) {
-    immediate.mutable_status()->set_code(envoy::type::v3::StatusCode::Unauthorized);
-    auto* hdr = immediate.mutable_headers()->add_set_headers();
-    // Adding x-envoy header is allowed since default overrides config.
-    hdr->mutable_header()->set_key("x-envoy-foo");
-    hdr->mutable_header()->set_raw_value("bar");
-  });
-  verifyDownstreamResponse(*response, 401);
-  EXPECT_FALSE(response->headers().get(LowerCaseString("x-envoy-foo")).empty());
-}
-
 // Test the filter with request body buffering enabled using
 // an ext_proc server that responds to the request_body message
 // by modifying a header that should cause an error.