From 10327fb855105966dc469328819ea09b26f1635e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 6 Apr 2024 15:30:01 +0200 Subject: [PATCH 1/3] backend-app-api: deprecate plugin path configuration and fix /api/* 404 Signed-off-by: Patrik Oldsberg --- .changeset/chilled-pigs-march.md | 5 +++++ .../core-services/http-router.md | 18 +----------------- .../core-services/root-http-router.md | 4 ++++ .../httpRouter/httpRouterServiceFactory.ts | 7 +++++++ .../DefaultRootHttpRouter.test.ts | 16 ++++++++++++++++ .../rootHttpRouter/DefaultRootHttpRouter.ts | 6 ++++++ 6 files changed, 39 insertions(+), 17 deletions(-) create mode 100644 .changeset/chilled-pigs-march.md diff --git a/.changeset/chilled-pigs-march.md b/.changeset/chilled-pigs-march.md new file mode 100644 index 0000000000..dd50833324 --- /dev/null +++ b/.changeset/chilled-pigs-march.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +Deprecate the `getPath` option for the `httpRouterServiceFactory` and more generally the ability to configure plugin API paths to be anything else than `/api/:pluginId/`. Requests towards `/api/*` that do not match an installed plugin will also no longer be handled by the index router, typically instead returning a 404. diff --git a/docs/backend-system/core-services/http-router.md b/docs/backend-system/core-services/http-router.md index e976d2c06c..4dfa7d80e1 100644 --- a/docs/backend-system/core-services/http-router.md +++ b/docs/backend-system/core-services/http-router.md @@ -39,20 +39,4 @@ createBackendPlugin({ ## Configuring the service -There's additional configuration that you can optionally pass to setup the `httpRouter` core service. - -- `getPath` - Can be used to generate a path for each plugin. Currently defaults to `/api/${pluginId}` - -You can configure these additional options by adding an override for the core service when calling `createBackend` like follows: - -```ts -import { httpRouterServiceFactory } from '@backstage/backend-app-api'; - -const backend = createBackend(); - -backend.add( - httpRouterServiceFactory({ - getPath: (pluginId: string) => `/plugins/${pluginId}`, - }), -); -``` +This service does not have any configuration options. diff --git a/docs/backend-system/core-services/root-http-router.md b/docs/backend-system/core-services/root-http-router.md index 8b9de00507..84be48cf87 100644 --- a/docs/backend-system/core-services/root-http-router.md +++ b/docs/backend-system/core-services/root-http-router.md @@ -7,6 +7,8 @@ description: Documentation for the Root Http Router service The root HTTP router is a service that allows you to register routes on the root of the backend service. This is useful for things like health checks, or other routes that you want to expose on the root of the backend service. It is used as the base router that backs the `httpRouter` service. Most likely you won't need to use this service directly, but rather use the `httpRouter` service. +The `/api/:pluginId/` path prefix is reserved for use by plugins to register their own routes via the [HttpRouter](./http-router.md) service. + ## Using the service The following example shows how to get the root HTTP router service in your `example` backend plugin to register a health check route. @@ -74,3 +76,5 @@ backend.add( }), ); ``` + +Note that requests towards `/api/*` will always fall through the `routes` if there is no matching plugin route, and never be caught by the configured `indexPath`. diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts b/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts index 5b881dca4c..f8ccdde780 100644 --- a/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts +++ b/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts @@ -43,6 +43,7 @@ export const httpRouterServiceFactory = createServiceFactory( deps: { plugin: coreServices.pluginMetadata, config: coreServices.rootConfig, + logger: coreServices.logger, lifecycle: coreServices.lifecycle, rootHttpRouter: coreServices.rootHttpRouter, auth: coreServices.auth, @@ -52,10 +53,16 @@ export const httpRouterServiceFactory = createServiceFactory( auth, httpAuth, config, + logger, plugin, rootHttpRouter, lifecycle, }) { + if (options?.getPath) { + logger.warn( + `DEPRECATION WARNING: The 'getPath' option for HttpRouterService is deprecated. The ability to reconfigure the '/api/' path prefix for plugins will be removed in the future.`, + ); + } const getPath = options?.getPath ?? (id => `/api/${id}`); const path = getPath(plugin.getId()); diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/DefaultRootHttpRouter.test.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/DefaultRootHttpRouter.test.ts index 86c117f7e0..af8340c91e 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/DefaultRootHttpRouter.test.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/DefaultRootHttpRouter.test.ts @@ -89,4 +89,20 @@ describe('DefaultRootHttpRouter', () => { expect('test').toBe('test'); }); + + it('should treat unknown /api/ routes as 404', async () => { + const router = DefaultRootHttpRouter.create(); + const app = express(); + app.use(router.handler()); + + router.use('/api/app', (_req, res) => res.status(201).end()); + router.use('/api/catalog', (_req, res) => res.status(202).end()); + + await request(app).get('/').expect(201); + await request(app).get('/api/catalog').expect(202); + await request(app).get('/unknown').expect(201); + await request(app).get('/api/unknown').expect(404); + + expect('test').toBe('test'); + }); }); diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/DefaultRootHttpRouter.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/DefaultRootHttpRouter.ts index 0a144af494..9d7d1de120 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/DefaultRootHttpRouter.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/DefaultRootHttpRouter.ts @@ -66,6 +66,12 @@ export class DefaultRootHttpRouter implements RootHttpRouterService { private constructor(indexPath?: string) { this.#indexPath = indexPath; this.#router.use(this.#namedRoutes); + + // Any request with a /api/ prefix will skip the index router, even if no named router matches + this.#router.use('/api/', (_req, _res, next) => { + next('router'); + }); + if (this.#indexPath) { this.#router.use(this.#indexRouter); } From 7522a9e51e9e4ad0c2258911f23112960a2bbbed Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 8 Apr 2024 13:47:53 +0200 Subject: [PATCH 2/3] docs/backend-system: reword 404 handling in root router docs Signed-off-by: Patrik Oldsberg --- docs/backend-system/core-services/root-http-router.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/backend-system/core-services/root-http-router.md b/docs/backend-system/core-services/root-http-router.md index 84be48cf87..97666cae9d 100644 --- a/docs/backend-system/core-services/root-http-router.md +++ b/docs/backend-system/core-services/root-http-router.md @@ -77,4 +77,4 @@ backend.add( ); ``` -Note that requests towards `/api/*` will always fall through the `routes` if there is no matching plugin route, and never be caught by the configured `indexPath`. +Note that requests towards `/api/*` will never be handled by the `routes` handler unless a matching plugin exists, and will instead typically falling through to the `middleware.notFound()` handler. That is the case regardless of whether there is a configured `indexPath` or not. From 3b3d9730e897fdf1edeb5c917997a3406ef9f56b Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 8 Apr 2024 13:54:37 +0200 Subject: [PATCH 3/3] backend-app-api: added root router test for case without index path Signed-off-by: Patrik Oldsberg --- .../rootHttpRouter/DefaultRootHttpRouter.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/DefaultRootHttpRouter.test.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/DefaultRootHttpRouter.test.ts index af8340c91e..fc1de9b205 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/DefaultRootHttpRouter.test.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/DefaultRootHttpRouter.test.ts @@ -105,4 +105,20 @@ describe('DefaultRootHttpRouter', () => { expect('test').toBe('test'); }); + + it('should treat unknown /api/ routes as 404 without an index path', async () => { + const router = DefaultRootHttpRouter.create({ indexPath: false }); + const app = express(); + app.use(router.handler()); + + router.use('/api/app', (_req, res) => res.status(201).end()); + router.use('/api/catalog', (_req, res) => res.status(202).end()); + + await request(app).get('/').expect(404); + await request(app).get('/api/catalog').expect(202); + await request(app).get('/unknown').expect(404); + await request(app).get('/api/unknown').expect(404); + + expect('test').toBe('test'); + }); });