RateLimit design doc

[stevesloka]

Updates #370 to provide a design doc implementation for RateLimiting.

Note: I have much of this developed on a branch, but want to get a good design doc agreed upon first.

Signed-off-by: Steve Sloka slokas@vmware.com

[davecheney]

LGTM.

I want this to be opt in, not the default for all installations.

The question is, if rate limiting is not enabled/defined -- i'm guessing its some global param we'll have to pass to contour -- then do we reject ingressroute objects that have a rate limit specified, or do we accept them, without a rate limit.

The former is annoying for portability across environments, the latter possibly means a config screwup in contour's deployment leads to a service running without RLS.

[davecheney]

This feels like an envoy implementation detail, is there a way we can infer this value from the rate limit parameters supplied by the user?

[stevesloka]

You can configure rate limiting per Route, not sure where else you would get these values from?

Ideally, I think we'd define the actual rules in the IngressRoute. For example, I could say, only accept 10 req/sec from any IP address. Or only allow 10 req/sec from anywhere. Then contour would write out the RateLimit config to that service.

Right now, the user has to configure rate limiting service by hand, then match up the configurations. I thought this hands-off approach was better (since it was simpler) and wouldn't require the Lyft implementation (you could run anything).

[stevesloka]

Thought about this some more, I made this a map[string]string so that users could define whatever values they want. I think this is better and a good catch on the design.

[davecheney]

We should probably only enable the rate limiting filter anywhere in the ingressroute chain has a rate limit stanza.

[stevesloka]

Yes I agree. I think looking at this now I never allowed for a spot to enable the service. That should probably be an arg to Contour as you'd only want to set it up once. Having that existing would be the trigger for contour to enable the filter, etc.

[stevesloka]

i'm guessing its some global param we'll have to pass to contour

I think we could add an arg to Contour which would be the url to the rate limit service. The only downside to the argument is that users must have access to the deployment to add this, but that might be ok.

then do we reject ingressroute objects that have a rate limit specified, or do we accept them, without a rate limit

For similar items, we've been just setting status on the IngressRoute, so could do that for this as well. But what would we do for Ingress resources? We could log an error, or add a status annotation to the resource which seems werid.

[stevesloka]

The other issue that I've come across now in looking at the implementation is the need for a way to restart Envoy in the event the bootstrap configuration needs to be changed.

For example, if we disable rate limiting by default, and then later on enable, that would require a new bootstrap configuration to be applied to Envoy. Same would happen if we needed to change the server/port names.

I cannot configure the cluster piece dynamically since referencing a bogus cluster causes Envoy to fail to start.

[davecheney]

@stevesloka feel free to rebase this and merge it any time you're ready.