global rate limiting: allow configuring enable_x_ratelimit_headers

[xaleeks]

Just checking, can our rate limiting feature querying capabilities like checking number of requests left and the time period in effect for current rate limits until they are reset.

Seems Envoy provide some of these APIs in v3

(extensions.filters.http.ratelimit.v3.RateLimit.XRateLimitHeadersRFCVersion) Defines the standard version to use for X-RateLimit headers emitted by the filter:

X-RateLimit-Limit - indicates the request-quota associated to the client in the current time-window followed by the description of the quota policy. The values are returned by the rate limiting service in current_limit field. Example: 10, 10;w=1;name=”per-ip”, 1000;w=3600.
X-RateLimit-Remaining - indicates the remaining requests in the current time-window. The values are returned by the rate limiting service in limit_remaining field.
X-RateLimit-Reset - indicates the number of seconds until reset of the current time-window. The values are returned by the rate limiting service in duration_until_reset field.

[skriss]

We don't enable these headers right now or expose a knob to turn them on, would be easy to add though, looks like just another boolean field to set in the filter config.

[skriss]

Should I consider this issue a request to add support for those?

[xaleeks]

yup, if it's not enabled then please consider this a feature request. If there are no objections to adding this, any chance we can get this in 1.14?

[skriss]

yup, if it's not enabled then please consider this a feature request. If there are no objections to adding this, any chance we can get this in 1.14?

Yep shouldn't be a big deal. Do you have thoughts on if these should be enabled by default?

[youngnick]

I propose we add them as available but defaulted off, and see if users would like us to change that.

[skriss]

I propose we add them as available but defaulted off, and see if users would like us to change that.

Yep makes sense to me - @xaleeks sound good?

[skriss]

So it looks like Envoy intends to be able to support multiple versions of the RFC draft, if there are more versions in the future (currently only draft 3 is supported: https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/ratelimit/v3/rate_limit.proto#enum-extensions-filters-http-ratelimit-v3-ratelimit-xratelimitheadersrfcversion).

WDYT about simplifying this for Contour, and just making it a binary disabled (default) or enabled setting? If enabled, we'll program Envoy to use the latest RFC draft version.

While I can see the value of what Envoy's doing, I don't know that we need to expose that complexity to the Contour user.

If, in the future, we saw a need to support multiple versions, we could replace the boolean config field with a new enum one. We'd just have to go through a deprecation cycle for the config setting.

[youngnick]

That's a great plan @skriss. I'll check out the PR today.