Rate limiting for frontends #2034

bparli · 2017-08-31T17:34:05Z

Description

This is to add a rate limiting capability in Traefik, as in #643

Integrate the oxy ratelimit package
Add Rate and RateLimit types for frontend configuration
Add rate limiting integration test
Update glide/vendor with new package dependencies
Add configuration example to basics doc

ldez · 2017-08-31T17:45:36Z

integration/integration_test.go

@@ -42,6 +42,7 @@ func init() {
 	check.Suite(&SimpleSuite{})
 	check.Suite(&TimeoutSuite{})
 	check.Suite(&WebsocketSuite{})
+	check.Suite(&RateLimitSuite{})


Could you move this line to be in alphabetical order?

ldez · 2017-08-31T17:46:07Z

integration/ratelimit_test.go

+	err = try.GetRequest("http://127.0.0.1:80/", 500*time.Millisecond, try.StatusCodeIs(http.StatusTooManyRequests))
+	c.Assert(err, checker.IsNil)
+
+	time.Sleep(3 * time.Second)


Could you explains this line?

ldez · 2017-08-31T17:47:01Z

server/server.go

@@ -890,6 +891,30 @@ func (server *Server) loadConfig(configurations types.Configurations, globalConf
 						}
 					}

+					if frontend.RateLimit != nil && len(frontend.RateLimit.RateSet) > 0 {


Could you isolate this into a function?

I think I have this comment addressed with the server.buildRateLimiter function I added. Of course, let me know if you had something else in mind though.

you did exactly what I wanted👍

ldez · 2017-08-31T17:48:09Z

glide.lock

@@ -331,7 +337,7 @@ imports:
 - name: github.com/Masterminds/semver
  version: 59c29afe1a994eacb71c833025ca7acf874bb1da
 - name: github.com/Masterminds/sprig
-  version: 9526be0327b26ad31aa70296a7b10704883976d5
+  version: e039e20e500c2c025d9145be375e27cf42a94174


I think this update is not required.

I agree this update doesn't seem to be necessary. After looking at this again, I'm actually sure not how to handle it though. For Masterminds/sprig, the versions don't seem to match between glide.yml and glide.lock. I'm not super familiar with glide, but I guess thats why glide went ahead with the update? Any advice is appreciated

ldez · 2017-08-31T17:48:28Z

glide.lock

@@ -95,6 +95,8 @@ imports:
 - name: github.com/coreos/etcd
  version: c400d05d0aa73e21e431c16145e558d624098018
  subpackages:
+  - Godeps/_workspace/src/github.com/ugorji/go/codec


Could you remove those lines?

ldez · 2017-08-31T17:50:22Z

docs/basics.md

@@ -278,6 +278,32 @@ Security related headers (HSTS headers, SSL redirection, Browser XSS filter, etc

 In this example, traffic routed through the first frontend will have the `X-Frame-Options` header set to `DENY`, and the second will only allow HTTPS request through, otherwise will return a 301 HTTPS redirect.

+#### Rate limiting
+
+Rate limiting can be configured per frontend.  Multiple sets of rates can be added to each frontend, but the time periods must be unique.  


Could you put one sentence by line?

ldez · 2017-08-31T17:50:27Z

docs/basics.md

+            burst = 10
+```
+
+In the above example, frontend1 is configured to limit requests by the client's ip address.  An average of 5 requests every 3 seconds is allowed and an average of 100 requests every 10 seconds.  These can "burst" up to 10 and 200 in each period respectively.  


Could you put one sentence by line?

nmengin

Many thanks for this great PR.
I have just one suggestion about the integration test. 😉

nmengin · 2017-09-01T10:20:17Z

integration/ratelimit_test.go

+	time.Sleep(3 * time.Second)
+	err = try.GetRequest("http://127.0.0.1:80/", 500*time.Millisecond, try.StatusCodeIs(http.StatusOK))
+	c.Assert(err, checker.IsNil)
+}


Maybe can you set lesser values to frontends.frontend1.ratelimit.rateset.rateset1 (average = 4 and burst = 5?) in the toml file and check if this rateset is respected too in the TestSimpleConfiguration test or in another one?

👍 You're right. Its a good idea to test that first time period too.

bparli · 2017-09-01T20:32:29Z

Thanks for the review and feedback guys. I think I have glide straightened out now

nmengin

LGTM 👏 👏

SantoDE

Thanks for the great pr :) Brings in a new nifty feature.

LGTM 👼

ldez

Great job !

LGTM

trondhindenes · 2017-12-07T18:49:05Z

I can't find the doc changes in this PR at https://docs.traefik.io/basics/, is there any reason for that?

ldez · 2017-12-07T18:59:06Z

@trondhindenes because it's a 1.5 features.
If you want to try 1.5, you can find the documentation here: https://docs.traefik.io/archive/ or http://v1-5.archive.docs.traefik.io/

trondhindenes · 2017-12-07T19:25:39Z

gotcha. Thanks!

tobernguyen · 2017-12-28T22:45:13Z

How can I use this in Traefik Ingress Controller on my Kubernetes cluster? Is there any way to set the rate limit using Annotations?

ldez · 2017-12-29T03:21:00Z

@tobernguyen For now, you cannot define rate limit with annotations.
This feature (k8s annotations for rate limit) come in v1.6: #2612

tobernguyen · 2017-12-29T03:24:48Z

@ldez that's awesome! Thank you very much.

traefiker added size/M status/0-needs-triage labels Aug 31, 2017

ldez added kind/enhancement a new or improved feature. status/2-needs-review and removed status/0-needs-triage labels Aug 31, 2017

ldez added this to the 1.5 milestone Aug 31, 2017

ldez added status/1-needs-design-review status/2-needs-review and removed status/2-needs-review status/1-needs-design-review labels Aug 31, 2017

ldez requested changes Sep 1, 2017

View reviewed changes

traefiker added the contributor/waiting-for-corrections label Sep 1, 2017

nmengin reviewed Sep 1, 2017

View reviewed changes

ldez removed the contributor/waiting-for-corrections label Sep 1, 2017

nmengin approved these changes Sep 6, 2017

View reviewed changes

SantoDE approved these changes Sep 7, 2017

View reviewed changes

yamalight mentioned this pull request Sep 8, 2017

Add support for rate-limiting exoframejs/exoframe#59

Closed

ldez force-pushed the ratelimit branch 3 times, most recently from 2d22aee to dff230b Compare September 9, 2017 08:43

ldez approved these changes Sep 9, 2017

View reviewed changes

ldez added status/3-needs-merge and removed status/2-needs-review labels Sep 9, 2017

ldez changed the title ~~Integrate oxy rate limiting~~ Rate limiting for frontends Sep 9, 2017

ldez added status/2-needs-review status/3-needs-merge and removed status/3-needs-merge status/2-needs-review labels Sep 9, 2017

traefiker added the status/4-merge-in-progress label Sep 9, 2017

ratelimit integration

019e064

traefiker force-pushed the ratelimit branch from dff230b to 019e064 Compare September 9, 2017 11:00

traefiker removed the status/4-merge-in-progress label Sep 9, 2017

traefiker merged commit d54417a into traefik:master Sep 9, 2017

traefiker removed the status/3-needs-merge label Sep 9, 2017

ldez mentioned this pull request Sep 9, 2017

implement rate limiting / api access (like tollbooth) #643

Closed

Hermsi1337 mentioned this pull request Dec 22, 2017

Rate limiting in Traefik 1.5 rawmind0/rancher-traefik#59

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rate limiting for frontends #2034

Rate limiting for frontends #2034

bparli commented Aug 31, 2017

ldez Aug 31, 2017

ldez Aug 31, 2017

ldez Aug 31, 2017

bparli Sep 8, 2017

ldez Sep 8, 2017

ldez Aug 31, 2017

bparli Sep 1, 2017

ldez Aug 31, 2017

ldez Aug 31, 2017

ldez Aug 31, 2017

nmengin left a comment

nmengin Sep 1, 2017 •

edited

bparli Sep 1, 2017

bparli commented Sep 1, 2017

nmengin left a comment

SantoDE left a comment

ldez left a comment

trondhindenes commented Dec 7, 2017

ldez commented Dec 7, 2017

trondhindenes commented Dec 7, 2017

tobernguyen commented Dec 28, 2017

ldez commented Dec 29, 2017

tobernguyen commented Dec 29, 2017

Rate limiting for frontends #2034

Rate limiting for frontends #2034

Conversation

bparli commented Aug 31, 2017

Description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmengin left a comment

Choose a reason for hiding this comment

nmengin Sep 1, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bparli commented Sep 1, 2017

nmengin left a comment

Choose a reason for hiding this comment

SantoDE left a comment

Choose a reason for hiding this comment

ldez left a comment

Choose a reason for hiding this comment

trondhindenes commented Dec 7, 2017

ldez commented Dec 7, 2017

trondhindenes commented Dec 7, 2017

tobernguyen commented Dec 28, 2017

ldez commented Dec 29, 2017

tobernguyen commented Dec 29, 2017

nmengin Sep 1, 2017 •

edited