feat: upgrade apisix-runtime to OpenResty 1.29.2.4#462
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates the APISIX runtime build script to pin OpenResty 1.29.2.4, change the OpenSSL 3 install invocation, add a verify_module_commit helper, and make module retrieval deterministic by handling copy-vs-clone and enforcing pinned commit SHAs. ChangesOpenResty 1.29.2.4 upgrade and module source retrieval improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Upgrades the apisix-runtime build script to build against OpenResty 1.29.2.4 and OpenSSL 3.4.1, including temporary pinning of specific dependency commits to keep required patches applying during the transition.
Changes:
- Bump OpenResty version to
1.29.2.4and keep OpenSSL at3.4.1. - Temporarily pin
ngx_multi_upstream_moduleandapisix-nginx-moduleto specific commits and add logic to clone+checkout those commits. - Adjust OpenSSL install step to
make install_sw install_ssldirs.
Comments suppressed due to low confidence (1)
build-apisix-runtime.sh:132
- Similarly, when
apisix_nginx_module_commitis set (it is by default), the clone always usesmain, soapisix_nginx_module_verisn’t used for cloning in the default path. This makes the behavior harder to reason about and relies ongit fetchby SHA for correctness. Consider cloning$apisix_nginx_module_verand then checking out the pinned commit (or add a comment explaining whymainis required).
apisix_nginx_module_cloned=1
apisix_nginx_module_clone_ref="$apisix_nginx_module_ver"
if [ -n "$apisix_nginx_module_commit" ]; then
apisix_nginx_module_clone_ref="main"
fi
git clone --depth=1 -b $apisix_nginx_module_clone_ref \
https://github.com/api7/apisix-nginx-module.git \
apisix-nginx-module-${apisix_nginx_module_ver}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
build-apisix-runtime.sh (1)
93-111:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce pinned commit even when module source is copied from local workdir.
When
repomatches the module name, the script copies local sources and skips commit checkout. Ifngx_multi_upstream_module_commit/apisix_nginx_module_commitis set, this can build from an unintended local HEAD and break reproducibility (Line 93 and Line 121 paths).Suggested fix
if [ -n "$ngx_multi_upstream_module_commit" ] && [ "$ngx_multi_upstream_module_cloned" = 1 ]; then git -C ngx_multi_upstream_module-${ngx_multi_upstream_module_ver} fetch --depth=1 \ origin "$ngx_multi_upstream_module_commit" git -C ngx_multi_upstream_module-${ngx_multi_upstream_module_ver} checkout \ "$ngx_multi_upstream_module_commit" +elif [ -n "$ngx_multi_upstream_module_commit" ]; then + current_commit=$(git -C ngx_multi_upstream_module-${ngx_multi_upstream_module_ver} rev-parse HEAD) + if [ "$current_commit" != "$ngx_multi_upstream_module_commit" ]; then + echo "ERROR: ngx_multi_upstream_module HEAD ($current_commit) != pinned commit ($ngx_multi_upstream_module_commit)" >&2 + exit 1 + fi fi @@ if [ -n "$apisix_nginx_module_commit" ] && [ "$apisix_nginx_module_cloned" = 1 ]; then git -C apisix-nginx-module-${apisix_nginx_module_ver} fetch --depth=1 \ origin "$apisix_nginx_module_commit" git -C apisix-nginx-module-${apisix_nginx_module_ver} checkout \ "$apisix_nginx_module_commit" +elif [ -n "$apisix_nginx_module_commit" ]; then + current_commit=$(git -C apisix-nginx-module-${apisix_nginx_module_ver} rev-parse HEAD) + if [ "$current_commit" != "$apisix_nginx_module_commit" ]; then + echo "ERROR: apisix-nginx-module HEAD ($current_commit) != pinned commit ($apisix_nginx_module_commit)" >&2 + exit 1 + fi fiAlso applies to: 121-139
🤖 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 93 - 111, The script currently copies local workdir when repo == ngx_multi_upstream_module and sets ngx_multi_upstream_module_cloned=0, which skips later commit checkout and breaks reproducibility if ngx_multi_upstream_module_commit is set; fix by ensuring that when ngx_multi_upstream_module_commit is non-empty you either perform a proper git clone of the repo at that commit (use ngx_multi_upstream_module_clone_ref and git clone/fetch/checkout) instead of raw cp, or after copying initialize a git repo inside ngx_multi_upstream_module-${ngx_multi_upstream_module_ver} (git init, git remote add origin, git fetch --depth=1 origin <commit>, git checkout <commit>) so the subsequent checkout logic works; apply the same change for the apisix_nginx_module code path (the analogous variables apisix_nginx_module_commit and apisix_nginx_module_cloned) so pinned commits are enforced in both cases.
🤖 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.
Outside diff comments:
In `@build-apisix-runtime.sh`:
- Around line 93-111: The script currently copies local workdir when repo ==
ngx_multi_upstream_module and sets ngx_multi_upstream_module_cloned=0, which
skips later commit checkout and breaks reproducibility if
ngx_multi_upstream_module_commit is set; fix by ensuring that when
ngx_multi_upstream_module_commit is non-empty you either perform a proper git
clone of the repo at that commit (use ngx_multi_upstream_module_clone_ref and
git clone/fetch/checkout) instead of raw cp, or after copying initialize a git
repo inside ngx_multi_upstream_module-${ngx_multi_upstream_module_ver} (git
init, git remote add origin, git fetch --depth=1 origin <commit>, git checkout
<commit>) so the subsequent checkout logic works; apply the same change for the
apisix_nginx_module code path (the analogous variables
apisix_nginx_module_commit and apisix_nginx_module_cloned) so pinned commits are
enforced in both cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f1d14880-9a35-40e0-b155-e602cc6fafff
📒 Files selected for processing (1)
build-apisix-runtime.sh
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
build-apisix-runtime.sh:150
- Same as above for apisix-nginx-module: the
cp -rpath will hitgit ... rev-parse HEADwhen a pinned commit is set, but that will fail (and terminate the script) if the copied directory isn't a valid git checkout/worktree. Please add an explicit git work-tree detection with a clearer error message (or skip the commit check when git metadata isn't present).
fi
if [ "$repo" == wasm-nginx-module ]; then
cp -r "$prev_workdir" ./wasm-nginx-module-${wasm_nginx_module_ver}
else
git clone --depth=1 -b $wasm_nginx_module_ver \
nic-6443
approved these changes
May 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upgrades the apisix-runtime build script to OpenResty 1.29.2.4 and OpenSSL 3.4.1.
The OpenResty patch module dependencies now use released tags:
1.3.31.19.5Validation:
git diff --checkbash -n build-apisix-runtime.sh