Merge decay backend + allowlist (PLA-8128/8129) into main - #3
Merged
Conversation
Adds BACKEND_TYPE=redis_decay: a continuously-decaying counter (GCRA family) executed as an atomic Redis EVAL. Unlike the fixed-window redis backend, cache keys carry no window timestamp, so budgets never reset at a boundary and the 2x boundary-burst is impossible by construction. Honors per-descriptor hits_addend. Fail-open on Redis errors. Cluster-safe: EVAL routes by cache key. Also adds SOFT_BREACH_HEADER_RATIO: when set (0..1), admitted requests whose remaining budget falls inside the soft band get an x-ratelimit-soft-breach: 1 response header, so clients can degrade before hard 429s. Computed generically from CurrentLimit/LimitRemaining, so it works with any cache backend. Covered by test/redis_decay (real EVAL execution on miniredis: boundary immunity, decay recovery, flood behavior, corrupt data, backward clock, fail-open, hits_addend) and test/service/soft_breach_test.go. All existing tests pass. Load-tested on Istio 1.30 / Gateway API: 3,950 rps sustained on one replica, +2.3ms p50 overhead, exact admission under concurrency. Refs envoyproxy#32 (rolling window request, open since 2018). Signed-off-by: ysolomon-plat <yossief.solomon@depop.com>
The backend swallowed Redis errors and returned OK. That hid outages from the RedisError stat and, worse, overrode the caller's failure-mode policy: an operator configuring failure_mode_deny would still have got fail-open. Panic with redis.RedisError as the fixed-window backend does, so the service layer classifies it and the configured policy decides. Found by a fault-injection test on a live cluster. Signed-off-by: ysolomon-plat <yossief.solomon@depop.com>
Replaces the global SOFT_BREACH_HEADER_RATIO with soft_requests_per_unit configured per descriptor, and sends the signal to the upstream service rather than back to the client. Both changes came from auditing the behaviour this mirrors. A ratio applied to every descriptor is not the same thing as a threshold configured on the one descriptor that wants it, and the consumer of the signal is the backend deciding whether to degrade, not the caller. Adding a field to the config struct is not sufficient on its own: validateYamlKeys keeps a separate allowlist, and GetLimit takes a defensive copy via an explicit struct literal, so a new field is silently dropped in both places. Covered by a test that parses real YAML rather than building the struct directly, which is what would have caught it. Signed-off-by: ysolomon-plat <yossief.solomon@depop.com>
Adds RETRY_AFTER_HEADER_ENABLED. When a request is rejected, the response carries Retry-After set to the breaching descriptor's own period in seconds, so clients back off instead of retrying immediately into a saturated bucket. Opt-in, matching the existing custom-header flag, so no existing deployment sees a change in response shape. Placed after the global shadow-mode conversion: a shadowed breach is admitted, so telling the client to back off would be wrong. Per-descriptor shadow_mode is skipped for the same reason. Upstream's own suite caught that ordering when the header was briefly emitted on a shadowed 200. Signed-off-by: ysolomon-plat <yossief.solomon@depop.com>
Adds a per-request allowlist check to cache_impl.go, run before any decay counter is touched: an operator SADDs a value to a Redis SET (allowlist:rl_ip / allowlist:rl_ua / allowlist:rl_subject) and it takes effect on the very next request, no deploy, no EnvoyFilter apply. Replaces the compiled-in Lua tables (IP_ALLOWLIST / USER_ID_ALLOWLIST) that made a config change a deploy. Matches ratelimiting.lua's existing, asymmetric exemption rules exactly (ip_rate_limit_with_pooling / user_agent_rate_limit_with_pooling / auth_rate_limit_with_pooling, :204-253): an allowlisted IP exempts rl_ip, rl_ua AND rl_subject together for the whole request; an allowlisted user-agent or user-id exempts only its own bucket; client_id is never exempted by any allowlist. This code has been running live in Depop's dev-default cluster since 2026-08-21 (deployed via a ConfigMap, source-controlled only there) without ever being pushed here, reviewed, or tested. Adds 8 tests covering the asymmetric exemption rules, the client_id exclusion, the no-allowlist baseline, and the Redis-down failure path (must still panic as redis.RedisError, matching the existing decay-pipeline failure convention).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mainnever actually got the redis_decay backend or the PLA-8129 runtime allowlist. PR feat: add redis_decay backend and soft-breach response header (internal CI run) #1 merged into this branch's earlier state, then PR feat(PLA-8129): runtime Redis-SET allowlist for the decay backend #2's allowlist commit landed on this branch afterward — so it never made it intomain.ratelimit-forkdeployment already runs the image built from this branch's tip (b998c20), so this PR brings the source-of-truth branch in line with what's actually deployed. No behavior change in the cluster.Test plan
maintip after merge matches the currently deployed image SHA