From b0d711e7184bbd9e2c6608ea6930c9be8c7637bf Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Thu, 6 May 2021 15:42:49 +0200 Subject: [PATCH 1/5] Catalog/next: Add support for dryRun which is used by the catalog-import plugin. Co-authored-by: Ben Lambert Signed-off-by: Johan Haals --- packages/catalog-client/src/CatalogClient.ts | 1 + .../src/next/DefaultLocationService.test.ts | 159 ++++++++++++++++++ .../src/next/DefaultLocationService.ts | 42 +++-- plugins/catalog-backend/src/next/types.ts | 1 - 4 files changed, 189 insertions(+), 14 deletions(-) create mode 100644 plugins/catalog-backend/src/next/DefaultLocationService.test.ts diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index a246b3abab..063db5a6a3 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -134,6 +134,7 @@ export class CatalogClient implements CatalogApi { throw new Error(`Location wasn't added: ${target}`); } + // TODO(jhaals): This will throw using the experimental catalog since all discovered entities are deferred. if (entities.length === 0) { throw new Error( `Location was added but has no entities specified yet: ${target}`, diff --git a/plugins/catalog-backend/src/next/DefaultLocationService.test.ts b/plugins/catalog-backend/src/next/DefaultLocationService.test.ts new file mode 100644 index 0000000000..662be5d3a6 --- /dev/null +++ b/plugins/catalog-backend/src/next/DefaultLocationService.test.ts @@ -0,0 +1,159 @@ +/* + * Copyright 2021 Spotify AB + * + * 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 { DefaultLocationService } from './DefaultLocationService'; +import { CatalogProcessingOrchestrator, LocationStore } from './types'; + +describe('DefaultLocationServiceTest', () => { + const orchestrator: jest.Mocked = { + process: jest.fn(), + }; + const store: jest.Mocked = { + deleteLocation: jest.fn(), + createLocation: jest.fn(), + listLocations: jest.fn(), + getLocation: jest.fn(), + }; + + beforeEach(() => jest.resetAllMocks()); + const locationService = new DefaultLocationService(store, orchestrator); + describe('createLocation', () => { + it('should support dry run', async () => { + orchestrator.process.mockResolvedValueOnce({ + ok: true, + state: new Map(), + completedEntity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Location', + metadata: { + name: 'foo', + }, + }, + deferredEntities: [ + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'bar', + }, + }, + ], + relations: [], + errors: [], + }); + + orchestrator.process.mockResolvedValueOnce({ + ok: true, + state: new Map(), + completedEntity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'bar', + }, + }, + deferredEntities: [], + relations: [], + errors: [], + }); + + await locationService.createLocation( + { type: 'url', target: 'https://backstage.io/catalog-info.yaml' }, + true, + ); + + expect(orchestrator.process).toBeCalledWith({ + entity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Location', + metadata: { + annotations: { + 'backstage.io/managed-by-location': + 'url:https://backstage.io/catalog-info.yaml', + 'backstage.io/managed-by-origin-location': + 'url:https://backstage.io/catalog-info.yaml', + }, + name: 'generated-bbad4f61e08f24e25d5c5e68e13e164f760aff06', + namespace: 'default', + }, + spec: { + target: 'https://backstage.io/catalog-info.yaml', + type: 'url', + }, + }, + state: expect.anything(), + }); + + expect(orchestrator.process).toBeCalledWith({ + entity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { name: 'bar' }, + }, + state: expect.anything(), + }); + expect(orchestrator.process).toBeCalledTimes(2); + expect(store.createLocation).not.toBeCalled(); + }); + + it('should create location', async () => { + const locationSpec = { + type: 'url', + target: 'https://backstage.io/catalog-info.yaml', + }; + + store.createLocation.mockResolvedValue({ + ...locationSpec, + id: '123', + }); + + await expect( + locationService.createLocation(locationSpec, false), + ).resolves.toEqual({ + entities: [], + location: { + id: '123', + target: 'https://backstage.io/catalog-info.yaml', + type: 'url', + }, + }); + expect(store.createLocation).toBeCalledWith({ + target: 'https://backstage.io/catalog-info.yaml', + type: 'url', + }); + }); + }); + describe('listLocations', () => { + it('should call locationStore.deleteLocation', async () => { + await locationService.listLocations(); + expect(store.listLocations).toBeCalled(); + }); + }); + + describe('deleteLocation', () => { + it('should call locationStore.deleteLocation', async () => { + await locationService.deleteLocation('123'); + expect(store.deleteLocation).toBeCalledWith('123'); + }); + }); + + describe('getLocation', () => { + it('should call locationStore.getLocation', async () => { + await locationService.getLocation('123'); + expect(store.getLocation).toBeCalledWith('123'); + }); + }); +}); diff --git a/plugins/catalog-backend/src/next/DefaultLocationService.ts b/plugins/catalog-backend/src/next/DefaultLocationService.ts index 3935f6e4d0..884410c2f0 100644 --- a/plugins/catalog-backend/src/next/DefaultLocationService.ts +++ b/plugins/catalog-backend/src/next/DefaultLocationService.ts @@ -25,6 +25,7 @@ import { LocationStore, CatalogProcessingOrchestrator, } from './types'; +import { locationSpecToMetadataName } from './util'; export class DefaultLocationService implements LocationService { constructor( @@ -41,7 +42,10 @@ export class DefaultLocationService implements LocationService { apiVersion: 'backstage.io/v1alpha1', kind: 'Location', metadata: { - name: `${spec.type}:${spec.target}`, + name: locationSpecToMetadataName({ + type: spec.type, + target: spec.target, + }), namespace: 'default', annotations: { [LOCATION_ANNOTATION]: `${spec.type}:${spec.target}`, @@ -49,22 +53,34 @@ export class DefaultLocationService implements LocationService { }, }, spec: { - location: { type: spec.type, target: spec.target }, + type: spec.type, + target: spec.target, }, }; - const processed = await this.orchestrator.process({ - entity, - eager: true, - state: new Map(), - }); - if (processed.ok) { - return { - location: { ...spec, id: `${spec.type}:${spec.target}` }, - entities: [processed.completedEntity], - }; + const unprocessedEntities: Entity[] = [entity]; + const entities: Entity[] = []; + const state = new Map(); // ignored + while (unprocessedEntities.length) { + const currentEntity = unprocessedEntities.pop(); + if (!currentEntity) { + continue; + } + const processed = await this.orchestrator.process({ + entity: currentEntity, + state, + }); + if (processed.ok) { + unprocessedEntities.push(...processed.deferredEntities); + entities.push(processed.completedEntity); + } else { + throw Error(processed.errors.join(', ')); + } } - throw Error('error handling not implemented.'); + return { + location: { ...spec, id: `${spec.type}:${spec.target}` }, + entities, + }; } const location = await this.store.createLocation(spec); diff --git a/plugins/catalog-backend/src/next/types.ts b/plugins/catalog-backend/src/next/types.ts index 410066eafd..fbc243ce69 100644 --- a/plugins/catalog-backend/src/next/types.ts +++ b/plugins/catalog-backend/src/next/types.ts @@ -59,7 +59,6 @@ export interface EntityProvider { export type EntityProcessingRequest = { entity: Entity; - eager?: boolean; state: Map; // Versions for multiple deployments etc }; From e53223f15c5f8730c916779952ea7aa2c975fabe Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 7 May 2021 15:01:17 +0200 Subject: [PATCH 2/5] Make listLocations conform with current API Signed-off-by: Johan Haals --- .../src/next/DefaultLocationService.test.ts | 19 +++++++++++++++++-- .../src/next/DefaultLocationService.ts | 7 +++++-- plugins/catalog-backend/src/next/types.ts | 6 +++++- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/plugins/catalog-backend/src/next/DefaultLocationService.test.ts b/plugins/catalog-backend/src/next/DefaultLocationService.test.ts index 662be5d3a6..bf2bee147a 100644 --- a/plugins/catalog-backend/src/next/DefaultLocationService.test.ts +++ b/plugins/catalog-backend/src/next/DefaultLocationService.test.ts @@ -137,8 +137,23 @@ describe('DefaultLocationServiceTest', () => { }); }); describe('listLocations', () => { - it('should call locationStore.deleteLocation', async () => { - await locationService.listLocations(); + it('should call locationStore.listLocations', async () => { + store.listLocations.mockResolvedValue([ + { + id: '123', + target: 'https://backstage.io/catalog-info.yaml', + type: 'url', + }, + ]); + await expect(locationService.listLocations()).resolves.toEqual([ + { + data: { + id: '123', + target: 'https://backstage.io/catalog-info.yaml', + type: 'url', + }, + }, + ]); expect(store.listLocations).toBeCalled(); }); }); diff --git a/plugins/catalog-backend/src/next/DefaultLocationService.ts b/plugins/catalog-backend/src/next/DefaultLocationService.ts index 884410c2f0..460ba61ec8 100644 --- a/plugins/catalog-backend/src/next/DefaultLocationService.ts +++ b/plugins/catalog-backend/src/next/DefaultLocationService.ts @@ -24,6 +24,7 @@ import { LocationService, LocationStore, CatalogProcessingOrchestrator, + LocationResponse, } from './types'; import { locationSpecToMetadataName } from './util'; @@ -87,9 +88,11 @@ export class DefaultLocationService implements LocationService { return { location, entities: [] }; } - listLocations(): Promise { - return this.store.listLocations(); + async listLocations(): Promise { + const locations = await this.store.listLocations(); + return locations.map(location => ({ data: location })); } + getLocation(id: string): Promise { return this.store.getLocation(id); } diff --git a/plugins/catalog-backend/src/next/types.ts b/plugins/catalog-backend/src/next/types.ts index fbc243ce69..05fd75522a 100644 --- a/plugins/catalog-backend/src/next/types.ts +++ b/plugins/catalog-backend/src/next/types.ts @@ -27,11 +27,15 @@ export interface LocationService { spec: LocationSpec, dryRun: boolean, ): Promise<{ location: Location; entities: Entity[] }>; - listLocations(): Promise; + listLocations(): Promise; getLocation(id: string): Promise; deleteLocation(id: string): Promise; } +export type LocationResponse = { + data: Location; +}; + export interface LocationStore { createLocation(spec: LocationSpec): Promise; listLocations(): Promise; From b23de0aeb671f02e81fa503fae974c81e377725f Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Mon, 10 May 2021 10:02:34 +0200 Subject: [PATCH 3/5] Revert "Make listLocations conform with current API" This reverts commit 449972081db302c6c2a2938a45f90beb82e117c8. Signed-off-by: Johan Haals --- .../src/next/DefaultLocationService.test.ts | 19 ++----------------- .../src/next/DefaultLocationService.ts | 7 ++----- plugins/catalog-backend/src/next/types.ts | 6 +----- 3 files changed, 5 insertions(+), 27 deletions(-) diff --git a/plugins/catalog-backend/src/next/DefaultLocationService.test.ts b/plugins/catalog-backend/src/next/DefaultLocationService.test.ts index bf2bee147a..662be5d3a6 100644 --- a/plugins/catalog-backend/src/next/DefaultLocationService.test.ts +++ b/plugins/catalog-backend/src/next/DefaultLocationService.test.ts @@ -137,23 +137,8 @@ describe('DefaultLocationServiceTest', () => { }); }); describe('listLocations', () => { - it('should call locationStore.listLocations', async () => { - store.listLocations.mockResolvedValue([ - { - id: '123', - target: 'https://backstage.io/catalog-info.yaml', - type: 'url', - }, - ]); - await expect(locationService.listLocations()).resolves.toEqual([ - { - data: { - id: '123', - target: 'https://backstage.io/catalog-info.yaml', - type: 'url', - }, - }, - ]); + it('should call locationStore.deleteLocation', async () => { + await locationService.listLocations(); expect(store.listLocations).toBeCalled(); }); }); diff --git a/plugins/catalog-backend/src/next/DefaultLocationService.ts b/plugins/catalog-backend/src/next/DefaultLocationService.ts index 460ba61ec8..884410c2f0 100644 --- a/plugins/catalog-backend/src/next/DefaultLocationService.ts +++ b/plugins/catalog-backend/src/next/DefaultLocationService.ts @@ -24,7 +24,6 @@ import { LocationService, LocationStore, CatalogProcessingOrchestrator, - LocationResponse, } from './types'; import { locationSpecToMetadataName } from './util'; @@ -88,11 +87,9 @@ export class DefaultLocationService implements LocationService { return { location, entities: [] }; } - async listLocations(): Promise { - const locations = await this.store.listLocations(); - return locations.map(location => ({ data: location })); + listLocations(): Promise { + return this.store.listLocations(); } - getLocation(id: string): Promise { return this.store.getLocation(id); } diff --git a/plugins/catalog-backend/src/next/types.ts b/plugins/catalog-backend/src/next/types.ts index 05fd75522a..fbc243ce69 100644 --- a/plugins/catalog-backend/src/next/types.ts +++ b/plugins/catalog-backend/src/next/types.ts @@ -27,15 +27,11 @@ export interface LocationService { spec: LocationSpec, dryRun: boolean, ): Promise<{ location: Location; entities: Entity[] }>; - listLocations(): Promise; + listLocations(): Promise; getLocation(id: string): Promise; deleteLocation(id: string): Promise; } -export type LocationResponse = { - data: Location; -}; - export interface LocationStore { createLocation(spec: LocationSpec): Promise; listLocations(): Promise; From 6185b62e385b700460e341246cce73fd0935bc18 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Mon, 10 May 2021 10:06:30 +0200 Subject: [PATCH 4/5] Wrap location in data to conform with current API Signed-off-by: Johan Haals --- plugins/catalog-backend/src/next/NextRouter.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/catalog-backend/src/next/NextRouter.ts b/plugins/catalog-backend/src/next/NextRouter.ts index 433abd97e9..b4660edea0 100644 --- a/plugins/catalog-backend/src/next/NextRouter.ts +++ b/plugins/catalog-backend/src/next/NextRouter.ts @@ -133,8 +133,8 @@ export async function createNextRouter( res.status(201).json(output); }) .get('/locations', async (_req, res) => { - const output = await locationService.listLocations(); - res.status(200).json(output); + const locations = await locationService.listLocations(); + res.status(200).json(locations.map(l => ({ data: l }))); }) .get('/locations/:id', async (req, res) => { From 60a6090f2403095296162b41876541fa83a08407 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Mon, 10 May 2021 14:32:18 +0200 Subject: [PATCH 5/5] Split dryRun to separate method, map errors. Signed-off-by: Johan Haals --- .../src/next/DefaultLocationService.ts | 94 ++++++++++--------- 1 file changed, 50 insertions(+), 44 deletions(-) diff --git a/plugins/catalog-backend/src/next/DefaultLocationService.ts b/plugins/catalog-backend/src/next/DefaultLocationService.ts index 884410c2f0..e628b1d77d 100644 --- a/plugins/catalog-backend/src/next/DefaultLocationService.ts +++ b/plugins/catalog-backend/src/next/DefaultLocationService.ts @@ -38,51 +38,8 @@ export class DefaultLocationService implements LocationService { dryRun: boolean, ): Promise<{ location: Location; entities: Entity[] }> { if (dryRun) { - const entity = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Location', - metadata: { - name: locationSpecToMetadataName({ - type: spec.type, - target: spec.target, - }), - namespace: 'default', - annotations: { - [LOCATION_ANNOTATION]: `${spec.type}:${spec.target}`, - [ORIGIN_LOCATION_ANNOTATION]: `${spec.type}:${spec.target}`, - }, - }, - spec: { - type: spec.type, - target: spec.target, - }, - }; - const unprocessedEntities: Entity[] = [entity]; - const entities: Entity[] = []; - const state = new Map(); // ignored - while (unprocessedEntities.length) { - const currentEntity = unprocessedEntities.pop(); - if (!currentEntity) { - continue; - } - const processed = await this.orchestrator.process({ - entity: currentEntity, - state, - }); - if (processed.ok) { - unprocessedEntities.push(...processed.deferredEntities); - entities.push(processed.completedEntity); - } else { - throw Error(processed.errors.join(', ')); - } - } - - return { - location: { ...spec, id: `${spec.type}:${spec.target}` }, - entities, - }; + return this.dryRunCreateLocation(spec); } - const location = await this.store.createLocation(spec); return { location, entities: [] }; } @@ -96,4 +53,53 @@ export class DefaultLocationService implements LocationService { deleteLocation(id: string): Promise { return this.store.deleteLocation(id); } + + private async dryRunCreateLocation( + spec: LocationSpec, + ): Promise<{ location: Location; entities: Entity[] }> { + const entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Location', + metadata: { + name: locationSpecToMetadataName({ + type: spec.type, + target: spec.target, + }), + namespace: 'default', + annotations: { + [LOCATION_ANNOTATION]: `${spec.type}:${spec.target}`, + [ORIGIN_LOCATION_ANNOTATION]: `${spec.type}:${spec.target}`, + }, + }, + spec: { + type: spec.type, + target: spec.target, + }, + }; + const unprocessedEntities: Entity[] = [entity]; + const entities: Entity[] = []; + const state = new Map(); // ignored + while (unprocessedEntities.length) { + const currentEntity = unprocessedEntities.pop(); + if (!currentEntity) { + continue; + } + const processed = await this.orchestrator.process({ + entity: currentEntity, + state, + }); + + if (processed.ok) { + unprocessedEntities.push(...processed.deferredEntities); + entities.push(processed.completedEntity); + } else { + throw Error(processed.errors.map(String).join(', ')); + } + } + + return { + location: { ...spec, id: `${spec.type}:${spec.target}` }, + entities, + }; + } }