From c5b6f34ec78039a6726b6d5ac4edd344899a02da Mon Sep 17 00:00:00 2001 From: Tim Hansen Date: Fri, 7 Feb 2025 13:57:07 -0700 Subject: [PATCH] Add default startup options Signed-off-by: Tim Hansen --- .changeset/twenty-carrots-jog.md | 18 +++++- packages/backend-app-api/config.d.ts | 25 +++++++- .../src/wiring/BackendInitializer.test.ts | 61 ++++++++++++++++++- .../src/wiring/BackendInitializer.ts | 31 ++++++---- packages/backend-app-api/src/wiring/types.ts | 13 ++-- 5 files changed, 123 insertions(+), 25 deletions(-) diff --git a/.changeset/twenty-carrots-jog.md b/.changeset/twenty-carrots-jog.md index 9a87c403ff..71858ccd11 100644 --- a/.changeset/twenty-carrots-jog.md +++ b/.changeset/twenty-carrots-jog.md @@ -8,10 +8,24 @@ Added a configuration to permit backend plugin failures on startup: backend: ... startup: - plugin-x: - optional: true + plugins: + plugin-x: + optional: true ``` This configuration permits `plugin-x` to fail on startup. Omitting the `startup` configuration matches the previous behavior, wherein any individual plugin failure is fatal to backend startup. + +The default can also be changed, so that all plugins are considered optional +unless otherwise specified: + +```yaml +backend: + startup: + default: + optional: true + plugins: + catalog: + optional: false +``` diff --git a/packages/backend-app-api/config.d.ts b/packages/backend-app-api/config.d.ts index 7ffa882fc4..8af778ca42 100644 --- a/packages/backend-app-api/config.d.ts +++ b/packages/backend-app-api/config.d.ts @@ -14,8 +14,6 @@ * limitations under the License. */ -import { BackendStartupOptions } from './src'; - export interface Config { backend?: { /** Used by the feature discovery service */ @@ -26,6 +24,27 @@ export interface Config { exclude?: string[]; }; - startup?: BackendStartupOptions; + startup?: { + default?: { + /** + * The default value for `optional` if not specified for a particular plugin. This defaults to + * false, which means `optional: true` must be specified for individual plugins to be considered + * optional. This can also be set to true, which flips the logic for individual plugins so that + * they must be set to `optional: false` to be required. + */ + optional?: boolean; + }; + plugins?: { + [pluginId: string]: { + /** + * Used to mark plugins as optional, which allows the backend to start up even in the event + * of a plugin failure. Plugin failures without this configuration are fatal. This can + * enable leaving a crashing plugin installed, but still permit backend startup, which may + * help troubleshoot data-dependent issues. + */ + optional?: boolean; + }; + }; + }; }; } diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts index 304b547d15..29fafa726d 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts @@ -565,7 +565,9 @@ describe('BackendInitializer', () => { mockServices.rootLifecycle.factory(), mockServices.rootLogger.factory(), mockServices.rootConfig.factory({ - data: { backend: { startup: { test: { optional: true } } } }, + data: { + backend: { startup: { plugins: { test: { optional: true } } } }, + }, }), ]); init.add( @@ -584,6 +586,63 @@ describe('BackendInitializer', () => { await init.start(); }); + it('should permit startup errors if the default is set', async () => { + const init = new BackendInitializer([ + mockServices.rootLifecycle.factory(), + mockServices.rootLogger.factory(), + mockServices.rootConfig.factory({ + data: { backend: { startup: { default: { optional: true } } } }, + }), + ]); + init.add( + createBackendPlugin({ + pluginId: 'test', + register(reg) { + reg.registerInit({ + deps: {}, + async init() { + throw new Error('NOPE'); + }, + }); + }, + }), + ); + await init.start(); + }); + + it('should forward errors for plugins explicitly marked as not optional when the default is true', async () => { + const init = new BackendInitializer([ + mockServices.rootLifecycle.factory(), + mockServices.rootLogger.factory(), + mockServices.rootConfig.factory({ + data: { + backend: { + startup: { + default: { optional: true }, + plugins: { test: { optional: false } }, + }, + }, + }, + }), + ]); + init.add( + createBackendPlugin({ + pluginId: 'test', + register(reg) { + reg.registerInit({ + deps: {}, + async init() { + throw new Error('NOPE'); + }, + }); + }, + }), + ); + await expect(init.start()).rejects.toThrow( + "Plugin 'test' startup failed; caused by Error: NOPE", + ); + }); + it('should forward errors when multiple plugins fail to start', async () => { const init = new BackendInitializer([]); init.add( diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index eb63ebe78b..5384423bfe 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.ts @@ -24,7 +24,8 @@ import { RootLifecycleService, createServiceFactory, } from '@backstage/backend-plugin-api'; -import { BackendStartupOptions, ServiceOrExtensionPoint } from './types'; +import { Config } from '@backstage/config'; +import { ServiceOrExtensionPoint } from './types'; // Direct internal import to avoid duplication // eslint-disable-next-line @backstage/no-relative-monorepo-imports import type { @@ -326,19 +327,14 @@ export class BackendInitializer { 'root', ); - const startupOptions = - rootConfig - ?.getOptionalConfig('backend.startup') - ?.get() ?? {}; - - // Gather backend pluginIds marked as optional, since these should not cause a startup failure - const optionalBackends = Object.entries(startupOptions) - .filter(([_, value]) => value?.optional) - .map(([key]) => key); - // All plugins are initialized in parallel const results = await Promise.allSettled( allPluginIds.map(async pluginId => { + const isPluginOptional = this.#getPluginOptionalityPredicate( + pluginId, + rootConfig, + ); + try { // Initialize all eager services await this.#serviceRegistry.initializeEagerServicesWithScope( @@ -407,7 +403,7 @@ export class BackendInitializer { await lifecycleService.startup(); } catch (error: unknown) { assertError(error); - if (optionalBackends.includes(pluginId)) { + if (isPluginOptional) { initLogger.onOptionalPluginFailed(pluginId, error); } else { initLogger.onPluginFailed(pluginId, error); @@ -639,6 +635,17 @@ export class BackendInitializer { } } } + + #getPluginOptionalityPredicate(pluginId: string, config?: Config): boolean { + const defaultStartupOptionalValue = + config?.getOptionalBoolean('backend.startup.default.optional') ?? false; + + return ( + config?.getOptionalBoolean( + `backend.startup.plugins.${pluginId}.optional`, + ) ?? defaultStartupOptionalValue + ); + } } function toInternalBackendFeature( diff --git a/packages/backend-app-api/src/wiring/types.ts b/packages/backend-app-api/src/wiring/types.ts index 0b19ec66ea..cea77268be 100644 --- a/packages/backend-app-api/src/wiring/types.ts +++ b/packages/backend-app-api/src/wiring/types.ts @@ -48,13 +48,12 @@ export type ServiceOrExtensionPoint = * @public */ export type BackendStartupOptions = { - [pluginId: string]: { - /** - * Used to mark plugins as optional, which allows the backend to start up even in the event - * of a plugin failure. Plugin failures without this configuration are fatal. This can - * enable leaving a crashing plugin installed, but still permit backend startup, which may - * help troubleshoot data-dependent issues. - */ + default?: { optional?: boolean; }; + plugins?: { + [pluginId: string]: { + optional?: boolean; + }; + }; };