From a214fdd34531ccd57e97c96ed56066a3af00295a Mon Sep 17 00:00:00 2001 From: Prashansa Kulshrestha Date: Wed, 6 Nov 2024 11:35:10 +0530 Subject: [PATCH] fix: added fix to validate for top-level type in parameter schemas while using oneOf in request-validator plugin. (#215) * fix: added fix to validate for top-level type in parameter schemas while using oneOf in request-validator plugin. As of now, deck file openapi2kong command was not checking for multiple types used while creating parameter schemas with oneOf, which is not supported by Kong request- validator plugin. Thus, we are forcing for defining a top-level type property and erroring out in case it is not present. * fix: lint issue fix * fix: extended validation to anyOf schemas * fix: fixed the case when no oneOf or anyOf schemas are present * fix: lint fix * test: added testcase for >1 definitions in req-validator generation * fix: fixed logical error that did not account for the uncertainty in maps iteration order in golang --- openapi2kong/jsonschema.go | 10 +- ...validator-plugin-oneOf-usage.expected.json | 78 ++++++++++++ ...-request-validator-plugin-oneOf-usage.yaml | 58 +++++++++ openapi2kong/openapi2kong.go | 6 +- openapi2kong/validator.go | 111 ++++++++++++++++-- 5 files changed, 247 insertions(+), 16 deletions(-) create mode 100644 openapi2kong/oas3_testfiles/17-request-validator-plugin-oneOf-usage.expected.json create mode 100644 openapi2kong/oas3_testfiles/17-request-validator-plugin-oneOf-usage.yaml diff --git a/openapi2kong/jsonschema.go b/openapi2kong/jsonschema.go index 809acc8..9272045 100644 --- a/openapi2kong/jsonschema.go +++ b/openapi2kong/jsonschema.go @@ -51,10 +51,12 @@ func dereferenceSchema(sr *base.SchemaProxy, seenBefore map[string]*base.SchemaP // extractSchema will extract a schema, including all sub-schemas/references and // return it as a single JSONschema string. All components will be moved under the -// "#/definitions/" key. -func extractSchema(s *base.SchemaProxy) string { +// "#/definitions/" key. Along with that, it will also return the schema as a +// map, so that if any further operations or validations need to be done on the schema +// they could be performed. +func extractSchema(s *base.SchemaProxy) (string, map[string]interface{}) { if s == nil || s.Schema() == nil { - return "" + return "", nil } seenBefore := make(map[string]*base.SchemaProxy) @@ -92,5 +94,5 @@ func extractSchema(s *base.SchemaProxy) string { result, _ := json.Marshal(finalSchema) // update the $ref values; this is safe because plain " (double-quotes) would be escaped if in actual values - return strings.ReplaceAll(string(result), "\"$ref\":\"#/components/schemas/", "\"$ref\":\"#/definitions/") + return strings.ReplaceAll(string(result), "\"$ref\":\"#/components/schemas/", "\"$ref\":\"#/definitions/"), finalSchema } diff --git a/openapi2kong/oas3_testfiles/17-request-validator-plugin-oneOf-usage.expected.json b/openapi2kong/oas3_testfiles/17-request-validator-plugin-oneOf-usage.expected.json new file mode 100644 index 0000000..35828e5 --- /dev/null +++ b/openapi2kong/oas3_testfiles/17-request-validator-plugin-oneOf-usage.expected.json @@ -0,0 +1,78 @@ +{ + "_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": "1446ecde-7037-5f9c-8537-8217e2a12bfa", + "methods": [ + "GET" + ], + "name": "example_params-test_get", + "paths": [ + "~/params/test$" + ], + "plugins": [ + { + "config": { + "body_schema": "{}", + "parameter_schema": [ + { + "explode": true, + "in": "query", + "name": "queryid", + "required": true, + "schema": "{\"oneOf\":[{\"example\":10,\"type\":\"integer\"},{\"example\":2.5,\"type\":\"number\"}],\"type\":\"number\"}", + "style": "form" + }, + { + "explode": false, + "in": "header", + "name": "testHeader", + "required": true, + "schema": "{\"$ref\":\"#/definitions/headerType\",\"definitions\":{\"headerType\":{\"oneOf\":[{\"$ref\":\"#/definitions/stringType\"},{\"$ref\":\"#/definitions/numberType\"}],\"type\":\"string\"},\"numberType\":{\"example\":2.5,\"type\":\"number\"},\"stringType\":{\"example\":\"10\",\"type\":\"string\"}}}", + "style": "simple" + }, + { + "explode": false, + "in": "header", + "name": "secondTestHeader", + "required": true, + "schema": "{\"$ref\":\"#/definitions/secondHeaderType\",\"definitions\":{\"numberType\":{\"example\":2.5,\"type\":\"number\"},\"secondHeaderType\":{\"oneOf\":[{\"$ref\":\"#/definitions/stringType\"},{\"$ref\":\"#/definitions/numberType\"}],\"type\":\"string\"},\"stringType\":{\"example\":\"10\",\"type\":\"string\"}}}", + "style": "simple" + } + ], + "version": "draft4" + }, + "enabled": true, + "id": "8bd60198-9b34-5f0b-9240-4826c7c331a0", + "name": "request-validator", + "tags": [ + "OAS3_import", + "OAS3file_17-request-validator-plugin-oneOf-usage.yaml" + ] + } + ], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_17-request-validator-plugin-oneOf-usage.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_17-request-validator-plugin-oneOf-usage.yaml" + ] + } + ], + "upstreams": [] +} \ No newline at end of file diff --git a/openapi2kong/oas3_testfiles/17-request-validator-plugin-oneOf-usage.yaml b/openapi2kong/oas3_testfiles/17-request-validator-plugin-oneOf-usage.yaml new file mode 100644 index 0000000..caff31a --- /dev/null +++ b/openapi2kong/oas3_testfiles/17-request-validator-plugin-oneOf-usage.yaml @@ -0,0 +1,58 @@ +# When the request-validator is added without a body or parameter schema +# the generator should automatically generate it. + +openapi: 3.0.2 + +info: + title: Example + version: 1.0.0 + +servers: + - url: http://backend.com/path + +x-kong-plugin-request-validator: {} + +paths: + /params/test: + get: + x-kong-plugin-request-validator: + enabled: true + config: + body_schema: '{}' + parameters: + - in: query + name: queryid + schema: + type: number + oneOf: + - type: integer + example: 10 + - type: number + example: 2.5 + required: true + - in: header + name: testHeader + schema: + $ref: '#/components/schemas/headerType' + required: true + - in: header + name: secondTestHeader + schema: + $ref: '#/components/schemas/secondHeaderType' + required: true +components: + schemas: + headerType: + type: string + oneOf: + - $ref: '#/components/schemas/stringType' + - $ref: '#/components/schemas/numberType' + secondHeaderType: + $ref: '#/components/schemas/headerType' + stringType: + type: string + example: "10" + numberType: + type: number + example: 2.5 + diff --git a/openapi2kong/openapi2kong.go b/openapi2kong/openapi2kong.go index 84c32a4..58fe1b7 100644 --- a/openapi2kong/openapi2kong.go +++ b/openapi2kong/openapi2kong.go @@ -1027,8 +1027,12 @@ 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, pathitem, opts.UUIDNamespace, + validatorPlugin, err := generateValidatorPlugin(operationValidatorConfig, operation, pathitem, opts.UUIDNamespace, operationBaseName, opts.SkipID, opts.InsoCompat) + if err != nil { + return nil, fmt.Errorf("failed to create validator plugin: %w", err) + } + operationPluginList = insertPlugin(operationPluginList, validatorPlugin) // construct the route diff --git a/openapi2kong/validator.go b/openapi2kong/validator.go index 8f9dc77..77a98f1 100644 --- a/openapi2kong/validator.go +++ b/openapi2kong/validator.go @@ -2,6 +2,7 @@ package openapi2kong import ( "encoding/json" + "fmt" "mime" "sort" "strings" @@ -33,16 +34,18 @@ 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, path *v3.PathItem, insoCompat bool) []map[string]interface{} { +func generateParameterSchema(operation *v3.Operation, path *v3.PathItem, + insoCompat bool, +) ([]map[string]interface{}, error) { pathParameters := path.Parameters operationParameters := operation.Parameters if pathParameters == nil && operationParameters == nil { - return nil + return nil, nil } totalLength := len(pathParameters) + len(operationParameters) if totalLength == 0 { - return nil + return nil, nil } combinedParameters := make([]*v3.Parameter, 0, totalLength) @@ -90,9 +93,15 @@ func generateParameterSchema(operation *v3.Operation, path *v3.PathItem, insoCom paramConf["required"] = false } - schema := extractSchema(parameter.Schema) + schema, schemaMap := extractSchema(parameter.Schema) if schema != "" { paramConf["schema"] = schema + + typeStr, oneOfAnyOfFound := fetchTopLevelType(schemaMap) + if typeStr == "" && oneOfAnyOfFound { + return nil, + fmt.Errorf(`parameter schemas for request-validator plugin must have a top-level type property`) + } } result[i] = paramConf @@ -100,7 +109,7 @@ func generateParameterSchema(operation *v3.Operation, path *v3.PathItem, insoCom } } - return result + return result, nil } func parseMediaType(mediaType string) (string, string, error) { @@ -137,7 +146,8 @@ func generateBodySchema(operation *v3.Operation) string { return "" } if typ == "application" && (subtype == "json" || strings.HasSuffix(subtype, "+json")) { - return extractSchema((*contentValue).Schema) + schema, _ := extractSchema((*contentValue).Schema) + return schema } contentItem = contentItem.Next() @@ -180,9 +190,9 @@ func generateContentTypes(operation *v3.Operation) []string { // on the JSON snippet, and the OAS inputs. This can return nil func generateValidatorPlugin(operationConfigJSON []byte, operation *v3.Operation, path *v3.PathItem, uuidNamespace uuid.UUID, baseName string, skipID bool, insoCompat bool, -) *map[string]interface{} { +) (*map[string]interface{}, error) { if len(operationConfigJSON) == 0 { - return nil + return nil, nil } logbasics.Debug("generating validator plugin", "operation", baseName) @@ -201,7 +211,10 @@ func generateValidatorPlugin(operationConfigJSON []byte, operation *v3.Operation } if config["parameter_schema"] == nil { - parameterSchema := generateParameterSchema(operation, path, insoCompat) + parameterSchema, err := generateParameterSchema(operation, path, insoCompat) + if err != nil { + return nil, err + } if parameterSchema != nil { config["parameter_schema"] = parameterSchema config["version"] = JSONSchemaVersion @@ -219,7 +232,7 @@ func generateValidatorPlugin(operationConfigJSON []byte, operation *v3.Operation // unless the content-types have been provided by the user if config["allowed_content_types"] == nil { // also not provided, so really nothing to validate, don't add a plugin - return nil + return nil, nil } // add an empty schema, which passes everything, but it also activates the // content-type check @@ -236,5 +249,81 @@ func generateValidatorPlugin(operationConfigJSON []byte, operation *v3.Operation } } - return &pluginConfig + return &pluginConfig, nil +} + +// This function checks if there is a oneOf or anyOf schema present in the passed schemaMap. +// The first return value (string) indicates the top-level type for the oneOf/anyOf schema. +// The second return value (bool) indicates if either of oneOf/anyOf is found in the schemaMap. +// +// 1. If the oneOf/anyOf schema is found, it tries to find the top-level type defined with +// the oneOf/anyOf schema. +// -- If the top-level type is found, it is returned along with "true". +// -- If the top-level type is not found, a blank string is returned with "true". +// 2. If the oneOf/anyOf schema is not found, the function will return +// a blank string with "false". +func fetchTopLevelType(schemaMap map[string]interface{}) (string, bool) { + var ( + typeStr string + oneOfFound bool + anyOfFound bool + ) + + isSlice := func(value interface{}) bool { + _, ok := value.([]interface{}) + return ok + } + + // We need to check for oneOf and anyOf first, as we need the + // top-level type from the same level from the map. + // Without checking for those, the recusion may enter the + // oneOf or anyOf maps and return the type from there. + // This would defeat our purpose of checking for the top-level type + + // Check if oneOf exists at the current level + if oneOf, ok := schemaMap["oneOf"]; ok { + oneOfFound = isSlice(oneOf) + } + + // Check if anyOf exists at the current level + if anyOf, ok := schemaMap["anyOf"]; ok { + anyOfFound = isSlice(anyOf) + } + + // Check if type exists at the current level + if typ, ok := schemaMap["type"]; ok { + if str, isString := typ.(string); isString { + typeStr = str + } + } + + // If both oneOf and type are found at this level, return them + if oneOfFound && typeStr != "" || anyOfFound && typeStr != "" { + return typeStr, true + } + + // Recursively search in nested objects + for _, value := range schemaMap { + switch v := value.(type) { + case map[string]interface{}: + if str, oneOfAnyOfFound := fetchTopLevelType(v); oneOfAnyOfFound { + return str, true + } + case []interface{}: + for _, item := range v { + if itemMap, isMap := item.(map[string]interface{}); isMap { + if str, oneOfAnyOfFound := fetchTopLevelType(itemMap); oneOfAnyOfFound { + return str, true + } + } + } + } + } + + if !oneOfFound && !anyOfFound { + // there is no oneOf or anyOf schema, thus returning false + return "", false + } + + return "", true }