feat: Support runtime control of connection rate limiting via socket#930
feat: Support runtime control of connection rate limiting via socket#930b1tamara wants to merge 2 commits into
Conversation
Co-authored-by: Alexander Nicke <alexander.nicke@sap.com> Co-authored-by: Dariquest <daria.anton@sap.com> Co-authored-by: M Rizwan Shaik <m.rizwan.shaik@sap.com>
cf-foundation-community-automation
Bot
moved this to Inbox
in Application Runtime Platform Working Group
| if_p("ha_proxy.connections_rate_limit.table_size", "ha_proxy.connections_rate_limit.window_size") do | ||
| if p("ha_proxy.connections_rate_limit.block", false) | ||
| if !p("ha_proxy.connections_rate_limit.connections", nil) | ||
| abort "Conflicting configuration: connections_rate_limit.connections must be set when connections_rate_limit.block is true (otherwise every client with >= 1 connection would be blocked)" |
There was a problem hiding this comment.
The abort is reasonable as a UX guard, but the rationale ("every client with >= 1 connection would be blocked") is not what would actually happen with the new rule: when proc.conn_rate_limit is undefined, the second guard { var(proc.conn_rate_limit) -m int gt 0 } evaluates falsy and the reject does not fire. So the real failure mode is "block flag honored but never enforced" (silent no-op), not "everyone locked out". The latter was true of the old reject rule format, but no longer applies. Suggest rewording, e.g.:
| abort "Conflicting configuration: connections_rate_limit.connections must be set when connections_rate_limit.block is true (otherwise every client with >= 1 connection would be blocked)" | |
| abort "connections_rate_limit.connections must be set in the manifest as the initial threshold when block is true; otherwise rate-limiting will be silently disabled until a value is set via the runtime API." |
|
|
||
| The `connections_rate_limit.block` flag and `connections_rate_limit.connections` threshold are stored as HAProxy process-level variables (`proc.conn_rate_block` and `proc.conn_rate_limit`) and can be changed at runtime without a reload. This requires `ha_proxy.master_cli_enable: true` or `ha_proxy.stats_enable: true`. | ||
|
|
||
| The socket is located at `/var/vcap/sys/run/haproxy/stats.sock`. You will likely need `sudo` to access it. |
There was a problem hiding this comment.
| The socket is located at `/var/vcap/sys/run/haproxy/stats.sock`. You will likely need `sudo` to access it. | |
| The socket is located at `/var/vcap/sys/run/haproxy/stats.sock`. You will likely need `root` permissions to access it. |
|
|
||
| > Note: You will likely need `sudo` permission to run socat. | ||
|
|
||
| ## Control Connection Rate Limiting via HAProxy Runtime API |
There was a problem hiding this comment.
Maybe make the two new proc. variables more prominent, by introducing them in an unsorted list, similar to the variables and options listed in the section "Configuration Options". It was first a bit hard for me to understand why we have these proc variables now.
One point about the naming:
connections_rate_limit.block -> proc.conn_rate_block
connections_rate_limit.connections -> proc.conn_rate_limit
Is inconsistent. If we could still change the it and accept it's breaking, I'd prefer the following variable naming:
connection_rate_limit.enabled -> proc.connection_rate_limit_enabled
connection_rate_limit.connections -> proc.connection_rate_limit_connections
This would be more consistent, and having enabled instead of block indicates that this is a boolean switch.
2 participants
The PR introduces process-level variables (for the properties block and connections) for the connection rate limit feature, enabling runtime control via the HAProxy Runtime API. It allows dynamic start/stop of rate limiting and on-the-fly threshold changes
Co-Authored-By: @Dariquest and @Mrizwanshaik