diff --git a/packages/frontend-app-api/package.json b/packages/frontend-app-api/package.json index c085c6dba6..65c8a65e71 100644 --- a/packages/frontend-app-api/package.json +++ b/packages/frontend-app-api/package.json @@ -35,6 +35,7 @@ "@backstage/config": "workspace:^", "@backstage/frontend-plugin-api": "workspace:^", "@backstage/plugin-graphiql": "workspace:^", + "@backstage/types": "workspace:^", "lodash": "^4.17.21" }, "peerDependencies": { diff --git a/packages/frontend-app-api/src/createApp.tsx b/packages/frontend-app-api/src/createApp.tsx index ecc86a5ed7..031ad031bd 100644 --- a/packages/frontend-app-api/src/createApp.tsx +++ b/packages/frontend-app-api/src/createApp.tsx @@ -56,6 +56,9 @@ export function createApp(options: { plugins: BackstagePlugin[] }): { readAppExtensionParameters(appConfig), ); + // TODO: validate the config of all extension instances + // We do it at this point to ensure that merging (if any) of config has already happened + // Create attachment map so that we can look attachments up during instance creation const attachmentMap = new Map< string, diff --git a/packages/frontend-app-api/src/wiring/parameters.test.ts b/packages/frontend-app-api/src/wiring/parameters.test.ts index ac50c555c3..9da03bb563 100644 --- a/packages/frontend-app-api/src/wiring/parameters.test.ts +++ b/packages/frontend-app-api/src/wiring/parameters.test.ts @@ -16,7 +16,9 @@ import { ConfigReader } from '@backstage/config'; import { Extension } from '@backstage/frontend-plugin-api'; +import { JsonValue } from '@backstage/types'; import { + expandShorthandExtensionParameters, mergeExtensionParameters, readAppExtensionParameters, } from './parameters'; @@ -253,7 +255,183 @@ describe('readAppExtensionParameters', () => { }), ), ).toThrow( - `Invalid extension configuration at app.extensions[0][core.router/routes], must not specify 'at' when using attachment shorthand form`, + `Invalid extension configuration at app.extensions[0][core.router/routes], must not redundantly specify 'at' when the extension input ID form of the key is used (with a slash); the 'at' is already implicitly 'core.router/routes'`, + ); + }); +}); + +describe('expandShorthandExtensionParameters', () => { + const resolveExtensionRef = jest.fn( + ref => ({ ref } as unknown as Extension), + ); + const generateExtensionId = jest.fn(() => 'generated.1'); + const run = (value: JsonValue) => { + return expandShorthandExtensionParameters( + value, + 1, + resolveExtensionRef, + generateExtensionId, + ); + }; + + it('rejects unknown keys', () => { + expect(() => run(null)).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1], must be a string or an object"`, + ); + expect(() => run(1)).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1], must be a string or an object"`, + ); + expect(() => run([])).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1], must be a string or an object"`, + ); + }); + + it('rejects the wrong number of keys', () => { + expect(() => run({})).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1], must have exactly one key, got none"`, + ); + expect(() => run({ a: {}, b: {} })).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1], must have exactly one key, got 'a', 'b'"`, + ); + }); + + it('rejects unknown values', () => { + expect(() => run({ a: null })).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][a], value must be a boolean, string, or object"`, + ); + expect(() => run({ a: 1 })).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][a], value must be a boolean, string, or object"`, + ); + expect(() => run({ a: [] })).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][a], value must be a boolean, string, or object"`, + ); + }); + + it('supports string key', () => { + expect(run('core.router')).toEqual({ + id: 'core.router', + }); + expect(() => run('')).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1], string shorthand cannot be the empty string"`, + ); + expect(() => run('core.router/routes')).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1], cannot target an extension instance input with the string shorthand (key cannot contain slashes; did you mean 'core.router'?)"`, + ); + }); + + it('supports boolean value', () => { + expect(run({ 'core.router': true })).toEqual({ + id: 'core.router', + disabled: false, + }); + expect(run({ 'core.router': false })).toEqual({ + id: 'core.router', + disabled: true, + }); + expect(() => + run({ 'core.router/routes': false }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][core.router/routes], cannot target an extension instance input (key cannot contain slashes; did you mean 'core.router'?)"`, + ); + }); + + it('supports string values', () => { + expect(run({ 'core.router': 'example-package#MyRouter' })).toEqual({ + id: 'core.router', + extension: { ref: 'example-package#MyRouter' }, + }); + expect(run({ 'core.router/routes': 'example-package#MyRouter' })).toEqual({ + id: 'generated.1', + at: 'core.router/routes', + extension: { ref: 'example-package#MyRouter' }, + }); + }); + + it('supports object id', () => { + expect( + run({ 'core.router/routes': { id: 'example-package#MyRouter' } }), + ).toEqual({ + id: 'example-package#MyRouter', + at: 'core.router/routes', + }); + expect(() => + run({ + 'core.router/routes': { + id: 'example-package#MyRouter', + at: 'core.router/routes', + }, + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][core.router/routes], must not redundantly specify 'at' when the extension input ID form of the key is used (with a slash); the 'at' is already implicitly 'core.router/routes'"`, + ); + }); + + it('supports object at', () => { + expect(run({ 'core.router': { at: 'other.root/inputs' } })).toEqual({ + id: 'core.router', + at: 'other.root/inputs', + }); + expect(() => + run({ + 'core.router': { + id: 'other-id', + }, + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][core.router], must not redundantly specify 'id' when the extension instance ID form of the key is used (without a slash); the 'id' is already implicitly 'core.router'"`, + ); + }); + + it('supports object disabled', () => { + expect(run({ 'core.router': { disabled: true } })).toEqual({ + id: 'core.router', + disabled: true, + }); + expect(run({ 'core.router': { disabled: false } })).toEqual({ + id: 'core.router', + disabled: false, + }); + expect(() => + run({ 'core.router': { disabled: 0 } }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][core.router].disabled, must be a boolean"`, + ); + }); + + it('supports object extension', () => { + expect( + run({ 'core.router/routes': { extension: 'example-package#MyRouter' } }), + ).toEqual({ + id: 'generated.1', + at: 'core.router/routes', + extension: { ref: 'example-package#MyRouter' }, + }); + expect(() => + run({ 'core.router/routes': { extension: 0 } }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][core.router/routes].extension, must be a string"`, + ); + }); + + it('supports object config', () => { + expect( + run({ 'core.router': { config: { disableRedirects: true } } }), + ).toEqual({ + id: 'core.router', + config: { disableRedirects: true }, + }); + expect(() => + run({ 'core.router': { config: 0 } }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][core.router].config, must be an object"`, + ); + }); + + it('rejects unknown object keys', () => { + expect(() => + run({ 'core.router': { foo: { settings: true } } }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][core.router].foo, unknown parameter; expected one of 'id', 'at', 'disabled', 'extension', 'config'"`, ); }); }); diff --git a/packages/frontend-app-api/src/wiring/parameters.ts b/packages/frontend-app-api/src/wiring/parameters.ts index 140dcd0367..4d934d8e99 100644 --- a/packages/frontend-app-api/src/wiring/parameters.ts +++ b/packages/frontend-app-api/src/wiring/parameters.ts @@ -19,6 +19,16 @@ import { Extension, ExtensionInstanceParameters, } from '@backstage/frontend-plugin-api'; +import { JsonValue } from '@backstage/types'; +import omitBy from 'lodash/omitBy'; + +const knownExtensionInstanceParameters = [ + 'id', + 'at', + 'disabled', + 'extension', + 'config', +]; // Since we'll never merge arrays in config the config reader context // isn't too much of a help. Fall back to manual config reading logic @@ -40,93 +50,202 @@ export function readAppExtensionParameters( return []; } - const generateIndex = (() => { - let generatedIndex = 1; - return () => `generated.${generatedIndex++}`; + const generateExtensionId = (() => { + let index = 1; + return () => `generated.${index++}`; })(); - return arr.map((arrayEntry, index) => { - function errorMsg(msg: string, key?: string, prop?: string) { - return `Invalid extension configuration at app.extensions[${index}]${ - key ? `[${key}]` : '' - }${prop ? `.${prop}` : ''}, ${msg}`; - } + return arr.map((arrayEntry, arrayIndex) => + expandShorthandExtensionParameters( + arrayEntry, + arrayIndex, + resolveExtensionRef, + generateExtensionId, + ), + ); +} - // Example YAML: - // - entity.card.about - if (typeof arrayEntry === 'string') { - return { id: arrayEntry }; - } +/** @internal */ +export function expandShorthandExtensionParameters( + arrayEntry: JsonValue, + arrayIndex: number, + resolveExtensionRef: (ref: string) => Extension, + generateExtensionId: () => string, +): Partial { + function errorMsg(msg: string, key?: string, prop?: string) { + return `Invalid extension configuration at app.extensions[${arrayIndex}]${ + key ? `[${key}]` : '' + }${prop ? `.${prop}` : ''}, ${msg}`; + } - // All other forms are single-key objects - if ( - typeof arrayEntry !== 'object' || - arrayEntry === null || - Array.isArray(arrayEntry) - ) { - throw new Error(errorMsg('must be a string or an object')); + // Example YAML: + // - entity.card.about + if (typeof arrayEntry === 'string') { + if (arrayEntry.includes('/')) { + const suggestion = arrayEntry.split('/')[0]; + throw new Error( + errorMsg( + `cannot target an extension instance input with the string shorthand (key cannot contain slashes; did you mean '${suggestion}'?)`, + ), + ); } - const keys = Object.keys(arrayEntry); - if (keys.length !== 1) { - const joinedKeys = `"${keys.join('", "')}"`; - throw new Error(errorMsg(`must have exactly one key, got ${joinedKeys}`)); + if (!arrayEntry) { + throw new Error(errorMsg('string shorthand cannot be the empty string')); } - - const key = keys[0]; - let value = arrayEntry[key]; - - let at = key.includes('/') ? key : undefined; - const id = at ? generateIndex() : key; - if (typeof value === 'boolean') { - if (at) { - throw new Error( - errorMsg('cannot be applied to an instance input', key), - ); - } - value = { disabled: !value }; - } else if (typeof value === 'string') { - value = { extension: value }; - } else if ( - typeof value !== 'object' || - value === null || - Array.isArray(value) - ) { - throw new Error(errorMsg('must be an object', key)); - } - - const atRef = value.at; - if (atRef !== undefined && typeof atRef !== 'string') { - throw new Error(errorMsg('must be a string', key, 'at')); - } else if (atRef !== undefined) { - if (at) { - throw new Error( - errorMsg( - `must not specify 'at' when using attachment shorthand form`, - key, - ), - ); - } - at = atRef; - } - const disabled = value.disabled; - if (disabled !== undefined && typeof disabled !== 'boolean') { - throw new Error(errorMsg('must be a boolean', key, 'disabled')); - } - const extensionRef = value.extension; - if (extensionRef !== undefined && typeof extensionRef !== 'string') { - throw new Error(errorMsg('must be a string', key, 'extension')); - } - const extension = extensionRef - ? resolveExtensionRef(extensionRef) - : undefined; return { + id: arrayEntry, + }; + } + + // All remaining cases are single-key objects + if ( + typeof arrayEntry !== 'object' || + arrayEntry === null || + Array.isArray(arrayEntry) + ) { + throw new Error(errorMsg('must be a string or an object')); + } + const keys = Object.keys(arrayEntry); + if (keys.length !== 1) { + const joinedKeys = keys.length ? `'${keys.join("', '")}'` : 'none'; + throw new Error(errorMsg(`must have exactly one key, got ${joinedKeys}`)); + } + + const key = keys[0]; + const value = arrayEntry[key]; + + // Example YAML: + // - catalog.page.cicd: false + if (typeof value === 'boolean') { + if (key.includes('/')) { + const suggestion = key.split('/')[0]; + throw new Error( + errorMsg( + `cannot target an extension instance input (key cannot contain slashes; did you mean '${suggestion}'?)`, + key, + ), + ); + } + + return { + id: key, + disabled: !value, + }; + } + + // Example YAML: + // - core.router/routes: '@backstage/plugin-some-plugin#MyPage' + // - some-plugin.page: '@internal/frontend-customizations#MyModifiedPage' + if (typeof value === 'string') { + if (key.includes('/')) { + return { + id: generateExtensionId(), + at: key, + extension: resolveExtensionRef(value), + }; + } + + return { + id: key, + extension: resolveExtensionRef(value), + }; + } + + // The remaining case is the generic object. Example YAML: + // - core.router/routes: + // id: redirects.catalog + // extension: '@backstage/core-app-api#Redirect' + // config: + // path: / + // to: /catalog + // - tech-radar.page: + // at: core.router/routes + // extension: '@backstage/plugin-tech-radar#TechRadarPage' + // config: + // path: /tech-radar + // width: 1500 + // height: 800 + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + throw new Error( + errorMsg('value must be a boolean, string, or object', key), + ); + } + + let id = value.id; + let at = value.at; + const disabled = value.disabled; + const extensionRef = value.extension; + const config = value.config; + + if (key.includes('/')) { + if (at !== undefined) { + throw new Error( + errorMsg( + `must not redundantly specify 'at' when the extension input ID form of the key is used (with a slash); the 'at' is already implicitly '${key}'`, + key, + ), + ); + } + if (id === undefined) { + id = generateExtensionId(); + } + at = key; + } else { + if (id !== undefined) { + throw new Error( + errorMsg( + `must not redundantly specify 'id' when the extension instance ID form of the key is used (without a slash); the 'id' is already implicitly '${key}'`, + key, + ), + ); + } + id = key; + } + + if (id !== undefined && typeof id !== 'string') { + throw new Error(errorMsg('must be a string', key, 'id')); + } else if (at !== undefined && typeof at !== 'string') { + throw new Error(errorMsg('must be a string', key, 'at')); + } else if (disabled !== undefined && typeof disabled !== 'boolean') { + throw new Error(errorMsg('must be a boolean', key, 'disabled')); + } else if (extensionRef !== undefined && typeof extensionRef !== 'string') { + throw new Error(errorMsg('must be a string', key, 'extension')); + } else if ( + config !== undefined && + (typeof config !== 'object' || config === null || Array.isArray(config)) + ) { + throw new Error(errorMsg('must be an object', key, 'config')); + } + + const unknownKeys = Object.keys(value).filter( + k => !knownExtensionInstanceParameters.includes(k), + ); + if (unknownKeys.length > 0) { + throw new Error( + errorMsg( + `unknown parameter; expected one of '${knownExtensionInstanceParameters.join( + "', '", + )}'`, + key, + unknownKeys.join(', '), + ), + ); + } + + const extension: Extension | undefined = extensionRef + ? resolveExtensionRef(extensionRef) + : undefined; + + return omitBy( + { id, at, disabled, extension, - config: value.config /* validate later */, - }; - }); + config, + }, + v => v === undefined, + ); } /** @internal */ diff --git a/yarn.lock b/yarn.lock index cc6ad40082..0efabb00de 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4140,6 +4140,7 @@ __metadata: "@backstage/config": "workspace:^" "@backstage/frontend-plugin-api": "workspace:^" "@backstage/plugin-graphiql": "workspace:^" + "@backstage/types": "workspace:^" "@testing-library/jest-dom": ^5.10.1 lodash: ^4.17.21 peerDependencies: