allows authorized clients to bypass rate limiter #599
Conversation
| @@ -28,6 +29,9 @@ func TestLimit1IP(t *testing.T) { | |||
|
|
|||
| {name: "success", callRPS: 100, limitRPS: 500, forwardedFor: false}, | |||
| {name: "block-me", callRPS: 1000, limitRPS: 500, forwardedFor: false}, | |||
|
|
|||
| {name: "allow-me", callRPS: 1000, limitRPS: 500, forwardedFor: false, allow: true}, | |||
There was a problem hiding this comment.
adds two more test cases where rps is greater than limit but it never gets 429
| @@ -77,3 +81,62 @@ func extractClientIP(r *http.Request) (string, error) { | |||
| } | |||
| return ip, nil | |||
| } | |||
|
|
|||
| type middleware struct { | |||
There was a problem hiding this comment.
implementation borrowed from https://github.com/sethvargo/go-limiter/blob/main/httplimit/middleware.go with an extra tweak
| return | ||
| } | ||
|
|
||
| // skip rate limiting checks if key is in allowlist |
There was a problem hiding this comment.
this is the additional change
cmd/api/main.go
Outdated
| @@ -450,11 +450,14 @@ func createAPIServer( | |||
| return nil, fmt.Errorf("parsing http ratelimiter interval: %s", err) | |||
| } | |||
|
|
|||
| allowList := strings.Split(httpConfig.AllowList, ",") | |||
There was a problem hiding this comment.
with this we can provide a list of addresses
brunocalza
force-pushed
the
bcalza/ratelim
branch
from
30a70c5
to
b57fc75
Compare
| Interval time.Duration | ||
| MaxRPI uint64 | ||
| Interval time.Duration | ||
| AllowList []string |
cmd/api/config.go
Outdated
| @@ -51,6 +51,7 @@ type HTTPConfig struct { | |||
|
|
|||
| RateLimInterval string `default:"1s"` | |||
| MaxRequestPerInterval uint64 `default:"10"` | |||
| AllowList string `default:""` | |||
There was a problem hiding this comment.
maybe a comment with an example of a list of IP addrs would be helpful here.
brunocalza
changed the title
Signed-off-by: Bruno Calza <[email protected]>
Signed-off-by: Bruno Calza <[email protected]>
brunocalza
force-pushed
the
bcalza/ratelim
branch
from
43c91c0
to
fc7b33b
Compare
brunocalza
force-pushed
the
bcalza/ratelim
branch
from
fc7b33b
to
22d6fe2
Compare
Signed-off-by: Bruno Calza <[email protected]>
brunocalza
force-pushed
the
bcalza/ratelim
branch
from
22d6fe2
to
606165a
Compare
| } | ||
|
|
||
| // skip rate limiting checks if secret key is provided | ||
| if key := r.Header.Get("Secret-Key"); key != "" && m.apiKey != "" { |
There was a problem hiding this comment.
should we name the header "Api-Key" instead of "Secret-Key" to make it consistent with the code?
There was a problem hiding this comment.
hmm maybe i get your point now. this is the special "API key" that can bypass the checks. so it makes sense.
Signed-off-by: Bruno Calza <[email protected]>
Summary
This PR adds a new config for the HTTP rate limiter:
ApiKey. With that, authorized clients (clients that provide the same key) will not be rate limited.Context
Our Studio application is being rate limited fairly easily.
Implementation overview
Adds a new config to be configured via env. Makes a change to the rate limiter middleware to ignore the authorized clients