Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -149,5 +149,8 @@ new_features:
added :ref:`enable_ja4_fingerprinting
<envoy_v3_api_field_extensions.filters.listener.tls_inspector.v3.TlsInspector.enable_ja4_fingerprinting>` to create
a JA4 fingerprint hash from the Client Hello message.
- area: local_ratelimit
change: |
``local_ratelimit`` will return ``x-ratelimit-reset`` header when the rate limit is exceeded.

deprecated:
9 changes: 9 additions & 0 deletions source/common/common/token_bucket_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,13 @@ double AtomicTokenBucketImpl::timeNowInSeconds() const {
return std::chrono::duration<double>(time_source_.monotonicTime().time_since_epoch()).count();
}

std::chrono::milliseconds AtomicTokenBucketImpl::nextTokenAvailable() const {
// If there are tokens available, return immediately.
if (remainingTokens() >= 1) {
return std::chrono::milliseconds(0);
}

return std::chrono::milliseconds(static_cast<uint64_t>(std::ceil((1 / fill_rate_) * 1000)));
Comment thread
zirain marked this conversation as resolved.
Outdated
}
Comment thread
zirain marked this conversation as resolved.
Comment on lines +107 to +116

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it's will be a very very very trival case, but it's possible when we check remainingTokens(), the remaining tokens are less then 1. Then, when we calculate the next available token, time passes, the 1 / fill_rate_ - (current_time - last_time) may return a minus value.

Comment on lines +107 to +116

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::chrono::milliseconds AtomicTokenBucketImpl::nextTokenAvailable() const {
// If there are tokens available, return immediately.
if (remainingTokens() >= 1) {
return std::chrono::milliseconds(0);
}
// Calculate time since the last fill.
double current_time = timeNowInSeconds();
double last_time = time_in_seconds_.load();
return std::chrono::milliseconds(
static_cast<uint64_t>(((1 / fill_rate_ - (current_time - last_time)) * 1000)));
}
std::chrono::milliseconds AtomicTokenBucketImpl::nextTokenAvailable() const {
// If there are tokens available, return immediately.
const double remaining_tokens = remainingTokens();
if (remaining_tokens >= 1) {
return std::chrono::milliseconds(0);
}
// Calculate time since the last fill.
return std::chrono::milliseconds(
static_cast<uint64_t>(((1 - remaining_tokens) / fill_rate_) * 1000));
}


} // namespace Envoy
6 changes: 6 additions & 0 deletions source/common/common/token_bucket_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,12 @@ class AtomicTokenBucketImpl {
*/
double remainingTokens() const;

/**
* Get the time to next token available. This is a snapshot and may change after the call.
* @return the time to next token available.
*/
std::chrono::milliseconds nextTokenAvailable() const;

private:
double timeNowInSeconds() const;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ class TokenBucketContext {

virtual uint64_t maxTokens() const PURE;
virtual uint64_t remainingTokens() const PURE;
virtual uint64_t resetSeconds() const PURE;
};

class RateLimitTokenBucket : public TokenBucketContext,
Expand All @@ -122,6 +123,9 @@ class RateLimitTokenBucket : public TokenBucketContext,
uint64_t remainingTokens() const override {
return static_cast<uint64_t>(token_bucket_.remainingTokens());
}
uint64_t resetSeconds() const override {
return static_cast<uint64_t>(token_bucket_.nextTokenAvailable().count() / 1000);
}

private:
AtomicTokenBucketImpl token_bucket_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,11 @@ Http::FilterHeadersStatus Filter::encodeHeaders(Http::ResponseHeaderMap& headers
headers.addReferenceKey(
HttpFilters::Common::RateLimit::XRateLimitHeaders::get().XRateLimitRemaining,
token_bucket_context_->remainingTokens());
if (token_bucket_context_->remainingTokens() == 0) {
headers.addReferenceKey(
HttpFilters::Common::RateLimit::XRateLimitHeaders::get().XRateLimitReset,
token_bucket_context_->resetSeconds());
}
Comment thread
zirain marked this conversation as resolved.
Outdated
}

return Http::FilterHeadersStatus::Continue;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "source/extensions/filters/common/local_ratelimit/local_ratelimit_impl.h"
#include "source/extensions/filters/http/common/ratelimit_headers.h"

#include "test/integration/http_protocol_integration.h"
#include "test/test_common/test_runtime.h"
Expand Down Expand Up @@ -203,6 +204,35 @@ name: envoy.filters.http.local_ratelimit
local_rate_limit_per_downstream_connection: {}
)EOF";

