feat(middleware): add rate_limit#10
Merged
Merged
Conversation
Second of the three defensive middleware. Enforces a per-IP token-bucket rate limit using golang.org/x/time/rate. Each unique client IP gets its own *rate.Limiter configured with rpm requests per minute and a burst capacity of burst tokens; exceeded requests are rejected with 429 and response.CodeRateLimited before the handler runs. Keying uses the same clientIP helper that logging already uses, so rate limiting honors X-Forwarded-For (Fly.io edge proxy) and falls back to RemoteAddr. To keep the two consumers in sync, the helper is moved out of logging.go into a dedicated clientip.go file; both logging.go and ratelimit.go now reference the same package-internal clientIP function. Behavior is unchanged, the logging tests still pass against the new location. rpm <= 0 disables the limiter entirely (returns a pass-through middleware), matching the convention established by body_limit. Memory note: the per-IP limiter map grows unbounded over process lifetime. Phase 2 explicitly accepts this (Phase 2 instruction §14 says eviction is not required); a background janitor will be added in a later phase if long-lived deployments see meaningful growth. Not yet wired into app.New(); the full defensive stack (body_limit + rate_limit + concurrency_limit) will land together in a later PR along with an end-to-end test. Tests cover within-burst passthrough, beyond-burst rejection with the standard error body, per-IP independence, X-Forwarded- For keying (important for the Fly.io edge shared-proxy case), time-based recovery with a 150 ms sleep, and the disabled paths for zero and negative rpm.
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
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
PR 4/9 of the Phase 2 series. Adds the second defensive middleware: per-IP token-bucket rate limiting backed by
golang.org/x/time/rate. Each unique client IP gets its own*rate.Limiter, and exceeded requests get429\ +response.CodeRateLimited` before reaching the handler.Also includes a small internal refactor: the `clientIP` helper moves from `logging.go` into a dedicated `clientip.go` file so both `logging.go` and `ratelimit.go` can share it without drift. Behavior is unchanged.
Not wired into `app.New()`. Wiring happens in PR 6 along with `body_limit` and `concurrency_limit`.
Key design decisions
Keying by
clientIP, notRemoteAddrFly.io terminates TLS at its edge, so every request arriving at the machine shares the same
RemoteAddr(the edge proxy's internal IP). WithoutX-Forwarded-Forkeying, every client on the planet would share a single limiter — totally broken.clientIPprefers the first entry ofX-Forwarded-Forand falls back toRemoteAddronly when no XFF header is present. The rate limit tests include aKeysByXForwardedForNotRemoteAddrcase that explicitly simulates the Fly edge scenario.rpm → tokens per second conversion
rate.Limitis tokens per second. The middleware converts from rpm viarate.Limit(float64(rpm) / 60.0). For the Phase 1 config defaults (RATE_LIMIT_RPM=10,RATE_LIMIT_BURST=20), that's 0.167 tokens/sec with a 20-token burst.rpm <= 0disables the limiterMatches the convention established by
body_limit. Useful for tests and opt-in no-limit local dev.Unbounded map growth is accepted for Phase 2
Per the Phase 2 instruction §14, eviction of stale per-IP entries is not required in this phase. The map will grow over process lifetime. For short-lived Fly.io machines (auto-stop after idle) this is a non-issue in practice; a background janitor can be added in a later phase if long-lived deployments see problematic growth. The design comment in
ratelimit.goflags this explicitly.clientIPrefactorMoved from
logging.gointo a dedicatedclientip.gofile to avoid duplication. Both callers live in the samemiddlewarepackage so the helper stays package-private (not exported). The logging tests still pass against the moved definition — the refactor is mechanical.Test plan
7 test cases in `ratelimit_test.go`:
Local checks
go vet ./...cleango test ./... -race -count=1all pass (existing + 7 new + 2 body_limit + 2 logging)golangci-lint run0 issuesgo mod tidyleaves go.mod / go.sum clean (CI tidy check will pass)Dependencies added
Per `docs/development_rules.md` §3 this is an explicitly-allowed dependency for rate limiting.
Phase 2 context
Intended to be squash-merged.