Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: added support for path-params outside methods for request-validation #216

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+233 −11
Open

feat: added support for path-params outside methods for request-validation #216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fc19cbd
feat: added support for path-params outside methods for request-valid…
Prashansa-K Oct 23, 2024
f9f9d82
fix: if path and operation params have same name and location,
Prashansa-K Oct 25, 2024
9f5522a
addressed PR comment
Prashansa-K Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions ...api2kong/oas3_testfiles/18-request-validator-plugin-path-params-outside-ops.expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
{
"_format_version": "3.0",
"services": [
{
"host": "backend.com",
"id": "730d612d-914b-5fe8-8ead-e6aa654318ef",
"name": "example",
"path": "/path",
"plugins": [],
"port": 80,
"protocol": "http",
"routes": [
{
"id": "bee0be08-646a-562a-91b9-71737169585b",
"methods": [
"GET"
],
"name": "example_test-common-param-common-param_get",
"paths": [
"~/test/common-param/(?\u003ccommon_param\u003e[^#?/]+)$"
],
"plugins": [
{
"config": {
"parameter_schema": [
{
"explode": false,
"in": "path",
"name": "common_param",
"required": true,
"schema": "{\"type\":\"integer\"}",
"style": "simple"
},
{
"explode": true,
"in": "query",
"name": "metadata",
"required": false,
"schema": "{\"type\":\"boolean\"}",
"style": "form"
}
],
"verbose_response": true,
"version": "draft4"
},
"enabled": true,
"id": "3bcb9a87-847d-5ccf-93dc-b2aa1b32b77d",
"name": "request-validator",
"tags": [
"OAS3_import",
"OAS3file_18-request-validator-plugin-path-params-outside-ops.yaml"
]
}
],
"regex_priority": 100,
"strip_path": false,
"tags": [
"OAS3_import",
"OAS3file_18-request-validator-plugin-path-params-outside-ops.yaml"
]
}
],
"tags": [
"OAS3_import",
"OAS3file_18-request-validator-plugin-path-params-outside-ops.yaml"
]
}
],
"upstreams": []
}
36 changes: 36 additions & 0 deletions openapi2kong/oas3_testfiles/18-request-validator-plugin-path-params-outside-ops.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# When the request-validator is added with path and operation parameters,
# the generator should automatically generate it.
Prashansa-K marked this conversation as resolved.
Show resolved Hide resolved

openapi: 3.0.2

info:
title: Example
version: 1.0.0

servers:
- url: http://backend.com/path

x-kong-plugin-request-validator: {}

paths:
/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
Comment on lines +16 to +29
Copy link
Member

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.

Copy link
Contributor Author

@Prashansa-K Prashansa-K Oct 25, 2024 •
edited
Loading

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.

responses:
'200':
description: OK
x-kong-plugin-request-validator:
enabled: true
config:
verbose_response: true
62 changes: 62 additions & 0 deletions ...g/oas3_testfiles/19-request-validator-plugin-op-params-override-path-params.expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
{
"_format_version": "3.0",
"services": [
{
"host": "backend.com",
"id": "730d612d-914b-5fe8-8ead-e6aa654318ef",
"name": "example",
"path": "/path",
"plugins": [],
"port": 80,
"protocol": "http",
"routes": [
{
"id": "bee0be08-646a-562a-91b9-71737169585b",
"methods": [
"GET"
],
"name": "example_test-common-param-common-param_get",
"paths": [
"~/test/common-param/(?\u003ccommon_param\u003e[^#?/]+)$"
],
"plugins": [
{
"config": {
"parameter_schema": [
{
"explode": false,
"in": "path",
"name": "common_param",
"required": true,
"schema": "{\"type\":\"integer\"}",
"style": "simple"
}
],
"verbose_response": true,
"version": "draft4"
},
"enabled": true,
"id": "3bcb9a87-847d-5ccf-93dc-b2aa1b32b77d",
"name": "request-validator",
"tags": [
"OAS3_import",
"OAS3file_19-request-validator-plugin-op-params-override-path-params.yaml"
]
}
],
"regex_priority": 100,
"strip_path": false,
"tags": [
"OAS3_import",
"OAS3file_19-request-validator-plugin-op-params-override-path-params.yaml"
]
}
],
"tags": [
"OAS3_import",
"OAS3file_19-request-validator-plugin-op-params-override-path-params.yaml"
]
}
],
"upstreams": []
}
36 changes: 36 additions & 0 deletions openapi2kong/oas3_testfiles/19-request-validator-plugin-op-params-override-path-params.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# When the request-validator is added with path and operation parameters,
# with both sharing same name and location, operation parameters override the path parameters.

