Merge pull request #8164 from RoadieHQ/infinite-location-loop

Fail when adding duplicate locations using the dry run flag

.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: location:default/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

@@ -19,6 +19,7 @@ import {
   LocationSpec,
   LOCATION_ANNOTATION,
   ORIGIN_LOCATION_ANNOTATION,
+  stringifyEntityRef,
 } from '@backstage/catalog-model';
 import {
   CatalogProcessingOrchestrator,
@@ -54,6 +55,43 @@ export class DefaultLocationService implements LocationService {
     return this.store.deleteLocation(id);
   }
 
+  private async processEntities(
+    unprocessedEntities: DeferredEntity[],
+  ): Promise<Entity[]> {
+    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) {
+        if (
+          entities.some(
+            e =>
+              stringifyEntityRef(e) ===
+              stringifyEntityRef(processed.completedEntity),
+          )
+        ) {
+          throw new Error(
+            `Duplicate nested entity: ${stringifyEntityRef(
+              processed.completedEntity,
+            )}`,
+          );
+        }
+        unprocessedEntities.push(...processed.deferredEntities);
+        entities.push(processed.completedEntity);
+      } else {
+        throw Error(processed.errors.map(String).join(', '));
+      }
+    }
+    return entities;
+  }
+
   private async dryRunCreateLocation(
     spec: LocationSpec,
   ): Promise<{ location: Location; entities: Entity[]; exists?: boolean }> {
@@ -86,24 +124,7 @@ 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.processEntities(unprocessedEntities);
 
     return {
       exists: await existsPromise,