feat: per-IP rate limiting + TTL eviction for RateLimiter#14
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends internal/security’s RateLimiter to support a separate per-IP limiting namespace and adds TTL-based eviction utilities to mitigate unbounded growth of stored limiter entries.
Changes:
- Added
AllowIP(ip string) boolto rate-limit by IP independently of the existing per-client key space. - Added
NewRateLimiterWithTTL(..., ttl)plusEvictStale()to support removing idle limiter entries. - Added limiter entry-count accessors intended for test introspection.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/security/ratelimit.go | Adds per-IP limiter map, TTL tracking, manual eviction method, and count accessors. |
| internal/security/ratelimit_test.go | Adds tests for AllowIP behavior and a TTL eviction test for IP entries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
25
to
+33
| // NewRateLimiter creates a RateLimiter with the given requests-per-minute and burst size. | ||
| func NewRateLimiter(rpm, burst int) *RateLimiter { | ||
| return NewRateLimiterWithTTL(rpm, burst, 10*time.Minute) | ||
| } | ||
|
|
||
| // NewRateLimiterWithTTL creates a RateLimiter whose idle entries are evicted after ttl. | ||
| func NewRateLimiterWithTTL(rpm, burst int, ttl time.Duration) *RateLimiter { | ||
| return &RateLimiter{ | ||
| limiters: make(map[string]*rate.Limiter), | ||
| rpm: rpm, | ||
| burst: burst, | ||
| limiters: make(map[string]*entry), |
Comment on lines
+97
to
+109
| // LimiterCount returns the number of tracked client entries (for testing). | ||
| func (rl *RateLimiter) LimiterCount() int { | ||
| rl.mu.Lock() | ||
| defer rl.mu.Unlock() | ||
| return len(rl.limiters) | ||
| } | ||
|
|
||
| // IPLimiterCount returns the number of tracked IP entries (for testing). | ||
| func (rl *RateLimiter) IPLimiterCount() int { | ||
| rl.mu.Lock() | ||
| defer rl.mu.Unlock() | ||
| return len(rl.ipLimiters) | ||
| } |
| time.Sleep(100 * time.Millisecond) | ||
| rl.EvictStale() | ||
| require.Equal(t, 0, rl.IPLimiterCount()) | ||
| } |
pythondatascrape
force-pushed
the
feat/per-ip-ratelimit
branch
from
6ce6795 to
424d77e
Compare
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
AllowIP(ip string) boolmethod with a separate namespace from per-client limitingNewRateLimiterWithTTLconstructor andEvictStale()for idle-entry cleanupLimiterCount/IPLimiterCountaccessors for test introspectionTest plan
TestAllowIP_*tests verify per-IP allow/block behaviourTestEvictStale_IPconfirms IP entries are evicted after TTLCloses #9, #6
🤖 Generated with Claude Code