diff --git a/.changeset/friendly-melons-shake.md b/.changeset/friendly-melons-shake.md new file mode 100644 index 0000000000..7bdbb58fea --- /dev/null +++ b/.changeset/friendly-melons-shake.md @@ -0,0 +1,6 @@ +--- +'@backstage/backend-dynamic-feature-service': patch +--- + +Enhance the `CommonJSModuleLoader` to add support for `resolvePackagePath` calls from backend dynamic plugins, with customizable package resolution, and make the `CommonJSModuleLoader` public API. +Fixing this backend dynamic plugin limitation related to `resolvePackagePath` is important for backend dynamic plugins which use the database, since database migration scripts systematically use `resolvePackagePath`. diff --git a/packages/backend-dynamic-feature-service/report.api.md b/packages/backend-dynamic-feature-service/report.api.md index ee2886a79f..6e53f3747f 100644 --- a/packages/backend-dynamic-feature-service/report.api.md +++ b/packages/backend-dynamic-feature-service/report.api.md @@ -68,6 +68,32 @@ export interface BaseDynamicPlugin { version: string; } +// @public (undocumented) +export class CommonJSModuleLoader implements ModuleLoader { + constructor(options: CommonJSModuleLoaderOptions); + // (undocumented) + bootstrap( + backstageRoot: string, + dynamicPluginsPaths: string[], + scannedPluginManifests: Map, + ): Promise; + // (undocumented) + load(packagePath: string): Promise; + // (undocumented) + readonly options: CommonJSModuleLoaderOptions; +} + +// @public (undocumented) +export type CommonJSModuleLoaderOptions = { + logger: LoggerService; + dynamicPluginPackageNameSuffixes?: String[]; + customResolveDynamicPackage?: ( + logger: LoggerService, + searchedPackageName: string, + scannedPluginManifests: Map, + ) => string | undefined; +}; + // @public (undocumented) export type DynamicPlugin = FrontendDynamicPlugin | BackendDynamicPlugin; @@ -264,7 +290,11 @@ export type LegacyPluginEnvironment = { // @public (undocumented) export interface ModuleLoader { // (undocumented) - bootstrap(backstageRoot: string, dynamicPluginPaths: string[]): Promise; + bootstrap( + backstageRoot: string, + dynamicPluginPaths: string[], + scannedPluginManifests?: Map, + ): Promise; // (undocumented) load(id: string): Promise; } diff --git a/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root/test-backend-dynamic/dist/index.cjs.js b/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root/test-backend-dynamic/dist/index.cjs.js index 6cc4a4f125..77c2e32bef 100644 --- a/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root/test-backend-dynamic/dist/index.cjs.js +++ b/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root/test-backend-dynamic/dist/index.cjs.js @@ -10,6 +10,8 @@ const url = require('url'); const privateDep = require('private-dep-with-frontend-plugin-index-path'); +const someResource = backendPluginApi.resolvePackagePath('plugin-test-backend', 'someResource.txt'); + const testPlugin = backendPluginApi.createBackendPlugin({ pluginId: "test", register(env) { diff --git a/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root/test-backend-dynamic/someResource.txt b/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root/test-backend-dynamic/someResource.txt new file mode 100644 index 0000000000..e69de29bb2 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 f83a1ad1a7..fb6aa5e48a 100644 --- a/packages/backend-dynamic-feature-service/src/features/features.test.ts +++ b/packages/backend-dynamic-feature-service/src/features/features.test.ts @@ -25,9 +25,11 @@ import path, { resolve as resolvePath } from 'path'; import { BackendFeature, createBackendPlugin, - LoggerService, } from '@backstage/backend-plugin-api'; -import { CommonJSModuleLoader } from '../loader/CommonJSModuleLoader'; +import { + CommonJSModuleLoader, + CommonJSModuleLoaderOptions, +} from '../loader/CommonJSModuleLoader'; import * as winston from 'winston'; import * as url from 'url'; import { MESSAGE } from 'triple-beam'; @@ -38,13 +40,14 @@ import { ScannedPluginPackage } from '../scanner'; jest.setTimeout(60_000); async function jestFreeTypescriptAwareModuleLoader( - logger: LoggerService, - dontBootstrap: boolean = false, + options: CommonJSModuleLoaderOptions & { + dontBootstrap?: boolean; + }, ) { - const loader = new CommonJSModuleLoader(logger); + const loader = new CommonJSModuleLoader(options); (loader as any).module = await loader.load('node:module'); loader.load(path.resolve(__dirname, '../../../cli/config/nodeTransform.cjs')); - if (dontBootstrap) { + if (options.dontBootstrap) { loader.bootstrap = async () => {}; } return loader; @@ -118,7 +121,10 @@ describe('dynamicPluginsFeatureLoader', () => { }), dynamicPluginsFeatureLoader({ moduleLoader: logger => - jestFreeTypescriptAwareModuleLoader(logger, true), + jestFreeTypescriptAwareModuleLoader({ + logger, + dontBootstrap: true, + }), logger: () => ({ transports: [mockedTransport], format: winston.format.simple(), @@ -145,6 +151,57 @@ describe('dynamicPluginsFeatureLoader', () => { ]); }); + it('should fail on resolvePackagePath because -dynamic suffix is not allowed for dynamic plugin packages.', async () => { + const dynamicPLuginsLister = new DynamicPluginLister(); + const mockedTransport = new MockedTransport(); + await startTestBackend({ + features: [ + mockServices.rootConfig.factory({ + data: { + dynamicPlugins: { + rootDirectory: dynamicPluginsRootDirectory, + }, + }, + }), + dynamicPluginsFeatureLoader({ + moduleLoader: logger => + jestFreeTypescriptAwareModuleLoader({ + logger, + dynamicPluginPackageNameSuffixes: [], + }), + logger: () => ({ + transports: [mockedTransport], + format: winston.format.simple(), + }), + }), + dynamicPLuginsLister.feature(), + ], + }); + expect(mockedTransport.logs).toContainEqual( + expect.stringMatching( + "error: an error occurred while loading dynamic backend plugin 'plugin-test-backend-dynamic' from '.*/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root/test-backend-dynamic", + ), + ); + expect(dynamicPLuginsLister.loadedPlugins).toMatchObject([ + { + name: 'plugin-test-backend-dynamic', + platform: 'node', + role: 'backend-plugin', + version: '0.0.0', + failure: + expect.stringMatching(`Error: Cannot find module 'plugin-test-backend/package.json' +Require stack: +- .*/packages/backend-plugin-api/src/paths.ts +- .*/packages/backend-plugin-api/src/index.ts +- .*/packages/backend-dynamic-feature-service/src/manager/plugin-manager.ts +- .*/packages/backend-dynamic-feature-service/src/manager/index.ts +- .*/packages/backend-dynamic-feature-service/src/features/__fixtures__/dynamic-plugins-root/test-backend-dynamic/dist/index.cjs.js +`), + }, + expect.anything(), + ]); + }); + it('should load and show the 2 dynamic plugins in a list of dynamic plugins returned by a static backend plugin', async () => { const dynamicPLuginsLister = new DynamicPluginLister(); await startTestBackend({ @@ -157,7 +214,8 @@ describe('dynamicPluginsFeatureLoader', () => { }, }), dynamicPluginsFeatureLoader({ - moduleLoader: jestFreeTypescriptAwareModuleLoader, + moduleLoader: logger => + jestFreeTypescriptAwareModuleLoader({ logger }), }), dynamicPLuginsLister.feature(), ], @@ -195,7 +253,8 @@ describe('dynamicPluginsFeatureLoader', () => { }, }), dynamicPluginsFeatureLoader({ - moduleLoader: logger => jestFreeTypescriptAwareModuleLoader(logger), + moduleLoader: logger => + jestFreeTypescriptAwareModuleLoader({ logger }), logger: config => { const label = config?.getString('customLogLabel') ?? 'no-label'; return { @@ -233,7 +292,8 @@ describe('dynamicPluginsFeatureLoader', () => { }, }), dynamicPluginsFeatureLoader({ - moduleLoader: jestFreeTypescriptAwareModuleLoader, + moduleLoader: logger => + jestFreeTypescriptAwareModuleLoader({ logger }), logger: () => ({ transports: [mockedTransport], format: winston.format.simple(), @@ -285,7 +345,8 @@ describe('dynamicPluginsFeatureLoader', () => { }, }), dynamicPluginsFeatureLoader({ - moduleLoader: jestFreeTypescriptAwareModuleLoader, + moduleLoader: logger => + jestFreeTypescriptAwareModuleLoader({ logger }), }), import('@backstage/plugin-app-backend'), ], @@ -321,7 +382,8 @@ describe('dynamicPluginsFeatureLoader', () => { }, }), dynamicPluginsFeatureLoader({ - moduleLoader: jestFreeTypescriptAwareModuleLoader, + moduleLoader: logger => + jestFreeTypescriptAwareModuleLoader({ logger }), }), ], }); @@ -372,7 +434,8 @@ describe('dynamicPluginsFeatureLoader', () => { }, }), dynamicPluginsFeatureLoader({ - moduleLoader: jestFreeTypescriptAwareModuleLoader, + moduleLoader: logger => + jestFreeTypescriptAwareModuleLoader({ logger }), logger: () => ({ transports: [mockedTransport], format: winston.format.simple(), diff --git a/packages/backend-dynamic-feature-service/src/loader/CommonJSModuleLoader.ts b/packages/backend-dynamic-feature-service/src/loader/CommonJSModuleLoader.ts index f9d4a48a44..50a6028422 100644 --- a/packages/backend-dynamic-feature-service/src/loader/CommonJSModuleLoader.ts +++ b/packages/backend-dynamic-feature-service/src/loader/CommonJSModuleLoader.ts @@ -16,17 +16,35 @@ import { ModuleLoader } from './types'; import { LoggerService } from '@backstage/backend-plugin-api'; import path from 'path'; +import { ScannedPluginManifest } from '../scanner'; +/** + * @public + */ +export type CommonJSModuleLoaderOptions = { + logger: LoggerService; + dynamicPluginPackageNameSuffixes?: String[]; + customResolveDynamicPackage?: ( + logger: LoggerService, + searchedPackageName: string, + scannedPluginManifests: Map, + ) => string | undefined; +}; + +/** + * @public + */ export class CommonJSModuleLoader implements ModuleLoader { private module: any; - constructor(public readonly logger: LoggerService) { + constructor(public readonly options: CommonJSModuleLoaderOptions) { this.module = require('node:module'); } async bootstrap( backstageRoot: string, dynamicPluginsPaths: string[], + scannedPluginManifests: Map, ): Promise { const backstageRootNodeModulesPath = `${backstageRoot}/node_modules`; const dynamicNodeModulesPaths = [ @@ -44,11 +62,91 @@ export class CommonJSModuleLoader implements ModuleLoader { dynamicNodeModulesPaths.some(p => nodeModulePath.startsWith(p)) ); }); - this.logger.debug( + this.options.logger.debug( `Overriding node_modules search path for dynamic plugin ${from} to: ${filtered}`, ); return filtered; }; + + // The whole piece of code below is a way to accomodate the limitations of + // the current `resolvePackagePath` implementation, which cannot be provided + // some custom locations where it should find the assets of some given packages. + // + // Since the packages for dynamic plugins are not located in the main backstage + // monorepo structure, and since dynamic plugins could also be repackaged + // (typically renamed with a `-dynamic` suffix), for now we have to customize + // module file name resolution here to support these use-cases. + // + // This might not be necessary anymore according to future enhancements to the + // `resolvePackagePath` feature. + const oldResolveFileName = this.module._resolveFilename; + this.module._resolveFilename = ( + request: string, + mod: NodeModule, + _: boolean, + options: any, + ): any => { + let errorToThrow: any; + try { + return oldResolveFileName(request, mod, _, options); + } catch (e) { + errorToThrow = e; + this.options.logger.debug( + `Could not resolve '${request}' inside the Core backstage backend application`, + e instanceof Error ? e : undefined, + ); + } + + // Are we trying to resolve a `package.json` from an originating module of the core backstage application + // (this is mostly done by calling `@backstage/backend-plugin-api/resolvePackagePath`). + const resolvingPackageJsonFromBackstageApplication = + request?.endsWith('/package.json') && + mod?.path && + !dynamicPluginsPaths.some(p => mod.path.startsWith(p)); + + // If not, we don't need the dedicated specfic case below. + if (!resolvingPackageJsonFromBackstageApplication) { + throw errorToThrow; + } + + this.options.logger.info( + `Resolving '${request}' in the dynamic backend plugins`, + ); + const searchedPackageName = request.replace(/\/package.json$/, ''); + + // First search for a dynamic plugin package matching the expected package name, + // taking in account accepted dynamic plugin package name suffixes + // (suffix accepted by default is '-dynamic'). + const searchedPackageNamesWithSuffixes = ( + this.options.dynamicPluginPackageNameSuffixes ?? ['-dynamic'] + ).map(s => `${searchedPackageName}${s}`); + for (const [realPath, pkg] of scannedPluginManifests.entries()) { + if ( + [searchedPackageName, ...searchedPackageNamesWithSuffixes].includes( + pkg.name, + ) + ) { + const resolvedPath = path.resolve(realPath, 'package.json'); + this.options.logger.info(`Resolved '${request}' at ${resolvedPath}`); + return resolvedPath; + } + } + + // If a custom resolution is provided, use it. + // This allows accomodating alternate ways to package dynamic plugins: + // static plugin package wrapped inside a distinct dynamic plugin package for example. + if (this.options.customResolveDynamicPackage) { + const resolvedPath = this.options.customResolveDynamicPackage( + this.options.logger, + searchedPackageName, + scannedPluginManifests, + ); + if (resolvedPath) { + return resolvedPath; + } + } + throw errorToThrow; + }; } async load(packagePath: string): Promise { diff --git a/packages/backend-dynamic-feature-service/src/loader/index.ts b/packages/backend-dynamic-feature-service/src/loader/index.ts index ff0892e66d..fe4f34dc90 100644 --- a/packages/backend-dynamic-feature-service/src/loader/index.ts +++ b/packages/backend-dynamic-feature-service/src/loader/index.ts @@ -15,3 +15,5 @@ */ export type { ModuleLoader } from './types'; +export type { CommonJSModuleLoaderOptions } from './CommonJSModuleLoader'; +export { CommonJSModuleLoader } from './CommonJSModuleLoader'; diff --git a/packages/backend-dynamic-feature-service/src/loader/types.ts b/packages/backend-dynamic-feature-service/src/loader/types.ts index 12a051e482..99f3c00607 100644 --- a/packages/backend-dynamic-feature-service/src/loader/types.ts +++ b/packages/backend-dynamic-feature-service/src/loader/types.ts @@ -14,11 +14,17 @@ * limitations under the License. */ +import { ScannedPluginManifest } from '../scanner'; + /** * @public */ export interface ModuleLoader { - bootstrap(backstageRoot: string, dynamicPluginPaths: string[]): Promise; + bootstrap( + backstageRoot: string, + dynamicPluginPaths: string[], + scannedPluginManifests?: Map, + ): Promise; load(id: string): Promise; } 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 a7e01b919e..d7133a804a 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 @@ -1086,9 +1086,25 @@ describe('backend-dynamic-feature-service', () => { expect(fromConfigSpier).toHaveBeenCalled(); expect(applyConfigSpier).toHaveBeenCalled(); expect(scanRootSpier).toHaveBeenCalled(); + const realPath = fs.realpathSync( + otherMockDir.resolve('a-dynamic-plugin'), + ); expect(mockedModuleLoader.bootstrap).toHaveBeenCalledWith( findPaths(__dirname).targetRoot, - [fs.realpathSync(otherMockDir.resolve('a-dynamic-plugin'))], + [realPath], + new Map([ + [ + realPath, + { + name: 'test', + main: 'dist/index.cjs.js', + version: '0.0.0', + backstage: { + role: 'backend-plugin', + }, + }, + ], + ]), ); expect(mockedModuleLoader.load).toHaveBeenCalledWith( mockDir.resolve( 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 61f4c8dc10..c64fca053c 100644 --- a/packages/backend-dynamic-feature-service/src/manager/plugin-manager.ts +++ b/packages/backend-dynamic-feature-service/src/manager/plugin-manager.ts @@ -70,18 +70,26 @@ export class DynamicPluginManager implements DynamicPluginProvider { const scannedPlugins = (await scanner.scanRoot()).packages; scanner.trackChanges(); const moduleLoader = - options.moduleLoader || new CommonJSModuleLoader(options.logger); + options.moduleLoader || + new CommonJSModuleLoader({ logger: options.logger }); const manager = new DynamicPluginManager( options.logger, scannedPlugins, moduleLoader, ); - const dynamicPluginsPaths = scannedPlugins.map(p => - fs.realpathSync(url.fileURLToPath(p.location)), + const scannedPluginManifestsPerRealPath = new Map( + scannedPlugins.map(p => [ + fs.realpathSync(url.fileURLToPath(p.location)), + p.manifest, + ]), ); - await moduleLoader.bootstrap(backstageRoot, dynamicPluginsPaths); + await moduleLoader.bootstrap( + backstageRoot, + [...scannedPluginManifestsPerRealPath.keys()], + scannedPluginManifestsPerRealPath, + ); scanner.subscribeToRootDirectoryChange(async () => { manager._availablePackages = (await scanner.scanRoot()).packages;