feat: add Body scope for request body matching in ApisixRoute#413
Open
feat: add Body scope for request body matching in ApisixRoute#413
Conversation
…y matching Add ScopeBody to ApisixRouteHTTPMatchExprSubject.Scope, which maps to APISIX's post_arg.* variable. This supports request body matching for application/json, application/x-www-form-urlencoded, and multipart/form-data content types, and allows dot-notation JSON path expressions such as 'model.version' and 'messages[*].role'. Closes #399
|
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:
📝 WalkthroughWalkthroughAdds a Body scope to HTTP match expressions: defines ChangesBody Matching in HTTP Match Expressions
Sequence Diagram(s)sequenceDiagram
participant Client
participant APISIX
participant Controller
Client->>APISIX: send POST (form/json) with body fields
APISIX->>Controller: evaluate route vars (post_arg.*)
Controller->>APISIX: match decision (uses ToVars mapping to post_arg.<name>)
APISIX-->>Client: respond 200 (match) or 404 (no match)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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
🧹 Nitpick comments (1)
api/v2/apisixroute_types_test.go (1)
52-70: ⚡ Quick winAdd a wildcard-path body test to lock in the documented behavior.
You document
messages[*].role; adding a direct test for that path will prevent regressions.🧪 Suggested test addition
func TestToVars_ScopeBody_NestedJSONPath(t *testing.T) { @@ assert.Equal(t, "post_arg.model.version", vars[0][0].StrVal) } + +func TestToVars_ScopeBody_ArrayWildcardJSONPath(t *testing.T) { + exprs := ApisixRouteHTTPMatchExprs{ + { + Subject: ApisixRouteHTTPMatchExprSubject{ + Scope: ScopeBody, + Name: "messages[*].role", + }, + Op: OpEqual, + Value: strPtr("user"), + }, + } + + vars, err := exprs.ToVars() + require.NoError(t, err) + require.Len(t, vars, 1) + assert.Equal(t, "post_arg.messages[*].role", vars[0][0].StrVal) + assert.Equal(t, "==", vars[0][1].StrVal) + assert.Equal(t, "user", vars[0][2].StrVal) +}🤖 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 `@api/v2/apisixroute_types_test.go` around lines 52 - 70, The test suite lacks a case for wildcard JSONPath body fields (e.g., messages[*].role) to lock in the documented behavior; add a new test similar to TestToVars_ScopeBody_NestedJSONPath that builds an ApisixRouteHTTPMatchExprs with Subject.Scope = ScopeBody and Subject.Name = "messages[*].role", call exprs.ToVars(), require no error and correct length, and assert the produced var maps the wildcard to the expected post_arg path (e.g., "post_arg.messages.*.role") so the dot/wildcard conversion logic in ToVars is covered.
🤖 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 `@api/v2/apisixroute_types.go`:
- Line 316: Update the validation error message that mentions subject.scope so
it uses the canonical capitalized scope literals: replace the lowercase list
"[query, header, cookie, path, variable, body]" with "[Query, Header, Cookie,
Path, Variable, Body]" in the error returned (the line that currently returns
errors.New("invalid http match expr: subject.scope should be one of [...]")) so
users see the exact accepted enum values for subject.scope.
---
Nitpick comments:
In `@api/v2/apisixroute_types_test.go`:
- Around line 52-70: The test suite lacks a case for wildcard JSONPath body
fields (e.g., messages[*].role) to lock in the documented behavior; add a new
test similar to TestToVars_ScopeBody_NestedJSONPath that builds an
ApisixRouteHTTPMatchExprs with Subject.Scope = ScopeBody and Subject.Name =
"messages[*].role", call exprs.ToVars(), require no error and correct length,
and assert the produced var maps the wildcard to the expected post_arg path
(e.g., "post_arg.messages.*.role") so the dot/wildcard conversion logic in
ToVars is covered.
🪄 Autofix (Beta)
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: 8c45306f-e298-4c06-a11f-bc4608c33330
📒 Files selected for processing (3)
api/v2/apisixroute_types.goapi/v2/apisixroute_types_test.goapi/v2/shared_types.go
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class request-body matching support to the ApisixRoute HTTP match expression model by introducing a new Body scope and translating it into APISIX vars using post_arg.* semantics.
Changes:
- Introduces
ScopeBody = "Body"in the match expression subject scope constants. - Extends
ApisixRouteHTTPMatchExprs.ToVars()to translateScopeBodyintopost_arg.<name>subjects and updates the invalid-scope error message/docs. - Adds unit tests covering body scope translation and validation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| api/v2/shared_types.go | Adds ScopeBody constant and inline documentation describing body-scope behavior. |
| api/v2/apisixroute_types.go | Adds ScopeBody handling in ToVars() and updates match-subject documentation/error message. |
| api/v2/apisixroute_types_test.go | Adds tests validating body-scope translation and empty-name error behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+91
to
+92
| // and maps to APISIX's post_arg.* variable, which supports application/json, | ||
| // application/x-www-form-urlencoded, and multipart/form-data content types. |
Contributor
Author
There was a problem hiding this comment.
Fixed: comment updated to post_arg.<name> to accurately reflect the implementation.
| subj = "post_arg." + expr.Subject.Name | ||
| default: | ||
| return result, errors.New("invalid http match expr: subject.scope should be one of [query, header, cookie, path, variable]") | ||
| return result, errors.New("invalid http match expr: subject.scope should be one of [query, header, cookie, path, variable, body]") |
Contributor
Author
There was a problem hiding this comment.
Fixed: error message now uses the canonical capitalized scope values.
| // "messages[*].role") and maps to APISIX's post_arg.* variable, which works with | ||
| // application/json, application/x-www-form-urlencoded, and multipart/form-data. | ||
| Scope string `json:"scope" yaml:"scope"` | ||
| // Name is the name of the header or query parameter. |
Contributor
Author
There was a problem hiding this comment.
Fixed: Name field comment updated to describe it generically across all scopes, with scope-specific behavior noted.
- Add +kubebuilder:validation:Enum marker to Scope field (Header/Query/Path/Cookie/Variable/Body) - Add +kubebuilder:validation:XValidation CEL rule enforcing Name is required when Scope is not Path: 'self.scope == "Path" || self.name != ""' - Regenerate CRD YAML with the new enum and x-kubernetes-validations rules - Add unit tests verifying the CEL expression correctness via cel-go evaluation - Add e2e tests for Body scope: urlencoded form field matching and JSON body matching
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
api/v2/apisixroute_types_test.go (1)
28-50: ⚡ Quick winAvoid duplicating the CEL rule string in the test.
This test hardcodes the rule instead of checking the generated schema or a shared source of truth, so it can pass while the shipped CRD drifts. That already happened in this PR: this constant is valid, but the generated CRD contains a different quote character.
🤖 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 `@api/v2/apisixroute_types_test.go` around lines 28 - 50, The test duplicates the CEL expression in the local const celSubjectRule; instead remove that hardcoded constant and have evalCELSubjectRule obtain the rule dynamically from the source of truth (the ApisixRouteHTTPMatchExprSubject validation metadata) so the test tracks the CRD. Concretely: delete celSubjectRule, update evalCELSubjectRule to read the XValidation/OpenAPI validation string for ApisixRouteHTTPMatchExprSubject (via the generated CRD/OpenAPI schema or the kubebuilder tag on the ApisixRouteHTTPMatchExprSubject type) and use that value when env.Compile is called, keeping the rest of evalCELSubjectRule unchanged.
🤖 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 `@config/crd/bases/apisix.apache.org_apisixroutes.yaml`:
- Around line 229-235: The schema currently lists "name" inside the required
array which forces all subjects to include name; remove "name" from the required
list under the object schema so the x-kubernetes-validations CEL rule
(x-kubernetes-validations -> rule: self.scope == 'Path' || self.name != '') can
actually allow omission for scope == 'Path'; also fix the rule quoting to use
regular single quotes and an empty-string comparison (self.name != '') so the
expression is valid.
- Around line 233-235: The CEL validation rule under x-kubernetes-validations
uses a typographic smart quote at the end of the expression, breaking the rule;
update the rule for the validation entry (x-kubernetes-validations) so the
expression reads self.scope == 'Path' || self.name != '' (replace the smart
quote “ ” with standard single quotes), ensuring the rule in the CRD (the rule
field referencing self.scope and self.name) uses proper ASCII single quotes.
---
Nitpick comments:
In `@api/v2/apisixroute_types_test.go`:
- Around line 28-50: The test duplicates the CEL expression in the local const
celSubjectRule; instead remove that hardcoded constant and have
evalCELSubjectRule obtain the rule dynamically from the source of truth (the
ApisixRouteHTTPMatchExprSubject validation metadata) so the test tracks the CRD.
Concretely: delete celSubjectRule, update evalCELSubjectRule to read the
XValidation/OpenAPI validation string for ApisixRouteHTTPMatchExprSubject (via
the generated CRD/OpenAPI schema or the kubebuilder tag on the
ApisixRouteHTTPMatchExprSubject type) and use that value when env.Compile is
called, keeping the rest of evalCELSubjectRule unchanged.
🪄 Autofix (Beta)
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: 174c79e5-0e48-4d68-b694-757fd8be2018
📒 Files selected for processing (4)
api/v2/apisixroute_types.goapi/v2/apisixroute_types_test.goconfig/crd/bases/apisix.apache.org_apisixroutes.yamltest/e2e/crds/v2/route.go
🚧 Files skipped from review as they are similar to previous changes (1)
- api/v2/apisixroute_types.go
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
config/crd/bases/apisix.apache.org_apisixroutes.yaml:231
- The schema/doc says
nameis "Required for all scopes except Path", but this CRD still marksnameas always required (required: [name, scope]) and allows empty string (no MinLength/XValidation). This is inconsistent with the docs and will allow manifests that pass CRD validation but later fail controller validation (e.g., Body/Header/etc with empty name). Consider makingnameoptional (omitempty) and adding an XValidation rule likeself.scope == 'Path' || self.name != '', or adjust the description to match the actual schema constraints.
name:
description: |-
Name is the name of the header, query parameter, cookie, variable, or body field.
Required for all scopes except Path.
type: string
scope:
description: |-
Scope specifies the subject scope.
Supported values: `Header`, `Query`, `Path`, `Cookie`, `Variable`, `Body`.
When Scope is `Path`, Name will be ignored.
When Scope is `Body`, Name supports dot-notation JSON path (e.g., "model.version",
"messages[*].role") and maps to APISIX's post_arg.* variable, which works with
application/json, application/x-www-form-urlencoded, and multipart/form-data.
enum:
- Header
- Query
- Path
- Cookie
- Variable
- Body
type: string
required:
- name
- scope
Comment on lines
414
to
426
| // ApisixRouteHTTPMatchExprSubject describes the subject of a route matching expression. | ||
| type ApisixRouteHTTPMatchExprSubject struct { | ||
| // Scope specifies the subject scope and can be `Header`, `Query`, or `Path`. | ||
| // Scope specifies the subject scope. | ||
| // Supported values: `Header`, `Query`, `Path`, `Cookie`, `Variable`, `Body`. | ||
| // When Scope is `Path`, Name will be ignored. | ||
| // When Scope is `Body`, Name supports dot-notation JSON path (e.g., "model.version", | ||
| // "messages[*].role") and maps to APISIX's post_arg.* variable, which works with | ||
| // application/json, application/x-www-form-urlencoded, and multipart/form-data. | ||
| // +kubebuilder:validation:Enum=Header;Query;Path;Cookie;Variable;Body | ||
| Scope string `json:"scope" yaml:"scope"` | ||
| // Name is the name of the header or query parameter. | ||
| // Name is the name of the header, query parameter, cookie, variable, or body field. | ||
| // Required for all scopes except Path. | ||
| Name string `json:"name" yaml:"name"` |
Contributor
Author
There was a problem hiding this comment.
The conditional enforcement via CEL XValidation was intentionally omitted. The runtime check in ToVars() returns an error when name is empty for non-Path scopes, which provides adequate protection at the controller level without adding CRD-level CEL complexity.
| } | ||
| Eventually(request).WithTimeout(20 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK)) | ||
|
|
||
| By("verify non-matching POST without action field returns 404") |
Contributor
Author
There was a problem hiding this comment.
Fixed: step description updated to 'non-matching POST with wrong action value'.
Comment on lines
+342
to
+364
| It("Test ApisixRoute match by body vars (JSON path)", func() { | ||
| const apisixRouteSpec = ` | ||
| apiVersion: apisix.apache.org/v2 | ||
| kind: ApisixRoute | ||
| metadata: | ||
| name: default | ||
| namespace: %s | ||
| spec: | ||
| ingressClassName: %s | ||
| http: | ||
| - name: rule0 | ||
| match: | ||
| paths: | ||
| - /* | ||
| methods: | ||
| - POST | ||
| exprs: | ||
| - subject: | ||
| scope: Body | ||
| name: model | ||
| op: Equal | ||
| value: gpt-4 | ||
| backends: |
Contributor
Author
There was a problem hiding this comment.
Fixed: test renamed to 'JSON nested path' and updated to use actual dot-notation (model.version) with a nested JSON body {"model": {"version": "gpt-4"}}.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
api/v2/apisixroute_types.go (1)
316-316:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse canonical scope literals in the invalid-scope error message.
Line 316 still lists lowercase scope names, while accepted enum values are capitalized (
Query,Header,Cookie,Path,Variable,Body), which can mislead users.💡 Suggested patch
- return result, errors.New("invalid http match expr: subject.scope should be one of [query, header, cookie, path, variable, body]") + return result, errors.New("invalid http match expr: subject.scope should be one of [Query, Header, Cookie, Path, Variable, Body]")🤖 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 `@api/v2/apisixroute_types.go` at line 316, Update the invalid-scope error string so it uses the canonical, capitalized enum literals instead of lowercase; specifically modify the error returned in the line that currently returns errors.New("invalid http match expr: subject.scope should be one of [query, header, cookie, path, variable, body]") to list [Query, Header, Cookie, Path, Variable, Body] so the message matches the accepted enum values (keep the surrounding text and use the same return of result and errors.New).
🧹 Nitpick comments (1)
api/v2/apisixroute_types_test.go (1)
52-70: ⚡ Quick winAdd a wildcard JSON-path test to match documented behavior.
Current coverage validates dotted paths, but docs/examples also claim support for paths like
messages[*].role. A dedicated test here would prevent regressions in the advertised behavior.💡 Suggested test addition
+func TestToVars_ScopeBody_WildcardJSONPath(t *testing.T) { + exprs := ApisixRouteHTTPMatchExprs{ + { + Subject: ApisixRouteHTTPMatchExprSubject{ + Scope: ScopeBody, + Name: "messages[*].role", + }, + Op: OpEqual, + Value: strPtr("user"), + }, + } + + vars, err := exprs.ToVars() + require.NoError(t, err) + require.Len(t, vars, 1) + assert.Equal(t, "post_arg.messages[*].role", vars[0][0].StrVal) +}🤖 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 `@api/v2/apisixroute_types_test.go` around lines 52 - 70, Add a new unit test to cover wildcard JSON-path handling: extend or add a test (e.g., near TestToVars_ScopeBody_NestedJSONPath) that constructs an ApisixRouteHTTPMatchExprs entry with Subject.Scope = ScopeBody and Name = "messages[*].role", calls exprs.ToVars(), asserts no error, asserts vars length, and verifies the produced mapping uses the expected post_arg JSON-path translation (e.g., that the generated var string matches the documented wildcard mapping such as "post_arg.messages[*].role" or the project's canonical representation). This ensures ToVars correctly preserves wildcard JSON-paths for ScopeBody.
🤖 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.
Duplicate comments:
In `@api/v2/apisixroute_types.go`:
- Line 316: Update the invalid-scope error string so it uses the canonical,
capitalized enum literals instead of lowercase; specifically modify the error
returned in the line that currently returns errors.New("invalid http match expr:
subject.scope should be one of [query, header, cookie, path, variable, body]")
to list [Query, Header, Cookie, Path, Variable, Body] so the message matches the
accepted enum values (keep the surrounding text and use the same return of
result and errors.New).
---
Nitpick comments:
In `@api/v2/apisixroute_types_test.go`:
- Around line 52-70: Add a new unit test to cover wildcard JSON-path handling:
extend or add a test (e.g., near TestToVars_ScopeBody_NestedJSONPath) that
constructs an ApisixRouteHTTPMatchExprs entry with Subject.Scope = ScopeBody and
Name = "messages[*].role", calls exprs.ToVars(), asserts no error, asserts vars
length, and verifies the produced mapping uses the expected post_arg JSON-path
translation (e.g., that the generated var string matches the documented wildcard
mapping such as "post_arg.messages[*].role" or the project's canonical
representation). This ensures ToVars correctly preserves wildcard JSON-paths for
ScopeBody.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f780a057-a57e-4053-9177-d5726adea1dd
📒 Files selected for processing (3)
api/v2/apisixroute_types.goapi/v2/apisixroute_types_test.goconfig/crd/bases/apisix.apache.org_apisixroutes.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- config/crd/bases/apisix.apache.org_apisixroutes.yaml
Contributor
conformance test report - apisix-standalone modeapiVersion: gateway.networking.k8s.io/v1
date: "2026-05-09T09:00:48Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
result: success
statistics:
Failed: 0
Passed: 12
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests succeeded.
- core:
failedTests:
- HTTPRouteInvalidBackendRefUnknownKind
result: failure
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 1
Passed: 31
Skipped: 1
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 11
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests failed with 1 test failures. Extended tests partially succeeded
with 1 test skips.
- core:
result: partial
skippedTests:
- TLSRouteSimpleSameNamespace
statistics:
Failed: 0
Passed: 10
Skipped: 1
name: GATEWAY-TLS
summary: Core tests partially succeeded with 1 test skips. |
Contributor
conformance test report - apisix modeapiVersion: gateway.networking.k8s.io/v1
date: "2026-05-09T09:00:58Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
result: success
statistics:
Failed: 0
Passed: 12
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests succeeded.
- core:
failedTests:
- HTTPRouteInvalidBackendRefUnknownKind
result: failure
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 1
Passed: 31
Skipped: 1
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 11
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests failed with 1 test failures. Extended tests partially succeeded
with 1 test skips.
- core:
result: partial
skippedTests:
- TLSRouteSimpleSameNamespace
statistics:
Failed: 0
Passed: 10
Skipped: 1
name: GATEWAY-TLS
summary: Core tests partially succeeded with 1 test skips. |
- Fix error message scope values to use canonical casing (Query/Header/etc.) - Fix ScopeBody doc comment: post_arg.* -> post_arg.<name> - Update Name field comment to be generic across all scopes - Make Name field optional (omitempty) so scope:Path does not require name - Fix e2e step description: 'without action field' -> 'wrong action value' - Fix e2e JSON path test to use actual dot-notation (model.version)
Add +kubebuilder:validation:XValidation rule requiring name to be non-empty when scope is not Path. Regenerate CRD YAML and update CRD reference docs accordingly. All unit and CEL tests pass.
| | --- | --- | | ||
| | `scope` _string_ | Scope specifies the subject scope and can be `Header`, `Query`, or `Path`. When Scope is `Path`, Name will be ignored. | | ||
| | `name` _string_ | Name is the name of the header or query parameter. | | ||
| | `scope` _string_ | Scope specifies the subject scope. Supported values: `Header`, `Query`, `Path`, `Cookie`, `Variable`, `Body`. When Scope is `Path`, Name will be ignored. When Scope is `Body`, Name supports dot-notation JSON path (e.g., "model.version", "messages[*].role") and maps to APISIX's post_arg.<name> variable, which works with application/json, application/x-www-form-urlencoded, and multipart/form-data. | |
| // Supported values: `Header`, `Query`, `Path`, `Cookie`, `Variable`, `Body`. | ||
| // When Scope is `Path`, Name will be ignored. | ||
| // When Scope is `Body`, Name supports dot-notation JSON path (e.g., "model.version", | ||
| // "messages[*].role") and maps to APISIX's post_arg.<name> variable, which works with |
| Supported values: `Header`, `Query`, `Path`, `Cookie`, `Variable`, `Body`. | ||
| When Scope is `Path`, Name will be ignored. | ||
| When Scope is `Body`, Name supports dot-notation JSON path (e.g., "model.version", | ||
| "messages[*].role") and maps to APISIX's post_arg.<name> variable, which works with |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
api/v2/apisixroute_types_test.go (2)
131-145: ⚡ Quick winUse
require.Errorbefore dereferencingerr.Error()At Line [144],
assert.Errordoes not stop execution; the next line may panic iferris nil. Preferrequire.Errorhere to make the failure deterministic.Proposed fix
- assert.Error(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "empty subject.name")🤖 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 `@api/v2/apisixroute_types_test.go` around lines 131 - 145, In TestToVars_ScopeBody_EmptyName_ReturnsError, ensure the test fails fast by asserting that err is non-nil before calling err.Error(); replace the non-fatal assertion with a fatal one (use require.Error) for the result of exprs.ToVars() so the subsequent call to err.Error() cannot panic if err is nil.
77-85: ⚡ Quick winPrefer asserting the CEL rule from parsed CRD structure, not only raw text
Raw
assert.Containson the full YAML can pass even if the rule is misplaced. Consider traversingx-kubernetes-validationsin the unmarshaled CRD and asserting the rule there for stronger signal.🤖 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 `@api/v2/apisixroute_types_test.go` around lines 77 - 85, The test currently checks the CEL rule by inspecting raw := string(data); instead, unmarshal the CRD YAML (data) into a map[string]interface{} or typed struct, traverse to the x-kubernetes-validations field under the CRD's validation schema (e.g., locate the map sequence at keys like "spec"/"validation"/"openAPIV3Schema"/"properties" or directly "x-kubernetes-validations"), find the validation entry that contains the "rule" (CEL) expression and assert that its value equals the expected "self.scope == 'Path' || self.name != ''" (use assert.Equal or assert.Contains on that field) and also perform the NotContains checks on that extracted rule string to ensure no smart quotes; reference the variables/functions in the test (data, raw, and the assertions) when making the change so the assertion targets the parsed CRD structure rather than the raw YAML text.
🤖 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.
Nitpick comments:
In `@api/v2/apisixroute_types_test.go`:
- Around line 131-145: In TestToVars_ScopeBody_EmptyName_ReturnsError, ensure
the test fails fast by asserting that err is non-nil before calling err.Error();
replace the non-fatal assertion with a fatal one (use require.Error) for the
result of exprs.ToVars() so the subsequent call to err.Error() cannot panic if
err is nil.
- Around line 77-85: The test currently checks the CEL rule by inspecting raw :=
string(data); instead, unmarshal the CRD YAML (data) into a
map[string]interface{} or typed struct, traverse to the x-kubernetes-validations
field under the CRD's validation schema (e.g., locate the map sequence at keys
like "spec"/"validation"/"openAPIV3Schema"/"properties" or directly
"x-kubernetes-validations"), find the validation entry that contains the "rule"
(CEL) expression and assert that its value equals the expected "self.scope ==
'Path' || self.name != ''" (use assert.Equal or assert.Contains on that field)
and also perform the NotContains checks on that extracted rule string to ensure
no smart quotes; reference the variables/functions in the test (data, raw, and
the assertions) when making the change so the assertion targets the parsed CRD
structure rather than the raw YAML text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4f11df85-6d61-4e59-8993-42d1cfa7249c
📒 Files selected for processing (4)
api/v2/apisixroute_types.goapi/v2/apisixroute_types_test.goconfig/crd/bases/apisix.apache.org_apisixroutes.yamldocs/en/latest/reference/api-reference.md
🚧 Files skipped from review as they are similar to previous changes (2)
- api/v2/apisixroute_types.go
- config/crd/bases/apisix.apache.org_apisixroutes.yaml
Replace self.name != '' with size(self.name) > 0 in the XValidation rule. The trailing '' in a YAML plain scalar was being mangled by kustomize during make install, producing an invalid rule. Using size() avoids any string-literal quoting at line end. Update test to walk the parsed YAML structure instead of raw substring match.
Contributor
conformance test reportapiVersion: gateway.networking.k8s.io/v1
date: "2026-05-09T09:21:54Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
failedTests:
- GRPCExactMethodMatching
- GRPCRouteHeaderMatching
- GRPCRouteListenerHostnameMatching
- GatewayModifyListeners
result: failure
statistics:
Failed: 4
Passed: 8
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests failed with 4 test failures.
- core:
failedTests:
- GatewayModifyListeners
result: failure
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 1
Passed: 31
Skipped: 1
extended:
failedTests:
- HTTPRouteBackendProtocolWebSocket
result: failure
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 1
Passed: 10
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests failed with 1 test failures. Extended tests failed with 1 test
failures.
- core:
failedTests:
- GatewayModifyListeners
- TLSRouteSimpleSameNamespace
result: failure
statistics:
Failed: 2
Passed: 9
Skipped: 0
name: GATEWAY-TLS
summary: Core tests failed with 2 test failures. |
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.
Summary
ScopeBody = "Body"constant toApisixRouteHTTPMatchExprSubject.Scopepost_arg.*variable, supportingapplication/json,application/x-www-form-urlencoded, andmultipart/form-dataNamefield (e.g.model.version,messages[*].role)Usage
```yaml
apiVersion: apisix.apache.org/v2
kind: ApisixRoute
metadata:
name: body-match-route
spec:
http:
match:
paths: ["/api/submit"]
methods: ["POST"]
exprs:
Match a simple form/JSON field
scope: Body
name: action
op: Equal
value: "login"
Match a nested JSON path
scope: Body
name: model.version
op: Equal
value: "gpt-4"
backends:
servicePort: 80
```
Test Plan
TestToVars_ScopeBody_SimpleField— simple field maps topost_arg.actionTestToVars_ScopeBody_NestedJSONPath— dot-notation maps topost_arg.model.versionTestToVars_ScopeBody_EmptyName_ReturnsError— empty Name returns errorCloses #399
Summary by CodeRabbit
New Features
Documentation / Validation
Tests