Ext_proc: support RouteCacheAction in the filter config

api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto

@@ -250,7 +250,6 @@ message ExternalProcessor {
   bool disable_clear_route_cache = 11
       [(udpa.annotations.field_migrate).oneof_promotion = "clear_route_cache_type"];
 
-  // [#not-implemented-hide:]
   // Specifies the action to be taken when an external processor response is
   // received in response to request headers. It is recommended to set this field than set
   // :ref:`disable_clear_route_cache <envoy_v3_api_field_extensions.filters.http.ext_proc.v3.ExternalProcessor.disable_clear_route_cache>`.

changelogs/current.yaml

@@ -7,6 +7,11 @@ behavior_changes:
     Changes the behavior of the ``SlotImpl`` class destructor. With this change the destructor can be called on any thread.
     This behavior can be reverted by setting the runtime flag ``envoy.reloadable_features.allow_slot_destroy_on_worker_threads``
     to false.
+- area: ext_proc
+  change: |
+    Adding support for
+    :ref:`route_cache_action <envoy_v3_api_field_extensions.filters.http.ext_proc.v3.ExternalProcessor.route_cache_action>`.
+    It specifies the route action to be taken when an external processor response is received in response to request headers.
 - area: http2
   change: |
     Changes the default value of ``envoy.reloadable_features.http2_use_oghttp2`` to true. This changes the codec used for HTTP/2

source/extensions/filters/http/ext_proc/ext_proc.h

@@ -154,8 +154,8 @@ class FilterConfig {
                Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr builder,
                Server::Configuration::CommonFactoryContext& context)
       : failure_mode_allow_(config.failure_mode_allow()),
-        disable_clear_route_cache_(config.disable_clear_route_cache()),
-        message_timeout_(message_timeout), max_message_timeout_ms_(max_message_timeout_ms),
+        route_cache_action_(config.route_cache_action()), message_timeout_(message_timeout),
+        max_message_timeout_ms_(max_message_timeout_ms),
         stats_(generateStats(stats_prefix, config.stat_prefix(), scope)),
         processing_mode_(config.processing_mode()),
         mutation_checker_(config.mutation_rules(), context.regexEngine()),
@@ -177,7 +177,18 @@ class FilterConfig {
             config.metadata_options().receiving_namespaces().untyped().end()),
         expression_manager_(builder, context.localInfo(), config.request_attributes(),
                             config.response_attributes()),
-        immediate_mutation_checker_(context.regexEngine()) {}
+        immediate_mutation_checker_(context.regexEngine()) {
+    if (config.disable_clear_route_cache() &&
+        (route_cache_action_ !=
+         envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor::DEFAULT)) {
+      ExceptionUtil::throwEnvoyException("disable_clear_route_cache and route_cache_action can not "
+                                         "be set to none-default at the same time.");
+    }
+    if (config.disable_clear_route_cache()) {
+      route_cache_action_ =
+          envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor::RETAIN;
+    }
+  }
 
   bool failureModeAllow() const { return failure_mode_allow_; }
 
@@ -198,7 +209,10 @@ class FilterConfig {
     return mutation_checker_;
   }
 
-  bool disableClearRouteCache() const { return disable_clear_route_cache_; }
+  envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor::RouteCacheAction
+  routeCacheAction() const {
+    return route_cache_action_;
+  }
 
   const std::vector<Matchers::StringMatcherPtr>& allowedHeaders() const { return allowed_headers_; }
   const std::vector<Matchers::StringMatcherPtr>& disallowedHeaders() const {
@@ -234,7 +248,8 @@ class FilterConfig {
     return {ALL_EXT_PROC_FILTER_STATS(POOL_COUNTER_PREFIX(scope, final_prefix))};
   }
   const bool failure_mode_allow_;
-  const bool disable_clear_route_cache_;
+  envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor::RouteCacheAction
+      route_cache_action_;
   const std::chrono::milliseconds message_timeout_;
   const uint32_t max_message_timeout_ms_;
 

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

@@ -420,27 +420,45 @@ void DecodingProcessorState::clearWatermark() {
 }
 
 void DecodingProcessorState::clearRouteCache(const CommonResponse& common_response) {
-  if (!common_response.clear_route_cache()) {
-    return;
-  }
+  bool response_clear_route_cache = common_response.clear_route_cache();
   if (filter_.config().isUpstream()) {
-    filter_.stats().clear_route_cache_upstream_ignored_.inc();
-    ENVOY_LOG(debug, "NOT clearing route cache. The filter is in upstream filter chain.");
+    if (response_clear_route_cache) {
+      filter_.stats().clear_route_cache_upstream_ignored_.inc();
+      ENVOY_LOG(debug, "NOT clearing route cache. The filter is in upstream filter chain.");
+    }
     return;
   }
-  // Only clear the route cache if there is a mutation to the header and clearing is allowed.
-  if (filter_.config().disableClearRouteCache()) {
-    filter_.stats().clear_route_cache_disabled_.inc();
-    ENVOY_LOG(debug, "NOT clearing route cache, it is disabled in the config");
+
+  if (!common_response.has_header_mutation()) {
+    if (response_clear_route_cache) {
+      filter_.stats().clear_route_cache_ignored_.inc();
+      ENVOY_LOG(debug, "NOT clearing route cache. No header mutation in the response");
+    }
     return;
   }
-  if (common_response.has_header_mutation()) {
-    ENVOY_LOG(debug, "clearing route cache");
+
+  // Filter is in downstream and response has header mutation.
+  switch (filter_.config().routeCacheAction()) {
+    PANIC_ON_PROTO_ENUM_SENTINEL_VALUES;
+  case envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor::DEFAULT:
+    if (response_clear_route_cache) {
+      ENVOY_LOG(debug, "Clearing route cache due to the filter RouterCacheAction is configured "
+                       "with DEFAULT and response has clear_route_cache set.");
+      decoder_callbacks_->downstreamCallbacks()->clearRouteCache();
+    }
+    break;
+  case envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor::CLEAR:
+    ENVOY_LOG(debug,
+              "Clearing route cache due to the filter RouterCacheAction is configured with CLEAR");
     decoder_callbacks_->downstreamCallbacks()->clearRouteCache();
-    return;
+    break;
+  case envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor::RETAIN:
+    if (response_clear_route_cache) {
+      filter_.stats().clear_route_cache_disabled_.inc();
+      ENVOY_LOG(debug, "NOT clearing route cache, it is disabled by the filter config");
+    }
+    break;
   }
-  filter_.stats().clear_route_cache_ignored_.inc();
-  ENVOY_LOG(debug, "NOT clearing route cache, no header mutations detected");
 }
 
 void EncodingProcessorState::setProcessingModeInternal(const ProcessingMode& mode) {

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

@@ -2714,6 +2714,7 @@ TEST_F(HttpFilterTest, ClearRouteCacheHeaderMutation) {
 
   EXPECT_EQ(config_->stats().clear_route_cache_disabled_.value(), 0);
   EXPECT_EQ(config_->stats().clear_route_cache_ignored_.value(), 0);
+  EXPECT_EQ(config_->stats().clear_route_cache_upstream_ignored_.value(), 0);
   EXPECT_EQ(config_->stats().streams_started_.value(), 1);
   EXPECT_EQ(config_->stats().stream_msgs_sent_.value(), 3);
   EXPECT_EQ(config_->stats().stream_msgs_received_.value(), 3);
@@ -2803,12 +2804,211 @@ TEST_F(HttpFilterTest, ClearRouteCacheUnchanged) {
 
   EXPECT_EQ(config_->stats().clear_route_cache_ignored_.value(), 1);
   EXPECT_EQ(config_->stats().clear_route_cache_disabled_.value(), 0);
+  EXPECT_EQ(config_->stats().clear_route_cache_upstream_ignored_.value(), 0);
   EXPECT_EQ(config_->stats().streams_started_.value(), 1);
   EXPECT_EQ(config_->stats().stream_msgs_sent_.value(), 3);
   EXPECT_EQ(config_->stats().stream_msgs_received_.value(), 3);
   EXPECT_EQ(config_->stats().streams_closed_.value(), 1);
 }
 
+// Using the default configuration, "clear_route_cache" flag not set. No header mutation.
+TEST_F(HttpFilterTest, ClearRouteCacheUnchangedNoClearFlag) {
+  initialize(R"EOF(
+  grpc_service:
+    envoy_grpc:
+      cluster_name: "ext_proc_server"
+  processing_mode:
+    response_body_mode: "BUFFERED"
+  )EOF");
+
+  // Do not call ClearRouteCache() for inbound traffic without header mutation.
+  EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, true));
+  processRequestHeaders(false, absl::nullopt);
+
+  EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->encodeHeaders(response_headers_, false));
+  processResponseHeaders(true, absl::nullopt);
+
+  Buffer::OwnedImpl resp_data("foo");
+  Buffer::OwnedImpl buffered_response_data;
+  setUpEncodingBuffering(buffered_response_data);
+
+  // There is no ClearRouteCache() call for outbound traffic regardless.
+  EXPECT_EQ(FilterDataStatus::StopIterationNoBuffer, filter_->encodeData(resp_data, true));
+  processResponseBody([](const HttpBody&, ProcessingResponse&, BodyResponse& resp) {
+    resp.mutable_response()->set_clear_route_cache(true);
+  });
+
+  filter_->onDestroy();
+
+  EXPECT_EQ(config_->stats().clear_route_cache_ignored_.value(), 0);
+  EXPECT_EQ(config_->stats().clear_route_cache_disabled_.value(), 0);
+  EXPECT_EQ(config_->stats().clear_route_cache_upstream_ignored_.value(), 0);
+}
+
+// Verify that the "disable_route_cache_clearing" and "route_cache_action"  setting
+// can not be set at the same time.
+TEST_F(HttpFilterTest, ClearRouteCacheDisableRouteCacheActionBothSet) {
+  std::string yaml = R"EOF(
+  grpc_service:
+    envoy_grpc:
+      cluster_name: "ext_proc_server"
+  processing_mode:
+    response_body_mode: "BUFFERED"
+  disable_clear_route_cache: true
+  route_cache_action: CLEAR
+  )EOF";
+
+  envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor proto_config{};
+  TestUtility::loadFromYaml(yaml, proto_config);
+  EXPECT_THROW_WITH_MESSAGE(
+      {
+        auto config = std::make_shared<FilterConfig>(
+            proto_config, 200ms, 10000, *stats_store_.rootScope(), "", false,
+            std::make_shared<Envoy::Extensions::Filters::Common::Expr::BuilderInstance>(
+                Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr)),
+            factory_context_);
+      },
+      EnvoyException,
+      "disable_clear_route_cache and route_cache_action can not be set to none-default at the same "
+      "time.");
+}
+
+// Verify that with header mutation in response, setting route_cache_action to CLEAR
+// will clear route cache even the response does not set clear_route_cache.
+TEST_F(HttpFilterTest, FilterRouteCacheActionSetToClearHeaderMutation) {
+  initialize(R"EOF(
+  grpc_service:
+    envoy_grpc:
+      cluster_name: "ext_proc_server"
+  route_cache_action: CLEAR
+  )EOF");
+
+  EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, true));
+  EXPECT_CALL(decoder_callbacks_.downstream_callbacks_, clearRouteCache());
+  processRequestHeaders(false, [](const HttpHeaders&, ProcessingResponse&, HeadersResponse& resp) {
+    auto* resp_headers_mut = resp.mutable_response()->mutable_header_mutation();
+    auto* resp_add = resp_headers_mut->add_set_headers();
+    resp_add->mutable_header()->set_key("x-new-header");
+    resp_add->mutable_header()->set_value("new");
+  });
+
+  EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->encodeHeaders(response_headers_, false));
+  processResponseHeaders(true, absl::nullopt);
+
+  filter_->onDestroy();
+
+  EXPECT_EQ(config_->stats().clear_route_cache_disabled_.value(), 0);
+  EXPECT_EQ(config_->stats().clear_route_cache_ignored_.value(), 0);
+  EXPECT_EQ(config_->stats().clear_route_cache_upstream_ignored_.value(), 0);
+  EXPECT_EQ(config_->stats().streams_started_.value(), 1);
+  EXPECT_EQ(config_->stats().stream_msgs_sent_.value(), 2);
+  EXPECT_EQ(config_->stats().stream_msgs_received_.value(), 2);
+  EXPECT_EQ(config_->stats().streams_closed_.value(), 1);
+}
+
+// Verify that without header mutation in response, setting route_cache_action to CLEAR
+// and set the clear_route_cache flag to true in the response will not clear route cache.
+TEST_F(HttpFilterTest, FilterRouteCacheActionSetToClearNoHeaderMutation) {
+  initialize(R"EOF(
+  grpc_service:
+    envoy_grpc:
+      cluster_name: "ext_proc_server"
+  route_cache_action: CLEAR
+  )EOF");
+
+  EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, true));
+  processRequestHeaders(false, [](const HttpHeaders&, ProcessingResponse&, HeadersResponse& resp) {
+    resp.mutable_response()->set_clear_route_cache(true);
+  });
+
+  EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->encodeHeaders(response_headers_, false));
+  processResponseHeaders(true, absl::nullopt);
+
+  filter_->onDestroy();
+
+  EXPECT_EQ(config_->stats().clear_route_cache_disabled_.value(), 0);
+  EXPECT_EQ(config_->stats().clear_route_cache_ignored_.value(), 1);
+}
+
+// Verify that without header mutation in response, setting route_cache_action to CLEAR and not
+// set the clear_route_cache flag to true in the response will not clear route cache.
+TEST_F(HttpFilterTest, FilterRouteCacheActionSetToClearResponseNotSetNoHeaderMutation) {
+  initialize(R"EOF(
+  grpc_service:
+    envoy_grpc:
+      cluster_name: "ext_proc_server"
+  route_cache_action: CLEAR
+  )EOF");
+
+  EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, true));
+  // There is no clear_route_cache set in the response. clear_route_cache_ignored_ is zero in this
+  // case.
+  processRequestHeaders(false, absl::nullopt);
+
+  EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->encodeHeaders(response_headers_, false));
+  processResponseHeaders(true, absl::nullopt);
+
+  filter_->onDestroy();
+
+  EXPECT_EQ(config_->stats().clear_route_cache_disabled_.value(), 0);
+  EXPECT_EQ(config_->stats().clear_route_cache_ignored_.value(), 0);
+}
+
+// Verify that setting route_cache_action to RETAIN will not clear route cache.
+TEST_F(HttpFilterTest, FilterRouteCacheActionSetToRetainWithHeaderMutation) {
+  initialize(R"EOF(
+  grpc_service:
+    envoy_grpc:
+      cluster_name: "ext_proc_server"
+  route_cache_action: RETAIN
+  )EOF");
+
+  EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, true));
+  processRequestHeaders(false, [](const HttpHeaders&, ProcessingResponse&, HeadersResponse& resp) {
+    auto* resp_headers_mut = resp.mutable_response()->mutable_header_mutation();
+    auto* resp_add = resp_headers_mut->add_set_headers();
+    resp_add->mutable_header()->set_key("x-new-header");
+    resp_add->mutable_header()->set_value("new");
+    resp.mutable_response()->set_clear_route_cache(true);
+  });
+
+  EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->encodeHeaders(response_headers_, false));
+  processResponseHeaders(true, absl::nullopt);
+
+  filter_->onDestroy();
+
+  EXPECT_EQ(config_->stats().clear_route_cache_disabled_.value(), 1);
+  EXPECT_EQ(config_->stats().clear_route_cache_ignored_.value(), 0);
+}
+
+// Verify that setting route_cache_action to RETAIN, response clear_route_cache flag not set,
+// will not clear route cache.
+TEST_F(HttpFilterTest, FilterRouteCacheActionSetToRetainResponseNotWithHeaderMutation) {
+  initialize(R"EOF(
+  grpc_service:
+    envoy_grpc:
+      cluster_name: "ext_proc_server"
+  route_cache_action: RETAIN
+  )EOF");
+
+  EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, true));
+  processRequestHeaders(false, [](const HttpHeaders&, ProcessingResponse&, HeadersResponse& resp) {
+    auto* resp_headers_mut = resp.mutable_response()->mutable_header_mutation();
+    auto* resp_add = resp_headers_mut->add_set_headers();
+    resp_add->mutable_header()->set_key("x-new-header");
+    resp_add->mutable_header()->set_value("new");
+  });
+
+  EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->encodeHeaders(response_headers_, false));
+  processResponseHeaders(true, absl::nullopt);
+
+  filter_->onDestroy();
+
+  // This counter will not increase as clear_route_cache flag in the response is not set.
+  EXPECT_EQ(config_->stats().clear_route_cache_disabled_.value(), 0);
+  EXPECT_EQ(config_->stats().clear_route_cache_ignored_.value(), 0);
+}
+
 // Using the default configuration, turn a GET into a POST.
 TEST_F(HttpFilterTest, ReplaceRequest) {
   initialize(R"EOF(