Skip to content

fix: update Envoy config to have circuit breaker settings, retries, increased timeouts and remove empty key query params #1184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 6, 2024
Merged

fix: update Envoy config to have circuit breaker settings, retries, increased timeouts and remove empty key query params #1184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions ansible/files/envoy_config/cds.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ resources:
socket_address:
address: 127.0.0.1
port_value: 8085
circuit_breakers:
thresholds:
- priority: DEFAULT
max_connections: 10000
max_pending_requests: 10000
max_requests: 10000
retry_budget:
budget_percent:
value: 100
min_retry_concurrency: 100
- '@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
name: gotrue
load_assignment:
Expand All @@ -21,6 +31,16 @@ resources:
socket_address:
address: 127.0.0.1
port_value: 9999
circuit_breakers:
thresholds:
- priority: DEFAULT
max_connections: 10000
max_pending_requests: 10000
max_requests: 10000
retry_budget:
budget_percent:
value: 100
min_retry_concurrency: 100
- '@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
name: postgrest
load_assignment:
Expand All @@ -32,6 +52,16 @@ resources:
socket_address:
address: 127.0.0.1
port_value: 3000
circuit_breakers:
thresholds:
- priority: DEFAULT
max_connections: 10000
max_pending_requests: 10000
max_requests: 10000
retry_budget:
budget_percent:
value: 100
min_retry_concurrency: 100
- '@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
name: postgrest_admin
load_assignment:
Expand All @@ -43,4 +73,14 @@ resources:
socket_address:
address: 127.0.0.1
port_value: 3001
circuit_breakers:
thresholds:
- priority: DEFAULT
max_connections: 10000
max_pending_requests: 10000
max_requests: 10000
retry_budget:
budget_percent:
value: 100
min_retry_concurrency: 100

43 changes: 36 additions & 7 deletions ansible/files/envoy_config/lds.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,23 @@ resources:
'@type': >-
type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua
source_codes:
remove_apikey_query_parameter:
filename: /etc/envoy/remove_apikey_query_parameter.lua
remove_apikey_and_empty_key_query_parameters:
inline_string: |-
function envoy_on_request(request_handle)
local path = request_handle:headers():get(":path")
request_handle
:headers()
:replace(":path", path:gsub("&=[^&]*", ""):gsub("?=[^&]*$", ""):gsub("?=[^&]*&", "?"))
:replace(":path", path:gsub("&apikey=[^&]*", ""):gsub("?apikey=[^&]*$", ""):gsub("?apikey=[^&]*&", "?"))
end
remove_empty_key_query_parameters:
inline_string: |-
function envoy_on_request(request_handle)
local path = request_handle:headers():get(":path")
request_handle
:headers()
:replace(":path", path:gsub("&=[^&]*", ""):gsub("?=[^&]*$", ""):gsub("?=[^&]*&", "?"))
end
- name: envoy.filters.http.router
typed_config:
'@type': >-
Expand Down Expand Up @@ -181,12 +196,14 @@ resources:
retry_policy:
num_retries: 3
retry_on: 5xx
timeout: 35s
typed_per_filter_config: *ref_0
- match:
prefix: /auth/v1/
route:
cluster: gotrue
prefix_rewrite: /
timeout: 35s
- match:
prefix: /rest/v1/
query_parameters:
Expand All @@ -197,20 +214,25 @@ resources:
route:
cluster: postgrest
prefix_rewrite: /
timeout: 120s
timeout: 125s
typed_per_filter_config:
envoy.filters.http.lua:
'@type': >-
type.googleapis.com/envoy.extensions.filters.http.lua.v3.LuaPerRoute
name: remove_apikey_query_parameter
name: remove_apikey_and_empty_key_query_parameter
- match:
prefix: /rest/v1/
request_headers_to_remove:
- apikey
route:
cluster: postgrest
prefix_rewrite: /
timeout: 120s
timeout: 125s
typed_per_filter_config:
envoy.filters.http.lua:
'@type': >-
type.googleapis.com/envoy.extensions.filters.http.lua.v3.LuaPerRoute
name: remove_empty_key_query_parameter
- match:
prefix: /rest-admin/v1/
query_parameters:
Expand All @@ -225,7 +247,7 @@ resources:
envoy.filters.http.lua:
'@type': >-
type.googleapis.com/envoy.extensions.filters.http.lua.v3.LuaPerRoute
name: remove_apikey_query_parameter
name: remove_apikey_and_empty_key_query_parameters
- match:
prefix: /rest-admin/v1/
request_headers_to_remove:
Expand All @@ -242,7 +264,7 @@ resources:
route:
cluster: postgrest
prefix_rewrite: /rpc/graphql
timeout: 120s
timeout: 125s
- match:
prefix: /admin/v1/
route:
Expand Down Expand Up @@ -293,6 +315,13 @@ resources:
direct_remote_ip:
address_prefix: 10.0.0.0
prefix_len: 8
include_attempt_count_in_response: true
retry_policy:
num_retries: 5
retry_back_off:
base_interval: 0.1s
max_interval: 1s
retry_on: gateway-error
stat_prefix: ingress_http
- '@type': type.googleapis.com/envoy.config.listener.v3.Listener
name: https_listener
Expand Down
8 changes: 0 additions & 8 deletions ansible/files/envoy_config/remove_apikey_query_parameter.lua
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion common-nix.vars.pkr.hcl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
postgres-version = "15.6.1.116"
postgres-version = "15.6.1.117"
51 changes: 50 additions & 1 deletion testinfra/test_ami.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ def test_postgrest_can_connect_to_db(host):
# There would be an error if the `apikey` query parameter isn't removed,
# since PostgREST treats query parameters as conditions.
#
# Worth testing since remove_apikey_query_parameter.lua uses regexp instead
# Worth testing since remove_apikey_query_parameters uses regexp instead
# of parsed query parameters.
def test_postgrest_starting_apikey_query_parameter_is_removed(host):
res = requests.get(
Expand Down Expand Up @@ -388,3 +388,52 @@ def test_postgrest_ending_apikey_query_parameter_is_removed(host):
},
)
assert res.ok

