Skip to content

Commit 930df17

Browse files
committed
fix: address PR review comments
- 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)
1 parent 24f7e0c commit 930df17

4 files changed

Lines changed: 20 additions & 18 deletions

File tree

api/v2/apisixroute_types.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ func (exprs ApisixRouteHTTPMatchExprs) ToVars() (result adc.Vars, err error) {
313313
case ScopeBody:
314314
subj = "post_arg." + expr.Subject.Name
315315
default:
316-
return result, errors.New("invalid http match expr: subject.scope should be one of [query, header, cookie, path, variable, body]")
316+
return result, errors.New("invalid http match expr: subject.scope should be one of [Query, Header, Cookie, Path, Variable, Body]")
317317
}
318318
this.SliceVal = append(this.SliceVal, adc.StringOrSlice{StrVal: subj})
319319

@@ -417,13 +417,15 @@ type ApisixRouteHTTPMatchExprSubject struct {
417417
// Supported values: `Header`, `Query`, `Path`, `Cookie`, `Variable`, `Body`.
418418
// When Scope is `Path`, Name will be ignored.
419419
// When Scope is `Body`, Name supports dot-notation JSON path (e.g., "model.version",
420-
// "messages[*].role") and maps to APISIX's post_arg.* variable, which works with
420+
// "messages[*].role") and maps to APISIX's post_arg.<name> variable, which works with
421421
// application/json, application/x-www-form-urlencoded, and multipart/form-data.
422422
// +kubebuilder:validation:Enum=Header;Query;Path;Cookie;Variable;Body
423423
Scope string `json:"scope" yaml:"scope"`
424-
// Name is the name of the header, query parameter, cookie, variable, or body field.
425-
// Required for all scopes except Path.
426-
Name string `json:"name" yaml:"name"`
424+
// Name is the name of the subject within the given scope: the header name, query
425+
// parameter name, cookie name, Nginx variable name, or body field name (dot-notation
426+
// JSON path supported for Body scope). Optional when Scope is Path.
427+
// +kubebuilder:validation:Optional
428+
Name string `json:"name,omitempty" yaml:"name,omitempty"`
427429
}
428430

429431
func init() {

api/v2/shared_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ const (
8888
ScopeVariable = "Variable"
8989
// ScopeBody means the route match expression subject is in the request body.
9090
// Name supports dot-notation JSON path (e.g., "model.version", "messages[*].role"),
91-
// and maps to APISIX's post_arg.* variable, which supports application/json,
91+
// and maps to APISIX's post_arg.<name> variable, which supports application/json,
9292
// application/x-www-form-urlencoded, and multipart/form-data content types.
9393
ScopeBody = "Body"
9494
)

config/crd/bases/apisix.apache.org_apisixroutes.yaml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -207,16 +207,17 @@ spec:
207207
properties:
208208
name:
209209
description: |-
210-
Name is the name of the header, query parameter, cookie, variable, or body field.
211-
Required for all scopes except Path.
210+
Name is the name of the subject within the given scope: the header name, query
211+
parameter name, cookie name, Nginx variable name, or body field name (dot-notation
212+
JSON path supported for Body scope). Optional when Scope is Path.
212213
type: string
213214
scope:
214215
description: |-
215216
Scope specifies the subject scope.
216217
Supported values: `Header`, `Query`, `Path`, `Cookie`, `Variable`, `Body`.
217218
When Scope is `Path`, Name will be ignored.
218219
When Scope is `Body`, Name supports dot-notation JSON path (e.g., "model.version",
219-
"messages[*].role") and maps to APISIX's post_arg.* variable, which works with
220+
"messages[*].role") and maps to APISIX's post_arg.<name> variable, which works with
220221
application/json, application/x-www-form-urlencoded, and multipart/form-data.
221222
enum:
222223
- Header
@@ -227,7 +228,6 @@ spec:
227228
- Body
228229
type: string
229230
required:
230-
- name
231231
- scope
232232
type: object
233233
value:

test/e2e/crds/v2/route.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ spec:
330330
}
331331
Eventually(request).WithTimeout(20 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK))
332332

333-
By("verify non-matching POST without action field returns 404")
333+
By("verify non-matching POST with wrong action value returns 404")
334334
s.NewAPISIXClient().POST("/post").
335335
WithFormField("action", "logout").
336336
Expect().Status(http.StatusNotFound)
@@ -339,7 +339,7 @@ spec:
339339
s.NewAPISIXClient().GET("/get").Expect().Status(http.StatusNotFound)
340340
})
341341

342-
It("Test ApisixRoute match by body vars (JSON path)", func() {
342+
It("Test ApisixRoute match by body vars (JSON nested path)", func() {
343343
const apisixRouteSpec = `
344344
apiVersion: apisix.apache.org/v2
345345
kind: ApisixRoute
@@ -358,29 +358,29 @@ spec:
358358
exprs:
359359
- subject:
360360
scope: Body
361-
name: model
361+
name: model.version
362362
op: Equal
363363
value: gpt-4
364364
backends:
365365
- serviceName: httpbin-service-e2e-test
366366
servicePort: 80
367367
`
368-
By("apply ApisixRoute with Body scope JSON path expr")
368+
By("apply ApisixRoute with Body scope dot-notation JSON path expr")
369369
var apisixRoute apiv2.ApisixRoute
370370
applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default"},
371371
&apisixRoute, fmt.Sprintf(apisixRouteSpec, s.Namespace(), s.Namespace()))
372372

373-
By("verify matching POST with JSON body model=gpt-4 returns 200")
373+
By("verify matching POST with JSON body {model: {version: gpt-4}} returns 200")
374374
request := func() int {
375375
return s.NewAPISIXClient().POST("/post").
376-
WithJSON(map[string]string{"model": "gpt-4"}).
376+
WithJSON(map[string]any{"model": map[string]string{"version": "gpt-4"}}).
377377
Expect().Raw().StatusCode
378378
}
379379
Eventually(request).WithTimeout(20 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK))
380380

381-
By("verify non-matching JSON body returns 404")
381+
By("verify non-matching JSON body with wrong nested value returns 404")
382382
s.NewAPISIXClient().POST("/post").
383-
WithJSON(map[string]string{"model": "gpt-3"}).
383+
WithJSON(map[string]any{"model": map[string]string{"version": "gpt-3"}}).
384384
Expect().Status(http.StatusNotFound)
385385
})
386386

0 commit comments

Comments
 (0)