From 5417077b085032d72799504e769c3db902730e03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 24 Sep 2025 13:05:58 +0200 Subject: [PATCH 1/2] break out provider/processor handling into a separate file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../src/service/CatalogBuilder.ts | 51 +---- .../catalog-backend/src/service/util.test.ts | 195 ++++++++++++++++++ plugins/catalog-backend/src/service/util.ts | 84 +++++++- 3 files changed, 282 insertions(+), 48 deletions(-) create mode 100644 plugins/catalog-backend/src/service/util.test.ts diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index 35997c03c5..23178238c7 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -111,6 +111,7 @@ import { catalogEntityPermissionResourceRef, CatalogPermissionRuleInput, } from '@backstage/plugin-catalog-node/alpha'; +import { filterAndSortProcessors, filterProviders } from './util'; export type CatalogEnvironment = { logger: LoggerService; @@ -525,11 +526,12 @@ export class CatalogBuilder { const locationStore = new DefaultLocationStore(dbClient); const configLocationProvider = new ConfigLocationEntityProvider(config); - const entityProviders = this.filterProviders( + const entityProviders = filterProviders( lodash.uniqBy( [...this.entityProviders, locationStore, configLocationProvider], provider => provider.getProviderName(), ), + config, ); const processingEngine = new DefaultCatalogProcessingEngine({ @@ -684,42 +686,7 @@ export class CatalogBuilder { this.checkMissingExternalProcessors(processors); - const filteredProcessors = this.filterProcessors(processors); - - // Lastly sort the processors by priority. Config can override the - // priority of a processor to allow control of 3rd party processors. - filteredProcessors.sort((a, b) => { - const getProcessorPriority = (processor: CatalogProcessor) => { - try { - return ( - config.getOptionalNumber( - `catalog.processorOptions.${processor.getProcessorName()}.priority`, - ) ?? - processor.getPriority?.() ?? - 20 - ); - } catch (_) { - // In case the processor config throws, just return default priority - return 20; - } - }; - - const aPriority = getProcessorPriority(a); - const bPriority = getProcessorPriority(b); - return aPriority - bPriority; - }); - - return filteredProcessors; - } - - private filterProcessors(processors: CatalogProcessor[]) { - const { config } = this.env; - return processors.filter( - p => - config.getOptionalBoolean( - `catalog.processorOptions.${p.getProcessorName()}.disabled`, - ) !== true, - ); + return filterAndSortProcessors(processors, config); } // TODO(Rugvip): These old processors are removed, for a while we'll be throwing @@ -832,16 +799,6 @@ export class CatalogBuilder { ); } - private filterProviders(providers: EntityProvider[]) { - const { config } = this.env; - return providers.filter( - p => - config.getOptionalBoolean( - `catalog.providerOptions.${p.getProviderName()}.disabled`, - ) !== true, - ); - } - private static getDefaultProcessingInterval( config: Config, ): ProcessingIntervalFunction { diff --git a/plugins/catalog-backend/src/service/util.test.ts b/plugins/catalog-backend/src/service/util.test.ts new file mode 100644 index 0000000000..9a0d2cc799 --- /dev/null +++ b/plugins/catalog-backend/src/service/util.test.ts @@ -0,0 +1,195 @@ +/* + * Copyright 2025 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 { mockServices } from '@backstage/backend-test-utils'; +import { filterAndSortProcessors, filterProviders } from './util'; + +describe('filterAndSortProcessors', () => { + it('should filter processors', () => { + const p1 = { getProcessorName: () => 'processor1' }; + const p2 = { getProcessorName: () => 'processor2' }; + + expect( + filterAndSortProcessors([p1, p2], mockServices.rootConfig({ data: {} })), + ).toEqual([p1, p2]); + + expect( + filterAndSortProcessors( + [p1, p2], + mockServices.rootConfig({ + data: { + catalog: { + processorOptions: { + processor1: { disabled: true }, + }, + }, + }, + }), + ), + ).toEqual([p2]); + + expect( + filterAndSortProcessors( + [p1, p2], + mockServices.rootConfig({ + data: { + catalog: { + processorOptions: { + processor2: { disabled: true }, + }, + }, + }, + }), + ), + ).toEqual([p1]); + }); + + it('should sort processors', () => { + const p1 = { getProcessorName: () => 'processor1', getPriority: () => 10 }; + const p2 = { getProcessorName: () => 'processor2' }; + const p3 = { + getProcessorName: () => 'processor3', + getPriority: () => { + throw new Error('failed'); + }, + }; + + expect( + filterAndSortProcessors( + [p1, p2, p3], + mockServices.rootConfig({ data: {} }), + ), + ).toEqual([p1, p2, p3]); // p2 and p3 got the defaults + + expect( + filterAndSortProcessors( + [p1, p2, p3], + mockServices.rootConfig({ + data: { + catalog: { + processorOptions: { + processor2: { priority: 2 }, + processor3: { priority: 1 }, + }, + }, + }, + }), + ), + ).toEqual([p3, p2, p1]); + }); + + it('rejects invalid config', () => { + const p1 = { getProcessorName: () => 'processor1' }; + const p2 = { getProcessorName: () => 'processor1' }; + + expect(() => + filterAndSortProcessors( + [p1, p2], + mockServices.rootConfig({ + data: { + catalog: { + processorOptions: { + processor1: { disabled: 'i guess so, maybe' }, + }, + }, + }, + }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Unable to convert config value for key 'catalog.processorOptions.processor1.disabled' in 'mock-config' to a boolean"`, + ); + + expect(() => + filterAndSortProcessors( + [p1, p2], + mockServices.rootConfig({ + data: { + catalog: { + processorOptions: { + processor1: { priority: 'somewhere in the middle, roughly' }, + }, + }, + }, + }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Unable to convert config value for key 'catalog.processorOptions.processor1.priority' in 'mock-config' to a number"`, + ); + }); +}); + +describe('filterProviders', () => { + it('should filter providers', () => { + const p1 = { getProviderName: () => 'provider1', connect: jest.fn() }; + const p2 = { getProviderName: () => 'provider2', connect: jest.fn() }; + + expect( + filterProviders([p1, p2], mockServices.rootConfig({ data: {} })), + ).toEqual([p1, p2]); + + expect( + filterProviders( + [p1, p2], + mockServices.rootConfig({ + data: { + catalog: { + providerOptions: { + provider1: { disabled: true }, + }, + }, + }, + }), + ), + ).toEqual([p2]); + + expect( + filterProviders( + [p1, p2], + mockServices.rootConfig({ + data: { + catalog: { + providerOptions: { + provider2: { disabled: true }, + }, + }, + }, + }), + ), + ).toEqual([p1]); + }); + + it('rejects invalid config', () => { + const p1 = { getProviderName: () => 'provider1', connect: jest.fn() }; + const p2 = { getProviderName: () => 'provider2', connect: jest.fn() }; + + expect(() => + filterProviders( + [p1, p2], + mockServices.rootConfig({ + data: { + catalog: { + providerOptions: { + provider1: { disabled: 'i guess so, maybe' }, + }, + }, + }, + }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Unable to convert config value for key 'catalog.providerOptions.provider1.disabled' in 'mock-config' to a boolean"`, + ); + }); +}); diff --git a/plugins/catalog-backend/src/service/util.ts b/plugins/catalog-backend/src/service/util.ts index 9847cb7225..54f6a6afae 100644 --- a/plugins/catalog-backend/src/service/util.ts +++ b/plugins/catalog-backend/src/service/util.ts @@ -24,12 +24,17 @@ import { QueryEntitiesInitialRequest, QueryEntitiesRequest, } from '../catalog/types'; -import { EntityFilter } from '@backstage/plugin-catalog-node'; +import { + CatalogProcessor, + EntityFilter, + EntityProvider, +} from '@backstage/plugin-catalog-node'; import { Entity, parseEntityRef, stringifyEntityRef, } from '@backstage/catalog-model'; +import { Config } from '@backstage/config'; export async function requireRequestBody(req: Request): Promise { const contentType = req.header('content-type'); @@ -166,3 +171,80 @@ export function expandLegacyCompoundRelationsInEntity(entity: Entity): Entity { } return entity; } + +/** + * Given a list of catalog processors, filter out the ones that are disabled + * through the `catalog.processorOptions` config and sort them by priority. + */ +export function filterAndSortProcessors( + processors: CatalogProcessor[], + config: Config, +): CatalogProcessor[] { + function getProcessorOptions( + processor: CatalogProcessor, + ): Config | undefined { + const root = config.getOptionalConfig('catalog.processorOptions'); + try { + return root?.getOptionalConfig(processor.getProcessorName()); + } catch { + // We silence errors specifically here, to cover for cases where the + // processor name contains special characters which makes the config + // reader throw an error. + return undefined; + } + } + + function getProcessorDisabled(processor: CatalogProcessor): boolean { + return ( + getProcessorOptions(processor)?.getOptionalBoolean('disabled') === true + ); + } + + function getProcessorPriority(processor: CatalogProcessor): number { + let priority = + getProcessorOptions(processor)?.getOptionalNumber('priority'); + + if (priority === undefined) { + try { + priority = processor.getPriority?.(); + } catch { + // In case the processor method throws, just return default priority + } + } + + return priority ?? 20; + } + + return processors + .filter(p => !getProcessorDisabled(p)) + .sort((a, b) => getProcessorPriority(a) - getProcessorPriority(b)); +} + +/** + * Given a list of entity providers, filter out the ones that are disabled + * through the `catalog.providerOptions` config. + */ +export function filterProviders( + providers: EntityProvider[], + config: Config, +): EntityProvider[] { + function getProviderOptions(provider: EntityProvider): Config | undefined { + const root = config.getOptionalConfig('catalog.providerOptions'); + try { + return root?.getOptionalConfig(provider.getProviderName()); + } catch { + // We silence errors specifically here, to cover for cases where the + // provider name contains special characters which makes the config + // reader throw an error. + return undefined; + } + } + + function getProviderDisabled(provider: EntityProvider): boolean { + return ( + getProviderOptions(provider)?.getOptionalBoolean('disabled') === true + ); + } + + return providers.filter(p => !getProviderDisabled(p)); +} From e037246a3a79199acf37bf9fe0207f0d3da86972 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 24 Sep 2025 13:47:11 +0200 Subject: [PATCH 2/2] do is instead of get MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- plugins/catalog-backend/src/service/util.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/catalog-backend/src/service/util.ts b/plugins/catalog-backend/src/service/util.ts index 54f6a6afae..2d3d4879cb 100644 --- a/plugins/catalog-backend/src/service/util.ts +++ b/plugins/catalog-backend/src/service/util.ts @@ -194,7 +194,7 @@ export function filterAndSortProcessors( } } - function getProcessorDisabled(processor: CatalogProcessor): boolean { + function isProcessorDisabled(processor: CatalogProcessor): boolean { return ( getProcessorOptions(processor)?.getOptionalBoolean('disabled') === true ); @@ -216,7 +216,7 @@ export function filterAndSortProcessors( } return processors - .filter(p => !getProcessorDisabled(p)) + .filter(p => !isProcessorDisabled(p)) .sort((a, b) => getProcessorPriority(a) - getProcessorPriority(b)); } @@ -240,11 +240,11 @@ export function filterProviders( } } - function getProviderDisabled(provider: EntityProvider): boolean { + function isProviderDisabled(provider: EntityProvider): boolean { return ( getProviderOptions(provider)?.getOptionalBoolean('disabled') === true ); } - return providers.filter(p => !getProviderDisabled(p)); + return providers.filter(p => !isProviderDisabled(p)); }