refactor(registry): use RateLimiter interface in middleware
- commit
- 5365741b3c7f600b99244c771be4eaa6071cca15
- author
- bnema · · 2026-01-18 02:14 UTC
- committer
- bnema · · 2026-01-18 02:17 UTC
- parent
- ab451b3
- signature
- unsigned
3 files · +196 -175
M internal/adapters/in/http/registry/ratelimit.go +18 -86
| @@ -6,113 +6,45 @@ import ( | |||
| 6 | 6 | "net" | |
| 7 | 7 | "net/http" | |
| 8 | 8 | "strings" | |
| 9 | - | "sync" | |
| 10 | 9 | ||
| 11 | - | "golang.org/x/time/rate" | |
| 10 | + | "github.com/bnema/zerowrap" | |
| 12 | 11 | ||
| 13 | 12 | "gordon/internal/adapters/dto" | |
| 13 | + | "gordon/internal/boundaries/out" | |
| 14 | 14 | ) | |
| 15 | 15 | ||
| 16 | - | // RateLimitConfig holds rate limiting configuration for the registry API. | |
| 17 | - | // These settings help prevent DoS attacks and brute force attempts. | |
| 18 | - | type RateLimitConfig struct { | |
| 19 | - | // Enabled controls whether rate limiting is active. | |
| 20 | - | // Default: true | |
| 21 | - | Enabled bool | |
| 22 | - | ||
| 23 | - | // GlobalRPS is the maximum requests per second across all clients. | |
| 24 | - | // Default: 500 | |
| 25 | - | GlobalRPS float64 | |
| 26 | - | ||
| 27 | - | // PerIPRPS is the maximum requests per second per client IP. | |
| 28 | - | // Default: 50 | |
| 29 | - | PerIPRPS float64 | |
| 30 | - | ||
| 31 | - | // Burst is the maximum burst size for rate limiters. | |
| 32 | - | // Default: 100 | |
| 33 | - | Burst int | |
| 34 | - | ||
| 35 | - | // TrustedProxies is a list of IP addresses or CIDR ranges that are trusted | |
| 36 | - | // to set X-Forwarded-For and X-Real-IP headers. If empty, these headers are | |
| 37 | - | // ignored and RemoteAddr is always used (secure default for direct exposure). | |
| 38 | - | // Example: ["127.0.0.1", "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16"] | |
| 39 | - | TrustedProxies []string | |
| 40 | - | } | |
| 41 | - | ||
| 42 | - | // DefaultRateLimitConfig returns sensible default rate limiting configuration. | |
| 43 | - | func DefaultRateLimitConfig() RateLimitConfig { | |
| 44 | - | return RateLimitConfig{ | |
| 45 | - | Enabled: true, | |
| 46 | - | GlobalRPS: 500, // 500 requests/second globally | |
| 47 | - | PerIPRPS: 50, // 50 requests/second per IP | |
| 48 | - | Burst: 100, // Allow burst of 100 requests | |
| 49 | - | } | |
| 50 | - | } | |
| 51 | - | ||
| 52 | - | // ipRateLimiter manages per-IP rate limiters. | |
| 53 | - | type ipRateLimiter struct { | |
| 54 | - | limiters map[string]*rate.Limiter | |
| 55 | - | mu sync.RWMutex | |
| 56 | - | rps float64 | |
| 57 | - | burst int | |
| 58 | - | } | |
| 59 | - | ||
| 60 | - | func newIPRateLimiter(rps float64, burst int) *ipRateLimiter { | |
| 61 | - | return &ipRateLimiter{ | |
| 62 | - | limiters: make(map[string]*rate.Limiter), | |
| 63 | - | rps: rps, | |
| 64 | - | burst: burst, | |
| 65 | - | } | |
| 66 | - | } | |
| 67 | - | ||
| 68 | - | func (l *ipRateLimiter) getLimiter(ip string) *rate.Limiter { | |
| 69 | - | l.mu.RLock() | |
| 70 | - | limiter, exists := l.limiters[ip] | |
| 71 | - | l.mu.RUnlock() | |
| 72 | - | ||
| 73 | - | if exists { | |
| 74 | - | return limiter | |
| 75 | - | } | |
| 76 | - | ||
| 77 | - | l.mu.Lock() | |
| 78 | - | defer l.mu.Unlock() | |
| 79 | - | ||
| 80 | - | // Double-check after acquiring write lock | |
| 81 | - | if limiter, exists = l.limiters[ip]; exists { | |
| 82 | - | return limiter | |
| 83 | - | } | |
| 84 | - | ||
| 85 | - | limiter = rate.NewLimiter(rate.Limit(l.rps), l.burst) | |
| 86 | - | l.limiters[ip] = limiter | |
| 87 | - | return limiter | |
| 88 | - | } | |
| 89 | - | ||
| 90 | 16 | // RateLimitMiddleware creates rate limiting middleware for the registry API. | |
| 91 | - | // Returns nil if rate limiting is disabled. | |
| 92 | - | func RateLimitMiddleware(config RateLimitConfig) func(http.Handler) http.Handler { | |
| 93 | - | if !config.Enabled { | |
| 17 | + | // It uses the provided RateLimiter interfaces for global and per-IP limits. | |
| 18 | + | // IP extraction (trusted proxy handling) remains in this HTTP adapter. | |
| 19 | + | func RateLimitMiddleware( | |
| 20 | + | globalLimiter out.RateLimiter, | |
| 21 | + | ipLimiter out.RateLimiter, | |
| 22 | + | trustedProxies []string, | |
| 23 | + | log zerowrap.Logger, | |
| 24 | + | ) func(http.Handler) http.Handler { | |
| 25 | + | // If either limiter is nil, pass through without rate limiting | |
| 26 | + | if globalLimiter == nil || ipLimiter == nil { | |
| 94 | 27 | return func(next http.Handler) http.Handler { | |
| 95 | - | return next // Pass through if disabled | |
| 28 | + | return next | |
| 96 | 29 | } | |
| 97 | 30 | } | |
| 98 | 31 | ||
| 99 | - | globalLimiter := rate.NewLimiter(rate.Limit(config.GlobalRPS), config.Burst) | |
| 100 | - | ipLimiter := newIPRateLimiter(config.PerIPRPS, config.Burst) | |
| 101 | - | ||
| 102 | 32 | // Parse trusted proxy CIDRs once at middleware creation | |
| 103 | - | trustedNets := parseTrustedProxies(config.TrustedProxies) | |
| 33 | + | trustedNets := parseTrustedProxies(trustedProxies) | |
| 104 | 34 | ||
| 105 | 35 | return func(next http.Handler) http.Handler { | |
| 106 | 36 | return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 37 | + | ctx := r.Context() | |
| 38 | + | ||
| 107 | 39 | // Check global rate limit | |
| 108 | - | if !globalLimiter.Allow() { | |
| 40 | + | if !globalLimiter.Allow(ctx, "global") { | |
| 109 | 41 | sendRateLimitError(w) | |
| 110 | 42 | return | |
| 111 | 43 | } | |
| 112 | 44 | ||
| 113 | 45 | // Check per-IP rate limit | |
| 114 | 46 | ip := getClientIP(r, trustedNets) | |
| 115 | - | if !ipLimiter.getLimiter(ip).Allow() { | |
| 47 | + | if !ipLimiter.Allow(ctx, "ip:"+ip) { | |
| 116 | 48 | sendRateLimitError(w) | |
| 117 | 49 | return | |
| 118 | 50 | } | |
| @@ -6,113 +6,45 @@ import ( | |||||
| 6 | "net" | 6 | "net" | ||
| 7 | "net/http" | 7 | "net/http" | ||
| 8 | "strings" | 8 | "strings" | ||
| 9 | - | "sync" | |||
| 10 | 9 | ||||
| 11 | - | "golang.org/x/time/rate" | 10 | + | "github.com/bnema/zerowrap" |
| 12 | 11 | ||||
| 13 | "gordon/internal/adapters/dto" | 12 | "gordon/internal/adapters/dto" | ||
| 13 | + | "gordon/internal/boundaries/out" | |||
| 14 | ) | 14 | ) | ||
| 15 | 15 | ||||
| 16 | - | // RateLimitConfig holds rate limiting configuration for the registry API. | |||
| 17 | - | // These settings help prevent DoS attacks and brute force attempts. | |||
| 18 | - | type RateLimitConfig struct { | |||
| 19 | - | // Enabled controls whether rate limiting is active. | |||
| 20 | - | // Default: true | |||
| 21 | - | Enabled bool | |||
| 22 | - | ||||
| 23 | - | // GlobalRPS is the maximum requests per second across all clients. | |||
| 24 | - | // Default: 500 | |||
| 25 | - | GlobalRPS float64 | |||
| 26 | - | ||||
| 27 | - | // PerIPRPS is the maximum requests per second per client IP. | |||
| 28 | - | // Default: 50 | |||
| 29 | - | PerIPRPS float64 | |||
| 30 | - | ||||
| 31 | - | // Burst is the maximum burst size for rate limiters. | |||
| 32 | - | // Default: 100 | |||
| 33 | - | Burst int | |||
| 34 | - | ||||
| 35 | - | // TrustedProxies is a list of IP addresses or CIDR ranges that are trusted | |||
| 36 | - | // to set X-Forwarded-For and X-Real-IP headers. If empty, these headers are | |||
| 37 | - | // ignored and RemoteAddr is always used (secure default for direct exposure). | |||
| 38 | - | // Example: ["127.0.0.1", "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16"] | |||
| 39 | - | TrustedProxies []string | |||
| 40 | - | } | |||
| 41 | - | ||||
| 42 | - | // DefaultRateLimitConfig returns sensible default rate limiting configuration. | |||
| 43 | - | func DefaultRateLimitConfig() RateLimitConfig { | |||
| 44 | - | return RateLimitConfig{ | |||
| 45 | - | Enabled: true, | |||
| 46 | - | GlobalRPS: 500, // 500 requests/second globally | |||
| 47 | - | PerIPRPS: 50, // 50 requests/second per IP | |||
| 48 | - | Burst: 100, // Allow burst of 100 requests | |||
| 49 | - | } | |||
| 50 | - | } | |||
| 51 | - | ||||
| 52 | - | // ipRateLimiter manages per-IP rate limiters. | |||
| 53 | - | type ipRateLimiter struct { | |||
| 54 | - | limiters map[string]*rate.Limiter | |||
| 55 | - | mu sync.RWMutex | |||
| 56 | - | rps float64 | |||
| 57 | - | burst int | |||
| 58 | - | } | |||
| 59 | - | ||||
| 60 | - | func newIPRateLimiter(rps float64, burst int) *ipRateLimiter { | |||
| 61 | - | return &ipRateLimiter{ | |||
| 62 | - | limiters: make(map[string]*rate.Limiter), | |||
| 63 | - | rps: rps, | |||
| 64 | - | burst: burst, | |||
| 65 | - | } | |||
| 66 | - | } | |||
| 67 | - | ||||
| 68 | - | func (l *ipRateLimiter) getLimiter(ip string) *rate.Limiter { | |||
| 69 | - | l.mu.RLock() | |||
| 70 | - | limiter, exists := l.limiters[ip] | |||
| 71 | - | l.mu.RUnlock() | |||
| 72 | - | ||||
| 73 | - | if exists { | |||
| 74 | - | return limiter | |||
| 75 | - | } | |||
| 76 | - | ||||
| 77 | - | l.mu.Lock() | |||
| 78 | - | defer l.mu.Unlock() | |||
| 79 | - | ||||
| 80 | - | // Double-check after acquiring write lock | |||
| 81 | - | if limiter, exists = l.limiters[ip]; exists { | |||
| 82 | - | return limiter | |||
| 83 | - | } | |||
| 84 | - | ||||
| 85 | - | limiter = rate.NewLimiter(rate.Limit(l.rps), l.burst) | |||
| 86 | - | l.limiters[ip] = limiter | |||
| 87 | - | return limiter | |||
| 88 | - | } | |||
| 89 | - | ||||
| 90 | // RateLimitMiddleware creates rate limiting middleware for the registry API. | 16 | // RateLimitMiddleware creates rate limiting middleware for the registry API. | ||
| 91 | - | // Returns nil if rate limiting is disabled. | 17 | + | // It uses the provided RateLimiter interfaces for global and per-IP limits. |
| 92 | - | func RateLimitMiddleware(config RateLimitConfig) func(http.Handler) http.Handler { | 18 | + | // IP extraction (trusted proxy handling) remains in this HTTP adapter. |
| 93 | - | if !config.Enabled { | 19 | + | func RateLimitMiddleware( |
| 20 | + | globalLimiter out.RateLimiter, | |||
| 21 | + | ipLimiter out.RateLimiter, | |||
| 22 | + | trustedProxies []string, | |||
| 23 | + | log zerowrap.Logger, | |||
| 24 | + | ) func(http.Handler) http.Handler { | |||
| 25 | + | // If either limiter is nil, pass through without rate limiting | |||
| 26 | + | if globalLimiter == nil || ipLimiter == nil { | |||
| 94 | return func(next http.Handler) http.Handler { | 27 | return func(next http.Handler) http.Handler { | ||
| 95 | - | return next // Pass through if disabled | 28 | + | return next |
| 96 | } | 29 | } | ||
| 97 | } | 30 | } | ||
| 98 | 31 | ||||
| 99 | - | globalLimiter := rate.NewLimiter(rate.Limit(config.GlobalRPS), config.Burst) | |||
| 100 | - | ipLimiter := newIPRateLimiter(config.PerIPRPS, config.Burst) | |||
| 101 | - | ||||
| 102 | // Parse trusted proxy CIDRs once at middleware creation | 32 | // Parse trusted proxy CIDRs once at middleware creation | ||
| 103 | - | trustedNets := parseTrustedProxies(config.TrustedProxies) | 33 | + | trustedNets := parseTrustedProxies(trustedProxies) |
| 104 | 34 | ||||
| 105 | return func(next http.Handler) http.Handler { | 35 | return func(next http.Handler) http.Handler { | ||
| 106 | return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | 36 | return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| 37 | + | ctx := r.Context() | |||
| 38 | + | ||||
| 107 | // Check global rate limit | 39 | // Check global rate limit | ||
| 108 | - | if !globalLimiter.Allow() { | 40 | + | if !globalLimiter.Allow(ctx, "global") { |
| 109 | sendRateLimitError(w) | 41 | sendRateLimitError(w) | ||
| 110 | return | 42 | return | ||
| 111 | } | 43 | } | ||
| 112 | 44 | ||||
| 113 | // Check per-IP rate limit | 45 | // Check per-IP rate limit | ||
| 114 | ip := getClientIP(r, trustedNets) | 46 | ip := getClientIP(r, trustedNets) | ||
| 115 | - | if !ipLimiter.getLimiter(ip).Allow() { | 47 | + | if !ipLimiter.Allow(ctx, "ip:"+ip) { |
| 116 | sendRateLimitError(w) | 48 | sendRateLimitError(w) | ||
| 117 | return | 49 | return | ||
| 118 | } | 50 | } | ||
M internal/adapters/in/http/registry/ratelimit_test.go +164 -81
| @@ -1,29 +1,29 @@ | |||
| 1 | 1 | package registry | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 4 | 5 | "net" | |
| 5 | 6 | "net/http" | |
| 6 | 7 | "net/http/httptest" | |
| 7 | 8 | "testing" | |
| 8 | 9 | ||
| 9 | 10 | "github.com/stretchr/testify/assert" | |
| 10 | - | "github.com/stretchr/testify/require" | |
| 11 | + | ||
| 12 | + | "gordon/internal/adapters/out/ratelimit" | |
| 13 | + | outmocks "gordon/internal/boundaries/out/mocks" | |
| 11 | 14 | ) | |
| 12 | 15 | ||
| 13 | 16 | func TestRateLimitMiddleware_Disabled(t *testing.T) { | |
| 14 | - | config := RateLimitConfig{ | |
| 15 | - | Enabled: false, | |
| 16 | - | } | |
| 17 | - | ||
| 18 | - | handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 17 | + | // When limiters are nil, middleware should pass through | |
| 18 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |
| 19 | 19 | w.WriteHeader(http.StatusOK) | |
| 20 | 20 | }) | |
| 21 | 21 | ||
| 22 | - | middleware := RateLimitMiddleware(config) | |
| 22 | + | middleware := RateLimitMiddleware(nil, nil, nil, testLogger()) | |
| 23 | 23 | wrappedHandler := middleware(handler) | |
| 24 | 24 | ||
| 25 | 25 | // Should pass through without rate limiting | |
| 26 | - | for i := 0; i < 1000; i++ { | |
| 26 | + | for i := 0; i < 100; i++ { | |
| 27 | 27 | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |
| 28 | 28 | rec := httptest.NewRecorder() | |
| 29 | 29 | wrappedHandler.ServeHTTP(rec, req) | |
| @@ -31,99 +31,191 @@ func TestRateLimitMiddleware_Disabled(t *testing.T) { | |||
| 31 | 31 | } | |
| 32 | 32 | } | |
| 33 | 33 | ||
| 34 | - | func TestRateLimitMiddleware_PerIPLimit(t *testing.T) { | |
| 35 | - | config := RateLimitConfig{ | |
| 36 | - | Enabled: true, | |
| 37 | - | GlobalRPS: 1000, // High global limit | |
| 38 | - | PerIPRPS: 1, // Very low per-IP limit for testing | |
| 39 | - | Burst: 100, // High burst to not interfere | |
| 40 | - | } | |
| 34 | + | func TestRateLimitMiddleware_GlobalLimitHit(t *testing.T) { | |
| 35 | + | globalLimiter := outmocks.NewMockRateLimiter(t) | |
| 36 | + | ipLimiter := outmocks.NewMockRateLimiter(t) | |
| 37 | + | ||
| 38 | + | // Global limiter returns false (rate limited) | |
| 39 | + | globalLimiter.EXPECT().Allow(context.Background(), "global").Return(false) | |
| 41 | 40 | ||
| 42 | - | handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 41 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |
| 43 | 42 | w.WriteHeader(http.StatusOK) | |
| 44 | 43 | }) | |
| 45 | 44 | ||
| 46 | - | middleware := RateLimitMiddleware(config) | |
| 45 | + | middleware := RateLimitMiddleware(globalLimiter, ipLimiter, nil, testLogger()) | |
| 47 | 46 | wrappedHandler := middleware(handler) | |
| 48 | 47 | ||
| 49 | - | // First request should succeed | |
| 50 | - | req1 := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |
| 51 | - | req1.RemoteAddr = "192.168.1.100:12345" | |
| 52 | - | rec1 := httptest.NewRecorder() | |
| 53 | - | wrappedHandler.ServeHTTP(rec1, req1) | |
| 54 | - | assert.Equal(t, http.StatusOK, rec1.Code) | |
| 48 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |
| 49 | + | req.RemoteAddr = "192.168.1.100:12345" | |
| 50 | + | rec := httptest.NewRecorder() | |
| 51 | + | wrappedHandler.ServeHTTP(rec, req) | |
| 55 | 52 | ||
| 56 | - | // Request from different IP should succeed (independent rate limiter) | |
| 57 | - | req2 := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |
| 58 | - | req2.RemoteAddr = "192.168.1.101:12345" | |
| 59 | - | rec2 := httptest.NewRecorder() | |
| 60 | - | wrappedHandler.ServeHTTP(rec2, req2) | |
| 61 | - | assert.Equal(t, http.StatusOK, rec2.Code) | |
| 53 | + | // Should return 429 when global limit hit | |
| 54 | + | assert.Equal(t, http.StatusTooManyRequests, rec.Code) | |
| 55 | + | assert.Contains(t, rec.Body.String(), "TOOMANYREQUESTS") | |
| 62 | 56 | } | |
| 63 | 57 | ||
| 64 | - | func TestRateLimitMiddleware_GlobalLimit(t *testing.T) { | |
| 65 | - | config := RateLimitConfig{ | |
| 66 | - | Enabled: true, | |
| 67 | - | GlobalRPS: 1, // Very low global limit for testing | |
| 68 | - | PerIPRPS: 100, | |
| 69 | - | Burst: 1, | |
| 70 | - | } | |
| 58 | + | func TestRateLimitMiddleware_PerIPLimitHit(t *testing.T) { | |
| 59 | + | globalLimiter := outmocks.NewMockRateLimiter(t) | |
| 60 | + | ipLimiter := outmocks.NewMockRateLimiter(t) | |
| 61 | + | ||
| 62 | + | // Global limiter allows, IP limiter denies | |
| 63 | + | globalLimiter.EXPECT().Allow(context.Background(), "global").Return(true) | |
| 64 | + | ipLimiter.EXPECT().Allow(context.Background(), "ip:192.168.1.100").Return(false) | |
| 71 | 65 | ||
| 72 | - | handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 66 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |
| 73 | 67 | w.WriteHeader(http.StatusOK) | |
| 74 | 68 | }) | |
| 75 | 69 | ||
| 76 | - | middleware := RateLimitMiddleware(config) | |
| 70 | + | middleware := RateLimitMiddleware(globalLimiter, ipLimiter, nil, testLogger()) | |
| 77 | 71 | wrappedHandler := middleware(handler) | |
| 78 | 72 | ||
| 79 | - | // First request should succeed | |
| 80 | - | req1 := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |
| 81 | - | req1.RemoteAddr = "192.168.1.100:12345" | |
| 82 | - | rec1 := httptest.NewRecorder() | |
| 83 | - | wrappedHandler.ServeHTTP(rec1, req1) | |
| 84 | - | assert.Equal(t, http.StatusOK, rec1.Code) | |
| 73 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |
| 74 | + | req.RemoteAddr = "192.168.1.100:12345" | |
| 75 | + | rec := httptest.NewRecorder() | |
| 76 | + | wrappedHandler.ServeHTTP(rec, req) | |
| 85 | 77 | ||
| 86 | - | // Second request should hit global limit even from different IP | |
| 87 | - | req2 := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |
| 88 | - | req2.RemoteAddr = "192.168.1.101:12345" | |
| 89 | - | rec2 := httptest.NewRecorder() | |
| 90 | - | wrappedHandler.ServeHTTP(rec2, req2) | |
| 91 | - | assert.Equal(t, http.StatusTooManyRequests, rec2.Code) | |
| 78 | + | // Should return 429 when per-IP limit hit | |
| 79 | + | assert.Equal(t, http.StatusTooManyRequests, rec.Code) | |
| 80 | + | } | |
| 81 | + | ||
| 82 | + | func TestRateLimitMiddleware_BothLimitsPass(t *testing.T) { | |
| 83 | + | globalLimiter := outmocks.NewMockRateLimiter(t) | |
| 84 | + | ipLimiter := outmocks.NewMockRateLimiter(t) | |
| 85 | + | ||
| 86 | + | // Both limiters allow | |
| 87 | + | globalLimiter.EXPECT().Allow(context.Background(), "global").Return(true) | |
| 88 | + | ipLimiter.EXPECT().Allow(context.Background(), "ip:192.168.1.100").Return(true) | |
| 89 | + | ||
| 90 | + | handlerCalled := false | |
| 91 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |
| 92 | + | handlerCalled = true | |
| 93 | + | w.WriteHeader(http.StatusOK) | |
| 94 | + | }) | |
| 95 | + | ||
| 96 | + | middleware := RateLimitMiddleware(globalLimiter, ipLimiter, nil, testLogger()) | |
| 97 | + | wrappedHandler := middleware(handler) | |
| 98 | + | ||
| 99 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |
| 100 | + | req.RemoteAddr = "192.168.1.100:12345" | |
| 101 | + | rec := httptest.NewRecorder() | |
| 102 | + | wrappedHandler.ServeHTTP(rec, req) | |
| 103 | + | ||
| 104 | + | // Should pass through to handler | |
| 105 | + | assert.Equal(t, http.StatusOK, rec.Code) | |
| 106 | + | assert.True(t, handlerCalled, "handler should have been called") | |
| 107 | + | } | |
| 108 | + | ||
| 109 | + | func TestRateLimitMiddleware_TrustedProxy(t *testing.T) { | |
| 110 | + | globalLimiter := outmocks.NewMockRateLimiter(t) | |
| 111 | + | ipLimiter := outmocks.NewMockRateLimiter(t) | |
| 112 | + | ||
| 113 | + | // Request comes from trusted proxy (127.0.0.1) with XFF header | |
| 114 | + | // Should use the X-Forwarded-For IP (203.0.113.50) for rate limiting | |
| 115 | + | globalLimiter.EXPECT().Allow(context.Background(), "global").Return(true) | |
| 116 | + | ipLimiter.EXPECT().Allow(context.Background(), "ip:203.0.113.50").Return(true) | |
| 117 | + | ||
| 118 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |
| 119 | + | w.WriteHeader(http.StatusOK) | |
| 120 | + | }) | |
| 121 | + | ||
| 122 | + | middleware := RateLimitMiddleware(globalLimiter, ipLimiter, []string{"127.0.0.1"}, testLogger()) | |
| 123 | + | wrappedHandler := middleware(handler) | |
| 124 | + | ||
| 125 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |
| 126 | + | req.RemoteAddr = "127.0.0.1:12345" | |
| 127 | + | req.Header.Set("X-Forwarded-For", "203.0.113.50") | |
| 128 | + | rec := httptest.NewRecorder() | |
| 129 | + | wrappedHandler.ServeHTTP(rec, req) | |
| 130 | + | ||
| 131 | + | assert.Equal(t, http.StatusOK, rec.Code) | |
| 132 | + | } | |
| 133 | + | ||
| 134 | + | func TestRateLimitMiddleware_UntrustedProxy(t *testing.T) { | |
| 135 | + | globalLimiter := outmocks.NewMockRateLimiter(t) | |
| 136 | + | ipLimiter := outmocks.NewMockRateLimiter(t) | |
| 137 | + | ||
| 138 | + | // Request from untrusted IP with XFF header should use RemoteAddr IP | |
| 139 | + | globalLimiter.EXPECT().Allow(context.Background(), "global").Return(true) | |
| 140 | + | ipLimiter.EXPECT().Allow(context.Background(), "ip:192.168.1.100").Return(true) | |
| 141 | + | ||
| 142 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |
| 143 | + | w.WriteHeader(http.StatusOK) | |
| 144 | + | }) | |
| 145 | + | ||
| 146 | + | middleware := RateLimitMiddleware(globalLimiter, ipLimiter, []string{"127.0.0.1"}, testLogger()) | |
| 147 | + | wrappedHandler := middleware(handler) | |
| 148 | + | ||
| 149 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |
| 150 | + | req.RemoteAddr = "192.168.1.100:12345" | |
| 151 | + | req.Header.Set("X-Forwarded-For", "203.0.113.50") // Should be ignored | |
| 152 | + | rec := httptest.NewRecorder() | |
| 153 | + | wrappedHandler.ServeHTTP(rec, req) | |
| 154 | + | ||
| 155 | + | assert.Equal(t, http.StatusOK, rec.Code) | |
| 92 | 156 | } | |
| 93 | 157 | ||
| 94 | 158 | func TestRateLimitMiddleware_ResponseFormat(t *testing.T) { | |
| 95 | - | config := RateLimitConfig{ | |
| 96 | - | Enabled: true, | |
| 97 | - | GlobalRPS: 1, | |
| 98 | - | PerIPRPS: 1, | |
| 99 | - | Burst: 1, | |
| 100 | - | } | |
| 159 | + | globalLimiter := outmocks.NewMockRateLimiter(t) | |
| 160 | + | ipLimiter := outmocks.NewMockRateLimiter(t) | |
| 161 | + | ||
| 162 | + | globalLimiter.EXPECT().Allow(context.Background(), "global").Return(false) | |
| 163 | + | ||
| 164 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |
| 165 | + | w.WriteHeader(http.StatusOK) | |
| 166 | + | }) | |
| 167 | + | ||
| 168 | + | middleware := RateLimitMiddleware(globalLimiter, ipLimiter, nil, testLogger()) | |
| 169 | + | wrappedHandler := middleware(handler) | |
| 170 | + | ||
| 171 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |
| 172 | + | req.RemoteAddr = "192.168.1.100:12345" | |
| 173 | + | rec := httptest.NewRecorder() | |
| 174 | + | wrappedHandler.ServeHTTP(rec, req) | |
| 175 | + | ||
| 176 | + | // Verify response format | |
| 177 | + | assert.Equal(t, http.StatusTooManyRequests, rec.Code) | |
| 178 | + | assert.Equal(t, "application/json", rec.Header().Get("Content-Type")) | |
| 179 | + | assert.Equal(t, "registry/2.0", rec.Header().Get("Docker-Distribution-API-Version")) | |
| 180 | + | assert.Equal(t, "1", rec.Header().Get("Retry-After")) | |
| 181 | + | assert.Contains(t, rec.Body.String(), "TOOMANYREQUESTS") | |
| 182 | + | } | |
| 101 | 183 | ||
| 102 | - | handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 184 | + | // Integration test using real MemoryStore implementation | |
| 185 | + | func TestRateLimitMiddleware_Integration(t *testing.T) { | |
| 186 | + | log := testLogger() | |
| 187 | + | globalLimiter := ratelimit.NewMemoryStore(1000, 100, log) // High global limit | |
| 188 | + | ipLimiter := ratelimit.NewMemoryStore(2, 2, log) // Low per-IP limit | |
| 189 | + | ||
| 190 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |
| 103 | 191 | w.WriteHeader(http.StatusOK) | |
| 104 | 192 | }) | |
| 105 | 193 | ||
| 106 | - | middleware := RateLimitMiddleware(config) | |
| 194 | + | middleware := RateLimitMiddleware(globalLimiter, ipLimiter, nil, log) | |
| 107 | 195 | wrappedHandler := middleware(handler) | |
| 108 | 196 | ||
| 109 | - | // First request succeeds | |
| 110 | - | req1 := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |
| 111 | - | req1.RemoteAddr = "192.168.1.100:12345" | |
| 112 | - | rec1 := httptest.NewRecorder() | |
| 113 | - | wrappedHandler.ServeHTTP(rec1, req1) | |
| 197 | + | // First 2 requests from same IP should succeed (burst) | |
| 198 | + | for i := 0; i < 2; i++ { | |
| 199 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |
| 200 | + | req.RemoteAddr = "192.168.1.100:12345" | |
| 201 | + | rec := httptest.NewRecorder() | |
| 202 | + | wrappedHandler.ServeHTTP(rec, req) | |
| 203 | + | assert.Equal(t, http.StatusOK, rec.Code, "request %d should succeed", i+1) | |
| 204 | + | } | |
| 205 | + | ||
| 206 | + | // 3rd request from same IP should be rate limited | |
| 207 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |
| 208 | + | req.RemoteAddr = "192.168.1.100:12345" | |
| 209 | + | rec := httptest.NewRecorder() | |
| 210 | + | wrappedHandler.ServeHTTP(rec, req) | |
| 211 | + | assert.Equal(t, http.StatusTooManyRequests, rec.Code, "3rd request should be rate limited") | |
| 114 | 212 | ||
| 115 | - | // Second request gets rate limited | |
| 213 | + | // Request from different IP should still succeed (independent limit) | |
| 116 | 214 | req2 := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |
| 117 | - | req2.RemoteAddr = "192.168.1.100:12346" | |
| 215 | + | req2.RemoteAddr = "192.168.1.101:12345" | |
| 118 | 216 | rec2 := httptest.NewRecorder() | |
| 119 | 217 | wrappedHandler.ServeHTTP(rec2, req2) | |
| 120 | - | ||
| 121 | - | // Verify response format | |
| 122 | - | assert.Equal(t, http.StatusTooManyRequests, rec2.Code) | |
| 123 | - | assert.Equal(t, "application/json", rec2.Header().Get("Content-Type")) | |
| 124 | - | assert.Equal(t, "registry/2.0", rec2.Header().Get("Docker-Distribution-API-Version")) | |
| 125 | - | assert.Equal(t, "1", rec2.Header().Get("Retry-After")) | |
| 126 | - | assert.Contains(t, rec2.Body.String(), "TOOMANYREQUESTS") | |
| 218 | + | assert.Equal(t, http.StatusOK, rec2.Code, "different IP should succeed") | |
| 127 | 219 | } | |
| 128 | 220 | ||
| 129 | 221 | func TestGetClientIP(t *testing.T) { | |
| @@ -286,12 +378,3 @@ func TestParseTrustedProxies(t *testing.T) { | |||
| 286 | 378 | }) | |
| 287 | 379 | } | |
| 288 | 380 | } | |
| 289 | - | ||
| 290 | - | func TestDefaultRateLimitConfig(t *testing.T) { | |
| 291 | - | config := DefaultRateLimitConfig() | |
| 292 | - | ||
| 293 | - | require.True(t, config.Enabled) | |
| 294 | - | assert.Equal(t, float64(500), config.GlobalRPS) | |
| 295 | - | assert.Equal(t, float64(50), config.PerIPRPS) | |
| 296 | - | assert.Equal(t, 100, config.Burst) | |
| 297 | - | } | |
| @@ -1,29 +1,29 @@ | |||||
| 1 | package registry | 1 | package registry | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "context" | |||
| 4 | "net" | 5 | "net" | ||
| 5 | "net/http" | 6 | "net/http" | ||
| 6 | "net/http/httptest" | 7 | "net/http/httptest" | ||
| 7 | "testing" | 8 | "testing" | ||
| 8 | 9 | ||||
| 9 | "github.com/stretchr/testify/assert" | 10 | "github.com/stretchr/testify/assert" | ||
| 10 | - | "github.com/stretchr/testify/require" | 11 | + | |
| 12 | + | "gordon/internal/adapters/out/ratelimit" | |||
| 13 | + | outmocks "gordon/internal/boundaries/out/mocks" | |||
| 11 | ) | 14 | ) | ||
| 12 | 15 | ||||
| 13 | func TestRateLimitMiddleware_Disabled(t *testing.T) { | 16 | func TestRateLimitMiddleware_Disabled(t *testing.T) { | ||
| 14 | - | config := RateLimitConfig{ | 17 | + | // When limiters are nil, middleware should pass through |
| 15 | - | Enabled: false, | 18 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { |
| 16 | - | } | |||
| 17 | - | ||||
| 18 | - | handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 19 | w.WriteHeader(http.StatusOK) | 19 | w.WriteHeader(http.StatusOK) | ||
| 20 | }) | 20 | }) | ||
| 21 | 21 | ||||
| 22 | - | middleware := RateLimitMiddleware(config) | 22 | + | middleware := RateLimitMiddleware(nil, nil, nil, testLogger()) |
| 23 | wrappedHandler := middleware(handler) | 23 | wrappedHandler := middleware(handler) | ||
| 24 | 24 | ||||
| 25 | // Should pass through without rate limiting | 25 | // Should pass through without rate limiting | ||
| 26 | - | for i := 0; i < 1000; i++ { | 26 | + | for i := 0; i < 100; i++ { |
| 27 | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | 27 | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | ||
| 28 | rec := httptest.NewRecorder() | 28 | rec := httptest.NewRecorder() | ||
| 29 | wrappedHandler.ServeHTTP(rec, req) | 29 | wrappedHandler.ServeHTTP(rec, req) | ||
| @@ -31,99 +31,191 @@ func TestRateLimitMiddleware_Disabled(t *testing.T) { | |||||
| 31 | } | 31 | } | ||
| 32 | } | 32 | } | ||
| 33 | 33 | ||||
| 34 | - | func TestRateLimitMiddleware_PerIPLimit(t *testing.T) { | 34 | + | func TestRateLimitMiddleware_GlobalLimitHit(t *testing.T) { |
| 35 | - | config := RateLimitConfig{ | 35 | + | globalLimiter := outmocks.NewMockRateLimiter(t) |
| 36 | - | Enabled: true, | 36 | + | ipLimiter := outmocks.NewMockRateLimiter(t) |
| 37 | - | GlobalRPS: 1000, // High global limit | 37 | + | |
| 38 | - | PerIPRPS: 1, // Very low per-IP limit for testing | 38 | + | // Global limiter returns false (rate limited) |
| 39 | - | Burst: 100, // High burst to not interfere | 39 | + | globalLimiter.EXPECT().Allow(context.Background(), "global").Return(false) |
| 40 | - | } | |||
| 41 | 40 | ||||
| 42 | - | handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | 41 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { |
| 43 | w.WriteHeader(http.StatusOK) | 42 | w.WriteHeader(http.StatusOK) | ||
| 44 | }) | 43 | }) | ||
| 45 | 44 | ||||
| 46 | - | middleware := RateLimitMiddleware(config) | 45 | + | middleware := RateLimitMiddleware(globalLimiter, ipLimiter, nil, testLogger()) |
| 47 | wrappedHandler := middleware(handler) | 46 | wrappedHandler := middleware(handler) | ||
| 48 | 47 | ||||
| 49 | - | // First request should succeed | 48 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) |
| 50 | - | req1 := httptest.NewRequest(http.MethodGet, "/v2/", nil) | 49 | + | req.RemoteAddr = "192.168.1.100:12345" |
| 51 | - | req1.RemoteAddr = "192.168.1.100:12345" | 50 | + | rec := httptest.NewRecorder() |
| 52 | - | rec1 := httptest.NewRecorder() | 51 | + | wrappedHandler.ServeHTTP(rec, req) |
| 53 | - | wrappedHandler.ServeHTTP(rec1, req1) | |||
| 54 | - | assert.Equal(t, http.StatusOK, rec1.Code) | |||
| 55 | 52 | ||||
| 56 | - | // Request from different IP should succeed (independent rate limiter) | 53 | + | // Should return 429 when global limit hit |
| 57 | - | req2 := httptest.NewRequest(http.MethodGet, "/v2/", nil) | 54 | + | assert.Equal(t, http.StatusTooManyRequests, rec.Code) |
| 58 | - | req2.RemoteAddr = "192.168.1.101:12345" | 55 | + | assert.Contains(t, rec.Body.String(), "TOOMANYREQUESTS") |
| 59 | - | rec2 := httptest.NewRecorder() | |||
| 60 | - | wrappedHandler.ServeHTTP(rec2, req2) | |||
| 61 | - | assert.Equal(t, http.StatusOK, rec2.Code) | |||
| 62 | } | 56 | } | ||
| 63 | 57 | ||||
| 64 | - | func TestRateLimitMiddleware_GlobalLimit(t *testing.T) { | 58 | + | func TestRateLimitMiddleware_PerIPLimitHit(t *testing.T) { |
| 65 | - | config := RateLimitConfig{ | 59 | + | globalLimiter := outmocks.NewMockRateLimiter(t) |
| 66 | - | Enabled: true, | 60 | + | ipLimiter := outmocks.NewMockRateLimiter(t) |
| 67 | - | GlobalRPS: 1, // Very low global limit for testing | 61 | + | |
| 68 | - | PerIPRPS: 100, | 62 | + | // Global limiter allows, IP limiter denies |
| 69 | - | Burst: 1, | 63 | + | globalLimiter.EXPECT().Allow(context.Background(), "global").Return(true) |
| 70 | - | } | 64 | + | ipLimiter.EXPECT().Allow(context.Background(), "ip:192.168.1.100").Return(false) |
| 71 | 65 | ||||
| 72 | - | handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | 66 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { |
| 73 | w.WriteHeader(http.StatusOK) | 67 | w.WriteHeader(http.StatusOK) | ||
| 74 | }) | 68 | }) | ||
| 75 | 69 | ||||
| 76 | - | middleware := RateLimitMiddleware(config) | 70 | + | middleware := RateLimitMiddleware(globalLimiter, ipLimiter, nil, testLogger()) |
| 77 | wrappedHandler := middleware(handler) | 71 | wrappedHandler := middleware(handler) | ||
| 78 | 72 | ||||
| 79 | - | // First request should succeed | 73 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) |
| 80 | - | req1 := httptest.NewRequest(http.MethodGet, "/v2/", nil) | 74 | + | req.RemoteAddr = "192.168.1.100:12345" |
| 81 | - | req1.RemoteAddr = "192.168.1.100:12345" | 75 | + | rec := httptest.NewRecorder() |
| 82 | - | rec1 := httptest.NewRecorder() | 76 | + | wrappedHandler.ServeHTTP(rec, req) |
| 83 | - | wrappedHandler.ServeHTTP(rec1, req1) | |||
| 84 | - | assert.Equal(t, http.StatusOK, rec1.Code) | |||
| 85 | 77 | ||||
| 86 | - | // Second request should hit global limit even from different IP | 78 | + | // Should return 429 when per-IP limit hit |
| 87 | - | req2 := httptest.NewRequest(http.MethodGet, "/v2/", nil) | 79 | + | assert.Equal(t, http.StatusTooManyRequests, rec.Code) |
| 88 | - | req2.RemoteAddr = "192.168.1.101:12345" | 80 | + | } |
| 89 | - | rec2 := httptest.NewRecorder() | 81 | + | |
| 90 | - | wrappedHandler.ServeHTTP(rec2, req2) | 82 | + | func TestRateLimitMiddleware_BothLimitsPass(t *testing.T) { |
| 91 | - | assert.Equal(t, http.StatusTooManyRequests, rec2.Code) | 83 | + | globalLimiter := outmocks.NewMockRateLimiter(t) |
| 84 | + | ipLimiter := outmocks.NewMockRateLimiter(t) | |||
| 85 | + | ||||
| 86 | + | // Both limiters allow | |||
| 87 | + | globalLimiter.EXPECT().Allow(context.Background(), "global").Return(true) | |||
| 88 | + | ipLimiter.EXPECT().Allow(context.Background(), "ip:192.168.1.100").Return(true) | |||
| 89 | + | ||||
| 90 | + | handlerCalled := false | |||
| 91 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |||
| 92 | + | handlerCalled = true | |||
| 93 | + | w.WriteHeader(http.StatusOK) | |||
| 94 | + | }) | |||
| 95 | + | ||||
| 96 | + | middleware := RateLimitMiddleware(globalLimiter, ipLimiter, nil, testLogger()) | |||
| 97 | + | wrappedHandler := middleware(handler) | |||
| 98 | + | ||||
| 99 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |||
| 100 | + | req.RemoteAddr = "192.168.1.100:12345" | |||
| 101 | + | rec := httptest.NewRecorder() | |||
| 102 | + | wrappedHandler.ServeHTTP(rec, req) | |||
| 103 | + | ||||
| 104 | + | // Should pass through to handler | |||
| 105 | + | assert.Equal(t, http.StatusOK, rec.Code) | |||
| 106 | + | assert.True(t, handlerCalled, "handler should have been called") | |||
| 107 | + | } | |||
| 108 | + | ||||
| 109 | + | func TestRateLimitMiddleware_TrustedProxy(t *testing.T) { | |||
| 110 | + | globalLimiter := outmocks.NewMockRateLimiter(t) | |||
| 111 | + | ipLimiter := outmocks.NewMockRateLimiter(t) | |||
| 112 | + | ||||
| 113 | + | // Request comes from trusted proxy (127.0.0.1) with XFF header | |||
| 114 | + | // Should use the X-Forwarded-For IP (203.0.113.50) for rate limiting | |||
| 115 | + | globalLimiter.EXPECT().Allow(context.Background(), "global").Return(true) | |||
| 116 | + | ipLimiter.EXPECT().Allow(context.Background(), "ip:203.0.113.50").Return(true) | |||
| 117 | + | ||||
| 118 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |||
| 119 | + | w.WriteHeader(http.StatusOK) | |||
| 120 | + | }) | |||
| 121 | + | ||||
| 122 | + | middleware := RateLimitMiddleware(globalLimiter, ipLimiter, []string{"127.0.0.1"}, testLogger()) | |||
| 123 | + | wrappedHandler := middleware(handler) | |||
| 124 | + | ||||
| 125 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |||
| 126 | + | req.RemoteAddr = "127.0.0.1:12345" | |||
| 127 | + | req.Header.Set("X-Forwarded-For", "203.0.113.50") | |||
| 128 | + | rec := httptest.NewRecorder() | |||
| 129 | + | wrappedHandler.ServeHTTP(rec, req) | |||
| 130 | + | ||||
| 131 | + | assert.Equal(t, http.StatusOK, rec.Code) | |||
| 132 | + | } | |||
| 133 | + | ||||
| 134 | + | func TestRateLimitMiddleware_UntrustedProxy(t *testing.T) { | |||
| 135 | + | globalLimiter := outmocks.NewMockRateLimiter(t) | |||
| 136 | + | ipLimiter := outmocks.NewMockRateLimiter(t) | |||
| 137 | + | ||||
| 138 | + | // Request from untrusted IP with XFF header should use RemoteAddr IP | |||
| 139 | + | globalLimiter.EXPECT().Allow(context.Background(), "global").Return(true) | |||
| 140 | + | ipLimiter.EXPECT().Allow(context.Background(), "ip:192.168.1.100").Return(true) | |||
| 141 | + | ||||
| 142 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |||
| 143 | + | w.WriteHeader(http.StatusOK) | |||
| 144 | + | }) | |||
| 145 | + | ||||
| 146 | + | middleware := RateLimitMiddleware(globalLimiter, ipLimiter, []string{"127.0.0.1"}, testLogger()) | |||
| 147 | + | wrappedHandler := middleware(handler) | |||
| 148 | + | ||||
| 149 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |||
| 150 | + | req.RemoteAddr = "192.168.1.100:12345" | |||
| 151 | + | req.Header.Set("X-Forwarded-For", "203.0.113.50") // Should be ignored | |||
| 152 | + | rec := httptest.NewRecorder() | |||
| 153 | + | wrappedHandler.ServeHTTP(rec, req) | |||
| 154 | + | ||||
| 155 | + | assert.Equal(t, http.StatusOK, rec.Code) | |||
| 92 | } | 156 | } | ||
| 93 | 157 | ||||
| 94 | func TestRateLimitMiddleware_ResponseFormat(t *testing.T) { | 158 | func TestRateLimitMiddleware_ResponseFormat(t *testing.T) { | ||
| 95 | - | config := RateLimitConfig{ | 159 | + | globalLimiter := outmocks.NewMockRateLimiter(t) |
| 96 | - | Enabled: true, | 160 | + | ipLimiter := outmocks.NewMockRateLimiter(t) |
| 97 | - | GlobalRPS: 1, | 161 | + | |
| 98 | - | PerIPRPS: 1, | 162 | + | globalLimiter.EXPECT().Allow(context.Background(), "global").Return(false) |
| 99 | - | Burst: 1, | 163 | + | |
| 100 | - | } | 164 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { |
| 165 | + | w.WriteHeader(http.StatusOK) | |||
| 166 | + | }) | |||
| 167 | + | ||||
| 168 | + | middleware := RateLimitMiddleware(globalLimiter, ipLimiter, nil, testLogger()) | |||
| 169 | + | wrappedHandler := middleware(handler) | |||
| 170 | + | ||||
| 171 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |||
| 172 | + | req.RemoteAddr = "192.168.1.100:12345" | |||
| 173 | + | rec := httptest.NewRecorder() | |||
| 174 | + | wrappedHandler.ServeHTTP(rec, req) | |||
| 175 | + | ||||
| 176 | + | // Verify response format | |||
| 177 | + | assert.Equal(t, http.StatusTooManyRequests, rec.Code) | |||
| 178 | + | assert.Equal(t, "application/json", rec.Header().Get("Content-Type")) | |||
| 179 | + | assert.Equal(t, "registry/2.0", rec.Header().Get("Docker-Distribution-API-Version")) | |||
| 180 | + | assert.Equal(t, "1", rec.Header().Get("Retry-After")) | |||
| 181 | + | assert.Contains(t, rec.Body.String(), "TOOMANYREQUESTS") | |||
| 182 | + | } | |||
| 101 | 183 | ||||
| 102 | - | handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | 184 | + | // Integration test using real MemoryStore implementation |
| 185 | + | func TestRateLimitMiddleware_Integration(t *testing.T) { | |||
| 186 | + | log := testLogger() | |||
| 187 | + | globalLimiter := ratelimit.NewMemoryStore(1000, 100, log) // High global limit | |||
| 188 | + | ipLimiter := ratelimit.NewMemoryStore(2, 2, log) // Low per-IP limit | |||
| 189 | + | ||||
| 190 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |||
| 103 | w.WriteHeader(http.StatusOK) | 191 | w.WriteHeader(http.StatusOK) | ||
| 104 | }) | 192 | }) | ||
| 105 | 193 | ||||
| 106 | - | middleware := RateLimitMiddleware(config) | 194 | + | middleware := RateLimitMiddleware(globalLimiter, ipLimiter, nil, log) |
| 107 | wrappedHandler := middleware(handler) | 195 | wrappedHandler := middleware(handler) | ||
| 108 | 196 | ||||
| 109 | - | // First request succeeds | 197 | + | // First 2 requests from same IP should succeed (burst) |
| 110 | - | req1 := httptest.NewRequest(http.MethodGet, "/v2/", nil) | 198 | + | for i := 0; i < 2; i++ { |
| 111 | - | req1.RemoteAddr = "192.168.1.100:12345" | 199 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) |
| 112 | - | rec1 := httptest.NewRecorder() | 200 | + | req.RemoteAddr = "192.168.1.100:12345" |
| 113 | - | wrappedHandler.ServeHTTP(rec1, req1) | 201 | + | rec := httptest.NewRecorder() |
| 202 | + | wrappedHandler.ServeHTTP(rec, req) | |||
| 203 | + | assert.Equal(t, http.StatusOK, rec.Code, "request %d should succeed", i+1) | |||
| 204 | + | } | |||
| 205 | + | ||||
| 206 | + | // 3rd request from same IP should be rate limited | |||
| 207 | + | req := httptest.NewRequest(http.MethodGet, "/v2/", nil) | |||
| 208 | + | req.RemoteAddr = "192.168.1.100:12345" | |||
| 209 | + | rec := httptest.NewRecorder() | |||
| 210 | + | wrappedHandler.ServeHTTP(rec, req) | |||
| 211 | + | assert.Equal(t, http.StatusTooManyRequests, rec.Code, "3rd request should be rate limited") | |||
| 114 | 212 | ||||
| 115 | - | // Second request gets rate limited | 213 | + | // Request from different IP should still succeed (independent limit) |
| 116 | req2 := httptest.NewRequest(http.MethodGet, "/v2/", nil) | 214 | req2 := httptest.NewRequest(http.MethodGet, "/v2/", nil) | ||
| 117 | - | req2.RemoteAddr = "192.168.1.100:12346" | 215 | + | req2.RemoteAddr = "192.168.1.101:12345" |
| 118 | rec2 := httptest.NewRecorder() | 216 | rec2 := httptest.NewRecorder() | ||
| 119 | wrappedHandler.ServeHTTP(rec2, req2) | 217 | wrappedHandler.ServeHTTP(rec2, req2) | ||
| 120 | - | 218 | + | assert.Equal(t, http.StatusOK, rec2.Code, "different IP should succeed") | |
| 121 | - | // Verify response format | |||
| 122 | - | assert.Equal(t, http.StatusTooManyRequests, rec2.Code) | |||
| 123 | - | assert.Equal(t, "application/json", rec2.Header().Get("Content-Type")) | |||
| 124 | - | assert.Equal(t, "registry/2.0", rec2.Header().Get("Docker-Distribution-API-Version")) | |||
| 125 | - | assert.Equal(t, "1", rec2.Header().Get("Retry-After")) | |||
| 126 | - | assert.Contains(t, rec2.Body.String(), "TOOMANYREQUESTS") | |||
| 127 | } | 219 | } | ||
| 128 | 220 | ||||
| 129 | func TestGetClientIP(t *testing.T) { | 221 | func TestGetClientIP(t *testing.T) { | ||
| @@ -286,12 +378,3 @@ func TestParseTrustedProxies(t *testing.T) { | |||||
| 286 | }) | 378 | }) | ||
| 287 | } | 379 | } | ||
| 288 | } | 380 | } | ||
| 289 | - | ||||
| 290 | - | func TestDefaultRateLimitConfig(t *testing.T) { | |||
| 291 | - | config := DefaultRateLimitConfig() | |||
| 292 | - | ||||
| 293 | - | require.True(t, config.Enabled) | |||
| 294 | - | assert.Equal(t, float64(500), config.GlobalRPS) | |||
| 295 | - | assert.Equal(t, float64(50), config.PerIPRPS) | |||
| 296 | - | assert.Equal(t, 100, config.Burst) | |||
| 297 | - | } | |||
M internal/app/run.go +14 -8
| @@ -27,6 +27,7 @@ import ( | |||
| 27 | 27 | "gordon/internal/adapters/out/filesystem" | |
| 28 | 28 | "gordon/internal/adapters/out/httpprober" | |
| 29 | 29 | "gordon/internal/adapters/out/logwriter" | |
| 30 | + | "gordon/internal/adapters/out/ratelimit" | |
| 30 | 31 | "gordon/internal/adapters/out/secrets" | |
| 31 | 32 | "gordon/internal/adapters/out/tokenstore" | |
| 32 | 33 | ||
| @@ -873,15 +874,20 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Ha | |||
| 873 | 874 | middleware.RequestLogger(log), | |
| 874 | 875 | } | |
| 875 | 876 | ||
| 876 | - | // Add rate limiting middleware | |
| 877 | - | rateLimitConfig := registry.RateLimitConfig{ | |
| 878 | - | Enabled: cfg.API.RateLimit.Enabled, | |
| 879 | - | GlobalRPS: cfg.API.RateLimit.GlobalRPS, | |
| 880 | - | PerIPRPS: cfg.API.RateLimit.PerIPRPS, | |
| 881 | - | Burst: cfg.API.RateLimit.Burst, | |
| 882 | - | TrustedProxies: cfg.API.RateLimit.TrustedProxies, | |
| 877 | + | // Create rate limiters using the factory | |
| 878 | + | var rateLimitMiddleware func(http.Handler) http.Handler | |
| 879 | + | if cfg.API.RateLimit.Enabled { | |
| 880 | + | globalLimiter := ratelimit.NewMemoryStore(cfg.API.RateLimit.GlobalRPS, cfg.API.RateLimit.Burst, log) | |
| 881 | + | ipLimiter := ratelimit.NewMemoryStore(cfg.API.RateLimit.PerIPRPS, cfg.API.RateLimit.Burst, log) | |
| 882 | + | rateLimitMiddleware = registry.RateLimitMiddleware( | |
| 883 | + | globalLimiter, | |
| 884 | + | ipLimiter, | |
| 885 | + | cfg.API.RateLimit.TrustedProxies, | |
| 886 | + | log, | |
| 887 | + | ) | |
| 888 | + | } else { | |
| 889 | + | rateLimitMiddleware = registry.RateLimitMiddleware(nil, nil, nil, log) | |
| 883 | 890 | } | |
| 884 | - | rateLimitMiddleware := registry.RateLimitMiddleware(rateLimitConfig) | |
| 885 | 891 | registryMiddlewares = append(registryMiddlewares, rateLimitMiddleware) | |
| 886 | 892 | ||
| 887 | 893 | if cfg.Auth.Enabled && svc.authSvc != nil { | |
| @@ -27,6 +27,7 @@ import ( | |||||
| 27 | "gordon/internal/adapters/out/filesystem" | 27 | "gordon/internal/adapters/out/filesystem" | ||
| 28 | "gordon/internal/adapters/out/httpprober" | 28 | "gordon/internal/adapters/out/httpprober" | ||
| 29 | "gordon/internal/adapters/out/logwriter" | 29 | "gordon/internal/adapters/out/logwriter" | ||
| 30 | + | "gordon/internal/adapters/out/ratelimit" | |||
| 30 | "gordon/internal/adapters/out/secrets" | 31 | "gordon/internal/adapters/out/secrets" | ||
| 31 | "gordon/internal/adapters/out/tokenstore" | 32 | "gordon/internal/adapters/out/tokenstore" | ||
| 32 | 33 | ||||
| @@ -873,15 +874,20 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Ha | |||||
| 873 | middleware.RequestLogger(log), | 874 | middleware.RequestLogger(log), | ||
| 874 | } | 875 | } | ||
| 875 | 876 | ||||
| 876 | - | // Add rate limiting middleware | 877 | + | // Create rate limiters using the factory |
| 877 | - | rateLimitConfig := registry.RateLimitConfig{ | 878 | + | var rateLimitMiddleware func(http.Handler) http.Handler |
| 878 | - | Enabled: cfg.API.RateLimit.Enabled, | 879 | + | if cfg.API.RateLimit.Enabled { |
| 879 | - | GlobalRPS: cfg.API.RateLimit.GlobalRPS, | 880 | + | globalLimiter := ratelimit.NewMemoryStore(cfg.API.RateLimit.GlobalRPS, cfg.API.RateLimit.Burst, log) |
| 880 | - | PerIPRPS: cfg.API.RateLimit.PerIPRPS, | 881 | + | ipLimiter := ratelimit.NewMemoryStore(cfg.API.RateLimit.PerIPRPS, cfg.API.RateLimit.Burst, log) |
| 881 | - | Burst: cfg.API.RateLimit.Burst, | 882 | + | rateLimitMiddleware = registry.RateLimitMiddleware( |
| 882 | - | TrustedProxies: cfg.API.RateLimit.TrustedProxies, | 883 | + | globalLimiter, |
| 884 | + | ipLimiter, | |||
| 885 | + | cfg.API.RateLimit.TrustedProxies, | |||
| 886 | + | log, | |||
| 887 | + | ) | |||
| 888 | + | } else { | |||
| 889 | + | rateLimitMiddleware = registry.RateLimitMiddleware(nil, nil, nil, log) | |||
| 883 | } | 890 | } | ||
| 884 | - | rateLimitMiddleware := registry.RateLimitMiddleware(rateLimitConfig) | |||
| 885 | registryMiddlewares = append(registryMiddlewares, rateLimitMiddleware) | 891 | registryMiddlewares = append(registryMiddlewares, rateLimitMiddleware) | ||
| 886 | 892 | ||||
| 887 | if cfg.Auth.Enabled && svc.authSvc != nil { | 893 | if cfg.Auth.Enabled && svc.authSvc != nil { | ||