diff --git a/.changeset/wild-jobs-greet.md b/.changeset/wild-jobs-greet.md new file mode 100644 index 0000000000..fdbe6d56ac --- /dev/null +++ b/.changeset/wild-jobs-greet.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-app-api': patch +--- + +Prevents root extension override and duplicated plugin extensions. diff --git a/packages/frontend-app-api/package.json b/packages/frontend-app-api/package.json index 772dd62a36..7cd4557ea1 100644 --- a/packages/frontend-app-api/package.json +++ b/packages/frontend-app-api/package.json @@ -24,6 +24,7 @@ }, "devDependencies": { "@backstage/cli": "workspace:^", + "@backstage/test-utils": "workspace:^", "@testing-library/jest-dom": "^5.10.1" }, "configSchema": "config.d.ts", diff --git a/packages/frontend-app-api/src/wiring/createApp.test.tsx b/packages/frontend-app-api/src/wiring/createApp.test.tsx new file mode 100644 index 0000000000..903b044ed3 --- /dev/null +++ b/packages/frontend-app-api/src/wiring/createApp.test.tsx @@ -0,0 +1,106 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + createExtension, + createPageExtension, + createPlugin, +} from '@backstage/frontend-plugin-api'; +import { createInstances } from './createApp'; + +import { MockConfigApi } from '@backstage/test-utils'; +import React from 'react'; +import { createRouteRef } from '@backstage/core-plugin-api'; + +describe('createInstances', () => { + it('throws an error when a root extension is parametrized', () => { + const config = new MockConfigApi({ + app: { + extensions: [ + { + root: { + at: '', + }, + }, + ], + }, + }); + const plugins = [ + createPlugin({ + id: 'plugin', + extensions: [], + }), + ]; + expect(() => createInstances({ config, plugins })).toThrow( + "A 'root' extension configuration was detected, but the root extension is not configurable", + ); + }); + + it('throws an error when a root extension is overridden', () => { + const config = new MockConfigApi({}); + const plugins = [ + createPlugin({ + id: 'plugin', + extensions: [ + createExtension({ + id: 'root', + at: 'core.routes/route', + inputs: {}, + output: {}, + factory() {}, + }), + ], + }), + ]; + expect(() => createInstances({ config, plugins })).toThrow( + "The following plugin(s) are overriding the 'root' extension which is forbidden: plugin", + ); + }); + + it('throws an error when duplicated extensions are detected', () => { + const config = new MockConfigApi({}); + + const ExtensionA = createPageExtension({ + id: 'A', + defaultPath: '/', + routeRef: createRouteRef({ id: 'A.route' }), + loader: async () =>
Extension A
, + }); + + const ExtensionB = createPageExtension({ + id: 'B', + defaultPath: '/', + routeRef: createRouteRef({ id: 'B.route' }), + loader: async () =>
Extension B
, + }); + + const PluginA = createPlugin({ + id: 'A', + extensions: [ExtensionA, ExtensionA], + }); + + const PluginB = createPlugin({ + id: 'B', + extensions: [ExtensionA, ExtensionB, ExtensionB], + }); + + const plugins = [PluginA, PluginB]; + + expect(() => createInstances({ config, plugins })).toThrow( + "The following extensions are duplicated: The extension 'A' was provided 2 time(s) by the plugin 'A' and 1 time(s) by the plugin 'B', The extension 'B' was provided 2 time(s) by the plugin 'B'", + ); + }); +}); diff --git a/packages/frontend-app-api/src/wiring/createApp.tsx b/packages/frontend-app-api/src/wiring/createApp.tsx index 35d2af6f6d..462bfcdccc 100644 --- a/packages/frontend-app-api/src/wiring/createApp.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.tsx @@ -209,18 +209,18 @@ export function createInstances(options: { function createInstance( instanceParams: ExtensionInstanceParameters, ): ExtensionInstance { - const existingInstance = instances.get(instanceParams.extension.id); + const extensionId = instanceParams.extension.id; + const existingInstance = instances.get(extensionId); if (existingInstance) { return existingInstance; } const attachments = new Map( - Array.from( - attachmentMap.get(instanceParams.extension.id)?.entries() ?? [], - ).map(([inputName, attachmentConfigs]) => [ - inputName, - attachmentConfigs.map(createInstance), - ]), + Array.from(attachmentMap.get(extensionId)?.entries() ?? []).map( + ([inputName, attachmentConfigs]) => { + return [inputName, attachmentConfigs.map(createInstance)]; + }, + ), ); const newInstance = createExtensionInstance({ @@ -230,7 +230,7 @@ export function createInstances(options: { attachments, }); - instances.set(instanceParams.extension.id, newInstance); + instances.set(extensionId, newInstance); return newInstance; } diff --git a/packages/frontend-app-api/src/wiring/parameters.ts b/packages/frontend-app-api/src/wiring/parameters.ts index 1b2831aaca..efe0fbf678 100644 --- a/packages/frontend-app-api/src/wiring/parameters.ts +++ b/packages/frontend-app-api/src/wiring/parameters.ts @@ -196,18 +196,32 @@ export function mergeExtensionParameters(options: { }): ExtensionInstanceParameters[] { const { sources, builtinExtensions, parameters } = options; + const pluginExtensions = sources.flatMap(source => { + return source.extensions.map(extension => ({ ...extension, source })); + }); + + // Prevent root override + if (pluginExtensions.some(({ id }) => id === 'root')) { + const rootPluginIds = pluginExtensions + .filter(({ id }) => id === 'root') + .map(({ source }) => source.id); + throw new Error( + `The following plugin(s) are overriding the 'root' extension which is forbidden: ${rootPluginIds.join( + ',', + )}`, + ); + } + const overrides = [ - ...sources.flatMap(plugin => - plugin.extensions.map(extension => ({ - extension, - params: { - source: plugin, - at: extension.at, - disabled: extension.disabled, - config: undefined as unknown, - }, - })), - ), + ...pluginExtensions.map(({ source, ...extension }) => ({ + extension, + params: { + source, + at: extension.at, + disabled: extension.disabled, + config: undefined as unknown, + }, + })), ...builtinExtensions.map(extension => ({ extension, params: { @@ -219,9 +233,53 @@ export function mergeExtensionParameters(options: { })), ]; + const duplicatedExtensionIds = new Set(); + const duplicatedExtensionData = overrides.reduce< + Record> + >((data, { extension, params }) => { + const extensionId = extension.id; + const extensionData = data?.[extensionId]; + if (extensionData) duplicatedExtensionIds.add(extensionId); + const pluginId = params.source?.id ?? 'internal'; + const pluginCount = extensionData?.[pluginId] ?? 0; + return { + ...data, + [extensionId]: { ...extensionData, [pluginId]: pluginCount + 1 }, + }; + }, {}); + + if (duplicatedExtensionIds.size > 0) { + throw new Error( + `The following extensions are duplicated: ${Array.from( + duplicatedExtensionIds, + ) + .map( + extensionId => + `The extension '${extensionId}' was provided ${Object.keys( + duplicatedExtensionData[extensionId], + ) + .map( + pluginId => + `${duplicatedExtensionData[extensionId][pluginId]} time(s) by the plugin '${pluginId}'`, + ) + .join(' and ')}`, + ) + .join(', ')}`, + ); + } + for (const overrideParam of parameters) { + const extensionId = overrideParam.id; + + // Prevent root parametrization + if (extensionId === 'root') { + throw new Error( + "A 'root' extension configuration was detected, but the root extension is not configurable", + ); + } + const existingIndex = overrides.findIndex( - e => e.extension.id === overrideParam.id, + e => e.extension.id === extensionId, ); if (existingIndex !== -1) { const existing = overrides[existingIndex]; @@ -243,7 +301,7 @@ export function mergeExtensionParameters(options: { } } } else { - throw new Error(`Extension ${overrideParam.id} does not exist`); + throw new Error(`Extension ${extensionId} does not exist`); } } diff --git a/yarn.lock b/yarn.lock index 0d7896d082..9cb61d15f6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4213,6 +4213,7 @@ __metadata: "@backstage/core-plugin-api": "workspace:^" "@backstage/frontend-plugin-api": "workspace:^" "@backstage/plugin-graphiql": "workspace:^" + "@backstage/test-utils": "workspace:^" "@backstage/types": "workspace:^" "@material-ui/core": ^4.12.4 "@testing-library/jest-dom": ^5.10.1