fix(apisix): use req_headers for active health checks - #541
Conversation
|
chenbowen seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe active health-check request header property is renamed from ChangesUpstream header configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Description
Apache APISIX defines custom request headers for active upstream health checks as
checks.active.req_headers, but ADC currently defines the field ashttp_request_headersin both its configuration schema and APISIX standalone typing.Because these schemas use
z.strictObject, a valid APISIX configuration containingreq_headersis rejected byadc lint,adc sync, and other commands that run the local lint stage. Conversely, the ADC-only field name cannot be accepted by the APISIX upstream schema.The canonical APISIX field is defined in
apisix/schema_def.luaand documented asupstream.checks.active.req_headers.Environment
ghcr.io/api7/adc:0.27.1sha256:f65f53dd966827ff485471d00b9ce8ca146f7aba187519817b7453e476a274b899142525450fThe reproduction only uses
adc lint, so it does not require a running APISIX instance.Why this update is needed
APISIX has used
req_headersfor active health-check request headers across its supported upstream schema. ADC'shttp_request_headersname does not match the Admin API/standalone configuration contract.This mismatch currently requires downstream users to patch the compiled
/main.cjsbundle withsed. Fixing the source schemas removes that image-level workaround and keeps ADC configuration validation aligned with APISIX.How to reproduce
Create
adc.yaml:Run ADC 0.27.1:
docker run --rm \ -v "$PWD/adc.yaml:/work/adc.yaml:ro" \ ghcr.io/api7/adc:0.27.1 \ lint -f /work/adc.yamlActual result before this change (exit status 1):
Expected result:
The locally built CLI with this change returns exit status 0 for the same file.
Changes
http_request_headerstoreq_headersin the SDK upstream schema.schema.jsonso both inline and referenced upstream definitions exposereq_headers.req_headers.Verification
Results:
dist/apps/cli/main.cjs: 2 occurrences ofreq_headers:and 0 occurrences ofhttp_request_headers:.Compatibility
This intentionally replaces the incorrect ADC-only field name. Configurations using
http_request_headersmust rename it toreq_headers; the old name is not valid in the APISIX upstream schema and cannot be synchronized successfully to APISIX.Checklist
Summary by CodeRabbit
New Features
req_headerssetting.Bug Fixes
req_headersfor services and upstreams.