diff --git a/api/envoy/extensions/filters/http/cache/v3/cache.proto b/api/envoy/extensions/filters/http/cache/v3/cache.proto index d47691c9b8c6..6c124e10f90f 100644 --- a/api/envoy/extensions/filters/http/cache/v3/cache.proto +++ b/api/envoy/extensions/filters/http/cache/v3/cache.proto @@ -6,10 +6,10 @@ import "envoy/config/route/v3/route_components.proto"; import "envoy/type/matcher/v3/string.proto"; import "google/protobuf/any.proto"; +import "google/protobuf/wrappers.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; -import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.extensions.filters.http.cache.v3"; option java_outer_classname = "CacheProto"; @@ -20,6 +20,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // [#protodoc-title: HTTP Cache Filter] // [#extension: envoy.filters.http.cache] +// [#next-free-field: 6] message CacheConfig { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.http.cache.v2alpha.CacheConfig"; @@ -49,9 +50,17 @@ message CacheConfig { repeated config.route.v3.QueryParameterMatcher query_parameters_excluded = 4; } - // Config specific to the cache storage implementation. + // Config specific to the cache storage implementation. Required unless ``disabled`` + // is true. // [#extension-category: envoy.http.cache] - google.protobuf.Any typed_config = 1 [(validate.rules).any = {required: true}]; + google.protobuf.Any typed_config = 1; + + // When true, the cache filter is a no-op filter. + // + // Possible use-cases for this include: + // - Turning a filter on and off with :ref:`ECDS `. + // [#comment: once route-specific overrides are implemented, they are the more likely use-case.] + google.protobuf.BoolValue disabled = 5; // List of matching rules that defines allowed ``Vary`` headers. // diff --git a/source/extensions/filters/http/cache/cache_filter.cc b/source/extensions/filters/http/cache/cache_filter.cc index 704c969f4e35..17e221308ab3 100644 --- a/source/extensions/filters/http/cache/cache_filter.cc +++ b/source/extensions/filters/http/cache/cache_filter.cc @@ -33,7 +33,7 @@ using CacheResponseCodeDetails = ConstSingleton; CacheFilter::CacheFilter(const envoy::extensions::filters::http::cache::v3::CacheConfig& config, const std::string&, Stats::Scope&, TimeSource& time_source, - HttpCache& http_cache) + OptRef http_cache) : time_source_(time_source), cache_(http_cache), vary_allow_list_(config.allowed_vary_headers()) {} @@ -58,6 +58,10 @@ void CacheFilter::onStreamComplete() { Http::FilterHeadersStatus CacheFilter::decodeHeaders(Http::RequestHeaderMap& headers, bool end_stream) { + if (!cache_) { + filter_state_ = FilterState::NotServingFromCache; + return Http::FilterHeadersStatus::Continue; + } ENVOY_STREAM_LOG(debug, "CacheFilter::decodeHeaders: {}", *decoder_callbacks_, headers); if (!end_stream) { ENVOY_STREAM_LOG( @@ -79,7 +83,7 @@ Http::FilterHeadersStatus CacheFilter::decodeHeaders(Http::RequestHeaderMap& hea LookupRequest lookup_request(headers, time_source_.systemTime(), vary_allow_list_); request_allows_inserts_ = !lookup_request.requestCacheControl().no_store_; is_head_request_ = headers.getMethodValue() == Http::Headers::get().MethodValues.Head; - lookup_ = cache_.makeLookupContext(std::move(lookup_request), *decoder_callbacks_); + lookup_ = cache_->makeLookupContext(std::move(lookup_request), *decoder_callbacks_); ASSERT(lookup_); getHeaders(headers); @@ -130,7 +134,7 @@ Http::FilterHeadersStatus CacheFilter::encodeHeaders(Http::ResponseHeaderMap& he if (request_allows_inserts_ && !is_head_request_ && CacheabilityUtils::isCacheableResponse(headers, vary_allow_list_)) { ENVOY_STREAM_LOG(debug, "CacheFilter::encodeHeaders inserting headers", *encoder_callbacks_); - insert_ = cache_.makeInsertContext(std::move(lookup_), *encoder_callbacks_); + insert_ = cache_->makeInsertContext(std::move(lookup_), *encoder_callbacks_); // Add metadata associated with the cached response. Right now this is only response_time; const ResponseMetadata metadata = {time_source_.systemTime()}; // TODO(capoferro): Note that there is currently no way to communicate back to the CacheFilter @@ -567,8 +571,8 @@ void CacheFilter::processSuccessfulValidation(Http::ResponseHeaderMap& response_ // TODO(yosrym93): else the cached entry should be deleted. // Update metadata associated with the cached response. Right now this is only response_time; const ResponseMetadata metadata = {time_source_.systemTime()}; - cache_.updateHeaders(*lookup_, response_headers, metadata, - [](bool updated ABSL_ATTRIBUTE_UNUSED) {}); + cache_->updateHeaders(*lookup_, response_headers, metadata, + [](bool updated ABSL_ATTRIBUTE_UNUSED) {}); insert_status_ = InsertStatus::HeaderUpdate; } diff --git a/source/extensions/filters/http/cache/cache_filter.h b/source/extensions/filters/http/cache/cache_filter.h index b6abcc927731..5b4977d1bbfc 100644 --- a/source/extensions/filters/http/cache/cache_filter.h +++ b/source/extensions/filters/http/cache/cache_filter.h @@ -54,7 +54,7 @@ class CacheFilter : public Http::PassThroughFilter, public: CacheFilter(const envoy::extensions::filters::http::cache::v3::CacheConfig& config, const std::string& stats_prefix, Stats::Scope& scope, TimeSource& time_source, - HttpCache& http_cache); + OptRef http_cache); // Http::StreamFilterBase void onDestroy() override; void onStreamComplete() override; @@ -127,7 +127,7 @@ class CacheFilter : public Http::PassThroughFilter, InsertStatus insertStatus() const; TimeSource& time_source_; - HttpCache& cache_; + OptRef cache_; LookupContextPtr lookup_; InsertContextPtr insert_; LookupResultPtr lookup_result_; diff --git a/source/extensions/filters/http/cache/config.cc b/source/extensions/filters/http/cache/config.cc index 47e7f370d2f0..bccdf4768b9c 100644 --- a/source/extensions/filters/http/cache/config.cc +++ b/source/extensions/filters/http/cache/config.cc @@ -10,20 +10,27 @@ namespace Cache { Http::FilterFactoryCb CacheFilterFactory::createFilterFactoryFromProtoTyped( const envoy::extensions::filters::http::cache::v3::CacheConfig& config, const std::string& stats_prefix, Server::Configuration::FactoryContext& context) { - const std::string type{TypeUtil::typeUrlToDescriptorFullName(config.typed_config().type_url())}; - HttpCacheFactory* const http_cache_factory = - Registry::FactoryRegistry::getFactoryByType(type); - if (http_cache_factory == nullptr) { - throw EnvoyException( - fmt::format("Didn't find a registered implementation for type: '{}'", type)); - } + std::shared_ptr cache; + if (!config.disabled().value()) { + if (!config.has_typed_config()) { + throw EnvoyException("at least one of typed_config or disabled must be set"); + } + const std::string type{TypeUtil::typeUrlToDescriptorFullName(config.typed_config().type_url())}; + HttpCacheFactory* const http_cache_factory = + Registry::FactoryRegistry::getFactoryByType(type); + if (http_cache_factory == nullptr) { + throw EnvoyException( + fmt::format("Didn't find a registered implementation for type: '{}'", type)); + } - auto cache = http_cache_factory->getCache(config, context); + cache = http_cache_factory->getCache(config, context); + } return [config, stats_prefix, &context, cache](Http::FilterChainFactoryCallbacks& callbacks) -> void { callbacks.addStreamFilter(std::make_shared(config, stats_prefix, context.scope(), - context.timeSource(), *cache)); + context.timeSource(), + cache ? *cache : OptRef{})); }; } diff --git a/test/extensions/filters/http/cache/cache_filter_test.cc b/test/extensions/filters/http/cache/cache_filter_test.cc index 0c0d88c6c2ef..ff23ccd7f76c 100644 --- a/test/extensions/filters/http/cache/cache_filter_test.cc +++ b/test/extensions/filters/http/cache/cache_filter_test.cc @@ -27,7 +27,7 @@ class CacheFilterTest : public ::testing::Test { protected: // The filter has to be created as a shared_ptr to enable shared_from_this() which is used in the // cache callbacks. - CacheFilterSharedPtr makeFilter(HttpCache& cache) { + CacheFilterSharedPtr makeFilter(OptRef cache) { auto filter = std::make_shared(config_, /*stats_prefix=*/"", context_.scope(), context_.timeSource(), cache); filter_state_ = std::make_shared( @@ -251,6 +251,13 @@ TEST_F(CacheFilterTest, CacheMiss) { } } +TEST_F(CacheFilterTest, Disabled) { + request_headers_.setHost("CacheDisabled"); + CacheFilterSharedPtr filter = makeFilter(OptRef{}); + EXPECT_EQ(filter->decodeHeaders(request_headers_, true), Http::FilterHeadersStatus::Continue); + filter->onDestroy(); +} + TEST_F(CacheFilterTest, CacheMissWithTrailers) { request_headers_.setHost("CacheMissWithTrailers"); const std::string body = "abc"; diff --git a/test/extensions/filters/http/cache/config_test.cc b/test/extensions/filters/http/cache/config_test.cc index aad9398b6d31..109d1e000fd7 100644 --- a/test/extensions/filters/http/cache/config_test.cc +++ b/test/extensions/filters/http/cache/config_test.cc @@ -33,6 +33,16 @@ TEST_F(CacheFilterFactoryTest, Basic) { ASSERT(dynamic_cast(filter.get())); } +TEST_F(CacheFilterFactoryTest, Disabled) { + config_.mutable_disabled()->set_value(true); + Http::FilterFactoryCb cb = factory_.createFilterFactoryFromProto(config_, "stats", context_); + Http::StreamFilterSharedPtr filter; + EXPECT_CALL(filter_callback_, addStreamFilter(_)).WillOnce(::testing::SaveArg<0>(&filter)); + cb(filter_callback_); + ASSERT(filter); + ASSERT(dynamic_cast(filter.get())); +} + TEST_F(CacheFilterFactoryTest, NoTypedConfig) { EXPECT_THROW(factory_.createFilterFactoryFromProto(config_, "stats", context_), EnvoyException); }