diff --git a/.changeset/gorgeous-months-doubt.md b/.changeset/gorgeous-months-doubt.md new file mode 100644 index 0000000000..d440e28b72 --- /dev/null +++ b/.changeset/gorgeous-months-doubt.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-openapi-utils': patch +--- + +Add a new `createRouter` method for generating an `express` router that validates against your spec. Also fixes a bug with the query parameters type resolution. diff --git a/.changeset/serious-bats-repair.md b/.changeset/serious-bats-repair.md new file mode 100644 index 0000000000..d93b49bda0 --- /dev/null +++ b/.changeset/serious-bats-repair.md @@ -0,0 +1,5 @@ +--- +'@backstage/repo-tools': patch +--- + +Update `schema openapi generate` command to now create a default router that can be imported and used directly. diff --git a/.changeset/smart-pandas-applaud.md b/.changeset/smart-pandas-applaud.md new file mode 100644 index 0000000000..5f2c297df5 --- /dev/null +++ b/.changeset/smart-pandas-applaud.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-todo-backend': minor +'@backstage/plugin-catalog-backend': minor +'@backstage/plugin-search-backend': minor +--- + +Now performs request validation based on OpenAPI schema through `@backstage/backend-openapi-utils`. Error responses for invalid input, like `"a"` instead of a number, may have changed. diff --git a/packages/backend-openapi-utils/README.md b/packages/backend-openapi-utils/README.md index ba286f94e8..1efab0e85b 100644 --- a/packages/backend-openapi-utils/README.md +++ b/packages/backend-openapi-utils/README.md @@ -13,16 +13,56 @@ This package is meant to provide a typed Express router for an OpenAPI spec. Bas 2. In your plugin's `src/service/createRouter.ts`, ```ts -import { ApiRouter } from `@backstage/backend-openapi-utils`; -import spec from '../schema/openapi.generated'; +import { createOpenApiRouter } from '../schema/openapi.generated'; // ... export function createRouter() { - const router = Router() as ApiRouter; - // ... - return router; + const router = createOpenApiRouter(); + // add routes to router, it's just an express router. + return router; } ``` +3. Add `@backstage/backend-openapi-utils` to your `package.json`'s `dependencies`. + +Why do I need to add this to `dependencies`? If you check the `src/schema/openapi.generated.ts` file, we're creating a router stub for you with the `@backstage/backend-openapi-utils` package. + +### Customization + +If the out of the box `router` doesn't work, you can do the following, + +```ts +import { createOpenApiRouter } from '../schema/openapi.generated'; +// ... +export function createRouter() { + // See https://github.com/cdimascio/express-openapi-validator/wiki/Documentation for available options. + const router = createOpenApiRouter(validatorOptions); + // add routes to router, it's just an express router. + return router; +} +``` + +If you need even more control -- say for example you wanted to update the spec at runtime -- you can do the following, + +```ts +import { spec } from '../schema/openapi.generated'; +import { createValidatedOpenApiRouter } from '@backstage/backend-openapi-utils'; +// ... +export function createRouter() { + // Update the spec here. + const newSpec = { ...spec, myproperty123: 123 }; + + // See https://github.com/cdimascio/express-openapi-validator/wiki/Documentation for available options. + const router = createValidatedOpenApiRouter( + newSpec, + validatorOptions, + ); + // add routes to router, it's just an express router. + return router; +} +``` + +## INTERNAL + ### Limitations 1. `as const` makes all fields `readonly` @@ -40,6 +80,10 @@ Router() as ApiRouter> ## Future Work -### Runtime validation +### Response Validation -Using a package like [`express-openapi-validator`](https://www.npmjs.com/package/express-openapi-validator), would allow us to remove [validation of request bodies with `AJV`](https://github.com/backstage/backstage/blob/e0506af8fc54074a160fb91c83d6cae8172d3bb3/plugins/catalog-backend/src/service/util.ts#L58). However, `AJV` currently doesn't have support for OpenAPI 3.1 and `express-openapi-validator` enforces full URL matching for paths, meaning it cannot be mounted at the router level. +This is a murky ground and something that will take a while to gain adoption. For now, keep responses in the spec and at the type level, but will need to work to drive adoption of response validation. + +### Common Error Format + +With the new `createRouter` method, we can start to control error response formats for input and coercion errors. diff --git a/packages/backend-openapi-utils/api-report.md b/packages/backend-openapi-utils/api-report.md index a4db8ac7bd..08ffa23a30 100644 --- a/packages/backend-openapi-utils/api-report.md +++ b/packages/backend-openapi-utils/api-report.md @@ -7,10 +7,12 @@ import type { ContentObject } from 'openapi3-ts'; import type core from 'express-serve-static-core'; import { FromSchema } from 'json-schema-to-ts'; import { JSONSchema7 } from 'json-schema-to-ts'; +import { middleware } from 'express-openapi-validator'; import type { OpenAPIObject } from 'openapi3-ts'; import type { ParameterObject } from 'openapi3-ts'; import type { ReferenceObject } from 'openapi3-ts'; import type { RequestBodyObject } from 'openapi3-ts'; +import { RequestHandler } from 'express'; import type { ResponseObject } from 'openapi3-ts'; import { Router } from 'express'; import type { SchemaObject } from 'openapi3-ts'; @@ -79,6 +81,15 @@ type CookieSchema< Method extends DocPathMethod, > = ParametersSchema, Method, ImmutableCookieObject>; +// @public +export function createValidatedOpenApiRouter( + spec: T, + options?: { + validatorOptions?: Partial['0']>; + middleware?: RequestHandler[]; + }, +): ApiRouter; + // @public (undocumented) type DiscriminateUnion = Extract< T, @@ -117,15 +128,18 @@ type DocParameters< Doc extends RequiredDoc, Path extends Extract, Method extends keyof Doc['paths'][Path], -> = DocOperation['parameters'] extends ReadonlyArray - ? { - [Index in keyof DocOperation< - Doc, - Path, - Method - >['parameters']]: DocParameter; - } - : never; +> = { + [Index in keyof DocOperation< + Doc, + Path, + Method + >['parameters'] as FromNumberStringToNumber]: DocParameter< + Doc, + Path, + Method, + Index + >; +}; // @public type DocPath< @@ -204,6 +218,10 @@ interface DocRequestMatcher< // @public (undocumented) type Filter = T extends U ? T : never; +// @public +type FromNumberStringToNumber = + NumberString extends `${infer R extends number}` ? R : never; + // @public (undocumented) type FullMap< T extends { @@ -332,6 +350,7 @@ declare namespace internal { ImmutablePathObject, ImmutableSchemaObject, DocParameter, + FromNumberStringToNumber, DocParameters, ParameterSchema, MapToSchema, @@ -425,17 +444,15 @@ type ParametersSchema< Path extends Extract, Method extends keyof Doc['paths'][Path], FilterType extends ImmutableParameterObject, -> = number extends keyof DocParameters - ? MapToSchema< - Doc, - FullMap< - MapDiscriminatedUnion< - Filter[number], FilterType>, - 'name' - > - > +> = MapToSchema< + Doc, + FullMap< + MapDiscriminatedUnion< + Filter>, FilterType>, + 'name' > - : never; + > +>; // @public (undocumented) type PathDoc = Pick; diff --git a/packages/backend-openapi-utils/package.json b/packages/backend-openapi-utils/package.json index 89a4621a49..a8d8931506 100644 --- a/packages/backend-openapi-utils/package.json +++ b/packages/backend-openapi-utils/package.json @@ -24,17 +24,21 @@ "postpack": "backstage-cli package postpack" }, "devDependencies": { - "@backstage/cli": "workspace:^" + "@backstage/cli": "workspace:^", + "supertest": "^6.1.3" }, "files": [ "dist" ], "dependencies": { + "@backstage/errors": "workspace:^", "@types/express": "^4.17.6", "@types/express-serve-static-core": "^4.17.5", "express": "^4.17.1", + "express-openapi-validator": "^5.0.4", "express-promise-router": "^4.1.0", "json-schema-to-ts": "^2.6.2", + "lodash": "^4.17.21", "openapi3-ts": "^3.1.2" } } diff --git a/packages/backend-openapi-utils/src/___fixtures__/single-path.ts b/packages/backend-openapi-utils/src/___fixtures__/single-path.ts new file mode 100644 index 0000000000..d050c5a88b --- /dev/null +++ b/packages/backend-openapi-utils/src/___fixtures__/single-path.ts @@ -0,0 +1,120 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export default { + openapi: '3.0.2', + info: { + title: 'Swagger Petstore - OpenAPI 3.0', + description: + "This is a sample Pet Store Server based on the OpenAPI 3.0 specification. You can find out more about\nSwagger at [http://swagger.io](http://swagger.io). In the third iteration of the pet store, we've switched to the design first approach!\nYou can now help us improve the API whether it's by making changes to the definition itself or to the code.\nThat way, with time, we can improve the API in general, and expose some of the new features in OAS3.\n\nSome useful links:\n- [The Pet Store repository](https://github.com/swagger-api/swagger-petstore)\n- [The source API definition for the Pet Store](https://github.com/swagger-api/swagger-petstore/blob/master/src/main/resources/openapi.yaml)", + termsOfService: 'http://swagger.io/terms/', + contact: { + email: 'apiteam@swagger.io', + }, + license: { + name: 'Apache 2.0', + url: 'http://www.apache.org/licenses/LICENSE-2.0.html', + }, + version: '1.0.17', + }, + externalDocs: { + description: 'Find out more about Swagger', + url: 'http://swagger.io', + }, + servers: [ + { + url: '/', + }, + ], + paths: { + '/pet/{petId}': { + get: { + summary: 'Find pet by ID', + description: 'Returns a single pet', + operationId: 'getPetById', + parameters: [ + { + name: 'petId', + in: 'path', + description: 'ID of pet to return', + required: true, + schema: { + type: 'integer', + format: 'int64', + }, + }, + ], + responses: { + '200': { + description: 'successful operation', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Pet', + }, + }, + }, + }, + '400': { + description: 'Invalid ID supplied', + }, + '404': { + description: 'Pet not found', + }, + }, + }, + }, + }, + components: { + schemas: { + Pet: { + required: ['name', 'photoUrls'], + type: 'object', + properties: { + id: { + type: 'integer', + format: 'int64', + example: 10, + }, + name: { + type: 'string', + example: 'doggie', + }, + photoUrls: { + type: 'array', + xml: { + wrapped: true, + }, + items: { + type: 'string', + xml: { + name: 'photoUrl', + }, + }, + }, + status: { + type: 'string', + description: 'pet status in the store', + enum: ['available', 'pending', 'sold'], + }, + }, + xml: { + name: 'pet', + }, + }, + }, + }, +} as const; diff --git a/packages/backend-openapi-utils/src/index.ts b/packages/backend-openapi-utils/src/index.ts index f8fdcff16b..3acdcbf6f0 100644 --- a/packages/backend-openapi-utils/src/index.ts +++ b/packages/backend-openapi-utils/src/index.ts @@ -23,3 +23,4 @@ import * as internal from './types'; export { internal }; export type { ApiRouter } from './router'; +export { createValidatedOpenApiRouter } from './stub'; diff --git a/packages/backend-openapi-utils/src/stub.test.ts b/packages/backend-openapi-utils/src/stub.test.ts new file mode 100644 index 0000000000..678445c818 --- /dev/null +++ b/packages/backend-openapi-utils/src/stub.test.ts @@ -0,0 +1,71 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { createValidatedOpenApiRouter } from './stub'; +import express from 'express'; +import request from 'supertest'; +import singlePathSpec from './___fixtures__/single-path'; + +describe('createRouter', () => { + it('does NOT override originalUrl and basePath after execution', async () => { + expect.assertions(2); + const router = createValidatedOpenApiRouter(singlePathSpec); + router.get('/pet/:petId', (req, res) => { + expect(req.baseUrl).toBe('/pet-store'); + expect(req.originalUrl).toBe(`/pet-store/pet/${req.params.petId}`); + res.send(''); + }); + + const appRouter = express(); + appRouter.use('/pet-store', router); + + await request(appRouter).get('/pet-store/pet/1'); + }); + + it('handles nested routes correctly (by treating plugin specs as full paths)', async () => { + expect.assertions(1); + const router = createValidatedOpenApiRouter(singlePathSpec); + const routerGetFn = jest.fn(); + router.get('/pet/:petId', (_, res) => { + routerGetFn(); + res.send(''); + }); + + const apiRouter = express.Router(); + apiRouter.use('/pet-store', router); + const appRouter = express(); + appRouter.use('/api', apiRouter); + + await request(appRouter).get('/api/pet-store/pet/1'); + expect(routerGetFn).toHaveBeenCalledTimes(1); + }); + + it('handles coercing parameters correctly', async () => { + expect.assertions(1); + const router = createValidatedOpenApiRouter(singlePathSpec); + router.get('/pet/:petId', (req, res) => { + expect(typeof req.params.petId).toBe('integer'); + res.send(''); + }); + + const apiRouter = express.Router(); + apiRouter.use('/pet-store', router); + const appRouter = express(); + appRouter.use('/api', apiRouter); + + await request(appRouter).get('/api/pet-store/pet/1'); + }); +}); diff --git a/packages/backend-openapi-utils/src/stub.ts b/packages/backend-openapi-utils/src/stub.ts new file mode 100644 index 0000000000..6ec0e1db3c --- /dev/null +++ b/packages/backend-openapi-utils/src/stub.ts @@ -0,0 +1,120 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import PromiseRouter from 'express-promise-router'; +import { ApiRouter } from './router'; +import { RequiredDoc } from './types'; +import { + ErrorRequestHandler, + RequestHandler, + NextFunction, + Request, + Response, + json, +} from 'express'; +import { InputError } from '@backstage/errors'; +import { middleware as OpenApiValidator } from 'express-openapi-validator'; + +type PropertyOverrideRequest = Request & { + [key: symbol]: string; +}; + +const baseUrlSymbol = Symbol(); +const originalUrlSymbol = Symbol(); + +function validatorErrorTransformer(): ErrorRequestHandler { + return (error: Error, _: Request, _2: Response, next: NextFunction) => { + next(new InputError(error.message)); + }; +} + +export function getDefaultRouterMiddleware() { + return [json()]; +} + +/** + * Create a new OpenAPI router with some default middleware. + * @param spec - Your OpenAPI spec imported as a JSON object. + * @param validatorOptions - `openapi-express-validator` options to override the defaults. + * @returns A new express router with validation middleware. + * @public + */ +export function createValidatedOpenApiRouter( + spec: T, + options?: { + validatorOptions?: Partial['0']>; + middleware?: RequestHandler[]; + }, +) { + const router = PromiseRouter() as ApiRouter; + router.use(options?.middleware || getDefaultRouterMiddleware()); + + /** + * Middleware to setup the routing for OpenApiValidator. OpenApiValidator expects `req.originalUrl` + * and `req.baseUrl` to be the full path. We adjust them here to basically be nothing and then + * revive the old values in the last function in this method. We could instead update `req.path` + * but that might affect the routing and I'd rather not. + * + * TODO: I opened https://github.com/cdimascio/express-openapi-validator/issues/843 + * to track this on the middleware side, but there was a similar ticket, https://github.com/cdimascio/express-openapi-validator/issues/113 + * that has had minimal activity. If that changes, update this to use a new option on their side. + */ + router.use((req: Request, _, next) => { + /** + * Express typings are weird. They don't recognize PropertyOverrideRequest as a valid + * Request child and try to overload as PathParams. Just cast it here, since we know + * what we're doing. + */ + const customRequest = req as PropertyOverrideRequest; + customRequest[baseUrlSymbol] = customRequest.baseUrl; + customRequest.baseUrl = ''; + customRequest[originalUrlSymbol] = customRequest.originalUrl; + customRequest.originalUrl = customRequest.url; + next(); + }); + + // TODO: Handle errors by converting from OpenApiValidator errors to known @backstage/errors errors. + router.use( + OpenApiValidator({ + validateRequests: { + coerceTypes: false, + allowUnknownQueryParameters: false, + }, + ignoreUndocumented: true, + validateResponses: false, + ...options?.validatorOptions, + apiSpec: spec as any, + }), + ); + + /** + * Revert `req.baseUrl` and `req.originalUrl` changes. This ensures that any further usage + * of these variables will be unchanged. + */ + router.use((req: Request, _, next) => { + const customRequest = req as PropertyOverrideRequest; + customRequest.baseUrl = customRequest[baseUrlSymbol]; + customRequest.originalUrl = customRequest[originalUrlSymbol]; + delete customRequest[baseUrlSymbol]; + delete customRequest[originalUrlSymbol]; + next(); + }); + + // Any errors from the middleware get through here. + router.use(validatorErrorTransformer()); + + return router; +} diff --git a/packages/backend-openapi-utils/src/types/params.ts b/packages/backend-openapi-utils/src/types/params.ts index b737450db7..52d61a2683 100644 --- a/packages/backend-openapi-utils/src/types/params.ts +++ b/packages/backend-openapi-utils/src/types/params.ts @@ -35,6 +35,7 @@ import { PathTemplate, RequiredDoc, SchemaRef, + ValueOf, } from './common'; import { FromSchema, JSONSchema7 } from 'json-schema-to-ts'; @@ -60,6 +61,14 @@ export type DocParameter< : never : DocOperation['parameters'][Parameter]; +/** + * Helper to convert from string to number, used to index arrays and pull out just the indices in the array. + * @public + */ +export type FromNumberStringToNumber< + NumberString extends string | number | symbol, +> = NumberString extends `${infer R extends number}` ? R : never; + /** * @public */ @@ -67,15 +76,18 @@ export type DocParameters< Doc extends RequiredDoc, Path extends Extract, Method extends keyof Doc['paths'][Path], -> = DocOperation['parameters'] extends ReadonlyArray - ? { - [Index in keyof DocOperation< - Doc, - Path, - Method - >['parameters']]: DocParameter; - } - : never; +> = { + [Index in keyof DocOperation< + Doc, + Path, + Method + >['parameters'] as FromNumberStringToNumber]: DocParameter< + Doc, + Path, + Method, + Index + >; +}; /** * @public @@ -111,17 +123,15 @@ export type ParametersSchema< Path extends Extract, Method extends keyof Doc['paths'][Path], FilterType extends ImmutableParameterObject, -> = number extends keyof DocParameters - ? MapToSchema< - Doc, - FullMap< - MapDiscriminatedUnion< - Filter[number], FilterType>, - 'name' - > - > +> = MapToSchema< + Doc, + FullMap< + MapDiscriminatedUnion< + Filter>, FilterType>, + 'name' > - : never; + > +>; /** * @public diff --git a/packages/repo-tools/src/commands/openapi/generate.ts b/packages/repo-tools/src/commands/openapi/generate.ts index e6c9f79f6e..52b8e5b840 100644 --- a/packages/repo-tools/src/commands/openapi/generate.ts +++ b/packages/repo-tools/src/commands/openapi/generate.ts @@ -53,8 +53,12 @@ async function generate( // ****************************************************************** // * THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. * // ****************************************************************** - -export default ${JSON.stringify(yaml, null, 2)} as const;`, +import {createValidatedOpenApiRouter} from '@backstage/backend-openapi-utils'; +export const spec = ${JSON.stringify(yaml, null, 2)} as const; +export const createOpenApiRouter = async ( + options?: Parameters['1'], +) => createValidatedOpenApiRouter(spec, options); +`, ); await exec(`yarn backstage-cli package lint --fix ${tsPath}`); diff --git a/packages/repo-tools/src/commands/openapi/verify.ts b/packages/repo-tools/src/commands/openapi/verify.ts index 4c4f310f73..2808e72702 100644 --- a/packages/repo-tools/src/commands/openapi/verify.ts +++ b/packages/repo-tools/src/commands/openapi/verify.ts @@ -41,10 +41,10 @@ async function verify(directoryPath: string) { const schema = await import(resolvePath(join(directoryPath, TS_MODULE))); - if (!schema.default) { - throw new Error(`\`${TS_SCHEMA_PATH}\` needs to have a default export.`); + if (!schema.spec) { + throw new Error(`\`${TS_SCHEMA_PATH}\` needs to have a 'spec' export.`); } - if (!isEqual(schema.default, yaml)) { + if (!isEqual(schema.spec, yaml)) { const path = relativePath(cliPaths.targetRoot, directoryPath); throw new Error( `\`${YAML_SCHEMA_PATH}\` and \`${TS_SCHEMA_PATH}\` do not match. Please run \`yarn backstage-repo-tools schema openapi generate ${path}\` to regenerate \`${TS_SCHEMA_PATH}\`.`, diff --git a/plugins/catalog-backend/package.json b/plugins/catalog-backend/package.json index ba48492208..0e9bf51e82 100644 --- a/plugins/catalog-backend/package.json +++ b/plugins/catalog-backend/package.json @@ -46,6 +46,7 @@ }, "dependencies": { "@backstage/backend-common": "workspace:^", + "@backstage/backend-openapi-utils": "workspace:^", "@backstage/backend-plugin-api": "workspace:^", "@backstage/backend-tasks": "workspace:^", "@backstage/catalog-client": "workspace:^", @@ -87,7 +88,6 @@ "zod": "^3.21.4" }, "devDependencies": { - "@backstage/backend-openapi-utils": "workspace:^", "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", "@backstage/plugin-permission-common": "workspace:^", diff --git a/plugins/catalog-backend/src/modules/core/PlaceholderProcessor.test.ts b/plugins/catalog-backend/src/modules/core/PlaceholderProcessor.test.ts index 352ebf5df9..f5e494f514 100644 --- a/plugins/catalog-backend/src/modules/core/PlaceholderProcessor.test.ts +++ b/plugins/catalog-backend/src/modules/core/PlaceholderProcessor.test.ts @@ -518,7 +518,7 @@ describe('jsonPlaceholderResolver', () => { it('rejects invalid json', async () => { read.mockResolvedValue(Buffer.from('}', 'utf-8')); await expect(jsonPlaceholderResolver(params)).rejects.toThrow( - 'Placeholder $a failed to parse JSON data at ./file.json, SyntaxError: Unexpected token } in JSON at position 0', + 'Placeholder $a failed to parse JSON data at ./file.json', ); }); }); diff --git a/plugins/catalog-backend/src/schema/openapi.generated.ts b/plugins/catalog-backend/src/schema/openapi.generated.ts index 9af42325ea..7428e5ace2 100644 --- a/plugins/catalog-backend/src/schema/openapi.generated.ts +++ b/plugins/catalog-backend/src/schema/openapi.generated.ts @@ -17,9 +17,10 @@ // ****************************************************************** // * THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. * // ****************************************************************** +import { createValidatedOpenApiRouter } from '@backstage/backend-openapi-utils'; -export default { - openapi: '3.1.0', +export const spec = { + openapi: '3.0.3', info: { title: '@backstage/plugin-catalog-backend', version: '1', @@ -97,8 +98,12 @@ export default { in: 'query', description: 'Restrict to just these fields in the response.', required: false, + allowReserved: true, schema: { - type: 'string', + type: 'array', + items: { + type: 'string', + }, }, }, filter: { @@ -107,7 +112,10 @@ export default { description: 'Filter for just the entities defined by this filter.', required: false, schema: { - type: 'string', + type: 'array', + items: { + type: 'string', + }, }, }, offset: { @@ -130,18 +138,16 @@ export default { minimum: 1, }, }, - sortField: { - name: 'sortField', + orderField: { + name: 'orderField', in: 'query', description: 'The fields to sort returned results by.', required: false, + allowReserved: true, schema: { type: 'array', items: { - type: 'array', - items: { - type: 'string', - }, + type: 'string', description: 'A two-item tuple of [field, order].', }, }, @@ -256,7 +262,7 @@ export default { }, ], description: 'Metadata fields common to all versions/kinds of entity.', - additionalProperties: false, + additionalProperties: true, }, EntityRelation: { type: 'object', @@ -305,7 +311,40 @@ export default { required: ['metadata', 'kind', 'apiVersion'], description: "The parts of the format that's common to all versions/kinds of entity.", - additionalProperties: false, + additionalProperties: true, + }, + NullableEntity: { + type: 'object', + properties: { + relations: { + type: 'array', + items: { + $ref: '#/components/schemas/EntityRelation', + }, + description: + 'The relations that this entity has with other entities.', + }, + spec: { + $ref: '#/components/schemas/JsonObject', + }, + metadata: { + $ref: '#/components/schemas/EntityMeta', + }, + kind: { + type: 'string', + description: 'The high level entity type being described.', + }, + apiVersion: { + type: 'string', + description: + 'The version of specification format for this particular entity that\nthis is written against.', + }, + }, + required: ['metadata', 'kind', 'apiVersion'], + description: + "The parts of the format that's common to all versions/kinds of entity.", + additionalProperties: true, + nullable: true, }, EntityAncestryResponse: { type: 'object', @@ -343,10 +382,7 @@ export default { items: { anyOf: [ { - $ref: '#/components/schemas/Entity', - }, - { - type: 'null', + $ref: '#/components/schemas/NullableEntity', }, ], }, @@ -405,10 +441,6 @@ export default { LocationSpec: { type: 'object', properties: { - presence: { - type: 'string', - enum: ['optional', 'required'], - }, target: { type: 'string', }, @@ -559,7 +591,8 @@ export default { 'A text to show to the user to inform about the choices made. Like, it could say\n"Found a CODEOWNERS file that covers this target, so we suggest leaving this\nfield empty; which would currently make it owned by X" where X is taken from the\ncodeowners file.', }, value: { - type: ['string', 'null'], + type: 'string', + nullable: true, }, state: { type: 'string', @@ -626,12 +659,8 @@ export default { target: { type: 'string', }, - presence: { - type: 'string', - enum: ['optional', 'required'], - }, }, - required: ['type', 'target', 'presence'], + required: ['type', 'target'], additionalProperties: false, }, SerializedError: { @@ -995,11 +1024,14 @@ export default { $ref: '#/components/parameters/limit', }, { - $ref: '#/components/parameters/sortField', + $ref: '#/components/parameters/orderField', }, { $ref: '#/components/parameters/cursor', }, + { + $ref: '#/components/parameters/filter', + }, { name: 'fullTextFilterTerm', in: 'query', @@ -1118,10 +1150,6 @@ export default { schema: { type: 'object', properties: { - presence: { - type: 'string', - enum: ['required', 'optional'], - }, target: { type: 'string', }, @@ -1129,7 +1157,7 @@ export default { type: 'string', }, }, - required: ['presence', 'target', 'type'], + required: ['target', 'type'], }, }, }, @@ -1328,7 +1356,8 @@ export default { type: 'string', }, entity: { - $ref: '#/components/schemas/Entity', + type: 'object', + additionalProperties: true, }, }, required: ['location', 'entity'], @@ -1340,3 +1369,6 @@ export default { }, }, } as const; +export const createOpenApiRouter = async ( + options?: Parameters['1'], +) => createValidatedOpenApiRouter(spec, options); diff --git a/plugins/catalog-backend/src/schema/openapi.yaml b/plugins/catalog-backend/src/schema/openapi.yaml index 701b4517bc..34e21adffd 100644 --- a/plugins/catalog-backend/src/schema/openapi.yaml +++ b/plugins/catalog-backend/src/schema/openapi.yaml @@ -1,4 +1,4 @@ -openapi: 3.1.0 +openapi: 3.0.3 info: title: '@backstage/plugin-catalog-backend' @@ -61,15 +61,20 @@ components: in: query description: Restrict to just these fields in the response. required: false + allowReserved: true schema: - type: string + type: array + items: + type: string filter: name: filter in: query description: Filter for just the entities defined by this filter. required: false schema: - type: string + type: array + items: + type: string offset: name: offset in: query @@ -86,17 +91,16 @@ components: schema: type: integer minimum: 1 - sortField: - name: sortField + orderField: + name: orderField in: query description: The fields to sort returned results by. required: false + allowReserved: true schema: type: array items: - type: array - items: - type: string + type: string description: A two-item tuple of [field, order]. explode: true style: form @@ -208,7 +212,7 @@ components: required: - name description: Metadata fields common to all versions/kinds of entity. - additionalProperties: false + additionalProperties: true EntityRelation: type: object properties: @@ -248,7 +252,34 @@ components: - kind - apiVersion description: The parts of the format that's common to all versions/kinds of entity. - additionalProperties: false + additionalProperties: true + NullableEntity: + type: object + properties: + relations: + type: array + items: + $ref: '#/components/schemas/EntityRelation' + description: The relations that this entity has with other entities. + spec: + $ref: '#/components/schemas/JsonObject' + metadata: + $ref: '#/components/schemas/EntityMeta' + kind: + type: string + description: The high level entity type being described. + apiVersion: + type: string + description: |- + The version of specification format for this particular entity that + this is written against. + required: + - metadata + - kind + - apiVersion + description: The parts of the format that's common to all versions/kinds of entity. + additionalProperties: true + nullable: true EntityAncestryResponse: type: object properties: @@ -279,8 +310,7 @@ components: type: array items: anyOf: - - $ref: '#/components/schemas/Entity' - - type: 'null' + - $ref: '#/components/schemas/NullableEntity' description: |- The list of entities, in the same order as the refs in the request. Entries that are null signify that no entity existed with that ref. @@ -326,11 +356,6 @@ components: LocationSpec: type: object properties: - presence: - type: string - enum: - - optional - - required target: type: string type: @@ -478,9 +503,8 @@ components: field empty; which would currently make it owned by X" where X is taken from the codeowners file. value: - type: - - string - - 'null' + type: string + nullable: true state: type: string enum: @@ -540,15 +564,9 @@ components: type: string target: type: string - presence: - type: string - enum: - - optional - - required required: - type - target - - presence additionalProperties: false SerializedError: allOf: @@ -763,8 +781,9 @@ paths: parameters: - $ref: '#/components/parameters/fields' - $ref: '#/components/parameters/limit' - - $ref: '#/components/parameters/sortField' + - $ref: '#/components/parameters/orderField' - $ref: '#/components/parameters/cursor' + - $ref: '#/components/parameters/filter' - name: fullTextFilterTerm in: query description: Text search term. @@ -841,17 +860,11 @@ paths: schema: type: object properties: - presence: - type: string - enum: - - required - - optional target: type: string type: type: string required: - - presence - target - type get: @@ -978,7 +991,8 @@ paths: location: type: string entity: - $ref: '#/components/schemas/Entity' + type: object + additionalProperties: true required: - location - entity diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index df934eae68..c14bc05476 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -247,6 +247,24 @@ describe('createRouter readonly disabled', () => { expect(response.status).toEqual(400); expect(response.body.error.message).toMatch(/Malformed cursor/); }); + + it('should throw in case of invalid limit', async () => { + const items: Entity[] = [ + { apiVersion: 'a', kind: 'b', metadata: { name: 'n' } }, + ]; + + entitiesCatalog.queryEntities.mockResolvedValueOnce({ + items, + totalItems: 100, + pageInfo: { nextCursor: mockCursor() }, + }); + + const response = await request(app).get(`/entities/by-query?limit=asdf`); + expect(response.status).toEqual(400); + expect(response.body.error.message).toMatch( + /request\/query\/limit must be integer/, + ); + }); }); describe('GET /entities/by-uid/:uid', () => { diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index 997cd481a6..30c1fd8e3f 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -25,7 +25,6 @@ import { import { Config } from '@backstage/config'; import { NotFoundError, serializeError } from '@backstage/errors'; import express from 'express'; -import Router from 'express-promise-router'; import { Logger } from 'winston'; import yn from 'yn'; import { z } from 'zod'; @@ -37,7 +36,6 @@ import { basicEntityFilter, entitiesBatchRequest, parseEntityFilterParams, - parseEntityPaginationParams, parseEntityTransformParams, parseQueryEntitiesParams, } from './request'; @@ -50,10 +48,10 @@ import { locationInput, validateRequestBody, } from './util'; -import type { ApiRouter } from '@backstage/backend-openapi-utils'; -import spec from '../schema/openapi.generated'; +import { createOpenApiRouter } from '../schema/openapi.generated'; import { PluginTaskScheduler } from '@backstage/backend-tasks'; import { getBearerTokenFromAuthorizationHeader } from '@backstage/plugin-auth-node'; +import { parseEntityPaginationParams } from './request/parseEntityPaginationParams'; /** * Options used by {@link createRouter}. @@ -80,6 +78,13 @@ export interface RouterOptions { export async function createRouter( options: RouterOptions, ): Promise { + const router = await createOpenApiRouter({ + validatorOptions: { + // We want the spec to be up to date with the expected value, but the return type needs + // to be controlled by the router implementation not the request validator. + ignorePaths: /^\/validate-entity\/?$/, + }, + }); const { entitiesCatalog, locationAnalyzer, @@ -90,8 +95,6 @@ export async function createRouter( logger, permissionIntegrationRouter, } = options; - const router = Router() as ApiRouter; - router.use(express.json()); const readonlyEnabled = config.getOptionalBoolean('catalog.readonly') || false; @@ -142,6 +145,7 @@ export async function createRouter( .get('/entities/by-query', async (req, res) => { const { items, pageInfo, totalItems } = await entitiesCatalog.queryEntities({ + limit: req.query.limit, ...parseQueryEntitiesParams(req.query), authorizationToken: getBearerTokenFromAuthorizationHeader( req.header('authorization'), diff --git a/plugins/catalog-backend/src/service/request/index.ts b/plugins/catalog-backend/src/service/request/index.ts index d9297fc122..dbfd34aef1 100644 --- a/plugins/catalog-backend/src/service/request/index.ts +++ b/plugins/catalog-backend/src/service/request/index.ts @@ -17,6 +17,5 @@ export { entitiesBatchRequest } from './entitiesBatchRequest'; export { basicEntityFilter } from './basicEntityFilter'; export { parseEntityFilterParams } from './parseEntityFilterParams'; -export { parseEntityPaginationParams } from './parseEntityPaginationParams'; export { parseEntityTransformParams } from './parseEntityTransformParams'; export { parseQueryEntitiesParams } from './parseQueryEntitiesParams'; diff --git a/plugins/catalog-backend/src/service/request/parseEntityPaginationParams.test.ts b/plugins/catalog-backend/src/service/request/parseEntityPaginationParams.test.ts index 331d453973..5e61f55b12 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityPaginationParams.test.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityPaginationParams.test.ts @@ -19,29 +19,23 @@ import { parseEntityPaginationParams } from './parseEntityPaginationParams'; describe('parseEntityPaginationParams', () => { it('works for the happy path', () => { expect(parseEntityPaginationParams({})).toBeUndefined(); - expect(parseEntityPaginationParams({ limit: '1' })).toEqual({ limit: 1 }); - expect(parseEntityPaginationParams({ offset: '0' })).toEqual({ offset: 0 }); - expect(parseEntityPaginationParams({ offset: '2' })).toEqual({ offset: 2 }); + expect(parseEntityPaginationParams({ limit: 1 })).toEqual({ limit: 1 }); + expect(parseEntityPaginationParams({ offset: 0 })).toEqual({ offset: 0 }); + expect(parseEntityPaginationParams({ offset: 2 })).toEqual({ offset: 2 }); expect(parseEntityPaginationParams({ after: 'x' })).toEqual({ after: 'x' }); expect( - parseEntityPaginationParams({ limit: '1', offset: '2', after: 'x' }), + parseEntityPaginationParams({ limit: 1, offset: 2, after: 'x' }), ).toEqual({ limit: 1, offset: 2, after: 'x' }); }); it('rejects bad values', () => { - expect(() => parseEntityPaginationParams({ limit: '' })).toThrow( - 'Invalid limit, not an integer', - ); - expect(() => parseEntityPaginationParams({ limit: '0' })).toThrow( + expect(() => parseEntityPaginationParams({ limit: 0 })).toThrow( 'Invalid limit, must be greater than zero', ); - expect(() => parseEntityPaginationParams({ limit: '-1' })).toThrow( + expect(() => parseEntityPaginationParams({ limit: -1 })).toThrow( 'Invalid limit, must be greater than zero', ); - expect(() => parseEntityPaginationParams({ offset: '' })).toThrow( - 'Invalid offset, not an integer', - ); - expect(() => parseEntityPaginationParams({ offset: '-1' })).toThrow( + expect(() => parseEntityPaginationParams({ offset: -1 })).toThrow( 'Invalid offset, must be zero or greater', ); expect(() => parseEntityPaginationParams({ after: '' })).toThrow( diff --git a/plugins/catalog-backend/src/service/request/parseEntityPaginationParams.ts b/plugins/catalog-backend/src/service/request/parseEntityPaginationParams.ts index 87b19e13de..61905aaa6a 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityPaginationParams.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityPaginationParams.ts @@ -16,19 +16,20 @@ import { InputError } from '@backstage/errors'; import { EntityPagination } from '../../catalog/types'; -import { parseIntegerParam, parseStringParam } from './common'; /** * Parses the pagination related parameters out of a query, e.g. * /entities?offset=100&limit=10 */ -export function parseEntityPaginationParams( - params: Record, -): EntityPagination | undefined { - const offset = parseIntegerParam(params.offset, 'offset'); - const limit = parseIntegerParam(params.limit, 'limit'); - const after = parseStringParam(params.after, 'after'); - +export function parseEntityPaginationParams({ + limit, + offset, + after, +}: { + offset?: number; + limit?: number; + after?: string; +}): EntityPagination | undefined { if (offset === undefined && limit === undefined && after === undefined) { return undefined; } diff --git a/plugins/catalog-backend/src/service/request/parseQueryEntitiesParams.test.ts b/plugins/catalog-backend/src/service/request/parseQueryEntitiesParams.test.ts index 43ba218eac..dfcfd58c7e 100644 --- a/plugins/catalog-backend/src/service/request/parseQueryEntitiesParams.test.ts +++ b/plugins/catalog-backend/src/service/request/parseQueryEntitiesParams.test.ts @@ -37,7 +37,6 @@ describe('parseQueryEntitiesParams', () => { const parsedObj = parseQueryEntitiesParams( validRequest, ) as QueryEntitiesInitialRequest; - expect(parsedObj.limit).toBe(3); expect(parsedObj.fields).toBeDefined(); expect(parsedObj.orderFields).toEqual([ { field: 'metadata.name', order: 'desc' }, @@ -54,7 +53,6 @@ describe('parseQueryEntitiesParams', () => { const parsedObj = parseQueryEntitiesParams( {}, ) as QueryEntitiesInitialRequest; - expect(parsedObj.limit).toBeUndefined(); expect(parsedObj.fields).toBeUndefined(); expect(parsedObj.orderFields).toBeUndefined(); expect(parsedObj.filter).toBeUndefined(); @@ -64,9 +62,6 @@ describe('parseQueryEntitiesParams', () => { }); it.each([ - { - limit: 'asd', - }, { filter: 3 }, { orderField: ['metadata.uid,diagonal'] }, { fields: [4] }, @@ -94,7 +89,6 @@ describe('parseQueryEntitiesParams', () => { const parsedObj = parseQueryEntitiesParams( validRequest, ) as QueryEntitiesCursorRequest; - expect(parsedObj.limit).toBe(3); expect(parsedObj.fields).toBeDefined(); expect(parsedObj.cursor).toEqual(cursor); }); @@ -118,7 +112,6 @@ describe('parseQueryEntitiesParams', () => { const parsedObj = parseQueryEntitiesParams( validRequest, ) as QueryEntitiesCursorRequest; - expect(parsedObj.limit).toBe(3); expect(parsedObj.fields).toBeDefined(); expect(parsedObj.cursor).toEqual(cursor); expect(parsedObj).not.toHaveProperty('filter'); @@ -130,18 +123,14 @@ describe('parseQueryEntitiesParams', () => { const parsedObj = parseQueryEntitiesParams( {}, ) as QueryEntitiesCursorRequest; - expect(parsedObj.limit).toBeUndefined(); expect(parsedObj.fields).toBeUndefined(); }); - it.each([ - { - limit: 'asd', + it.each([{ cursor: [] }, { fields: [4] }])( + 'should throw if some parameter is not valid %p', + params => { + expect(() => parseQueryEntitiesParams(params)).toThrow(); }, - { cursor: [] }, - { fields: [4] }, - ])('should throw if some parameter is not valid %p', params => { - expect(() => parseQueryEntitiesParams(params)).toThrow(); - }); + ); }); }); diff --git a/plugins/catalog-backend/src/service/request/parseQueryEntitiesParams.ts b/plugins/catalog-backend/src/service/request/parseQueryEntitiesParams.ts index ae8b649d13..9b55f193b9 100644 --- a/plugins/catalog-backend/src/service/request/parseQueryEntitiesParams.ts +++ b/plugins/catalog-backend/src/service/request/parseQueryEntitiesParams.ts @@ -20,7 +20,7 @@ import { QueryEntitiesRequest, } from '../../catalog/types'; import { decodeCursor } from '../util'; -import { parseIntegerParam, parseStringParam } from './common'; +import { parseStringParam } from './common'; import { parseEntityFilterParams } from './parseEntityFilterParams'; import { parseEntityOrderFieldParams } from './parseEntityOrderFieldParams'; import { parseEntityTransformParams } from './parseEntityTransformParams'; @@ -28,16 +28,14 @@ import { parseFullTextFilterFields } from './parseFullTextFilterFields'; export function parseQueryEntitiesParams( params: Record, -): Omit { +): Omit { const fields = parseEntityTransformParams(params); - const limit = parseIntegerParam(params.limit, 'limit'); const cursor = parseStringParam(params.cursor, 'cursor'); if (cursor) { const decodedCursor = decodeCursor(cursor); const response: Omit = { cursor: decodedCursor, fields, - limit, }; return response; } @@ -54,7 +52,6 @@ export function parseQueryEntitiesParams( const response: Omit = { fields, filter, - limit, orderFields, fullTextFilter: { term: fullTextFilterTerm || '', diff --git a/plugins/search-backend/package.json b/plugins/search-backend/package.json index 60f3c04d54..8627fef0bb 100644 --- a/plugins/search-backend/package.json +++ b/plugins/search-backend/package.json @@ -37,6 +37,7 @@ }, "dependencies": { "@backstage/backend-common": "workspace:^", + "@backstage/backend-openapi-utils": "workspace:^", "@backstage/backend-plugin-api": "workspace:^", "@backstage/config": "workspace:^", "@backstage/errors": "workspace:^", @@ -57,7 +58,6 @@ "zod": "^3.21.4" }, "devDependencies": { - "@backstage/backend-openapi-utils": "workspace:^", "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", "@types/supertest": "^2.0.8", diff --git a/plugins/search-backend/src/schema/openapi.generated.ts b/plugins/search-backend/src/schema/openapi.generated.ts index 28ca9b5339..9bf2e08d3e 100644 --- a/plugins/search-backend/src/schema/openapi.generated.ts +++ b/plugins/search-backend/src/schema/openapi.generated.ts @@ -17,8 +17,9 @@ // ****************************************************************** // * THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. * // ****************************************************************** +import { createValidatedOpenApiRouter } from '@backstage/backend-openapi-utils'; -export default { +export const spec = { openapi: '3.0.3', info: { title: '@backstage/plugin-search-backend', @@ -210,3 +211,6 @@ export default { }, }, } as const; +export const createOpenApiRouter = async ( + options?: Parameters['1'], +) => createValidatedOpenApiRouter(spec, options); diff --git a/plugins/search-backend/src/service/router.test.ts b/plugins/search-backend/src/service/router.test.ts index b4772dd62d..7ec8603307 100644 --- a/plugins/search-backend/src/service/router.test.ts +++ b/plugins/search-backend/src/service/router.test.ts @@ -21,7 +21,6 @@ import { IndexBuilder } from '@backstage/plugin-search-backend-node'; import { SearchEngine } from '@backstage/plugin-search-common'; import express from 'express'; import request from 'supertest'; - import { createRouter } from './router'; const mockPermissionEvaluator: PermissionEvaluator = { diff --git a/plugins/search-backend/src/service/router.ts b/plugins/search-backend/src/service/router.ts index 3643eec38a..dca632dfaf 100644 --- a/plugins/search-backend/src/service/router.ts +++ b/plugins/search-backend/src/service/router.ts @@ -15,7 +15,6 @@ */ import express from 'express'; -import Router from 'express-promise-router'; import { Logger } from 'winston'; import { z } from 'zod'; import { errorHandler } from '@backstage/backend-common'; @@ -35,8 +34,7 @@ import { } from '@backstage/plugin-search-common'; import { SearchEngine } from '@backstage/plugin-search-common'; import { AuthorizedSearchEngine } from './AuthorizedSearchEngine'; -import type { ApiRouter } from '@backstage/backend-openapi-utils'; -import spec from '../schema/openapi.generated'; +import { createOpenApiRouter } from '../schema/openapi.generated'; const jsonObjectSchema: z.ZodSchema = z.lazy(() => { const jsonValueSchema: z.ZodSchema = z.lazy(() => @@ -73,6 +71,7 @@ const allowedLocationProtocols = ['http:', 'https:']; export async function createRouter( options: RouterOptions, ): Promise { + const router = await createOpenApiRouter(); const { engine: inputEngine, types, permissions, config, logger } = options; const maxPageLimit = @@ -86,14 +85,7 @@ export async function createRouter( .optional(), pageCursor: z.string().optional(), pageLimit: z - .string() - .transform(pageLimit => parseInt(pageLimit, 10)) - .refine( - pageLimit => !isNaN(pageLimit), - pageLimit => ({ - message: `The page limit "${pageLimit}" is not a number`, - }), - ) + .number() .refine( pageLimit => pageLimit <= maxPageLimit, pageLimit => ({ @@ -148,7 +140,6 @@ export async function createRouter( })), }); - const router = Router() as ApiRouter; router.get('/query', async (req, res) => { const parseResult = requestSchema.passthrough().safeParse(req.query); diff --git a/plugins/todo-backend/package.json b/plugins/todo-backend/package.json index e173e9a02c..066add9977 100644 --- a/plugins/todo-backend/package.json +++ b/plugins/todo-backend/package.json @@ -30,6 +30,7 @@ }, "dependencies": { "@backstage/backend-common": "workspace:^", + "@backstage/backend-openapi-utils": "workspace:^", "@backstage/backend-plugin-api": "workspace:^", "@backstage/catalog-client": "workspace:^", "@backstage/catalog-model": "workspace:^", @@ -45,7 +46,6 @@ "yn": "^4.0.0" }, "devDependencies": { - "@backstage/backend-openapi-utils": "workspace:^", "@backstage/cli": "workspace:^", "@types/supertest": "^2.0.8", "msw": "^1.0.0", diff --git a/plugins/todo-backend/src/lib/utils.ts b/plugins/todo-backend/src/lib/utils.ts index cf0c2e4ea6..63bf754f01 100644 --- a/plugins/todo-backend/src/lib/utils.ts +++ b/plugins/todo-backend/src/lib/utils.ts @@ -16,23 +16,6 @@ import { InputError } from '@backstage/errors'; -export const parseIntegerParam = ( - str: unknown, - ctx: string, -): number | undefined => { - if (str === undefined) { - return undefined; - } - if (typeof str !== 'string') { - throw new InputError(`invalid ${ctx}, must be a string`); - } - const parsed = parseInt(str, 10); - if (!Number.isInteger(parsed) || String(parsed) !== str) { - throw new InputError(`invalid ${ctx}, not an integer`); - } - return parsed; -}; - export const parseOrderByParam = ( str: unknown, allowedFields: T, diff --git a/plugins/todo-backend/src/schema/openapi.generated.ts b/plugins/todo-backend/src/schema/openapi.generated.ts index 54486a5615..a58a96ddb8 100644 --- a/plugins/todo-backend/src/schema/openapi.generated.ts +++ b/plugins/todo-backend/src/schema/openapi.generated.ts @@ -17,8 +17,9 @@ // ****************************************************************** // * THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. * // ****************************************************************** +import { createValidatedOpenApiRouter } from '@backstage/backend-openapi-utils'; -export default { +export const spec = { openapi: '3.0.3', info: { title: '@backstage/plugin-todo-backend', @@ -149,7 +150,8 @@ export default { { name: 'entity', in: 'query', - required: true, + required: false, + allowReserved: true, schema: { type: 'string', minLength: 1, @@ -206,3 +208,6 @@ export default { }, }, } as const; +export const createOpenApiRouter = async ( + options?: Parameters['1'], +) => createValidatedOpenApiRouter(spec, options); diff --git a/plugins/todo-backend/src/schema/openapi.yaml b/plugins/todo-backend/src/schema/openapi.yaml index 3fba129644..0e6203e002 100644 --- a/plugins/todo-backend/src/schema/openapi.yaml +++ b/plugins/todo-backend/src/schema/openapi.yaml @@ -95,7 +95,8 @@ paths: parameters: - name: entity in: query - required: true + required: false + allowReserved: true schema: type: string minLength: 1 diff --git a/plugins/todo-backend/src/service/router.test.ts b/plugins/todo-backend/src/service/router.test.ts index f2d9f2c7a3..0bf591316c 100644 --- a/plugins/todo-backend/src/service/router.test.ts +++ b/plugins/todo-backend/src/service/router.test.ts @@ -19,11 +19,7 @@ import request from 'supertest'; import { errorHandler } from '@backstage/backend-common'; import { createRouter } from './router'; -import { - parseFilterParam, - parseIntegerParam, - parseOrderByParam, -} from '../lib/utils'; +import { parseFilterParam, parseOrderByParam } from '../lib/utils'; import { TodoService } from './types'; const mockListBody = { @@ -136,58 +132,44 @@ describe('createRouter', () => { }); it('rejects invalid queries', async () => { - await expect( - request(app).get('/v1/todos?entity=k:n&entity=k:n'), - ).resolves.toMatchObject( - matchErrorResponse(400, 'InputError', 'entity query must be a string'), - ); + request(app) + .get('/v1/todos?entity=k:n&entity=k:n') + .expect(400) + .expect( + matchErrorResponse( + 400, + 'InputError', + 'entity query must be a string', + ), + ); - await expect( - request(app).get('/v1/todos?entity=:n'), - ).resolves.toMatchObject( - matchErrorResponse( - 400, - 'InputError', - 'Invalid entity ref, TypeError: Entity reference ":n" was not on the form [:][/]', - ), - ); + request(app) + .get('/v1/todos?entity=:n') + .expect(400) + .expect( + matchErrorResponse( + 400, + 'InputError', + 'Invalid entity ref, TypeError: Entity reference ":n" was not on the form [:][/]', + ), + ); - await expect( - request(app).get('/v1/todos?offset=1.5'), - ).resolves.toMatchObject( - matchErrorResponse( - 400, - 'InputError', - 'invalid offset query, not an integer', - ), - ); + request(app) + .get('/v1/todos?offset=1.5') + .expect(400) + .expect( + matchErrorResponse( + 400, + 'InputError', + 'invalid offset query, not an integer', + ), + ); expect(mockService.listTodos).not.toHaveBeenCalled(); }); }); }); -describe('parseIntegerParam', () => { - it('should parse a param', () => { - expect(parseIntegerParam('1', 'ctx')).toBe(1); - }); - - it('should reject invalid params', () => { - expect(() => parseIntegerParam(['1'], 'ctx')).toThrow( - 'invalid ctx, must be a string', - ); - expect(() => parseIntegerParam('1.5', 'ctx')).toThrow( - 'invalid ctx, not an integer', - ); - expect(() => parseIntegerParam('foo', 'ctx')).toThrow( - 'invalid ctx, not an integer', - ); - expect(() => parseIntegerParam('1foo', 'ctx')).toThrow( - 'invalid ctx, not an integer', - ); - }); -}); - describe('parseOrderByParam', () => { it('should parse a param', () => { expect(parseOrderByParam('a=asc', ['a'])).toEqual({ diff --git a/plugins/todo-backend/src/service/router.ts b/plugins/todo-backend/src/service/router.ts index 364cbef6b0..53b0a02668 100644 --- a/plugins/todo-backend/src/service/router.ts +++ b/plugins/todo-backend/src/service/router.ts @@ -17,16 +17,13 @@ import { CompoundEntityRef, parseEntityRef } from '@backstage/catalog-model'; import { InputError } from '@backstage/errors'; import express from 'express'; -import Router from 'express-promise-router'; import { type TodoService, TODO_FIELDS } from './types'; import { getBearerToken, parseFilterParam, - parseIntegerParam, parseOrderByParam, } from '../lib/utils'; -import spec from '../schema/openapi.generated'; -import type { ApiRouter } from '@backstage/backend-openapi-utils'; +import { createOpenApiRouter } from '../schema/openapi.generated'; /** @public */ export interface RouterOptions { @@ -37,14 +34,12 @@ export interface RouterOptions { export async function createRouter( options: RouterOptions, ): Promise { + const router = await createOpenApiRouter(); + router.use(express.json()); const { todoService } = options; - const router = Router() as ApiRouter; - router.use(express.json()); - router.get('/v1/todos', async (req, res) => { - const offset = parseIntegerParam(req.query.offset, 'offset query'); - const limit = parseIntegerParam(req.query.limit, 'limit query'); + const { offset, limit } = req.query; const orderBy = parseOrderByParam(req.query.orderBy, TODO_FIELDS); const filters = parseFilterParam(req.query.filter, TODO_FIELDS); diff --git a/yarn.lock b/yarn.lock index 52a770e2b8..d9ee847259 100644 --- a/yarn.lock +++ b/yarn.lock @@ -39,7 +39,7 @@ __metadata: languageName: node linkType: hard -"@apidevtools/json-schema-ref-parser@npm:^9.0.6": +"@apidevtools/json-schema-ref-parser@npm:^9.0.6, @apidevtools/json-schema-ref-parser@npm:^9.1.2": version: 9.1.2 resolution: "@apidevtools/json-schema-ref-parser@npm:9.1.2" dependencies: @@ -3744,12 +3744,16 @@ __metadata: resolution: "@backstage/backend-openapi-utils@workspace:packages/backend-openapi-utils" dependencies: "@backstage/cli": "workspace:^" + "@backstage/errors": "workspace:^" "@types/express": ^4.17.6 "@types/express-serve-static-core": ^4.17.5 express: ^4.17.1 + express-openapi-validator: ^5.0.4 express-promise-router: ^4.1.0 json-schema-to-ts: ^2.6.2 + lodash: ^4.17.21 openapi3-ts: ^3.1.2 + supertest: ^6.1.3 languageName: unknown linkType: soft @@ -12687,7 +12691,7 @@ __metadata: languageName: node linkType: hard -"@jsdevtools/ono@npm:^7.1.3": +"@jsdevtools/ono@npm:7.1.3, @jsdevtools/ono@npm:^7.1.3": version: 7.1.3 resolution: "@jsdevtools/ono@npm:7.1.3" checksum: 2297fcd472ba810bffe8519d2249171132844c7174f3a16634f9260761c8c78bc0428a4190b5b6d72d45673c13918ab9844d706c3ed4ef8f62ab11a2627a08ad @@ -17341,6 +17345,15 @@ __metadata: languageName: node linkType: hard +"@types/multer@npm:^1.4.7": + version: 1.4.7 + resolution: "@types/multer@npm:1.4.7" + dependencies: + "@types/express": "*" + checksum: 680cb0710aa25264d20cdcdaf34c212b636b55ea141310f06c25354ab1401193c7aa6839f9d22abf64a223fab1f2b8287f2512b0bef7e1628c4e9ffe54b4aeb2 + languageName: node + linkType: hard + "@types/ndjson@npm:^2.0.1": version: 2.0.1 resolution: "@types/ndjson@npm:2.0.1" @@ -17413,9 +17426,9 @@ __metadata: linkType: hard "@types/node@npm:^18.11.17": - version: 18.16.19 - resolution: "@types/node@npm:18.16.19" - checksum: 63c31f09616508aa7135380a4c79470a897b75f9ff3a70eb069e534dfabdec3f32fb0f9df5939127f1086614d980ddea0fa5e8cc29a49103c4f74cd687618aaf + version: 18.16.8 + resolution: "@types/node@npm:18.16.8" + checksum: d2ec9b61abd260b0fa1e33e4eca8f0089ebb6f6ec1ad2f036a4d01af11dba4f1dbff95b2a1f5dbe9eab3a6f38ed4bd8bf95eb2de9ad96b1caa1f67671e623c1d languageName: node linkType: hard @@ -19004,7 +19017,7 @@ __metadata: languageName: node linkType: hard -"ajv@npm:^8.0.0, ajv@npm:^8.10.0, ajv@npm:^8.12.0, ajv@npm:^8.6.0, ajv@npm:^8.6.3, ajv@npm:^8.8.0, ajv@npm:^8.8.2": +"ajv@npm:^8.0.0, ajv@npm:^8.10.0, ajv@npm:^8.11.2, ajv@npm:^8.12.0, ajv@npm:^8.6.0, ajv@npm:^8.6.3, ajv@npm:^8.8.0, ajv@npm:^8.8.2": version: 8.12.0 resolution: "ajv@npm:8.12.0" dependencies: @@ -19141,6 +19154,13 @@ __metadata: languageName: node linkType: hard +"append-field@npm:^1.0.0": + version: 1.0.0 + resolution: "append-field@npm:1.0.0" + checksum: 482ba08acc0ecef00fe7da6bf2f8e48359a9905ee1af525f3120c9260c02e91eedf0579b59d898e8d8455b6c199e340bc0a2fd4b9e02adaa29a8a86c722b37f9 + languageName: node + linkType: hard + "aproba@npm:^1.0.3": version: 1.2.0 resolution: "aproba@npm:1.2.0" @@ -20473,7 +20493,7 @@ __metadata: languageName: node linkType: hard -"busboy@npm:^1.6.0": +"busboy@npm:^1.0.0, busboy@npm:^1.6.0": version: 1.6.0 resolution: "busboy@npm:1.6.0" dependencies: @@ -21656,6 +21676,18 @@ __metadata: languageName: node linkType: hard +"concat-stream@npm:^1.5.2": + version: 1.6.2 + resolution: "concat-stream@npm:1.6.2" + dependencies: + buffer-from: ^1.0.0 + inherits: ^2.0.3 + readable-stream: ^2.2.2 + typedarray: ^0.0.6 + checksum: 1ef77032cb4459dcd5187bd710d6fc962b067b64ec6a505810de3d2b8cc0605638551b42f8ec91edf6fcd26141b32ef19ad749239b58fae3aba99187adc32285 + languageName: node + linkType: hard + "concat-stream@npm:^2.0.0": version: 2.0.0 resolution: "concat-stream@npm:2.0.0" @@ -21796,7 +21828,7 @@ __metadata: languageName: node linkType: hard -"content-type@npm:~1.0.4, content-type@npm:~1.0.5": +"content-type@npm:^1.0.5, content-type@npm:~1.0.4, content-type@npm:~1.0.5": version: 1.0.5 resolution: "content-type@npm:1.0.5" checksum: 566271e0a251642254cde0f845f9dd4f9856e52d988f4eb0d0dcffbb7a1f8ec98de7a5215fc628f3bce30fe2fb6fd2bc064b562d721658c59b544e2d34ea2766 @@ -25183,6 +25215,28 @@ __metadata: languageName: node linkType: hard +"express-openapi-validator@npm:^5.0.4": + version: 5.0.4 + resolution: "express-openapi-validator@npm:5.0.4" + dependencies: + "@apidevtools/json-schema-ref-parser": ^9.1.2 + "@types/multer": ^1.4.7 + ajv: ^8.11.2 + ajv-draft-04: ^1.0.0 + ajv-formats: ^2.1.1 + content-type: ^1.0.5 + lodash.clonedeep: ^4.5.0 + lodash.get: ^4.4.2 + lodash.uniq: ^4.5.0 + lodash.zipobject: ^4.1.3 + media-typer: ^1.1.0 + multer: ^1.4.5-lts.1 + ono: ^7.1.3 + path-to-regexp: ^6.2.0 + checksum: 5f41d632324fd4217ca74353cb9c9bf50ada6f6ecd5396e9a059f6ce05e66c4698706bb14b7a4ea5a88a4190b82327e37d899dd9a22301dfd6717e24930c81b6 + languageName: node + linkType: hard + "express-prom-bundle@npm:^6.3.6": version: 6.6.0 resolution: "express-prom-bundle@npm:6.6.0" @@ -31041,6 +31095,13 @@ __metadata: languageName: node linkType: hard +"lodash.zipobject@npm:^4.1.3": + version: 4.1.3 + resolution: "lodash.zipobject@npm:4.1.3" + checksum: 1ab635b665c0488a905779705a6683e9024115176e9e947d75d2a6b1e8673230fdb11c417788fbaf26d71e1cac5ad8e59a558924612cbf7d6615780836048883 + languageName: node + linkType: hard + "lodash@npm:4.17.21, lodash@npm:^4.15.0, lodash@npm:^4.17.10, lodash@npm:^4.17.11, lodash@npm:^4.17.14, lodash@npm:^4.17.15, lodash@npm:^4.17.19, lodash@npm:^4.17.20, lodash@npm:^4.17.21, lodash@npm:^4.17.4, lodash@npm:^4.7.0, lodash@npm:~4.17.0, lodash@npm:~4.17.15, lodash@npm:~4.17.21": version: 4.17.21 resolution: "lodash@npm:4.17.21" @@ -31659,6 +31720,13 @@ __metadata: languageName: node linkType: hard +"media-typer@npm:^1.1.0": + version: 1.1.0 + resolution: "media-typer@npm:1.1.0" + checksum: a58dd60804df73c672942a7253ccc06815612326dc1c0827984b1a21704466d7cde351394f47649e56cf7415e6ee2e26e000e81b51b3eebb5a93540e8bf93cbd + languageName: node + linkType: hard + "mem-fs-editor@npm:^8.1.2 || ^9.0.0": version: 9.3.0 resolution: "mem-fs-editor@npm:9.3.0" @@ -32498,6 +32566,17 @@ __metadata: languageName: node linkType: hard +"mkdirp@npm:^0.5.4": + version: 0.5.6 + resolution: "mkdirp@npm:0.5.6" + dependencies: + minimist: ^1.2.6 + bin: + mkdirp: bin/cmd.js + checksum: 0c91b721bb12c3f9af4b77ebf73604baf350e64d80df91754dc509491ae93bf238581e59c7188360cec7cb62fc4100959245a42cfe01834efedc5e9d068376c2 + languageName: node + linkType: hard + "mkdirp@npm:^1.0.3, mkdirp@npm:^1.0.4": version: 1.0.4 resolution: "mkdirp@npm:1.0.4" @@ -32619,6 +32698,21 @@ __metadata: languageName: node linkType: hard +"multer@npm:^1.4.5-lts.1": + version: 1.4.5-lts.1 + resolution: "multer@npm:1.4.5-lts.1" + dependencies: + append-field: ^1.0.0 + busboy: ^1.0.0 + concat-stream: ^1.5.2 + mkdirp: ^0.5.4 + object-assign: ^4.1.1 + type-is: ^1.6.4 + xtend: ^4.0.0 + checksum: d6dfa78a6ec592b74890412f8962da8a87a3dcfe20f612e039b735b8e0faa72c735516c447f7de694ee0d981eb0a1b892fb9e2402a0348dc6091d18c38d89ecc + languageName: node + linkType: hard + "multicast-dns@npm:^7.2.4": version: 7.2.4 resolution: "multicast-dns@npm:7.2.4" @@ -33594,6 +33688,15 @@ __metadata: languageName: node linkType: hard +"ono@npm:^7.1.3": + version: 7.1.3 + resolution: "ono@npm:7.1.3" + dependencies: + "@jsdevtools/ono": 7.1.3 + checksum: d341681f1bdd08071760a8d92d37e0e5fb483c6f5c510543a17896c8ee7bdd399a375c632d39f9c78bd2aeab4e5e2eaae9ae0ab71c9738276ba8459c18ce41c4 + languageName: node + linkType: hard + "open@npm:^7.4.2": version: 7.4.2 resolution: "open@npm:7.4.2" @@ -36887,9 +36990,9 @@ __metadata: languageName: node linkType: hard -"readable-stream@npm:^2.0.0, readable-stream@npm:^2.0.1, readable-stream@npm:^2.0.2, readable-stream@npm:^2.0.5, readable-stream@npm:^2.0.6, readable-stream@npm:^2.3.3, readable-stream@npm:^2.3.5, readable-stream@npm:^2.3.6": - version: 2.3.7 - resolution: "readable-stream@npm:2.3.7" +"readable-stream@npm:^2.0.0, readable-stream@npm:^2.0.1, readable-stream@npm:^2.0.2, readable-stream@npm:^2.0.5, readable-stream@npm:^2.0.6, readable-stream@npm:^2.2.2, readable-stream@npm:^2.3.3, readable-stream@npm:^2.3.5, readable-stream@npm:^2.3.6": + version: 2.3.8 + resolution: "readable-stream@npm:2.3.8" dependencies: core-util-is: ~1.0.0 inherits: ~2.0.3 @@ -36898,7 +37001,7 @@ __metadata: safe-buffer: ~5.1.1 string_decoder: ~1.1.1 util-deprecate: ~1.0.1 - checksum: e4920cf7549a60f8aaf694d483a0e61b2a878b969d224f89b3bc788b8d920075132c4b55a7494ee944c7b6a9a0eada28a7f6220d80b0312ece70bbf08eeca755 + checksum: 65645467038704f0c8aaf026a72fbb588a9e2ef7a75cd57a01702ee9db1c4a1e4b03aaad36861a6a0926546a74d174149c8c207527963e0c2d3eee2f37678a42 languageName: node linkType: hard @@ -40488,9 +40591,9 @@ __metadata: linkType: hard "ts-log@npm:^2.2.3": - version: 2.2.3 - resolution: "ts-log@npm:2.2.3" - checksum: 8aa34a2724d7915ddc6de8d0c27eb48f67206b52c42656f1ea2a7ffb080fa664db5fb22b42bd3c72b5b95cc6eb4612196b7d1e28f4bc146191ebc2a9683af548 + version: 2.2.5 + resolution: "ts-log@npm:2.2.5" + checksum: 28f78ab15b8555d56c089dbc243327d8ce4331219956242a29fc4cb3bad6bb0cb8234dd17a292381a1b1dba99a7e4849a2181b2e1a303e8247e9f4ca4e284f2d languageName: node linkType: hard @@ -40714,7 +40817,7 @@ __metadata: languageName: node linkType: hard -"type-is@npm:~1.6.18": +"type-is@npm:^1.6.4, type-is@npm:~1.6.18": version: 1.6.18 resolution: "type-is@npm:1.6.18" dependencies: @@ -40908,11 +41011,11 @@ __metadata: linkType: hard "undici@npm:^5.12.0, undici@npm:^5.8.0": - version: 5.19.1 - resolution: "undici@npm:5.19.1" + version: 5.22.1 + resolution: "undici@npm:5.22.1" dependencies: busboy: ^1.6.0 - checksum: 57ee94ee74d944faa41dbcb2faf4e0c90069708d3aaae860185884e51376b5d457728352a8396d69a3c9cb752b62ff99a19a664c5aacb7ee61cc488af499a01c + checksum: 048a3365f622be44fb319316cedfaa241c59cf7f3368ae7667a12323447e1822e8cc3d00f6956c852d1478a6fde1cbbe753f49e05f2fdaed229693e716ebaf35 languageName: node linkType: hard