diff --git a/packages/backend-openapi-utils/src/schema/errors.ts b/packages/backend-openapi-utils/src/schema/errors.ts index 44de054c53..e1950bcce9 100644 --- a/packages/backend-openapi-utils/src/schema/errors.ts +++ b/packages/backend-openapi-utils/src/schema/errors.ts @@ -15,6 +15,8 @@ */ import { Operation } from './types'; +import { ErrorObject } from 'ajv'; +import { humanifyAjvError } from './utils'; export class OperationError extends Error { constructor(operation: Operation, message: string) { @@ -35,3 +37,31 @@ export class OperationResponseError extends Error { ); } } + +export class OperationParsingError extends OperationError { + constructor(operation: Operation, type: string, errors: ErrorObject[]) { + super( + operation, + `${type} validation failed.\n - ${errors + .map(humanifyAjvError) + .join('\n - ')}`, + ); + } +} + +export class OperationParsingResponseError extends OperationResponseError { + constructor( + operation: Operation, + response: Response, + type: string, + errors: ErrorObject[], + ) { + super( + operation, + response, + `${type} validation failed.\n - ${errors + .map(humanifyAjvError) + .join('\n - ')}`, + ); + } +} diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts index 717256e266..6ae2f4f8a0 100644 --- a/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts @@ -91,11 +91,11 @@ describe('query parameters', () => { const request = { url: 'http://localhost:8080/api/search?param=hello', } as Request; - await expect( - parser.parse(request), - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"["GET /api/search"] Query parameter validation failed"`, - ); + await expect(parser.parse(request)).rejects + .toThrowErrorMatchingInlineSnapshot(` + "["GET /api/search"] Query parameter validation failed. + - Value should be of type number" + `); }); }); }); @@ -514,11 +514,11 @@ describe('path parameters', () => { const request = { url: 'http://localhost:8080/api/item/hello', } as Request; - await expect( - parser.parse(request), - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"["GET /api/item/{id}"] Path parameter validation failed"`, - ); + await expect(parser.parse(request)).rejects + .toThrowErrorMatchingInlineSnapshot(` + "["GET /api/item/{id}"] Path parameter validation failed. + - Value should be of type number" + `); }); }); }); diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.ts index 65e6439b7e..19d49ef6ef 100644 --- a/packages/backend-openapi-utils/src/schema/parameter-validation.ts +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.ts @@ -23,7 +23,7 @@ import { ValidatorParams, } from './types'; import Ajv from 'ajv'; -import { OperationError } from './errors'; +import { OperationError, OperationParsingError } from './errors'; import { mockttpToFetchRequest } from './utils'; type ReferencelessSchemaObject = SchemaObject & { $ref?: never }; @@ -150,9 +150,10 @@ export class QueryParameterParser const validate = this.ajv.compile(parameter.schema); const valid = validate(param); if (!valid) { - throw new OperationError( + throw new OperationParsingError( this.operation, - 'Query parameter validation failed', + 'Query parameter', + validate.errors!, ); } queryParameters[name] = param; @@ -339,9 +340,10 @@ export class HeaderParameterParser const valid = validate(header); if (!valid) { - throw new OperationError( + throw new OperationParsingError( this.operation, - 'Header parameter validation failed', + 'Header parameter', + validate.errors!, ); } headerParameters[name] = header; @@ -384,9 +386,10 @@ export class PathParameterParser const valid = validate(param); if (!valid) { - throw new OperationError( + throw new OperationParsingError( this.operation, - 'Path parameter validation failed', + 'Path parameter', + validate.errors!, ); } pathParameters[name] = param; diff --git a/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts b/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts index a4452763fc..df8b33402c 100644 --- a/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts @@ -61,11 +61,11 @@ describe('request body', () => { const requestBody = { query: 1, }; - await expect( - parser.parse(toRequest(requestBody)), - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"["POST /api/search"] Request body validation failed."`, - ); + await expect(parser.parse(toRequest(requestBody))).rejects + .toThrowErrorMatchingInlineSnapshot(` + "["POST /api/search"] Request body validation failed. + - "/query" should be of type string" + `); }); it('should throw error if request body is required but missing', async () => { diff --git a/packages/backend-openapi-utils/src/schema/request-body-validation.ts b/packages/backend-openapi-utils/src/schema/request-body-validation.ts index 22873c8100..f7681f01c2 100644 --- a/packages/backend-openapi-utils/src/schema/request-body-validation.ts +++ b/packages/backend-openapi-utils/src/schema/request-body-validation.ts @@ -17,7 +17,7 @@ import { JsonObject } from '@backstage/types'; import { Operation, ParserOptions, RequestParser } from './types'; import { ValidateFunction } from 'ajv'; -import { OperationError } from './errors'; +import { OperationError, OperationParsingError } from './errors'; import { RequestBodyObject, SchemaObject } from 'openapi3-ts'; class DisabledRequestBodyParser @@ -120,9 +120,10 @@ export class RequestBodyParser const body = (await request.json()) as JsonObject; const valid = this.validate(body); if (!valid) { - throw new OperationError( + throw new OperationParsingError( this.operation, - `Request body validation failed.`, + `Request body`, + this.validate.errors!, ); } return body; diff --git a/packages/backend-openapi-utils/src/schema/response-body-validation.test.ts b/packages/backend-openapi-utils/src/schema/response-body-validation.test.ts index 9ad47b8a19..6cd098b2e7 100644 --- a/packages/backend-openapi-utils/src/schema/response-body-validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/response-body-validation.test.ts @@ -60,11 +60,11 @@ describe('response body', () => { const responseBody = { result: 1, }; - await expect( - parser.parse(toResponse(responseBody)), - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"["GET /api/search" (200)]: Response body validation failed"`, - ); + await expect(parser.parse(toResponse(responseBody))).rejects + .toThrowErrorMatchingInlineSnapshot(` + "["GET /api/search" (200)]: Response body validation failed. + - The "result" property is not allowed" + `); }); it('should throw error if response body is required but missing', async () => { diff --git a/packages/backend-openapi-utils/src/schema/response-body-validation.ts b/packages/backend-openapi-utils/src/schema/response-body-validation.ts index 116fc888e8..1a2a475afc 100644 --- a/packages/backend-openapi-utils/src/schema/response-body-validation.ts +++ b/packages/backend-openapi-utils/src/schema/response-body-validation.ts @@ -16,7 +16,11 @@ import { JsonObject } from '@backstage/types'; import { Operation, ParserOptions, ResponseParser } from './types'; -import { OperationError, OperationResponseError } from './errors'; +import { + OperationError, + OperationParsingResponseError, + OperationResponseError, +} from './errors'; import Ajv from 'ajv'; import { OperationObject, ResponseObject } from 'openapi3-ts'; @@ -58,6 +62,7 @@ export class ResponseBodyParser const responseSchemas = operation.schema.responses; for (const [statusCode, schema] of Object.entries(responseSchemas)) { if (!schema.content) { + // Skip responses without content, eg 204 No Content. continue; } else if (!schema.content['application/json']) { throw new OperationError( @@ -107,6 +112,7 @@ export class ResponseBodyParser ); } const schema = responseSchema.content!['application/json'].schema; + // This is a bit of type laziness. Ideally, this would be a type-narrowing function, but I wasn't able to get the types to work. if (!schema) { throw new OperationError(this.operation, 'No schema found in response'); } @@ -133,10 +139,11 @@ export class ResponseBodyParser const jsonBody = (await response.json()) as JsonObject; const valid = validate(jsonBody); if (!valid) { - throw new OperationResponseError( + throw new OperationParsingResponseError( this.operation, response, - 'Response body validation failed', + 'Response body', + validate.errors!, ); } return jsonBody; @@ -144,9 +151,8 @@ export class ResponseBodyParser private findResponseSchema( operationSchema: OperationObject, - response: Response, + { status }: Response, ): ResponseObject | undefined { - const { status } = response; return ( operationSchema.responses?.[status] ?? operationSchema.responses?.default ); diff --git a/packages/backend-openapi-utils/src/schema/utils.ts b/packages/backend-openapi-utils/src/schema/utils.ts index cc00d00095..7f9385a404 100644 --- a/packages/backend-openapi-utils/src/schema/utils.ts +++ b/packages/backend-openapi-utils/src/schema/utils.ts @@ -14,6 +14,7 @@ * limitations under the License. */ import { CompletedRequest, CompletedResponse } from 'mockttp'; +import { ErrorObject } from 'ajv'; export function mockttpToFetchRequest(request: CompletedRequest) { const headers = new Headers(request.rawHeaders); @@ -34,3 +35,19 @@ export function mockttpToFetchResponse(response: CompletedResponse) { text: () => response.body?.getText(), } as Response; } + +export function humanifyAjvError(error: ErrorObject) { + switch (error.keyword) { + case 'required': + return `The ${error.params.missingProperty} property is required`; + case 'type': + console.log(error); + return `${ + error.instancePath ? `"${error.instancePath}"` : 'Value' + } should be of type ${error.params.type}`; + case 'additionalProperties': + return `The "${error.params.additionalProperty}" property is not allowed`; + default: + return error.message; + } +} diff --git a/packages/backend-openapi-utils/src/schema/validation.test.ts b/packages/backend-openapi-utils/src/schema/validation.test.ts index 3f172c16fe..3b73250cd9 100644 --- a/packages/backend-openapi-utils/src/schema/validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/validation.test.ts @@ -164,11 +164,11 @@ describe('OpenApiProxyValidator', () => { statusCode: 200, }); - await expect( - async () => await validator.validate(request, response), - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"["GET /api/search"] Query parameter validation failed"`, - ); + await expect(async () => await validator.validate(request, response)) + .rejects.toThrowErrorMatchingInlineSnapshot(` + "["GET /api/search"] Query parameter validation failed. + - Value should be of type number" + `); }); it('accepts valid parameter', async () => { diff --git a/packages/backend-openapi-utils/src/schema/validation.ts b/packages/backend-openapi-utils/src/schema/validation.ts index fda09b2297..d9d80ddcf5 100644 --- a/packages/backend-openapi-utils/src/schema/validation.ts +++ b/packages/backend-openapi-utils/src/schema/validation.ts @@ -24,7 +24,7 @@ import { RequestBodyParser } from './request-body-validation'; import { mockttpToFetchRequest, mockttpToFetchResponse } from './utils'; import { ResponseBodyParser } from './response-body-validation'; -const ajv = new Ajv({ allErrors: true }); // options can be passed, e.g. {allErrors: true} +const ajv = new Ajv({ allErrors: true }); class RequestBodyValidator implements Validator { schema: OpenAPIObject; @@ -34,6 +34,7 @@ class RequestBodyValidator implements Validator { async validate({ pair, operation }: ValidatorParams) { const { request } = pair; + // NOTE: There may be a worthwhile optimization here to cache these results to avoid re-parsing the schema for every request. As is, I don't think this is a big deal. const parser = RequestBodyParser.fromOperation(operation, { ajv }); const fetchRequest = mockttpToFetchRequest(request); await parser.parse(fetchRequest); @@ -48,12 +49,19 @@ class ResponseBodyValidator implements Validator { async validate({ pair, operation }: ValidatorParams) { const { response } = pair; + // NOTE: There may be a worthwhile optimization here to cache these results to avoid re-parsing the schema for every request. As is, I don't think this is a big deal. const parser = ResponseBodyParser.fromOperation(operation, { ajv }); const fetchResponse = mockttpToFetchResponse(response); await parser.parse(fetchResponse); } } +/** + * Find an operation in an OpenAPI schema that matches a request. This is done by comparing the request URL to the paths in the schema. + * @param openApiSchema - The OpenAPI schema to search for the operation in. + * @param request - The request to find the operation for. + * @returns A tuple of the path and the operation object that matches the request. + */ export function findOperationByRequest( openApiSchema: OpenAPIObject, request: CompletedRequest, @@ -72,6 +80,7 @@ export function findOperationByRequest( if (pathParts[i] === parts[i]) { continue; } + // If the path part is a parameter, we can count it as a match. eg /api/{id} will match /api/1 if (pathParts[i].startsWith('{') && pathParts[i].endsWith('}')) { continue; }