openapi: 3.0.2

info:
title: Example
version: 1.0.0

servers:
- url: http://backend.com/path

x-kong-plugin-request-validator: {}

paths:
/test/common-param/{common-param}:
parameters:
- in: path
name: common-param
schema:
type: string
required: true
get:
parameters:
- in: path
name: common-param
schema:
type: integer
required: true
responses:
'200':
description: OK
x-kong-plugin-request-validator:
enabled: true
config:
verbose_response: true
2 changes: 1 addition & 1 deletion openapi2kong/openapi2kong.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1027,7 +1027,7 @@ func Convert(content []byte, opts O2kOptions) (map[string]interface{}, error) {

// Extract the request-validator config from the plugin list, generate it and reinsert
operationValidatorConfig, operationPluginList = getValidatorPlugin(operationPluginList, pathValidatorConfig)
validatorPlugin := generateValidatorPlugin(operationValidatorConfig, operation, opts.UUIDNamespace,
validatorPlugin := generateValidatorPlugin(operationValidatorConfig, operation, pathitem, opts.UUIDNamespace,
operationBaseName, opts.SkipID, opts.InsoCompat)
operationPluginList = insertPlugin(operationPluginList, validatorPlugin)

Expand Down
38 changes: 28 additions & 10 deletions openapi2kong/validator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,37 @@ func getDefaultParamStyle(givenStyle string, paramType string) string {
// generateParameterSchema returns the given schema if there is one, a generated
// schema if it was specified, or nil if there is none.
// Parameters include path, query, and headers
func generateParameterSchema(operation *v3.Operation, insoCompat bool) []map[string]interface{} {
parameters := operation.Parameters
if parameters == nil {
func generateParameterSchema(operation *v3.Operation, path *v3.PathItem, insoCompat bool) []map[string]interface{} {
pathParameters := path.Parameters
operationParameters := operation.Parameters
if pathParameters == nil && operationParameters == nil {
return nil
}

if len(parameters) == 0 {
totalLength := len(pathParameters) + len(operationParameters)
if totalLength == 0 {
return nil
}

result := make([]map[string]interface{}, len(parameters))
combinedParameters := make([]*v3.Parameter, 0, totalLength)

for _, pathParam := range pathParameters {
for _, opParam := range operationParameters {
// If path parameter and operation parameter share the same name and location
// operation parameter overrides the path parameter. Thus, if this check passes,
// Then we add the path param, else we skip it.
if pathParam.Name != opParam.Name && pathParam.In != opParam.In {
combinedParameters = append(combinedParameters, pathParam)
}
}
}

combinedParameters = append(combinedParameters, operationParameters...)

result := make([]map[string]interface{}, len(combinedParameters))
i := 0
for _, parameter := range parameters {

for _, parameter := range combinedParameters {
if parameter != nil {
style := getDefaultParamStyle(parameter.Style, parameter.In)

Expand Down Expand Up @@ -160,16 +178,16 @@ func generateContentTypes(operation *v3.Operation) []string {

// generateValidatorPlugin generates the validator plugin configuration, based
// on the JSON snippet, and the OAS inputs. This can return nil
func generateValidatorPlugin(configJSON []byte, operation *v3.Operation,
func generateValidatorPlugin(operationConfigJSON []byte, operation *v3.Operation, path *v3.PathItem,
uuidNamespace uuid.UUID, baseName string, skipID bool, insoCompat bool,
) *map[string]interface{} {
if len(configJSON) == 0 {
if len(operationConfigJSON) == 0 {
return nil
}
logbasics.Debug("generating validator plugin", "operation", baseName)

var pluginConfig map[string]interface{}
_ = json.Unmarshal(configJSON, &pluginConfig)
_ = json.Unmarshal(operationConfigJSON, &pluginConfig)

// create a new ID here based on the operation
if !skipID {
Expand All @@ -183,7 +201,7 @@ func generateValidatorPlugin(configJSON []byte, operation *v3.Operation,
}

if config["parameter_schema"] == nil {
parameterSchema := generateParameterSchema(operation, insoCompat)
parameterSchema := generateParameterSchema(operation, path, insoCompat)
if parameterSchema != nil {
config["parameter_schema"] = parameterSchema
config["version"] = JSONSchemaVersion
Expand Down
Loading