diff --git a/.changeset/thirty-fireants-occur.md b/.changeset/thirty-fireants-occur.md new file mode 100644 index 0000000000..dc9348ac6e --- /dev/null +++ b/.changeset/thirty-fireants-occur.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Detect a duplicate entities when adding locations through dry run diff --git a/plugins/catalog-backend/src/service/DefaultLocationService.test.ts b/plugins/catalog-backend/src/service/DefaultLocationService.test.ts index a089989d20..562bae4867 100644 --- a/plugins/catalog-backend/src/service/DefaultLocationService.test.ts +++ b/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, diff --git a/plugins/catalog-backend/src/service/DefaultLocationService.ts b/plugins/catalog-backend/src/service/DefaultLocationService.ts index 2166341935..8dd2274842 100644 --- a/plugins/catalog-backend/src/service/DefaultLocationService.ts +++ b/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 { + 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,