# There would be an error if the empty key query parameter isn't removed,
# since PostgREST treats empty key query parameters as malformed input.
#
# Worth testing since remove_apikey_and_empty_key_query_parameters uses regexp instead
# of parsed query parameters.
def test_postgrest_starting_empty_key_query_parameter_is_removed(host):
res = requests.get(
f"http://{host.backend.get_hostname()}/rest/v1/buckets",
headers={
"accept-profile": "storage",
},
params={
"": "empty_key",
"id": "eq.absent",
"apikey": service_role_key,
},
)
assert res.ok


def test_postgrest_middle_empty_key_query_parameter_is_removed(host):
res = requests.get(
f"http://{host.backend.get_hostname()}/rest/v1/buckets",
headers={
"accept-profile": "storage",
},
params={
"apikey": service_role_key,
"": "empty_key",
"id": "eq.absent",
},
)
assert res.ok


def test_postgrest_ending_empty_key_query_parameter_is_removed(host):
res = requests.get(
f"http://{host.backend.get_hostname()}/rest/v1/buckets",
headers={
"accept-profile": "storage",
},
params={
"id": "eq.absent",
"apikey": service_role_key,
"": "empty_key",
},
)
assert res.ok
53 changes: 51 additions & 2 deletions testinfra/test_ami_nix.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ def test_postgrest_can_connect_to_db(host):
# There would be an error if the `apikey` query parameter isn't removed,
# since PostgREST treats query parameters as conditions.
#
# Worth testing since remove_apikey_query_parameter.lua uses regexp instead
# Worth testing since remove_apikey_query_parameters uses regexp instead
# of parsed query parameters.
def test_postgrest_starting_apikey_query_parameter_is_removed(host):
res = requests.get(
Expand Down Expand Up @@ -387,4 +387,53 @@ def test_postgrest_ending_apikey_query_parameter_is_removed(host):
"apikey": service_role_key,
},
)
assert res.ok
assert res.ok

# There would be an error if the empty key query parameter isn't removed,
# since PostgREST treats empty key query parameters as malformed input.
#
# Worth testing since remove_apikey_and_empty_key_query_parameters uses regexp instead
# of parsed query parameters.
def test_postgrest_starting_empty_key_query_parameter_is_removed(host):
res = requests.get(
f"http://{host.backend.get_hostname()}/rest/v1/buckets",
headers={
"accept-profile": "storage",
},
params={
"": "empty_key",
"id": "eq.absent",
"apikey": service_role_key,
},
)
assert res.ok


def test_postgrest_middle_empty_key_query_parameter_is_removed(host):
res = requests.get(
f"http://{host.backend.get_hostname()}/rest/v1/buckets",
headers={
"accept-profile": "storage",
},
params={
"apikey": service_role_key,
"": "empty_key",
"id": "eq.absent",
},
)
assert res.ok


def test_postgrest_ending_empty_key_query_parameter_is_removed(host):
res = requests.get(
f"http://{host.backend.get_hostname()}/rest/v1/buckets",
headers={
"accept-profile": "storage",
},
params={
"id": "eq.absent",
"apikey": service_role_key,
"": "empty_key",
},
)
assert res.ok
Loading