Skip to content

Commit ccab863

Browse files
thebengeuthebengeu
committed
fix: update Envoy config to have circuit breaker settings, retries, increased timeouts and remove empty key query params
1 parent cd9c3f0 commit ccab863

File tree

5 files changed

+177
-18
lines changed

5 files changed

+177
-18
lines changed

‎ansible/files/envoy_config/cds.yaml

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,16 @@ resources:
1010
socket_address:
1111
address: 127.0.0.1
1212
port_value: 8085
13+
circuit_breakers:
14+
thresholds:
15+
- priority: DEFAULT
16+
max_connections: 10000
17+
max_pending_requests: 10000
18+
max_requests: 10000
19+
retry_budget:
20+
budget_percent:
21+
value: 100
22+
min_retry_concurrency: 100
1323
- '@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
1424
name: gotrue
1525
load_assignment:
@@ -21,6 +31,16 @@ resources:
2131
socket_address:
2232
address: 127.0.0.1
2333
port_value: 9999
34+
circuit_breakers:
35+
thresholds:
36+
- priority: DEFAULT
37+
max_connections: 10000
38+
max_pending_requests: 10000
39+
max_requests: 10000
40+
retry_budget:
41+
budget_percent:
42+
value: 100
43+
min_retry_concurrency: 100
2444
- '@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
2545
name: postgrest
2646
load_assignment:
@@ -32,6 +52,16 @@ resources:
3252
socket_address:
3353
address: 127.0.0.1
3454
port_value: 3000
55+
circuit_breakers:
56+
thresholds:
57+
- priority: DEFAULT
58+
max_connections: 10000
59+
max_pending_requests: 10000
60+
max_requests: 10000
61+
retry_budget:
62+
budget_percent:
63+
value: 100
64+
min_retry_concurrency: 100
3565
- '@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
3666
name: postgrest_admin
3767
load_assignment:
@@ -43,4 +73,14 @@ resources:
4373
socket_address:
4474
address: 127.0.0.1
4575
port_value: 3001
76+
circuit_breakers:
77+
thresholds:
78+
- priority: DEFAULT
79+
max_connections: 10000
80+
max_pending_requests: 10000
81+
max_requests: 10000
82+
retry_budget:
83+
budget_percent:
84+
value: 100
85+
min_retry_concurrency: 100
4686

‎ansible/files/envoy_config/lds.yaml

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,23 @@ resources:
8787
'@type': >-
8888
type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua
8989
source_codes:
90-
remove_apikey_query_parameter:
91-
filename: /etc/envoy/remove_apikey_query_parameter.lua
90+
remove_apikey_and_empty_key_query_parameters:
91+
inline_string: |-
92+
function envoy_on_request(request_handle)
93+
local path = request_handle:headers():get(":path")
94+
request_handle
95+
:headers()
96+
:replace(":path", path:gsub("&=[^&]*", ""):gsub("?=[^&]*$", ""):gsub("?=[^&]*&", "?"))
97+
:replace(":path", path:gsub("&apikey=[^&]*", ""):gsub("?apikey=[^&]*$", ""):gsub("?apikey=[^&]*&", "?"))
98+
end
99+
remove_empty_key_query_parameters:
100+
inline_string: |-
101+
function envoy_on_request(request_handle)
102+
local path = request_handle:headers():get(":path")
103+
request_handle
104+
:headers()
105+
:replace(":path", path:gsub("&=[^&]*", ""):gsub("?=[^&]*$", ""):gsub("?=[^&]*&", "?"))
106+
end
92107
- name: envoy.filters.http.router
93108
typed_config:
94109
'@type': >-
@@ -181,12 +196,14 @@ resources:
181196
retry_policy:
182197
num_retries: 3
183198
retry_on: 5xx
199+
timeout: 35s
184200
typed_per_filter_config: *ref_0
185201
- match:
186202
prefix: /auth/v1/
187203
route:
188204
cluster: gotrue
189205
prefix_rewrite: /
206+
timeout: 35s
190207
- match:
191208
prefix: /rest/v1/
192209
query_parameters:
@@ -197,20 +214,25 @@ resources:
197214
route:
198215
cluster: postgrest
199216
prefix_rewrite: /
200-
timeout: 120s
217+
timeout: 125s
201218
typed_per_filter_config:
202219
envoy.filters.http.lua:
203220
'@type': >-
204221
type.googleapis.com/envoy.extensions.filters.http.lua.v3.LuaPerRoute
205-
name: remove_apikey_query_parameter
222+
name: remove_apikey_and_empty_key_query_parameter
206223
- match:
207224
prefix: /rest/v1/
208225
request_headers_to_remove:
209226
- apikey
210227
route:
211228
cluster: postgrest
212229
prefix_rewrite: /
213-
timeout: 120s
230+
timeout: 125s
231+
typed_per_filter_config:
232+
envoy.filters.http.lua:
233+
'@type': >-
234+
type.googleapis.com/envoy.extensions.filters.http.lua.v3.LuaPerRoute
235+
name: remove_empty_key_query_parameter
214236
- match:
215237
prefix: /rest-admin/v1/
216238
query_parameters:
@@ -225,7 +247,7 @@ resources:
225247
envoy.filters.http.lua:
226248
'@type': >-
227249
type.googleapis.com/envoy.extensions.filters.http.lua.v3.LuaPerRoute
228-
name: remove_apikey_query_parameter
250+
name: remove_apikey_and_empty_key_query_parameters
229251
- match:
230252
prefix: /rest-admin/v1/
231253
request_headers_to_remove:
@@ -242,7 +264,7 @@ resources:
242264
route:
243265
cluster: postgrest
244266
prefix_rewrite: /rpc/graphql
245-
timeout: 120s
267+
timeout: 125s
246268
- match:
247269
prefix: /admin/v1/
248270
route:
@@ -293,6 +315,13 @@ resources:
293315
direct_remote_ip:
294316
address_prefix: 10.0.0.0
295317
prefix_len: 8
318+
include_attempt_count_in_response: true
319+
retry_policy:
320+
num_retries: 5
321+
retry_back_off:
322+
base_interval: 0.1s
323+
max_interval: 1s
324+
retry_on: gateway-error
296325
stat_prefix: ingress_http
297326
- '@type': type.googleapis.com/envoy.config.listener.v3.Listener
298327
name: https_listener

