Request buffering middleware #2217

harnash · 2017-10-05T08:01:13Z

What does this PR do?

This approach allows better control over requests processing incoming into Traefik. It allows to specify buffer limits and maximum request size that will be processed by Traefik. I've decided to use global settings for all requests (which seems enough for us) but it may more convenient to specify those settings per frontend.

For more background about this change please see the linked issue and mentioned PR.

Motivation

Fixes #2043

More

Additional Notes

This is related to PR #2209

~~This code will not test properly since Traefik uses forked vulcand/oxy dependency which does not have buffer package included (merge with upstream needed).~~

emilevauge

Hey @harnash, thanks for your contribution!
Instead of setting these options on the whole instance, I was thinking that it would be better to set them by backend. WDYT ? /cc @containous/traefik

harnash · 2017-10-06T14:20:45Z

@emilevauge I was thinking about something similar. Since it is still blocked I will try to move those settings into backend config.

nmengin · 2017-10-26T13:17:35Z

ping @harnash

Did you try to move the settings into backend config? Do you need some help?

harnash · 2017-10-27T07:19:22Z

@nmengin Hey. Sadly no. I had my hands full but I'll try to look at this this weekend

harnash · 2017-10-29T20:22:56Z

@nmengin I've just pushed f120a47 which moves configuration to backend config. I might deploy this on our dev cluster soon to test it but I'm not sure I can since to build it I need vulcand/oxy/buffering package which is missing in the containous' fork. Any advice how to deal with this?

juliens · 2017-11-10T09:31:06Z

As we need to have the last vulcand/oxy, I think we must wait for #2390

juliens · 2017-12-08T13:11:46Z

The new oxy version is merged on master, can you rebase ?
Thx

harnash · 2017-12-09T20:28:04Z

@juliens It is rebased but tests will still be failing since fork is missing oxy.buffer package.

ldez · 2017-12-09T23:08:14Z

@harnash pb fixed

nmengin

Hello @harnash .

Many thanks for this PR 👍

In the description, you said you would adding tests and documentation.
Is it still panned? Because I'm sure it can be helpful 😉

WDYT?

harnash · 2017-12-15T20:00:15Z

@nmengin I'll look into that

ldez · 2018-01-11T09:55:52Z

@harnash any news?

harnash · 2018-01-11T20:40:53Z

@ldez sorry I hadn't time to look at this yet but I'll schedule some time tomorrow.

ldez

In addition to what @nmengin said, you also need to remove:

TraefikBackendBufferingEnabled
SuffixBackendBufferingEnabled
pathBackendBufferingEnabled

ldez · 2018-01-28T14:34:07Z

provider/kubernetes/builder_configuration_test.go

@@ -90,6 +90,48 @@ func circuitBreaker(exp string) func(*types.Backend) {
 	}
 }

+func maxRequestBodyBytes(value int64) func(*types.Backend) {


Could you rewrite this section like that:

func buffering(opts ...func(*types.Buffering)) func(*types.Backend) { return func(b *types.Backend) { if b.Buffering == nil { b.Buffering = &types.Buffering{} } for _, opt := range opts { opt(b.Buffering) } } } func maxRequestBodyBytes(value int64) func(*types.Buffering) { return func(b *types.Buffering) { b.MaxRequestBodyBytes = value } } func memRequestBodyBytes(value int64) func(*types.Buffering) { return func(b *types.Buffering) { b.MemRequestBodyBytes = value } } func maxResponseBodyBytes(value int64) func(*types.Buffering) { return func(b *types.Buffering) { b.MaxResponseBodyBytes = value } } func memResponseBodyBytes(value int64) func(*types.Buffering) { return func(b *types.Buffering) { b.MemResponseBodyBytes = value } } func retrying(exp string) func(*types.Buffering) { return func(b *types.Buffering) { b.RetryExpression = exp } }

ldez · 2018-01-28T14:50:18Z

provider/kv/kv_config.go

@@ -273,6 +274,23 @@ func (p *Provider) getHealthCheck(rootPath string) *types.HealthCheck {
 	}
 }

+func (p *Provider) getBuffering(rootPath string) *types.Buffering {
+	enabled := p.getBool(false, rootPath, pathBackendBufferingEnabled)


could you rewrite this method like that:

func (p *Provider) getBufferingReal(rootPath string) *types.Buffering { pathsBuffering := p.list(rootPath, pathBackendBuffering) var buffering *types.Buffering if len(pathsBuffering) > 0 { if buffering == nil { buffering = &types.Buffering{} } buffering.MaxRequestBodyBytes = p.getInt64(0, rootPath, pathBackendBufferingMaxRequestBodyBytes) buffering.MaxResponseBodyBytes = p.getInt64(0, rootPath, pathBackendBufferingMaxResponseBodyBytes) buffering.MemRequestBodyBytes = p.getInt64(0, rootPath, pathBackendBufferingMemRequestBodyBytes) buffering.MemResponseBodyBytes = p.getInt64(0, rootPath, pathBackendBufferingMemResponseBodyBytes) buffering.RetryExpression = p.get("", rootPath, pathBackendBufferingRetryExpression) } return buffering }

ldez · 2018-01-28T15:03:41Z

server/server.go

@@ -1560,3 +1571,31 @@ func (s *Server) wrapHTTPHandlerWithAccessLog(handler http.Handler, frontendName
 	}
 	return handler
 }
