From dd395335bc252ff02c07e6457b519792fba917f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20MORI?= Date: Mon, 8 Aug 2022 15:06:01 +0200 Subject: [PATCH] Allow unknown typed location from being registered via the location service by configuration settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane MORI --- .changeset/flat-plums-shout.md | 5 + plugins/catalog-backend/config.d.ts | 17 +++ .../src/service/CatalogBuilder.ts | 2 +- .../service/DefaultLocationService.test.ts | 116 +++++++++++++++++- .../src/service/DefaultLocationService.ts | 10 +- 5 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 .changeset/flat-plums-shout.md diff --git a/.changeset/flat-plums-shout.md b/.changeset/flat-plums-shout.md new file mode 100644 index 0000000000..578952c9c3 --- /dev/null +++ b/.changeset/flat-plums-shout.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Allow unknown typed location from being registered via the location service by configuration settings diff --git a/plugins/catalog-backend/config.d.ts b/plugins/catalog-backend/config.d.ts index 950ed2f63c..473db9a35a 100644 --- a/plugins/catalog-backend/config.d.ts +++ b/plugins/catalog-backend/config.d.ts @@ -54,6 +54,23 @@ export interface Config { */ readonly?: boolean; + /** + * Allow unknown type when create location. + * + * Setting 'locationService.create.allowUnknownType=false' forbids to create + * location with unknown type through location service. + * This is the default value. + * + * Setting 'locationService.create.allowUnknownType=true' allows to create + * location with unknown type through location service. + * + */ + locationService?: { + create?: { + allowUnknownType: boolean; + }; + }; + /** * A set of static locations that the catalog shall always keep itself * up-to-date with. This is commonly used for large, permanent integrations diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index e9e35301c3..57b5160a97 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -465,7 +465,7 @@ export class CatalogBuilder { const locationAnalyzer = this.locationAnalyzer ?? new RepoLocationAnalyzer(logger, integrations); const locationService = new AuthorizedLocationService( - new DefaultLocationService(locationStore, orchestrator), + new DefaultLocationService(locationStore, orchestrator, config), permissionEvaluator, ); const refreshService = new AuthorizedRefreshService( diff --git a/plugins/catalog-backend/src/service/DefaultLocationService.test.ts b/plugins/catalog-backend/src/service/DefaultLocationService.test.ts index 4399d48d69..e936d2de93 100644 --- a/plugins/catalog-backend/src/service/DefaultLocationService.test.ts +++ b/plugins/catalog-backend/src/service/DefaultLocationService.test.ts @@ -18,6 +18,7 @@ import { DefaultLocationService } from './DefaultLocationService'; import { CatalogProcessingOrchestrator } from '../processing/types'; import { LocationStore } from './types'; import { InputError } from '@backstage/errors'; +import { ConfigReader } from '@backstage/config'; describe('DefaultLocationServiceTest', () => { const orchestrator: jest.Mocked = { @@ -29,7 +30,17 @@ describe('DefaultLocationServiceTest', () => { listLocations: jest.fn(), getLocation: jest.fn(), }; - const locationService = new DefaultLocationService(store, orchestrator); + + const mockConfig = (allowUnknownType: boolean) => + new ConfigReader({ + catalog: { + locationService: { + create: { + allowUnknownType, + }, + }, + }, + }); beforeEach(() => { jest.resetAllMocks(); @@ -81,6 +92,11 @@ describe('DefaultLocationServiceTest', () => { errors: [], }); + const locationService = new DefaultLocationService( + store, + orchestrator, + mockConfig(false), + ); await locationService.createLocation( { type: 'url', target: 'https://backstage.io/catalog-info.yaml' }, true, @@ -144,6 +160,12 @@ describe('DefaultLocationServiceTest', () => { store.listLocations.mockResolvedValueOnce([ { id: '137', ...locationSpec }, ]); + + const locationService = new DefaultLocationService( + store, + orchestrator, + mockConfig(false), + ); const result = await locationService.createLocation( { type: 'url', target: 'https://backstage.io/catalog-info.yaml' }, true, @@ -197,6 +219,11 @@ describe('DefaultLocationServiceTest', () => { errors: [], }); + const locationService = new DefaultLocationService( + store, + orchestrator, + mockConfig(false), + ); await expect( locationService.createLocation( { type: 'url', target: 'https://backstage.io/catalog-info.yaml' }, @@ -225,6 +252,12 @@ describe('DefaultLocationServiceTest', () => { store.listLocations.mockResolvedValueOnce([ { id: '987', type: 'url', target: 'https://example.com' }, ]); + + const locationService = new DefaultLocationService( + store, + orchestrator, + mockConfig(false), + ); const result = await locationService.createLocation( { type: 'url', target: 'https://backstage.io/catalog-info.yaml' }, true, @@ -243,6 +276,11 @@ describe('DefaultLocationServiceTest', () => { id: '123', }); + const locationService = new DefaultLocationService( + store, + orchestrator, + mockConfig(false), + ); await expect( locationService.createLocation(locationSpec, false), ).resolves.toEqual({ @@ -259,7 +297,61 @@ describe('DefaultLocationServiceTest', () => { }); }); - it('should not allow locations of unknown types', async () => { + it('should create location with unknown type if configuration allows it', async () => { + const locationSpec = { + type: 'unknown', + target: 'https://backstage.io/catalog-info.yaml', + }; + + store.createLocation.mockResolvedValue({ + ...locationSpec, + id: '123', + }); + + const locationService = new DefaultLocationService( + store, + orchestrator, + mockConfig(true), + ); + await expect( + locationService.createLocation(locationSpec, false), + ).resolves.toEqual({ + entities: [], + location: { + id: '123', + target: 'https://backstage.io/catalog-info.yaml', + type: 'unknown', + }, + }); + expect(store.createLocation).toBeCalledWith({ + target: 'https://backstage.io/catalog-info.yaml', + type: 'unknown', + }); + }); + + it('should not allow locations of unknown types by default', async () => { + const locationService = new DefaultLocationService( + store, + orchestrator, + new ConfigReader({}), + ); + await expect( + locationService.createLocation( + { + type: 'unknown', + target: 'https://backstage.io/catalog-info.yaml', + }, + false, + ), + ).rejects.toThrow(InputError); + }); + + it('should not allow locations of unknown types if configuration forbids it', async () => { + const locationService = new DefaultLocationService( + store, + orchestrator, + mockConfig(false), + ); await expect( locationService.createLocation( { @@ -279,6 +371,11 @@ describe('DefaultLocationServiceTest', () => { errors: [new Error('Error: Unable to read url')], }); + const locationService = new DefaultLocationService( + store, + orchestrator, + mockConfig(false), + ); await expect( locationService.createLocation( { @@ -293,6 +390,11 @@ describe('DefaultLocationServiceTest', () => { describe('listLocations', () => { it('should call locationStore.deleteLocation', async () => { + const locationService = new DefaultLocationService( + store, + orchestrator, + mockConfig(false), + ); await locationService.listLocations(); expect(store.listLocations).toBeCalled(); }); @@ -300,6 +402,11 @@ describe('DefaultLocationServiceTest', () => { describe('deleteLocation', () => { it('should call locationStore.deleteLocation', async () => { + const locationService = new DefaultLocationService( + store, + orchestrator, + mockConfig(false), + ); await locationService.deleteLocation('123'); expect(store.deleteLocation).toBeCalledWith('123'); }); @@ -307,6 +414,11 @@ describe('DefaultLocationServiceTest', () => { describe('getLocation', () => { it('should call locationStore.getLocation', async () => { + const locationService = new DefaultLocationService( + store, + orchestrator, + mockConfig(false), + ); await locationService.getLocation('123'); expect(store.getLocation).toBeCalledWith('123'); }); diff --git a/plugins/catalog-backend/src/service/DefaultLocationService.ts b/plugins/catalog-backend/src/service/DefaultLocationService.ts index 819cead63d..e70d72a9ee 100644 --- a/plugins/catalog-backend/src/service/DefaultLocationService.ts +++ b/plugins/catalog-backend/src/service/DefaultLocationService.ts @@ -26,18 +26,26 @@ import { LocationInput, LocationService, LocationStore } from './types'; import { locationSpecToMetadataName } from '../util/conversion'; import { InputError } from '@backstage/errors'; import { DeferredEntity } from '@backstage/plugin-catalog-node'; +import { Config } from '@backstage/config'; export class DefaultLocationService implements LocationService { constructor( private readonly store: LocationStore, private readonly orchestrator: CatalogProcessingOrchestrator, + private readonly config: Config, ) {} async createLocation( input: LocationInput, dryRun: boolean, ): Promise<{ location: Location; entities: Entity[]; exists?: boolean }> { - if (input.type !== 'url') { + const allowUnknownTypeConfigName = + 'catalog.locationService.create.allowUnknownType'; + const allowUnknownType = + this.config.has(allowUnknownTypeConfigName) && + this.config.getBoolean(allowUnknownTypeConfigName); + + if (!allowUnknownType && input.type !== 'url') { throw new InputError(`Registered locations must be of type 'url'`); } if (dryRun) {