From 124520078c8ee3e78486d715b93f095a7710b522 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 28 Jan 2022 09:49:55 +0000 Subject: [PATCH 1/5] allow backend to start if proxy target is not set Signed-off-by: Brian Fletcher --- .../proxy-backend/src/service/router.test.ts | 71 +++++++++++++++---- plugins/proxy-backend/src/service/router.ts | 13 ++-- 2 files changed, 66 insertions(+), 18 deletions(-) diff --git a/plugins/proxy-backend/src/service/router.test.ts b/plugins/proxy-backend/src/service/router.test.ts index ddb524d3aa..f27a8cf3ae 100644 --- a/plugins/proxy-backend/src/service/router.test.ts +++ b/plugins/proxy-backend/src/service/router.test.ts @@ -30,23 +30,66 @@ const mockCreateProxyMiddleware = createProxyMiddleware as jest.MockedFunction< >; describe('createRouter', () => { - it('works', async () => { - const logger = getVoidLogger(); - const config = new ConfigReader({ - backend: { - baseUrl: 'https://example.com:7007', - listen: { - port: 7007, + describe('where all proxy config are valid', () => { + it('works', async () => { + const logger = getVoidLogger(); + const config = new ConfigReader({ + backend: { + baseUrl: 'https://example.com:7007', + listen: { + port: 7007, + }, }, - }, + proxy: { + '/test': { + target: 'https://example.com', + headers: { + Authorization: 'Bearer supersecret', + }, + }, + }, + }); + const discovery = SingleHostDiscovery.fromConfig(config); + const router = await createRouter({ + config, + logger, + discovery, + }); + expect(router).toBeDefined(); }); - const discovery = SingleHostDiscovery.fromConfig(config); - const router = await createRouter({ - config, - logger, - discovery, + }); + + describe('where buildMiddleware would fail', () => { + it('works', async () => { + const logger = getVoidLogger(); + logger.warn = jest.fn(); + const config = new ConfigReader({ + backend: { + baseUrl: 'https://example.com:7007', + listen: { + port: 7007, + }, + }, + // no target would cause the buildMiddleware to fail + proxy: { + '/test': { + headers: { + Authorization: 'Bearer supersecret', + }, + }, + }, + }); + const discovery = SingleHostDiscovery.fromConfig(config); + const router = await createRouter({ + config, + logger, + discovery, + }); + expect((logger.warn as jest.Mock).mock.calls[0][0]).toEqual( + 'skipped configuring /test due to Proxy target must be a string', + ); + expect(router).toBeDefined(); }); - expect(router).toBeDefined(); }); }); diff --git a/plugins/proxy-backend/src/service/router.ts b/plugins/proxy-backend/src/service/router.ts index c2123ae4b1..f9ce9943f5 100644 --- a/plugins/proxy-backend/src/service/router.ts +++ b/plugins/proxy-backend/src/service/router.ts @@ -185,11 +185,16 @@ export async function createRouter( const { pathname: pathPrefix } = new URL(externalUrl); const proxyConfig = options.config.getOptional('proxy') ?? {}; + Object.entries(proxyConfig).forEach(([route, proxyRouteConfig]) => { - router.use( - route, - buildMiddleware(pathPrefix, options.logger, route, proxyRouteConfig), - ); + try { + router.use( + route, + buildMiddleware(pathPrefix, options.logger, route, proxyRouteConfig), + ); + } catch (e) { + options.logger.warn(`skipped configuring ${route} due to ${e.message}`); + } }); return router; From 332d3decb20de2127caed1f76852b9763e045d54 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 28 Jan 2022 09:53:38 +0000 Subject: [PATCH 2/5] adds changeset Signed-off-by: Brian Fletcher --- .changeset/quick-jars-wait.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/quick-jars-wait.md diff --git a/.changeset/quick-jars-wait.md b/.changeset/quick-jars-wait.md new file mode 100644 index 0000000000..c6299760be --- /dev/null +++ b/.changeset/quick-jars-wait.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-proxy-backend': patch +--- + +If one of the configured proxies is configured badly enough to cause an exception when the middleware is being built it will now pass by it and continue the server startup. Previously the backend would fail to startup. From 8030dc7dd5bc8fc55e87ed62083d3fce7da6b555 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 28 Jan 2022 11:28:14 +0000 Subject: [PATCH 3/5] only skip proxies if configured to do so Signed-off-by: Brian Fletcher --- .../proxy-backend/src/service/router.test.ts | 32 ++++++++++++++++++- plugins/proxy-backend/src/service/router.ts | 7 +++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/plugins/proxy-backend/src/service/router.test.ts b/plugins/proxy-backend/src/service/router.test.ts index f27a8cf3ae..6b4d6b21a7 100644 --- a/plugins/proxy-backend/src/service/router.test.ts +++ b/plugins/proxy-backend/src/service/router.test.ts @@ -60,7 +60,36 @@ describe('createRouter', () => { }); describe('where buildMiddleware would fail', () => { - it('works', async () => { + it('throws an error if skip failures is not set', async () => { + const logger = getVoidLogger(); + logger.warn = jest.fn(); + const config = new ConfigReader({ + backend: { + baseUrl: 'https://example.com:7007', + listen: { + port: 7007, + }, + }, + // no target would cause the buildMiddleware to fail + proxy: { + '/test': { + headers: { + Authorization: 'Bearer supersecret', + }, + }, + }, + }); + const discovery = SingleHostDiscovery.fromConfig(config); + await expect( + createRouter({ + config, + logger, + discovery, + }), + ).rejects.toThrow(new Error('Proxy target must be a string')); + }); + + it('works if skip failures is set', async () => { const logger = getVoidLogger(); logger.warn = jest.fn(); const config = new ConfigReader({ @@ -84,6 +113,7 @@ describe('createRouter', () => { config, logger, discovery, + skipBrokenProxies: true, }); expect((logger.warn as jest.Mock).mock.calls[0][0]).toEqual( 'skipped configuring /test due to Proxy target must be a string', diff --git a/plugins/proxy-backend/src/service/router.ts b/plugins/proxy-backend/src/service/router.ts index f9ce9943f5..7ea59d74eb 100644 --- a/plugins/proxy-backend/src/service/router.ts +++ b/plugins/proxy-backend/src/service/router.ts @@ -51,6 +51,7 @@ export interface RouterOptions { logger: Logger; config: Config; discovery: PluginEndpointDiscovery; + skipBrokenProxies?: boolean; } export interface ProxyConfig extends Options { @@ -193,7 +194,11 @@ export async function createRouter( buildMiddleware(pathPrefix, options.logger, route, proxyRouteConfig), ); } catch (e) { - options.logger.warn(`skipped configuring ${route} due to ${e.message}`); + if (options.skipBrokenProxies) { + options.logger.warn(`skipped configuring ${route} due to ${e.message}`); + } else { + throw e; + } } }); From 947f1098ee24334c921ee25e568f42d84709eca7 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 28 Jan 2022 11:31:09 +0000 Subject: [PATCH 4/5] adds details to the changeset Signed-off-by: Brian Fletcher --- .changeset/quick-jars-wait.md | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/.changeset/quick-jars-wait.md b/.changeset/quick-jars-wait.md index c6299760be..61bde0d377 100644 --- a/.changeset/quick-jars-wait.md +++ b/.changeset/quick-jars-wait.md @@ -2,4 +2,25 @@ '@backstage/plugin-proxy-backend': patch --- -If one of the configured proxies is configured badly enough to cause an exception when the middleware is being built it will now pass by it and continue the server startup. Previously the backend would fail to startup. +If one of the configured proxies is configured badly enough to cause an exception when the middleware is being built it can now be configured to pass the bad proxy by and continue the server startup. Previously the backend would fail to startup. + +To configure it to pass by failing proxies: + +``` +const router = await createRouter({ + config, + logger, + discovery, + skipBrokenProxies: true, +}); +``` + +If you would like it to fail if a proxy is configured badly: + +``` +const router = await createRouter({ + config, + logger, + discovery, +}); +``` From 27e1ab0ec4222dceaa054b8e54e9a85181dc68f5 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 28 Jan 2022 13:51:47 +0000 Subject: [PATCH 5/5] addressing review suggestions Signed-off-by: Brian Fletcher --- .changeset/quick-jars-wait.md | 4 ++-- plugins/proxy-backend/src/service/router.test.ts | 2 +- plugins/proxy-backend/src/service/router.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.changeset/quick-jars-wait.md b/.changeset/quick-jars-wait.md index 61bde0d377..bbf6b42a89 100644 --- a/.changeset/quick-jars-wait.md +++ b/.changeset/quick-jars-wait.md @@ -2,7 +2,7 @@ '@backstage/plugin-proxy-backend': patch --- -If one of the configured proxies is configured badly enough to cause an exception when the middleware is being built it can now be configured to pass the bad proxy by and continue the server startup. Previously the backend would fail to startup. +Adds a new option `skipInvalidTargets` for the proxy `createRouter` which allows the proxy backend to be started with an invalid proxy configuration. If configured, it will simply skip the failed proxy and mount the other valid proxies. To configure it to pass by failing proxies: @@ -11,7 +11,7 @@ const router = await createRouter({ config, logger, discovery, - skipBrokenProxies: true, + skipInvalidProxies: true, }); ``` diff --git a/plugins/proxy-backend/src/service/router.test.ts b/plugins/proxy-backend/src/service/router.test.ts index 6b4d6b21a7..3bfcc889f4 100644 --- a/plugins/proxy-backend/src/service/router.test.ts +++ b/plugins/proxy-backend/src/service/router.test.ts @@ -113,7 +113,7 @@ describe('createRouter', () => { config, logger, discovery, - skipBrokenProxies: true, + skipInvalidProxies: true, }); expect((logger.warn as jest.Mock).mock.calls[0][0]).toEqual( 'skipped configuring /test due to Proxy target must be a string', diff --git a/plugins/proxy-backend/src/service/router.ts b/plugins/proxy-backend/src/service/router.ts index 7ea59d74eb..ca2c7870d8 100644 --- a/plugins/proxy-backend/src/service/router.ts +++ b/plugins/proxy-backend/src/service/router.ts @@ -51,7 +51,7 @@ export interface RouterOptions { logger: Logger; config: Config; discovery: PluginEndpointDiscovery; - skipBrokenProxies?: boolean; + skipInvalidProxies?: boolean; } export interface ProxyConfig extends Options { @@ -194,7 +194,7 @@ export async function createRouter( buildMiddleware(pathPrefix, options.logger, route, proxyRouteConfig), ); } catch (e) { - if (options.skipBrokenProxies) { + if (options.skipInvalidProxies) { options.logger.warn(`skipped configuring ${route} due to ${e.message}`); } else { throw e;