From e187e99b4702d7acf02dad9e84cca217b1b7982c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 22 May 2024 15:21:24 +0200 Subject: [PATCH 1/3] port proxy tests to msw2 to try to get rid of spurious build errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- plugins/proxy-backend/package.json | 2 +- .../src/service/router.config.test.ts | 15 ++++++--------- .../src/service/router.credentials.test.ts | 15 +++++++-------- yarn.lock | 2 +- 4 files changed, 15 insertions(+), 19 deletions(-) diff --git a/plugins/proxy-backend/package.json b/plugins/proxy-backend/package.json index d92dba9069..1ca6cbe5f5 100644 --- a/plugins/proxy-backend/package.json +++ b/plugins/proxy-backend/package.json @@ -74,7 +74,7 @@ "@types/supertest": "^2.0.8", "@types/uuid": "^9.0.0", "@types/yup": "^0.32.0", - "msw": "^1.0.0", + "msw": "^2.0.0", "node-fetch": "^2.6.7", "portfinder": "^1.0.32", "supertest": "^6.1.3" diff --git a/plugins/proxy-backend/src/service/router.config.test.ts b/plugins/proxy-backend/src/service/router.config.test.ts index 0de4787658..20aed5452a 100644 --- a/plugins/proxy-backend/src/service/router.config.test.ts +++ b/plugins/proxy-backend/src/service/router.config.test.ts @@ -24,7 +24,7 @@ import { StaticConfigSource, } from '@backstage/config-loader'; import express from 'express'; -import { rest } from 'msw'; +import { http, HttpResponse } from 'msw'; import { setupServer } from 'msw/node'; import request from 'supertest'; import { createRouter } from './router'; @@ -35,14 +35,11 @@ import { mockServices } from '@backstage/backend-test-utils'; 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(), - }), - ), + http.get('https://non-existing-example.com/', req => + HttpResponse.json({ + url: req.request.url.toString(), + headers: req.request.headers, + }), ), ); diff --git a/plugins/proxy-backend/src/service/router.credentials.test.ts b/plugins/proxy-backend/src/service/router.credentials.test.ts index 1496b6ffb6..85fad9f038 100644 --- a/plugins/proxy-backend/src/service/router.credentials.test.ts +++ b/plugins/proxy-backend/src/service/router.credentials.test.ts @@ -21,7 +21,7 @@ import { } from '@backstage/backend-test-utils'; import { ResponseError } from '@backstage/errors'; import { JsonObject } from '@backstage/types'; -import { rest } from 'msw'; +import { http, HttpResponse, passthrough } from 'msw'; import { setupServer } from 'msw/node'; import fetch from 'node-fetch'; import portFinder from 'portfinder'; @@ -82,13 +82,12 @@ describe('credentials', () => { }; worker.use( - rest.all(`${baseUrl}/*`, req => req.passthrough()), - rest.get('http://target.com/*', (req, res, ctx) => { - const auth = req.headers.get('authorization'); - return res( - ctx.status(200), - ctx.json({ payload: { forwardedAuthorization: auth ?? false } }), - ); + http.all(`${baseUrl}/*`, () => passthrough()), + http.get('http://target.com/*', req => { + const auth = req.request.headers.get('authorization'); + return HttpResponse.json({ + payload: { forwardedAuthorization: auth ?? false }, + }); }), ); diff --git a/yarn.lock b/yarn.lock index d006daa23c..a823457dce 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6471,7 +6471,7 @@ __metadata: express-promise-router: ^4.1.0 http-proxy-middleware: ^2.0.0 morgan: ^1.10.0 - msw: ^1.0.0 + msw: ^2.0.0 node-fetch: ^2.6.7 portfinder: ^1.0.32 supertest: ^6.1.3 From 539b1382afbb13b296d941d411b8b506d03abad3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 23 May 2024 09:49:53 +0200 Subject: [PATCH 2/3] improve the config test too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- plugins/proxy-backend/package.json | 4 +- .../src/service/router.config.test.ts | 111 ++++++++---------- .../src/service/router.credentials.test.ts | 4 +- yarn.lock | 2 - 4 files changed, 53 insertions(+), 68 deletions(-) diff --git a/plugins/proxy-backend/package.json b/plugins/proxy-backend/package.json index 1ca6cbe5f5..3d3ccfaa0d 100644 --- a/plugins/proxy-backend/package.json +++ b/plugins/proxy-backend/package.json @@ -71,13 +71,11 @@ "@backstage/config-loader": "workspace:^", "@backstage/errors": "workspace:^", "@types/http-proxy-middleware": "^1.0.0", - "@types/supertest": "^2.0.8", "@types/uuid": "^9.0.0", "@types/yup": "^0.32.0", "msw": "^2.0.0", "node-fetch": "^2.6.7", - "portfinder": "^1.0.32", - "supertest": "^6.1.3" + "portfinder": "^1.0.32" }, "configSchema": "config.d.ts" } diff --git a/plugins/proxy-backend/src/service/router.config.test.ts b/plugins/proxy-backend/src/service/router.config.test.ts index 20aed5452a..c868092612 100644 --- a/plugins/proxy-backend/src/service/router.config.test.ts +++ b/plugins/proxy-backend/src/service/router.config.test.ts @@ -14,51 +14,46 @@ * limitations under the License. */ +import { createBackend } from '@backstage/backend-defaults'; import { - HostDiscovery, - loggerToWinstonLogger, -} from '@backstage/backend-common'; + coreServices, + createServiceFactory, +} from '@backstage/backend-plugin-api'; +import { + mockServices, + setupRequestMockHandlers, +} from '@backstage/backend-test-utils'; import { ConfigSources, MutableConfigSource, StaticConfigSource, } from '@backstage/config-loader'; -import express from 'express'; -import { http, HttpResponse } from 'msw'; +import { HttpResponse, http, passthrough } from 'msw'; import { setupServer } from 'msw/node'; -import request from 'supertest'; -import { createRouter } from './router'; -import { mockServices } from '@backstage/backend-test-utils'; +import fetch from 'node-fetch'; +import portFinder from 'portfinder'; // this test is stored in its own file to work around the mocked // http-proxy-middleware module used in the main test file describe('createRouter reloadable configuration', () => { - const server = setupServer( - http.get('https://non-existing-example.com/', req => - HttpResponse.json({ - url: req.request.url.toString(), - headers: req.request.headers, - }), - ), - ); - - beforeAll(() => - server.listen({ - onUnhandledRequest: ({ headers }, print) => { - if (headers.get('User-Agent') === 'supertest') { - return; - } - print.error(); - }, - }), - ); - - afterAll(() => server.close()); - afterEach(() => server.resetHandlers()); + const server = setupServer(); + setupRequestMockHandlers(server); it('should be able to observe the config', async () => { - const logger = loggerToWinstonLogger(mockServices.logger.mock()); + const host = 'localhost'; + const port = await portFinder.getPortPromise({ host }); + const baseUrl = `http://${host}:${port}`; + + server.use( + http.all(`${baseUrl}/*`, passthrough), + http.get('https://non-existing-example.com/*', req => + HttpResponse.json({ + url: req.request.url.toString(), + headers: req.request.headers, + }), + ), + ); // Grab the subscriber function and use mutable config data to mock a config file change const mutableConfigSource = MutableConfigSource.create({ data: {} }); @@ -67,18 +62,14 @@ describe('createRouter reloadable configuration', () => { StaticConfigSource.create({ data: { backend: { - baseUrl: 'http://localhost:7007', - listen: { - port: 7007, - }, + baseUrl, + listen: { host, port }, }, proxy: { endpoints: { '/test': { target: 'https://non-existing-example.com', - pathRewrite: { - '.*': '/', - }, + credentials: 'dangerously-allow-unauthenticated', }, }, }, @@ -88,40 +79,38 @@ describe('createRouter reloadable configuration', () => { ]), ); - const discovery = HostDiscovery.fromConfig(config); - const router = await createRouter({ - config, - logger, - discovery, + const backend = createBackend(); + backend.add(import('../alpha')); + backend.add( + createServiceFactory({ + service: coreServices.rootConfig, + deps: {}, + factory: () => config, + }), + ); + backend.add(mockServices.rootLogger.factory()); + await backend.start(); + + await expect(fetch(`${baseUrl}/api/proxy/test`)).resolves.toMatchObject({ + status: 200, }); - 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); + await expect( + fetch(`${baseUrl}/api/proxy/test2`), + ).resolves.not.toMatchObject({ status: 200 }); mutableConfigSource.setData({ proxy: { endpoints: { '/test2': { target: 'https://non-existing-example.com', - pathRewrite: { - '.*': '/', - }, + credentials: 'dangerously-allow-unauthenticated', }, }, }, }); - const response2 = await agent.get('/test2'); - - expect(response2.status).toEqual(200); + await expect(fetch(`${baseUrl}/api/proxy/test2`)).resolves.toMatchObject({ + status: 200, + }); }); }); diff --git a/plugins/proxy-backend/src/service/router.credentials.test.ts b/plugins/proxy-backend/src/service/router.credentials.test.ts index 85fad9f038..53d0f157f3 100644 --- a/plugins/proxy-backend/src/service/router.credentials.test.ts +++ b/plugins/proxy-backend/src/service/router.credentials.test.ts @@ -35,7 +35,7 @@ describe('credentials', () => { it('handles all valid credentials settings', async () => { const host = 'localhost'; - const port = await portFinder.getPortPromise(); + const port = await portFinder.getPortPromise({ host }); const baseUrl = `http://${host}:${port}`; const config = { @@ -82,7 +82,7 @@ describe('credentials', () => { }; worker.use( - http.all(`${baseUrl}/*`, () => passthrough()), + http.all(`${baseUrl}/*`, passthrough), http.get('http://target.com/*', req => { const auth = req.request.headers.get('authorization'); return HttpResponse.json({ diff --git a/yarn.lock b/yarn.lock index a823457dce..3288ed9a2e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6464,7 +6464,6 @@ __metadata: "@backstage/types": "workspace:^" "@types/express": ^4.17.6 "@types/http-proxy-middleware": ^1.0.0 - "@types/supertest": ^2.0.8 "@types/uuid": ^9.0.0 "@types/yup": ^0.32.0 express: ^4.17.1 @@ -6474,7 +6473,6 @@ __metadata: msw: ^2.0.0 node-fetch: ^2.6.7 portfinder: ^1.0.32 - supertest: ^6.1.3 uuid: ^9.0.0 winston: ^3.2.1 yaml: ^2.0.0 From fdcaf5d93829f5a2f23fe756a9d1b22aa3563262 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 23 May 2024 11:37:47 +0200 Subject: [PATCH 3/3] move to startTestBackend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- plugins/proxy-backend/package.json | 4 +- .../src/service/router.config.test.ts | 93 +++++++++---------- .../src/service/router.credentials.test.ts | 83 ++++++++--------- yarn.lock | 2 +- 4 files changed, 87 insertions(+), 95 deletions(-) diff --git a/plugins/proxy-backend/package.json b/plugins/proxy-backend/package.json index 3d3ccfaa0d..af0d9d584e 100644 --- a/plugins/proxy-backend/package.json +++ b/plugins/proxy-backend/package.json @@ -65,6 +65,7 @@ "yup": "^1.0.0" }, "devDependencies": { + "@backstage/backend-app-api": "workspace:^", "@backstage/backend-defaults": "workspace:^", "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", @@ -74,8 +75,7 @@ "@types/uuid": "^9.0.0", "@types/yup": "^0.32.0", "msw": "^2.0.0", - "node-fetch": "^2.6.7", - "portfinder": "^1.0.32" + "node-fetch": "^2.6.7" }, "configSchema": "config.d.ts" } diff --git a/plugins/proxy-backend/src/service/router.config.test.ts b/plugins/proxy-backend/src/service/router.config.test.ts index c868092612..ba81553a4f 100644 --- a/plugins/proxy-backend/src/service/router.config.test.ts +++ b/plugins/proxy-backend/src/service/router.config.test.ts @@ -14,14 +14,13 @@ * limitations under the License. */ -import { createBackend } from '@backstage/backend-defaults'; import { coreServices, createServiceFactory, } from '@backstage/backend-plugin-api'; import { - mockServices, setupRequestMockHandlers, + startTestBackend, } from '@backstage/backend-test-utils'; import { ConfigSources, @@ -31,7 +30,6 @@ import { import { HttpResponse, http, passthrough } from 'msw'; import { setupServer } from 'msw/node'; import fetch from 'node-fetch'; -import portFinder from 'portfinder'; // this test is stored in its own file to work around the mocked // http-proxy-middleware module used in the main test file @@ -41,30 +39,12 @@ describe('createRouter reloadable configuration', () => { setupRequestMockHandlers(server); it('should be able to observe the config', async () => { - const host = 'localhost'; - const port = await portFinder.getPortPromise({ host }); - const baseUrl = `http://${host}:${port}`; - - server.use( - http.all(`${baseUrl}/*`, passthrough), - http.get('https://non-existing-example.com/*', req => - HttpResponse.json({ - url: req.request.url.toString(), - headers: req.request.headers, - }), - ), - ); - // Grab the subscriber function and use mutable config data to mock a config file change const mutableConfigSource = MutableConfigSource.create({ data: {} }); const config = await ConfigSources.toConfig( ConfigSources.merge([ StaticConfigSource.create({ data: { - backend: { - baseUrl, - listen: { host, port }, - }, proxy: { endpoints: { '/test': { @@ -79,38 +59,53 @@ describe('createRouter reloadable configuration', () => { ]), ); - const backend = createBackend(); - backend.add(import('../alpha')); - backend.add( - createServiceFactory({ - service: coreServices.rootConfig, - deps: {}, - factory: () => config, - }), - ); - backend.add(mockServices.rootLogger.factory()); - await backend.start(); - - await expect(fetch(`${baseUrl}/api/proxy/test`)).resolves.toMatchObject({ - status: 200, + const backend = await startTestBackend({ + features: [ + import('../alpha'), + createServiceFactory({ + service: coreServices.rootConfig, + deps: {}, + factory: () => config, + }), + ], }); - await expect( - fetch(`${baseUrl}/api/proxy/test2`), - ).resolves.not.toMatchObject({ status: 200 }); - mutableConfigSource.setData({ - proxy: { - endpoints: { - '/test2': { - target: 'https://non-existing-example.com', - credentials: 'dangerously-allow-unauthenticated', + try { + const baseUrl = `http://localhost:${backend.server.port()}`; + + server.use( + http.all(`${baseUrl}/*`, passthrough), + http.get('https://non-existing-example.com/*', req => + HttpResponse.json({ + url: req.request.url.toString(), + headers: req.request.headers, + }), + ), + ); + + await expect(fetch(`${baseUrl}/api/proxy/test`)).resolves.toMatchObject({ + status: 200, + }); + await expect( + fetch(`${baseUrl}/api/proxy/test2`), + ).resolves.not.toMatchObject({ status: 200 }); + + mutableConfigSource.setData({ + proxy: { + endpoints: { + '/test2': { + target: 'https://non-existing-example.com', + credentials: 'dangerously-allow-unauthenticated', + }, }, }, - }, - }); + }); - await expect(fetch(`${baseUrl}/api/proxy/test2`)).resolves.toMatchObject({ - status: 200, - }); + await expect(fetch(`${baseUrl}/api/proxy/test2`)).resolves.toMatchObject({ + status: 200, + }); + } finally { + await backend.stop(); + } }); }); diff --git a/plugins/proxy-backend/src/service/router.credentials.test.ts b/plugins/proxy-backend/src/service/router.credentials.test.ts index 53d0f157f3..54c865ba11 100644 --- a/plugins/proxy-backend/src/service/router.credentials.test.ts +++ b/plugins/proxy-backend/src/service/router.credentials.test.ts @@ -14,17 +14,20 @@ * limitations under the License. */ -import { createBackend } from '@backstage/backend-defaults'; +import { + authServiceFactory, + httpAuthServiceFactory, +} from '@backstage/backend-app-api'; import { mockServices, setupRequestMockHandlers, + startTestBackend, } from '@backstage/backend-test-utils'; import { ResponseError } from '@backstage/errors'; import { JsonObject } from '@backstage/types'; -import { http, HttpResponse, passthrough } from 'msw'; +import { HttpResponse, http, passthrough } from 'msw'; import { setupServer } from 'msw/node'; import fetch from 'node-fetch'; -import portFinder from 'portfinder'; // this test is stored in its own file to work around the mocked // http-proxy-middleware module used in the main test file @@ -34,14 +37,8 @@ describe('credentials', () => { setupRequestMockHandlers(worker); it('handles all valid credentials settings', async () => { - const host = 'localhost'; - const port = await portFinder.getPortPromise({ host }); - const baseUrl = `http://${host}:${port}`; - const config = { backend: { - baseUrl, - listen: { host, port }, auth: { externalAccess: [ { @@ -81,42 +78,42 @@ describe('credentials', () => { }, }; - worker.use( - http.all(`${baseUrl}/*`, passthrough), - http.get('http://target.com/*', req => { - const auth = req.request.headers.get('authorization'); - return HttpResponse.json({ - payload: { forwardedAuthorization: auth ?? false }, - }); - }), - ); - - async function call(options: { - endpoint: string; - authorization: string | false; - }): Promise { - const { endpoint, authorization } = options; - return fetch(`${baseUrl}/api/proxy/${endpoint}/just-some-path`, { - headers: authorization ? { Authorization: authorization } : {}, - }).then(async res => { - if (!res.ok) { - throw await ResponseError.fromResponse(res); - } - return res.json(); - }); - } - - // Create an actual backend instead of a test backend, because we want to - // use the real HTTP server that provides the protection middleware etc. A - // bit harder to test, but at least we can use static external access tokens - // for it. - const backend = createBackend(); - backend.add(import('../alpha')); - backend.add(mockServices.rootConfig.factory({ data: config })); - backend.add(mockServices.rootLogger.factory()); - await backend.start(); + const backend = await startTestBackend({ + features: [ + import('../alpha'), + mockServices.rootConfig.factory({ data: config }), + authServiceFactory(), + httpAuthServiceFactory(), + ], + }); try { + const baseUrl = `http://localhost:${backend.server.port()}`; + worker.use( + http.all(`${baseUrl}/*`, passthrough), + http.get('http://target.com/*', req => { + const auth = req.request.headers.get('authorization'); + return HttpResponse.json({ + payload: { forwardedAuthorization: auth ?? false }, + }); + }), + ); + + const call = async (options: { + endpoint: string; + authorization: string | false; + }): Promise => { + const { endpoint, authorization } = options; + return fetch(`${baseUrl}/api/proxy/${endpoint}/just-some-path`, { + headers: authorization ? { Authorization: authorization } : {}, + }).then(async res => { + if (!res.ok) { + throw await ResponseError.fromResponse(res); + } + return res.json(); + }); + }; + // simple credentials config await expect( call({ endpoint: 'simple', authorization: false }), diff --git a/yarn.lock b/yarn.lock index 3288ed9a2e..329a17a855 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6453,6 +6453,7 @@ __metadata: version: 0.0.0-use.local resolution: "@backstage/plugin-proxy-backend@workspace:plugins/proxy-backend" dependencies: + "@backstage/backend-app-api": "workspace:^" "@backstage/backend-common": "workspace:^" "@backstage/backend-defaults": "workspace:^" "@backstage/backend-plugin-api": "workspace:^" @@ -6472,7 +6473,6 @@ __metadata: morgan: ^1.10.0 msw: ^2.0.0 node-fetch: ^2.6.7 - portfinder: ^1.0.32 uuid: ^9.0.0 winston: ^3.2.1 yaml: ^2.0.0