diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 3df7e7d8d2490..e42bd100e0fea 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -52,6 +52,9 @@ removed_config_or_runtime: - area: dns change: | Removed runtime flag ``envoy.reloadable_features.dns_details`` and legacy code paths. +- area: local_ratelimit + change: | + Removed runtime guard ``envoy.reloadable_features.no_timer_based_rate_limit_token_bucket`` and legacy code paths. new_features: - area: oauth2 diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index a3e1533937606..4b4b50d0dd9c2 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -70,7 +70,6 @@ RUNTIME_GUARD(envoy_reloadable_features_logging_with_fast_json_formatter); RUNTIME_GUARD(envoy_reloadable_features_lua_flow_control_while_http_call); RUNTIME_GUARD(envoy_reloadable_features_mmdb_files_reload_enabled); RUNTIME_GUARD(envoy_reloadable_features_no_extension_lookup_by_name); -RUNTIME_GUARD(envoy_reloadable_features_no_timer_based_rate_limit_token_bucket); RUNTIME_GUARD(envoy_reloadable_features_normalize_rds_provider_config); RUNTIME_GUARD(envoy_reloadable_features_oauth2_use_refresh_token); RUNTIME_GUARD(envoy_reloadable_features_original_dst_rely_on_idle_timeout); diff --git a/source/extensions/filters/common/local_ratelimit/local_ratelimit_impl.cc b/source/extensions/filters/common/local_ratelimit/local_ratelimit_impl.cc index d9fb6ad9f9bbe..72c6975e8f21c 100644 --- a/source/extensions/filters/common/local_ratelimit/local_ratelimit_impl.cc +++ b/source/extensions/filters/common/local_ratelimit/local_ratelimit_impl.cc @@ -75,95 +75,14 @@ ShareProviderManagerSharedPtr ShareProviderManager::singleton(Event::Dispatcher& }); } -TimerTokenBucket::TimerTokenBucket(uint64_t max_tokens, uint64_t tokens_per_fill, - std::chrono::milliseconds fill_interval, uint64_t multiplier, - LocalRateLimiterImpl& parent) - : multiplier_(multiplier), parent_(parent), max_tokens_(max_tokens), - tokens_per_fill_(tokens_per_fill), fill_interval_(fill_interval), - // Calculate the fill rate in tokens per second. - fill_rate_(tokens_per_fill / - std::chrono::duration_cast>(fill_interval).count()) { - ASSERT(multiplier_ != 0); - tokens_ = max_tokens; - fill_time_ = parent_.time_source_.monotonicTime(); -} - -absl::optional TimerTokenBucket::remainingFillInterval() const { - using namespace std::literals; - - const auto time_after_last_fill = std::chrono::duration_cast( - parent_.time_source_.monotonicTime() - fill_time_.load()); - - // Note that the fill timer may be delayed because other tasks are running on the main thread. - // So it's possible that the time_after_last_fill is greater than fill_interval_. - if (time_after_last_fill >= fill_interval_) { - return {}; - } - - return absl::ToInt64Seconds(absl::FromChrono(fill_interval_) - - absl::Seconds((time_after_last_fill) / 1s)); -} - -bool TimerTokenBucket::consume(double, uint64_t to_consume) { - // Relaxed consistency is used for all operations because we don't care about ordering, just the - // final atomic correctness. - uint64_t expected_tokens = tokens_.load(std::memory_order_relaxed); - do { - // expected_tokens is either initialized above or reloaded during the CAS failure below. - if (expected_tokens < to_consume) { - return false; - } - - // Testing hook. - parent_.synchronizer_.syncPoint("allowed_pre_cas"); - - // Loop while the weak CAS fails trying to subtract tokens from expected. - } while (!tokens_.compare_exchange_weak(expected_tokens, expected_tokens - to_consume, - std::memory_order_relaxed)); - - // We successfully decremented the counter by tokens. - return true; -} - -void TimerTokenBucket::onFillTimer(uint64_t refill_counter, double factor) { - // Descriptors are refilled every Nth timer hit where N is the ratio of the - // descriptor refill interval over the global refill interval. For example, - // if the descriptor refill interval is 150ms and the global refill - // interval is 50ms, this descriptor is refilled every 3rd call. - ASSERT(multiplier_ != 0); - if (refill_counter % multiplier_ != 0) { - return; - } - - const uint64_t tokens_per_fill = std::ceil(tokens_per_fill_ * factor); - - // Relaxed consistency is used for all operations because we don't care about ordering, just the - // final atomic correctness. - uint64_t expected_tokens = tokens_.load(std::memory_order_relaxed); - uint64_t new_tokens_value{}; - do { - // expected_tokens is either initialized above or reloaded during the CAS failure below. - new_tokens_value = std::min(max_tokens_, expected_tokens + tokens_per_fill); - - // Testing hook. - parent_.synchronizer_.syncPoint("on_fill_timer_pre_cas"); - - // Loop while the weak CAS fails trying to update the tokens value. - } while ( - !tokens_.compare_exchange_weak(expected_tokens, new_tokens_value, std::memory_order_relaxed)); - - // Update fill time at last. - fill_time_ = parent_.time_source_.monotonicTime(); -} - -AtomicTokenBucket::AtomicTokenBucket(uint64_t max_tokens, uint64_t tokens_per_fill, - std::chrono::milliseconds fill_interval, - TimeSource& time_source) +RateLimitTokenBucket::RateLimitTokenBucket(uint64_t max_tokens, uint64_t tokens_per_fill, + std::chrono::milliseconds fill_interval, + TimeSource& time_source) : token_bucket_(max_tokens, time_source, // Calculate the fill rate in tokens per second. tokens_per_fill / std::chrono::duration(fill_interval).count()) {} -bool AtomicTokenBucket::consume(double factor, uint64_t to_consume) { +bool RateLimitTokenBucket::consume(double factor, uint64_t to_consume) { ASSERT(!(factor <= 0.0 || factor > 1.0)); auto cb = [tokens = to_consume / factor](double total) { return total < tokens ? 0.0 : tokens; }; return token_bucket_.consume(cb) != 0.0; @@ -175,28 +94,17 @@ LocalRateLimiterImpl::LocalRateLimiterImpl( const Protobuf::RepeatedPtrField< envoy::extensions::common::ratelimit::v3::LocalRateLimitDescriptor>& descriptors, bool always_consume_default_token_bucket, ShareProviderSharedPtr shared_provider) - : fill_timer_(fill_interval > std::chrono::milliseconds(0) - ? dispatcher.createTimer([this] { onFillTimer(); }) - : nullptr), - time_source_(dispatcher.timeSource()), share_provider_(std::move(shared_provider)), - always_consume_default_token_bucket_(always_consume_default_token_bucket), - no_timer_based_rate_limit_token_bucket_(Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.no_timer_based_rate_limit_token_bucket")) { - if (fill_timer_ && fill_interval < std::chrono::milliseconds(50)) { - throw EnvoyException("local rate limit token bucket fill timer must be >= 50ms"); - } - - if (no_timer_based_rate_limit_token_bucket_) { - default_token_bucket_ = std::make_shared(max_tokens, tokens_per_fill, - fill_interval, time_source_); - } else { - default_token_bucket_ = - std::make_shared(max_tokens, tokens_per_fill, fill_interval, 1, *this); - } - - if (fill_timer_ && default_token_bucket_->fillInterval().count() > 0 && - !no_timer_based_rate_limit_token_bucket_) { - fill_timer_->enableTimer(default_token_bucket_->fillInterval()); + : time_source_(dispatcher.timeSource()), share_provider_(std::move(shared_provider)), + always_consume_default_token_bucket_(always_consume_default_token_bucket) { + + // Ignore the default token bucket if fill_interval is 0 because 0 fill_interval means nothing + // and has undefined behavior. + if (fill_interval.count() > 0) { + if (fill_interval < std::chrono::milliseconds(50)) { + throw EnvoyException("local rate limit token bucket fill timer must be >= 50ms"); + } + default_token_bucket_ = std::make_shared(max_tokens, tokens_per_fill, + fill_interval, time_source_); } for (const auto& descriptor : descriptors) { @@ -218,23 +126,10 @@ LocalRateLimiterImpl::LocalRateLimiterImpl( throw EnvoyException("local rate limit descriptor token bucket fill timer must be >= 50ms"); } - if (per_descriptor_fill_interval.count() % fill_interval.count() != 0) { - throw EnvoyException( - "local rate descriptor limit is not a multiple of token bucket fill timer"); - } - // Save the multiplicative factor to control the descriptor refill frequency. - const auto per_descriptor_multiplier = per_descriptor_fill_interval / fill_interval; - - RateLimitTokenBucketSharedPtr per_descriptor_token_bucket; - if (no_timer_based_rate_limit_token_bucket_) { - per_descriptor_token_bucket = std::make_shared( - per_descriptor_max_tokens, per_descriptor_tokens_per_fill, per_descriptor_fill_interval, - time_source_); - } else { - per_descriptor_token_bucket = std::make_shared( - per_descriptor_max_tokens, per_descriptor_tokens_per_fill, per_descriptor_fill_interval, - per_descriptor_multiplier, *this); - } + RateLimitTokenBucketSharedPtr per_descriptor_token_bucket = + std::make_shared(per_descriptor_max_tokens, + per_descriptor_tokens_per_fill, + per_descriptor_fill_interval, time_source_); auto result = descriptors_.emplace(std::move(new_descriptor), std::move(per_descriptor_token_bucket)); @@ -245,29 +140,7 @@ LocalRateLimiterImpl::LocalRateLimiterImpl( } } -LocalRateLimiterImpl::~LocalRateLimiterImpl() { - if (fill_timer_ != nullptr) { - fill_timer_->disableTimer(); - } -} - -void LocalRateLimiterImpl::onFillTimer() { - // Since descriptors tokens are refilled whenever the remainder of dividing refill_counter_ - // by descriptor.multiplier_ is zero and refill_counter_ is initialized to zero, it must be - // incremented before executing the onFillTimerDescriptorHelper() method to prevent all - // descriptors tokens from being refilled at the first time hit, regardless of its fill - // interval configuration. - refill_counter_++; - const double share_factor = - share_provider_ != nullptr ? share_provider_->getTokensShareFactor() : 1.0; - - default_token_bucket_->onFillTimer(refill_counter_, share_factor); - for (const auto& descriptor : descriptors_) { - descriptor.second->onFillTimer(refill_counter_, share_factor); - } - - fill_timer_->enableTimer(default_token_bucket_->fillInterval()); -} +LocalRateLimiterImpl::~LocalRateLimiterImpl() = default; struct MatchResult { std::reference_wrapper token_bucket; @@ -312,6 +185,13 @@ LocalRateLimiterImpl::Result LocalRateLimiterImpl::requestAllowed( // See if the request is forbidden by the default token bucket. if (matched_results.empty() || always_consume_default_token_bucket_) { + if (default_token_bucket_ == nullptr) { + return {true, matched_results.empty() + ? makeOptRefFromPtr(nullptr) + : makeOptRef(matched_results[0].token_bucket.get())}; + } + ASSERT(default_token_bucket_ != nullptr); + if (const bool result = default_token_bucket_->consume(share_factor); !result) { // If the request is forbidden by the default token bucket, return the result and the // default token bucket. diff --git a/source/extensions/filters/common/local_ratelimit/local_ratelimit_impl.h b/source/extensions/filters/common/local_ratelimit/local_ratelimit_impl.h index 0e987ad3fdc48..8c0a0bf97124b 100644 --- a/source/extensions/filters/common/local_ratelimit/local_ratelimit_impl.h +++ b/source/extensions/filters/common/local_ratelimit/local_ratelimit_impl.h @@ -64,70 +64,28 @@ class TokenBucketContext { virtual uint64_t maxTokens() const PURE; virtual uint64_t remainingTokens() const PURE; - virtual absl::optional remainingFillInterval() const PURE; }; -class RateLimitTokenBucket : public TokenBucketContext { -public: - virtual bool consume(double factor = 1.0, uint64_t tokens = 1) PURE; - virtual void onFillTimer(uint64_t refill_counter, double factor = 1.0) PURE; - virtual std::chrono::milliseconds fillInterval() const PURE; - virtual double fillRate() const PURE; -}; -using RateLimitTokenBucketSharedPtr = std::shared_ptr; - class LocalRateLimiterImpl; -// Token bucket that implements based on the periodic timer. -class TimerTokenBucket : public RateLimitTokenBucket { +class RateLimitTokenBucket : public TokenBucketContext { public: - TimerTokenBucket(uint64_t max_tokens, uint64_t tokens_per_fill, - std::chrono::milliseconds fill_interval, uint64_t multiplier, - LocalRateLimiterImpl& parent); + RateLimitTokenBucket(uint64_t max_tokens, uint64_t tokens_per_fill, + std::chrono::milliseconds fill_interval, TimeSource& time_source); // RateLimitTokenBucket - bool consume(double factor = 1.0, uint64_t tokens = 1) override; - void onFillTimer(uint64_t refill_counter, double factor) override; - std::chrono::milliseconds fillInterval() const override { return fill_interval_; } - double fillRate() const override { return fill_rate_; } - uint64_t maxTokens() const override { return max_tokens_; } - uint64_t remainingTokens() const override { return tokens_.load(); } - absl::optional remainingFillInterval() const override; - - // Descriptor refill interval is a multiple of the timer refill interval. - // For example, if the descriptor refill interval is 150ms and the global - // refill interval is 50ms, the value is 3. Every 3rd invocation of - // the global timer, the descriptor is refilled. - const uint64_t multiplier_{}; - LocalRateLimiterImpl& parent_; - std::atomic tokens_{}; - std::atomic fill_time_{}; - - const uint64_t max_tokens_{}; - const uint64_t tokens_per_fill_{}; - const std::chrono::milliseconds fill_interval_{}; - const double fill_rate_{}; -}; + bool consume(double factor = 1.0, uint64_t tokens = 1); + double fillRate() const { return token_bucket_.fillRate(); } -class AtomicTokenBucket : public RateLimitTokenBucket { -public: - AtomicTokenBucket(uint64_t max_tokens, uint64_t tokens_per_fill, - std::chrono::milliseconds fill_interval, TimeSource& time_source); - - // RateLimitTokenBucket - bool consume(double factor = 1.0, uint64_t tokens = 1) override; - void onFillTimer(uint64_t, double) override {} - std::chrono::milliseconds fillInterval() const override { return {}; } - double fillRate() const override { return token_bucket_.fillRate(); } uint64_t maxTokens() const override { return static_cast(token_bucket_.maxTokens()); } uint64_t remainingTokens() const override { return static_cast(token_bucket_.remainingTokens()); } - absl::optional remainingFillInterval() const override { return {}; } private: AtomicTokenBucketImpl token_bucket_; }; +using RateLimitTokenBucketSharedPtr = std::shared_ptr; class LocalRateLimiterImpl { public: @@ -148,24 +106,15 @@ class LocalRateLimiterImpl { Result requestAllowed(absl::Span request_descriptors) const; private: - void onFillTimer(); - RateLimitTokenBucketSharedPtr default_token_bucket_; - const Event::TimerPtr fill_timer_; TimeSource& time_source_; RateLimit::LocalDescriptor::Map descriptors_; - // Refill counter is incremented per each refill timer hit. - uint64_t refill_counter_{0}; ShareProviderSharedPtr share_provider_; mutable Thread::ThreadSynchronizer synchronizer_; // Used for testing only. const bool always_consume_default_token_bucket_{}; - const bool no_timer_based_rate_limit_token_bucket_{}; - - friend class LocalRateLimiterImplTest; - friend class TimerTokenBucket; }; } // namespace LocalRateLimit diff --git a/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc b/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc index bd494e7f89ecd..8af1dd4498072 100644 --- a/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc +++ b/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc @@ -203,11 +203,6 @@ Http::FilterHeadersStatus Filter::encodeHeaders(Http::ResponseHeaderMap& headers headers.addReferenceKey( HttpFilters::Common::RateLimit::XRateLimitHeaders::get().XRateLimitRemaining, token_bucket_context_->remainingTokens()); - const auto reset = token_bucket_context_->remainingFillInterval(); - if (reset.has_value()) { - headers.addReferenceKey( - HttpFilters::Common::RateLimit::XRateLimitHeaders::get().XRateLimitReset, reset.value()); - } } return Http::FilterHeadersStatus::Continue; diff --git a/test/extensions/filters/common/local_ratelimit/local_ratelimit_test.cc b/test/extensions/filters/common/local_ratelimit/local_ratelimit_test.cc index c83b8b5f735ea..0e57f7b078963 100644 --- a/test/extensions/filters/common/local_ratelimit/local_ratelimit_test.cc +++ b/test/extensions/filters/common/local_ratelimit/local_ratelimit_test.cc @@ -113,25 +113,6 @@ class MockShareProvider : public ShareProvider { class LocalRateLimiterImplTest : public testing::Test { public: - void initializeTimer() { - fill_timer_ = new Event::MockTimer(&dispatcher_); - EXPECT_CALL(*fill_timer_, enableTimer(_, nullptr)); - EXPECT_CALL(*fill_timer_, disableTimer()); - } - - void initialize(const std::chrono::milliseconds fill_interval, const uint32_t max_tokens, - const uint32_t tokens_per_fill, ShareProviderSharedPtr share_provider = nullptr) { - TestScopedRuntime runtime; - runtime.mergeValues( - {{"envoy.reloadable_features.no_timer_based_rate_limit_token_bucket", "false"}}); - - initializeTimer(); - - rate_limiter_ = - std::make_shared(fill_interval, max_tokens, tokens_per_fill, - dispatcher_, descriptors_, true, share_provider); - } - void initializeWithAtomicTokenBucket(const std::chrono::milliseconds fill_interval, const uint32_t max_tokens, const uint32_t tokens_per_fill, ShareProviderSharedPtr share_provider = nullptr) { @@ -140,14 +121,12 @@ class LocalRateLimiterImplTest : public testing::Test { dispatcher_, descriptors_, true, share_provider); } - Thread::ThreadSynchronizer& synchronizer() { return rate_limiter_->synchronizer_; } Envoy::Protobuf::RepeatedPtrField< envoy::extensions::common::ratelimit::v3::LocalRateLimitDescriptor> descriptors_; std::vector route_descriptors_; NiceMock dispatcher_; - Event::MockTimer* fill_timer_{}; std::shared_ptr rate_limiter_; }; @@ -158,222 +137,8 @@ TEST_F(LocalRateLimiterImplTest, TooFastFillRate) { EnvoyException, "local rate limit token bucket fill timer must be >= 50ms"); } -// Verify various token bucket CAS edge cases. -TEST_F(LocalRateLimiterImplTest, CasEdgeCases) { - // This tests the case in which an allowed check races with the fill timer. - { - initialize(std::chrono::milliseconds(50), 1, 1); - - synchronizer().enable(); - - // Start a thread and start the fill callback. This will wait pre-CAS. - synchronizer().waitOn("on_fill_timer_pre_cas"); - std::thread t1([&] { - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(50), nullptr)); - fill_timer_->invokeCallback(); - }); - // Wait until the thread is actually waiting. - synchronizer().barrierOn("on_fill_timer_pre_cas"); - - // This should succeed. - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - - // Now signal the thread to continue which should cause a CAS failure and the loop to repeat. - synchronizer().signal("on_fill_timer_pre_cas"); - t1.join(); - - // 1 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - } - - // This tests the case in which two allowed checks race. - { - initialize(std::chrono::milliseconds(200), 1, 1); - - synchronizer().enable(); - - // Start a thread and see if we are under limit. This will wait pre-CAS. - synchronizer().waitOn("allowed_pre_cas"); - std::thread t1( - [&] { EXPECT_FALSE(rate_limiter_->requestAllowed(route_descriptors_).allowed); }); - // Wait until the thread is actually waiting. - synchronizer().barrierOn("allowed_pre_cas"); - - // Consume a token on this thread, which should cause the CAS to fail on the other thread. - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - synchronizer().signal("allowed_pre_cas"); - t1.join(); - } -} - -// Verify token bucket functionality with a single token. -TEST_F(LocalRateLimiterImplTest, TokenBucket) { - initialize(std::chrono::milliseconds(200), 1, 1); - - // 1 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - - // 0 -> 1 tokens - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(200), nullptr)); - fill_timer_->invokeCallback(); - - // 1 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - - // 0 -> 1 tokens - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(200), nullptr)); - fill_timer_->invokeCallback(); - - // 1 -> 1 tokens - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(200), nullptr)); - fill_timer_->invokeCallback(); - - // 1 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(route_descriptors_).allowed); -} - -// Verify token bucket functionality with max tokens and tokens per fill > 1. -TEST_F(LocalRateLimiterImplTest, TokenBucketMultipleTokensPerFill) { - initialize(std::chrono::milliseconds(200), 2, 2); - - // 2 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - - // 0 -> 2 tokens - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(200), nullptr)); - fill_timer_->invokeCallback(); - - // 2 -> 1 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - - // 1 -> 2 tokens - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(200), nullptr)); - fill_timer_->invokeCallback(); - - // 2 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(route_descriptors_).allowed); -} - -// Verify token bucket functionality with max tokens and tokens per fill > 1 and -// share provider is used. -TEST_F(LocalRateLimiterImplTest, TokenBucketMultipleTokensPerFillWithShareProvider) { - auto share_provider = std::make_shared(); - EXPECT_CALL(*share_provider, getTokensShareFactor()) - .WillRepeatedly(testing::Invoke([]() -> double { return 0.5; })); - - // Final tokens per fill is 2/2 = 1. - initialize(std::chrono::milliseconds(200), 2, 2, share_provider); - - // The limiter will be initialized with max tokens and it will not be shared. - // So, the initial tokens is 2. - // 2 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - - // The tokens per fill will be handled by the share provider and it will be 1. - // 0 -> 1 tokens - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(200), nullptr)); - fill_timer_->invokeCallback(); - - // 1 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - - // 0 -> 1 tokens - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(200), nullptr)); - fill_timer_->invokeCallback(); - - // 1 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(route_descriptors_).allowed); -} - -// Verify token bucket functionality with max tokens > tokens per fill. -TEST_F(LocalRateLimiterImplTest, TokenBucketMaxTokensGreaterThanTokensPerFill) { - initialize(std::chrono::milliseconds(200), 2, 1); - - // 2 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - - // 0 -> 1 tokens - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(200), nullptr)); - fill_timer_->invokeCallback(); - - // 1 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(route_descriptors_).allowed); -} - -// Verify token bucket status of max tokens, remaining tokens and remaining fill interval. -TEST_F(LocalRateLimiterImplTest, TokenBucketStatus) { - initialize(std::chrono::milliseconds(3000), 2, 2); - - // 2 -> 1 tokens - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(3000), nullptr)); - auto rate_limit_result = rate_limiter_->requestAllowed(route_descriptors_); - EXPECT_TRUE(rate_limit_result.allowed); - - EXPECT_EQ(rate_limit_result.token_bucket_context->maxTokens(), 2); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingTokens(), 1); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingFillInterval().value(), 3.0); - - // 1 -> 0 tokens - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(1000), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - EXPECT_TRUE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - - // Note that the route descriptors are not changed so we can reuse the same token bucket context. - EXPECT_EQ(rate_limit_result.token_bucket_context->maxTokens(), 2); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingTokens(), 0); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingFillInterval().value(), 2.0); - - // 0 -> 0 tokens - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(1000), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - EXPECT_FALSE(rate_limiter_->requestAllowed(route_descriptors_).allowed); - - // Note that the route descriptors are not changed so we can reuse the same token bucket context. - EXPECT_EQ(rate_limit_result.token_bucket_context->maxTokens(), 2); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingTokens(), 0); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingFillInterval().value(), 1.0); - - // 0 -> 2 tokens - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(1000), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - fill_timer_->invokeCallback(); - - // Note that the route descriptors are not changed so we can reuse the same token bucket context. - EXPECT_EQ(rate_limit_result.token_bucket_context->maxTokens(), 2); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingTokens(), 2); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingFillInterval().value(), 3.0); -} - class LocalRateLimiterDescriptorImplTest : public LocalRateLimiterImplTest { public: - void initializeWithDescriptor(const std::chrono::milliseconds fill_interval, - const uint32_t max_tokens, const uint32_t tokens_per_fill) { - - TestScopedRuntime runtime; - runtime.mergeValues( - {{"envoy.reloadable_features.no_timer_based_rate_limit_token_bucket", "false"}}); - - initializeTimer(); - - rate_limiter_ = std::make_shared( - fill_interval, max_tokens, tokens_per_fill, dispatcher_, descriptors_); - } - void initializeWithAtomicTokenBucketDescriptor(const std::chrono::milliseconds fill_interval, const uint32_t max_tokens, const uint32_t tokens_per_fill) { @@ -406,18 +171,9 @@ class LocalRateLimiterDescriptorImplTest : public LocalRateLimiterImplTest { // Default token bucket std::vector descriptor_{{{{"foo2", "bar2"}}}}; std::vector descriptor2_{{{{"hello", "world"}, {"foo", "bar"}}}}; + std::vector no_match_descriptor_{{{{"no_match", "no_match"}}}}; }; -// Verify descriptor rate limit time interval is multiple of token bucket fill interval. -TEST_F(LocalRateLimiterDescriptorImplTest, DescriptorRateLimitDivisibleByTokenFillInterval) { - TestUtility::loadFromYaml(fmt::format(single_descriptor_config_yaml, 10, 10, "60s"), - *descriptors_.Add()); - - EXPECT_THROW_WITH_MESSAGE( - LocalRateLimiterImpl(std::chrono::milliseconds(59000), 2, 1, dispatcher_, descriptors_), - EnvoyException, "local rate descriptor limit is not a multiple of token bucket fill timer"); -} - // Verify descriptor rate limit time with small fill interval is rejected. TEST_F(LocalRateLimiterDescriptorImplTest, DescriptorRateLimitSmallFillInterval) { // Set fill interval to 10 milliseconds. @@ -446,308 +202,6 @@ TEST_F(LocalRateLimiterDescriptorImplTest, DescriptorRateLimitNoExceptionWithout LocalRateLimiterImpl(std::chrono::milliseconds(59000), 2, 1, dispatcher_, descriptors_)); } -// Verify various token bucket CAS edge cases for descriptors. -TEST_F(LocalRateLimiterDescriptorImplTest, CasEdgeCasesDescriptor) { - // This tests the case in which an allowed check races with the fill timer. - { - TestUtility::loadFromYaml(fmt::format(single_descriptor_config_yaml, 1, 1, "0.1s"), - *descriptors_.Add()); - initializeWithDescriptor(std::chrono::milliseconds(50), 1, 1); - - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(50), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(50), nullptr)); - fill_timer_->invokeCallback(); - - synchronizer().enable(); - - // Start a thread and start the fill callback. This will wait pre-CAS. - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(50), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - synchronizer().waitOn("on_fill_timer_pre_cas"); - std::thread t1([&] { - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(50), nullptr)); - fill_timer_->invokeCallback(); - }); - // Wait until the thread is actually waiting. - synchronizer().barrierOn("on_fill_timer_pre_cas"); - - // This should succeed. - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); - - // Now signal the thread to continue which should cause a CAS failure and the loop to repeat. - synchronizer().signal("on_fill_timer_pre_cas"); - t1.join(); - - // 1 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor_).allowed); - } - - // This tests the case in which two allowed checks race. - { - initializeWithDescriptor(std::chrono::milliseconds(50), 1, 1); - - synchronizer().enable(); - - // Start a thread and see if we are under limit. This will wait pre-CAS. - synchronizer().waitOn("allowed_pre_cas"); - std::thread t1([&] { EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor_).allowed); }); - // Wait until the thread is actually waiting. - synchronizer().barrierOn("allowed_pre_cas"); - - // Consume a token on this thread, which should cause the CAS to fail on the other thread. - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); - synchronizer().signal("allowed_pre_cas"); - t1.join(); - } -} - -TEST_F(LocalRateLimiterDescriptorImplTest, TokenBucketDescriptor2) { - TestUtility::loadFromYaml(fmt::format(single_descriptor_config_yaml, 1, 1, "0.1s"), - *descriptors_.Add()); - initializeWithDescriptor(std::chrono::milliseconds(50), 1, 1); - - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor_).allowed); - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(100), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); -} - -// Verify token bucket functionality with a single token. -TEST_F(LocalRateLimiterDescriptorImplTest, TokenBucketDescriptor) { - TestUtility::loadFromYaml(fmt::format(single_descriptor_config_yaml, 1, 1, "0.1s"), - *descriptors_.Add()); - initializeWithDescriptor(std::chrono::milliseconds(50), 1, 1); - - // 1 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor_).allowed); - - // 0 -> 1 tokens - for (int i = 0; i < 2; i++) { - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(50), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(50), nullptr)); - fill_timer_->invokeCallback(); - } - - // 1 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor_).allowed); - - // 0 -> 1 tokens - for (int i = 0; i < 2; i++) { - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(50), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(50), nullptr)); - fill_timer_->invokeCallback(); - } - - // 1 -> 1 tokens - for (int i = 0; i < 2; i++) { - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(50), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(50), nullptr)); - fill_timer_->invokeCallback(); - } - - // 1 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor_).allowed); -} - -// Verify token bucket functionality with request per unit > 1. -TEST_F(LocalRateLimiterDescriptorImplTest, TokenBucketMultipleTokensPerFillDescriptor) { - TestUtility::loadFromYaml(fmt::format(single_descriptor_config_yaml, 2, 2, "0.1s"), - *descriptors_.Add()); - initializeWithDescriptor(std::chrono::milliseconds(50), 2, 2); - - // 2 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor_).allowed); - - // 0 -> 2 tokens - for (int i = 0; i < 2; i++) { - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(50), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(50), nullptr)); - fill_timer_->invokeCallback(); - } - - // 2 -> 1 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); - - // 1 -> 2 tokens - for (int i = 0; i < 2; i++) { - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(50), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(50), nullptr)); - fill_timer_->invokeCallback(); - } - - // 2 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor_).allowed); -} - -// Verify token bucket functionality with multiple descriptors. -TEST_F(LocalRateLimiterDescriptorImplTest, TokenBucketDifferentDescriptorDifferentRateLimits) { - TestUtility::loadFromYaml(fmt::format(multiple_descriptor_config_yaml, 1, 1, "1s"), - *descriptors_.Add()); - TestUtility::loadFromYaml(fmt::format(single_descriptor_config_yaml, 1, 1, "2s"), - *descriptors_.Add()); - initializeWithDescriptor(std::chrono::milliseconds(1000), 3, 1); - - // 1 -> 0 tokens for descriptor_ and descriptor2_ - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor2_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor2_).allowed); - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor_).allowed); - - // 0 -> 1 tokens for descriptor2_ - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(1000), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(1000), nullptr)); - fill_timer_->invokeCallback(); - - // 1 -> 0 tokens for descriptor2_ and 0 only for descriptor_ - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor2_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor2_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor_).allowed); -} - -// Verify token bucket functionality with multiple descriptors sorted. -TEST_F(LocalRateLimiterDescriptorImplTest, - TokenBucketDifferentDescriptorDifferentRateLimitsSorted) { - TestUtility::loadFromYaml(fmt::format(multiple_descriptor_config_yaml, 1, 1, "1s"), - *descriptors_.Add()); - TestUtility::loadFromYaml(fmt::format(single_descriptor_config_yaml, 2, 2, "1s"), - *descriptors_.Add()); - initializeWithDescriptor(std::chrono::milliseconds(50), 3, 3); - std::vector descriptors{{{{"hello", "world"}, {"foo", "bar"}}}, - {{{"foo2", "bar2"}}}}; - - // Descriptors are sorted as descriptor2 < descriptor < global - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptors).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptors).allowed); - // Request limited by descriptor2 will not consume tokens from descriptor. - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); -} - -// Verify token bucket status of max tokens, remaining tokens and remaining fill interval. -TEST_F(LocalRateLimiterDescriptorImplTest, TokenBucketDescriptorStatus) { - TestUtility::loadFromYaml(fmt::format(single_descriptor_config_yaml, 2, 2, "3s"), - *descriptors_.Add()); - initializeWithDescriptor(std::chrono::milliseconds(1000), 2, 2); - - // 2 -> 1 tokens - auto rate_limit_result = rate_limiter_->requestAllowed(descriptor_); - - EXPECT_TRUE(rate_limit_result.allowed); - - // Note that the route descriptors are not changed so we can reuse the same token bucket context. - EXPECT_EQ(rate_limit_result.token_bucket_context->maxTokens(), 2); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingTokens(), 1); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingFillInterval().value(), 3.0); - - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(1000), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(1000), nullptr)); - fill_timer_->invokeCallback(); - - // 1 -> 0 tokens - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); - // Note that the route descriptors are not changed so we can reuse the same token bucket context. - EXPECT_EQ(rate_limit_result.token_bucket_context->maxTokens(), 2); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingTokens(), 0); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingFillInterval().value(), 2.0); - - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(1000), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(1000), nullptr)); - fill_timer_->invokeCallback(); - - // 0 -> 0 tokens - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor_).allowed); - // Note that the route descriptors are not changed so we can reuse the same token bucket context. - EXPECT_EQ(rate_limit_result.token_bucket_context->maxTokens(), 2); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingTokens(), 0); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingFillInterval().value(), 1.0); - - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(1000), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(1000), nullptr)); - fill_timer_->invokeCallback(); - - // 0 -> 2 tokens - // Note that the route descriptors are not changed so we can reuse the same token bucket context. - EXPECT_EQ(rate_limit_result.token_bucket_context->maxTokens(), 2); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingTokens(), 2); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingFillInterval().value(), 3.0); -} - -// Verify token bucket status of max tokens, remaining tokens and remaining fill interval with -// multiple descriptors. -TEST_F(LocalRateLimiterDescriptorImplTest, TokenBucketDifferentDescriptorStatus) { - TestUtility::loadFromYaml(fmt::format(multiple_descriptor_config_yaml, 1, 1, "1s"), - *descriptors_.Add()); - TestUtility::loadFromYaml(fmt::format(single_descriptor_config_yaml, 2, 2, "3s"), - *descriptors_.Add()); - initializeWithDescriptor(std::chrono::milliseconds(1000), 20, 20); - - // 2 -> 1 tokens for descriptor_ - auto rate_limit_result = rate_limiter_->requestAllowed(descriptor_); - - EXPECT_TRUE(rate_limit_result.allowed); - EXPECT_EQ(rate_limit_result.token_bucket_context->maxTokens(), 2); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingTokens(), 1); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingFillInterval().value(), 3); - - // 1 -> 0 tokens for descriptor_ - EXPECT_TRUE(rate_limiter_->requestAllowed(descriptor_).allowed); - EXPECT_EQ(rate_limit_result.token_bucket_context->maxTokens(), 2); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingTokens(), 0); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingFillInterval().value(), 3); - - // 1 -> 0 tokens for descriptor2_ - auto rate_limit_result2 = rate_limiter_->requestAllowed(descriptor2_); - EXPECT_TRUE(rate_limit_result2.allowed); - - EXPECT_EQ(rate_limit_result2.token_bucket_context->maxTokens(), 1); - EXPECT_EQ(rate_limit_result2.token_bucket_context->remainingTokens(), 0); - EXPECT_EQ(rate_limit_result2.token_bucket_context->remainingFillInterval().value(), 1); - - // 0 -> 0 tokens for descriptor_ and descriptor2_ - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor2_).allowed); - EXPECT_FALSE(rate_limiter_->requestAllowed(descriptor_).allowed); - - // 0 -> 1 tokens for descriptor2_ - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(1000), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(1000), nullptr)); - fill_timer_->invokeCallback(); - - EXPECT_EQ(rate_limit_result2.token_bucket_context->maxTokens(), 1); - EXPECT_EQ(rate_limit_result2.token_bucket_context->remainingTokens(), 1); - EXPECT_EQ(rate_limit_result2.token_bucket_context->remainingFillInterval().value(), 1); - - // 0 -> 2 tokens for descriptor_ - for (int i = 0; i < 2; i++) { - dispatcher_.globalTimeSystem().advanceTimeAndRun(std::chrono::milliseconds(1000), dispatcher_, - Envoy::Event::Dispatcher::RunType::NonBlock); - EXPECT_CALL(*fill_timer_, enableTimer(std::chrono::milliseconds(1000), nullptr)); - fill_timer_->invokeCallback(); - } - - EXPECT_EQ(rate_limit_result.token_bucket_context->maxTokens(), 2); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingTokens(), 2); - EXPECT_EQ(rate_limit_result.token_bucket_context->remainingFillInterval().value(), 3.0); -} - // Verify token bucket functionality with a single token. TEST_F(LocalRateLimiterImplTest, AtomicTokenBucket) { initializeWithAtomicTokenBucket(std::chrono::milliseconds(200), 1, 1); @@ -1079,6 +533,27 @@ TEST_F(LocalRateLimiterDescriptorImplTest, AtomicTokenBucketDescriptorStatus) { EXPECT_EQ(rate_limit_result.token_bucket_context->remainingTokens(), 2); } +// Verify null default token bucket. +TEST_F(LocalRateLimiterDescriptorImplTest, NullDefaultTokenBucket) { + TestUtility::loadFromYaml(fmt::format(single_descriptor_config_yaml, 2, 2, "3s"), + *descriptors_.Add()); + initializeWithAtomicTokenBucketDescriptor(std::chrono::milliseconds(0), 0, 0); + + // 2 -> 1 tokens + auto rate_limit_result = rate_limiter_->requestAllowed(descriptor_); + + EXPECT_TRUE(rate_limit_result.allowed); + + // Note that the route descriptors are not changed so we can reuse the same token bucket context. + EXPECT_EQ(rate_limit_result.token_bucket_context->maxTokens(), 2); + EXPECT_EQ(rate_limit_result.token_bucket_context->remainingTokens(), 1); + + // Not match any descriptor and default token bucket is null. + auto no_match_result = rate_limiter_->requestAllowed(no_match_descriptor_); + EXPECT_TRUE(no_match_result.allowed); + EXPECT_FALSE(no_match_result.token_bucket_context.has_value()); +} + } // Namespace LocalRateLimit } // namespace Common } // namespace Filters diff --git a/test/extensions/filters/http/local_ratelimit/config_test.cc b/test/extensions/filters/http/local_ratelimit/config_test.cc index 444d7f3f88abb..b54b17ffc3c7a 100644 --- a/test/extensions/filters/http/local_ratelimit/config_test.cc +++ b/test/extensions/filters/http/local_ratelimit/config_test.cc @@ -23,7 +23,6 @@ stat_prefix: test NiceMock context; - EXPECT_CALL(context.server_factory_context_.dispatcher_, createTimer_(_)).Times(0); auto callback = factory.createFilterFactoryFromProto(*proto_config, "stats", context).value(); Http::MockFilterChainFactoryCallbacks filter_callback; EXPECT_CALL(filter_callback, addStreamFilter(_)); @@ -60,7 +59,6 @@ stat_prefix: test NiceMock context; - EXPECT_CALL(context.dispatcher_, createTimer_(_)); const auto route_config = factory .createRouteSpecificFilterConfig(*proto_config, context, @@ -85,7 +83,6 @@ stat_prefix: test NiceMock context; - EXPECT_CALL(context.dispatcher_, createTimer_(_)); const auto route_config = factory .createRouteSpecificFilterConfig(*proto_config, context, @@ -128,7 +125,6 @@ stat_prefix: test NiceMock context; - EXPECT_CALL(context.dispatcher_, createTimer_(_)); EXPECT_THROW(factory .createRouteSpecificFilterConfig(*proto_config, context, ProtobufMessage::getNullValidationVisitor()) @@ -175,7 +171,6 @@ stat_prefix: test NiceMock context; - EXPECT_CALL(context.dispatcher_, createTimer_(_)).Times(0); EXPECT_THROW(factory .createRouteSpecificFilterConfig(*proto_config, context, ProtobufMessage::getNullValidationVisitor()) @@ -230,7 +225,6 @@ stat_prefix: test NiceMock context; - EXPECT_CALL(context.dispatcher_, createTimer_(_)); const auto route_config = factory .createRouteSpecificFilterConfig(*proto_config, context, @@ -240,61 +234,6 @@ stat_prefix: test EXPECT_TRUE(config->requestAllowed({}).allowed); } -TEST(Factory, RouteSpecificFilterConfigWithDescriptorsTimerNotDivisible) { - const std::string config_yaml = R"( -stat_prefix: test -token_bucket: - max_tokens: 1 - tokens_per_fill: 1 - fill_interval: 100s -filter_enabled: - runtime_key: test_enabled - default_value: - numerator: 100 - denominator: HUNDRED -filter_enforced: - runtime_key: test_enforced - default_value: - numerator: 100 - denominator: HUNDRED -response_headers_to_add: - - append_action: OVERWRITE_IF_EXISTS_OR_ADD - header: - key: x-test-rate-limit - value: 'true' -descriptors: -- entries: - - key: hello - value: world - - key: foo - value: bar - token_bucket: - max_tokens: 10 - tokens_per_fill: 10 - fill_interval: 1s -- entries: - - key: foo2 - value: bar2 - token_bucket: - max_tokens: 100 - tokens_per_fill: 100 - fill_interval: 86400s - )"; - - LocalRateLimitFilterConfig factory; - ProtobufTypes::MessagePtr proto_config = factory.createEmptyRouteConfigProto(); - TestUtility::loadFromYaml(config_yaml, *proto_config); - - NiceMock context; - - EXPECT_CALL(context.dispatcher_, createTimer_(_)); - EXPECT_THROW(factory - .createRouteSpecificFilterConfig(*proto_config, context, - ProtobufMessage::getNullValidationVisitor()) - .value(), - EnvoyException); -} - TEST(Factory, NonexistingHeaderFormatter) { const std::string config_yaml = R"( stat_prefix: test @@ -469,7 +408,6 @@ local_cluster_rate_limit: {} const auto* local_cluster = context.cluster_manager_.active_clusters_.at("local_cluster").get(); EXPECT_CALL(*local_cluster, prioritySet()).WillOnce(ReturnRef(priority_set)); - EXPECT_CALL(context.dispatcher_, createTimer_(_)); EXPECT_TRUE(factory .createRouteSpecificFilterConfig(*proto_config, context, ProtobufMessage::getNullValidationVisitor()) diff --git a/test/extensions/filters/http/local_ratelimit/filter_test.cc b/test/extensions/filters/http/local_ratelimit/filter_test.cc index 1801d4ee6f370..d70b00b71dd8a 100644 --- a/test/extensions/filters/http/local_ratelimit/filter_test.cc +++ b/test/extensions/filters/http/local_ratelimit/filter_test.cc @@ -312,33 +312,6 @@ TEST_F(FilterTest, RequestRateLimitedXRateLimitHeaders) { EXPECT_EQ(1U, findCounter("test.http_local_rate_limit.rate_limited")); } -TEST_F(FilterTest, RequestRateLimitedXRateLimitHeadersWithTimerBasedTokenBucket) { - TestScopedRuntime runtime; - runtime.mergeValues( - {{"envoy.reloadable_features.no_timer_based_rate_limit_token_bucket", "false"}}); - - setup(fmt::format(config_yaml, "false", "1", "false", "DRAFT_VERSION_03")); - - auto request_headers = Http::TestRequestHeaderMapImpl(); - auto response_headers = Http::TestResponseHeaderMapImpl(); - - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->encodeHeaders(response_headers, false)); - EXPECT_EQ("1", response_headers.get_("x-ratelimit-limit")); - EXPECT_EQ("0", response_headers.get_("x-ratelimit-remaining")); - EXPECT_EQ("1000", response_headers.get_("x-ratelimit-reset")); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_2_->decodeHeaders(request_headers, false)); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_2_->encodeHeaders(response_headers, false)); - EXPECT_EQ("1", response_headers.get_("x-ratelimit-limit")); - EXPECT_EQ("0", response_headers.get_("x-ratelimit-remaining")); - EXPECT_EQ("1000", response_headers.get_("x-ratelimit-reset")); - EXPECT_EQ(2U, findCounter("test.http_local_rate_limit.enabled")); - EXPECT_EQ(1U, findCounter("test.http_local_rate_limit.enforced")); - EXPECT_EQ(1U, findCounter("test.http_local_rate_limit.ok")); - EXPECT_EQ(1U, findCounter("test.http_local_rate_limit.rate_limited")); -} - TEST_F(FilterTest, RequestRateLimitedXRateLimitHeadersWithoutRunningDecodeHeaders) { setup(fmt::format(config_yaml, "false", "1", "false", "DRAFT_VERSION_03")); @@ -822,34 +795,6 @@ TEST_F(DescriptorFilterTest, RouteDescriptorRequestRatelimitedWithoutXRateLimitH EXPECT_EQ(1U, findCounter("test.http_local_rate_limit.rate_limited")); } -TEST_F(DescriptorFilterTest, - RouteDescriptorRequestRatelimitedXRateLimitHeadersWithTimerTokenBucket) { - TestScopedRuntime runtime; - runtime.mergeValues( - {{"envoy.reloadable_features.no_timer_based_rate_limit_token_bucket", "false"}}); - - setUpTest(fmt::format(descriptor_config_yaml, "0", "DRAFT_VERSION_03", "0", "0")); - - EXPECT_CALL(decoder_callbacks_.route_->route_entry_.rate_limit_policy_, - getApplicableRateLimit(0)); - - EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _)) - .WillOnce(testing::SetArgReferee<0>(descriptor_)); - - auto request_headers = Http::TestRequestHeaderMapImpl(); - auto response_headers = Http::TestResponseHeaderMapImpl(); - - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, - filter_->decodeHeaders(request_headers, false)); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->encodeHeaders(response_headers, false)); - EXPECT_EQ("0", response_headers.get_("x-ratelimit-limit")); - EXPECT_EQ("0", response_headers.get_("x-ratelimit-remaining")); - EXPECT_EQ("60", response_headers.get_("x-ratelimit-reset")); - EXPECT_EQ(1U, findCounter("test.http_local_rate_limit.enabled")); - EXPECT_EQ(1U, findCounter("test.http_local_rate_limit.enforced")); - EXPECT_EQ(1U, findCounter("test.http_local_rate_limit.rate_limited")); -} - TEST_F(DescriptorFilterTest, NoVHRateLimitOption) { setUpTest(fmt::format(descriptor_config_yaml, "1", "\"OFF\"", "1", "0")); diff --git a/test/extensions/filters/network/local_ratelimit/local_ratelimit_fuzz_test.cc b/test/extensions/filters/network/local_ratelimit/local_ratelimit_fuzz_test.cc index 45bcd52280972..087b0b40497ea 100644 --- a/test/extensions/filters/network/local_ratelimit/local_ratelimit_fuzz_test.cc +++ b/test/extensions/filters/network/local_ratelimit/local_ratelimit_fuzz_test.cc @@ -63,7 +63,6 @@ DEFINE_PROTO_FUZZER( Stats::IsolatedStoreImpl stats_store; Singleton::ManagerImpl singleton_manager; static NiceMock runtime; - Event::MockTimer* fill_timer = new Event::MockTimer(&dispatcher); envoy::extensions::filters::network::local_ratelimit::v3::LocalRateLimit proto_config = input.config(); ConfigSharedPtr config = nullptr; @@ -93,8 +92,6 @@ DEFINE_PROTO_FUZZER( break; } case envoy::extensions::filters::network::local_ratelimit::Action::kRefill: { - EXPECT_CALL(*fill_timer, enableTimer(fill_interval, nullptr)); - fill_timer->invokeCallback(); break; } default: