From 8593dfa13a644c67b9d1b73c9ad01b3b6be345fa Mon Sep 17 00:00:00 2001 From: David Festal Date: Tue, 15 Oct 2024 13:06:42 +0200 Subject: [PATCH] refactor(backend-dynamic-feature-service): Improve alpha package support Signed-off-by: David Festal --- .changeset/warm-tomatoes-develop.md | 7 ++ .../report.api.md | 2 + .../alpha/package.json | 5 ++ .../dist/alpha.cjs.js | 23 +++++ .../dist/index.cjs.js | 3 + .../test-backend-alpha-dynamic/package.json | 36 ++++++++ .../src/features/features.test.ts | 72 ++++++++++++++++ .../src/manager/plugin-manager.test.ts | 80 +++++++++++++++++- .../src/manager/plugin-manager.ts | 22 +++-- .../src/scanner/plugin-scanner.test.ts | 11 ++- .../src/scanner/plugin-scanner.ts | 84 ++++++++++--------- .../src/scanner/types.ts | 1 + .../src/schemas/schemas.ts | 5 +- 13 files changed, 290 insertions(+), 61 deletions(-) create mode 100644 .changeset/warm-tomatoes-develop.md create mode 100644 packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/alpha/package.json create mode 100644 packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/dist/alpha.cjs.js create mode 100644 packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/dist/index.cjs.js create mode 100644 packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/package.json diff --git a/.changeset/warm-tomatoes-develop.md b/.changeset/warm-tomatoes-develop.md new file mode 100644 index 0000000000..100692f612 --- /dev/null +++ b/.changeset/warm-tomatoes-develop.md @@ -0,0 +1,7 @@ +--- +'@backstage/backend-dynamic-feature-service': patch +--- + +Improve the way alpha packages are supported when loading dynamic backend plugins. +The `ScannedPluginPackage` descriptor of dynamic backend plugins loaded from their alpha `package.json` now contain both the main package manifest and the alpha manifest. Previously it used to contain only the content of the alpha `package.json`, which is nearly empty. +This will make it easier to use or display metadata of loaded dynamic backend plugins, which is contained in the main manifest. diff --git a/packages/backend-dynamic-feature-service/report.api.md b/packages/backend-dynamic-feature-service/report.api.md index 7003b3bc2c..97e1f0dde9 100644 --- a/packages/backend-dynamic-feature-service/report.api.md +++ b/packages/backend-dynamic-feature-service/report.api.md @@ -287,6 +287,8 @@ export type ScannedPluginManifest = BackstagePackageJson & // @public (undocumented) export interface ScannedPluginPackage { + // (undocumented) + alphaManifest?: BackstagePackageJson; // (undocumented) location: URL; // (undocumented) diff --git a/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/alpha/package.json b/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/alpha/package.json new file mode 100644 index 0000000000..0b10e92f3b --- /dev/null +++ b/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/alpha/package.json @@ -0,0 +1,5 @@ +{ + "name": "plugin-test-backend-alpha-dynamic__alpha", + "version": "0.0.0", + "main": "../dist/alpha.cjs.js" +} diff --git a/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/dist/alpha.cjs.js b/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/dist/alpha.cjs.js new file mode 100644 index 0000000000..1b4cb79d30 --- /dev/null +++ b/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/dist/alpha.cjs.js @@ -0,0 +1,23 @@ +'use strict'; + +Object.defineProperty(exports, '__esModule', { value: true }); + +var backendPluginApi = require('@backstage/backend-plugin-api'); + +const testAlphaPlugin = backendPluginApi.createBackendPlugin({ + pluginId: "test-alpha", + register(env) { + env.registerInit({ + deps: { + logger: backendPluginApi.coreServices.rootLogger, + }, + async init({ + logger, + }) { + logger.info("This plugin has been loaded from the alpha package."); + } + }); + } +}); + +exports.default = testAlphaPlugin; diff --git a/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/dist/index.cjs.js b/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/dist/index.cjs.js new file mode 100644 index 0000000000..cf5c9078e2 --- /dev/null +++ b/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/dist/index.cjs.js @@ -0,0 +1,3 @@ +'use strict'; + +Object.defineProperty(exports, '__esModule', { value: true }); diff --git a/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/package.json b/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/package.json new file mode 100644 index 0000000000..90cee28b0f --- /dev/null +++ b/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root-for-alpha/test-backend-alpha-dynamic/package.json @@ -0,0 +1,36 @@ +{ + "name": "plugin-test-backend-alpha-dynamic", + "version": "0.0.0", + "description": "A test dynamic backend module that exposes alpha API.", + "backstage": { + "role": "backend-plugin", + "pluginId": "test-alpha", + "pluginPackages": [ + "plugin-test-backend-alpha" + ] + }, + "publishConfig": { + "access": "public" + }, + "keywords": [ + "backstage", + "dynamic" + ], + "exports": { + ".": { + "require": "./dist/index.cjs.js", + "default": "./dist/index.cjs.js" + }, + "./alpha": { + "require": "./dist/alpha.cjs.js", + "default": "./dist/alpha.cjs.js" + }, + "./package.json": "./package.json" + }, + "main": "./dist/index.cjs.js", + "files": [ + "dist", + "alpha" + ], + "bundleDependencies": true +} diff --git a/packages/backend-dynamic-feature-service/src/features/features.test.ts b/packages/backend-dynamic-feature-service/src/features/features.test.ts index ad46b0c5f8..fe32d61d52 100644 --- a/packages/backend-dynamic-feature-service/src/features/features.test.ts +++ b/packages/backend-dynamic-feature-service/src/features/features.test.ts @@ -29,8 +29,10 @@ import { } from '@backstage/backend-plugin-api'; import { CommonJSModuleLoader } from '../loader/CommonJSModuleLoader'; import * as winston from 'winston'; +import * as url from 'url'; import { MESSAGE } from 'triple-beam'; import { overridePackagePathResolution } from '@backstage/backend-plugin-api/testUtils'; +import { ScannedPluginPackage } from '../scanner'; // these can get a bit slow in CI jest.setTimeout(60_000); @@ -67,6 +69,8 @@ class MockedTransport extends winston.transports.Console { class DynamicPluginLister { readonly loadedPlugins: DynamicPlugin[] = []; + getScannedPackage?: (plugin: DynamicPlugin) => ScannedPluginPackage; + feature(): BackendFeature { // eslint-disable-next-line consistent-this const that = this; @@ -78,6 +82,8 @@ class DynamicPluginLister { dynamicPlugins: dynamicPluginsServiceRef, }, async init({ dynamicPlugins }) { + that.getScannedPackage = plugin => + dynamicPlugins.getScannedPackage(plugin); that.loadedPlugins.push( ...dynamicPlugins.plugins({ includeFailed: true }), ); @@ -308,4 +314,70 @@ describe('dynamicPluginsFeatureLoader', () => { }, }); }); + + it('should load a backend plugin from the alpha package first', async () => { + const dynamicPLuginsLister = new DynamicPluginLister(); + const mockedTransport = new MockedTransport(); + const dynamicPluginsRootForAlpha = resolvePath( + __dirname, + '__fixtures__/dynamic-plugins-root-for-alpha', + ); + await startTestBackend({ + features: [ + mockServices.rootConfig.factory({ + data: { + dynamicPlugins: { + rootDirectory: dynamicPluginsRootForAlpha, + }, + }, + }), + dynamicPluginsFeatureLoader({ + moduleLoader: jestFreeTypescriptAwareModuleLoader, + transports: [mockedTransport], + format: winston.format.simple(), + }), + dynamicPLuginsLister.feature(), + ], + }); + + expect(mockedTransport.logs).toContainEqual( + 'info: This plugin has been loaded from the alpha package. {"service":"backstage"}', + ); + + const loadedPlugins = dynamicPLuginsLister.loadedPlugins; + expect(loadedPlugins).toMatchObject([ + { + installer: { + kind: 'new', + }, + name: 'plugin-test-backend-alpha-dynamic', + platform: 'node', + role: 'backend-plugin', + version: '0.0.0', + }, + ]); + expect( + dynamicPLuginsLister.getScannedPackage?.(loadedPlugins[0]), + ).toMatchObject({ + location: url.pathToFileURL( + path.resolve(dynamicPluginsRootForAlpha, 'test-backend-alpha-dynamic'), + ), + manifest: { + name: 'plugin-test-backend-alpha-dynamic', + version: '0.0.0', + description: 'A test dynamic backend module that exposes alpha API.', + backstage: { + role: 'backend-plugin', + pluginId: 'test-alpha', + pluginPackages: ['plugin-test-backend-alpha'], + }, + keywords: ['backstage', 'dynamic'], + }, + alphaManifest: { + name: 'plugin-test-backend-alpha-dynamic__alpha', + version: '0.0.0', + main: '../dist/alpha.cjs.js', + }, + }); + }); }); diff --git a/packages/backend-dynamic-feature-service/src/manager/plugin-manager.test.ts b/packages/backend-dynamic-feature-service/src/manager/plugin-manager.test.ts index b6854e3263..a7e01b919e 100644 --- a/packages/backend-dynamic-feature-service/src/manager/plugin-manager.test.ts +++ b/packages/backend-dynamic-feature-service/src/manager/plugin-manager.test.ts @@ -44,7 +44,7 @@ import { PluginScanner } from '../scanner/plugin-scanner'; import { findPaths } from '@backstage/cli-common'; import { createMockDirectory } from '@backstage/backend-test-utils'; import { rootLifecycleServiceFactory } from '@backstage/backend-defaults/rootLifecycle'; -import { PackageRole } from '@backstage/cli-node'; +import { BackstagePackageJson, PackageRole } from '@backstage/cli-node'; describe('backend-dynamic-feature-service', () => { const mockDir = createMockDirectory(); @@ -61,6 +61,13 @@ describe('backend-dynamic-feature-service', () => { relativePath: string[]; content: string; }; + alpha?: { + packageManifest: BackstagePackageJson; + indexFile: { + relativePath: string[]; + content: string; + }; + }; expectedLogs?(location: URL): { errors?: LogContent[]; warns?: LogContent[]; @@ -127,7 +134,7 @@ describe('backend-dynamic-feature-service', () => { }, indexFile: { relativePath: ['dist', 'index.cjs.js'], - content: `const alpha = { $$type: '@backstage/BackendFeature' }; exports["default"] = alpha;`, + content: `const feature = { $$type: '@backstage/BackendFeature' }; exports["default"] = feature;`, }, expectedLogs(location) { return { @@ -158,6 +165,60 @@ describe('backend-dynamic-feature-service', () => { ); }, }, + { + name: 'should load the alpha variant of a backend plugin in priority', + packageManifest: { + name: 'backend-dynamic-plugin-test', + version: '0.0.0', + backstage: { + role: 'backend-plugin', + }, + main: 'dist/index.cjs.js', + }, + indexFile: { + relativePath: ['dist', 'index.cjs.js'], + content: `throw 'should not take this one';`, + }, + alpha: { + packageManifest: { + name: 'backend-dynamic-plugin-test', + version: '0.0.0', + main: '../dist/alpha.cjs.js', + }, + indexFile: { + relativePath: ['dist', 'alpha.cjs.js'], + content: `const alpha = { $$type: '@backstage/BackendFeature' }; exports["default"] = alpha;`, + }, + }, + expectedLogs(location) { + return { + infos: [ + { + message: `loaded dynamic backend plugin 'backend-dynamic-plugin-test' from '${location}/alpha'`, + }, + ], + }; + }, + checkLoadedPlugins(plugins) { + expect(plugins).toMatchObject([ + { + name: 'backend-dynamic-plugin-test', + version: '0.0.0', + role: 'backend-plugin', + platform: 'node', + installer: { + kind: 'new', + }, + }, + ]); + const installer: NewBackendPluginInstaller = ( + plugins[0] as BackendDynamicPlugin + ).installer as NewBackendPluginInstaller; + expect((installer.install() as BackendFeature).$$type).toEqual( + '@backstage/BackendFeature', + ); + }, + }, { name: 'should successfully load a new backend plugin by the default BackendFeatureFactory', packageManifest: { @@ -580,11 +641,12 @@ describe('backend-dynamic-feature-service', () => { const plugin: ScannedPluginPackage = { location: url.pathToFileURL(mockDir.resolve(randomUUID())), manifest: tc.packageManifest, + alphaManifest: tc.alpha?.packageManifest, }; const mockedFiles = { [path.join(url.fileURLToPath(plugin.location), 'package.json')]: - JSON.stringify(plugin), + JSON.stringify(tc.packageManifest), }; if (tc.indexFile) { mockedFiles[ @@ -594,6 +656,18 @@ describe('backend-dynamic-feature-service', () => { ) ] = tc.indexFile.content; } + if (tc.alpha) { + mockedFiles[ + path.join(url.fileURLToPath(plugin.location), 'alpha', 'package.json') + ] = JSON.stringify(tc.alpha.packageManifest); + mockedFiles[ + path.join( + url.fileURLToPath(plugin.location), + ...tc.alpha.indexFile.relativePath, + ) + ] = tc.alpha.indexFile.content; + } + mockDir.setContent(mockedFiles); const logger = new MockedLogger(); diff --git a/packages/backend-dynamic-feature-service/src/manager/plugin-manager.ts b/packages/backend-dynamic-feature-service/src/manager/plugin-manager.ts index a94a01f0f1..61f4c8dc10 100644 --- a/packages/backend-dynamic-feature-service/src/manager/plugin-manager.ts +++ b/packages/backend-dynamic-feature-service/src/manager/plugin-manager.ts @@ -36,7 +36,6 @@ import { } from '@backstage/backend-plugin-api'; import { PackageRole, PackageRoles } from '@backstage/cli-node'; import { findPaths } from '@backstage/cli-common'; -import path from 'path'; import * as fs from 'fs'; import { FeatureDiscoveryService, @@ -79,13 +78,7 @@ export class DynamicPluginManager implements DynamicPluginProvider { ); const dynamicPluginsPaths = scannedPlugins.map(p => - fs.realpathSync( - path.dirname( - path.dirname( - path.resolve(url.fileURLToPath(p.location), p.manifest.main), - ), - ), - ), + fs.realpathSync(url.fileURLToPath(p.location)), ); await moduleLoader.bootstrap(backstageRoot, dynamicPluginsPaths); @@ -164,8 +157,13 @@ export class DynamicPluginManager implements DynamicPluginProvider { private async loadBackendPlugin( plugin: ScannedPluginPackage, ): Promise { + const usedPluginManifest = + plugin.alphaManifest?.main ?? plugin.manifest.main; + const usedPluginLocation = plugin.alphaManifest?.main + ? `${plugin.location}/alpha` + : plugin.location; const packagePath = url.fileURLToPath( - `${plugin.location}/${plugin.manifest.main}`, + `${usedPluginLocation}/${usedPluginManifest}`, ); const dynamicPlugin: BackendDynamicPlugin = { name: plugin.manifest.name, @@ -194,12 +192,12 @@ export class DynamicPluginManager implements DynamicPluginProvider { } if (dynamicPlugin.installer) { this.logger.info( - `loaded dynamic backend plugin '${plugin.manifest.name}' from '${plugin.location}'`, + `loaded dynamic backend plugin '${plugin.manifest.name}' from '${usedPluginLocation}'`, ); } else { dynamicPlugin.failure = `the module should either export a 'BackendFeature' or 'BackendFeatureFactory' as default export, or export a 'const dynamicPluginInstaller: BackendDynamicPluginInstaller' field as dynamic loading entrypoint.`; this.logger.error( - `dynamic backend plugin '${plugin.manifest.name}' could not be loaded from '${plugin.location}': ${dynamicPlugin.failure}`, + `dynamic backend plugin '${plugin.manifest.name}' could not be loaded from '${usedPluginLocation}': ${dynamicPlugin.failure}`, ); } return dynamicPlugin; @@ -210,7 +208,7 @@ export class DynamicPluginManager implements DynamicPluginProvider { : new Error(error); dynamicPlugin.failure = `${typedError.name}: ${typedError.message}`; this.logger.error( - `an error occurred while loading dynamic backend plugin '${plugin.manifest.name}' from '${plugin.location}'`, + `an error occurred while loading dynamic backend plugin '${plugin.manifest.name}' from '${usedPluginLocation}'`, typedError, ); return dynamicPlugin; diff --git a/packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.test.ts b/packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.test.ts index 3086f2c16e..84273cc706 100644 --- a/packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.test.ts +++ b/packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.test.ts @@ -430,16 +430,19 @@ Please add '${mockDir.resolve( expectedPluginPackages: [ { location: url.pathToFileURL( - mockDir.resolve( - 'backstageRoot/dist-dynamic/test-backend-plugin/alpha', - ), + mockDir.resolve('backstageRoot/dist-dynamic/test-backend-plugin'), ), manifest: { name: 'test-backend-plugin-dynamic', version: '0.0.0', - main: '../dist/alpha.cjs.js', + main: 'dist/index.cjs.js', backstage: { role: 'backend-plugin' }, }, + alphaManifest: { + name: 'test-backend-plugin-dynamic', + version: '0.0.0', + main: '../dist/alpha.cjs.js', + }, }, ], }, diff --git a/packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.ts b/packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.ts index 7091999e7b..ca8924cbab 100644 --- a/packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.ts +++ b/packages/backend-dynamic-feature-service/src/scanner/plugin-scanner.ts @@ -21,7 +21,6 @@ import * as chokidar from 'chokidar'; import * as path from 'path'; import * as url from 'url'; import debounce from 'lodash/debounce'; -import { PackagePlatform, PackageRoles } from '@backstage/cli-node'; import { LoggerService } from '@backstage/backend-plugin-api'; export interface DynamicPluginScannerOptions { @@ -37,6 +36,15 @@ export interface ScanRootResponse { export const configKey = 'dynamicPlugins'; +class WrappedError extends Error { + wrapped: Error; + + constructor(message: string, wrapped: Error) { + super(message); + this.wrapped = wrapped; + } +} + export class PluginScanner { private _rootDirectory?: string; private configUnsubscribe?: () => void; @@ -161,52 +169,25 @@ export class PluginScanner { } let scannedPlugin: ScannedPluginPackage; - let platform: PackagePlatform; try { scannedPlugin = await this.scanDir(pluginHome); if (!scannedPlugin.manifest.main) { throw new Error("field 'main' not found in 'package.json'"); } - if (scannedPlugin.manifest.backstage?.role) { - platform = PackageRoles.getRoleInfo( - scannedPlugin.manifest.backstage.role, - ).platform; - } else { + if (!scannedPlugin.manifest.backstage?.role) { throw new Error("field 'backstage.role' not found in 'package.json'"); } } catch (e) { - this.logger.error( - `failed to load dynamic plugin manifest from '${pluginHome}'`, - e, - ); + if (e instanceof WrappedError) { + this.logger.error(e.message, e.wrapped); + } else { + this.logger.error( + `failed to load dynamic plugin manifest from '${pluginHome}'`, + e, + ); + } continue; } - - if (platform === 'node') { - if (this.preferAlpha) { - const pluginHomeAlpha = path.resolve(pluginHome, 'alpha'); - if (existsSync(pluginHomeAlpha)) { - if ((await fs.lstat(pluginHomeAlpha)).isDirectory()) { - const backstage = scannedPlugin.manifest.backstage; - try { - scannedPlugin = await this.scanDir(pluginHomeAlpha); - } catch (e) { - this.logger.error( - `failed to load dynamic plugin manifest from '${pluginHomeAlpha}'`, - e, - ); - continue; - } - scannedPlugin.manifest.backstage = backstage; - } else { - this.logger.warn( - `skipping '${pluginHomeAlpha}' since it is not a directory`, - ); - } - } - } - } - scannedPlugins.push(scannedPlugin); } return { packages: scannedPlugins }; @@ -216,10 +197,37 @@ export class PluginScanner { const manifestFile = path.resolve(pluginHome, 'package.json'); const content = await fs.readFile(manifestFile); const manifest: ScannedPluginManifest = JSON.parse(content.toString()); - return { + const scannedPluginPackage: ScannedPluginPackage = { location: url.pathToFileURL(pluginHome), manifest: manifest, }; + + if (this.preferAlpha) { + const pluginHomeAlpha = path.resolve(pluginHome, 'alpha'); + if (existsSync(pluginHomeAlpha)) { + if ((await fs.lstat(pluginHomeAlpha)).isDirectory()) { + try { + const alphaContent = await fs.readFile( + path.resolve(pluginHomeAlpha, 'package.json'), + ); + scannedPluginPackage.alphaManifest = JSON.parse( + alphaContent.toString(), + ); + } catch (e) { + throw new WrappedError( + `failed to load dynamic plugin manifest from '${pluginHome}/alpha'`, + e, + ); + } + } else { + this.logger.warn( + `skipping '${pluginHomeAlpha}' since it is not a directory`, + ); + } + } + } + + return scannedPluginPackage; } async trackChanges(): Promise { diff --git a/packages/backend-dynamic-feature-service/src/scanner/types.ts b/packages/backend-dynamic-feature-service/src/scanner/types.ts index 5bac92f064..bfb3e21c89 100644 --- a/packages/backend-dynamic-feature-service/src/scanner/types.ts +++ b/packages/backend-dynamic-feature-service/src/scanner/types.ts @@ -22,6 +22,7 @@ import { BackstagePackageJson, PackageRole } from '@backstage/cli-node'; export interface ScannedPluginPackage { location: URL; manifest: ScannedPluginManifest; + alphaManifest?: BackstagePackageJson; } /** diff --git a/packages/backend-dynamic-feature-service/src/schemas/schemas.ts b/packages/backend-dynamic-feature-service/src/schemas/schemas.ts index 52fb5eab6d..0e873e986c 100644 --- a/packages/backend-dynamic-feature-service/src/schemas/schemas.ts +++ b/packages/backend-dynamic-feature-service/src/schemas/schemas.ts @@ -161,10 +161,7 @@ async function gatherDynamicPluginsSchemas( let schemaLocation = schemaLocator(pluginPackage); if (!path.isAbsolute(schemaLocation)) { - let pluginLocation = url.fileURLToPath(pluginPackage.location); - if (path.basename(pluginLocation) === 'alpha') { - pluginLocation = path.dirname(pluginLocation); - } + const pluginLocation = url.fileURLToPath(pluginPackage.location); schemaLocation = path.resolve(pluginLocation, schemaLocation); }