shreemaan-abhishek commented on code in PR #13778:
URL: https://github.com/apache/apisix/pull/13778#discussion_r3765250765


##########
apisix/plugins/ai-transport/http.lua:
##########
@@ -19,17 +19,130 @@
 -- Provides HTTP client lifecycle management for AI provider requests.
 
 local core = require("apisix.core")
-local http = require("resty.http")
 local ngx_now = ngx.now
 local pairs = pairs
 local ipairs = ipairs
 local pcall = pcall
+local require = require
 local type = type
 local str_lower = string.lower
 local tostring = tostring
 
+local FFI_CLIENT = "ngx_http_ffi_client"
+local LUA_RESTY_HTTP = "lua-resty-http"
+
+-- the client name in the config is not the module name
+local CLIENT_MODULES = {
+    [FFI_CLIENT] = "resty.ngx_http_ffi_client",
+    [LUA_RESTY_HTTP] = "resty.http",
+}
+
+local attr_schema = {
+    type = "object",
+    properties = {
+        http_client = {
+            type = "string",
+            enum = {FFI_CLIENT, LUA_RESTY_HTTP},
+            default = FFI_CLIENT,
+        },
+    },
+}
+
 local _M = {}
 
+local http_client
+local http_client_is_ffi
+
+
+--- Pick the outbound HTTP client.
+-- `plugin_attr.ai-proxy.http_client` names it: "ngx_http_ffi_client", the
+-- default, or "lua-resty-http". The first is a C client with the same object
+-- API as the second and around half its outbound CPU cost, and it exists only
+-- when the gateway runtime was built with the module.
+-- Resolved on first request, because local_conf is not readable while the
+-- module is still loading, and cached only once a client has been loaded.
+local function resolve_client()
+    if http_client then
+        return http_client
+    end
+
+    local local_conf = core.config.local_conf()
+    local attr = core.table.try_read_attr(local_conf, "plugin_attr", 
"ai-proxy") or {}
+
+    local ok, err = core.schema.check(attr_schema, attr)
+    if not ok then
+        core.log.error("invalid plugin_attr.ai-proxy: ", err)
+        return nil, "invalid plugin_attr.ai-proxy: " .. err
+    end
+
+    local name = attr.http_client or FFI_CLIENT
+    local module_name = CLIENT_MODULES[name]
+
+    local mod
+    ok, mod = pcall(require, module_name)
+    if not ok or type(mod) ~= "table" then
+        core.log.error(module_name, " is not available: ", mod)
+        return nil, module_name .. " is not available: " .. tostring(mod)
+    end
+
+    http_client = mod
+    http_client_is_ffi = name == FFI_CLIENT
+
+    return http_client
+end
+
+
+--- Resolve the upstream name the way every other socket in the gateway does.
+-- Cosockets are patched (apisix/patch.lua) to run names through
+-- core.resolver, which honours dns_resolver, /etc/hosts and the search
+-- domains. The C client dials on its own and only sees nginx's `resolver`,
+-- so the name is resolved here and kept for the Host header and the SNI.
+local function resolve_upstream_host(params)

Review Comment:
   Filed as api7/ngx_http_ffi_client#45.
   
   Worth separating two things in your question. The duplication part is 
addressed in this PR: the resolution now lives in `apisix/utils/http.lua` 
rather than in the AI transport, so plugins share it instead of each carrying a 
copy. What a hook would fix is the remaining footgun — every caller still has 
to remember to call `resolve_upstream_host()` before `connect()`, and 
forgetting it fails only against hostnames that nginx's resolver cannot answer, 
which is easy to miss locally.
   
   One design note I put in the issue: this cannot follow the same route as the 
trust-store fix in #43. There the value lived in ngx_lua's loc conf and was 
readable from C. `core.resolver` is a Lua module whose lookup yields, so C 
cannot call it synchronously — the hook has to sit in the Lua binding and 
resolve before entering the FFI boundary. That makes it cheap, no C change 
needed.
   
   Three details from implementing the workaround that the hook needs to 
preserve, all in the issue: resolve before the keepalive pool key is derived 
(two names resolving to different addresses must not share a pooled connection, 
the same invariant #43 established for the CA), keep the original name for the 
`Host` header and the SNI (the binding defaults `ssl_server_name` to 
`opts.host`, so substituting first silently turns the SNI into an IP and breaks 
verification), and short-circuit IP literals.
   
   Once it lands, `resolve_upstream_host()` and its call site come out.



-- 
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]

Reply via email to