From 3256f14401234784d0ce482a8ee7f78cdde6eec0 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 2 Apr 2024 15:01:52 +0200 Subject: [PATCH] backend-app-api: stop loading modules if parent plugin is not present Signed-off-by: Patrik Oldsberg --- .changeset/lazy-rats-fail.md | 5 +++ .changeset/pink-mirrors-jam.md | 5 +++ .../src/wiring/BackendInitializer.test.ts | 19 ++++++++- .../src/wiring/BackendInitializer.ts | 4 +- .../src/next/wiring/TestBackend.ts | 41 ++++++++++++++++++- 5 files changed, 68 insertions(+), 6 deletions(-) create mode 100644 .changeset/lazy-rats-fail.md create mode 100644 .changeset/pink-mirrors-jam.md diff --git a/.changeset/lazy-rats-fail.md b/.changeset/lazy-rats-fail.md new file mode 100644 index 0000000000..4b5719477c --- /dev/null +++ b/.changeset/lazy-rats-fail.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': minor +--- + +**BREAKING**: Modules are no longer loaded unless the plugin that they extend is present. diff --git a/.changeset/pink-mirrors-jam.md b/.changeset/pink-mirrors-jam.md new file mode 100644 index 0000000000..7ad8d4bb72 --- /dev/null +++ b/.changeset/pink-mirrors-jam.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-test-utils': patch +--- + +`startTestBackend` will now add placeholder plugins when a modules are provided without their parent plugin. diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts index c025d19fa7..9d08d6dd8f 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts @@ -60,6 +60,16 @@ const baseFactories = [ loggerServiceFactory(), ]; +const testPlugin = createBackendPlugin({ + pluginId: 'test', + register(reg) { + reg.registerInit({ + deps: {}, + async init() {}, + }); + }, +})(); + describe('BackendInitializer', () => { it('should initialize root scoped services', async () => { const rootFactory = jest.fn(); @@ -99,6 +109,7 @@ describe('BackendInitializer', () => { }); const init = new BackendInitializer(baseFactories); + init.add(testPlugin); init.add( createBackendModule({ pluginId: 'test', @@ -172,6 +183,7 @@ describe('BackendInitializer', () => { it('should forward errors when modules fail to start', async () => { const init = new BackendInitializer([]); + init.add(testPlugin); init.add( createBackendModule({ pluginId: 'test', @@ -222,6 +234,7 @@ describe('BackendInitializer', () => { it('should reject duplicate modules', async () => { const init = new BackendInitializer([]); + init.add(testPlugin); init.add( createBackendModule({ pluginId: 'test', @@ -262,6 +275,7 @@ describe('BackendInitializer', () => { factory: () => new MockLogger(), })(), ]); + init.add(testPlugin); init.add( createBackendModule({ pluginId: 'test', @@ -308,9 +322,10 @@ describe('BackendInitializer', () => { }, })(), ); + init.add(testPlugin); init.add( createBackendModule({ - pluginId: 'test-b', + pluginId: 'test', moduleId: 'mod', register(reg) { reg.registerInit({ @@ -321,7 +336,7 @@ describe('BackendInitializer', () => { })(), ); await expect(init.start()).rejects.toThrow( - "Illegal dependency: Module 'mod' for plugin 'test-b' attempted to depend on extension point 'a' for plugin 'test-a'. Extension points can only be used within their plugin's scope.", + "Illegal dependency: Module 'mod' for plugin 'test' attempted to depend on extension point 'a' for plugin 'test-a'. Extension points can only be used within their plugin's scope.", ); }); }); diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index 2da3a5a226..f85cbc4932 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.ts @@ -228,9 +228,7 @@ export class BackendInitializer { } } - const allPluginIds = [ - ...new Set([...pluginInits.keys(), ...moduleInits.keys()]), - ]; + const allPluginIds = [...pluginInits.keys()]; // All plugins are initialized in parallel await Promise.all( diff --git a/packages/backend-test-utils/src/next/wiring/TestBackend.ts b/packages/backend-test-utils/src/next/wiring/TestBackend.ts index 6b58ebc5ea..10041dce4f 100644 --- a/packages/backend-test-utils/src/next/wiring/TestBackend.ts +++ b/packages/backend-test-utils/src/next/wiring/TestBackend.ts @@ -29,6 +29,7 @@ import { ExtensionPoint, coreServices, createBackendModule, + createBackendPlugin, } from '@backstage/backend-plugin-api'; import { mockServices } from '../services'; import { ConfigReader } from '@backstage/config'; @@ -84,6 +85,42 @@ export const defaultServiceFactories = [ mockServices.urlReader.factory(), ]; +/** + * Given a set of features, return an array of plugins that ensures that each + * module in the provided set of features has a corresponding plugin. + * @internal + */ +function createPluginsForOrphanModules(features: Array) { + const pluginIds = new Set(); + const modulePluginIds = new Set(); + + for (const feature of features) { + if (isInternalBackendFeature(feature)) { + const registrations = feature.getRegistrations(); + for (const registration of registrations) { + if (registration.type === 'plugin') { + pluginIds.add(registration.pluginId); + } else if (registration.type === 'module') { + modulePluginIds.add(registration.pluginId); + } + } + } + } + + for (const pluginId of pluginIds) { + modulePluginIds.delete(pluginId); + } + + return Array.from(modulePluginIds).map(pluginId => + createBackendPlugin({ + pluginId, + register(reg) { + reg.registerInit({ deps: {}, async init() {} }); + }, + }), + ); +} + /** * Given a set of extension points and features, find the extension * points that we mock and tie them to the correct plugin ID. @@ -277,7 +314,9 @@ export async function startTestBackend( for (const m of createExtensionPointTestModules(features, extensionPoints)) { backend.add(m); } - + for (const p of createPluginsForOrphanModules(features)) { + backend.add(p); + } for (const feature of features) { backend.add(feature); }