chore(xo-server/api): use Ajv instead of schema-inspector

- standard JSON schema
- faster
- maintained

New implementation also pre-compile schemas which means that params validation for each call is faster and incorrect schemas are detected at startup.
This commit is contained in:
Julien Fontanet 2022-08-11 15:50:04 +02:00
parent 82df6089c3
commit d7f29e7363
9 changed files with 130 additions and 59 deletions

View File

@ -113,7 +113,6 @@
"pw": "^0.0.4", "pw": "^0.0.4",
"readable-stream": "^4.1.0", "readable-stream": "^4.1.0",
"redis": "^4.2.0", "redis": "^4.2.0",
"schema-inspector": "^2.0.1",
"semver": "^7.3.2", "semver": "^7.3.2",
"serve-static": "^1.13.1", "serve-static": "^1.13.1",
"set-cookie-parser": "^2.3.5", "set-cookie-parser": "^2.3.5",

View File

@ -14,7 +14,7 @@ const SCHEMA_SETTINGS = {
properties: { properties: {
concurrency: { concurrency: {
type: 'number', type: 'number',
gt: 0, minimum: 0,
optional: true, optional: true,
}, },
}, },
@ -210,7 +210,7 @@ getLogs.params = {
after: { type: ['number', 'string'], optional: true }, after: { type: ['number', 'string'], optional: true },
before: { type: ['number', 'string'], optional: true }, before: { type: ['number', 'string'], optional: true },
limit: { type: 'number', optional: true }, limit: { type: 'number', optional: true },
'*': { type: 'any' }, '*': {},
} }
// ----------------------------------------------------------------------------- // -----------------------------------------------------------------------------
@ -364,7 +364,7 @@ fetchFiles.params = {
}, },
paths: { paths: {
items: { type: 'string' }, items: { type: 'string' },
minLength: 1, minItems: 1,
type: 'array', type: 'array',
}, },
remote: { remote: {

View File

@ -123,7 +123,7 @@ export async function exportContent({ vdi, format = VHD }) {
exportContent.description = 'export the content of a VDI' exportContent.description = 'export the content of a VDI'
exportContent.params = { exportContent.params = {
id: { type: 'string' }, id: { type: 'string' },
format: { eq: [VMDK, VHD], optional: true }, format: { enum: [VMDK, VHD], optional: true },
} }
exportContent.resolve = { exportContent.resolve = {
vdi: ['id', ['VDI', 'VDI-snapshot'], 'view'], vdi: ['id', ['VDI', 'VDI-snapshot'], 'view'],

View File

@ -171,7 +171,7 @@ restartAgent.resolve = {
} }
// TODO: remove deprecated alias // TODO: remove deprecated alias
export { restartAgent as restart_agent } // eslint-disable-line camelcase export const restart_agent = 'restartAgent' // eslint-disable-line camelcase
// ------------------------------------------------------------------- // -------------------------------------------------------------------

View File

@ -52,11 +52,7 @@ createBonded.params = {
}, },
}, },
mtu: { type: ['integer', 'string'], optional: true }, mtu: { type: ['integer', 'string'], optional: true },
// RegExp since schema-inspector does not provide a param check based on an enumeration bondMode: { enum: getBondModes() },
bondMode: {
type: 'string',
pattern: new RegExp(`^(${getBondModes().join('|')})$`),
},
} }
createBonded.resolve = { createBonded.resolve = {

View File

@ -303,9 +303,7 @@ create.params = {
existingDisks: { existingDisks: {
optional: true, optional: true,
type: 'object', type: 'object',
additionalProperties: {
// Do not for a type object.
items: {
type: 'object', type: 'object',
properties: { properties: {
size: { size: {
@ -325,7 +323,7 @@ create.params = {
copyHostBiosStrings: { type: 'boolean', optional: true }, copyHostBiosStrings: { type: 'boolean', optional: true },
// other params are passed to `editVm` // other params are passed to `editVm`
'*': { type: 'any' }, '*': {},
} }
create.resolve = { create.resolve = {
@ -594,8 +592,7 @@ set.params = {
high_availability: { high_availability: {
optional: true, optional: true,
pattern: new RegExp(`^(${getHaValues().join('|')})$`), enum: getHaValues(),
type: 'string',
}, },
// Number of virtual CPUs to allocate. // Number of virtual CPUs to allocate.
@ -803,7 +800,7 @@ convertToTemplate.resolve = {
} }
// TODO: remove when no longer used. // TODO: remove when no longer used.
export { convertToTemplate as convert } export const convert = 'convertToTemplate'
// ------------------------------------------------------------------- // -------------------------------------------------------------------

View File

@ -201,8 +201,7 @@ getVolumeInfo.params = {
type: 'string', type: 'string',
}, },
infoType: { infoType: {
type: 'string', enum: Object.keys(VOLUME_INFO_TYPES),
eq: Object.keys(VOLUME_INFO_TYPES),
}, },
} }
getVolumeInfo.resolve = { getVolumeInfo.resolve = {
@ -230,7 +229,7 @@ profileStatus.params = {
type: 'string', type: 'string',
}, },
changeStatus: { changeStatus: {
type: 'bool', type: 'boolean',
optional: true, optional: true,
}, },
} }

View File

@ -1,10 +1,10 @@
import emitAsync from '@xen-orchestra/emit-async' import emitAsync from '@xen-orchestra/emit-async'
import { createLogger } from '@xen-orchestra/log' import { createLogger } from '@xen-orchestra/log'
import Ajv from 'ajv'
import forEach from 'lodash/forEach.js' import forEach from 'lodash/forEach.js'
import kindOf from 'kindof' import kindOf from 'kindof'
import ms from 'ms' import ms from 'ms'
import schemaInspector from 'schema-inspector'
import { AsyncLocalStorage } from 'async_hooks' import { AsyncLocalStorage } from 'async_hooks'
import { format, JsonRpcError, MethodNotFound } from 'json-rpc-peer' import { format, JsonRpcError, MethodNotFound } from 'json-rpc-peer'
@ -55,23 +55,14 @@ const XAPI_ERROR_TO_XO_ERROR = {
const hasPermission = (actual, expected) => PERMISSIONS[actual] >= PERMISSIONS[expected] const hasPermission = (actual, expected) => PERMISSIONS[actual] >= PERMISSIONS[expected]
const ajv = new Ajv({ allErrors: true, allowUnionTypes: true })
function checkParams(method, params) { function checkParams(method, params) {
const schema = method.params const { validate } = method
if (!schema) { if (validate !== undefined) {
return if (!validate(params)) {
} throw errors.invalidParameters(validate.errors)
}
const result = schemaInspector.validate(
{
properties: schema,
strict: true,
type: 'object',
},
params
)
if (!result.valid) {
throw errors.invalidParameters(result.error)
} }
} }
@ -97,6 +88,73 @@ function checkPermission(method) {
} }
} }
function adaptJsonSchema(schema) {
if (schema.enum !== undefined) {
return schema
}
const is = (({ type }) => {
if (typeof type === 'string') {
return t => t === type
}
const types = new Set(type)
return t => types.has(t)
})(schema)
if (is('array')) {
const { items } = schema
if (items !== undefined) {
if (Array.isArray(items)) {
for (let i = 0, n = items.length; i < n; ++i) {
items[i] = adaptJsonSchema(items[i])
}
} else {
schema.items = adaptJsonSchema(items)
}
}
}
if (is('object')) {
const { properties = {} } = schema
let keys = Object.keys(properties)
for (const key of keys) {
properties[key] = adaptJsonSchema(properties[key])
}
const { additionalProperties } = schema
if (schema === undefined) {
const wildCard = properties['*']
if (wildCard === undefined) {
// we want additional properties to be disabled by default
schema.additionalProperties = false
} else {
delete properties['*']
keys = Object.keys(properties)
schema.additionalProperties = wildCard
}
} else if (typeof additionalProperties === 'object') {
schema.additionalProperties = adaptJsonSchema(additionalProperties)
}
// we want properties to be required by default unless explicitly marked so
// we use property `optional` instead of object `required`
if (schema.required === undefined) {
const required = keys.filter(key => {
const value = properties[key]
const required = !value.optional
delete value.optional
return required
})
if (required.length !== 0) {
schema.required = required
}
}
}
return schema
}
async function resolveParams(method, params) { async function resolveParams(method, params) {
const resolve = method.resolve const resolve = method.resolve
if (!resolve) { if (!resolve) {
@ -177,13 +235,40 @@ export default class Api {
throw new Error(`API method ${name} already exists`) throw new Error(`API method ${name} already exists`)
} }
Object.keys(method).forEach(prop => { // alias
if (!(prop in ALLOWED_METHOD_PROPS)) { if (typeof method === 'string') {
throw new Error(`invalid prop ${prop} for API method ${name}`) Object.defineProperty(methods, name, {
} configurable: true,
}) enumerable: true,
get() {
return methods[method]
},
})
} else {
Object.keys(method).forEach(prop => {
if (!(prop in ALLOWED_METHOD_PROPS)) {
throw new Error(`invalid prop ${prop} for API method ${name}`)
}
})
methods[name] = method const { params } = method
if (params !== undefined) {
let schema = { type: 'object', properties: params }
try {
schema = adaptJsonSchema(schema)
method.validate = ajv.compile(schema)
} catch (error) {
log.warn('failed to compile method params schema', {
error,
method: name,
schema,
})
throw error
}
}
methods[name] = method
}
let remove = () => { let remove = () => {
delete methods[name] delete methods[name]
@ -199,15 +284,17 @@ export default class Api {
const addMethod = (method, name) => { const addMethod = (method, name) => {
name = base + name name = base + name
if (typeof method === 'function') { const type = typeof method
if (type === 'string') {
removes.push(this.addApiMethod(name, base + method))
} else if (type === 'function') {
removes.push(this.addApiMethod(name, method)) removes.push(this.addApiMethod(name, method))
return } else {
const oldBase = base
base = name + '.'
forEach(method, addMethod)
base = oldBase
} }
const oldBase = base
base = name + '.'
forEach(method, addMethod)
base = oldBase
} }
try { try {

View File

@ -4095,7 +4095,7 @@ async-settle@^1.0.0:
dependencies: dependencies:
async-done "^1.2.2" async-done "^1.2.2"
async@^2.6.2, async@~2.6.3: async@^2.6.2:
version "2.6.4" version "2.6.4"
resolved "https://registry.npmjs.org/async/-/async-2.6.4.tgz" resolved "https://registry.npmjs.org/async/-/async-2.6.4.tgz"
integrity sha512-mzo5dfJYwAn29PeiJ0zvwTo04zj8HDJj0Mn8TD7sno7q12prdbnasKJHhkm2c1LgrhlJ0teaea8860oxi51mGA== integrity sha512-mzo5dfJYwAn29PeiJ0zvwTo04zj8HDJj0Mn8TD7sno7q12prdbnasKJHhkm2c1LgrhlJ0teaea8860oxi51mGA==
@ -16392,13 +16392,6 @@ scheduler@^0.20.2:
loose-envify "^1.1.0" loose-envify "^1.1.0"
object-assign "^4.1.1" object-assign "^4.1.1"
schema-inspector@^2.0.1:
version "2.0.1"
resolved "https://registry.npmjs.org/schema-inspector/-/schema-inspector-2.0.1.tgz"
integrity sha512-lqR4tOVfoqf9Z8cgX/zvXuWPnTWCqrc4WSgeSPDDc1bWbMABaqdSTY98xj7iRKHOIRtKjc4M8EWCgUu5ASlHkg==
dependencies:
async "~2.6.3"
schema-utils@^1.0.0: schema-utils@^1.0.0:
version "1.0.0" version "1.0.0"
resolved "https://registry.npmjs.org/schema-utils/-/schema-utils-1.0.0.tgz" resolved "https://registry.npmjs.org/schema-utils/-/schema-utils-1.0.0.tgz"