From 27a139dad54e882f5c4334767a0e9958f24ad678 Mon Sep 17 00:00:00 2001 From: David Festal Date: Thu, 31 Aug 2023 11:53:24 +0200 Subject: [PATCH] Fixes after second round of review Signed-off-by: David Festal --- packages/backend-plugin-manager/api-report.md | 18 +++----- .../src/loader/types.ts | 4 -- .../src/scanner/plugin-scanner.test.ts | 41 ++++++++++++++++++- .../src/scanner/plugin-scanner.ts | 24 +++++------ .../src/scanner/types.ts | 13 +++--- 5 files changed, 60 insertions(+), 40 deletions(-) diff --git a/packages/backend-plugin-manager/api-report.md b/packages/backend-plugin-manager/api-report.md index 026b1d0553..9617dc1096 100644 --- a/packages/backend-plugin-manager/api-report.md +++ b/packages/backend-plugin-manager/api-report.md @@ -4,6 +4,7 @@ ```ts import { BackendFeature } from '@backstage/backend-plugin-api'; +import { BackstagePackageJson } from '@backstage/cli-node'; import { CatalogBuilder } from '@backstage/plugin-catalog-backend'; import { Config } from '@backstage/config'; import { EventBroker } from '@backstage/plugin-events-node'; @@ -150,8 +151,6 @@ export interface ModuleLoader { bootstrap(backstageRoot: string, dynamicPluginPaths: string[]): Promise; // (undocumented) load(id: string): Promise; - // (undocumented) - logger: LoggerService; } // @public (undocumented) @@ -182,18 +181,11 @@ export class PluginManager implements BackendPluginProvider { } // @public (undocumented) -export interface ScannedPluginManifest { - // (undocumented) - backstage: { - role: PackageRole; +export type ScannedPluginManifest = BackstagePackageJson & + Required> & + Required> & { + backstage: Required; }; - // (undocumented) - main: string; - // (undocumented) - name: string; - // (undocumented) - version: string; -} // @public (undocumented) export interface ScannedPluginPackage { diff --git a/packages/backend-plugin-manager/src/loader/types.ts b/packages/backend-plugin-manager/src/loader/types.ts index ebd3a1e7ce..12a051e482 100644 --- a/packages/backend-plugin-manager/src/loader/types.ts +++ b/packages/backend-plugin-manager/src/loader/types.ts @@ -14,14 +14,10 @@ * limitations under the License. */ -import { LoggerService } from '@backstage/backend-plugin-api'; - /** * @public */ export interface ModuleLoader { - logger: LoggerService; - bootstrap(backstageRoot: string, dynamicPluginPaths: string[]): Promise; load(id: string): Promise; diff --git a/packages/backend-plugin-manager/src/scanner/plugin-scanner.test.ts b/packages/backend-plugin-manager/src/scanner/plugin-scanner.test.ts index aff671808b..25f07baeaa 100644 --- a/packages/backend-plugin-manager/src/scanner/plugin-scanner.test.ts +++ b/packages/backend-plugin-manager/src/scanner/plugin-scanner.test.ts @@ -640,8 +640,45 @@ Please add '/backstageRoot/node_modules' to the 'NODE_PATH' when running the bac message: "failed to load dynamic plugin manifest from '/backstageRoot/dist-dynamic/test-backend-plugin'", meta: { - name: 'TypeError', - message: "Cannot read properties of undefined (reading 'role')", + name: 'Error', + message: "field 'backstage.role' not found in 'package.json'", + }, + }, + ], + }, + }, + { + name: 'missing main field in package.json', + fileSystem: { + '/backstageRoot': mockFs.directory({ + items: { + 'dist-dynamic': mockFs.directory({ + items: { + 'test-backend-plugin': mockFs.directory({ + items: { + 'package.json': mockFs.file({ + content: JSON.stringify({ + name: 'test-backend-plugin-dynamic', + version: '0.0.0', + backstage: { role: 'backend-plugin' }, + }), + }), + }, + }), + }, + }), + }, + }), + }, + expectedPluginPackages: [], + expectedLogs: { + errors: [ + { + message: + "failed to load dynamic plugin manifest from '/backstageRoot/dist-dynamic/test-backend-plugin'", + meta: { + name: 'Error', + message: "field 'main' not found in 'package.json'", }, }, ], diff --git a/packages/backend-plugin-manager/src/scanner/plugin-scanner.ts b/packages/backend-plugin-manager/src/scanner/plugin-scanner.ts index 39d55bdb5c..bdcf786619 100644 --- a/packages/backend-plugin-manager/src/scanner/plugin-scanner.ts +++ b/packages/backend-plugin-manager/src/scanner/plugin-scanner.ts @@ -143,21 +143,19 @@ export class PluginScanner { } let scannedPlugin: ScannedPluginPackage; - try { - scannedPlugin = await this.scanDir(pluginHome); - } catch (e) { - this.logger.error( - `failed to load dynamic plugin manifest from '${pluginHome}'`, - e, - ); - continue; - } - let platform: PackagePlatform; try { - platform = PackageRoles.getRoleInfo( - scannedPlugin.manifest.backstage.role, - ).platform; + 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 { + throw new Error("field 'backstage.role' not found in 'package.json'"); + } } catch (e) { this.logger.error( `failed to load dynamic plugin manifest from '${pluginHome}'`, diff --git a/packages/backend-plugin-manager/src/scanner/types.ts b/packages/backend-plugin-manager/src/scanner/types.ts index 726babaaaa..b456ffeea1 100644 --- a/packages/backend-plugin-manager/src/scanner/types.ts +++ b/packages/backend-plugin-manager/src/scanner/types.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { PackageRole } from '@backstage/cli-node'; +import { BackstagePackageJson } from '@backstage/cli-node'; /** * @public @@ -27,11 +27,8 @@ export interface ScannedPluginPackage { /** * @public */ -export interface ScannedPluginManifest { - name: string; - version: string; - backstage: { - role: PackageRole; +export type ScannedPluginManifest = BackstagePackageJson & + Required> & + Required> & { + backstage: Required; }; - main: string; -}