+
+func (s *Server) buildBufferingMiddleware(handler http.Handler, config *types.Buffering) (http.Handler, error) {
+	var lb *buffer.Buffer


could you rewrite this method like that:

func (s *Server) buildBufferingMiddleware(handler http.Handler, config *types.Buffering) (http.Handler, error) { log.Debugf("Setting up buffering: request limits: %d (mem), %d (max), response limits: %d (mem), %d (max) with retry: '%s'", config.MemRequestBodyBytes, config.MaxRequestBodyBytes, config.MemResponseBodyBytes, config.MaxResponseBodyBytes, config.RetryExpression) if len(config.RetryExpression) > 0 { return buffer.New( handler, buffer.MemRequestBodyBytes(config.MemRequestBodyBytes), buffer.MaxRequestBodyBytes(config.MaxRequestBodyBytes), buffer.MemResponseBodyBytes(config.MemResponseBodyBytes), buffer.MaxResponseBodyBytes(config.MaxResponseBodyBytes), buffer.Retry(config.RetryExpression), ) } return buffer.New( handler, buffer.MemRequestBodyBytes(config.MemRequestBodyBytes), buffer.MaxRequestBodyBytes(config.MaxRequestBodyBytes), buffer.MemResponseBodyBytes(config.MemResponseBodyBytes), buffer.MaxResponseBodyBytes(config.MaxResponseBodyBytes), ) }

ldez · 2018-01-28T15:07:24Z

provider/kv/keynames.go

@@ -15,6 +15,12 @@ const (
 	pathBackendServers                          = "/servers/"
 	pathBackendServerURL                        = "/url"
 	pathBackendServerWeight                     = "/weight"
+	pathBackendBufferingEnabled                 = "/buffering/enabled"


could you modify this section like that:

const ( // ... pathBackendBuffering = "/buffering/" pathBackendBufferingMaxResponseBodyBytes = pathBackendBuffering + "maxresponsebodybytes" pathBackendBufferingMemResponseBodyBytes = pathBackendBuffering + "memresponsebodybytes" pathBackendBufferingMaxRequestBodyBytes = pathBackendBuffering + "maxrequestbodybytes" pathBackendBufferingMemRequestBodyBytes = pathBackendBuffering + "memrequestbodybytes" pathBackendBufferingRetryExpression = pathBackendBuffering + "retryexpression" // ... )

ldez · 2018-01-28T15:10:24Z

provider/kubernetes/kubernetes_test.go

+					server("http://10.14.0.1:8080", weight(1)),
+					server("http://10.12.0.1:8080", weight(1))),
+				lbMethod("wrr"),
+				maxRequestBodyBytes(10485760),


buffering( maxRequestBodyBytes(10485760), memRequestBodyBytes(2097152), maxResponseBodyBytes(10485760), memResponseBodyBytes(2097152), retrying("IsNetworkError() && Attempts() <= 2"), ),

ldez · 2018-01-28T15:11:39Z

provider/label/names.go

@@ -80,6 +87,12 @@ const (
 	TraefikBackendLoadBalancerStickinessCookieName = Prefix + SuffixBackendLoadBalancerStickinessCookieName
 	TraefikBackendMaxConnAmount                    = Prefix + SuffixBackendMaxConnAmount
 	TraefikBackendMaxConnExtractorFunc             = Prefix + SuffixBackendMaxConnExtractorFunc
+	TraefikBackendBufferingEnabled                 = Prefix + SuffixBackendBufferingEnabled


const ( // ... TraefikBackendBuffering = Prefix + SuffixBackendBuffering // ... )

harnash · 2018-01-29T07:30:58Z

I'll address those comments soon.

ldez · 2018-01-31T10:36:02Z

Due to some conflicts with other PRs and the fact your PR is old (we are sorry for the delay), we will fix your PR.

…ering config

Fixed rancher tests.

…some tests

ldez

LGTM 🎉

juliens

LGTM

nmengin

LGTM 🎉 👏 👍

traefiker added size/S status/0-needs-triage labels Oct 5, 2017

ldez added area/middleware status/1-needs-design-review and removed status/0-needs-triage labels Oct 5, 2017

emilevauge reviewed Oct 6, 2017

View reviewed changes

ldez added the contributor/waiting-for-corrections label Oct 9, 2017

ldez added the kind/enhancement a new or improved feature. label Oct 26, 2017

ldez added contributor/waiting-for-corrections and removed contributor/waiting-for-corrections labels Oct 30, 2017

ldez removed the contributor/waiting-for-corrections label Nov 9, 2017

emilevauge added status/2-needs-review and removed status/1-needs-design-review labels Dec 8, 2017

nmengin requested changes Dec 13, 2017

View reviewed changes

traefiker added the contributor/waiting-for-corrections label Dec 13, 2017

nmengin added the contributor/waiting-for-feedback label Dec 13, 2017

ldez removed the contributor/waiting-for-feedback label Dec 13, 2017

ldez added contributor/waiting-for-corrections and removed contributor/waiting-for-corrections labels Dec 15, 2017

ldez suggested changes Jan 28, 2018

View reviewed changes

ldez added this to the 1.6 milestone Jan 29, 2018

ldez removed the contributor/waiting-for-corrections label Jan 31, 2018

harnash and others added 15 commits January 31, 2018 11:38

Added buffering middleware + config

999e91f

Moved buffering middleware configuration to backends

75f99c6

Fix golint error

2e7591d

Updated documentation + update consul_catalog provider to handle buff…

45f7117

…ering config

Update docker provider to use new buffering config + some template fixes

3a86249

Updated ecs provider to support buffering configuration

bd298ab

Updated marathon provider to support buffering configuration

8aea2f2

Updated rancher provider to support buffering middleware

b408094

Updated kv provider to support buffering configuration

19f674c

Fixed rancher tests.

Updated k8s provider to handle buffering annotations/configuraiton + …

b32cf97

…some tests

Updated mesos provider to support buffering configuration

4133931

Updated documentation

7c9a62e

review.

8e4a6ee

chore: update vendor.

513b3fb

doc: move documentation.

ea0c2c6

ldez approved these changes Jan 31, 2018

View reviewed changes

juliens approved these changes Jan 31, 2018

View reviewed changes

doc: add providers documentation.

990d36b

nmengin approved these changes Jan 31, 2018

View reviewed changes

traefiker approved these changes Jan 31, 2018

View reviewed changes

ldez added status/3-needs-merge and removed status/2-needs-review labels Jan 31, 2018

traefiker merged commit a81171d into traefik:master Jan 31, 2018

traefiker removed the status/3-needs-merge label Jan 31, 2018

harnash deleted the buffering_middleware branch March 23, 2018 10:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Request buffering middleware #2217

Request buffering middleware #2217

harnash commented Oct 5, 2017 •

edited by ldez

Loading

emilevauge left a comment

harnash commented Oct 6, 2017

nmengin commented Oct 26, 2017 •

edited

Loading

harnash commented Oct 27, 2017

harnash commented Oct 29, 2017

juliens commented Nov 10, 2017

juliens commented Dec 8, 2017

harnash commented Dec 9, 2017 •

edited

Loading

ldez commented Dec 9, 2017

nmengin left a comment

harnash commented Dec 15, 2017

ldez commented Jan 11, 2018

harnash commented Jan 11, 2018

ldez left a comment

ldez Jan 28, 2018

ldez Jan 28, 2018

ldez Jan 28, 2018

ldez Jan 28, 2018

ldez Jan 28, 2018

ldez Jan 28, 2018

harnash commented Jan 29, 2018

ldez commented Jan 31, 2018

ldez left a comment •

edited

Loading

juliens left a comment

nmengin left a comment

Request buffering middleware #2217

Request buffering middleware #2217

Conversation

harnash commented Oct 5, 2017 • edited by ldez Loading

What does this PR do?

Motivation

More

Additional Notes

emilevauge left a comment

Choose a reason for hiding this comment

harnash commented Oct 6, 2017

nmengin commented Oct 26, 2017 • edited Loading

harnash commented Oct 27, 2017

harnash commented Oct 29, 2017

juliens commented Nov 10, 2017

juliens commented Dec 8, 2017

harnash commented Dec 9, 2017 • edited Loading

ldez commented Dec 9, 2017

nmengin left a comment

Choose a reason for hiding this comment

harnash commented Dec 15, 2017

ldez commented Jan 11, 2018

harnash commented Jan 11, 2018

ldez left a comment

Choose a reason for hiding this comment

ldez Jan 28, 2018

Choose a reason for hiding this comment

ldez Jan 28, 2018

Choose a reason for hiding this comment

ldez Jan 28, 2018

Choose a reason for hiding this comment

ldez Jan 28, 2018

Choose a reason for hiding this comment

ldez Jan 28, 2018

Choose a reason for hiding this comment

ldez Jan 28, 2018

Choose a reason for hiding this comment

harnash commented Jan 29, 2018

ldez commented Jan 31, 2018

ldez left a comment • edited Loading

Choose a reason for hiding this comment

juliens left a comment

Choose a reason for hiding this comment

nmengin left a comment

Choose a reason for hiding this comment

harnash commented Oct 5, 2017 •

edited by ldez

Loading

nmengin commented Oct 26, 2017 •

edited

Loading

harnash commented Dec 9, 2017 •

edited

Loading

ldez left a comment •

edited

Loading