From b3fac9c107f59e2354fd6b8c57307113540d2579 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 13 Dec 2022 16:59:29 +0100 Subject: [PATCH] ignore processors emitting entities that are their own children MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/smart-zoos-wash.md | 5 +++++ .../DefaultCatalogProcessingOrchestrator.test.ts | 13 +++++++++++-- .../src/processing/ProcessorOutputCollector.ts | 12 ++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 .changeset/smart-zoos-wash.md diff --git a/.changeset/smart-zoos-wash.md b/.changeset/smart-zoos-wash.md new file mode 100644 index 0000000000..ffb5ca0f01 --- /dev/null +++ b/.changeset/smart-zoos-wash.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Ignore attempts at emitting the current entity as a child of itself. diff --git a/plugins/catalog-backend/src/processing/DefaultCatalogProcessingOrchestrator.test.ts b/plugins/catalog-backend/src/processing/DefaultCatalogProcessingOrchestrator.test.ts index 9731022582..ceb6992d7a 100644 --- a/plugins/catalog-backend/src/processing/DefaultCatalogProcessingOrchestrator.test.ts +++ b/plugins/catalog-backend/src/processing/DefaultCatalogProcessingOrchestrator.test.ts @@ -258,13 +258,22 @@ describe('DefaultCatalogProcessingOrchestrator', () => { }, }; + const child: Entity = { + apiVersion: '1', + kind: 'Component', + metadata: { + name: 'Test2', + namespace: 'test1', + }, + }; + it('enforces catalog rules', async () => { const integrations = ScmIntegrations.fromConfig(new ConfigReader({})); const processor: jest.Mocked = { getProcessorName: jest.fn(), validateEntityKind: jest.fn(async () => true), readLocation: jest.fn(async (_l, _o, emit) => { - emit(processingResult.entity({ type: 't', target: 't' }, entity)); + emit(processingResult.entity({ type: 't', target: 't' }, child)); return true; }), }; @@ -300,7 +309,7 @@ describe('DefaultCatalogProcessingOrchestrator', () => { getProcessorName: jest.fn(), validateEntityKind: jest.fn(async () => true), readLocation: jest.fn(async (_l, _o, emit) => { - emit(processingResult.entity({ type: 't', target: 't' }, entity)); + emit(processingResult.entity({ type: 't', target: 't' }, child)); return true; }), }; diff --git a/plugins/catalog-backend/src/processing/ProcessorOutputCollector.ts b/plugins/catalog-backend/src/processing/ProcessorOutputCollector.ts index 19f3b33df4..55282baa38 100644 --- a/plugins/catalog-backend/src/processing/ProcessorOutputCollector.ts +++ b/plugins/catalog-backend/src/processing/ProcessorOutputCollector.ts @@ -19,6 +19,7 @@ import { ANNOTATION_LOCATION, ANNOTATION_ORIGIN_LOCATION, stringifyLocationRef, + stringifyEntityRef, } from '@backstage/catalog-model'; import { assertError } from '@backstage/errors'; import { Logger } from 'winston'; @@ -89,6 +90,17 @@ export class ProcessorOutputCollector { return; } + // The processor contract says you should return the "trunk" (current) + // entity, not emit it. But it happens that this is misunderstood or + // accidentally forgotten. This can lead to circular references which at + // best is wasteful, so we try to be helpful by ignoring such emitted + // entities. + if ( + stringifyEntityRef(entity) === stringifyEntityRef(this.parentEntity) + ) { + return; + } + // Note that at this point, we have only validated the envelope part of // the entity data. Annotations are not part of that, so we have to be // defensive. If the annotations were malformed (e.g. were not a valid