feat: build apisix-runtime with ngx_http_ffi_client - #480
feat: build apisix-runtime with ngx_http_ffi_client#480shreemaan-abhishek wants to merge 3 commits into
Conversation
ai-proxy, ai-proxy-multi and ai-request-rewrite send every outbound LLM request through ngx_http_ffi_client when the runtime carries it, and cost about a third of the outbound CPU time they do on lua-resty-http. The module has to be in the runtime for that to happen. api7/ngx_http_ffi_client is still private and carries no tags, so it is pinned by commit and fetched with NGX_HTTP_FFI_CLIENT_TOKEN. The token reaches the container as a BuildKit secret rather than a build arg, so it stays out of image history, and the fetch runs with the trace off so it stays out of the build log. A build without the token warns and leaves the module out; ai-proxy falls back to lua-resty-http on such a runtime, so both runtimes work. The module compiles against lua-nginx-module's public co-ctx API, which arrived in 0.10.29. OpenResty 1.29.2.4 bundles 0.10.31rc2 and has it.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRuntime packaging now supports the optional private ChangesFFI client module packaging
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant Makefile
participant RuntimeDockerfile
participant BuildScript
participant OpenResty
GitHubActions->>Makefile: Set NGX_HTTP_FFI_CLIENT_TOKEN
Makefile->>RuntimeDockerfile: Pass BuildKit secret
RuntimeDockerfile->>BuildScript: Export NGX_HTTP_FFI_CLIENT_TOKEN
BuildScript->>BuildScript: Fetch or use local ngx_http_ffi_client
BuildScript->>OpenResty: Apply conditional module configuration
BuildScript->>BuildScript: Install Lua binding when available
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
The script runs under set -x, so assigning NGX_HTTP_FFI_CLIENT_TOKEN and testing it with [ -n ... ] both echoed the token into the build log. Only a derived yes/no now reaches the trace.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build-apisix-runtime.sh`:
- Around line 42-46: Enforce the approved commit in ngx_http_ffi_client_ver
instead of allowing arbitrary refs: validate that it equals
f13fcfa4e923ad82844bf49d9d3b3d283371ef66, then verify the commit fetched by the
ngx_http_ffi_client build flow before compiling it. Update
build-apisix-runtime.sh at lines 42-46 and 147-172, and keep README.md lines
124-125 consistent with the fixed commit.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 20ac434e-1fdd-49f9-be57-fb78049936e6
📒 Files selected for processing (10)
.github/workflows/package-apisix-runtime-deb-openresty-1.21.yml.github/workflows/package-apisix-runtime-deb-ubuntu20.04.yml.github/workflows/package-apisix-runtime-rpm-el.yml.github/workflows/package-apisix-runtime-rpm-ubi.yml.github/workflows/release-apisix-runtime.ymlMakefileREADME.mdbuild-apisix-runtime.shdockerfiles/Dockerfile.apisix-runtime.debdockerfiles/Dockerfile.apisix-runtime.rpm
| ngx_http_ffi_client_ver=${ngx_http_ffi_client_ver:-"f13fcfa4e923ad82844bf49d9d3b3d283371ef66"} | ||
| if [[ ! "$ngx_http_ffi_client_ver" =~ ^[A-Za-z0-9._/-]+$ ]]; then | ||
| echo "ERROR: invalid ngx_http_ffi_client_ver: $ngx_http_ffi_client_ver" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find every entry point that can supply ngx_http_ffi_client_ver.
rg -n -C 3 '\bngx_http_ffi_client_ver\b|build_apisix_runtime|build-apisix-runtime\.sh' \
--glob '*.sh' --glob 'Dockerfile*' --glob 'Makefile' .
# Confirm whether any build path forwards this variable into the build-script environment.
rg -n -C 3 'NGX_HTTP_FFI_CLIENT_TOKEN|ngx_http_ffi_client_ver|--build-arg|--secret' \
Makefile dockerfiles .github build-apisix-runtime.shRepository: api7/apisix-build-tools
Length of output: 21159
Dependency Pin Bypass (CWE-829): Inclusion of Functionality from Untrusted Control Sphere
Enforce the fixed ngx_http_ffi_client commit.
ngx_http_ffi_client_ver accepts refs such as main. When the token is set, git fetch retrieves that ref and compiles it into the runtime. Require the exact approved commit f13fcfa4e923ad82844bf49d9d3b3d283371ef66, verify the fetched commit, and keep README.md consistent.
📍 Affects 2 files
build-apisix-runtime.sh#L42-L46(this comment)build-apisix-runtime.sh#L147-L172README.md#L124-L125
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build-apisix-runtime.sh` around lines 42 - 46, Enforce the approved commit in
ngx_http_ffi_client_ver instead of allowing arbitrary refs: validate that it
equals f13fcfa4e923ad82844bf49d9d3b3d283371ef66, then verify the commit fetched
by the ngx_http_ffi_client build flow before compiling it. Update
build-apisix-runtime.sh at lines 42-46 and 147-172, and keep README.md lines
124-125 consistent with the fixed commit.
|
Pushed 74ead74: the script runs under |
|
End-to-end check on the artifact this branch produces. NGX_HTTP_FFI_CLIENT_TOKEN=<token> make build-apisix-runtime-deb \
app=apisix-runtime runtime_version=0.0.0 \
image_base=debian image_tag=bookworm-slim arch=linux/amd64The resulting image carries the module three ways:
Then, inside that image, driving the same sequence
The CI jobs on this PR run without the secret, so they exercise the other path: the build warns and produces a runtime with no module, which is the fallback the gateway side handles. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build-apisix-runtime.sh`:
- Around line 49-54: Unset NGX_HTTP_FFI_CLIENT_TOKEN immediately after the
authenticated fetch block completes, before subsequent commands such as patch.sh
or configure run; leave the existing token-detection logic unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb5d4616-2daa-44e8-91ee-49b8eb7d67c2
📒 Files selected for processing (1)
build-apisix-runtime.sh
| NGX_HTTP_FFI_CLIENT_TOKEN=${NGX_HTTP_FFI_CLIENT_TOKEN:-} | ||
| if [ -n "$NGX_HTTP_FFI_CLIENT_TOKEN" ]; then | ||
| ngx_http_ffi_client_have_token="yes" | ||
| else | ||
| ngx_http_ffi_client_have_token="no" | ||
| fi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the token lifecycle without executing the build.
rg -n -C 8 \
'NGX_HTTP_FFI_CLIENT_TOKEN|unset|env -u|git|Authorization|http\.extraHeader|https?://' \
build-apisix-runtime.shRepository: api7/apisix-build-tools
Length of output: 5890
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact credentialed fetch and all subsequent child-process launches.
sed -n '1,12p;145,190p;213,245p' build-apisix-runtime.sh
# Confirm Bash keeps an imported variable exported after the assignment used by the script.
env NGX_HTTP_FFI_CLIENT_TOKEN='probe-token' bash -c '
NGX_HTTP_FFI_CLIENT_TOKEN=${NGX_HTTP_FFI_CLIENT_TOKEN:-}
printf "parent-exported=%s\n" "$(export -p | grep -c "NGX_HTTP_FFI_CLIENT_TOKEN")"
printf "child-value=%s\n" "$(env | sed -n "s/^NGX_HTTP_FFI_CLIENT_TOKEN=//p")"
'Repository: api7/apisix-build-tools
Length of output: 3714
Sensitive Data Exposure (CWE-526)
Reachability: External
Clear NGX_HTTP_FFI_CLIENT_TOKEN after the authenticated fetch. The variable remains exported and is inherited by later commands such as ./patch.sh and ./configure. Add unset NGX_HTTP_FFI_CLIENT_TOKEN immediately after the fetch block.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 50-50: A credential-bearing variable (e.g. PASSWORD, PASSWD, SECRET, TOKEN, API_KEY) is assigned a hardcoded string literal. Secrets committed to a script are exposed in source control, process listings, and shell history, and cannot be rotated without a code change. Read the value from a secrets manager or an injected environment variable at runtime instead (e.g. PASSWORD="${DB_PASSWORD:?must be set}"), and never commit the literal.
Context: ngx_http_ffi_client_have_token="yes"
Note: [CWE-798] Use of Hard-coded Credentials.
(hardcoded-password-assignment-bash)
[warning] 52-52: A credential-bearing variable (e.g. PASSWORD, PASSWD, SECRET, TOKEN, API_KEY) is assigned a hardcoded string literal. Secrets committed to a script are exposed in source control, process listings, and shell history, and cannot be rotated without a code change. Read the value from a secrets manager or an injected environment variable at runtime instead (e.g. PASSWORD="${DB_PASSWORD:?must be set}"), and never commit the literal.
Context: ngx_http_ffi_client_have_token="no"
Note: [CWE-798] Use of Hard-coded Credentials.
(hardcoded-password-assignment-bash)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build-apisix-runtime.sh` around lines 49 - 54, Unset
NGX_HTTP_FFI_CLIENT_TOKEN immediately after the authenticated fetch block
completes, before subsequent commands such as patch.sh or configure run; leave
the existing token-detection logic unchanged.
The build itself stays lenient so pull requests from forks, which get no secrets, still pass. A release is different: without the token it would produce an official runtime with no ngx_http_ffi_client and say so only in a warning buried in the build log.
|
Worth recording how the secret behaves here. Every green check on this PR ran the fallback path. The build log shows: with zero The reason is simply that Which left a hole: if the secret is never added, this merges green and every official runtime ships without the module, announced only by a warning buried in a build log. Pushed 1f18875 to close it, so the release workflow fails fast when the secret is missing while the build stays lenient and fork PRs keep passing. If you would rather releases degrade quietly than fail, that guard is a self-contained commit and easy to drop. Note also that Actions log masking would not have caught the |
|
Superseded by #481, which carries the identical commits with the head branch on this repository rather than a fork. Closing to keep review in one place. |
1 participant
What this does
Adds
ngx_http_ffi_clienttoapisix-runtime.It is the C HTTP client the AI plugins use for outbound LLM requests. On the
paired gateway PRs (apache/apisix and api7/api7-ee-3-gateway),
ai-proxy,ai-proxy-multiandai-request-rewriteprefer it overlua-resty-httpandspend roughly a third of the outbound CPU time doing so. The module has to be in
the runtime for any of that to take effect.
The private-repository problem
api7/ngx_http_ffi_clientis still private and carries no tags, so:ngx_http_ffi_client_ver;NGX_HTTP_FFI_CLIENT_TOKEN, which reaches the container asa BuildKit secret rather than a build arg, so it stays out of image history,
and the fetch runs with the trace off so it stays out of the build log;
The gateway side resolves the client at runtime and falls back to
lua-resty-httpwhen the module is absent, so a runtime built either way works.Only the outbound CPU cost differs.
That makes this reviewable and safe today, but it is a workaround. A public
runtime whose contents depend on a secret is not a good end state. The clean
fix is to make
api7/ngx_http_ffi_clientpublic, after which the token, theBuildKit secret and the conditional can all be dropped for a plain
git clone --depth=1 -b <tag>like every other module here. Happy to redo itthat way if that is the direction.
Why this runtime and not api7ee-runtime
The module uses lua-nginx-module's public co-ctx API
(
ngx_http_lua_get_co_ctx_data,ngx_http_lua_set_co_ctx_cleanup,ngx_http_lua_cleanup_co_ctx_pending_operation), which landed in 0.10.29.apisix-runtimebuilds OpenResty 1.29.2.4, which bundles ngx_lua 0.10.31rc2.It has them.
release/api7ee-runtimeis pinned to OpenResty 1.21.4.4, which bundles0.10.25. It does not, and the module fails to compile there with three
implicit-declaration errors.
Both the gateway repos pin
APISIX_RUNTIME=1.3.11, which is this runtime, sothis branch is the one that matters.
Verification
--add-modulepointing at the pinned commit:compiles clean, and the module's own Test::Nginx suite passes in full
(415 assertions across 15 files).
above: fails on the three missing symbols.
credential in the resulting
.git/config.reaches the
RUNstep, without it the step sees an empty value and the buildcontinues, and
docker historyon the resulting image contains no trace of it.make build-apisix-runtime-debwith a real token to check the wiringthrough the Makefile and Dockerfile.
Summary by CodeRabbit
New Features
ngx_http_ffi_clientmodule when a valid access token is provided during runtime builds.lua-resty-httpinstead.Documentation
Bug Fixes