static constexpr absl::string_view limit_header_filter_config_ =
R"EOF(
name: envoy.filters.http.local_ratelimit
typed_config:
"@type": type.googleapis.com/envoy.extensions.filters.http.local_ratelimit.v3.LocalRateLimit
stat_prefix: http_local_rate_limiter
enableXRatelimitHeaders: DRAFT_VERSION_03
token_bucket:
max_tokens: 1
tokens_per_fill: 1
fill_interval: 1000s
filter_enabled:
runtime_key: local_rate_limit_enabled
default_value:
numerator: 100
denominator: HUNDRED
filter_enforced:
runtime_key: local_rate_limit_enforced
default_value:
numerator: 100
denominator: HUNDRED
response_headers_to_add:
- append_action: OVERWRITE_IF_EXISTS_OR_ADD
header:
key: x-local-rate-limit
value: 'true'
local_rate_limit_per_downstream_connection: {}
)EOF";

static constexpr absl::string_view filter_config_with_blank_value_descriptor_ =
R"EOF(
name: envoy.filters.http.local_ratelimit
Expand Down Expand Up @@ -489,6 +519,46 @@ TEST_P(LocalRateLimitFilterIntegrationTest, DenyRequestWithinSameConnection) {
EXPECT_EQ(18, response->body().size());
}

TEST_P(LocalRateLimitFilterIntegrationTest, LimitHeaderTest) {
Comment thread
zirain marked this conversation as resolved.
Outdated
initializeFilter(fmt::format(limit_header_filter_config_, "true"));

codec_client_ = makeHttpConnection(lookupPort("http"));
auto response = codec_client_->makeRequestWithBody(default_request_headers_, 0);

waitForNextUpstreamRequest();
upstream_request_->encodeHeaders(default_response_headers_, 1);

ASSERT_TRUE(response->waitForEndStream());

EXPECT_TRUE(upstream_request_->complete());
EXPECT_EQ(0U, upstream_request_->bodyLength());
EXPECT_TRUE(response->complete());
EXPECT_EQ("200", response->headers().getStatusValue());
EXPECT_EQ(0, response->body().size());
EXPECT_THAT(
response->headers(),
Http::HeaderValueOf(
Extensions::HttpFilters::Common::RateLimit::XRateLimitHeaders::get().XRateLimitLimit,
"1"));

response = codec_client_->makeRequestWithBody(default_request_headers_, 0);

ASSERT_TRUE(response->waitForEndStream());
EXPECT_TRUE(response->complete());
EXPECT_EQ("429", response->headers().getStatusValue());
EXPECT_EQ(18, response->body().size());
EXPECT_THAT(
response->headers(),
Http::HeaderValueOf(
Extensions::HttpFilters::Common::RateLimit::XRateLimitHeaders::get().XRateLimitRemaining,
"0"));
EXPECT_THAT(
response->headers(),
Http::HeaderValueOf(
Extensions::HttpFilters::Common::RateLimit::XRateLimitHeaders::get().XRateLimitReset,
"1000"));
}

TEST_P(LocalRateLimitFilterIntegrationTest, PermitRequestAcrossDifferentConnections) {
initializeFilter(fmt::format(filter_config_, "true"));

Expand Down Expand Up @@ -562,7 +632,6 @@ TEST_P(LocalRateLimitFilterIntegrationTest, BasicTestPerRouteAndRds) {
EXPECT_TRUE(response->complete());
EXPECT_EQ("200", response->headers().getStatusValue());
EXPECT_EQ(0, response->body().size());

cleanupUpstreamAndDownstream();

cleanUpXdsConnection();
Expand Down