Merge pull request #3649 from backstage/freben/oops-dollar-ref

catalog-backend: leave unknown placeholder-lookalikes untouched

.changeset/breezy-chefs-report.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend': patch
+---
+
+leave unknown placeholder-lookalikes untouched in the catalog processing loop

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();
   });

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),
         }),