From a4fa1ce09033eea3b2f74120706acd43a992f81d Mon Sep 17 00:00:00 2001 From: Crevil Date: Sat, 2 Jul 2022 09:24:33 +0200 Subject: [PATCH 1/3] Add config hot reloading to proxy-backend When working with the proxy configuration or plugins facilitating the proxy package the workflow is currently somewhat cumbersom as changes to the proxy configuration requires a full restart of the backend server. Use cases where the proxy configuration is updated is when testing out new routes for plugins to use and updating header configuration like authorization tokens. This change updates the proxy-backend to subscribe to configuration changes and update the router when changes to the proxy is made. Signed-off-by: Crevil --- .changeset/mean-adults-argue.md | 5 ++ .../proxy-backend/src/service/router.test.ts | 49 +++++++++++++++++++ plugins/proxy-backend/src/service/router.ts | 30 ++++++++++-- 3 files changed, 81 insertions(+), 3 deletions(-) create mode 100644 .changeset/mean-adults-argue.md diff --git a/.changeset/mean-adults-argue.md b/.changeset/mean-adults-argue.md new file mode 100644 index 0000000000..2ba26f165d --- /dev/null +++ b/.changeset/mean-adults-argue.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-proxy-backend': minor +--- + +The proxy-backend now automatically reloads configuration when app-config.yaml is updated. diff --git a/plugins/proxy-backend/src/service/router.test.ts b/plugins/proxy-backend/src/service/router.test.ts index 3bfcc889f4..d3f7718c58 100644 --- a/plugins/proxy-backend/src/service/router.test.ts +++ b/plugins/proxy-backend/src/service/router.test.ts @@ -57,6 +57,55 @@ 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 ca2c7870d8..3a6323eb2f 100644 --- a/plugins/proxy-backend/src/service/router.ts +++ b/plugins/proxy-backend/src/service/router.ts @@ -186,8 +186,34 @@ export async function createRouter( const { pathname: pathPrefix } = new URL(externalUrl); const proxyConfig = options.config.getOptional('proxy') ?? {}; + configureMiddlewares(options, router, pathPrefix, proxyConfig); - Object.entries(proxyConfig).forEach(([route, proxyRouteConfig]) => { + if (options.config.subscribe) { + let currentKey = JSON.stringify(proxyConfig); + + options.config.subscribe(() => { + const newProxyConfig = options.config.getOptional('proxy') ?? {}; + const newKey = JSON.stringify(newProxyConfig); + + if (currentKey !== newKey) { + currentKey = newKey; + + router.stack = []; + configureMiddlewares(options, router, pathPrefix, newProxyConfig); + } + }); + } + + return router; +} + +function configureMiddlewares( + options: RouterOptions, + router: express.Router, + pathPrefix: string, + proxyConfig: any, +) { + Object.entries(proxyConfig).forEach(([route, proxyRouteConfig]) => { try { router.use( route, @@ -201,6 +227,4 @@ export async function createRouter( } } }); - - return router; } From cd720cbfa3d5238784f8bc0e03f570a71f305280 Mon Sep 17 00:00:00 2001 From: Crevil Date: Mon, 4 Jul 2022 14:39:46 +0200 Subject: [PATCH 2/3] Changeset is a patch Signed-off-by: Crevil --- .changeset/mean-adults-argue.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/mean-adults-argue.md b/.changeset/mean-adults-argue.md index 2ba26f165d..22eacab054 100644 --- a/.changeset/mean-adults-argue.md +++ b/.changeset/mean-adults-argue.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-proxy-backend': minor +'@backstage/plugin-proxy-backend': patch --- The proxy-backend now automatically reloads configuration when app-config.yaml is updated. From dd48ca1bba3dbcabe6cc5435d655cad601899700 Mon Sep 17 00:00:00 2001 From: Crevil Date: Fri, 8 Jul 2022 21:05:50 +0200 Subject: [PATCH 3/3] 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, + ); } }); }