diff --git a/management/server/http/middleware/rate_limiter.go b/management/server/http/middleware/rate_limiter.go index 30d863add..bfd44afee 100644 --- a/management/server/http/middleware/rate_limiter.go +++ b/management/server/http/middleware/rate_limiter.go @@ -57,6 +57,10 @@ func RateLimiterConfigFromEnv() (cfg *RateLimiterConfig, enabled bool) { rpm = value } } + if rpm <= 0 { + log.Warnf("%s=%d is non-positive, using default %d", RateLimitingRPMEnv, rpm, defaultAPIRPM) + rpm = defaultAPIRPM + } burst := defaultAPIBurst if v := os.Getenv(RateLimitingBurstEnv); v != "" { @@ -67,6 +71,10 @@ func RateLimiterConfigFromEnv() (cfg *RateLimiterConfig, enabled bool) { burst = value } } + if burst <= 0 { + log.Warnf("%s=%d is non-positive, using default %d", RateLimitingBurstEnv, burst, defaultAPIBurst) + burst = defaultAPIBurst + } return &RateLimiterConfig{ RequestsPerMinute: float64(rpm), @@ -121,6 +129,10 @@ func (rl *APIRateLimiter) UpdateConfig(config *RateLimiterConfig) { if config == nil { return } + if config.RequestsPerMinute <= 0 || config.Burst <= 0 { + log.Warnf("UpdateConfig: ignoring invalid rpm=%v burst=%d", config.RequestsPerMinute, config.Burst) + return + } newRPS := rate.Limit(config.RequestsPerMinute / 60.0) newBurst := config.Burst diff --git a/management/server/http/middleware/rate_limiter_test.go b/management/server/http/middleware/rate_limiter_test.go index 0497377af..4b97d1874 100644 --- a/management/server/http/middleware/rate_limiter_test.go +++ b/management/server/http/middleware/rate_limiter_test.go @@ -226,6 +226,27 @@ func TestAPIRateLimiter_UpdateConfig_NilIgnored(t *testing.T) { assert.False(t, rl.Allow("k")) } +func TestAPIRateLimiter_UpdateConfig_NonPositiveIgnored(t *testing.T) { + rl := NewAPIRateLimiter(&RateLimiterConfig{ + RequestsPerMinute: 60, + Burst: 1, + CleanupInterval: time.Minute, + LimiterTTL: time.Minute, + }) + defer rl.Stop() + + assert.True(t, rl.Allow("k")) + assert.False(t, rl.Allow("k")) + + rl.UpdateConfig(&RateLimiterConfig{RequestsPerMinute: 0, Burst: 0, CleanupInterval: time.Minute, LimiterTTL: time.Minute}) + rl.UpdateConfig(&RateLimiterConfig{RequestsPerMinute: -1, Burst: 5, CleanupInterval: time.Minute, LimiterTTL: time.Minute}) + rl.UpdateConfig(&RateLimiterConfig{RequestsPerMinute: 60, Burst: -1, CleanupInterval: time.Minute, LimiterTTL: time.Minute}) + + rl.Reset("k") + assert.True(t, rl.Allow("k")) + assert.False(t, rl.Allow("k"), "burst should still be 1 — invalid UpdateConfig calls were ignored") +} + func TestAPIRateLimiter_ConcurrentAllowAndUpdate(t *testing.T) { rl := NewAPIRateLimiter(&RateLimiterConfig{ RequestsPerMinute: 600, @@ -299,4 +320,10 @@ func TestRateLimiterConfigFromEnv(t *testing.T) { assert.False(t, enabled) assert.Equal(t, float64(defaultAPIRPM), cfg.RequestsPerMinute) assert.Equal(t, defaultAPIBurst, cfg.Burst) + + t.Setenv(RateLimitingRPMEnv, "0") + t.Setenv(RateLimitingBurstEnv, "-5") + cfg, _ = RateLimiterConfigFromEnv() + assert.Equal(t, float64(defaultAPIRPM), cfg.RequestsPerMinute, "non-positive rpm must fall back to default") + assert.Equal(t, defaultAPIBurst, cfg.Burst, "non-positive burst must fall back to default") }