From 60a3faaa3438f903eb1c219d396c21ed4fa8a2d1 Mon Sep 17 00:00:00 2001 From: Opender Singh Date: Tue, 20 Apr 2021 13:30:17 +1200 Subject: [PATCH] Respect request validator plugin defined at the document root and hierarchy of plugin definitions (#3293) --- .../request-validator-plugin.expected.json | 40 +++- .../request-validator-plugin.yaml | 15 +- .../src/__tests__/common.test.js | 37 ++++ packages/openapi-2-kong/src/common.js | 17 ++ .../__tests__/plugins.test.js | 79 ++++---- .../__tests__/services.test.js | 187 ++++++++++++++++++ .../src/declarative-config/plugins.js | 96 +++++---- .../src/declarative-config/services.js | 23 ++- .../src/kubernetes/__tests__/plugins.test.js | 37 ---- .../openapi-2-kong/src/kubernetes/plugins.js | 17 +- 10 files changed, 401 insertions(+), 147 deletions(-) diff --git a/packages/openapi-2-kong/src/__fixtures__/request-validator-plugin.expected.json b/packages/openapi-2-kong/src/__fixtures__/request-validator-plugin.expected.json index a69ce48390..c82a4a3ff7 100644 --- a/packages/openapi-2-kong/src/__fixtures__/request-validator-plugin.expected.json +++ b/packages/openapi-2-kong/src/__fixtures__/request-validator-plugin.expected.json @@ -3,7 +3,16 @@ "services": [ { "name": "Example", - "plugins": [], + "plugins": [ + { + "config": { + "body_schema": "{}", + "version": "draft4" + }, + "enabled": true, + "name": "request-validator" + } + ], "routes": [ { "methods": [ @@ -21,7 +30,33 @@ "application/xml" ], "body_schema": "{\"type\":\"object\",\"properties\":{\"id\":{\"type\":\"integer\"},\"name\":{\"type\":\"string\"}}}", - "verbose_response": true, + "version": "draft4" + }, + "enabled": true, + "name": "request-validator" + } + ], + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_request-validator-plugin.yaml" + ] + }, + { + "methods": [ + "GET" + ], + "name": "Example-global-get", + "paths": [ + "/global$" + ], + "plugins": [ + { + "config": { + "allowed_content_types": [ + "application/json" + ], + "body_schema": "{\"type\":\"object\",\"properties\":{\"id\":{\"type\":\"integer\"},\"name\":{\"type\":\"string\"}}}", "version": "draft4" }, "enabled": true, @@ -55,7 +90,6 @@ "style": "form" } ], - "verbose_response": true, "version": "draft4" }, "enabled": true, diff --git a/packages/openapi-2-kong/src/__fixtures__/request-validator-plugin.yaml b/packages/openapi-2-kong/src/__fixtures__/request-validator-plugin.yaml index 10417e783f..3e4eda3779 100644 --- a/packages/openapi-2-kong/src/__fixtures__/request-validator-plugin.yaml +++ b/packages/openapi-2-kong/src/__fixtures__/request-validator-plugin.yaml @@ -7,13 +7,22 @@ info: servers: - url: http://backend.com/path +x-kong-plugin-request-validator: + paths: + /global: + get: + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/jsonSchema' /params: get: x-kong-plugin-request-validator: enabled: true config: - verbose_response: true + body_scheme: '{}' parameters: - in: path name: userId @@ -22,10 +31,6 @@ paths: required: true /body: post: - x-kong-plugin-request-validator: - enabled: true - config: - verbose_response: true requestBody: content: application/json: diff --git a/packages/openapi-2-kong/src/__tests__/common.test.js b/packages/openapi-2-kong/src/__tests__/common.test.js index ab2cca9bd2..5213d5796a 100644 --- a/packages/openapi-2-kong/src/__tests__/common.test.js +++ b/packages/openapi-2-kong/src/__tests__/common.test.js @@ -1,5 +1,6 @@ // @flow import { + distinctByProperty, fillServerVariables, generateSlug, getMethodAnnotationName, @@ -239,4 +240,40 @@ describe('common', () => { expect(result.pathname).toBe('/api/v1'); }); }); + + describe('distinctByProperty()', () => { + it('returns empty array if no truthy items', () => { + expect(distinctByProperty([], i => i)).toHaveLength(0); + expect(distinctByProperty([undefined], i => i)).toHaveLength(0); + expect(distinctByProperty([null, undefined, ''], i => i)).toHaveLength(0); + }); + + it('should remove objects with the same property selector - removes 2/4', () => { + const item1 = { name: 'a', value: 'first' }; + const item2 = { name: 'a', value: 'second' }; + const item3 = { name: 'b', value: 'third' }; + const item4 = { name: 'b', value: 'fourth' }; + const items = [item1, item2, item3, item4]; + + // distinct by the name property + const filtered = distinctByProperty(items, i => i.name); + + // Should remove item2 and item4 + expect(filtered).toEqual([item1, item3]); + }); + + it('should remove objects with the same property selector - removes none', () => { + const item1 = { name: 'a', value: 'first' }; + const item2 = { name: 'a', value: 'second' }; + const item3 = { name: 'b', value: 'third' }; + const item4 = { name: 'b', value: 'fourth' }; + const items = [item1, item2, item3, item4]; + + // distinct by the value property + const filtered = distinctByProperty(items, i => i.value); + + // Should remove no items + expect(filtered).toEqual(items); + }); + }); }); diff --git a/packages/openapi-2-kong/src/common.js b/packages/openapi-2-kong/src/common.js index 16f9897f32..3c3238524d 100644 --- a/packages/openapi-2-kong/src/common.js +++ b/packages/openapi-2-kong/src/common.js @@ -148,3 +148,20 @@ export function joinPath(p1: string, p2: string): string { return `${p1}/${p2}`; } + +// Select first unique instance of an array item depending on the property selector +export function distinctByProperty(arr: Array, propertySelector: (item: T) => any): Array { + const result: Array = []; + const set = new Set(); + + for (const item of arr.filter(i => i)) { + const selector = propertySelector(item); + if (set.has(selector)) { + continue; + } + + set.add(selector); + result.push(item); + } + return result; +} diff --git a/packages/openapi-2-kong/src/declarative-config/__tests__/plugins.test.js b/packages/openapi-2-kong/src/declarative-config/__tests__/plugins.test.js index f547edc37e..7b3272b7a0 100644 --- a/packages/openapi-2-kong/src/declarative-config/__tests__/plugins.test.js +++ b/packages/openapi-2-kong/src/declarative-config/__tests__/plugins.test.js @@ -1,12 +1,26 @@ // @flow -import { generateServerPlugins, generatePlugin, generateRequestValidatorPlugin } from '../plugins'; +import { generateGlobalPlugins, generateRequestValidatorPlugin } from '../plugins'; describe('plugins', () => { - describe('generateServerPlugins()', () => { - it('generates plugin given a server with a plugin attached', async () => { - const server = { - url: 'https://insomnia.rest', + describe('generateGlobalPlugins()', () => { + it('generates plugin given a spec with a plugin attached', async () => { + const api: OpenApi3Spec = { + openapi: '3.0.2', + info: { + title: 'something', + version: '12', + }, + paths: {}, + 'x-kong-plugin-request-validator': { + enabled: false, + config: { verbose_response: true }, + }, + 'x-kong-plugin-abcd': { + config: { + some_config: ['something'], + }, + }, 'x-kong-plugin-key-auth': { name: 'key-auth', config: { @@ -15,51 +29,36 @@ describe('plugins', () => { }, }; - const result = generateServerPlugins(server); - expect(result).toEqual([ + const result = generateGlobalPlugins(api); + expect(result.plugins).toEqual([ + { + name: 'abcd', // name from plugin tag + config: { + some_config: ['something'], + }, + }, { name: 'key-auth', config: { key_names: ['x-api-key'], }, }, + { + config: { + body_schema: '{}', + verbose_response: true, + version: 'draft4', + }, + enabled: false, + name: 'request-validator', + }, ]); - }); - }); - describe('generatePlugin()', () => { - it('generates plugin given a plugin key, and value', async () => { - const pluginKey = 'x-kong-plugin-key-auth'; - const pluginValue = { - name: 'key-auth', + expect(result.requestValidatorPlugin).toEqual({ config: { - key_names: ['x-api-key'], - }, - }; - - const result = generatePlugin(pluginKey, pluginValue); - expect(result).toEqual({ - name: 'key-auth', - config: { - key_names: ['x-api-key'], - }, - }); - }); - - it('generates name from key when missing `name` from value', async () => { - const pluginKey = 'x-kong-plugin-key-auth'; - const pluginValue = { - config: { - key_names: ['x-api-key'], - }, - }; - - const result = generatePlugin(pluginKey, pluginValue); - expect(result).toEqual({ - name: 'key-auth', - config: { - key_names: ['x-api-key'], + verbose_response: true, }, + enabled: false, }); }); }); diff --git a/packages/openapi-2-kong/src/declarative-config/__tests__/services.test.js b/packages/openapi-2-kong/src/declarative-config/__tests__/services.test.js index e70b2ebea1..4711284836 100644 --- a/packages/openapi-2-kong/src/declarative-config/__tests__/services.test.js +++ b/packages/openapi-2-kong/src/declarative-config/__tests__/services.test.js @@ -67,6 +67,193 @@ describe('services', () => { ]); }); + it('generates routes with request validator plugin from operation over path over global', async () => { + const api: OpenApi3Spec = await parseSpec({ + openapi: '3.0', + info: { version: '1.0', title: 'My API' }, + 'x-kong-plugin-request-validator': { config: { parameter_schema: 'global' } }, // global req validator plugin + servers: [ + { + url: 'https://server1.com/path', + }, + ], + paths: { + '/dogs': { + summary: 'Dog stuff', + get: {}, + post: { + summary: 'Ignored summary', + 'x-kong-plugin-request-validator': { + // operation req validator plugin + config: { parameter_schema: 'operation' }, + }, + }, + }, + '/cats': { + summary: 'Dog stuff', + 'x-kong-plugin-request-validator': { config: { parameter_schema: 'path' } }, // path req validator plugin + get: {}, + post: { + 'x-kong-plugin-request-validator': { + config: { parameter_schema: 'operation' }, // operation req validator plugin + }, + }, + }, + }, + }); + + const result = generateServices(api, ['Tag']); + expect(result).toEqual([ + { + name: 'My_API', + url: 'https://server1.com/path', + plugins: [ + { + config: { + parameter_schema: 'global', + version: 'draft4', + }, + enabled: true, + name: 'request-validator', + }, + ], + tags: ['Tag'], + routes: [ + { + name: 'My_API-dogs-get', + strip_path: false, + methods: ['GET'], + paths: ['/dogs$'], + tags: ['Tag'], + plugins: [ + { + name: 'request-validator', + // should apply global plugin + config: { parameter_schema: 'global', version: 'draft4' }, + enabled: true, + }, + ], + }, + { + name: 'My_API-dogs-post', + strip_path: false, + methods: ['POST'], + paths: ['/dogs$'], + tags: ['Tag'], + plugins: [ + { + // should have operation plugin + config: { parameter_schema: 'operation', version: 'draft4' }, + enabled: true, + name: 'request-validator', + }, + ], + }, + { + name: 'My_API-cats-get', + strip_path: false, + methods: ['GET'], + paths: ['/cats$'], + tags: ['Tag'], + plugins: [ + { + name: 'request-validator', + // should apply path plugin + config: { parameter_schema: 'path', version: 'draft4' }, + enabled: true, + }, + ], + }, + { + name: 'My_API-cats-post', + strip_path: false, + methods: ['POST'], + paths: ['/cats$'], + tags: ['Tag'], + plugins: [ + { + // should have operation plugin + config: { parameter_schema: 'operation', version: 'draft4' }, + enabled: true, + name: 'request-validator', + }, + ], + }, + ], + }, + ]); + }); + + it('generates routes with plugins from operation over path', async () => { + const api: OpenApi3Spec = await parseSpec({ + openapi: '3.0', + info: { version: '1.0', title: 'My API' }, + servers: [ + { + url: 'https://server1.com/path', + }, + ], + paths: { + '/dogs': { + summary: 'Dog stuff', + 'x-kong-plugin-key-auth': { + config: { + key_names: ['path'], + }, + }, + get: {}, + post: { + 'x-kong-plugin-key-auth': { + config: { + key_names: ['operation'], + }, + }, + }, + }, + }, + }); + + const result = generateServices(api, ['Tag']); + expect(result).toEqual([ + { + name: 'My_API', + url: 'https://server1.com/path', + plugins: [], + tags: ['Tag'], + routes: [ + { + name: 'My_API-dogs-get', + strip_path: false, + methods: ['GET'], + paths: ['/dogs$'], + tags: ['Tag'], + plugins: [ + { + name: 'key-auth', + // should apply path plugin + config: { key_names: ['path'] }, + }, + ], + }, + { + name: 'My_API-dogs-post', + strip_path: false, + methods: ['POST'], + paths: ['/dogs$'], + tags: ['Tag'], + plugins: [ + { + name: 'key-auth', + // should apply path plugin + config: { key_names: ['operation'] }, + }, + ], + }, + ], + }, + ]); + }); + it('fails with no servers', async () => { const api: OpenApi3Spec = await parseSpec({ openapi: '3.0', diff --git a/packages/openapi-2-kong/src/declarative-config/plugins.js b/packages/openapi-2-kong/src/declarative-config/plugins.js index 1531c935ba..df440ef5cc 100644 --- a/packages/openapi-2-kong/src/declarative-config/plugins.js +++ b/packages/openapi-2-kong/src/declarative-config/plugins.js @@ -1,28 +1,23 @@ // @flow -import { getPluginNameFromKey, isPluginKey } from '../common'; +import { distinctByProperty, getPluginNameFromKey, isPluginKey } from '../common'; export function isRequestValidatorPluginKey(key: string): boolean { return key.match(/-request-validator$/) != null; } -type GeneratorFn = (key: string, value: Object, iterable: Object | Array) => DCPlugin; +export function generatePlugins(item: Object): Array { + // When generating plugins, ignore the request validator plugin + // because it is handled at the operation level + const pluginFilter = ([key, _]) => isPluginKey(key) && !isRequestValidatorPluginKey(key); -export function generatePlugins(item: Object, generator: GeneratorFn): Array { - const plugins: Array = []; - - for (const key of Object.keys(item)) { - if (!isPluginKey(key)) { - continue; - } - - plugins.push(generator(key, item[key], item)); - } - - return plugins; + // Server plugins should load from the api spec root and from the server + return Object.entries(item) + .filter(pluginFilter) + .map(generatePlugin); } -export function generatePlugin(key: string, value: Object): DCPlugin { +function generatePlugin([key, value]: [string, Object]): DCPlugin { const plugin: DCPlugin = { name: value.name || getPluginNameFromKey(key), }; @@ -40,10 +35,10 @@ export function generatePlugin(key: string, value: Object): DCPlugin { */ const ALLOW_ALL_SCHEMA = '{}'; -function generateParameterSchema(operation: OA3Operation): Array | typeof undefined { +function generateParameterSchema(operation?: OA3Operation): Array | typeof undefined { let parameterSchema; - if (operation.parameters?.length) { + if (operation?.parameters?.length) { parameterSchema = []; for (const p of operation.parameters) { // The following is valid config to allow all content to pass, in the case where schema is not defined @@ -73,7 +68,7 @@ function generateParameterSchema(operation: OA3Operation): Array | typeo } function generateBodyOptions( - operation: OA3Operation, + operation?: OA3Operation, ): { bodySchema: string | typeof undefined, allowedContentTypes: Array | typeof undefined, @@ -81,7 +76,7 @@ function generateBodyOptions( let bodySchema; let allowedContentTypes; - const bodyContent = (operation.requestBody: Object)?.content; + const bodyContent = (operation?.requestBody: Object)?.content; if (bodyContent) { const jsonContentType = 'application/json'; @@ -95,7 +90,7 @@ function generateBodyOptions( return { bodySchema, allowedContentTypes }; } -export function generateRequestValidatorPlugin(plugin: Object, operation: OA3Operation): DCPlugin { +export function generateRequestValidatorPlugin(plugin: Object, operation?: OA3Operation): DCPlugin { const config: { [string]: Object } = { version: 'draft4', // Fixed version }; @@ -142,34 +137,51 @@ export function generateRequestValidatorPlugin(plugin: Object, operation: OA3Ope }; } -export function generateServerPlugins(server: OA3Server): Array { - const plugins: Array = []; +export function generateGlobalPlugins( + api: OpenApi3Spec, +): { plugins: Array, requestValidatorPlugin?: Object } { + const globalPlugins = generatePlugins(api); - for (const key of Object.keys(server)) { - if (!isPluginKey(key)) { - continue; - } - - plugins.push(generatePlugin(key, server[key])); + const requestValidatorPlugin = getRequestValidatorPluginDirective(api); + if (requestValidatorPlugin) { + globalPlugins.push(generateRequestValidatorPlugin(requestValidatorPlugin)); } - - return plugins; + return { + // Server plugins take precedence over global plugins + plugins: distinctByProperty(globalPlugins, plugin => plugin.name), + requestValidatorPlugin, + }; } -export function generateOperationPlugins(operation: OA3Operation): Array { - const plugins: Array = []; +export function generatePathPlugins(pathItem: OA3PathItem): Array { + return generatePlugins(pathItem); +} - for (const key of Object.keys(operation)) { - if (!isPluginKey(key)) { - continue; - } +export function generateOperationPlugins( + operation: OA3Operation, + pathPlugins: Array, + parentValidatorPlugin?: Object, +): Array { + const operationPlugins: Array = generatePlugins(operation); - if (isRequestValidatorPluginKey(key)) { - plugins.push(generateRequestValidatorPlugin(operation[key], operation)); - } else { - plugins.push(generatePlugin(key, operation[key])); - } + // Check if validator plugin exists on the operation + const operationValidatorPlugin = getRequestValidatorPluginDirective(operation); + + // Use the operation or parent validator plugin, or skip if neither exist + const validatorPluginToUse = operationValidatorPlugin || parentValidatorPlugin; + if (validatorPluginToUse) { + operationPlugins.push(generateRequestValidatorPlugin(validatorPluginToUse, operation)); } - return plugins; + // Operation plugins take precedence over path plugins + return distinctByProperty([...operationPlugins, ...pathPlugins], plugin => plugin.name); +} + +export function getRequestValidatorPluginDirective(obj: Object): Object | null { + const key = Object.keys(obj) + .filter(isPluginKey) + .find(isRequestValidatorPluginKey); + + // If the key is defined but is blank (therefore should be fully generated) then default to {} + return key ? obj[key] || {} : null; } diff --git a/packages/openapi-2-kong/src/declarative-config/services.js b/packages/openapi-2-kong/src/declarative-config/services.js index 0861d1e37e..f1647a92ae 100644 --- a/packages/openapi-2-kong/src/declarative-config/services.js +++ b/packages/openapi-2-kong/src/declarative-config/services.js @@ -10,7 +10,12 @@ import { } from '../common'; import { generateSecurityPlugins } from './security-plugins'; -import { generateOperationPlugins, generateServerPlugins } from './plugins'; +import { + generateOperationPlugins, + generatePathPlugins, + generateGlobalPlugins, + getRequestValidatorPluginDirective, +} from './plugins'; export function generateServices(api: OpenApi3Spec, tags: Array): Array { const servers = getAllServers(api); @@ -31,10 +36,14 @@ export function generateService( ): DCService { const serverUrl = fillServerVariables(server); const name = getName(api); + + // Service plugins + const globalPlugins = generateGlobalPlugins(api); + const service: DCService = { name, url: serverUrl, - plugins: generateServerPlugins(server), + plugins: globalPlugins.plugins, routes: [], tags, }; @@ -42,7 +51,9 @@ export function generateService( for (const routePath of Object.keys(api.paths)) { const pathItem: OA3PathItem = api.paths[routePath]; - // TODO: Add path plugins to route + const pathValidatorPlugin = getRequestValidatorPluginDirective(pathItem); + const pathPlugins = generatePathPlugins(pathItem); + for (const method of Object.keys(pathItem)) { if ( method !== 'get' && @@ -76,7 +87,11 @@ export function generateService( // Generate generic and security-related plugin objects const securityPlugins = generateSecurityPlugins(operation, api); - const regularPlugins = generateOperationPlugins(operation); + const regularPlugins = generateOperationPlugins( + operation, + pathPlugins, + pathValidatorPlugin || globalPlugins.requestValidatorPlugin, // Path plugin takes precedence over global + ); const plugins = [...regularPlugins, ...securityPlugins]; // Add plugins if there are any diff --git a/packages/openapi-2-kong/src/kubernetes/__tests__/plugins.test.js b/packages/openapi-2-kong/src/kubernetes/__tests__/plugins.test.js index a82e11cb16..96cc62e8d5 100644 --- a/packages/openapi-2-kong/src/kubernetes/__tests__/plugins.test.js +++ b/packages/openapi-2-kong/src/kubernetes/__tests__/plugins.test.js @@ -10,7 +10,6 @@ import { getServerPlugins, normalizeOperationPlugins, normalizePathPlugins, - distinctByProperty, prioritizePlugins, } from '../plugins'; import { HttpMethod } from '../../common'; @@ -702,40 +701,4 @@ describe('plugins', () => { expect(result).toEqual([oo, pp, ss, gg]); }); }); - - describe('distinctByProperty()', () => { - it('returns empty array if no truthy items', () => { - expect(distinctByProperty([], i => i)).toHaveLength(0); - expect(distinctByProperty([undefined], i => i)).toHaveLength(0); - expect(distinctByProperty([null, undefined, ''], i => i)).toHaveLength(0); - }); - - it('should remove objects with the same property selector - removes 2/4', () => { - const item1 = { name: 'a', value: 'first' }; - const item2 = { name: 'a', value: 'second' }; - const item3 = { name: 'b', value: 'third' }; - const item4 = { name: 'b', value: 'fourth' }; - const items = [item1, item2, item3, item4]; - - // distinct by the name property - const filtered = distinctByProperty(items, i => i.name); - - // Should remove item2 and item4 - expect(filtered).toEqual([item1, item3]); - }); - - it('should remove objects with the same property selector - removes none', () => { - const item1 = { name: 'a', value: 'first' }; - const item2 = { name: 'a', value: 'second' }; - const item3 = { name: 'b', value: 'third' }; - const item4 = { name: 'b', value: 'fourth' }; - const items = [item1, item2, item3, item4]; - - // distinct by the value property - const filtered = distinctByProperty(items, i => i.value); - - // Should remove no items - expect(filtered).toEqual(items); - }); - }); }); diff --git a/packages/openapi-2-kong/src/kubernetes/plugins.js b/packages/openapi-2-kong/src/kubernetes/plugins.js index 310caaf67d..89cd96a712 100644 --- a/packages/openapi-2-kong/src/kubernetes/plugins.js +++ b/packages/openapi-2-kong/src/kubernetes/plugins.js @@ -1,6 +1,7 @@ // @flow import { + distinctByProperty, getPaths, getPluginNameFromKey, getServers, @@ -213,19 +214,3 @@ export function prioritizePlugins( // Select first of each type of plugin return distinctByProperty(plugins, p => p.plugin); } - -export function distinctByProperty(arr: Array, propertySelector: (item: T) => any): Array { - const result: Array = []; - const set = new Set(); - - for (const item of arr.filter(i => i)) { - const selector = propertySelector(item); - if (set.has(selector)) { - continue; - } - - set.add(selector); - result.push(item); - } - return result; -}