-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: added support for path-params outside methods for request-validation #216
base: main
Are you sure you want to change the base?
Conversation
…ation. This change allows the deck file openapi2kong command to create request validation configuration for path parameters directly under a route, not nested under an operation/method. Till now, we supported path parameters nested under a method. Fixes: #207
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #216 +/- ##
==========================================
+ Coverage 68.15% 69.08% +0.93%
==========================================
Files 24 24
Lines 2553 3290 +737
==========================================
+ Hits 1740 2273 +533
- Misses 644 848 +204
Partials 169 169 ☔ View full report in Codecov by Sentry. |
openapi2kong/oas3_testfiles/18-request-validator-plugin-path-params-outside-ops.yaml
Show resolved
Hide resolved
/test/common-param/{common-param}: | ||
parameters: | ||
- in: path | ||
name: common-param | ||
schema: | ||
type: integer | ||
required: true | ||
get: | ||
parameters: | ||
- in: query | ||
name: metadata | ||
schema: | ||
type: boolean | ||
required: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a second path-parameter, that gets added to the path-level parameters array, but ALSO to the operation level? The one on the operation level should override the path-level one by the same name.
probably best to also add that description to the top comments explaining the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a different test for this.
Also, for a config like below:
paths:
/test/common-param/{common-param}:
parameters:
- in: path
name: common-param
schema:
type: integer
required: true
get:
parameters:
- in: path
name: common-param
schema:
type: number
required: true
I have added type as number in operation param schema. The request-validator doesn't seem to override, as tested. So, if this is the expected case, I have added the logic to skip path-level parameters in case operation one shares the same name and location.
prioritise operation param
- in: path | ||
name: common-param | ||
schema: | ||
type: integer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the parameters are both the same. Can we change the path level one to a string
to verify that the operation level one overrides it?
type: integer | |
type: string |
This change allows the deck file openapi2kong command to create request validation configuration for path parameters directly under a route, not nested under an operation/method. Till now, we supported path parameters nested under a method.
Fixes: #207