Fail when adding duplicate locations using the dry run flag

This change here adds in logic to the DefaultLocationService that will prematurely terminate the dry run addition of locations if there is a duplicate entity in the unprocessed entities.
Adding this logic here will avoid an infinite loop if a monorepo arch is used (for example if a target references itself). It will also enforce some best practices.
I have also added tests.

You can test this out using this catalog-info.yaml: https://github.com/RoadieHQ/monorepo-infinte-loop/blob/main/catalog-info.yaml

Signed-off-by: Nicolas Arnold <nic@roadie.io>

.changeset/thirty-fireants-occur.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend': patch
+---
+
+Detect a duplicate entities when adding locations through dry run

plugins/catalog-backend/src/service/DefaultLocationService.test.ts

@@ -148,6 +148,57 @@ describe('DefaultLocationServiceTest', () => {
       expect(result.exists).toBe(true);
     });
 
+    it('should fail when there are duplicate entities using dry run', async () => {
+      store.listLocations.mockResolvedValueOnce([]);
+      orchestrator.process.mockResolvedValueOnce({
+        ok: true,
+        state: {},
+        completedEntity: {
+          apiVersion: 'backstage.io/v1alpha1',
+          kind: 'Location',
+          metadata: {
+            name: 'foo',
+          },
+        },
+        deferredEntities: [
+          {
+            entity: {
+              apiVersion: 'backstage.io/v1alpha1',
+              kind: 'Location',
+              metadata: {
+                name: 'foo',
+              },
+            },
+            locationKey: 'file:///tmp/mock.yaml',
+          },
+        ],
+        relations: [],
+        errors: [],
+      });
+
+      orchestrator.process.mockResolvedValueOnce({
+        ok: true,
+        state: {},
+        completedEntity: {
+          apiVersion: 'backstage.io/v1alpha1',
+          kind: 'Location',
+          metadata: {
+            name: 'foo',
+          },
+        },
+        deferredEntities: [],
+        relations: [],
+        errors: [],
+      });
+
+      await expect(
+        locationService.createLocation(
+          { type: 'url', target: 'https://backstage.io/catalog-info.yaml' },
+          true,
+        ),
+      ).rejects.toThrowError('Duplicate nested entity: foo');
+    });
+
     it('should return exists false when the location does not exist beforehand', async () => {
       orchestrator.process.mockResolvedValueOnce({
         ok: true,

plugins/catalog-backend/src/service/DefaultLocationService.ts

@@ -54,6 +54,34 @@ export class DefaultLocationService implements LocationService {
     return this.store.deleteLocation(id);
   }
 
+  private async getUnprocessedEntities(
+    unprocessed: DeferredEntity[],
+    entities: Entity[],
+  ): Promise<Entity[]> {
+    const currentEntity = unprocessed.pop();
+    if (!currentEntity) {
+      return [];
+    }
+    const processed = await this.orchestrator.process({
+      entity: currentEntity.entity,
+      state: {}, // we process without the existing cache
+    });
+
+    if (processed.ok) {
+      const { metadata, kind } = processed.completedEntity;
+      if (
+        entities.some(e => e.metadata.name === metadata.name && e.kind === kind)
+      ) {
+        throw new Error(`Duplicate nested entity: ${metadata.name}`);
+      }
+      return await this.getUnprocessedEntities(
+        [...unprocessed, ...processed.deferredEntities],
+        [...entities, processed.completedEntity],
+      );
+    }
+    throw Error(processed.errors.map(String).join(', '));
+  }
+
   private async dryRunCreateLocation(
     spec: LocationSpec,
   ): Promise<{ location: Location; entities: Entity[]; exists?: boolean }> {
@@ -86,24 +114,10 @@ export class DefaultLocationService implements LocationService {
     const unprocessedEntities: DeferredEntity[] = [
       { entity, locationKey: `${spec.type}:${spec.target}` },
     ];
-    const entities: Entity[] = [];
-    while (unprocessedEntities.length) {
-      const currentEntity = unprocessedEntities.pop();
-      if (!currentEntity) {
-        continue;
-      }
-      const processed = await this.orchestrator.process({
-        entity: currentEntity.entity,
-        state: {}, // we process without the existing cache
-      });
-
-      if (processed.ok) {
-        unprocessedEntities.push(...processed.deferredEntities);
-        entities.push(processed.completedEntity);
-      } else {
-        throw Error(processed.errors.map(String).join(', '));
-      }
-    }
+    const entities: Entity[] = await this.getUnprocessedEntities(
+      unprocessedEntities,
+      [],
+    );
 
     return {
       exists: await existsPromise,