From 37f742aef23fa029731062549eac8de1957f7a0e Mon Sep 17 00:00:00 2001 From: Sam Christen Oliphant Date: Wed, 5 Mar 2025 08:52:20 -0500 Subject: [PATCH 1/7] proof of concept --- router/core/ratelimiter.go | 22 ++++++++++++++----- .../operationstorage/redis/rdcloser_test.go | 3 ++- router/pkg/config/config.go | 1 + router/pkg/config/config.schema.json | 5 +++++ .../pkg/config/testdata/config_defaults.json | 3 ++- router/pkg/config/testdata/config_full.json | 3 ++- 6 files changed, 28 insertions(+), 9 deletions(-) diff --git a/router/core/ratelimiter.go b/router/core/ratelimiter.go index 2e2b0d2268..39abb79dff 100644 --- a/router/core/ratelimiter.go +++ b/router/core/ratelimiter.go @@ -6,10 +6,11 @@ import ( "encoding/json" "errors" "fmt" - rd "github.com/wundergraph/cosmo/router/internal/persistedoperation/operationstorage/redis" "io" "sync" + rd "github.com/wundergraph/cosmo/router/internal/persistedoperation/operationstorage/redis" + "github.com/expr-lang/expr/vm" "github.com/go-redis/redis_rate/v10" "github.com/wundergraph/cosmo/router/internal/expr" @@ -23,10 +24,9 @@ var ( type CosmoRateLimiterOptions struct { RedisClient rd.RDCloser Debug bool - RejectStatusCode int - KeySuffixExpression string + FailOpen bool } func NewCosmoRateLimiter(opts *CosmoRateLimiterOptions) (rl *CosmoRateLimiter, err error) { @@ -36,6 +36,10 @@ func NewCosmoRateLimiter(opts *CosmoRateLimiterOptions) (rl *CosmoRateLimiter, e limiter: limiter, debug: opts.Debug, rejectStatusCode: opts.RejectStatusCode, + failOpen: opts.FailOpen, + } + if err != nil { + return nil, err } if rl.rejectStatusCode == 0 { rl.rejectStatusCode = 200 @@ -57,6 +61,8 @@ type CosmoRateLimiter struct { rejectStatusCode int keySuffixProgram *vm.Program + + failOpen bool } func (c *CosmoRateLimiter) RateLimitPreFetch(ctx *resolve.Context, info *resolve.FetchInfo, input json.RawMessage) (result *resolve.RateLimitDeny, err error) { @@ -70,11 +76,15 @@ func (c *CosmoRateLimiter) RateLimitPreFetch(ctx *resolve.Context, info *resolve Period: ctx.RateLimitOptions.Period, } key, err := c.generateKey(ctx) - if err != nil { + if err != nil && c.failOpen == true{ + return nil, nil + } else if err != nil { return nil, err } allow, err := c.limiter.AllowN(ctx.Context(), key, limit, requestRate) - if err != nil { + if err != nil && c.failOpen == true{ + return nil, nil + } else if err != nil { return nil, err } c.setRateLimitStats(ctx, key, requestRate, allow.Remaining, allow.RetryAfter.Milliseconds(), allow.ResetAfter.Milliseconds()) @@ -190,4 +200,4 @@ func WithRateLimiterStats(ctx *resolve.Context) *resolve.Context { stats := &rateLimitStatsCtx{} withStats := context.WithValue(ctx.Context(), rateLimitStatsCtxKey{}, stats) return ctx.WithContext(withStats) -} +} \ No newline at end of file diff --git a/router/internal/persistedoperation/operationstorage/redis/rdcloser_test.go b/router/internal/persistedoperation/operationstorage/redis/rdcloser_test.go index 6bd4cbd807..743e1750c1 100644 --- a/router/internal/persistedoperation/operationstorage/redis/rdcloser_test.go +++ b/router/internal/persistedoperation/operationstorage/redis/rdcloser_test.go @@ -2,10 +2,11 @@ package rd import ( "fmt" + "testing" + "github.com/alicebob/miniredis/v2" "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" - "testing" ) func TestRedisCloser(t *testing.T) { diff --git a/router/pkg/config/config.go b/router/pkg/config/config.go index 8f40676bc2..43bc754034 100644 --- a/router/pkg/config/config.go +++ b/router/pkg/config/config.go @@ -449,6 +449,7 @@ type RateLimitConfiguration struct { Debug bool `yaml:"debug" envDefault:"false" env:"RATE_LIMIT_DEBUG"` KeySuffixExpression string `yaml:"key_suffix_expression,omitempty" env:"RATE_LIMIT_KEY_SUFFIX_EXPRESSION"` ErrorExtensionCode RateLimitErrorExtensionCode `yaml:"error_extension_code"` + FailOpen bool `yaml:"fail_open" env:"RATE_LIMIT_FAIL_OPEN"` } type RateLimitErrorExtensionCode struct { diff --git a/router/pkg/config/config.schema.json b/router/pkg/config/config.schema.json index db09cec9e4..6cb389eeed 100644 --- a/router/pkg/config/config.schema.json +++ b/router/pkg/config/config.schema.json @@ -1751,6 +1751,11 @@ "default": "RATE_LIMIT_EXCEEDED" } } + }, + "fail_open": { + "type": "boolean", + "description": "Enable Rate Limit fail open on redis availability failure. This interacts with Redis timeout configuration parameters, essentially adding to each requests latency in failure.", + "default": false } } }, diff --git a/router/pkg/config/testdata/config_defaults.json b/router/pkg/config/testdata/config_defaults.json index 9cbc593802..913334b9a1 100644 --- a/router/pkg/config/testdata/config_defaults.json +++ b/router/pkg/config/testdata/config_defaults.json @@ -218,7 +218,8 @@ "ErrorExtensionCode": { "Enabled": true, "Code": "RATE_LIMIT_EXCEEDED" - } + }, + "FailOpen": false }, "LocalhostFallbackInsideDocker": true, "CDN": { diff --git a/router/pkg/config/testdata/config_full.json b/router/pkg/config/testdata/config_full.json index 0cdbc10142..a387687db9 100644 --- a/router/pkg/config/testdata/config_full.json +++ b/router/pkg/config/testdata/config_full.json @@ -436,7 +436,8 @@ "ErrorExtensionCode": { "Enabled": true, "Code": "RATE_LIMIT_EXCEEDED" - } + }, + "FailOpen": false }, "LocalhostFallbackInsideDocker": true, "CDN": { From 89923f037836521e4a6423706ef9131e0ec36e21 Mon Sep 17 00:00:00 2001 From: Sam Christen Oliphant Date: Wed, 5 Mar 2025 09:08:18 -0500 Subject: [PATCH 2/7] add failOpen log on client init --- router/core/router.go | 1 + 1 file changed, 1 insertion(+) diff --git a/router/core/router.go b/router/core/router.go index e0c72539e0..5eeff554f9 100644 --- a/router/core/router.go +++ b/router/core/router.go @@ -1216,6 +1216,7 @@ func (r *Router) Start(ctx context.Context) error { zap.Int("burst", r.rateLimit.SimpleStrategy.Burst), zap.Duration("duration", r.Config.rateLimit.SimpleStrategy.Period), zap.Bool("rejectExceeding", r.Config.rateLimit.SimpleStrategy.RejectExceedingRequests), + zap.Bool("failOpen", r.Config.rateLimit.FailOpen), ) } From f7aa17c06e8413714d6f8e32ee26de917db7cfdf Mon Sep 17 00:00:00 2001 From: Sam Christen Oliphant Date: Wed, 5 Mar 2025 09:24:07 -0500 Subject: [PATCH 3/7] linter moving things around --- router/core/ratelimiter.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/router/core/ratelimiter.go b/router/core/ratelimiter.go index 39abb79dff..6e490be795 100644 --- a/router/core/ratelimiter.go +++ b/router/core/ratelimiter.go @@ -9,11 +9,10 @@ import ( "io" "sync" - rd "github.com/wundergraph/cosmo/router/internal/persistedoperation/operationstorage/redis" - "github.com/expr-lang/expr/vm" "github.com/go-redis/redis_rate/v10" "github.com/wundergraph/cosmo/router/internal/expr" + rd "github.com/wundergraph/cosmo/router/internal/persistedoperation/operationstorage/redis" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve" ) From f5b23f0b5fa260674197aaf35ef32993c5ded642 Mon Sep 17 00:00:00 2001 From: Sam Christen Oliphant <127764018+Sam-tesouro@users.noreply.github.com> Date: Wed, 5 Mar 2025 09:35:49 -0500 Subject: [PATCH 4/7] Update rdcloser_test.go linter has lots of opinions --- .../persistedoperation/operationstorage/redis/rdcloser_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/router/internal/persistedoperation/operationstorage/redis/rdcloser_test.go b/router/internal/persistedoperation/operationstorage/redis/rdcloser_test.go index 743e1750c1..6bd4cbd807 100644 --- a/router/internal/persistedoperation/operationstorage/redis/rdcloser_test.go +++ b/router/internal/persistedoperation/operationstorage/redis/rdcloser_test.go @@ -2,11 +2,10 @@ package rd import ( "fmt" - "testing" - "github.com/alicebob/miniredis/v2" "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" + "testing" ) func TestRedisCloser(t *testing.T) { From 91bd66359e0270b2979d7f47a38605b85e918eae Mon Sep 17 00:00:00 2001 From: Sam Christen Oliphant Date: Fri, 7 Mar 2025 22:03:32 -0500 Subject: [PATCH 5/7] mvp --- router/core/graph_server.go | 1 + router/core/ratelimiter.go | 17 ++++++++++------- router/core/router.go | 1 + .../operationstorage/redis/rdcloser.go | 10 ++++++++-- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/router/core/graph_server.go b/router/core/graph_server.go index 0f8ae95f0d..dcdcb609f1 100644 --- a/router/core/graph_server.go +++ b/router/core/graph_server.go @@ -1042,6 +1042,7 @@ func (s *graphServer) buildGraphMux(ctx context.Context, Debug: s.rateLimit.Debug, RejectStatusCode: s.rateLimit.SimpleStrategy.RejectStatusCode, KeySuffixExpression: s.rateLimit.KeySuffixExpression, + FailOpen: s.rateLimit.FailOpen, }) if err != nil { return nil, fmt.Errorf("failed to create rate limiter: %w", err) diff --git a/router/core/ratelimiter.go b/router/core/ratelimiter.go index 6e490be795..3e645f3cd1 100644 --- a/router/core/ratelimiter.go +++ b/router/core/ratelimiter.go @@ -9,10 +9,11 @@ import ( "io" "sync" + rd "github.com/wundergraph/cosmo/router/internal/persistedoperation/operationstorage/redis" + "github.com/expr-lang/expr/vm" "github.com/go-redis/redis_rate/v10" "github.com/wundergraph/cosmo/router/internal/expr" - rd "github.com/wundergraph/cosmo/router/internal/persistedoperation/operationstorage/redis" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve" ) @@ -23,9 +24,12 @@ var ( type CosmoRateLimiterOptions struct { RedisClient rd.RDCloser Debug bool + RejectStatusCode int + KeySuffixExpression string - FailOpen bool + + FailOpen bool } func NewCosmoRateLimiter(opts *CosmoRateLimiterOptions) (rl *CosmoRateLimiter, err error) { @@ -56,11 +60,10 @@ type CosmoRateLimiter struct { client rd.RDCloser limiter *redis_rate.Limiter debug bool - rejectStatusCode int keySuffixProgram *vm.Program - + failOpen bool } @@ -75,13 +78,13 @@ func (c *CosmoRateLimiter) RateLimitPreFetch(ctx *resolve.Context, info *resolve Period: ctx.RateLimitOptions.Period, } key, err := c.generateKey(ctx) - if err != nil && c.failOpen == true{ + if err != nil && c.failOpen{ return nil, nil } else if err != nil { return nil, err } allow, err := c.limiter.AllowN(ctx.Context(), key, limit, requestRate) - if err != nil && c.failOpen == true{ + if err != nil && c.failOpen{ return nil, nil } else if err != nil { return nil, err @@ -199,4 +202,4 @@ func WithRateLimiterStats(ctx *resolve.Context) *resolve.Context { stats := &rateLimitStatsCtx{} withStats := context.WithValue(ctx.Context(), rateLimitStatsCtxKey{}, stats) return ctx.WithContext(withStats) -} \ No newline at end of file +} diff --git a/router/core/router.go b/router/core/router.go index 5eeff554f9..9138dd6943 100644 --- a/router/core/router.go +++ b/router/core/router.go @@ -867,6 +867,7 @@ func (r *Router) bootstrap(ctx context.Context) error { URLs: r.Config.rateLimit.Storage.URLs, ClusterEnabled: r.Config.rateLimit.Storage.ClusterEnabled, Logger: r.logger, + FailOpen: r.Config.rateLimit.FailOpen, }) if err != nil { return fmt.Errorf("failed to create redis client: %w", err) diff --git a/router/internal/persistedoperation/operationstorage/redis/rdcloser.go b/router/internal/persistedoperation/operationstorage/redis/rdcloser.go index 46fba0f7c0..945317cef7 100644 --- a/router/internal/persistedoperation/operationstorage/redis/rdcloser.go +++ b/router/internal/persistedoperation/operationstorage/redis/rdcloser.go @@ -3,10 +3,11 @@ package rd import ( "context" "fmt" - "github.com/redis/go-redis/v9" - "go.uber.org/zap" "net/url" "strings" + + "github.com/redis/go-redis/v9" + "go.uber.org/zap" ) // RDCloser is an interface that combines the redis.Cmdable and io.Closer interfaces, ensuring that we can close the @@ -20,6 +21,7 @@ type RedisCloserOptions struct { URLs []string ClusterEnabled bool Password string + FailOpen bool } func NewRedisCloser(opts *RedisCloserOptions) (RDCloser, error) { @@ -73,6 +75,10 @@ func NewRedisCloser(opts *RedisCloserOptions) (RDCloser, error) { } if isFunctioning, err := IsFunctioningClient(rdb); !isFunctioning { + if(opts.FailOpen) { + opts.Logger.Warn(fmt.Sprintf("Ratelimit Fail Open activated: redis client is currently not responding with provided URLs: %q", err)) + return rdb, nil + } return rdb, fmt.Errorf("failed to create a functioning redis client with the provided URLs: %w", err) } From 36fab7d9d18d1481e1eae35012e826e0f07bd238 Mon Sep 17 00:00:00 2001 From: Sam Christen Oliphant Date: Fri, 7 Mar 2025 22:06:05 -0500 Subject: [PATCH 6/7] remove impossible condition --- router/core/ratelimiter.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/router/core/ratelimiter.go b/router/core/ratelimiter.go index 3e645f3cd1..2b1c7b4b8e 100644 --- a/router/core/ratelimiter.go +++ b/router/core/ratelimiter.go @@ -41,9 +41,6 @@ func NewCosmoRateLimiter(opts *CosmoRateLimiterOptions) (rl *CosmoRateLimiter, e rejectStatusCode: opts.RejectStatusCode, failOpen: opts.FailOpen, } - if err != nil { - return nil, err - } if rl.rejectStatusCode == 0 { rl.rejectStatusCode = 200 } From ce3bb3cff8a74e04fb8392a5fc1499fbf429a69e Mon Sep 17 00:00:00 2001 From: Sam Christen Oliphant Date: Fri, 7 Mar 2025 22:28:42 -0500 Subject: [PATCH 7/7] add envDefault --- router/pkg/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/router/pkg/config/config.go b/router/pkg/config/config.go index 43bc754034..488b642436 100644 --- a/router/pkg/config/config.go +++ b/router/pkg/config/config.go @@ -449,7 +449,7 @@ type RateLimitConfiguration struct { Debug bool `yaml:"debug" envDefault:"false" env:"RATE_LIMIT_DEBUG"` KeySuffixExpression string `yaml:"key_suffix_expression,omitempty" env:"RATE_LIMIT_KEY_SUFFIX_EXPRESSION"` ErrorExtensionCode RateLimitErrorExtensionCode `yaml:"error_extension_code"` - FailOpen bool `yaml:"fail_open" env:"RATE_LIMIT_FAIL_OPEN"` + FailOpen bool `yaml:"fail_open" envDefault:"false" env:"RATE_LIMIT_FAIL_OPEN"` } type RateLimitErrorExtensionCode struct {