From dd48ca1bba3dbcabe6cc5435d655cad601899700 Mon Sep 17 00:00:00 2001 From: Crevil Date: Fri, 8 Jul 2022 21:05:50 +0200 Subject: [PATCH] Replace router instead of modifying it Signed-off-by: Crevil --- plugins/proxy-backend/package.json | 1 + .../src/service/router.config.test.ts | 115 ++++++++++++++++++ .../proxy-backend/src/service/router.test.ts | 49 -------- plugins/proxy-backend/src/service/router.ts | 14 ++- 4 files changed, 126 insertions(+), 53 deletions(-) create mode 100644 plugins/proxy-backend/src/service/router.config.test.ts diff --git a/plugins/proxy-backend/package.json b/plugins/proxy-backend/package.json index 7eba901c54..c90c983597 100644 --- a/plugins/proxy-backend/package.json +++ b/plugins/proxy-backend/package.json @@ -51,6 +51,7 @@ "@types/supertest": "^2.0.8", "@types/uuid": "^8.0.0", "@types/yup": "^0.29.13", + "msw": "^0.42.0", "supertest": "^6.1.3" }, "files": [ diff --git a/plugins/proxy-backend/src/service/router.config.test.ts b/plugins/proxy-backend/src/service/router.config.test.ts new file mode 100644 index 0000000000..5f90f847e4 --- /dev/null +++ b/plugins/proxy-backend/src/service/router.config.test.ts @@ -0,0 +1,115 @@ +/* + * Copyright 2022 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 { getVoidLogger, SingleHostDiscovery } from '@backstage/backend-common'; +import { ConfigReader } from '@backstage/config'; +import express from 'express'; +import { rest } from 'msw'; +import { setupServer } from 'msw/node'; +import request from 'supertest'; +import { createRouter } from './router'; + +// this test is stored in its own file to work around the mocked +// http-proxy-middleware module used in the rest of the tests + +describe('createRouter reloadable configuration', () => { + const server = setupServer( + rest.get('https://non-existing-example.com/', (req, res, ctx) => + res( + ctx.status(200), + ctx.json({ + url: req.url.toString(), + headers: req.headers.all(), + }), + ), + ), + ); + + beforeAll(() => + server.listen({ + onUnhandledRequest: ({ headers }, print) => { + if (headers.get('User-Agent') === 'supertest') { + return; + } + print.error(); + }, + }), + ); + + afterAll(() => server.close()); + afterEach(() => server.resetHandlers()); + + it('should be able to observe the config', async () => { + const logger = getVoidLogger(); + + // Grab the subscriber function and use mutable config data to mock a config file change + let subscriber: () => void; + const mutableConfigData: any = { + backend: { + baseUrl: 'http://localhost:7007', + listen: { + port: 7007, + }, + }, + proxy: { + '/test': { + target: 'https://non-existing-example.com', + pathRewrite: { + '.*': '/', + }, + }, + }, + }; + + const mockConfig = Object.assign(new ConfigReader(mutableConfigData), { + subscribe: (s: () => void) => { + subscriber = s; + return { unsubscribe: () => {} }; + }, + }); + + const discovery = SingleHostDiscovery.fromConfig(mockConfig); + const router = await createRouter({ + config: mockConfig, + logger, + discovery, + }); + expect(router).toBeDefined(); + + const app = express(); + app.use(router); + + const agent = request.agent(app); + // this is set to let msw pass test requests through the mock server + agent.set('User-Agent', 'supertest'); + + const response1 = await agent.get('/test'); + + expect(response1.status).toEqual(200); + + mutableConfigData.proxy['/test2'] = { + target: 'https://non-existing-example.com', + pathRewrite: { + '.*': '/', + }, + }; + subscriber!(); + + const response2 = await agent.get('/test2'); + + expect(response2.status).toEqual(200); + }); +}); diff --git a/plugins/proxy-backend/src/service/router.test.ts b/plugins/proxy-backend/src/service/router.test.ts index d3f7718c58..3bfcc889f4 100644 --- a/plugins/proxy-backend/src/service/router.test.ts +++ b/plugins/proxy-backend/src/service/router.test.ts @@ -57,55 +57,6 @@ describe('createRouter', () => { }); expect(router).toBeDefined(); }); - - it('should be able to observe the config', async () => { - const logger = getVoidLogger(); - - // Grab the subscriber function and use mutable config data to mock a config file change - let subscriber: () => void; - const mutableConfigData: any = { - backend: { - baseUrl: 'https://example.com:7007', - listen: { - port: 7007, - }, - }, - proxy: { - '/test': { - target: 'https://example.com', - headers: { - Authorization: 'Bearer supersecret', - }, - }, - }, - }; - - const mockConfig = Object.assign(new ConfigReader(mutableConfigData), { - subscribe: (s: () => void) => { - subscriber = s; - return { unsubscribe: () => {} }; - }, - }); - - const discovery = SingleHostDiscovery.fromConfig(mockConfig); - const router = await createRouter({ - config: mockConfig, - logger, - discovery, - }); - expect(router).toBeDefined(); - - expect(router.stack[0].regexp).toEqual(/^\/test\/?(?=\/|$)/i); - expect(router.stack[1]).toBeUndefined(); - - mutableConfigData.proxy['/test2'] = { - target: 'https://example.com', - }; - subscriber!(); - - expect(router.stack[0].regexp).toEqual(/^\/test\/?(?=\/|$)/i); - expect(router.stack[1].regexp).toEqual(/^\/test2\/?(?=\/|$)/i); - }); }); describe('where buildMiddleware would fail', () => { diff --git a/plugins/proxy-backend/src/service/router.ts b/plugins/proxy-backend/src/service/router.ts index 3a6323eb2f..e0910da145 100644 --- a/plugins/proxy-backend/src/service/router.ts +++ b/plugins/proxy-backend/src/service/router.ts @@ -181,12 +181,14 @@ export async function createRouter( options: RouterOptions, ): Promise { const router = Router(); + let currentRouter = Router(); const externalUrl = await options.discovery.getExternalBaseUrl('proxy'); const { pathname: pathPrefix } = new URL(externalUrl); const proxyConfig = options.config.getOptional('proxy') ?? {}; - configureMiddlewares(options, router, pathPrefix, proxyConfig); + configureMiddlewares(options, currentRouter, pathPrefix, proxyConfig); + router.use((...args) => currentRouter(...args)); if (options.config.subscribe) { let currentKey = JSON.stringify(proxyConfig); @@ -197,9 +199,13 @@ export async function createRouter( if (currentKey !== newKey) { currentKey = newKey; - - router.stack = []; - configureMiddlewares(options, router, pathPrefix, newProxyConfig); + currentRouter = Router(); + configureMiddlewares( + options, + currentRouter, + pathPrefix, + newProxyConfig, + ); } }); }