Skip to content

Commit

Permalink
Ext_proc: support RouteCacheAction in the filter config (#33830)
Browse files Browse the repository at this point in the history 
This PR is a follow up change for: #33582.

It is to support RouteCacheAction to force clearing the route cache in ext_proc filter even side stream server does not send the clear_route_cache in the response.

Signed-off-by: Yanjun Xiang <yanjunxiang@google.com>
  • Loading branch information
@yanjunxiang-google
yanjunxiang-google committed May 9, 2024
1 parent 1d180ac commit a7d3789
Show file tree
Hide file tree
Showing 5 changed files with 257 additions and 20 deletions.
1 change: 0 additions & 1 deletion api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto
View file
Expand Up @@ -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>`.
Expand Down
5 changes: 5 additions & 0 deletions changelogs/current.yaml
View file
Expand Up @@ -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
Expand Down
25 changes: 20 additions & 5 deletions source/extensions/filters/http/ext_proc/ext_proc.h
View file
Expand Up @@ -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()),
Expand All @@ -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_; }

Expand All @@ -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 {
Expand Down Expand Up @@ -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_;

Expand Down
46 changes: 32 additions & 14 deletions source/extensions/filters/http/ext_proc/processor_state.cc
View file
Expand Up @@ -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) {
Expand Down
200 changes: 200 additions & 0 deletions test/extensions/filters/http/ext_proc/filter_test.cc
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit a7d3789

Please sign in to comment.