From 6e8bb3ac076765280a591bc50e76ea6240892a9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 9 Dec 2020 16:10:30 +0100 Subject: [PATCH] catalog-backend: leave unknown placeholder-lookalikes untouched --- .changeset/breezy-chefs-report.md | 5 +++ .../processors/PlaceholderProcessor.test.ts | 38 ++++++++----------- .../processors/PlaceholderProcessor.ts | 19 +++++++--- 3 files changed, 34 insertions(+), 28 deletions(-) create mode 100644 .changeset/breezy-chefs-report.md diff --git a/.changeset/breezy-chefs-report.md b/.changeset/breezy-chefs-report.md new file mode 100644 index 0000000000..baaec3021f --- /dev/null +++ b/.changeset/breezy-chefs-report.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +leave unknown placeholder-lookalikes untouched in the catalog processing loop diff --git a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts index 5a3511af6d..8b2632ad5d 100644 --- a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts @@ -88,7 +88,7 @@ describe('PlaceholderProcessor', () => { ); }); - it('rejects multiple placeholders', async () => { + it('ignores multiple placeholders', async () => { const processor = new PlaceholderProcessor({ resolvers: { foo: jest.fn(), @@ -96,41 +96,35 @@ describe('PlaceholderProcessor', () => { }, reader, }); + const entity: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'n', x: { $foo: 'a', $bar: 'b' } }, + }; await expect( - processor.preProcessEntity( - { - apiVersion: 'a', - kind: 'k', - metadata: { name: 'n', x: { $foo: 'a', $bar: 'b' } }, - }, - { type: 'a', target: 'b' }, - ), - ).rejects.toThrow( - 'Placeholders have to be on the form of a single $-prefixed key in an object', - ); + processor.preProcessEntity(entity, { type: 'a', target: 'b' }), + ).resolves.toEqual(entity); expect(read).not.toBeCalled(); }); - it('rejects unknown placeholders', async () => { + it('ignores unknown placeholders', async () => { const processor = new PlaceholderProcessor({ resolvers: { bar: jest.fn(), }, reader, }); + const entity: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'n', x: { $foo: 'a' } }, + }; await expect( - processor.preProcessEntity( - { - apiVersion: 'a', - kind: 'k', - metadata: { name: 'n', x: { $foo: 'a' } }, - }, - { type: 'a', target: 'b' }, - ), - ).rejects.toThrow('Encountered unknown placeholder $foo'); + processor.preProcessEntity(entity, { type: 'a', target: 'b' }), + ).resolves.toEqual(entity); expect(read).not.toBeCalled(); }); diff --git a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts index 0e23d51f61..b7b3df7b6e 100644 --- a/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.ts @@ -76,21 +76,28 @@ export class PlaceholderProcessor implements CatalogProcessor { ? [data, false] : [Object.fromEntries(entries.map(([k, [v]]) => [k, v])), true]; } else if (keys.length !== 1) { - throw new Error( - 'Placeholders have to be on the form of a single $-prefixed key in an object', - ); + // This was an object that had more than one key, some of which were + // dollar prefixed. We only handle the case where there is exactly one + // such key; anything else is left alone. + return [data, false]; } const resolverKey = keys[0].substr(1); + const resolverValue = data[keys[0]]; const resolver = this.options.resolvers[resolverKey]; - if (!resolver) { - throw new Error(`Encountered unknown placeholder \$${resolverKey}`); + if (!resolver || typeof resolverValue !== 'string') { + // If there was no such placeholder resolver or if the value was not a + // string, we err on the side of safety and assume that this is + // something that's best left alone. For example, if the input contains + // JSONSchema, there may be "$ref": "#/definitions/node" nodes in the + // document. + return [data, false]; } return [ await resolver({ key: resolverKey, - value: data[keys[0]], + value: resolverValue, baseUrl: location.target, read: this.options.reader.read.bind(this.options.reader), }),