Skip to content

Commit 4c39f7d

Browse files
authored
nginz: enable unlimited_requests_endpoint for authenticated requests, too (#3138)
Fix a rate-limit exemption whereby authenticated endpoints did not get the unlimited_requests_endpoint, if set, applied. This is a concern for the webapp and calls to /assets, which can happen in larger numbers on initial loading. A previous change in [this PR](#2786) had no effect. This PR also increases default rate limits, to compensate for [new ingress controller chart](#3140 default topologyAwareRouting.
1 parent b5afd86 commit 4c39f7d

File tree

3 files changed

+9
-4
lines changed

3 files changed

+9
-4
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix a rate-limit exemption whereby authenticated endpoints did not get the unlimited_requests_endpoint, if set, applied. This is a concern for the webapp and calls to /assets, which can happen in larger numbers on initial loading. A previous change in [this PR](https://github.com/wireapp/wire-server/pull/2786) had no effect. This PR also increases default rate limits, to compensate for [new ingress controller chart](https://github.com/wireapp/wire-server/pull/3140)'s default topologyAwareRouting.

charts/nginz/templates/conf/_nginx.conf.tpl

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,6 @@ http {
162162
limit_req_log_level warn;
163163
limit_conn_log_level warn;
164164

165-
# Limit by $zauth_user if present and not part of rate limit exemptions
166-
limit_req zone=reqs_per_user burst=20;
167165
limit_conn conns_per_user 25;
168166

169167
#
@@ -257,6 +255,12 @@ http {
257255
limit_conn conns_per_addr 20;
258256
{{- end }}
259257
{{- end }}
258+
{{- else }}
259+
{{- if ($location.unlimited_requests_endpoint) }}
260+
# Note that this endpoint has no rate limit per user for authenticated requests
261+
{{- else }}
262+
limit_req zone=reqs_per_user burst=20;
263+
{{- end }}
260264
{{- end }}
261265

262266
{{- if ($location.oauth_scope) }}

charts/nginz/values.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ nginx_conf:
5656
- /search/common
5757

5858
default_client_max_body_size: "256k"
59-
rate_limit_reqs_per_user: "10r/s"
60-
rate_limit_reqs_per_addr: "5r/m"
59+
rate_limit_reqs_per_user: "30r/s"
60+
rate_limit_reqs_per_addr: "15r/m"
6161

6262
# This value must be a list of strings. Each string is copied verbatim into
6363
# the nginx.conf after the default 'limit_req_zone' directives. This should be

0 commit comments

Comments
 (0)