You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "Copilot (via GitHub)" <gi...@apache.org> on 2026/02/03 01:32:11 UTC

Re: [PR] Lua plugin support for connection exempt list [trafficserver]

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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org