Compare commits

...
3 Commits
Author SHA1 Message Date
github-actions[bot]andGitHub e138b44d00 chore(deps): update module go.yaml.in/yaml/v3 to v3.0.6
Automated dependency update. All tests passed.
2026-11-03 03:05:12 +00:00
lukaszraczylo 3a6403c30b fix(ratelimit): bound memory and CPU per request
The sustained gate stored one entry for every attempt, denied ones too, and summed all entries on each request. A flood grew memory with rate times interval and made each request slower. Both gates now keep only the newest limit plus one attempt times. A Lean proof shows this decides the same as the full count. One check dropped from 29,502 ns and 11 allocations to 43 ns and none. The go-ratecounter dependency is gone.
2026-11-03 00:55:35 +00:00
lukaszraczylo cff11f2b25 fix(redis): namespace cache keys when no prefix is set
With an empty prefix, Clear deleted every key in the Redis database, including keys of other applications. Cache keys now start with gmp: by default. Existing cache entries become unreachable after the upgrade and expire by TTL.
2026-11-03 00:49:02 +00:00
10 changed files with 210 additions and 129 deletions
+9 -1
View File
@@ -83,7 +83,15 @@ type RedisClientConfig struct {
// entry.
const defaultTTL = 60 * time.Second
// DefaultKeyPrefix namespaces cache keys when no prefix is configured, so Clear
// and CountQueries never touch keys of other applications on a shared Redis.
const DefaultKeyPrefix = "gmp:"
func New(redisClientConfig *RedisClientConfig) (*RedisConfig, error) {
prefix := redisClientConfig.Prefix
if prefix == "" {
prefix = DefaultKeyPrefix
}
c := &RedisConfig{
client: redis.NewClient(&redis.Options{
Addr: redisClientConfig.RedisServer,
@@ -91,7 +99,7 @@ func New(redisClientConfig *RedisClientConfig) (*RedisConfig, error) {
DB: redisClientConfig.RedisDB,
}),
ctx: context.Background(),
prefix: redisClientConfig.Prefix,
prefix: prefix,
builderPool: &sync.Pool{
New: func() any {
return &strings.Builder{}
+30
View File
@@ -0,0 +1,30 @@
package libpack_cache_redis
import (
"testing"
"github.com/alicebob/miniredis/v2"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// Pins theorems/Theorems/RedisClear.lean: with no configured prefix, Clear must still spare foreign keys.
func TestClear_DefaultPrefixSparesForeignKeys(t *testing.T) {
t.Parallel()
s, err := miniredis.Run()
require.NoError(t, err)
t.Cleanup(s.Close)
require.NoError(t, s.Set("other-app:session", "keep"))
c, err := New(&RedisClientConfig{RedisServer: s.Addr()})
require.NoError(t, err)
require.NoError(t, c.Set("q1", []byte("v"), 0))
require.NoError(t, c.Clear())
assert.True(t, s.Exists("other-app:session"), "foreign key was deleted")
_, found, err := c.Get("q1")
require.NoError(t, err)
assert.False(t, found, "own key must be cleared")
}
+4 -4
View File
@@ -131,11 +131,11 @@ func (suite *RedisConfigSuite) TestGetKeys() {
err = suite.redisConfig.Set("otherkey", []byte("othervalue"), ttl)
assert.NoError(suite.T(), err)
keys, _ := suite.redisConfig.client.Keys(suite.redisConfig.ctx, "testkey*").Result()
expectedKeys := []string{"testkey1", "testkey2"}
keys, _ := suite.redisConfig.client.Keys(suite.redisConfig.ctx, DefaultKeyPrefix+"testkey*").Result()
expectedKeys := []string{DefaultKeyPrefix + "testkey1", DefaultKeyPrefix + "testkey2"}
assert.ElementsMatch(suite.T(), expectedKeys, keys)
suite.redisConfig.client.Del(suite.redisConfig.ctx, "testkey1", "testkey2", "otherkey")
suite.redisConfig.client.Del(suite.redisConfig.ctx, DefaultKeyPrefix+"testkey1", DefaultKeyPrefix+"testkey2", DefaultKeyPrefix+"otherkey")
}
func (suite *RedisConfigSuite) TestGetKeysCount() {
@@ -154,5 +154,5 @@ func (suite *RedisConfigSuite) TestGetKeysCount() {
assert.NoError(suite.T(), err)
assert.Equal(suite.T(), int64(3), count3)
suite.redisConfig.client.Del(suite.redisConfig.ctx, "testkey1", "testkey2", "otherkey")
suite.redisConfig.client.Del(suite.redisConfig.ctx, DefaultKeyPrefix+"testkey1", DefaultKeyPrefix+"testkey2", DefaultKeyPrefix+"otherkey")
}
+1 -2
View File
@@ -20,7 +20,6 @@ require (
github.com/graphql-go/graphql v0.8.1
github.com/jackc/pgx/v5 v5.12.0
github.com/lukaszraczylo/ask v0.0.0-20240916204100-6e9ef53a62d9
github.com/lukaszraczylo/go-ratecounter v0.1.13
github.com/lukaszraczylo/go-simple-graphql v1.2.89
github.com/lukaszraczylo/oss-telemetry v0.2.5
github.com/redis/go-redis/v9 v9.23.0
@@ -66,7 +65,7 @@ require (
go.opentelemetry.io/otel/metric v1.47.0 // indirect
go.opentelemetry.io/proto/otlp v1.11.1 // indirect
go.uber.org/atomic v1.12.0 // indirect
go.yaml.in/yaml/v3 v3.0.5 // indirect
go.yaml.in/yaml/v3 v3.0.6 // indirect
golang.org/x/crypto v0.58.0 // indirect
golang.org/x/net v0.62.0 // indirect
golang.org/x/sync v0.24.0 // indirect
+2 -4
View File
@@ -72,8 +72,6 @@ github.com/klauspost/cpuid/v2 v2.4.0 h1:S6Hrbc7+ywsr0r+RLapfGBHfyefhCTwEh3A0tV91
github.com/klauspost/cpuid/v2 v2.4.0/go.mod h1:19jmZ9mjzoF//ddRSUsv0zfBTJWh3QJh9FNxZTMrGxU=
github.com/lukaszraczylo/ask v0.0.0-20240916204100-6e9ef53a62d9 h1:pL8B9mjv6RPUfKYYGm/uJ7QL6Ndf+z+OEl0qJE6KmEc=
github.com/lukaszraczylo/ask v0.0.0-20240916204100-6e9ef53a62d9/go.mod h1:M+UVdyqZs++xtEPrascaVmZdOMhCnxjZ2SgH+xHpR0c=
github.com/lukaszraczylo/go-ratecounter v0.1.13 h1:p1r53oyMb9ZnMbqub6IF4CWhURFiz7k967I0GbUyQMA=
github.com/lukaszraczylo/go-ratecounter v0.1.13/go.mod h1:TqXEOCtFJStk1i0tkipprv1kiDHGon1MVUisjSTBSKM=
github.com/lukaszraczylo/go-simple-graphql v1.2.89 h1:Xbu1Ny+a0lT2Sr2SaSC8mcHmGQDwGD4TJKk4DDd+PwA=
github.com/lukaszraczylo/go-simple-graphql v1.2.89/go.mod h1:PxQYblQDZISmYYj8sNfazAWxAOh1rhAtU208y+uPV8s=
github.com/lukaszraczylo/oss-telemetry v0.2.5 h1:W6Qgwp7IY4yBRO361MmnEAmM59motdQpbAFAx8v6fJM=
@@ -144,8 +142,8 @@ go.uber.org/automaxprocs v1.6.0 h1:O3y2/QNTOdbF+e/dpXNNW7Rx2hZ4sTIPyybbxyNqTUs=
go.uber.org/automaxprocs v1.6.0/go.mod h1:ifeIMSnPZuznNm6jmdzmU3/bfk01Fe2fotchwEFJ8r8=
go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw=
go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg=
go.yaml.in/yaml/v3 v3.0.6 h1:zcDfvjXEqTQmpjniUBLc91xYJMB2c4QOFxEF/LCEckM=
go.yaml.in/yaml/v3 v3.0.6/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg=
golang.org/x/crypto v0.58.0 h1:COkYLr4k7nDI4r6QtMML2AfzeIswWHAJthLS6K/M/54=
golang.org/x/crypto v0.58.0/go.mod h1:Xh+kl5A+M0gMiWDytH2Hhr5Z7WbNCpv/eB/EFgfSk+w=
golang.org/x/net v0.62.0 h1:18ELalfM5+XNRwzM2nYqXsZQ9ByS1uhj2O/U7+5jqRY=
+34 -89
View File
@@ -8,17 +8,17 @@ import (
"time"
"github.com/goccy/go-json"
goratecounter "github.com/lukaszraczylo/go-ratecounter"
libpack_logger "github.com/lukaszraczylo/graphql-monitoring-proxy/logging"
)
// RateLimitConfig holds the rate limit configuration for a role
type RateLimitConfig struct {
RateCounterTicker *goratecounter.RateCounter
// sustained enforces Req over the full Interval.
sustained *attemptWindow
// burstCounter enforces Burst over a short trailing window, independent
// of RateCounterTicker's sustained-rate window. Nil when Burst <= 0
// of the sustained window. Nil when Burst <= 0
// (burst disabled). See burstWindow and checkRateLimit for semantics.
burstCounter *burstWindow
burstCounter *attemptWindow
Endpoints []string `json:"endpoints,omitempty"`
Interval time.Duration `json:"interval"`
Req int `json:"req"`
@@ -107,51 +107,6 @@ func burstWindowDuration(interval time.Duration) time.Duration {
return w
}
// burstWindow is a minimal, clock-injectable, trailing-window request
// counter used to enforce RateLimitConfig.Burst independently of the
// sustained-rate RateCounterTicker.
//
// [email protected] cannot provide this second, shorter window
// cleanly: the window length lives on *RateCounter itself, not per named
// Counter (helpers.go: GetRate -> c.parent.interval.Seconds()), so two
// different window lengths need two separate RateCounter instances. Each
// instance drives its own background goroutine off wall-clock time.Now with
// no seam to inject a fake clock (helpers.go: start(), cleanUpOldValues(),
// addValue() all call time.Now() directly). That would force any test of
// window rollover to sleep on real time. burstWindow is implemented here
// instead, with a swappable clock, so tests can advance time deterministically.
type burstWindow struct {
mu sync.Mutex
now func() time.Time
window time.Duration
hits []time.Time
}
// newBurstWindow returns a burstWindow that tracks hits over the trailing
// duration window, using the real wall clock.
func newBurstWindow(window time.Duration) *burstWindow {
return &burstWindow{now: time.Now, window: window}
}
// recordAndCount records a hit at the current time, drops hits older than
// the trailing window, and returns the resulting hit count within the
// window. Safe for concurrent use.
func (b *burstWindow) recordAndCount() int {
b.mu.Lock()
defer b.mu.Unlock()
now := b.now()
cutoff := now.Add(-b.window)
live := b.hits[:0]
for _, t := range b.hits {
if t.After(cutoff) {
live = append(live, t)
}
}
live = append(live, now)
b.hits = live
return len(b.hits)
}
// Variable to hold the current load config function - allows for testing
var loadConfigFunc = loadConfigFromPath
@@ -237,14 +192,12 @@ func loadConfigFromPath(path string) error {
newRateLimits := make(map[string]RateLimitConfig, len(config.RateLimit))
for key, value := range config.RateLimit {
value.RateCounterTicker = goratecounter.NewRateCounter().WithConfig(goratecounter.RateCounterConfig{
Interval: value.Interval,
})
value.sustained = newAttemptWindow(value.Interval, value.Req)
// Burst disabled (<=0) leaves burstCounter nil, so checkRateLimit
// performs the Req-only check exactly as before burst existed.
if value.Burst > 0 {
value.burstCounter = newBurstWindow(burstWindowDuration(value.Interval))
value.burstCounter = newAttemptWindow(burstWindowDuration(value.Interval), value.Burst)
}
if cfg.LogLevel == "DEBUG" {
@@ -279,7 +232,7 @@ func rateLimitedRequest(userID, userRole string) bool {
// Try to get config from atomic value first for better performance
if configInterface := rateLimitConfigAtomic.Load(); configInterface != nil {
if config, ok := configInterface.(map[string]RateLimitConfig); ok {
if roleConfig, exists := config[userRole]; exists && roleConfig.RateCounterTicker != nil {
if roleConfig, exists := config[userRole]; exists && roleConfig.sustained != nil {
return checkRateLimit(userID, userRole, roleConfig, "")
}
}
@@ -290,7 +243,7 @@ func rateLimitedRequest(userID, userRole string) bool {
roleConfig, ok := rateLimits[userRole]
rateLimitMu.RUnlock()
if !ok || roleConfig.RateCounterTicker == nil {
if !ok || roleConfig.sustained == nil {
cfg.Logger.Warning(&libpack_logger.LogMessage{
Message: "Rate limit role not found or ticker not initialized - defaulting to deny",
Pairs: map[string]any{"user_role": userRole},
@@ -315,45 +268,37 @@ func rateLimitedRequest(userID, userRole string) bool {
// sustained count. Burst >= Req therefore never rejects anything the sustained
// gate admits; Burst only takes effect when it is below Req.
func checkRateLimit(userID, userRole string, roleConfig RateLimitConfig, endpoint string) bool {
roleConfig.RateCounterTicker.Incr(1)
tickerRate := roleConfig.RateCounterTicker.Get()
debug := cfg.Logger.IsLevelEnabled(libpack_logger.LEVEL_DEBUG)
logDetails := map[string]any{
"user_role": userRole,
"user_id": userID,
"rate": tickerRate,
"config_rate": roleConfig.Req,
"interval": roleConfig.Interval,
"endpoint": endpoint,
}
cfg.Logger.Debug(&libpack_logger.LogMessage{
Message: "Rate limit ticker",
Pairs: map[string]any{"log_details": logDetails},
})
if tickerRate > int64(roleConfig.Req) {
cfg.Logger.Debug(&libpack_logger.LogMessage{
Message: "Rate limit exceeded",
Pairs: map[string]any{"log_details": logDetails},
})
if roleConfig.sustained.recordAndExceeds() {
if debug {
cfg.Logger.Debug(&libpack_logger.LogMessage{
Message: "Rate limit exceeded",
Pairs: map[string]any{
"user_role": userRole,
"user_id": userID,
"config_rate": roleConfig.Req,
"interval": roleConfig.Interval,
"endpoint": endpoint,
},
})
}
return false
}
if roleConfig.burstCounter != nil {
burstCount := roleConfig.burstCounter.recordAndCount()
burstLogDetails := map[string]any{
"user_role": userRole,
"user_id": userID,
"burst_count": burstCount,
"config_burst": roleConfig.Burst,
"endpoint": endpoint,
}
if burstCount > roleConfig.Burst {
cfg.Logger.Debug(&libpack_logger.LogMessage{
Message: "Burst limit exceeded",
Pairs: map[string]any{"log_details": burstLogDetails},
})
if roleConfig.burstCounter.recordAndExceeds() {
if debug {
cfg.Logger.Debug(&libpack_logger.LogMessage{
Message: "Burst limit exceeded",
Pairs: map[string]any{
"user_role": userRole,
"user_id": userID,
"config_burst": roleConfig.Burst,
"endpoint": endpoint,
},
})
}
return false
}
}
+14 -23
View File
@@ -10,7 +10,6 @@ import (
"time"
"github.com/goccy/go-json"
goratecounter "github.com/lukaszraczylo/go-ratecounter"
libpack_logger "github.com/lukaszraczylo/graphql-monitoring-proxy/logging"
)
@@ -65,8 +64,8 @@ func (suite *Tests) Test_loadRatelimitConfig() {
suite.Contains(rateLimits, "user")
suite.Equal(100, rateLimits["admin"].Req)
suite.Equal(10, rateLimits["user"].Req)
suite.NotNil(rateLimits["admin"].RateCounterTicker)
suite.NotNil(rateLimits["user"].RateCounterTicker)
suite.NotNil(rateLimits["admin"].sustained)
suite.NotNil(rateLimits["user"].sustained)
})
// Test loading config from non-existent path
@@ -154,23 +153,17 @@ func (suite *Tests) Test_rateLimitedRequest() {
rateLimits = make(map[string]RateLimitConfig)
// Admin role with high limit
adminCounter := goratecounter.NewRateCounter().WithConfig(goratecounter.RateCounterConfig{
Interval: 1 * time.Second,
})
rateLimits["admin"] = RateLimitConfig{
RateCounterTicker: adminCounter,
Interval: 1 * time.Second,
Req: 100,
sustained: newAttemptWindow(1*time.Second, 100),
Interval: 1 * time.Second,
Req: 100,
}
// User role with low limit
userCounter := goratecounter.NewRateCounter().WithConfig(goratecounter.RateCounterConfig{
Interval: 1 * time.Second,
})
rateLimits["user"] = RateLimitConfig{
RateCounterTicker: userCounter,
Interval: 1 * time.Second,
Req: 2, // Set very low for testing
sustained: newAttemptWindow(1*time.Second, 2),
Interval: 1 * time.Second,
Req: 2, // Set very low for testing
}
rateLimitMu.Unlock()
@@ -327,19 +320,17 @@ func (f *fakeClock) Advance(d time.Duration) {
}
// newBurstTestRoleConfig mirrors the RateLimitConfig construction performed
// by loadConfigFromPath: a real RateCounterTicker for the sustained check,
// by loadConfigFromPath: a real sustained window for the Req check,
// and (only when burst > 0) a burstCounter sized by burstWindowDuration.
func newBurstTestRoleConfig(interval time.Duration, req, burst int) RateLimitConfig {
rc := RateLimitConfig{
RateCounterTicker: goratecounter.NewRateCounter().WithConfig(goratecounter.RateCounterConfig{
Interval: interval,
}),
Interval: interval,
Req: req,
Burst: burst,
sustained: newAttemptWindow(interval, req),
Interval: interval,
Req: req,
Burst: burst,
}
if burst > 0 {
rc.burstCounter = newBurstWindow(burstWindowDuration(interval))
rc.burstCounter = newAttemptWindow(burstWindowDuration(interval), burst)
}
return rc
}
+49
View File
@@ -0,0 +1,49 @@
package main
import (
"sync"
"time"
)
// attemptWindow decides "more than limit attempts in the trailing window" while
// storing only the newest limit+1 attempt times. The attempt number limit+1
// counted back from now lies inside the window exactly when the full count
// exceeds limit (see theorems/Theorems/SlidingWindow.lean), so memory and time
// per attempt stay constant however hard a client floods.
type attemptWindow struct {
now func() time.Time
base time.Time // monotonic origin for stored offsets
ring []int64 // offsets from base, grows to capacity then wraps
mu sync.Mutex
window time.Duration
head int // index of the oldest stored attempt once the ring is full
size int // limit+1: attempts kept; the ring grows lazily up to this
}
func newAttemptWindow(window time.Duration, limit int) *attemptWindow {
return &attemptWindow{
now: time.Now,
base: time.Now(),
window: window,
size: max(limit, 0) + 1,
}
}
// recordAndExceeds records an attempt now and reports whether the attempts in
// the trailing window, this one included, outnumber the limit.
func (w *attemptWindow) recordAndExceeds() bool {
w.mu.Lock()
defer w.mu.Unlock()
t := int64(w.now().Sub(w.base))
if len(w.ring) < w.size {
w.ring = append(w.ring, t)
} else {
w.ring[w.head] = t
w.head = (w.head + 1) % len(w.ring)
}
if len(w.ring) < w.size {
return false
}
return w.ring[w.head] > t-int64(w.window)
}
+64
View File
@@ -0,0 +1,64 @@
package main
import (
"math/rand"
"testing"
"time"
)
// naiveExceeds is the exact reference: count every attempt in the trailing window.
func naiveExceeds(attempts []time.Time, now time.Time, window time.Duration, limit int) bool {
cutoff := now.Add(-window)
n := 0
for _, t := range attempts {
if t.After(cutoff) {
n++
}
}
return n > limit
}
// Pins theorems/Theorems/SlidingWindow.lean: storing the newest limit+1 attempts decides like the full count.
func TestAttemptWindow_MatchesExactCount(t *testing.T) {
for _, limit := range []int{0, 1, 3, 10} {
rng := rand.New(rand.NewSource(int64(limit) + 1))
window := 10 * time.Second
fc := newFakeClock(time.Unix(1_000_000, 0))
w := newAttemptWindow(window, limit)
w.now = fc.Now
w.base = fc.Now()
var attempts []time.Time
for i := 0; i < 5000; i++ {
fc.Advance(time.Duration(rng.Intn(3000)) * time.Millisecond)
attempts = append(attempts, fc.Now())
want := naiveExceeds(attempts, fc.Now(), window, limit)
if got := w.recordAndExceeds(); got != want {
t.Fatalf("limit=%d step=%d: got %v, want %v", limit, i, got, want)
}
}
}
}
func TestAttemptWindow_StorageIsBounded(t *testing.T) {
const limit = 10
fc := newFakeClock(time.Unix(1_000_000, 0))
w := newAttemptWindow(time.Hour, limit)
w.now = fc.Now
w.base = fc.Now()
for i := 0; i < 100_000; i++ {
w.recordAndExceeds()
fc.Advance(time.Millisecond)
}
if got := len(w.ring); got > limit+1 {
t.Fatalf("stored %d timestamps, want <= %d", got, limit+1)
}
}
func BenchmarkAttemptWindow(b *testing.B) {
w := newAttemptWindow(time.Hour, 1000)
b.ReportAllocs()
for i := 0; i < b.N; i++ {
w.recordAndExceeds()
}
}
+3 -6
View File
@@ -11,7 +11,6 @@ import (
fiber "github.com/gofiber/fiber/v3"
gorillaws "github.com/gorilla/websocket"
goratecounter "github.com/lukaszraczylo/go-ratecounter"
libpack_logger "github.com/lukaszraczylo/graphql-monitoring-proxy/logging"
libpack_monitoring "github.com/lukaszraczylo/graphql-monitoring-proxy/monitoring"
"github.com/stretchr/testify/assert"
@@ -652,11 +651,9 @@ func TestHandleWebSocketOrDefault_RateLimitedUserRejected(t *testing.T) {
rateLimitMu.Lock()
rateLimits = map[string]RateLimitConfig{
defaultValue: {
RateCounterTicker: goratecounter.NewRateCounter().WithConfig(goratecounter.RateCounterConfig{
Interval: 1 * time.Second,
}),
Interval: 1 * time.Second,
Req: 0,
sustained: newAttemptWindow(1*time.Second, 0),
Interval: 1 * time.Second,
Req: 0,
},
}
rateLimitConfigAtomic.Store(rateLimits)