Copilot commented on code in PR #12849:
URL: https://github.com/apache/trafficserver/pull/12849#discussion_r2756732567
##########
plugins/lua/ts_lua_misc.cc:
##########
@@ -631,3 +646,54 @@ ts_lua_get_traffic_server_version(lua_State *L)
lua_pushstring(L, s);
return 1;
}
+
+static int
+ts_lua_connection_limit_exempt_list_add(lua_State *L)
+{
+ size_t len;
+ const char *ip_ranges;
+
+ ip_ranges = luaL_checklstring(L, 1, &len);
+
+ if (ip_ranges && len > 0) {
+ TSReturnCode ret =
TSConnectionLimitExemptListAdd(std::string_view(ip_ranges, len));
Review Comment:
These new Lua bindings add user-visible behavior but there doesn’t appear to
be a gold test covering them. Consider adding a pluginTest/lua gold test that
exercises add/remove/clear and verifies the per-client connection limit exempt
behavior (e.g., max_connections_in throttles before add, then no throttling
after adding 127.0.0.1/32, and throttling returns after remove/clear).
##########
plugins/lua/ts_lua_misc.cc:
##########
@@ -631,3 +646,54 @@ ts_lua_get_traffic_server_version(lua_State *L)
lua_pushstring(L, s);
return 1;
}
+
+static int
+ts_lua_connection_limit_exempt_list_add(lua_State *L)
+{
+ size_t len;
+ const char *ip_ranges;
+
+ ip_ranges = luaL_checklstring(L, 1, &len);
+
+ if (ip_ranges && len > 0) {
+ TSReturnCode ret =
TSConnectionLimitExemptListAdd(std::string_view(ip_ranges, len));
+ if (ret == TS_SUCCESS) {
+ lua_pushboolean(L, 1);
+ } else {
+ lua_pushboolean(L, 0);
+ }
+ } else {
+ lua_pushboolean(L, 0);
+ }
Review Comment:
`luaL_checklstring` never returns null and the underlying TS API treats an
empty string as a successful no-op (it will parse zero ranges and return
TS_SUCCESS). The `len > 0` check causes
`ts.connection_limit_exempt_list_add("")` to return false even though the
wrapped API would succeed; consider removing the length check and returning
based solely on the TSConnectionLimitExemptListAdd return code.
##########
plugins/lua/ts_lua_misc.cc:
##########
@@ -631,3 +646,54 @@ ts_lua_get_traffic_server_version(lua_State *L)
lua_pushstring(L, s);
return 1;
}
+
+static int
+ts_lua_connection_limit_exempt_list_add(lua_State *L)
+{
+ size_t len;
+ const char *ip_ranges;
+
+ ip_ranges = luaL_checklstring(L, 1, &len);
+
+ if (ip_ranges && len > 0) {
+ TSReturnCode ret =
TSConnectionLimitExemptListAdd(std::string_view(ip_ranges, len));
+ if (ret == TS_SUCCESS) {
+ lua_pushboolean(L, 1);
+ } else {
+ lua_pushboolean(L, 0);
+ }
+ } else {
+ lua_pushboolean(L, 0);
+ }
+
+ return 1;
+}
+
+static int
+ts_lua_connection_limit_exempt_list_remove(lua_State *L)
+{
+ size_t len;
+ const char *ip_ranges;
+
+ ip_ranges = luaL_checklstring(L, 1, &len);
+
+ if (ip_ranges && len > 0) {
+ TSReturnCode ret =
TSConnectionLimitExemptListRemove(std::string_view(ip_ranges, len));
+ if (ret == TS_SUCCESS) {
+ lua_pushboolean(L, 1);
+ } else {
+ lua_pushboolean(L, 0);
+ }
+ } else {
+ lua_pushboolean(L, 0);
+ }
Review Comment:
Same as add(): the `len > 0` guard makes an empty string return false even
though the wrapped TSConnectionLimitExemptListRemove would succeed as a no-op.
Consider removing the length check and returning the boolean based on the
TSConnectionLimitExemptListRemove return code.
##########
doc/admin-guide/plugins/lua.en.rst:
##########
@@ -452,6 +452,73 @@ Here is an example:
:ref:`TOP <admin-plugins-ts-lua>`
+ts.connection_limit_exempt_list_add
+------------------------------------
+**syntax:** *success = ts.connection_limit_exempt_list_add(IP_RANGES)*
+
+**context:** global
+
+**description**: Add IP ranges to the per-client connection limit exempt list.
This function wraps the TSConnectionLimitExemptListAdd API.
+
+The IP_RANGES parameter should be a string containing one or more IP address
ranges in CIDR notation, separated by commas. Client connections from these IP
ranges will be exempt from per-client connection limits.
+
Review Comment:
The docs say IP_RANGES must be in CIDR notation, but the underlying parsing
(swoc::IPRange) accepts a single IP address and dash ranges in addition to
CIDR. Consider updating the description to match the supported formats (and
note that whitespace around comma-separated entries is not trimmed, so `", "`
may fail).
##########
doc/admin-guide/plugins/lua.en.rst:
##########
@@ -452,6 +452,73 @@ Here is an example:
:ref:`TOP <admin-plugins-ts-lua>`
+ts.connection_limit_exempt_list_add
+------------------------------------
+**syntax:** *success = ts.connection_limit_exempt_list_add(IP_RANGES)*
+
+**context:** global
+
+**description**: Add IP ranges to the per-client connection limit exempt list.
This function wraps the TSConnectionLimitExemptListAdd API.
+
+The IP_RANGES parameter should be a string containing one or more IP address
ranges in CIDR notation, separated by commas. Client connections from these IP
ranges will be exempt from per-client connection limits.
+
+Returns true on success, false on failure.
+
+Here is an example:
+
+::
+
+ if ts.connection_limit_exempt_list_add('10.0.0.0/8,192.168.1.0/24') then
+ ts.debug('Successfully added IP ranges to exempt list')
+ else
+ ts.error('Failed to add IP ranges to exempt list')
+ end
+
+:ref:`TOP <admin-plugins-ts-lua>`
+
+ts.connection_limit_exempt_list_remove
+---------------------------------------
+**syntax:** *success = ts.connection_limit_exempt_list_remove(IP_RANGES)*
+
+**context:** global
+
+**description**: Remove IP ranges from the per-client connection limit exempt
list. This function wraps the TSConnectionLimitExemptListRemove API.
+
+The IP_RANGES parameter should be a string containing one or more IP address
ranges in CIDR notation, separated by commas.
Review Comment:
Same doc issue as add(): IP_RANGES is documented as CIDR-only, but the
underlying API accepts single IPs and dash ranges too; also consider noting
that whitespace around comma separators is not ignored.
```suggestion
The IP_RANGES parameter should be a string containing one or more IP address
specifications separated by commas. Each
specification may be a CIDR range (for example, ``192.168.1.0/24``), a
single IP address (for example, ``192.168.1.10``),
or a dash-separated IP range (for example, ``192.168.1.10-192.168.1.20``).
Whitespace around the commas is not ignored,
so do not include spaces before or after commas.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