‎ansible/files/envoy_config/remove_apikey_query_parameter.lua

Lines changed: 0 additions & 8 deletions
This file was deleted.

‎testinfra/test_ami.py

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ def test_postgrest_can_connect_to_db(host):
343343
# There would be an error if the `apikey` query parameter isn't removed,
344344
# since PostgREST treats query parameters as conditions.
345345
#
346-
# Worth testing since remove_apikey_query_parameter.lua uses regexp instead
346+
# Worth testing since remove_apikey_query_parameters uses regexp instead
347347
# of parsed query parameters.
348348
def test_postgrest_starting_apikey_query_parameter_is_removed(host):
349349
res = requests.get(
@@ -388,3 +388,52 @@ def test_postgrest_ending_apikey_query_parameter_is_removed(host):
388388
},
389389
)
390390
assert res.ok
391+
392+
# There would be an error if the empty key query parameter isn't removed,
393+
# since PostgREST treats empty key query parameters as malformed input.
394+
#
395+
# Worth testing since remove_apikey_and_empty_key_query_parameters uses regexp instead
396+
# of parsed query parameters.
397+
def test_postgrest_starting_empty_key_query_parameter_is_removed(host):
398+
res = requests.get(
399+
f"http://{host.backend.get_hostname()}/rest/v1/buckets",
400+
headers={
401+
"accept-profile": "storage",
402+
},
403+
params={
404+
"": "empty_key",
405+
"id": "eq.absent",
406+
"apikey": service_role_key,
407+
},
408+
)
409+
assert res.ok
410+
411+
412+
def test_postgrest_middle_empty_key_query_parameter_is_removed(host):
413+
res = requests.get(
414+
f"http://{host.backend.get_hostname()}/rest/v1/buckets",
415+
headers={
416+
"accept-profile": "storage",
417+
},
418+
params={
419+
"apikey": service_role_key,
420+
"": "empty_key",
421+
"id": "eq.absent",
422+
},
423+
)
424+
assert res.ok
425+
426+
427+
def test_postgrest_ending_empty_key_query_parameter_is_removed(host):
428+
res = requests.get(
429+
f"http://{host.backend.get_hostname()}/rest/v1/buckets",
430+
headers={
431+
"accept-profile": "storage",
432+
},
433+
params={
434+
"id": "eq.absent",
435+
"apikey": service_role_key,
436+
"": "empty_key",
437+
},
438+
)
439+
assert res.ok

‎testinfra/test_ami_nix.py

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ def test_postgrest_can_connect_to_db(host):
343343
# There would be an error if the `apikey` query parameter isn't removed,
344344
# since PostgREST treats query parameters as conditions.
345345
#
346-
# Worth testing since remove_apikey_query_parameter.lua uses regexp instead
346+
# Worth testing since remove_apikey_query_parameters uses regexp instead
347347
# of parsed query parameters.
348348
def test_postgrest_starting_apikey_query_parameter_is_removed(host):
349349
res = requests.get(
@@ -387,4 +387,53 @@ def test_postgrest_ending_apikey_query_parameter_is_removed(host):
387387
"apikey": service_role_key,
388388
},
389389
)
390-
assert res.ok
390+
assert res.ok
391+
392+
# There would be an error if the empty key query parameter isn't removed,
393+
# since PostgREST treats empty key query parameters as malformed input.
394+
#
395+
# Worth testing since remove_apikey_and_empty_key_query_parameters uses regexp instead
396+
# of parsed query parameters.
397+
def test_postgrest_starting_empty_key_query_parameter_is_removed(host):
398+
res = requests.get(
399+
f"http://{host.backend.get_hostname()}/rest/v1/buckets",
400+
headers={
401+
"accept-profile": "storage",
402+
},
403+
params={
404+
"": "empty_key",
405+
"id": "eq.absent",
406+
"apikey": service_role_key,
407+
},
408+
)
409+
assert res.ok
410+
411+
412+
def test_postgrest_middle_empty_key_query_parameter_is_removed(host):
413+
res = requests.get(
414+
f"http://{host.backend.get_hostname()}/rest/v1/buckets",
415+
headers={
416+
"accept-profile": "storage",
417+
},
418+
params={
419+
"apikey": service_role_key,
420+
"": "empty_key",
421+
"id": "eq.absent",
422+
},
423+
)
424+
assert res.ok
425+
426+
427+
def test_postgrest_ending_empty_key_query_parameter_is_removed(host):
428+
res = requests.get(
429+
f"http://{host.backend.get_hostname()}/rest/v1/buckets",
430+
headers={
431+
"accept-profile": "storage",
432+
},
433+
params={
434+
"id": "eq.absent",
435+
"apikey": service_role_key,
436+
"": "empty_key",
437+
},
438+
)
439+
assert res.ok

0 commit comments

Comments
 (0)