diff --git a/.changeset/brave-teeth-reply.md b/.changeset/brave-teeth-reply.md new file mode 100644 index 0000000000..8a500692a7 --- /dev/null +++ b/.changeset/brave-teeth-reply.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Internal refactor to remove remnants of the old backend system diff --git a/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.test.ts b/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.test.ts index ab54a7b885..93c22aab1e 100644 --- a/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.test.ts +++ b/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.test.ts @@ -70,6 +70,7 @@ describe('DefaultCatalogProcessingEngine', () => { orchestrator: orchestrator, stitcher: stitcher, createHash: () => hash, + scheduler: mockServices.scheduler(), }); db.transaction.mockImplementation(cb => cb((() => {}) as any)); @@ -136,6 +137,7 @@ describe('DefaultCatalogProcessingEngine', () => { knex: {} as any, orchestrator: orchestrator, stitcher: stitcher, + scheduler: mockServices.scheduler(), createHash: () => hash, }); @@ -219,6 +221,7 @@ describe('DefaultCatalogProcessingEngine', () => { knex: {} as any, orchestrator: orchestrator, stitcher: stitcher, + scheduler: mockServices.scheduler(), createHash: () => hash, }); @@ -296,6 +299,7 @@ describe('DefaultCatalogProcessingEngine', () => { knex: {} as any, orchestrator: orchestrator, stitcher: stitcher, + scheduler: mockServices.scheduler(), createHash: () => hash, }); @@ -355,6 +359,7 @@ describe('DefaultCatalogProcessingEngine', () => { knex: {} as any, orchestrator: orchestrator, stitcher: stitcher, + scheduler: mockServices.scheduler(), createHash: () => hash, pollingIntervalMs: 100, }); @@ -470,6 +475,7 @@ describe('DefaultCatalogProcessingEngine', () => { knex: {} as any, orchestrator: orchestrator, stitcher: stitcher, + scheduler: mockServices.scheduler(), createHash: () => hash, pollingIntervalMs: 100, }); @@ -575,6 +581,7 @@ describe('DefaultCatalogProcessingEngine', () => { knex: {} as any, orchestrator: orchestrator, stitcher: stitcher, + scheduler: mockServices.scheduler(), createHash: () => hash, pollingIntervalMs: 100, }); @@ -658,6 +665,7 @@ describe('DefaultCatalogProcessingEngine', () => { knex: {} as any, orchestrator: orchestrator, stitcher: stitcher, + scheduler: mockServices.scheduler(), createHash: () => hash, pollingIntervalMs: 100, }); @@ -746,6 +754,7 @@ describe('DefaultCatalogProcessingEngine', () => { knex: {} as any, orchestrator: orchestrator, stitcher: stitcher, + scheduler: mockServices.scheduler(), createHash: () => hash, pollingIntervalMs: 100, }); diff --git a/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.ts b/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.ts index 266b80f9a4..7061309175 100644 --- a/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.ts +++ b/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.ts @@ -59,7 +59,7 @@ const stableStringifyArray = (arr: any[]) => { // is just one. export class DefaultCatalogProcessingEngine { private readonly config: Config; - private readonly scheduler?: SchedulerService; + private readonly scheduler: SchedulerService; private readonly logger: LoggerService; private readonly knex: Knex; private readonly processingDatabase: ProcessingDatabase; @@ -79,7 +79,7 @@ export class DefaultCatalogProcessingEngine { constructor(options: { config: Config; - scheduler?: SchedulerService; + scheduler: SchedulerService; logger: LoggerService; knex: Knex; processingDatabase: ProcessingDatabase; @@ -370,25 +370,17 @@ export class DefaultCatalogProcessingEngine { } }; - if (this.scheduler) { - const abortController = new AbortController(); + const abortController = new AbortController(); + this.scheduler.scheduleTask({ + id: 'catalog_orphan_cleanup', + frequency: { milliseconds: this.orphanCleanupIntervalMs }, + timeout: { milliseconds: this.orphanCleanupIntervalMs * 0.8 }, + fn: runOnce, + signal: abortController.signal, + }); - this.scheduler.scheduleTask({ - id: 'catalog_orphan_cleanup', - frequency: { milliseconds: this.orphanCleanupIntervalMs }, - timeout: { milliseconds: this.orphanCleanupIntervalMs * 0.8 }, - fn: runOnce, - signal: abortController.signal, - }); - - return () => { - abortController.abort(); - }; - } - - const intervalKey = setInterval(runOnce, this.orphanCleanupIntervalMs); return () => { - clearInterval(intervalKey); + abortController.abort(); }; } } diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index dcd677152e..40fcc3a624 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -119,10 +119,10 @@ export type CatalogEnvironment = { reader: UrlReaderService; permissions: PermissionsService | PermissionAuthorizer; permissionsRegistry?: PermissionsRegistryService; - scheduler?: SchedulerService; + scheduler: SchedulerService; auth: AuthService; httpAuth: HttpAuthService; - auditor?: AuditorService; + auditor: AuditorService; }; /** diff --git a/plugins/catalog-backend/src/service/DefaultRefreshService.test.ts b/plugins/catalog-backend/src/service/DefaultRefreshService.test.ts index a89b21f79f..f999400b8a 100644 --- a/plugins/catalog-backend/src/service/DefaultRefreshService.test.ts +++ b/plugins/catalog-backend/src/service/DefaultRefreshService.test.ts @@ -121,6 +121,7 @@ describe('DefaultRefreshService', () => { processingDatabase: db, knex: knex, stitcher: stitcher, + scheduler: mockServices.scheduler(), orchestrator: { async process(request: EntityProcessingRequest) { const entityRef = stringifyEntityRef(request.entity); diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index 0246a3d56b..299395552a 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -156,12 +156,12 @@ describe('createRouter readonly disabled', () => { logger: mockServices.logger.mock(), refreshService, config: new ConfigReader(undefined), - permissionIntegrationRouter: express.Router(), auth: mockServices.auth(), httpAuth: mockServices.httpAuth(), locationAnalyzer, permissionsService, enableRelationsCompatibility: true, // added + auditor: mockServices.auditor.mock(), }); app = await wrapServer(express().use(router)); @@ -218,12 +218,12 @@ describe('createRouter readonly disabled', () => { logger: mockServices.logger.mock(), refreshService, config: new ConfigReader(undefined), - permissionIntegrationRouter: express.Router(), auth: mockServices.auth(), httpAuth: mockServices.httpAuth(), locationAnalyzer, permissionsService, enableRelationsCompatibility: true, + auditor: mockServices.auditor.mock(), }); app = await wrapServer(express().use(router)); entitiesCatalog.entities.mockResolvedValueOnce({ @@ -951,6 +951,7 @@ describe('createRouter readonly and raw json enabled', () => { permissionIntegrationRouter: express.Router(), auth: mockServices.auth(), httpAuth: mockServices.httpAuth(), + orchestrator: { process: jest.fn() }, permissionsService, auditor: mockServices.auditor.mock(), }); @@ -1171,6 +1172,7 @@ describe('NextRouter permissioning', () => { }), auth: mockServices.auth(), httpAuth: mockServices.httpAuth(), + orchestrator: { process: jest.fn() }, permissionsService, auditor: mockServices.auditor.mock(), }); diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index 9c9b97eb4c..dc7678a3b9 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -20,7 +20,6 @@ import { HttpAuthService, LoggerService, PermissionsService, - SchedulerService, } from '@backstage/backend-plugin-api'; import { ANNOTATION_LOCATION, @@ -70,17 +69,15 @@ export interface RouterOptions { entitiesCatalog?: EntitiesCatalog; locationAnalyzer?: LocationAnalyzer; locationService: LocationService; - orchestrator?: CatalogProcessingOrchestrator; + orchestrator: CatalogProcessingOrchestrator; refreshService?: RefreshService; - scheduler?: SchedulerService; logger: LoggerService; config: Config; permissionIntegrationRouter?: express.Router; auth: AuthService; httpAuth: HttpAuthService; permissionsService: PermissionsService; - // TODO: Require AuditorService once `backend-legacy` is removed - auditor?: AuditorService; + auditor: AuditorService; enableRelationsCompatibility?: boolean; } @@ -124,7 +121,7 @@ export async function createRouter( router.post('/refresh', async (req, res) => { const { authorizationToken, ...restBody } = req.body; - const auditorEvent = await auditor?.createEvent({ + const auditorEvent = await auditor.createEvent({ eventId: 'entity-mutate', severityLevel: 'medium', meta: { @@ -160,7 +157,7 @@ export async function createRouter( if (entitiesCatalog) { router .get('/entities', async (req, res) => { - const auditorEvent = await auditor?.createEvent({ + const auditorEvent = await auditor.createEvent({ eventId: 'entity-fetch', request: req, meta: { @@ -258,7 +255,7 @@ export async function createRouter( } }) .get('/entities/by-query', async (req, res) => { - const auditorEvent = await auditor?.createEvent({ + const auditorEvent = await auditor.createEvent({ eventId: 'entity-fetch', request: req, meta: { @@ -311,7 +308,7 @@ export async function createRouter( .get('/entities/by-uid/:uid', async (req, res) => { const { uid } = req.params; - const auditorEvent = await auditor?.createEvent({ + const auditorEvent = await auditor.createEvent({ eventId: 'entity-fetch', request: req, meta: { @@ -356,7 +353,7 @@ export async function createRouter( .delete('/entities/by-uid/:uid', async (req, res) => { const { uid } = req.params; - const auditorEvent = await auditor?.createEvent({ + const auditorEvent = await auditor.createEvent({ eventId: 'entity-mutate', severityLevel: 'medium', request: req, @@ -385,7 +382,7 @@ export async function createRouter( const { kind, namespace, name } = req.params; const entityRef = stringifyEntityRef({ kind, namespace, name }); - const auditorEvent = await auditor?.createEvent({ + const auditorEvent = await auditor.createEvent({ eventId: 'entity-fetch', request: req, meta: { @@ -420,7 +417,7 @@ export async function createRouter( const { kind, namespace, name } = req.params; const entityRef = stringifyEntityRef({ kind, namespace, name }); - const auditorEvent = await auditor?.createEvent({ + const auditorEvent = await auditor.createEvent({ eventId: 'entity-fetch', request: req, meta: { @@ -456,7 +453,7 @@ export async function createRouter( }, ) .post('/entities/by-refs', async (req, res) => { - const auditorEvent = await auditor?.createEvent({ + const auditorEvent = await auditor.createEvent({ eventId: 'entity-fetch', request: req, meta: { @@ -495,7 +492,7 @@ export async function createRouter( } }) .get('/entity-facets', async (req, res) => { - const auditorEvent = await auditor?.createEvent({ + const auditorEvent = await auditor.createEvent({ eventId: 'entity-facets', request: req, }); @@ -525,7 +522,7 @@ export async function createRouter( const location = await validateRequestBody(req, locationInput); const dryRun = yn(req.query.dryRun, { default: false }); - const auditorEvent = await auditor?.createEvent({ + const auditorEvent = await auditor.createEvent({ eventId: 'location-mutate', severityLevel: dryRun ? 'low' : 'medium', request: req, @@ -570,7 +567,7 @@ export async function createRouter( } }) .get('/locations', async (req, res) => { - const auditorEvent = await auditor?.createEvent({ + const auditorEvent = await auditor.createEvent({ eventId: 'location-fetch', request: req, meta: { @@ -597,7 +594,7 @@ export async function createRouter( .get('/locations/:id', async (req, res) => { const { id } = req.params; - const auditorEvent = await auditor?.createEvent({ + const auditorEvent = await auditor.createEvent({ eventId: 'location-fetch', request: req, meta: { @@ -628,7 +625,7 @@ export async function createRouter( .delete('/locations/:id', async (req, res) => { const { id } = req.params; - const auditorEvent = await auditor?.createEvent({ + const auditorEvent = await auditor.createEvent({ eventId: 'location-mutate', severityLevel: 'medium', request: req, @@ -659,7 +656,7 @@ export async function createRouter( const { kind, namespace, name } = req.params; const locationRef = `${kind}:${namespace}/${name}`; - const auditorEvent = await auditor?.createEvent({ + const auditorEvent = await auditor.createEvent({ eventId: 'location-fetch', request: req, meta: { @@ -692,7 +689,7 @@ export async function createRouter( if (locationAnalyzer) { router.post('/analyze-location', async (req, res) => { - const auditorEvent = await auditor?.createEvent({ + const auditorEvent = await auditor.createEvent({ eventId: 'location-analyze', request: req, }); @@ -743,86 +740,84 @@ export async function createRouter( }); } - if (orchestrator) { - router.post('/validate-entity', async (req, res) => { - const auditorEvent = await auditor?.createEvent({ - eventId: 'entity-validate', - request: req, + router.post('/validate-entity', async (req, res) => { + const auditorEvent = await auditor.createEvent({ + eventId: 'entity-validate', + request: req, + }); + + try { + const bodySchema = z.object({ + entity: z.unknown(), + location: z.string(), }); + let body: z.infer; + let entity: Entity; + let location: { type: string; target: string }; try { - const bodySchema = z.object({ - entity: z.unknown(), - location: z.string(), - }); - - let body: z.infer; - let entity: Entity; - let location: { type: string; target: string }; - try { - body = await validateRequestBody(req, bodySchema); - entity = validateEntityEnvelope(body.entity); - location = parseLocationRef(body.location); - if (location.type !== 'url') - throw new TypeError( - `Invalid location ref ${body.location}, only 'url:' is supported, e.g. url:https://host/path`, - ); - } catch (err) { - await auditorEvent?.fail({ - error: err, - }); - - return res.status(400).json({ - errors: [serializeError(err)], - }); - } - - const credentials = await httpAuth.credentials(req); - const authorizedValidationService = new AuthorizedValidationService( - orchestrator, - permissionsService, - ); - const processingResult = await authorizedValidationService.process( - { - entity: { - ...entity, - metadata: { - ...entity.metadata, - annotations: { - [ANNOTATION_LOCATION]: body.location, - [ANNOTATION_ORIGIN_LOCATION]: body.location, - ...entity.metadata.annotations, - }, - }, - }, - }, - credentials, - ); - - if (!processingResult.ok) { - const errors = processingResult.errors.map(e => serializeError(e)); - - await auditorEvent?.fail({ - // TODO(Rugvip): Seems like there aren't proper types for AggregateError yet - error: (AggregateError as any)(errors, 'Could not validate entity'), - }); - - res.status(400).json({ - errors, - }); - } - - await auditorEvent?.success(); - - return res.status(200).end(); + body = await validateRequestBody(req, bodySchema); + entity = validateEntityEnvelope(body.entity); + location = parseLocationRef(body.location); + if (location.type !== 'url') + throw new TypeError( + `Invalid location ref ${body.location}, only 'url:' is supported, e.g. url:https://host/path`, + ); } catch (err) { await auditorEvent?.fail({ error: err, }); - throw err; + + return res.status(400).json({ + errors: [serializeError(err)], + }); } - }); - } + + const credentials = await httpAuth.credentials(req); + const authorizedValidationService = new AuthorizedValidationService( + orchestrator, + permissionsService, + ); + const processingResult = await authorizedValidationService.process( + { + entity: { + ...entity, + metadata: { + ...entity.metadata, + annotations: { + [ANNOTATION_LOCATION]: body.location, + [ANNOTATION_ORIGIN_LOCATION]: body.location, + ...entity.metadata.annotations, + }, + }, + }, + }, + credentials, + ); + + if (!processingResult.ok) { + const errors = processingResult.errors.map(e => serializeError(e)); + + await auditorEvent?.fail({ + // TODO(Rugvip): Seems like there aren't proper types for AggregateError yet + error: (AggregateError as any)(errors, 'Could not validate entity'), + }); + + res.status(400).json({ + errors, + }); + } + + await auditorEvent?.success(); + + return res.status(200).end(); + } catch (err) { + await auditorEvent?.fail({ + error: err, + }); + throw err; + } + }); return router; } diff --git a/plugins/catalog-backend/src/tests/integration.test.ts b/plugins/catalog-backend/src/tests/integration.test.ts index 719c5e6ccf..486d1bfb99 100644 --- a/plugins/catalog-backend/src/tests/integration.test.ts +++ b/plugins/catalog-backend/src/tests/integration.test.ts @@ -296,6 +296,7 @@ class TestHarness { knex: options.db, orchestrator, stitcher, + scheduler: mockServices.scheduler(), createHash: () => createHash('sha1'), pollingIntervalMs: 50, onProcessingError: event => {