From 9b83957b693e039399a0d0c94c15e6e9ced55081 Mon Sep 17 00:00:00 2001 From: Nao Itoi Date: Fri, 13 Dec 2024 17:59:35 -0800 Subject: [PATCH 1/5] Added TTL Cache to ext_authz. --- .../extensions/filters/http/ext_authz/BUILD | 2 +- .../filters/http/ext_authz/caches/cache.hpp | 260 ++++++++++++++++++ .../http/ext_authz/caches/cache_policy.hpp | 89 ++++++ .../ext_authz/caches/lru_cache_policy.hpp | 77 ++++++ .../filters/http/ext_authz/ext_authz.cc | 49 +++- .../filters/http/ext_authz/ext_authz.h | 13 + .../filters/http/ext_authz/ttl_cache.h | 60 ++++ 7 files changed, 548 insertions(+), 2 deletions(-) create mode 100644 source/extensions/filters/http/ext_authz/caches/cache.hpp create mode 100644 source/extensions/filters/http/ext_authz/caches/cache_policy.hpp create mode 100644 source/extensions/filters/http/ext_authz/caches/lru_cache_policy.hpp create mode 100644 source/extensions/filters/http/ext_authz/ttl_cache.h diff --git a/source/extensions/filters/http/ext_authz/BUILD b/source/extensions/filters/http/ext_authz/BUILD index 02970d791e31..2aa320d40241 100644 --- a/source/extensions/filters/http/ext_authz/BUILD +++ b/source/extensions/filters/http/ext_authz/BUILD @@ -15,7 +15,7 @@ envoy_extension_package() envoy_cc_library( name = "ext_authz", srcs = ["ext_authz.cc"], - hdrs = ["ext_authz.h"], + hdrs = ["ext_authz.h", "ttl_cache.h", "caches/cache.hpp", "caches/cache_policy.hpp", "caches/lru_cache_policy.hpp",], deps = [ "//envoy/http:codes_interface", "//envoy/stats:stats_macros", diff --git a/source/extensions/filters/http/ext_authz/caches/cache.hpp b/source/extensions/filters/http/ext_authz/caches/cache.hpp new file mode 100644 index 000000000000..26e385d565e5 --- /dev/null +++ b/source/extensions/filters/http/ext_authz/caches/cache.hpp @@ -0,0 +1,260 @@ +/** + * \file + * \brief Generic cache implementation + */ +#ifndef CACHE_HPP +#define CACHE_HPP + +#include "cache_policy.hpp" + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace caches +{ +/** + * \brief Wrapper over the given value type to allow safe returning of a value from the cache + */ +template +using WrappedValue = std::shared_ptr; + +/** + * \brief Fixed sized cache that can be used with different policy types (e.g. LRU, FIFO, LFU) + * \tparam Key Type of a key (should be hashable) + * \tparam Value Type of a value stored in the cache + * \tparam Policy Type of a policy to be used with the cache + * \tparam HashMap Type of a hashmap to use for cache operations. Should have `std::unordered_map` + * compatible interface + */ +template class Policy = NoCachePolicy, + typename HashMap = std::unordered_map>> +class fixed_sized_cache +{ + public: + using map_type = HashMap; + using value_type = typename map_type::mapped_type; + using iterator = typename map_type::iterator; + using const_iterator = typename map_type::const_iterator; + using operation_guard = typename std::lock_guard; + using on_erase_cb = + typename std::function; + + /** + * \brief Fixed sized cache constructor + * \throw std::invalid_argument + * \param[in] max_size Maximum size of the cache + * \param[in] policy Cache policy to use + * \param[in] on_erase on_erase_cb function to be called when cache's element get erased + */ + explicit fixed_sized_cache( + size_t max_size, const Policy policy = Policy{}, + on_erase_cb on_erase = [](const Key &, const value_type &) {}) + : cache_policy{policy}, max_cache_size{max_size}, on_erase_callback{on_erase} + { + if (max_cache_size == 0) + { + throw std::invalid_argument{"Size of the cache should be non-zero"}; + } + } + + ~fixed_sized_cache() noexcept + { + Clear(); + } + + /** + * \brief Put element into the cache + * \param[in] key Key value to use + * \param[in] value Value to assign to the given key + */ + void Put(const Key &key, const Value &value) noexcept + { + operation_guard lock{safe_op}; + auto elem_it = FindElem(key); + + if (elem_it == cache_items_map.end()) + { + // add new element to the cache + if (cache_items_map.size() + 1 > max_cache_size) + { + auto disp_candidate_key = cache_policy.ReplCandidate(); + + Erase(disp_candidate_key); + } + + Insert(key, value); + } + else + { + // update previous value + Update(key, value); + } + } + + /** + * \brief Try to get an element by the given key from the cache + * \param[in] key Get element by key + * \return Pair of iterator that points to the element and boolean value that shows + * whether get operation has been successful or not. If pair's boolean value is false, + * the element is not presented in the cache. If pair's boolean value is true, + * returned iterator can be used to get access to the element + */ + std::pair TryGet(const Key &key) const noexcept + { + operation_guard lock{safe_op}; + const auto result = GetInternal(key); + + return std::make_pair(result.second ? result.first->second : nullptr, + result.second); + } + + /** + * \brief Get element from the cache if present + * \warning This method will change in the future with an optional class capabilities + * to avoid throwing exceptions + * \throw std::range_error + * \param[in] key Get element by key + * \return Reference to the value stored by the specified key in the cache + */ + value_type Get(const Key &key) const + { + operation_guard lock{safe_op}; + auto elem = GetInternal(key); + + if (elem.second) + { + return elem.first->second; + } + else + { + throw std::range_error{"No such element in the cache"}; + } + } + + /** + * \brief Check whether the given key is presented in the cache + * \param[in] key Element key to check + * \retval true Element is presented in the case + * \retval false Element is not presented in the case + */ + bool Cached(const Key &key) const noexcept + { + operation_guard lock{safe_op}; + return FindElem(key) != cache_items_map.cend(); + } + + /** + * \brief Get number of elements in cache + * \return Number of elements currently stored in the cache + */ + std::size_t Size() const + { + operation_guard lock{safe_op}; + + return cache_items_map.size(); + } + + /** + * Remove an element specified by key + * \param[in] key Key parameter + * \retval true if an element specified by key was found and deleted + * \retval false if an element is not present in a cache + */ + bool Remove(const Key &key) + { + operation_guard lock{safe_op}; + + auto elem = FindElem(key); + + if (elem == cache_items_map.end()) + { + return false; + } + + Erase(elem); + + return true; + } + + protected: + void Clear() + { + operation_guard lock{safe_op}; + + std::for_each(begin(), end(), + [&](const std::pair &el) + { cache_policy.Erase(el.first); }); + cache_items_map.clear(); + } + + const_iterator begin() const noexcept + { + return cache_items_map.cbegin(); + } + + const_iterator end() const noexcept + { + return cache_items_map.cend(); + } + + protected: + void Insert(const Key &key, const Value &value) + { + cache_policy.Insert(key); + cache_items_map.emplace(std::make_pair(key, std::make_shared(value))); + } + + void Erase(const_iterator elem) + { + cache_policy.Erase(elem->first); + on_erase_callback(elem->first, elem->second); + cache_items_map.erase(elem); + } + + void Erase(const Key &key) + { + auto elem_it = FindElem(key); + + Erase(elem_it); + } + + void Update(const Key &key, const Value &value) + { + cache_policy.Touch(key); + cache_items_map[key] = std::make_shared(value); + } + + const_iterator FindElem(const Key &key) const + { + return cache_items_map.find(key); + } + + std::pair GetInternal(const Key &key) const noexcept + { + auto elem_it = FindElem(key); + + if (elem_it != end()) + { + cache_policy.Touch(key); + return {elem_it, true}; + } + + return {elem_it, false}; + } + + private: + map_type cache_items_map; + mutable Policy cache_policy; + mutable std::mutex safe_op; + std::size_t max_cache_size; + on_erase_cb on_erase_callback; +}; +} // namespace caches + +#endif // CACHE_HPP diff --git a/source/extensions/filters/http/ext_authz/caches/cache_policy.hpp b/source/extensions/filters/http/ext_authz/caches/cache_policy.hpp new file mode 100644 index 000000000000..b5da1a5dee63 --- /dev/null +++ b/source/extensions/filters/http/ext_authz/caches/cache_policy.hpp @@ -0,0 +1,89 @@ +/** + * \file + * \brief Cache policy interface declaration + */ +#ifndef CACHE_POLICY_HPP +#define CACHE_POLICY_HPP + +#include + +namespace caches +{ + +/** + * \brief Cache policy abstract base class + * \tparam Key Type of a key a policy works with + */ +template +class ICachePolicy +{ + public: + virtual ~ICachePolicy() = default; + + /** + * \brief Handle element insertion in a cache + * \param[in] key Key that should be used by the policy + */ + virtual void Insert(const Key &key) = 0; + + /** + * \brief Handle request to the key-element in a cache + * \param key + */ + virtual void Touch(const Key &key) = 0; + /** + * \brief Handle element deletion from a cache + * \param[in] key Key that should be used by the policy + */ + virtual void Erase(const Key &key) = 0; + + /** + * \brief Return a key of a replacement candidate + * \return Replacement candidate according to selected policy + */ + virtual const Key &ReplCandidate() const = 0; +}; + +/** + * \brief Basic no caching policy class + * \details Preserve any key provided. Erase procedure can get rid of any added keys + * without specific rules: a replacement candidate will be the first element in the + * underlying container. As unordered container can be used in the implementation + * there are no warranties that the first/last added key will be erased + * \tparam Key Type of a key a policy works with + */ +template +class NoCachePolicy : public ICachePolicy +{ + public: + NoCachePolicy() = default; + ~NoCachePolicy() noexcept override = default; + + void Insert(const Key &key) override + { + key_storage.emplace(key); + } + + void Touch(const Key &key) noexcept override + { + // do not do anything + (void)key; + } + + void Erase(const Key &key) noexcept override + { + key_storage.erase(key); + } + + // return a key of a displacement candidate + const Key &ReplCandidate() const noexcept override + { + return *key_storage.cbegin(); + } + + private: + std::unordered_set key_storage; +}; +} // namespace caches + +#endif // CACHE_POLICY_HPP diff --git a/source/extensions/filters/http/ext_authz/caches/lru_cache_policy.hpp b/source/extensions/filters/http/ext_authz/caches/lru_cache_policy.hpp new file mode 100644 index 000000000000..948bb8d2280a --- /dev/null +++ b/source/extensions/filters/http/ext_authz/caches/lru_cache_policy.hpp @@ -0,0 +1,77 @@ +/** + * \file + * \brief LRU cache implementation + */ +#ifndef LRU_CACHE_POLICY_HPP +#define LRU_CACHE_POLICY_HPP + +#include "cache.hpp" +#include "cache_policy.hpp" +#include +#include + +namespace caches +{ +/** + * \brief LRU (Least Recently Used) cache policy + * \details LRU policy in the case of replacement removes the least recently used element. + * That is, in the case of replacement necessity, that cache policy returns a key that + * has not been touched recently. For example, cache maximum size is 3 and 3 elements have + * been added - `A`, `B`, `C`. Then the following actions were made: + * ``` + * Cache placement order: A, B, C + * Cache elements: A, B, C + * # Cache access: + * - A touched, B touched + * # LRU element in the cache: C + * # Cache access: + * - B touched, C touched + * # LRU element in the cache: A + * # Put new element: D + * # LRU replacement candidate: A + * + * Cache elements: B, C, D + * ``` + * \tparam Key Type of a key a policy works with + */ +template +class LRUCachePolicy : public ICachePolicy +{ + public: + using lru_iterator = typename std::list::iterator; + + LRUCachePolicy() = default; + ~LRUCachePolicy() = default; + + void Insert(const Key &key) override + { + lru_queue.emplace_front(key); + key_finder[key] = lru_queue.begin(); + } + + void Touch(const Key &key) override + { + // move the touched element at the beginning of the lru_queue + lru_queue.splice(lru_queue.begin(), lru_queue, key_finder[key]); + } + + void Erase(const Key &) noexcept override + { + // remove the least recently used element + key_finder.erase(lru_queue.back()); + lru_queue.pop_back(); + } + + // return a key of a displacement candidate + const Key &ReplCandidate() const noexcept override + { + return lru_queue.back(); + } + + private: + std::list lru_queue; + std::unordered_map key_finder; +}; +} // namespace caches + +#endif // LRU_CACHE_POLICY_HPP diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index 7874c8a94b74..abbe6539e93e 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -69,7 +69,6 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::ext_authz::v3 failure_mode_allow_header_add_(config.failure_mode_allow_header_add()), clear_route_cache_(config.clear_route_cache()), max_request_bytes_(config.with_request_body().max_request_bytes()), - // `pack_as_bytes_` should be true when configured with the HTTP service because there is no // difference to where the body is written in http requests, and a value of false here will // cause non UTF-8 body content to be changed when it doesn't need to. @@ -119,6 +118,8 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::ext_authz::v3 charge_cluster_response_stats_( PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, charge_cluster_response_stats, true)), stats_(generateStats(stats_prefix, config.stat_prefix(), scope)), + cache_(100, std::chrono::seconds(5)), // response cache + ext_authz_ok_(pool_.add(createPoolStatName(config.stat_prefix(), "ok"))), ext_authz_denied_(pool_.add(createPoolStatName(config.stat_prefix(), "denied"))), ext_authz_error_(pool_.add(createPoolStatName(config.stat_prefix(), "error"))), @@ -278,6 +279,37 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, return Http::FilterHeadersStatus::Continue; } + // Try reading response from the response cache + const auto auth_header = headers.get(Http::LowerCaseString("x-verkada-auth")); + + if (!auth_header.empty()) { + const std::string auth_header_str(auth_header[0]->value().getStringView()); + // ENVOY_STREAM_LOG(info, "Checking cache for header {}", *decoder_callbacks_, auth_header_str); + + // Retrieve the HTTP status code from the cache + auto cached_status_code = config_->resp_cache().Get(auth_header_str); + if (cached_status_code) { + ENVOY_STREAM_LOG(info, "Cache HIT for token {}: HTTP status {}", + *decoder_callbacks_, auth_header_str, *cached_status_code); + + if (*cached_status_code >= 200 && *cached_status_code < 300) { + // Any 2xx response is a success: let the request proceed + return Http::FilterHeadersStatus::Continue; + } else { + // Non-2xx response: reject the request + decoder_callbacks_->streamInfo().setResponseFlag( + StreamInfo::CoreResponseFlag::UnauthorizedExternalService); + decoder_callbacks_->sendLocalReply( + static_cast(*cached_status_code), "Unauthorized", nullptr, absl::nullopt, + Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzDenied); + + return Http::FilterHeadersStatus::StopIteration; + } + } + } else { + ENVOY_STREAM_LOG(info, "Cannot check cache because auth_header is empty", *decoder_callbacks_); + } + request_headers_ = &headers; const auto check_settings = per_route_flags.check_settings_; buffer_data_ = (config_->withRequestBody() || check_settings.has_with_request_body()) && @@ -302,6 +334,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, } // Initiate a call to the authorization server since we are not disabled. + ENVOY_STREAM_LOG(info, "Cache miss. Call external authorization service.", *decoder_callbacks_); initiateCall(headers); return filter_return_ == FilterReturn::StopDecoding ? Http::FilterHeadersStatus::StopAllIterationAndWatermark @@ -472,6 +505,20 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { using Filters::Common::ExtAuthz::CheckStatus; Stats::StatName empty_stat_name; + // Extract the actual HTTP status code + const int http_status_code = static_cast(response->status_code); + + // Log the response status + //ENVOY_LOG(info, "ext_authz response status: {}", http_status_code); + // ENVOY_STREAM_LOG(info, "ex_authx got response status code'{}'", *decoder_callbacks_, http_status_code); + // If x-verkada-auth header exists, cache the response status code + const auto auth_header = request_headers_->get(Http::LowerCaseString("x-verkada-auth")); + if (!auth_header.empty()) { + const std::string auth_header_str(auth_header[0]->value().getStringView()); + //ENVOY_LOG(info, "Caching response: {} with HTTP status: {}", auth_header_str, http_status_code); + config_->resp_cache().Put(auth_header_str, http_status_code); // Store the HTTP status code + } + updateLoggingInfo(); if (!response->dynamic_metadata.fields().empty()) { diff --git a/source/extensions/filters/http/ext_authz/ext_authz.h b/source/extensions/filters/http/ext_authz/ext_authz.h index 237dda979e54..9f81700ebded 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.h +++ b/source/extensions/filters/http/ext_authz/ext_authz.h @@ -26,6 +26,13 @@ #include "source/extensions/filters/common/ext_authz/ext_authz_http_impl.h" #include "source/extensions/filters/common/mutation_rules/mutation_rules.h" +// For response cache +#include "source/extensions/filters/http/ext_authz/ttl_cache.h" +#include "source/extensions/filters/http/ext_authz/caches/cache.hpp" +#include "source/extensions/filters/http/ext_authz/caches/cache_policy.hpp" +#include "source/extensions/filters/http/ext_authz/caches/lru_cache_policy.hpp" +using ttl_cache_t = TTLCache; + namespace Envoy { namespace Extensions { namespace HttpFilters { @@ -131,6 +138,9 @@ class FilterConfig { bool headersAsBytes() const { return encode_raw_headers_; } + ttl_cache_t& resp_cache() { return cache_; } + //const ttl_cache_t& resp_cache() const { return cache_; } + Filters::Common::MutationRules::CheckResult checkDecoderHeaderMutation(const Filters::Common::MutationRules::CheckOperation& operation, const Http::LowerCaseString& key, absl::string_view value) const { @@ -271,6 +281,9 @@ class FilterConfig { Filters::Common::ExtAuthz::MatcherSharedPtr allowed_headers_matcher_; Filters::Common::ExtAuthz::MatcherSharedPtr disallowed_headers_matcher_; + // Response cache + ttl_cache_t cache_; + public: // TODO(nezdolik): deprecate cluster scope stats counters in favor of filter scope stats // (ExtAuthzFilterStats stats_). diff --git a/source/extensions/filters/http/ext_authz/ttl_cache.h b/source/extensions/filters/http/ext_authz/ttl_cache.h new file mode 100644 index 000000000000..92f7464e5b0b --- /dev/null +++ b/source/extensions/filters/http/ext_authz/ttl_cache.h @@ -0,0 +1,60 @@ +#ifndef TTL_CACHE_H +#define TTL_CACHE_H + +#include "caches/cache.hpp" +#include "caches/cache_policy.hpp" +#include "caches/lru_cache_policy.hpp" +#include +#include +#include + +// Define the cache type using LRU policy +using lru_cache_t = caches::fixed_sized_cache; + +template class Policy> +class TTLCache { +public: + TTLCache(size_t max_size, std::chrono::milliseconds ttl); + + void Put(const Key& key, const Value& value); + std::optional Get(const Key& key); + size_t Size() const; + +private: + caches::fixed_sized_cache cache_; + std::chrono::milliseconds ttl_; + std::unordered_map expiration_times_; +}; + +template class Policy> +TTLCache::TTLCache(size_t max_size, std::chrono::milliseconds ttl) + : cache_(max_size), ttl_(ttl) {} + +template class Policy> +void TTLCache::Put(const Key& key, const Value& value) { + auto now = std::chrono::steady_clock::now(); + expiration_times_[key] = now + ttl_; + cache_.Put(key, value); +} + +template class Policy> +std::optional TTLCache::Get(const Key& key) { + auto now = std::chrono::steady_clock::now(); + if (expiration_times_.find(key) != expiration_times_.end() && expiration_times_[key] > now) { + auto value = cache_.Get(key); + if (value) { + return (*value); + } + } else { + cache_.Remove(key); + expiration_times_.erase(key); + } + return std::nullopt; +} + +template class Policy> +size_t TTLCache::Size() const { + return cache_.Size(); +} + +#endif // TTL_CACHE_H \ No newline at end of file From f3d2265c2115dc1c192149c92f38726c3aec612f Mon Sep 17 00:00:00 2001 From: Nao Itoi Date: Wed, 8 Jan 2025 09:39:23 -0800 Subject: [PATCH 2/5] Make cache size and TTL configurable. --- .../extensions/filters/http/ext_authz/v3/ext_authz.proto | 6 +++++- source/extensions/filters/http/ext_authz/caches/cache.hpp | 2 ++ source/extensions/filters/http/ext_authz/ext_authz.cc | 5 +++-- source/extensions/filters/http/ext_authz/ext_authz.h | 3 +++ 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto b/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto index d3bca54baccc..c86a937f48fb 100644 --- a/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto +++ b/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto @@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // External Authorization :ref:`configuration overview `. // [#extension: envoy.filters.http.ext_authz] -// [#next-free-field: 30] +// [#next-free-field: 32] message ExtAuthz { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.http.ext_authz.v3.ExtAuthz"; @@ -310,6 +310,10 @@ message ExtAuthz { // Field ``latency_us`` is exposed for CEL and logging when using gRPC or HTTP service. // Fields ``bytesSent`` and ``bytesReceived`` are exposed for CEL and logging only when using gRPC service. bool emit_filter_state_stats = 29; + + // Configuration parameters for ext_authz response cache + uint32 response_cache_max_size = 30; + uint32 response_cache_ttl = 31; } // Configuration for buffering the request data. diff --git a/source/extensions/filters/http/ext_authz/caches/cache.hpp b/source/extensions/filters/http/ext_authz/caches/cache.hpp index 26e385d565e5..c2090a9419b2 100644 --- a/source/extensions/filters/http/ext_authz/caches/cache.hpp +++ b/source/extensions/filters/http/ext_authz/caches/cache.hpp @@ -162,6 +162,8 @@ class fixed_sized_cache /** * Remove an element specified by key + * Nao : I believe this function has a bug because lru_cache_policy::Erase() always removes the least recently used element, regardless of the specified key. + * Don't call Remove(). * \param[in] key Key parameter * \retval true if an element specified by key was found and deleted * \retval false if an element is not present in a cache diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index abbe6539e93e..038ab7b063b0 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -118,8 +118,9 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::ext_authz::v3 charge_cluster_response_stats_( PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, charge_cluster_response_stats, true)), stats_(generateStats(stats_prefix, config.stat_prefix(), scope)), - cache_(100, std::chrono::seconds(5)), // response cache - + response_cache_max_size_(config.response_cache_max_size() != 0 ? config.response_cache_max_size() : 100), + response_cache_ttl_(config.response_cache_ttl() != 0 ? config.response_cache_ttl() : 10), + cache_(response_cache_max_size_, response_cache_ttl_), // response cache ext_authz_ok_(pool_.add(createPoolStatName(config.stat_prefix(), "ok"))), ext_authz_denied_(pool_.add(createPoolStatName(config.stat_prefix(), "denied"))), ext_authz_error_(pool_.add(createPoolStatName(config.stat_prefix(), "error"))), diff --git a/source/extensions/filters/http/ext_authz/ext_authz.h b/source/extensions/filters/http/ext_authz/ext_authz.h index 9f81700ebded..65d0f5dfd816 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.h +++ b/source/extensions/filters/http/ext_authz/ext_authz.h @@ -281,6 +281,9 @@ class FilterConfig { Filters::Common::ExtAuthz::MatcherSharedPtr allowed_headers_matcher_; Filters::Common::ExtAuthz::MatcherSharedPtr disallowed_headers_matcher_; + // Fields for response cache configuration + uint32_t response_cache_max_size_; + std::chrono::seconds response_cache_ttl_; // Response cache ttl_cache_t cache_; From 6072e2521e10aed71a91d6b0ca53c5123f576a29 Mon Sep 17 00:00:00 2001 From: Nao Itoi Date: Thu, 9 Jan 2025 08:52:33 -0800 Subject: [PATCH 3/5] Simplified cache to FIFO cache to save memory footprint. --- .../extensions/filters/http/ext_authz/BUILD | 2 +- .../filters/http/ext_authz/caches/cache.hpp | 262 ------------------ .../http/ext_authz/caches/cache_policy.hpp | 89 ------ .../ext_authz/caches/lru_cache_policy.hpp | 77 ----- .../filters/http/ext_authz/ext_authz.cc | 12 +- .../filters/http/ext_authz/ext_authz.h | 13 +- .../filters/http/ext_authz/fifo_cache.h | 120 ++++++++ .../filters/http/ext_authz/ttl_cache.h | 60 ---- 8 files changed, 130 insertions(+), 505 deletions(-) delete mode 100644 source/extensions/filters/http/ext_authz/caches/cache.hpp delete mode 100644 source/extensions/filters/http/ext_authz/caches/cache_policy.hpp delete mode 100644 source/extensions/filters/http/ext_authz/caches/lru_cache_policy.hpp create mode 100644 source/extensions/filters/http/ext_authz/fifo_cache.h delete mode 100644 source/extensions/filters/http/ext_authz/ttl_cache.h diff --git a/source/extensions/filters/http/ext_authz/BUILD b/source/extensions/filters/http/ext_authz/BUILD index 2aa320d40241..c5523b2453f2 100644 --- a/source/extensions/filters/http/ext_authz/BUILD +++ b/source/extensions/filters/http/ext_authz/BUILD @@ -15,7 +15,7 @@ envoy_extension_package() envoy_cc_library( name = "ext_authz", srcs = ["ext_authz.cc"], - hdrs = ["ext_authz.h", "ttl_cache.h", "caches/cache.hpp", "caches/cache_policy.hpp", "caches/lru_cache_policy.hpp",], + hdrs = ["ext_authz.h", "fifo_cache.h",], deps = [ "//envoy/http:codes_interface", "//envoy/stats:stats_macros", diff --git a/source/extensions/filters/http/ext_authz/caches/cache.hpp b/source/extensions/filters/http/ext_authz/caches/cache.hpp deleted file mode 100644 index c2090a9419b2..000000000000 --- a/source/extensions/filters/http/ext_authz/caches/cache.hpp +++ /dev/null @@ -1,262 +0,0 @@ -/** - * \file - * \brief Generic cache implementation - */ -#ifndef CACHE_HPP -#define CACHE_HPP - -#include "cache_policy.hpp" - -#include -#include -#include -#include -#include -#include -#include -#include - -namespace caches -{ -/** - * \brief Wrapper over the given value type to allow safe returning of a value from the cache - */ -template -using WrappedValue = std::shared_ptr; - -/** - * \brief Fixed sized cache that can be used with different policy types (e.g. LRU, FIFO, LFU) - * \tparam Key Type of a key (should be hashable) - * \tparam Value Type of a value stored in the cache - * \tparam Policy Type of a policy to be used with the cache - * \tparam HashMap Type of a hashmap to use for cache operations. Should have `std::unordered_map` - * compatible interface - */ -template class Policy = NoCachePolicy, - typename HashMap = std::unordered_map>> -class fixed_sized_cache -{ - public: - using map_type = HashMap; - using value_type = typename map_type::mapped_type; - using iterator = typename map_type::iterator; - using const_iterator = typename map_type::const_iterator; - using operation_guard = typename std::lock_guard; - using on_erase_cb = - typename std::function; - - /** - * \brief Fixed sized cache constructor - * \throw std::invalid_argument - * \param[in] max_size Maximum size of the cache - * \param[in] policy Cache policy to use - * \param[in] on_erase on_erase_cb function to be called when cache's element get erased - */ - explicit fixed_sized_cache( - size_t max_size, const Policy policy = Policy{}, - on_erase_cb on_erase = [](const Key &, const value_type &) {}) - : cache_policy{policy}, max_cache_size{max_size}, on_erase_callback{on_erase} - { - if (max_cache_size == 0) - { - throw std::invalid_argument{"Size of the cache should be non-zero"}; - } - } - - ~fixed_sized_cache() noexcept - { - Clear(); - } - - /** - * \brief Put element into the cache - * \param[in] key Key value to use - * \param[in] value Value to assign to the given key - */ - void Put(const Key &key, const Value &value) noexcept - { - operation_guard lock{safe_op}; - auto elem_it = FindElem(key); - - if (elem_it == cache_items_map.end()) - { - // add new element to the cache - if (cache_items_map.size() + 1 > max_cache_size) - { - auto disp_candidate_key = cache_policy.ReplCandidate(); - - Erase(disp_candidate_key); - } - - Insert(key, value); - } - else - { - // update previous value - Update(key, value); - } - } - - /** - * \brief Try to get an element by the given key from the cache - * \param[in] key Get element by key - * \return Pair of iterator that points to the element and boolean value that shows - * whether get operation has been successful or not. If pair's boolean value is false, - * the element is not presented in the cache. If pair's boolean value is true, - * returned iterator can be used to get access to the element - */ - std::pair TryGet(const Key &key) const noexcept - { - operation_guard lock{safe_op}; - const auto result = GetInternal(key); - - return std::make_pair(result.second ? result.first->second : nullptr, - result.second); - } - - /** - * \brief Get element from the cache if present - * \warning This method will change in the future with an optional class capabilities - * to avoid throwing exceptions - * \throw std::range_error - * \param[in] key Get element by key - * \return Reference to the value stored by the specified key in the cache - */ - value_type Get(const Key &key) const - { - operation_guard lock{safe_op}; - auto elem = GetInternal(key); - - if (elem.second) - { - return elem.first->second; - } - else - { - throw std::range_error{"No such element in the cache"}; - } - } - - /** - * \brief Check whether the given key is presented in the cache - * \param[in] key Element key to check - * \retval true Element is presented in the case - * \retval false Element is not presented in the case - */ - bool Cached(const Key &key) const noexcept - { - operation_guard lock{safe_op}; - return FindElem(key) != cache_items_map.cend(); - } - - /** - * \brief Get number of elements in cache - * \return Number of elements currently stored in the cache - */ - std::size_t Size() const - { - operation_guard lock{safe_op}; - - return cache_items_map.size(); - } - - /** - * Remove an element specified by key - * Nao : I believe this function has a bug because lru_cache_policy::Erase() always removes the least recently used element, regardless of the specified key. - * Don't call Remove(). - * \param[in] key Key parameter - * \retval true if an element specified by key was found and deleted - * \retval false if an element is not present in a cache - */ - bool Remove(const Key &key) - { - operation_guard lock{safe_op}; - - auto elem = FindElem(key); - - if (elem == cache_items_map.end()) - { - return false; - } - - Erase(elem); - - return true; - } - - protected: - void Clear() - { - operation_guard lock{safe_op}; - - std::for_each(begin(), end(), - [&](const std::pair &el) - { cache_policy.Erase(el.first); }); - cache_items_map.clear(); - } - - const_iterator begin() const noexcept - { - return cache_items_map.cbegin(); - } - - const_iterator end() const noexcept - { - return cache_items_map.cend(); - } - - protected: - void Insert(const Key &key, const Value &value) - { - cache_policy.Insert(key); - cache_items_map.emplace(std::make_pair(key, std::make_shared(value))); - } - - void Erase(const_iterator elem) - { - cache_policy.Erase(elem->first); - on_erase_callback(elem->first, elem->second); - cache_items_map.erase(elem); - } - - void Erase(const Key &key) - { - auto elem_it = FindElem(key); - - Erase(elem_it); - } - - void Update(const Key &key, const Value &value) - { - cache_policy.Touch(key); - cache_items_map[key] = std::make_shared(value); - } - - const_iterator FindElem(const Key &key) const - { - return cache_items_map.find(key); - } - - std::pair GetInternal(const Key &key) const noexcept - { - auto elem_it = FindElem(key); - - if (elem_it != end()) - { - cache_policy.Touch(key); - return {elem_it, true}; - } - - return {elem_it, false}; - } - - private: - map_type cache_items_map; - mutable Policy cache_policy; - mutable std::mutex safe_op; - std::size_t max_cache_size; - on_erase_cb on_erase_callback; -}; -} // namespace caches - -#endif // CACHE_HPP diff --git a/source/extensions/filters/http/ext_authz/caches/cache_policy.hpp b/source/extensions/filters/http/ext_authz/caches/cache_policy.hpp deleted file mode 100644 index b5da1a5dee63..000000000000 --- a/source/extensions/filters/http/ext_authz/caches/cache_policy.hpp +++ /dev/null @@ -1,89 +0,0 @@ -/** - * \file - * \brief Cache policy interface declaration - */ -#ifndef CACHE_POLICY_HPP -#define CACHE_POLICY_HPP - -#include - -namespace caches -{ - -/** - * \brief Cache policy abstract base class - * \tparam Key Type of a key a policy works with - */ -template -class ICachePolicy -{ - public: - virtual ~ICachePolicy() = default; - - /** - * \brief Handle element insertion in a cache - * \param[in] key Key that should be used by the policy - */ - virtual void Insert(const Key &key) = 0; - - /** - * \brief Handle request to the key-element in a cache - * \param key - */ - virtual void Touch(const Key &key) = 0; - /** - * \brief Handle element deletion from a cache - * \param[in] key Key that should be used by the policy - */ - virtual void Erase(const Key &key) = 0; - - /** - * \brief Return a key of a replacement candidate - * \return Replacement candidate according to selected policy - */ - virtual const Key &ReplCandidate() const = 0; -}; - -/** - * \brief Basic no caching policy class - * \details Preserve any key provided. Erase procedure can get rid of any added keys - * without specific rules: a replacement candidate will be the first element in the - * underlying container. As unordered container can be used in the implementation - * there are no warranties that the first/last added key will be erased - * \tparam Key Type of a key a policy works with - */ -template -class NoCachePolicy : public ICachePolicy -{ - public: - NoCachePolicy() = default; - ~NoCachePolicy() noexcept override = default; - - void Insert(const Key &key) override - { - key_storage.emplace(key); - } - - void Touch(const Key &key) noexcept override - { - // do not do anything - (void)key; - } - - void Erase(const Key &key) noexcept override - { - key_storage.erase(key); - } - - // return a key of a displacement candidate - const Key &ReplCandidate() const noexcept override - { - return *key_storage.cbegin(); - } - - private: - std::unordered_set key_storage; -}; -} // namespace caches - -#endif // CACHE_POLICY_HPP diff --git a/source/extensions/filters/http/ext_authz/caches/lru_cache_policy.hpp b/source/extensions/filters/http/ext_authz/caches/lru_cache_policy.hpp deleted file mode 100644 index 948bb8d2280a..000000000000 --- a/source/extensions/filters/http/ext_authz/caches/lru_cache_policy.hpp +++ /dev/null @@ -1,77 +0,0 @@ -/** - * \file - * \brief LRU cache implementation - */ -#ifndef LRU_CACHE_POLICY_HPP -#define LRU_CACHE_POLICY_HPP - -#include "cache.hpp" -#include "cache_policy.hpp" -#include -#include - -namespace caches -{ -/** - * \brief LRU (Least Recently Used) cache policy - * \details LRU policy in the case of replacement removes the least recently used element. - * That is, in the case of replacement necessity, that cache policy returns a key that - * has not been touched recently. For example, cache maximum size is 3 and 3 elements have - * been added - `A`, `B`, `C`. Then the following actions were made: - * ``` - * Cache placement order: A, B, C - * Cache elements: A, B, C - * # Cache access: - * - A touched, B touched - * # LRU element in the cache: C - * # Cache access: - * - B touched, C touched - * # LRU element in the cache: A - * # Put new element: D - * # LRU replacement candidate: A - * - * Cache elements: B, C, D - * ``` - * \tparam Key Type of a key a policy works with - */ -template -class LRUCachePolicy : public ICachePolicy -{ - public: - using lru_iterator = typename std::list::iterator; - - LRUCachePolicy() = default; - ~LRUCachePolicy() = default; - - void Insert(const Key &key) override - { - lru_queue.emplace_front(key); - key_finder[key] = lru_queue.begin(); - } - - void Touch(const Key &key) override - { - // move the touched element at the beginning of the lru_queue - lru_queue.splice(lru_queue.begin(), lru_queue, key_finder[key]); - } - - void Erase(const Key &) noexcept override - { - // remove the least recently used element - key_finder.erase(lru_queue.back()); - lru_queue.pop_back(); - } - - // return a key of a displacement candidate - const Key &ReplCandidate() const noexcept override - { - return lru_queue.back(); - } - - private: - std::list lru_queue; - std::unordered_map key_finder; -}; -} // namespace caches - -#endif // LRU_CACHE_POLICY_HPP diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index 038ab7b063b0..1f0ee8cf17da 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -285,11 +285,9 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, if (!auth_header.empty()) { const std::string auth_header_str(auth_header[0]->value().getStringView()); - // ENVOY_STREAM_LOG(info, "Checking cache for header {}", *decoder_callbacks_, auth_header_str); - // Retrieve the HTTP status code from the cache - auto cached_status_code = config_->resp_cache().Get(auth_header_str); - if (cached_status_code) { + auto cached_status_code = config_->resp_cache().Get(auth_header_str.c_str()); + if (cached_status_code.has_value()) { ENVOY_STREAM_LOG(info, "Cache HIT for token {}: HTTP status {}", *decoder_callbacks_, auth_header_str, *cached_status_code); @@ -507,7 +505,7 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { Stats::StatName empty_stat_name; // Extract the actual HTTP status code - const int http_status_code = static_cast(response->status_code); + const int http_status_code = static_cast(response->status_code); // Assuming this static cast is safe because http status code should be <= 0xffff // Log the response status //ENVOY_LOG(info, "ext_authz response status: {}", http_status_code); @@ -516,8 +514,8 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { const auto auth_header = request_headers_->get(Http::LowerCaseString("x-verkada-auth")); if (!auth_header.empty()) { const std::string auth_header_str(auth_header[0]->value().getStringView()); - //ENVOY_LOG(info, "Caching response: {} with HTTP status: {}", auth_header_str, http_status_code); - config_->resp_cache().Put(auth_header_str, http_status_code); // Store the HTTP status code + ENVOY_LOG(info, "Caching response: {} with HTTP status: {}", auth_header_str, http_status_code); + config_->resp_cache().Insert(auth_header_str.c_str(), http_status_code); // Store the HTTP status code } updateLoggingInfo(); diff --git a/source/extensions/filters/http/ext_authz/ext_authz.h b/source/extensions/filters/http/ext_authz/ext_authz.h index 65d0f5dfd816..8bfe290d6187 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.h +++ b/source/extensions/filters/http/ext_authz/ext_authz.h @@ -27,11 +27,7 @@ #include "source/extensions/filters/common/mutation_rules/mutation_rules.h" // For response cache -#include "source/extensions/filters/http/ext_authz/ttl_cache.h" -#include "source/extensions/filters/http/ext_authz/caches/cache.hpp" -#include "source/extensions/filters/http/ext_authz/caches/cache_policy.hpp" -#include "source/extensions/filters/http/ext_authz/caches/lru_cache_policy.hpp" -using ttl_cache_t = TTLCache; +#include "source/extensions/filters/http/ext_authz/fifo_cache.h" namespace Envoy { namespace Extensions { @@ -138,8 +134,7 @@ class FilterConfig { bool headersAsBytes() const { return encode_raw_headers_; } - ttl_cache_t& resp_cache() { return cache_; } - //const ttl_cache_t& resp_cache() const { return cache_; } + fifo_cache::FIFOEvictionCache& resp_cache() { return cache_; } Filters::Common::MutationRules::CheckResult checkDecoderHeaderMutation(const Filters::Common::MutationRules::CheckOperation& operation, @@ -283,9 +278,9 @@ class FilterConfig { // Fields for response cache configuration uint32_t response_cache_max_size_; - std::chrono::seconds response_cache_ttl_; + uint32_t response_cache_ttl_; // Response cache - ttl_cache_t cache_; + fifo_cache::FIFOEvictionCache cache_; public: // TODO(nezdolik): deprecate cluster scope stats counters in favor of filter scope stats diff --git a/source/extensions/filters/http/ext_authz/fifo_cache.h b/source/extensions/filters/http/ext_authz/fifo_cache.h new file mode 100644 index 000000000000..a7aa3a069a9e --- /dev/null +++ b/source/extensions/filters/http/ext_authz/fifo_cache.h @@ -0,0 +1,120 @@ +#ifndef FIFO_CACHE_H +#define FIFO_CACHE_H + +#include +#include +#include +#include // For uint16_t +#include // For strcmp +#include // For debug output +#include // For std::chrono + +namespace fifo_cache { + +/** + A simple cache class with TTL. + It has FIFO eviction policy. Compared to other policies like LRU, this is memory efficient, because it does not need to store the order of elements. + It restricts stored values to 16-bit unsigned integers. This also makes it memory efficient. + */ +class FIFOEvictionCache { +public: + // By default, TTL will be 10 seconds. + FIFOEvictionCache(std::size_t max_size, int default_ttl_seconds = 10) + : max_cache_size(max_size), default_ttl_seconds(default_ttl_seconds) {} + + bool Insert(const char* key, uint16_t value, int ttl_seconds = -1) { + std::lock_guard lock(safe_op); + const char* c_key = strdup(key); + if (ttl_seconds == -1) { + ttl_seconds = default_ttl_seconds; + } + auto expiration_time = std::chrono::steady_clock::now() + std::chrono::seconds(ttl_seconds); + CacheItem item = {value, expiration_time}; + auto it = cache_items_map.find(c_key); + if (it == cache_items_map.end()) { + if (cache_items_map.size() >= max_cache_size) { + Evict(); + } + cache_items_map[c_key] = item; + } else { + cache_items_map[c_key] = item; + } + return true; + } + + bool Erase(const char* key) { + std::lock_guard lock(safe_op); + auto it = cache_items_map.find(key); + if (it != cache_items_map.end()) { + free(const_cast(it->first)); + cache_items_map.erase(it); + return true; + } + return false; + } + + std::optional Get(const char* key) { + std::lock_guard lock(safe_op); + auto it = cache_items_map.find(key); + if (it != cache_items_map.end()) { + if (std::chrono::steady_clock::now() < it->second.expiration_time) { + return it->second.value; + } else { + // Item has expired + free(const_cast(it->first)); + cache_items_map.erase(it); + } + } + return std::nullopt; + } + + size_t Size() const { + std::lock_guard lock(safe_op); + return cache_items_map.size(); + } + + ~FIFOEvictionCache() { + for (auto& pair : cache_items_map) { + free(const_cast(pair.first)); + } + } + +private: + struct CacheItem { + uint16_t value; + std::chrono::steady_clock::time_point expiration_time; + }; + + void Evict() { + if (!cache_items_map.empty()) { + auto it = cache_items_map.begin(); + free(const_cast(it->first)); + cache_items_map.erase(it); + } + } + + struct CharPtrHash { + std::size_t operator()(const char* str) const { + std::size_t hash = 0; + while (*str) { + hash = hash * 101 + *str++; + } + return hash; + } + }; + + struct CharPtrEqual { + bool operator()(const char* lhs, const char* rhs) const { + return std::strcmp(lhs, rhs) == 0; + } + }; + + std::unordered_map cache_items_map; + std::size_t max_cache_size; + int default_ttl_seconds; + mutable std::mutex safe_op; +}; + +} // namespace fifo_cache + +#endif // FIFO_CACHE_H \ No newline at end of file diff --git a/source/extensions/filters/http/ext_authz/ttl_cache.h b/source/extensions/filters/http/ext_authz/ttl_cache.h deleted file mode 100644 index 92f7464e5b0b..000000000000 --- a/source/extensions/filters/http/ext_authz/ttl_cache.h +++ /dev/null @@ -1,60 +0,0 @@ -#ifndef TTL_CACHE_H -#define TTL_CACHE_H - -#include "caches/cache.hpp" -#include "caches/cache_policy.hpp" -#include "caches/lru_cache_policy.hpp" -#include -#include -#include - -// Define the cache type using LRU policy -using lru_cache_t = caches::fixed_sized_cache; - -template class Policy> -class TTLCache { -public: - TTLCache(size_t max_size, std::chrono::milliseconds ttl); - - void Put(const Key& key, const Value& value); - std::optional Get(const Key& key); - size_t Size() const; - -private: - caches::fixed_sized_cache cache_; - std::chrono::milliseconds ttl_; - std::unordered_map expiration_times_; -}; - -template class Policy> -TTLCache::TTLCache(size_t max_size, std::chrono::milliseconds ttl) - : cache_(max_size), ttl_(ttl) {} - -template class Policy> -void TTLCache::Put(const Key& key, const Value& value) { - auto now = std::chrono::steady_clock::now(); - expiration_times_[key] = now + ttl_; - cache_.Put(key, value); -} - -template class Policy> -std::optional TTLCache::Get(const Key& key) { - auto now = std::chrono::steady_clock::now(); - if (expiration_times_.find(key) != expiration_times_.end() && expiration_times_[key] > now) { - auto value = cache_.Get(key); - if (value) { - return (*value); - } - } else { - cache_.Remove(key); - expiration_times_.erase(key); - } - return std::nullopt; -} - -template class Policy> -size_t TTLCache::Size() const { - return cache_.Size(); -} - -#endif // TTL_CACHE_H \ No newline at end of file From 24e0560f22db268828be587a1fe7d8590e13ee97 Mon Sep 17 00:00:00 2001 From: Nao Itoi Date: Thu, 9 Jan 2025 12:19:28 -0800 Subject: [PATCH 4/5] Read auth header names from configuration --- .../filters/http/ext_authz/v3/ext_authz.proto | 4 +- .../filters/http/ext_authz/ext_authz.cc | 42 ++++++++++++------- .../filters/http/ext_authz/ext_authz.h | 9 +++- 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto b/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto index c86a937f48fb..aa61f1d5f404 100644 --- a/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto +++ b/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto @@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // External Authorization :ref:`configuration overview `. // [#extension: envoy.filters.http.ext_authz] -// [#next-free-field: 32] +// [#next-free-field: 33] message ExtAuthz { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.http.ext_authz.v3.ExtAuthz"; @@ -314,6 +314,8 @@ message ExtAuthz { // Configuration parameters for ext_authz response cache uint32 response_cache_max_size = 30; uint32 response_cache_ttl = 31; + // Array of header names. ext_authz will use the first header that appears in the HTTP request as the cache key. + repeated string response_cache_header_names = 32; } // Configuration for buffering the request data. diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index 1f0ee8cf17da..9c1fe952f1f0 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -59,6 +59,19 @@ void fillMetadataContext(const std::vector& source_metadat } } +// Utility function to find the first set auth header, based on the candidate headers in configuration +absl::optional get_first_auth_header( + const Http::RequestHeaderMap& headers, + const std::vector& header_names) { + for (const auto& header_name : header_names) { + const auto header = headers.get(Http::LowerCaseString(header_name)); + if (!header.empty()) { + return header[0]->value().getStringView(); + } + } + return absl::nullopt; +} + } // namespace FilterConfig::FilterConfig(const envoy::extensions::filters::http::ext_authz::v3::ExtAuthz& config, @@ -120,7 +133,9 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::ext_authz::v3 stats_(generateStats(stats_prefix, config.stat_prefix(), scope)), response_cache_max_size_(config.response_cache_max_size() != 0 ? config.response_cache_max_size() : 100), response_cache_ttl_(config.response_cache_ttl() != 0 ? config.response_cache_ttl() : 10), - cache_(response_cache_max_size_, response_cache_ttl_), // response cache + response_cache_header_names_(config.response_cache_header_names().begin(), + config.response_cache_header_names().end()), // Initialize header names + response_cache_(response_cache_max_size_, response_cache_ttl_), // response cache ext_authz_ok_(pool_.add(createPoolStatName(config.stat_prefix(), "ok"))), ext_authz_denied_(pool_.add(createPoolStatName(config.stat_prefix(), "denied"))), ext_authz_error_(pool_.add(createPoolStatName(config.stat_prefix(), "error"))), @@ -281,12 +296,12 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, } // Try reading response from the response cache - const auto auth_header = headers.get(Http::LowerCaseString("x-verkada-auth")); + const auto auth_header = get_first_auth_header(headers, config_->responseCacheHeaderNames()); - if (!auth_header.empty()) { - const std::string auth_header_str(auth_header[0]->value().getStringView()); + if (auth_header.has_value()) { + const std::string auth_header_str(auth_header.value()); // Retrieve the HTTP status code from the cache - auto cached_status_code = config_->resp_cache().Get(auth_header_str.c_str()); + auto cached_status_code = config_->responseCache().Get(auth_header_str.c_str()); if (cached_status_code.has_value()) { ENVOY_STREAM_LOG(info, "Cache HIT for token {}: HTTP status {}", *decoder_callbacks_, auth_header_str, *cached_status_code); @@ -304,6 +319,9 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, return Http::FilterHeadersStatus::StopIteration; } + } else { + ENVOY_STREAM_LOG(info, "Cache miss for auth token {}. Will call external authz.", + *decoder_callbacks_, auth_header_str); } } else { ENVOY_STREAM_LOG(info, "Cannot check cache because auth_header is empty", *decoder_callbacks_); @@ -333,7 +351,6 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, } // Initiate a call to the authorization server since we are not disabled. - ENVOY_STREAM_LOG(info, "Cache miss. Call external authorization service.", *decoder_callbacks_); initiateCall(headers); return filter_return_ == FilterReturn::StopDecoding ? Http::FilterHeadersStatus::StopAllIterationAndWatermark @@ -507,15 +524,12 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { // Extract the actual HTTP status code const int http_status_code = static_cast(response->status_code); // Assuming this static cast is safe because http status code should be <= 0xffff - // Log the response status - //ENVOY_LOG(info, "ext_authz response status: {}", http_status_code); - // ENVOY_STREAM_LOG(info, "ex_authx got response status code'{}'", *decoder_callbacks_, http_status_code); - // If x-verkada-auth header exists, cache the response status code - const auto auth_header = request_headers_->get(Http::LowerCaseString("x-verkada-auth")); - if (!auth_header.empty()) { - const std::string auth_header_str(auth_header[0]->value().getStringView()); + // If auth header exists, cache the response status code + const auto auth_header = get_first_auth_header(*request_headers_, config_->responseCacheHeaderNames()); + if (auth_header.has_value()) { + const std::string auth_header_str(auth_header.value()); ENVOY_LOG(info, "Caching response: {} with HTTP status: {}", auth_header_str, http_status_code); - config_->resp_cache().Insert(auth_header_str.c_str(), http_status_code); // Store the HTTP status code + config_->responseCache().Insert(auth_header_str.c_str(), http_status_code); // Store the HTTP status code } updateLoggingInfo(); diff --git a/source/extensions/filters/http/ext_authz/ext_authz.h b/source/extensions/filters/http/ext_authz/ext_authz.h index 8bfe290d6187..3c8d91d906fb 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.h +++ b/source/extensions/filters/http/ext_authz/ext_authz.h @@ -134,7 +134,11 @@ class FilterConfig { bool headersAsBytes() const { return encode_raw_headers_; } - fifo_cache::FIFOEvictionCache& resp_cache() { return cache_; } + fifo_cache::FIFOEvictionCache& responseCache() { return response_cache_; } + + const std::vector& responseCacheHeaderNames() const { + return response_cache_header_names_; + } Filters::Common::MutationRules::CheckResult checkDecoderHeaderMutation(const Filters::Common::MutationRules::CheckOperation& operation, @@ -279,8 +283,9 @@ class FilterConfig { // Fields for response cache configuration uint32_t response_cache_max_size_; uint32_t response_cache_ttl_; + std::vector response_cache_header_names_; // New field for header names // Response cache - fifo_cache::FIFOEvictionCache cache_; + fifo_cache::FIFOEvictionCache response_cache_; public: // TODO(nezdolik): deprecate cluster scope stats counters in favor of filter scope stats From bc52c1e6d95acf8a148278a08e2d8984b1b13bd5 Mon Sep 17 00:00:00 2001 From: Nao Itoi Date: Fri, 10 Jan 2025 14:26:02 -0800 Subject: [PATCH 5/5] Fix style and libraries according to Envoy guideline. Signed-off-by: Nao Itoi --- .../filters/http/ext_authz/v3/ext_authz.proto | 2 + .../extensions/filters/http/ext_authz/BUILD | 5 +- .../filters/http/ext_authz/ext_authz.cc | 44 ++-- .../filters/http/ext_authz/ext_authz.h | 4 +- .../filters/http/ext_authz/fifo_cache.h | 220 ++++++++++-------- 5 files changed, 159 insertions(+), 116 deletions(-) diff --git a/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto b/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto index aa61f1d5f404..335a35c163bc 100644 --- a/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto +++ b/api/envoy/extensions/filters/http/ext_authz/v3/ext_authz.proto @@ -313,7 +313,9 @@ message ExtAuthz { // Configuration parameters for ext_authz response cache uint32 response_cache_max_size = 30; + uint32 response_cache_ttl = 31; + // Array of header names. ext_authz will use the first header that appears in the HTTP request as the cache key. repeated string response_cache_header_names = 32; } diff --git a/source/extensions/filters/http/ext_authz/BUILD b/source/extensions/filters/http/ext_authz/BUILD index c5523b2453f2..e8fe47cfc184 100644 --- a/source/extensions/filters/http/ext_authz/BUILD +++ b/source/extensions/filters/http/ext_authz/BUILD @@ -15,7 +15,10 @@ envoy_extension_package() envoy_cc_library( name = "ext_authz", srcs = ["ext_authz.cc"], - hdrs = ["ext_authz.h", "fifo_cache.h",], + hdrs = [ + "ext_authz.h", + "fifo_cache.h", + ], deps = [ "//envoy/http:codes_interface", "//envoy/stats:stats_macros", diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index 9c1fe952f1f0..511902a1f4ce 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -59,10 +59,11 @@ void fillMetadataContext(const std::vector& source_metadat } } -// Utility function to find the first set auth header, based on the candidate headers in configuration -absl::optional get_first_auth_header( - const Http::RequestHeaderMap& headers, - const std::vector& header_names) { +// Utility function to find the first set auth header, based on the candidate headers in +// configuration +absl::optional +get_first_auth_header(const Http::RequestHeaderMap& headers, + const std::vector& header_names) { for (const auto& header_name : header_names) { const auto header = headers.get(Http::LowerCaseString(header_name)); if (!header.empty()) { @@ -131,11 +132,14 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::ext_authz::v3 charge_cluster_response_stats_( PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, charge_cluster_response_stats, true)), stats_(generateStats(stats_prefix, config.stat_prefix(), scope)), - response_cache_max_size_(config.response_cache_max_size() != 0 ? config.response_cache_max_size() : 100), + response_cache_max_size_( + config.response_cache_max_size() != 0 ? config.response_cache_max_size() : 100), response_cache_ttl_(config.response_cache_ttl() != 0 ? config.response_cache_ttl() : 10), - response_cache_header_names_(config.response_cache_header_names().begin(), - config.response_cache_header_names().end()), // Initialize header names - response_cache_(response_cache_max_size_, response_cache_ttl_), // response cache + response_cache_header_names_( + config.response_cache_header_names().begin(), + config.response_cache_header_names().end()), // Initialize header names + response_cache_(response_cache_max_size_, response_cache_ttl_, + factory_context.timeSource()), // response cache ext_authz_ok_(pool_.add(createPoolStatName(config.stat_prefix(), "ok"))), ext_authz_denied_(pool_.add(createPoolStatName(config.stat_prefix(), "denied"))), ext_authz_error_(pool_.add(createPoolStatName(config.stat_prefix(), "error"))), @@ -303,8 +307,8 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, // Retrieve the HTTP status code from the cache auto cached_status_code = config_->responseCache().Get(auth_header_str.c_str()); if (cached_status_code.has_value()) { - ENVOY_STREAM_LOG(info, "Cache HIT for token {}: HTTP status {}", - *decoder_callbacks_, auth_header_str, *cached_status_code); + ENVOY_STREAM_LOG(info, "Cache HIT for token {}: HTTP status {}", *decoder_callbacks_, + auth_header_str, *cached_status_code); if (*cached_status_code >= 200 && *cached_status_code < 300) { // Any 2xx response is a success: let the request proceed @@ -312,16 +316,16 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, } else { // Non-2xx response: reject the request decoder_callbacks_->streamInfo().setResponseFlag( - StreamInfo::CoreResponseFlag::UnauthorizedExternalService); + StreamInfo::CoreResponseFlag::UnauthorizedExternalService); decoder_callbacks_->sendLocalReply( - static_cast(*cached_status_code), "Unauthorized", nullptr, absl::nullopt, - Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzDenied); + static_cast(*cached_status_code), "Unauthorized", nullptr, absl::nullopt, + Filters::Common::ExtAuthz::ResponseCodeDetails::get().AuthzDenied); return Http::FilterHeadersStatus::StopIteration; } } else { - ENVOY_STREAM_LOG(info, "Cache miss for auth token {}. Will call external authz.", - *decoder_callbacks_, auth_header_str); + ENVOY_STREAM_LOG(info, "Cache miss for auth token {}. Will call external authz.", + *decoder_callbacks_, auth_header_str); } } else { ENVOY_STREAM_LOG(info, "Cannot check cache because auth_header is empty", *decoder_callbacks_); @@ -522,14 +526,18 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) { Stats::StatName empty_stat_name; // Extract the actual HTTP status code - const int http_status_code = static_cast(response->status_code); // Assuming this static cast is safe because http status code should be <= 0xffff + const int http_status_code = + static_cast(response->status_code); // Assuming this static cast is safe because + // http status code should be <= 0xffff // If auth header exists, cache the response status code - const auto auth_header = get_first_auth_header(*request_headers_, config_->responseCacheHeaderNames()); + const auto auth_header = + get_first_auth_header(*request_headers_, config_->responseCacheHeaderNames()); if (auth_header.has_value()) { const std::string auth_header_str(auth_header.value()); ENVOY_LOG(info, "Caching response: {} with HTTP status: {}", auth_header_str, http_status_code); - config_->responseCache().Insert(auth_header_str.c_str(), http_status_code); // Store the HTTP status code + config_->responseCache().Insert(auth_header_str.c_str(), + http_status_code); // Store the HTTP status code } updateLoggingInfo(); diff --git a/source/extensions/filters/http/ext_authz/ext_authz.h b/source/extensions/filters/http/ext_authz/ext_authz.h index 3c8d91d906fb..2bbfc2728656 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.h +++ b/source/extensions/filters/http/ext_authz/ext_authz.h @@ -134,7 +134,7 @@ class FilterConfig { bool headersAsBytes() const { return encode_raw_headers_; } - fifo_cache::FIFOEvictionCache& responseCache() { return response_cache_; } + FIFOEvictionCache& responseCache() { return response_cache_; } const std::vector& responseCacheHeaderNames() const { return response_cache_header_names_; @@ -285,7 +285,7 @@ class FilterConfig { uint32_t response_cache_ttl_; std::vector response_cache_header_names_; // New field for header names // Response cache - fifo_cache::FIFOEvictionCache response_cache_; + FIFOEvictionCache response_cache_; public: // TODO(nezdolik): deprecate cluster scope stats counters in favor of filter scope stats diff --git a/source/extensions/filters/http/ext_authz/fifo_cache.h b/source/extensions/filters/http/ext_authz/fifo_cache.h index a7aa3a069a9e..19936c72f162 100644 --- a/source/extensions/filters/http/ext_authz/fifo_cache.h +++ b/source/extensions/filters/http/ext_authz/fifo_cache.h @@ -1,120 +1,150 @@ -#ifndef FIFO_CACHE_H -#define FIFO_CACHE_H +#pragma once -#include -#include -#include -#include // For uint16_t -#include // For strcmp -#include // For debug output -#include // For std::chrono +#include // For std::shuffle +#include // For std::default_random_engine +#include -namespace fifo_cache { +#include "envoy/common/time.h" + +#include "source/common/common/thread.h" + +#include "absl/container/flat_hash_map.h" +#include "absl/types/optional.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace ExtAuthz { /** A simple cache class with TTL. - It has FIFO eviction policy. Compared to other policies like LRU, this is memory efficient, because it does not need to store the order of elements. - It restricts stored values to 16-bit unsigned integers. This also makes it memory efficient. + It has a random subset eviction policy. This is memory efficient because it does not need to store + the order of elements. It restricts stored values to 16-bit unsigned integers, making it + memory efficient. */ class FIFOEvictionCache { public: - // By default, TTL will be 10 seconds. - FIFOEvictionCache(std::size_t max_size, int default_ttl_seconds = 10) - : max_cache_size(max_size), default_ttl_seconds(default_ttl_seconds) {} - - bool Insert(const char* key, uint16_t value, int ttl_seconds = -1) { - std::lock_guard lock(safe_op); - const char* c_key = strdup(key); - if (ttl_seconds == -1) { - ttl_seconds = default_ttl_seconds; - } - auto expiration_time = std::chrono::steady_clock::now() + std::chrono::seconds(ttl_seconds); - CacheItem item = {value, expiration_time}; - auto it = cache_items_map.find(c_key); - if (it == cache_items_map.end()) { - if (cache_items_map.size() >= max_cache_size) { - Evict(); - } - cache_items_map[c_key] = item; - } else { - cache_items_map[c_key] = item; - } - return true; - } + // By default, TTL will be 10 seconds. + FIFOEvictionCache(std::size_t max_size, int default_ttl_seconds, Envoy::TimeSource& time_source) + : max_cache_size(max_size), default_ttl_seconds(default_ttl_seconds), + time_source_(time_source) {} - bool Erase(const char* key) { - std::lock_guard lock(safe_op); - auto it = cache_items_map.find(key); - if (it != cache_items_map.end()) { - free(const_cast(it->first)); - cache_items_map.erase(it); - return true; - } - return false; + ~FIFOEvictionCache() { + for (auto& pair : cache_items_map) { + free(const_cast(pair.first)); } + } - std::optional Get(const char* key) { - std::lock_guard lock(safe_op); - auto it = cache_items_map.find(key); - if (it != cache_items_map.end()) { - if (std::chrono::steady_clock::now() < it->second.expiration_time) { - return it->second.value; - } else { - // Item has expired - free(const_cast(it->first)); - cache_items_map.erase(it); - } - } - return std::nullopt; + bool Insert(const char* key, uint16_t value, int ttl_seconds = -1) { + Thread::LockGuard lock{mutex_}; + const char* c_key = strdup(key); + if (ttl_seconds == -1) { + ttl_seconds = default_ttl_seconds; + } + auto expiration_time = time_source_.monotonicTime() + std::chrono::seconds(ttl_seconds); + CacheItem item = {value, expiration_time}; + auto it = cache_items_map.find(c_key); + if (it == cache_items_map.end()) { + if (cache_items_map.size() >= max_cache_size) { + Evict(); + } + cache_items_map[c_key] = item; + } else { + cache_items_map[c_key] = item; } + return true; + } - size_t Size() const { - std::lock_guard lock(safe_op); - return cache_items_map.size(); + bool Erase(const char* key) { + Thread::LockGuard lock{mutex_}; + auto it = cache_items_map.find(key); + if (it != cache_items_map.end()) { + free(const_cast(it->first)); + cache_items_map.erase(it); + return true; } + return false; + } - ~FIFOEvictionCache() { - for (auto& pair : cache_items_map) { - free(const_cast(pair.first)); - } + absl::optional Get(const char* key) { + Thread::LockGuard lock{mutex_}; + auto it = cache_items_map.find(key); + if (it != cache_items_map.end()) { + if (time_source_.monotonicTime() < it->second.expiration_time) { + return it->second.value; + } else { + // Item has expired + free(const_cast(it->first)); + cache_items_map.erase(it); + } } + return absl::nullopt; + } + + size_t Size() const { + Thread::LockGuard lock{mutex_}; + return cache_items_map.size(); + } private: - struct CacheItem { - uint16_t value; - std::chrono::steady_clock::time_point expiration_time; - }; - - void Evict() { - if (!cache_items_map.empty()) { - auto it = cache_items_map.begin(); - free(const_cast(it->first)); - cache_items_map.erase(it); + struct CacheItem { + uint16_t value; + std::chrono::steady_clock::time_point expiration_time; + }; + + void Evict() { + if (cache_items_map.size() > 0) { + // Select a random subset of items + std::vector keys; + for (const auto& pair : cache_items_map) { + keys.push_back(pair.first); + } + std::default_random_engine rng(std::random_device{}()); + std::shuffle(keys.begin(), keys.end(), rng); + + // Sort the subset by TTL + std::sort(keys.begin(), keys.begin() + std::min(keys.size(), size_t(10)), + [this](const char* lhs, const char* rhs) { + return cache_items_map[lhs].expiration_time < + cache_items_map[rhs].expiration_time; + }); + + // Evict the items with the nearest TTL + for (size_t i = 0; i < std::min(keys.size(), size_t(3)); ++i) { + auto it = cache_items_map.find(keys[i]); + if (it != cache_items_map.end()) { + free(const_cast(it->first)); + cache_items_map.erase(it); } + } } + } - struct CharPtrHash { - std::size_t operator()(const char* str) const { - std::size_t hash = 0; - while (*str) { - hash = hash * 101 + *str++; - } - return hash; - } - }; + struct CharPtrHash { + std::size_t operator()(const char* str) const { + std::size_t hash = 0; + while (*str) { + hash = hash * 101 + *str++; + } + return hash; + } + }; - struct CharPtrEqual { - bool operator()(const char* lhs, const char* rhs) const { - return std::strcmp(lhs, rhs) == 0; - } - }; + struct CharPtrEqual { + bool operator()(const char* lhs, const char* rhs) const { return std::strcmp(lhs, rhs) == 0; } + }; - std::unordered_map cache_items_map; - std::size_t max_cache_size; - int default_ttl_seconds; - mutable std::mutex safe_op; -}; + absl::flat_hash_map cache_items_map; -} // namespace fifo_cache + mutable Thread::MutexBasicLockable + mutex_; // Mark mutex_ as mutable to allow locking in const methods + + std::size_t max_cache_size; + int default_ttl_seconds; + Envoy::TimeSource& time_source_; // Reference to TimeSource +}; -#endif // FIFO_CACHE_H \ No newline at end of file +} // namespace ExtAuthz +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy