From de96628b0ae0abf6e25b7a0a8a7fe237ed655d29 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 24 Sep 2022 17:00:53 +0200 Subject: [PATCH 01/10] catalog-backend: add initial integration test harness + test Signed-off-by: Patrik Oldsberg --- .../catalog-backend/src/integration.test.ts | 399 ++++++++++++++++++ .../DefaultCatalogProcessingEngine.ts | 2 +- 2 files changed, 400 insertions(+), 1 deletion(-) create mode 100644 plugins/catalog-backend/src/integration.test.ts diff --git a/plugins/catalog-backend/src/integration.test.ts b/plugins/catalog-backend/src/integration.test.ts new file mode 100644 index 0000000000..b702dc1180 --- /dev/null +++ b/plugins/catalog-backend/src/integration.test.ts @@ -0,0 +1,399 @@ +/* + * Copyright 2022 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Knex } from 'knex'; +import { Logger } from 'winston'; +import { ConfigReader } from '@backstage/config'; +import { JsonObject } from '@backstage/types'; +import { + BuiltinKindsEntityProcessor, + CatalogProcessingEngine, + EntityProvider, +} from './index'; +import { DatabaseManager, getVoidLogger } from '@backstage/backend-common'; +import { PermissionEvaluator } from '@backstage/plugin-permission-common'; +import { Entity, EntityPolicies } from '@backstage/catalog-model'; +import { defaultEntityDataParser } from './modules/util/parse'; +import { DefaultCatalogProcessingOrchestrator } from './processing/DefaultCatalogProcessingOrchestrator'; +import { applyDatabaseMigrations } from './database/migrations'; +import { DefaultProcessingDatabase } from './database/DefaultProcessingDatabase'; +import { ScmIntegrations } from '@backstage/integration'; +import { DefaultCatalogRulesEnforcer } from './ingestion/CatalogRules'; +import { Stitcher } from './stitching/Stitcher'; +import { DefaultEntitiesCatalog } from './service/DefaultEntitiesCatalog'; +import { DefaultCatalogProcessingEngine } from './processing/DefaultCatalogProcessingEngine'; +import { createHash } from 'crypto'; +import { DefaultRefreshService } from './service/DefaultRefreshService'; +import { connectEntityProviders } from './processing/connectEntityProviders'; +import { EntitiesCatalog } from './catalog/types'; +import { RefreshOptions, RefreshService } from './service/types'; +import { EntityProviderConnection } from '@backstage/plugin-catalog-node'; +import { RefreshStateItem } from './database/types'; + +const voidLogger = getVoidLogger(); + +class TestProvider implements EntityProvider { + #connection?: EntityProviderConnection; + + getProviderName(): string { + return 'test'; + } + + async connect(connection: EntityProviderConnection): Promise { + this.#connection = connection; + } + + getConnection() { + if (!this.#connection) { + throw new Error('Provider is not connected yet'); + } + return this.#connection; + } +} + +type ProgressTracker = NonNullable< + ConstructorParameters[7] +>; + +class ProxyProgressTracker implements ProgressTracker { + #inner: ProgressTracker; + + constructor(inner: ProgressTracker) { + this.#inner = inner; + } + + processStart(item: RefreshStateItem) { + return this.#inner.processStart(item, voidLogger); + } + + setTracker(tracker: ProgressTracker) { + this.#inner = tracker; + } +} + +class NoopProgressTracker implements ProgressTracker { + static emptyTracking = { + markFailed() {}, + markProcessorsCompleted() {}, + markSuccessfulWithChanges() {}, + markSuccessfulWithErrors() {}, + markSuccessfulWithNoChanges() {}, + }; + + processStart() { + return NoopProgressTracker.emptyTracking; + } +} + +class WaitingProgressTracker implements ProgressTracker { + #resolve: (errors: Record) => void; + #promise: Promise>; + #counts = new Map(); + #errors = new Map(); + #inFlight = new Array>(); + + constructor(private readonly entityRefs?: Set) { + let resolve: (errors: Record) => void; + this.#promise = new Promise>(_resolve => { + resolve = _resolve; + }); + this.#resolve = resolve!; + } + + processStart(item: RefreshStateItem) { + if (this.entityRefs && !this.entityRefs.has(item.entityRef)) { + return NoopProgressTracker.emptyTracking; + } + + let resolve: () => void; + this.#inFlight.push( + new Promise(_resolve => { + resolve = _resolve; + }), + ); + + const currentCount = this.#counts.get(item.id) ?? 0; + + const onDone = () => { + this.#counts.set(item.id, currentCount + 1); + + if (Array.from(this.#counts.values()).every(c => c >= 2)) { + this.#resolve(Object.fromEntries(this.#errors)); + } + }; + return { + markFailed: (error: Error) => { + this.#errors.set(item.entityRef, error); + onDone(); + resolve(); + }, + markProcessorsCompleted() {}, + markSuccessfulWithChanges: () => { + this.#errors.delete(item.entityRef); + this.#counts.set(item.id, 0); + resolve(); + }, + markSuccessfulWithErrors: () => { + this.#errors.delete(item.entityRef); + onDone(); + resolve(); + }, + markSuccessfulWithNoChanges: () => { + onDone(); + resolve(); + }, + }; + } + + async wait(): Promise> { + return this.#promise; + } + + async waitForFinish(): Promise { + await Promise.all(this.#inFlight.slice()); + } +} + +class TestHarness { + readonly #catalog: EntitiesCatalog; + readonly #engine: CatalogProcessingEngine; + readonly #refresh: RefreshService; + readonly #provider: TestProvider; + readonly #proxyProgressTracker: ProxyProgressTracker; + + static async create(options?: { + config?: JsonObject; + logger?: Logger; + db?: Knex; + permissions?: PermissionEvaluator; + processEntity?(entity: Entity): Promise; + onProcessingError?(event: { + unprocessedEntity: Entity; + errors: Error[]; + }): void; + }) { + const config = new ConfigReader( + options?.config ?? { + backend: { + database: { + client: 'better-sqlite3', + connection: ':memory:', + }, + }, + }, + ); + const logger = options?.logger ?? getVoidLogger(); + const db = + options?.db /* (await TestDatabases.create().init('SQLITE_3')); */ ?? + (await DatabaseManager.fromConfig(config, { logger }) + .forPlugin('catalog') + .getClient()); + + await applyDatabaseMigrations(db); + + const processingDatabase = new DefaultProcessingDatabase({ + database: db, + logger, + refreshInterval: () => 0.05, + }); + + const integrations = ScmIntegrations.fromConfig(config); + const rulesEnforcer = DefaultCatalogRulesEnforcer.fromConfig(config); + const orchestrator = new DefaultCatalogProcessingOrchestrator({ + processors: [ + { + getProcessorName: () => 'test', + async preProcessEntity(entity: Entity) { + if (options?.processEntity) { + return options?.processEntity(entity); + } + return entity; + }, + }, + new BuiltinKindsEntityProcessor(), + ], + integrations, + rulesEnforcer, + logger, + parser: defaultEntityDataParser, + // policy: new SchemaValidEntityPolicy(), + policy: EntityPolicies.allOf([]), + }); + const stitcher = new Stitcher(db, logger); + const catalog = new DefaultEntitiesCatalog(db, stitcher); + const proxyProgressTracker = new ProxyProgressTracker( + new NoopProgressTracker(), + ); + + const engine = new DefaultCatalogProcessingEngine( + logger, + processingDatabase, + orchestrator, + stitcher, + () => createHash('sha1'), + 50, + event => { + options?.onProcessingError?.(event); + }, + proxyProgressTracker, + ); + + const refresh = new DefaultRefreshService({ database: processingDatabase }); + + const provider = new TestProvider(); + + await connectEntityProviders(processingDatabase, [provider]); + + return new TestHarness( + catalog, + engine, + refresh, + provider, + proxyProgressTracker, + ); + } + + constructor( + catalog: EntitiesCatalog, + engine: CatalogProcessingEngine, + refresh: RefreshService, + provider: TestProvider, + proxyProgressTracker: ProxyProgressTracker, + ) { + this.#catalog = catalog; + this.#engine = engine; + this.#refresh = refresh; + this.#provider = provider; + this.#proxyProgressTracker = proxyProgressTracker; + } + + async process(entityRefs?: Set) { + this.#engine.start(); + + const tracker = new WaitingProgressTracker(entityRefs); + this.#proxyProgressTracker.setTracker(tracker); + const errors = await tracker.wait(); + + this.#engine.stop(); + await tracker.waitForFinish(); + + this.#proxyProgressTracker.setTracker(new NoopProgressTracker()); + + return errors; + } + + async setInputEntities(entities: Entity[]) { + return this.#provider.getConnection().applyMutation({ + type: 'full', + entities: entities.map(entity => ({ entity })), + }); + } + + async getOutputEntities(): Promise { + const { entities } = await this.#catalog.entities(); + return entities; + } + + async refresh(options: RefreshOptions) { + return this.#refresh.refresh(options); + } +} + +describe('Catalog Backend Integration', () => { + it('should add entities and update errors', async () => { + let triggerError = false; + + const harness = await TestHarness.create({ + async processEntity(entity: Entity) { + if (triggerError) { + delete entity.spec; + } + return entity; + }, + }); + + await harness.setInputEntities([ + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'test', + annotations: { + 'backstage.io/managed-by-location': 'url:.', + 'backstage.io/managed-by-origin-location': 'url:.', + }, + }, + spec: { + type: 'service', + owner: 'guest', + lifecycle: 'production', + }, + }, + ]); + + await expect(harness.getOutputEntities()).resolves.toEqual([]); + await expect(harness.process()).resolves.toEqual({}); + + await expect(harness.getOutputEntities()).resolves.toEqual([ + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: expect.objectContaining({ name: 'test' }), + spec: expect.objectContaining({ type: 'service' }), + relations: [ + { + target: { kind: 'group', namespace: 'default', name: 'guest' }, + type: 'ownedBy', + targetRef: 'group:default/guest', + }, + ], + }, + ]); + + triggerError = true; + + await expect(harness.process()).resolves.toEqual({}); + + await expect(harness.getOutputEntities()).resolves.toEqual([ + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: expect.objectContaining({ name: 'test' }), + spec: expect.objectContaining({ type: 'service' }), + relations: expect.any(Array), + status: { + items: [ + { + level: 'error', + type: 'backstage.io/catalog-processing', + message: expect.stringMatching( + /InputError: Processor BuiltinKindsEntityProcessor threw an error/, + ), + error: expect.objectContaining({ + cause: expect.objectContaining({ + message: + " must have required property 'spec' - missingProperty: spec", + name: 'TypeError', + }), + name: 'InputError', + }), + }, + ], + }, + }, + ]); + }); +}); diff --git a/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.ts b/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.ts index fb9518c783..4693ffc559 100644 --- a/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.ts +++ b/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.ts @@ -32,7 +32,6 @@ import { startTaskPipeline } from './TaskPipeline'; const CACHE_TTL = 5; export class DefaultCatalogProcessingEngine implements CatalogProcessingEngine { - private readonly tracker = progressTracker(); private stopFunc?: () => void; constructor( @@ -46,6 +45,7 @@ export class DefaultCatalogProcessingEngine implements CatalogProcessingEngine { unprocessedEntity: Entity; errors: Error[]; }) => Promise | void, + private readonly tracker = progressTracker(), ) {} async start() { From c505860ee59eec86c7afe7eee0e0174d72182d8d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 25 Sep 2022 20:40:42 +0200 Subject: [PATCH 02/10] catalog-backend: integration test, forward more process args and throw on unhandled error Signed-off-by: Patrik Oldsberg --- .../catalog-backend/src/integration.test.ts | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/plugins/catalog-backend/src/integration.test.ts b/plugins/catalog-backend/src/integration.test.ts index b702dc1180..a742d86ba7 100644 --- a/plugins/catalog-backend/src/integration.test.ts +++ b/plugins/catalog-backend/src/integration.test.ts @@ -40,7 +40,11 @@ import { DefaultRefreshService } from './service/DefaultRefreshService'; import { connectEntityProviders } from './processing/connectEntityProviders'; import { EntitiesCatalog } from './catalog/types'; import { RefreshOptions, RefreshService } from './service/types'; -import { EntityProviderConnection } from '@backstage/plugin-catalog-node'; +import { + CatalogProcessorEmit, + EntityProviderConnection, + LocationSpec, +} from '@backstage/plugin-catalog-node'; import { RefreshStateItem } from './database/types'; const voidLogger = getVoidLogger(); @@ -179,7 +183,11 @@ class TestHarness { logger?: Logger; db?: Knex; permissions?: PermissionEvaluator; - processEntity?(entity: Entity): Promise; + processEntity?( + entity: Entity, + location: LocationSpec, + emit: CatalogProcessorEmit, + ): Promise; onProcessingError?(event: { unprocessedEntity: Entity; errors: Error[]; @@ -216,9 +224,13 @@ class TestHarness { processors: [ { getProcessorName: () => 'test', - async preProcessEntity(entity: Entity) { + async preProcessEntity( + entity: Entity, + location: LocationSpec, + emit: CatalogProcessorEmit, + ) { if (options?.processEntity) { - return options?.processEntity(entity); + return options?.processEntity(entity, location, emit); } return entity; }, @@ -246,7 +258,13 @@ class TestHarness { () => createHash('sha1'), 50, event => { - options?.onProcessingError?.(event); + if (options?.onProcessingError) { + options.onProcessingError(event); + } else { + throw new Error( + `Catalog processing error, ${event.errors.join(', ')}`, + ); + } }, proxyProgressTracker, ); From 746d2517455274c26ff3ff45efbd4edf95301d9c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 25 Sep 2022 21:35:24 +0200 Subject: [PATCH 03/10] catalog-backend: integration test, remove builtin kinds processor Signed-off-by: Patrik Oldsberg --- .../catalog-backend/src/integration.test.ts | 50 +++++++------------ 1 file changed, 18 insertions(+), 32 deletions(-) diff --git a/plugins/catalog-backend/src/integration.test.ts b/plugins/catalog-backend/src/integration.test.ts index a742d86ba7..db7bd0a9d2 100644 --- a/plugins/catalog-backend/src/integration.test.ts +++ b/plugins/catalog-backend/src/integration.test.ts @@ -18,11 +18,7 @@ import { Knex } from 'knex'; import { Logger } from 'winston'; import { ConfigReader } from '@backstage/config'; import { JsonObject } from '@backstage/types'; -import { - BuiltinKindsEntityProcessor, - CatalogProcessingEngine, - EntityProvider, -} from './index'; +import { CatalogProcessingEngine, EntityProvider } from './index'; import { DatabaseManager, getVoidLogger } from '@backstage/backend-common'; import { PermissionEvaluator } from '@backstage/plugin-permission-common'; import { Entity, EntityPolicies } from '@backstage/catalog-model'; @@ -224,6 +220,9 @@ class TestHarness { processors: [ { getProcessorName: () => 'test', + async validateEntityKind() { + return true; + }, async preProcessEntity( entity: Entity, location: LocationSpec, @@ -235,7 +234,6 @@ class TestHarness { return entity; }, }, - new BuiltinKindsEntityProcessor(), ], integrations, rulesEnforcer, @@ -337,7 +335,7 @@ describe('Catalog Backend Integration', () => { const harness = await TestHarness.create({ async processEntity(entity: Entity) { if (triggerError) { - delete entity.spec; + throw new Error('NOPE'); } return entity; }, @@ -354,11 +352,6 @@ describe('Catalog Backend Integration', () => { 'backstage.io/managed-by-origin-location': 'url:.', }, }, - spec: { - type: 'service', - owner: 'guest', - lifecycle: 'production', - }, }, ]); @@ -370,14 +363,7 @@ describe('Catalog Backend Integration', () => { apiVersion: 'backstage.io/v1alpha1', kind: 'Component', metadata: expect.objectContaining({ name: 'test' }), - spec: expect.objectContaining({ type: 'service' }), - relations: [ - { - target: { kind: 'group', namespace: 'default', name: 'guest' }, - type: 'ownedBy', - targetRef: 'group:default/guest', - }, - ], + relations: [], }, ]); @@ -390,24 +376,24 @@ describe('Catalog Backend Integration', () => { apiVersion: 'backstage.io/v1alpha1', kind: 'Component', metadata: expect.objectContaining({ name: 'test' }), - spec: expect.objectContaining({ type: 'service' }), - relations: expect.any(Array), + relations: [], status: { items: [ { level: 'error', type: 'backstage.io/catalog-processing', - message: expect.stringMatching( - /InputError: Processor BuiltinKindsEntityProcessor threw an error/, - ), - error: expect.objectContaining({ - cause: expect.objectContaining({ - message: - " must have required property 'spec' - missingProperty: spec", - name: 'TypeError', - }), + message: + 'InputError: Processor Object threw an error while preprocessing; caused by Error: NOPE', + error: { name: 'InputError', - }), + message: + 'Processor Object threw an error while preprocessing; caused by Error: NOPE', + cause: { + name: 'Error', + message: 'NOPE', + stack: expect.stringMatching(/^Error: NOPE/), + }, + }, }, ], }, From e1b03e97e6b515547e7aeacb5537efc96bde085f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 25 Sep 2022 21:53:37 +0200 Subject: [PATCH 04/10] catalog-backend: integration test, simplify output entities structure Signed-off-by: Patrik Oldsberg --- .../catalog-backend/src/integration.test.ts | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/plugins/catalog-backend/src/integration.test.ts b/plugins/catalog-backend/src/integration.test.ts index db7bd0a9d2..6659ece759 100644 --- a/plugins/catalog-backend/src/integration.test.ts +++ b/plugins/catalog-backend/src/integration.test.ts @@ -21,7 +21,11 @@ import { JsonObject } from '@backstage/types'; import { CatalogProcessingEngine, EntityProvider } from './index'; import { DatabaseManager, getVoidLogger } from '@backstage/backend-common'; import { PermissionEvaluator } from '@backstage/plugin-permission-common'; -import { Entity, EntityPolicies } from '@backstage/catalog-model'; +import { + Entity, + EntityPolicies, + stringifyEntityRef, +} from '@backstage/catalog-model'; import { defaultEntityDataParser } from './modules/util/parse'; import { DefaultCatalogProcessingOrchestrator } from './processing/DefaultCatalogProcessingOrchestrator'; import { applyDatabaseMigrations } from './database/migrations'; @@ -318,9 +322,9 @@ class TestHarness { }); } - async getOutputEntities(): Promise { + async getOutputEntities(): Promise> { const { entities } = await this.#catalog.entities(); - return entities; + return Object.fromEntries(entities.map(e => [stringifyEntityRef(e), e])); } async refresh(options: RefreshOptions) { @@ -355,24 +359,24 @@ describe('Catalog Backend Integration', () => { }, ]); - await expect(harness.getOutputEntities()).resolves.toEqual([]); + await expect(harness.getOutputEntities()).resolves.toEqual({}); await expect(harness.process()).resolves.toEqual({}); - await expect(harness.getOutputEntities()).resolves.toEqual([ - { + await expect(harness.getOutputEntities()).resolves.toEqual({ + 'component:default/test': { apiVersion: 'backstage.io/v1alpha1', kind: 'Component', metadata: expect.objectContaining({ name: 'test' }), relations: [], }, - ]); + }); triggerError = true; await expect(harness.process()).resolves.toEqual({}); - await expect(harness.getOutputEntities()).resolves.toEqual([ - { + await expect(harness.getOutputEntities()).resolves.toEqual({ + 'component:default/test': { apiVersion: 'backstage.io/v1alpha1', kind: 'Component', metadata: expect.objectContaining({ name: 'test' }), @@ -398,6 +402,6 @@ describe('Catalog Backend Integration', () => { ], }, }, - ]); + }); }); }); From a0b1c65f9bcac9eda54b7fb54c1fb56ca1d258f1 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 25 Sep 2022 22:29:36 +0200 Subject: [PATCH 05/10] catalog-backend: integration test, add orphan test Signed-off-by: Patrik Oldsberg --- .../catalog-backend/src/integration.test.ts | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/plugins/catalog-backend/src/integration.test.ts b/plugins/catalog-backend/src/integration.test.ts index 6659ece759..f5afa34f30 100644 --- a/plugins/catalog-backend/src/integration.test.ts +++ b/plugins/catalog-backend/src/integration.test.ts @@ -44,6 +44,7 @@ import { CatalogProcessorEmit, EntityProviderConnection, LocationSpec, + processingResult, } from '@backstage/plugin-catalog-node'; import { RefreshStateItem } from './database/types'; @@ -404,4 +405,91 @@ describe('Catalog Backend Integration', () => { }, }); }); + + it('should orphan entities', async () => { + const generatedApis = ['api-1', 'api-2']; + + const harness = await TestHarness.create({ + async processEntity( + entity: Entity, + location: LocationSpec, + emit: CatalogProcessorEmit, + ) { + if (entity.metadata.name === 'test') { + for (const api of generatedApis) { + emit( + processingResult.entity(location, { + apiVersion: 'backstage.io/v1alpha1', + kind: 'API', + metadata: { + name: api, + annotations: { + 'backstage.io/managed-by-location': 'url:.', + 'backstage.io/managed-by-origin-location': 'url:.', + }, + }, + }), + ); + } + } + return entity; + }, + }); + + await harness.setInputEntities([ + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'test', + annotations: { + 'backstage.io/managed-by-location': 'url:.', + 'backstage.io/managed-by-origin-location': 'url:.', + }, + }, + }, + ]); + + await expect(harness.getOutputEntities()).resolves.toEqual({}); + await expect(harness.process()).resolves.toEqual({}); + + await expect(harness.getOutputEntities()).resolves.toEqual({ + 'component:default/test': { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: expect.objectContaining({ name: 'test' }), + relations: [], + }, + 'api:default/api-1': expect.objectContaining({ + metadata: expect.objectContaining({ name: 'api-1' }), + }), + 'api:default/api-2': expect.objectContaining({ + metadata: expect.objectContaining({ name: 'api-2' }), + }), + }); + + generatedApis.pop(); + + await expect(harness.process()).resolves.toEqual({}); + + await expect(harness.getOutputEntities()).resolves.toEqual({ + 'component:default/test': { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: expect.objectContaining({ name: 'test' }), + relations: [], + }, + 'api:default/api-1': expect.objectContaining({ + metadata: expect.objectContaining({ name: 'api-1' }), + }), + 'api:default/api-2': expect.objectContaining({ + metadata: expect.objectContaining({ + name: 'api-2', + annotations: expect.objectContaining({ + 'backstage.io/orphan': 'true', + }), + }), + }), + }); + }); }); From a9e59694c00b9c6fc7fcb08a9a9b930f49d35095 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 26 Sep 2022 10:20:37 +0200 Subject: [PATCH 06/10] catalog-backend: fix integration test processing wait Signed-off-by: Patrik Oldsberg --- plugins/catalog-backend/src/integration.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/catalog-backend/src/integration.test.ts b/plugins/catalog-backend/src/integration.test.ts index f5afa34f30..745678fc70 100644 --- a/plugins/catalog-backend/src/integration.test.ts +++ b/plugins/catalog-backend/src/integration.test.ts @@ -131,6 +131,7 @@ class WaitingProgressTracker implements ProgressTracker { ); const currentCount = this.#counts.get(item.id) ?? 0; + this.#counts.set(item.id, currentCount); const onDone = () => { this.#counts.set(item.id, currentCount + 1); From f6f75515f15527fd9c5bb90559a2fc67adb3072a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 26 Sep 2022 10:32:09 +0200 Subject: [PATCH 07/10] catalog-backend: integration test, add provider replacement test Signed-off-by: Patrik Oldsberg --- .../catalog-backend/src/integration.test.ts | 51 ++++++++++++++++++- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/plugins/catalog-backend/src/integration.test.ts b/plugins/catalog-backend/src/integration.test.ts index 745678fc70..e61a7d7038 100644 --- a/plugins/catalog-backend/src/integration.test.ts +++ b/plugins/catalog-backend/src/integration.test.ts @@ -317,10 +317,13 @@ class TestHarness { return errors; } - async setInputEntities(entities: Entity[]) { + async setInputEntities(entities: (Entity & { locationKey?: string })[]) { return this.#provider.getConnection().applyMutation({ type: 'full', - entities: entities.map(entity => ({ entity })), + entities: entities.map(({ locationKey, ...entity }) => ({ + entity, + locationKey, + })), }); } @@ -493,4 +496,48 @@ describe('Catalog Backend Integration', () => { }), }); }); + + it('should not replace matching provided entities', async () => { + const harness = await TestHarness.create(); + + const entityA = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'a', + annotations: { + 'backstage.io/managed-by-location': 'url:.', + 'backstage.io/managed-by-origin-location': 'url:.', + }, + }, + }; + const entityB = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'b', + annotations: { + 'backstage.io/managed-by-location': 'url:.', + 'backstage.io/managed-by-origin-location': 'url:.', + }, + }, + }; + + const entities = [entityA, { locationKey: 'loc', ...entityB }]; + + await harness.setInputEntities(entities); + await expect(harness.process()).resolves.toEqual({}); + + const outputEntities = await harness.getOutputEntities(); + + await expect(harness.getOutputEntities()).resolves.toEqual({ + 'component:default/a': expect.anything(), + 'component:default/b': expect.anything(), + }); + + await harness.setInputEntities(entities); + await expect(harness.process()).resolves.toEqual({}); + + await expect(harness.getOutputEntities()).resolves.toEqual(outputEntities); + }); }); From bd3d7012a97069124378b6879ef56c0bb23a59b6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 25 Sep 2022 23:52:00 +0200 Subject: [PATCH 08/10] catalog-backend: fix location key check in entity provider delta diff Signed-off-by: Patrik Oldsberg --- .../src/database/DefaultProcessingDatabase.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts index f6fc7fa5fa..3dc5325178 100644 --- a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts +++ b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts @@ -784,7 +784,10 @@ export class DefaultProcessingDatabase implements ProcessingDatabase { if (!oldRef) { // Add any entity that does not exist in the database toAdd.push(upsertItem); - } else if (oldRef.locationKey !== item.deferred.locationKey) { + } else if ( + (oldRef?.locationKey ?? undefined) !== + (item.deferred.locationKey ?? undefined) + ) { // Remove and then re-add any entity that exists, but with a different location key toRemove.push(item.ref); toAdd.push(upsertItem); From 8cb6e101054c323e3c5a4d09aa397e790f620d02 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 26 Sep 2022 10:56:08 +0200 Subject: [PATCH 09/10] changesets: added changeset for catalog provider diffing fix Signed-off-by: Patrik Oldsberg --- .changeset/poor-clouds-ring.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/poor-clouds-ring.md diff --git a/.changeset/poor-clouds-ring.md b/.changeset/poor-clouds-ring.md new file mode 100644 index 0000000000..62ba3928a7 --- /dev/null +++ b/.changeset/poor-clouds-ring.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Fixed a bug where entities provided without a location key would always replace existing entities, rather than updating them. From be617b42a44edc99a5e8cbd5179d6d26a8272206 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 27 Sep 2022 10:03:46 +0200 Subject: [PATCH 10/10] catalog-backend: integration test review fixes Signed-off-by: Patrik Oldsberg --- plugins/catalog-backend/src/integration.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/catalog-backend/src/integration.test.ts b/plugins/catalog-backend/src/integration.test.ts index e61a7d7038..67b63428db 100644 --- a/plugins/catalog-backend/src/integration.test.ts +++ b/plugins/catalog-backend/src/integration.test.ts @@ -207,7 +207,7 @@ class TestHarness { ); const logger = options?.logger ?? getVoidLogger(); const db = - options?.db /* (await TestDatabases.create().init('SQLITE_3')); */ ?? + options?.db ?? (await DatabaseManager.fromConfig(config, { logger }) .forPlugin('catalog') .getClient()); @@ -245,7 +245,6 @@ class TestHarness { rulesEnforcer, logger, parser: defaultEntityDataParser, - // policy: new SchemaValidEntityPolicy(), policy: EntityPolicies.allOf([]), }); const stitcher = new Stitcher(db, logger); @@ -303,10 +302,11 @@ class TestHarness { } async process(entityRefs?: Set) { - this.#engine.start(); - const tracker = new WaitingProgressTracker(entityRefs); this.#proxyProgressTracker.setTracker(tracker); + + this.#engine.start(); + const errors = await tracker.wait(); this.#engine.stop();