From c71e8770e243a71d6013b14b443c836eb0ba9f10 Mon Sep 17 00:00:00 2001 From: Matt Wise Date: Tue, 13 Feb 2024 09:12:48 -0600 Subject: [PATCH 01/26] update tests to include subdirectory validation Signed-off-by: Matt Wise --- .../awsS3/awsS3-mock-object3.yaml | 1 + .../tree/ReadableArrayResponse.test.ts | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 packages/backend-common/src/reading/__fixtures__/awsS3/awsS3-mock-object3.yaml diff --git a/packages/backend-common/src/reading/__fixtures__/awsS3/awsS3-mock-object3.yaml b/packages/backend-common/src/reading/__fixtures__/awsS3/awsS3-mock-object3.yaml new file mode 100644 index 0000000000..b622af142a --- /dev/null +++ b/packages/backend-common/src/reading/__fixtures__/awsS3/awsS3-mock-object3.yaml @@ -0,0 +1 @@ +site_name: Test3 diff --git a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts index e2bfe1af59..7420f14d70 100644 --- a/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ReadableArrayResponse.test.ts @@ -31,6 +31,12 @@ const file2 = fs.readFileSync( path.resolve(__filename, '../../__fixtures__/awsS3/awsS3-mock-object2.yaml'), ); +const dir1 = 'dir1'; +const name3 = `file3.yaml`; +const file3 = fs.readFileSync( + path.resolve(__filename, '../../__fixtures__/awsS3/awsS3-mock-object3.yaml'), +); + describe('ReadableArrayResponse', () => { const sourceDir = createMockDirectory(); const targetDir = createMockDirectory(); @@ -39,6 +45,9 @@ describe('ReadableArrayResponse', () => { sourceDir.setContent({ [name1]: file1, [name2]: file2, + [dir1]: { + [name3]: file3, + }, }); targetDir.clear(); }); @@ -56,11 +65,13 @@ describe('ReadableArrayResponse', () => { const path1 = sourceDir.resolve(name1); const path2 = sourceDir.resolve(name2); + const path3 = sourceDir.resolve(`${dir1}/${name3}`); it('should read files', async () => { const arr: FromReadableArrayOptions = [ { data: createReadStream(path1), path: path1 }, { data: createReadStream(path2), path: path2 }, + { data: createReadStream(path3), path: path3 }, ]; const res = new ReadableArrayResponse(arr, targetDir.path, 'etag'); @@ -69,18 +80,21 @@ describe('ReadableArrayResponse', () => { expect(files).toEqual([ { path: path1, content: expect.any(Function) }, { path: path2, content: expect.any(Function) }, + { path: path3, content: expect.any(Function) }, ]); const contents = await Promise.all(files.map(f => f.content())); - expect(contents).toEqual([file1, file2]); + expect(contents).toEqual([file1, file2, file3]); }); it('should extract entire archive into directory', async () => { const relativePath1 = relative(sourceDir.path, path1); const relativePath2 = relative(sourceDir.path, path2); + const relativePath3 = relative(sourceDir.path, path3); const arr: FromReadableArrayOptions = [ { data: createReadStream(path1), path: relativePath1 }, { data: createReadStream(path2), path: relativePath2 }, + { data: createReadStream(path3), path: relativePath3 }, ]; const res = new ReadableArrayResponse(arr, targetDir.path, 'etag'); @@ -89,6 +103,9 @@ describe('ReadableArrayResponse', () => { expect(targetDir.content({ path: dir })).toEqual({ [name1]: file1.toString('utf8'), [name2]: file2.toString('utf8'), + [dir1]: { + [name3]: file3.toString('utf8'), + }, }); }); }); From 534786aa217935228bf26a63b69281be3ccf9e21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ragnar=20Halld=C3=B3rsson?= Date: Thu, 15 Feb 2024 21:38:00 +0100 Subject: [PATCH 02/26] fix(catalog-backend-module-azure): Add branch to Code Search query when provided MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ragnar Halldórsson --- .changeset/real-keys-juggle.md | 5 ++ .../src/lib/azure.test.ts | 60 ++++++++++++++++++ .../src/lib/azure.ts | 39 ++++++++---- .../AzureDevOpsDiscoveryProcessor.test.ts | 63 +++++++++++++++++++ .../AzureDevOpsDiscoveryProcessor.ts | 20 +++++- .../providers/AzureDevOpsEntityProvider.ts | 1 + 6 files changed, 175 insertions(+), 13 deletions(-) create mode 100644 .changeset/real-keys-juggle.md diff --git a/.changeset/real-keys-juggle.md b/.changeset/real-keys-juggle.md new file mode 100644 index 0000000000..cb323853de --- /dev/null +++ b/.changeset/real-keys-juggle.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend-module-azure': minor +--- + +Fixed issue where specifying a branch for discovery did not work diff --git a/plugins/catalog-backend-module-azure/src/lib/azure.test.ts b/plugins/catalog-backend-module-azure/src/lib/azure.test.ts index 493f0dda51..ce06fd6221 100644 --- a/plugins/catalog-backend-module-azure/src/lib/azure.test.ts +++ b/plugins/catalog-backend-module-azure/src/lib/azure.test.ts @@ -214,6 +214,66 @@ describe('azure', () => { ).resolves.toEqual(response.results); }); + it('searches in specific branch if parameter is set', async () => { + const response: CodeSearchResponse = { + count: 1, + results: [ + { + fileName: 'catalog-info.yaml', + path: '/catalog-info.yaml', + project: { + name: '*', + }, + repository: { + name: 'backstage', + }, + }, + ], + }; + + server.use( + rest.post( + `https://almsearch.dev.azure.com/shopify/_apis/search/codesearchresults`, + (req, res, ctx) => { + expect(req.headers.get('Authorization')).toBe('Basic OkFCQw=='); + expect(req.body).toEqual({ + searchText: + 'path:/catalog-info.yaml repo:backstage proj:engineering', + $orderBy: [ + { + field: 'path', + sortOrder: 'ASC', + }, + ], + $skip: 0, + $top: 1000, + filters: { + Branch: ['development'], + }, + }); + return res(ctx.json(response)); + }, + ), + ); + + const { credentialsProvider, azureConfig } = createFixture( + 'dev.azure.com', + 'ABC', + ); + + await expect( + codeSearch( + credentialsProvider, + azureConfig, + 'shopify', + 'engineering', + 'backstage', + '/catalog-info.yaml', + 'development', + ), + ).resolves.toEqual(response.results); + }); + it('can search using onpremise api', async () => { const response: CodeSearchResponse = { count: 1, diff --git a/plugins/catalog-backend-module-azure/src/lib/azure.ts b/plugins/catalog-backend-module-azure/src/lib/azure.ts index c8ff8dd739..3bdff90110 100644 --- a/plugins/catalog-backend-module-azure/src/lib/azure.ts +++ b/plugins/catalog-backend-module-azure/src/lib/azure.ts @@ -37,6 +37,16 @@ export interface CodeSearchResultItem { branch?: string; } +interface CodeSearchRequest { + searchText: string; + $orderBy: Array<{ field: string; sortOrder: string }>; + $skip: number; + $top: number; + filters?: { + Branch: string[]; + }; +} + const isCloud = (host: string) => host === 'dev.azure.com'; const PAGE_SIZE = 1000; @@ -48,6 +58,7 @@ export async function codeSearch( project: string, repo: string, path: string, + branch: string, ): Promise { const searchBaseUrl = isCloud(azureConfig.host) ? 'https://almsearch.dev.azure.com' @@ -62,23 +73,29 @@ export async function codeSearch( url: `https://${azureConfig.host}/${org}`, }); + const searchRequestBody: CodeSearchRequest = { + searchText: `path:${path} repo:${repo || '*'} proj:${project || '*'}`, + $orderBy: [ + { + field: 'path', + sortOrder: 'ASC', + }, + ], + $skip: items.length, + $top: PAGE_SIZE, + }; + + if (branch) { + searchRequestBody.filters = { Branch: [branch] }; + } + const response = await fetch(searchUrl, { headers: { ...credentials?.headers, 'Content-Type': 'application/json', }, method: 'POST', - body: JSON.stringify({ - searchText: `path:${path} repo:${repo || '*'} proj:${project || '*'}`, - $orderBy: [ - { - field: 'path', - sortOrder: 'ASC', - }, - ], - $skip: items.length, - $top: PAGE_SIZE, - }), + body: JSON.stringify(searchRequestBody), }); if (response.status !== 200) { diff --git a/plugins/catalog-backend-module-azure/src/processors/AzureDevOpsDiscoveryProcessor.test.ts b/plugins/catalog-backend-module-azure/src/processors/AzureDevOpsDiscoveryProcessor.test.ts index 36dd920621..0817465d83 100644 --- a/plugins/catalog-backend-module-azure/src/processors/AzureDevOpsDiscoveryProcessor.test.ts +++ b/plugins/catalog-backend-module-azure/src/processors/AzureDevOpsDiscoveryProcessor.test.ts @@ -35,6 +35,7 @@ describe('AzureDevOpsDiscoveryProcessor', () => { project: 'my-proj', repo: '', catalogPath: '/catalog-info.yaml', + branch: '', }); expect( @@ -47,6 +48,7 @@ describe('AzureDevOpsDiscoveryProcessor', () => { project: 'engineering', repo: 'backstage', catalogPath: '/catalog.yaml', + branch: '', }); expect( @@ -59,6 +61,20 @@ describe('AzureDevOpsDiscoveryProcessor', () => { project: 'engineering', repo: 'backstage', catalogPath: '/src/*/catalog.yaml', + branch: '', + }); + + expect( + parseUrl( + 'https://azuredevops.mycompany.com/spotify/engineering/_git/backstage?path=/src/*/catalog.yaml&version=GBdevelopment', + ), + ).toEqual({ + baseUrl: 'https://azuredevops.mycompany.com', + org: 'spotify', + project: 'engineering', + repo: 'backstage', + catalogPath: '/src/*/catalog.yaml', + branch: 'development', }); }); @@ -164,6 +180,7 @@ describe('AzureDevOpsDiscoveryProcessor', () => { 'engineering', '', '/catalog-info.yaml', + '', ); expect(emitter).toHaveBeenCalledTimes(2); expect(emitter).toHaveBeenCalledWith({ @@ -214,6 +231,7 @@ describe('AzureDevOpsDiscoveryProcessor', () => { 'engineering', 'backstage', '/catalog-info.yaml', + '', ); expect(emitter).toHaveBeenCalledTimes(1); expect(emitter).toHaveBeenCalledWith({ @@ -227,6 +245,49 @@ describe('AzureDevOpsDiscoveryProcessor', () => { }); }); + it('output locations with branch if specified in target', async () => { + const location: LocationSpec = { + type: 'azure-discovery', + target: + 'https://dev.azure.com/shopify/engineering/_git/backstage?path=/catalog-info.yaml&version=GBdevelopment', + }; + mockCodeSearch.mockResolvedValueOnce([ + { + fileName: 'catalog-info.yaml', + path: '/catalog-info.yaml', + repository: { + name: 'backstage', + }, + project: { + name: '*', + }, + }, + ]); + const emitter = jest.fn(); + + await processor.readLocation(location, false, emitter); + + expect(mockCodeSearch).toHaveBeenCalledWith( + expect.anything(), + { host: 'dev.azure.com' }, + 'shopify', + 'engineering', + 'backstage', + '/catalog-info.yaml', + 'development', + ); + expect(emitter).toHaveBeenCalledTimes(1); + expect(emitter).toHaveBeenCalledWith({ + type: 'location', + location: { + type: 'url', + target: + 'https://dev.azure.com/shopify/engineering/_git/backstage?path=/catalog-info.yaml&version=GBdevelopment', + presence: 'optional', + }, + }); + }); + it('output single locations with different file name from code search', async () => { const location: LocationSpec = { type: 'azure-discovery', @@ -256,6 +317,7 @@ describe('AzureDevOpsDiscoveryProcessor', () => { 'engineering', '', '/src/*/catalog.yaml', + '', ); expect(emitter).toHaveBeenCalledTimes(1); expect(emitter).toHaveBeenCalledWith({ @@ -286,6 +348,7 @@ describe('AzureDevOpsDiscoveryProcessor', () => { 'engineering', 'backstage', '/catalog-info.yaml', + '', ); expect(emitter).not.toHaveBeenCalled(); }); diff --git a/plugins/catalog-backend-module-azure/src/processors/AzureDevOpsDiscoveryProcessor.ts b/plugins/catalog-backend-module-azure/src/processors/AzureDevOpsDiscoveryProcessor.ts index d9638e6cc7..ddb3c5f59f 100644 --- a/plugins/catalog-backend-module-azure/src/processors/AzureDevOpsDiscoveryProcessor.ts +++ b/plugins/catalog-backend-module-azure/src/processors/AzureDevOpsDiscoveryProcessor.ts @@ -92,7 +92,7 @@ export class AzureDevOpsDiscoveryProcessor implements CatalogProcessor { ); } - const { baseUrl, org, project, repo, catalogPath } = parseUrl( + const { baseUrl, org, project, repo, catalogPath, branch } = parseUrl( location.target, ); this.logger.info( @@ -106,6 +106,7 @@ export class AzureDevOpsDiscoveryProcessor implements CatalogProcessor { project, repo, catalogPath, + branch, ); this.logger.debug( @@ -113,10 +114,16 @@ export class AzureDevOpsDiscoveryProcessor implements CatalogProcessor { ); for (const file of files) { + let target = `${baseUrl}/${org}/${project}/_git/${file.repository.name}?path=${file.path}`; + + if (branch) { + target += `&version=GB${branch}`; + } + emit( processingResult.location({ type: 'url', - target: `${baseUrl}/${org}/${project}/_git/${file.repository.name}?path=${file.path}`, + target, // Not all locations may actually exist, since the user defined them as a wildcard pattern. // Thus, we emit them as optional and let the downstream processor find them while not outputting // an error if it couldn't. @@ -138,11 +145,18 @@ export function parseUrl(urlString: string): { project: string; repo: string; catalogPath: string; + branch: string; } { const url = new URL(urlString); const path = url.pathname.slice(1).split('/'); const catalogPath = url.searchParams.get('path') || '/catalog-info.yaml'; + let branch = url.searchParams.get('version') || ''; + + if (branch.startsWith('GB')) { + // DevOps prefixes branch names with 'GB' in URLs + branch = branch.slice(2); + } if (path.length === 2 && path[0].length && path[1].length) { return { @@ -151,6 +165,7 @@ export function parseUrl(urlString: string): { project: decodeURIComponent(path[1]), repo: '', catalogPath, + branch, }; } else if ( path.length === 4 && @@ -165,6 +180,7 @@ export function parseUrl(urlString: string): { project: decodeURIComponent(path[1]), repo: decodeURIComponent(path[3]), catalogPath, + branch, }; } diff --git a/plugins/catalog-backend-module-azure/src/providers/AzureDevOpsEntityProvider.ts b/plugins/catalog-backend-module-azure/src/providers/AzureDevOpsEntityProvider.ts index f56369a0d1..44e618ffcd 100644 --- a/plugins/catalog-backend-module-azure/src/providers/AzureDevOpsEntityProvider.ts +++ b/plugins/catalog-backend-module-azure/src/providers/AzureDevOpsEntityProvider.ts @@ -158,6 +158,7 @@ export class AzureDevOpsEntityProvider implements EntityProvider { this.config.project, this.config.repository, this.config.path, + this.config.branch || '', ); logger.info(`Discovered ${files.length} catalog files`); From 4fcb4667f07bdba361e0118d15229276a194ef08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ragnar=20Halld=C3=B3rsson?= Date: Thu, 15 Feb 2024 22:15:41 +0100 Subject: [PATCH 03/26] chore(catalog-backend-module-azure): Update discovery documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ragnar Halldórsson --- docs/integrations/azure/discovery.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/integrations/azure/discovery.md b/docs/integrations/azure/discovery.md index 742fbb6076..ad454c0ed1 100644 --- a/docs/integrations/azure/discovery.md +++ b/docs/integrations/azure/discovery.md @@ -160,11 +160,14 @@ catalog: # Or use a custom file format and location - type: azure-discovery target: https://dev.azure.com/myorg/myproject/_git/*?path=/src/*/catalog-info.yaml + # And optionally provide a specific branch name using the version parameter + - type: azure-discovery + target: https://dev.azure.com/myorg/myproject/_git/*?path=/catalog-info.yaml&version=GBdevelopment ``` Note the `azure-discovery` type, as this is not a regular `url` processor. -When using a custom pattern, the target is composed of five parts: +When using a custom pattern, the target is composed of six parts: - The base instance URL, `https://dev.azure.com` in this case - The organization name which is required, `myorg` in this case @@ -175,3 +178,4 @@ When using a custom pattern, the target is composed of five parts: - The path within each repository to find the catalog YAML file. This will usually be `/catalog-info.yaml`, `/src/*/catalog-info.yaml` or a similar variation for catalog files stored in the root directory of each repository. +- The repository branch to scan which is optional, `development` in this case. The `GB` prefix is mandatory, as this is how Azure DevOps identifies the version as a branch. If omitted, the repo's default branch will be scanned. From 7446616db052d65474cb0fe3f7ef85c5d33307d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ragnar=20Halld=C3=B3rsson?= Date: Thu, 15 Feb 2024 22:29:20 +0100 Subject: [PATCH 04/26] chore(catalog-backend-module-azure): Update tests I missed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ragnar Halldórsson --- plugins/catalog-backend-module-azure/src/lib/azure.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/catalog-backend-module-azure/src/lib/azure.test.ts b/plugins/catalog-backend-module-azure/src/lib/azure.test.ts index ce06fd6221..21fc77e7f7 100644 --- a/plugins/catalog-backend-module-azure/src/lib/azure.test.ts +++ b/plugins/catalog-backend-module-azure/src/lib/azure.test.ts @@ -89,6 +89,7 @@ describe('azure', () => { 'engineering', '', '/catalog-info.yaml', + '', ), ).resolves.toEqual([]); }); @@ -154,6 +155,7 @@ describe('azure', () => { 'engineering', '', '/catalog-info.yaml', + '', ), ).resolves.toEqual(response.results); }); @@ -210,6 +212,7 @@ describe('azure', () => { 'engineering', 'backstage', '/catalog-info.yaml', + '', ), ).resolves.toEqual(response.results); }); @@ -325,6 +328,7 @@ describe('azure', () => { 'engineering', '', '/catalog-info.yaml', + '', ), ).resolves.toEqual(response.results); }); @@ -384,6 +388,7 @@ describe('azure', () => { 'engineering', 'backstage', '/catalog-info.yaml', + '', ), ).resolves.toHaveLength(totalCount); }); From 7a3d7fcfbd101ebcc695a48d24e9d0428127c529 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ragnar=20Halld=C3=B3rsson?= Date: Thu, 15 Feb 2024 22:39:19 +0100 Subject: [PATCH 05/26] chore(catalog-backend-module-azure): Change version bump to patch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ragnar Halldórsson --- .changeset/real-keys-juggle.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/real-keys-juggle.md b/.changeset/real-keys-juggle.md index cb323853de..55bdbf4db3 100644 --- a/.changeset/real-keys-juggle.md +++ b/.changeset/real-keys-juggle.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-catalog-backend-module-azure': minor +'@backstage/plugin-catalog-backend-module-azure': patch --- Fixed issue where specifying a branch for discovery did not work From d8a54d0a8342965f84677a5a8e395c5c015b0e27 Mon Sep 17 00:00:00 2001 From: Aramis Date: Sat, 27 Jan 2024 22:34:40 -0500 Subject: [PATCH 06/26] feat(catalog): add support for entity policies in the new backend. Signed-off-by: Aramis --- .changeset/thirty-bags-try.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/thirty-bags-try.md diff --git a/.changeset/thirty-bags-try.md b/.changeset/thirty-bags-try.md new file mode 100644 index 0000000000..7a8156e44c --- /dev/null +++ b/.changeset/thirty-bags-try.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': minor +--- + +Add support for defining entity policies in the new backend. From 31392e59b852f4b6384cc84bb03318ade9a088bb Mon Sep 17 00:00:00 2001 From: Aramis Date: Tue, 30 Jan 2024 10:59:28 -0500 Subject: [PATCH 07/26] update to a separate catalog extension and add support for replacing Signed-off-by: Aramis --- .../src/service/CatalogPlugin.ts | 40 ++++++++++++++++++- plugins/catalog-node/api-report-alpha.md | 14 +++++++ plugins/catalog-node/src/alpha.ts | 2 + plugins/catalog-node/src/extensions.ts | 17 +++++++- 4 files changed, 71 insertions(+), 2 deletions(-) diff --git a/plugins/catalog-backend/src/service/CatalogPlugin.ts b/plugins/catalog-backend/src/service/CatalogPlugin.ts index bfe8ef5a16..5bcaaba1fb 100644 --- a/plugins/catalog-backend/src/service/CatalogPlugin.ts +++ b/plugins/catalog-backend/src/service/CatalogPlugin.ts @@ -17,7 +17,7 @@ import { createBackendPlugin, coreServices, } from '@backstage/backend-plugin-api'; -import { Entity } from '@backstage/catalog-model'; +import { Entity, EntityPolicy } from '@backstage/catalog-model'; import { CatalogBuilder, CatalogPermissionRuleInput } from './CatalogBuilder'; import { CatalogAnalysisExtensionPoint, @@ -26,6 +26,8 @@ import { catalogProcessingExtensionPoint, CatalogPermissionExtensionPoint, catalogPermissionExtensionPoint, + CatalogModelExtensionPoint, + catalogModelExtensionPoint, } from '@backstage/plugin-catalog-node/alpha'; import { CatalogProcessor, @@ -124,6 +126,33 @@ class CatalogPermissionExtensionPointImpl } } +class CatalogModelExtensionPointImpl implements CatalogModelExtensionPoint { + #entityPolicies = new Array(); + #replaced = false; + + addEntityPolicies(...policies: Array>) { + this.#entityPolicies.push(...policies.flat()); + } + + replaceEntityPolicies(...policies: Array) { + if (this.#replaced) { + throw new Error( + `You've already replaced the entity policies. If you want to add more entity policies, see 'addEntityPolicies'.`, + ); + } + this.#entityPolicies = [...policies]; + this.#replaced = true; + } + + get entityPolicies() { + return this.#entityPolicies; + } + + get replaced() { + return this.#replaced; + } +} + /** * Catalog plugin * @alpha @@ -150,6 +179,9 @@ export const catalogPlugin = createBackendPlugin({ permissionExtensions, ); + const modelExtensions = new CatalogModelExtensionPointImpl(); + env.registerExtensionPoint(catalogModelExtensionPoint, modelExtensions); + env.registerInit({ deps: { logger: coreServices.logger, @@ -193,6 +225,12 @@ export const catalogPlugin = createBackendPlugin({ builder.addLocationAnalyzers(...analysisExtensions.locationAnalyzers); builder.addPermissionRules(...permissionExtensions.permissionRules); + if (modelExtensions.replaced) { + builder.replaceEntityPolicies(modelExtensions.entityPolicies); + } else { + builder.addEntityPolicy(...modelExtensions.entityPolicies); + } + const { processingEngine, router } = await builder.build(); await processingEngine.start(); diff --git a/plugins/catalog-node/api-report-alpha.md b/plugins/catalog-node/api-report-alpha.md index 5be4b114c0..64382e56bb 100644 --- a/plugins/catalog-node/api-report-alpha.md +++ b/plugins/catalog-node/api-report-alpha.md @@ -7,6 +7,7 @@ import { CatalogApi } from '@backstage/catalog-client'; import { CatalogProcessor } from '@backstage/plugin-catalog-node'; import { EntitiesSearchFilter } from '@backstage/plugin-catalog-node'; import { Entity } from '@backstage/catalog-model'; +import { EntityPolicy } from '@backstage/catalog-model'; import { EntityProvider } from '@backstage/plugin-catalog-node'; import { ExtensionPoint } from '@backstage/backend-plugin-api'; import { PermissionRule } from '@backstage/plugin-permission-node'; @@ -24,6 +25,19 @@ export interface CatalogAnalysisExtensionPoint { // @alpha (undocumented) export const catalogAnalysisExtensionPoint: ExtensionPoint; +// @alpha (undocumented) +export interface CatalogModelExtensionPoint { + // (undocumented) + addEntityPolicies( + ...policies: Array> + ): void; + // (undocumented) + replaceEntityPolicies(...policies: Array): void; +} + +// @alpha (undocumented) +export const catalogModelExtensionPoint: ExtensionPoint; + // @alpha (undocumented) export interface CatalogPermissionExtensionPoint { // (undocumented) diff --git a/plugins/catalog-node/src/alpha.ts b/plugins/catalog-node/src/alpha.ts index 3e958d1df1..91b5bb99b6 100644 --- a/plugins/catalog-node/src/alpha.ts +++ b/plugins/catalog-node/src/alpha.ts @@ -22,3 +22,5 @@ export { catalogAnalysisExtensionPoint } from './extensions'; export type { CatalogPermissionRuleInput } from './extensions'; export type { CatalogPermissionExtensionPoint } from './extensions'; export { catalogPermissionExtensionPoint } from './extensions'; +export type { CatalogModelExtensionPoint } from './extensions'; +export { catalogModelExtensionPoint } from './extensions'; diff --git a/plugins/catalog-node/src/extensions.ts b/plugins/catalog-node/src/extensions.ts index e2ee1b4f4b..7c63e51c69 100644 --- a/plugins/catalog-node/src/extensions.ts +++ b/plugins/catalog-node/src/extensions.ts @@ -15,7 +15,7 @@ */ import { createExtensionPoint } from '@backstage/backend-plugin-api'; -import { Entity } from '@backstage/catalog-model'; +import { Entity, EntityPolicy } from '@backstage/catalog-model'; import { CatalogProcessor, EntitiesSearchFilter, @@ -45,6 +45,15 @@ export interface CatalogProcessingExtensionPoint { ): void; } +/** @alpha */ +export interface CatalogModelExtensionPoint { + addEntityPolicies( + ...policies: Array> + ): void; + + replaceEntityPolicies(...policies: Array): void; +} + /** * @alpha */ @@ -68,6 +77,12 @@ export const catalogAnalysisExtensionPoint = id: 'catalog.analysis', }); +/** @alpha */ +export const catalogModelExtensionPoint = + createExtensionPoint({ + id: 'catalog.model', + }); + /** * @alpha */ From 0aba8f018ed590ce1ef233e90179088e98775794 Mon Sep 17 00:00:00 2001 From: Aramis Date: Fri, 2 Feb 2024 15:29:27 -0500 Subject: [PATCH 08/26] add test cases for the new transformer Signed-off-by: Aramis Signed-off-by: aramissennyeydd --- .../core/policyToProcessorTransformer.test.ts | 90 +++++++++++++++++++ .../core/policyToProcessorTransformer.ts | 41 +++++++++ 2 files changed, 131 insertions(+) create mode 100644 plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.test.ts create mode 100644 plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.ts diff --git a/plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.test.ts b/plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.test.ts new file mode 100644 index 0000000000..46338ce6cb --- /dev/null +++ b/plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.test.ts @@ -0,0 +1,90 @@ +/* + * Copyright 2024 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 { Entity, EntityPolicy } from '@backstage/catalog-model'; +import { policyToProcessorTransformer } from './policyToProcessorTransformer'; +import { clone } from 'lodash'; + +describe('policyToProcessorTransformer', () => { + const entityToProcess: Entity = { + apiVersion: 'backstage.io/v1alpha', + kind: 'Component', + metadata: { + name: 'test', + }, + }; + it('modifies the entity if the policy modifies the entity', async () => { + const policy: EntityPolicy = { + async enforce(entity) { + entity.kind = 'Group'; + return entity; + }, + }; + const processor = policyToProcessorTransformer(policy); + const clonedEntity = clone(entityToProcess); + const entity = await processor.preProcessEntity?.( + clonedEntity, + {} as any, + jest.fn(), + {} as any, + {} as any, + ); + expect(entity).toBeTruthy(); + expect(entity?.kind).toBe('Group'); + expect(entity?.apiVersion).toBe('backstage.io/v1alpha'); + expect(entity?.metadata.name).toBe('test'); + }); + + it('does not modify the entity if the policy returns undefined', async () => { + const policy: EntityPolicy = { + async enforce() { + return undefined; + }, + }; + const processor = policyToProcessorTransformer(policy); + const clonedEntity = clone(entityToProcess); + const entity = await processor.preProcessEntity?.( + clonedEntity, + {} as any, + jest.fn(), + {} as any, + {} as any, + ); + expect(entity).toBeTruthy(); + expect(entity?.kind).toBe('Component'); + expect(entity?.apiVersion).toBe('backstage.io/v1alpha'); + expect(entity?.metadata.name).toBe('test'); + }); + + it('bubbles up processor error', async () => { + const policy: EntityPolicy = { + async enforce() { + throw new TypeError('Invalid value for metadata.name'); + }, + }; + const processor = policyToProcessorTransformer(policy); + const clonedEntity = clone(entityToProcess); + await expect( + processor.preProcessEntity?.( + clonedEntity, + {} as any, + jest.fn(), + {} as any, + {} as any, + ), + ).rejects.toThrow(/Invalid value for metadata.name/); + }); +}); diff --git a/plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.ts b/plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.ts new file mode 100644 index 0000000000..ba1ce254c7 --- /dev/null +++ b/plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.ts @@ -0,0 +1,41 @@ +/* + * Copyright 2024 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 { EntityPolicy } from '@backstage/catalog-model'; +import { CatalogProcessor } from '@backstage/plugin-catalog-node'; + +/** + * Transform a given entity policy to an entity processor. + * @param policy The policy to transform + * @returns A new entity processor that uses the entity policy. + */ +export function policyToProcessorTransformer( + policy: EntityPolicy, +): CatalogProcessor { + return { + getProcessorName() { + return policy.constructor.name; + }, + async preProcessEntity(entity) { + // If enforcing the policy fails, throw the policy error. + const result = await policy.enforce(entity); + if (!result) { + return entity; + } + return result; + }, + }; +} From 447d8b6fcf6ee080001643292df0c13f56de3b06 Mon Sep 17 00:00:00 2001 From: Aramis Date: Fri, 2 Feb 2024 16:42:39 -0500 Subject: [PATCH 09/26] add field validators Signed-off-by: Aramis --- .../src/service/CatalogPlugin.ts | 27 +++++++------------ plugins/catalog-node/src/extensions.ts | 15 +++++++++-- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/plugins/catalog-backend/src/service/CatalogPlugin.ts b/plugins/catalog-backend/src/service/CatalogPlugin.ts index 5bcaaba1fb..1fca11e482 100644 --- a/plugins/catalog-backend/src/service/CatalogPlugin.ts +++ b/plugins/catalog-backend/src/service/CatalogPlugin.ts @@ -17,7 +17,7 @@ import { createBackendPlugin, coreServices, } from '@backstage/backend-plugin-api'; -import { Entity, EntityPolicy } from '@backstage/catalog-model'; +import { Entity, EntityPolicy, Validators } from '@backstage/catalog-model'; import { CatalogBuilder, CatalogPermissionRuleInput } from './CatalogBuilder'; import { CatalogAnalysisExtensionPoint, @@ -36,6 +36,7 @@ import { ScmLocationAnalyzer, } from '@backstage/plugin-catalog-node'; import { loggerToWinstonLogger } from '@backstage/backend-common'; +import { merge } from 'lodash'; class CatalogProcessingExtensionPointImpl implements CatalogProcessingExtensionPoint @@ -128,28 +129,22 @@ class CatalogPermissionExtensionPointImpl class CatalogModelExtensionPointImpl implements CatalogModelExtensionPoint { #entityPolicies = new Array(); - #replaced = false; + #fieldValidators: Partial = {}; addEntityPolicies(...policies: Array>) { this.#entityPolicies.push(...policies.flat()); } - replaceEntityPolicies(...policies: Array) { - if (this.#replaced) { - throw new Error( - `You've already replaced the entity policies. If you want to add more entity policies, see 'addEntityPolicies'.`, - ); - } - this.#entityPolicies = [...policies]; - this.#replaced = true; + setFieldValidators(validators: Partial): void { + merge(this.#fieldValidators, validators); } get entityPolicies() { return this.#entityPolicies; } - get replaced() { - return this.#replaced; + get fieldValidators() { + return this.#fieldValidators; } } @@ -224,12 +219,8 @@ export const catalogPlugin = createBackendPlugin({ ); builder.addLocationAnalyzers(...analysisExtensions.locationAnalyzers); builder.addPermissionRules(...permissionExtensions.permissionRules); - - if (modelExtensions.replaced) { - builder.replaceEntityPolicies(modelExtensions.entityPolicies); - } else { - builder.addEntityPolicy(...modelExtensions.entityPolicies); - } + builder.addEntityPolicy(...modelExtensions.entityPolicies); + builder.setFieldFormatValidators(modelExtensions.fieldValidators); const { processingEngine, router } = await builder.build(); diff --git a/plugins/catalog-node/src/extensions.ts b/plugins/catalog-node/src/extensions.ts index 7c63e51c69..004a46445e 100644 --- a/plugins/catalog-node/src/extensions.ts +++ b/plugins/catalog-node/src/extensions.ts @@ -15,7 +15,7 @@ */ import { createExtensionPoint } from '@backstage/backend-plugin-api'; -import { Entity, EntityPolicy } from '@backstage/catalog-model'; +import { Entity, EntityPolicy, Validators } from '@backstage/catalog-model'; import { CatalogProcessor, EntitiesSearchFilter, @@ -47,11 +47,22 @@ export interface CatalogProcessingExtensionPoint { /** @alpha */ export interface CatalogModelExtensionPoint { + /** + * @deprecated Use `policyToProcessorTransformer` from `@backstage/plugin-catalog-backend` + * and `CatalogProcessingExtensionPoint.addProcessor` instead. + */ addEntityPolicies( ...policies: Array> ): void; - replaceEntityPolicies(...policies: Array): void; + /** + * Sets the validator function to use for one or more special fields of an + * entity. This is useful if the default rules for formatting of fields are + * not sufficient. + * + * @param validators - The (subset of) validators to set + */ + setFieldValidators(validators: Partial): void; } /** From af751400d679474aae497623075e973f479aa7c2 Mon Sep 17 00:00:00 2001 From: Aramis Date: Fri, 2 Feb 2024 16:42:47 -0500 Subject: [PATCH 10/26] fix api report Signed-off-by: Aramis Signed-off-by: aramissennyeydd --- plugins/catalog-backend/api-report.md | 5 +++++ plugins/catalog-backend/src/modules/core/index.ts | 1 + .../src/modules/core/policyToProcessorTransformer.ts | 3 ++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/plugins/catalog-backend/api-report.md b/plugins/catalog-backend/api-report.md index a38a964d48..e00436846a 100644 --- a/plugins/catalog-backend/api-report.md +++ b/plugins/catalog-backend/api-report.md @@ -421,6 +421,11 @@ export type PlaceholderResolverRead = PlaceholderResolverRead_2; // @public @deprecated (undocumented) export type PlaceholderResolverResolveUrl = PlaceholderResolverResolveUrl_2; +// @public +export function policyToProcessorTransformer( + policy: EntityPolicy, +): CatalogProcessor_2; + // @public export type ProcessingIntervalFunction = () => number; diff --git a/plugins/catalog-backend/src/modules/core/index.ts b/plugins/catalog-backend/src/modules/core/index.ts index 365def0bfe..54f16c99b1 100644 --- a/plugins/catalog-backend/src/modules/core/index.ts +++ b/plugins/catalog-backend/src/modules/core/index.ts @@ -24,3 +24,4 @@ export { PlaceholderProcessor } from './PlaceholderProcessor'; export type { PlaceholderProcessorOptions } from './PlaceholderProcessor'; export { UrlReaderProcessor } from './UrlReaderProcessor'; export { parseEntityYaml } from '../util/parse'; +export { policyToProcessorTransformer } from './policyToProcessorTransformer'; diff --git a/plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.ts b/plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.ts index ba1ce254c7..69b2788779 100644 --- a/plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.ts +++ b/plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.ts @@ -19,8 +19,9 @@ import { CatalogProcessor } from '@backstage/plugin-catalog-node'; /** * Transform a given entity policy to an entity processor. - * @param policy The policy to transform + * @param policy - The policy to transform * @returns A new entity processor that uses the entity policy. + * @public */ export function policyToProcessorTransformer( policy: EntityPolicy, From 9281ab3bb6c94417d1dbc6ece6f1a167e447be44 Mon Sep 17 00:00:00 2001 From: Aramis Date: Fri, 2 Feb 2024 16:53:57 -0500 Subject: [PATCH 11/26] fix api reports Signed-off-by: Aramis Signed-off-by: aramissennyeydd --- plugins/catalog-node/api-report-alpha.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/catalog-node/api-report-alpha.md b/plugins/catalog-node/api-report-alpha.md index 64382e56bb..9243fa98fe 100644 --- a/plugins/catalog-node/api-report-alpha.md +++ b/plugins/catalog-node/api-report-alpha.md @@ -15,6 +15,7 @@ import { PermissionRuleParams } from '@backstage/plugin-permission-common'; import { PlaceholderResolver } from '@backstage/plugin-catalog-node'; import { ScmLocationAnalyzer } from '@backstage/plugin-catalog-node'; import { ServiceRef } from '@backstage/backend-plugin-api'; +import { Validators } from '@backstage/catalog-model'; // @alpha (undocumented) export interface CatalogAnalysisExtensionPoint { @@ -27,12 +28,11 @@ export const catalogAnalysisExtensionPoint: ExtensionPoint> ): void; - // (undocumented) - replaceEntityPolicies(...policies: Array): void; + setFieldValidators(validators: Partial): void; } // @alpha (undocumented) From 28a0882dbbe7c48d92649f3324f3321564109d0a Mon Sep 17 00:00:00 2001 From: Aramis Date: Mon, 12 Feb 2024 08:38:48 -0500 Subject: [PATCH 12/26] rename to transformLegacyPolicyToProcessor Signed-off-by: Aramis Signed-off-by: aramissennyeydd --- plugins/catalog-backend/api-report.md | 10 +++++----- plugins/catalog-backend/src/modules/core/index.ts | 2 +- ...est.ts => transformLegacyPolicyToProcessor.test.ts} | 10 +++++----- ...nsformer.ts => transformLegacyPolicyToProcessor.ts} | 2 +- plugins/catalog-node/src/extensions.ts | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) rename plugins/catalog-backend/src/modules/core/{policyToProcessorTransformer.test.ts => transformLegacyPolicyToProcessor.test.ts} (88%) rename plugins/catalog-backend/src/modules/core/{policyToProcessorTransformer.ts => transformLegacyPolicyToProcessor.ts} (96%) diff --git a/plugins/catalog-backend/api-report.md b/plugins/catalog-backend/api-report.md index e00436846a..92a2fbb280 100644 --- a/plugins/catalog-backend/api-report.md +++ b/plugins/catalog-backend/api-report.md @@ -421,11 +421,6 @@ export type PlaceholderResolverRead = PlaceholderResolverRead_2; // @public @deprecated (undocumented) export type PlaceholderResolverResolveUrl = PlaceholderResolverResolveUrl_2; -// @public -export function policyToProcessorTransformer( - policy: EntityPolicy, -): CatalogProcessor_2; - // @public export type ProcessingIntervalFunction = () => number; @@ -455,6 +450,11 @@ export const processingResult: Readonly<{ // @public @deprecated (undocumented) export type ScmLocationAnalyzer = ScmLocationAnalyzer_2; +// @public +export function transformLegacyPolicyToProcessor( + policy: EntityPolicy, +): CatalogProcessor_2; + // @public (undocumented) export class UrlReaderProcessor implements CatalogProcessor_2 { constructor(options: { reader: UrlReader; logger: Logger }); diff --git a/plugins/catalog-backend/src/modules/core/index.ts b/plugins/catalog-backend/src/modules/core/index.ts index 54f16c99b1..e410c3824d 100644 --- a/plugins/catalog-backend/src/modules/core/index.ts +++ b/plugins/catalog-backend/src/modules/core/index.ts @@ -24,4 +24,4 @@ export { PlaceholderProcessor } from './PlaceholderProcessor'; export type { PlaceholderProcessorOptions } from './PlaceholderProcessor'; export { UrlReaderProcessor } from './UrlReaderProcessor'; export { parseEntityYaml } from '../util/parse'; -export { policyToProcessorTransformer } from './policyToProcessorTransformer'; +export { transformLegacyPolicyToProcessor } from './transformLegacyPolicyToProcessor'; diff --git a/plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.test.ts b/plugins/catalog-backend/src/modules/core/transformLegacyPolicyToProcessor.test.ts similarity index 88% rename from plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.test.ts rename to plugins/catalog-backend/src/modules/core/transformLegacyPolicyToProcessor.test.ts index 46338ce6cb..a38074723c 100644 --- a/plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.test.ts +++ b/plugins/catalog-backend/src/modules/core/transformLegacyPolicyToProcessor.test.ts @@ -15,10 +15,10 @@ */ import { Entity, EntityPolicy } from '@backstage/catalog-model'; -import { policyToProcessorTransformer } from './policyToProcessorTransformer'; +import { transformLegacyPolicyToProcessor } from './transformLegacyPolicyToProcessor'; import { clone } from 'lodash'; -describe('policyToProcessorTransformer', () => { +describe('transformLegacyPolicyToProcessor', () => { const entityToProcess: Entity = { apiVersion: 'backstage.io/v1alpha', kind: 'Component', @@ -33,7 +33,7 @@ describe('policyToProcessorTransformer', () => { return entity; }, }; - const processor = policyToProcessorTransformer(policy); + const processor = transformLegacyPolicyToProcessor(policy); const clonedEntity = clone(entityToProcess); const entity = await processor.preProcessEntity?.( clonedEntity, @@ -54,7 +54,7 @@ describe('policyToProcessorTransformer', () => { return undefined; }, }; - const processor = policyToProcessorTransformer(policy); + const processor = transformLegacyPolicyToProcessor(policy); const clonedEntity = clone(entityToProcess); const entity = await processor.preProcessEntity?.( clonedEntity, @@ -75,7 +75,7 @@ describe('policyToProcessorTransformer', () => { throw new TypeError('Invalid value for metadata.name'); }, }; - const processor = policyToProcessorTransformer(policy); + const processor = transformLegacyPolicyToProcessor(policy); const clonedEntity = clone(entityToProcess); await expect( processor.preProcessEntity?.( diff --git a/plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.ts b/plugins/catalog-backend/src/modules/core/transformLegacyPolicyToProcessor.ts similarity index 96% rename from plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.ts rename to plugins/catalog-backend/src/modules/core/transformLegacyPolicyToProcessor.ts index 69b2788779..40a3e749ab 100644 --- a/plugins/catalog-backend/src/modules/core/policyToProcessorTransformer.ts +++ b/plugins/catalog-backend/src/modules/core/transformLegacyPolicyToProcessor.ts @@ -23,7 +23,7 @@ import { CatalogProcessor } from '@backstage/plugin-catalog-node'; * @returns A new entity processor that uses the entity policy. * @public */ -export function policyToProcessorTransformer( +export function transformLegacyPolicyToProcessor( policy: EntityPolicy, ): CatalogProcessor { return { diff --git a/plugins/catalog-node/src/extensions.ts b/plugins/catalog-node/src/extensions.ts index 004a46445e..df1b8b0d57 100644 --- a/plugins/catalog-node/src/extensions.ts +++ b/plugins/catalog-node/src/extensions.ts @@ -48,7 +48,7 @@ export interface CatalogProcessingExtensionPoint { /** @alpha */ export interface CatalogModelExtensionPoint { /** - * @deprecated Use `policyToProcessorTransformer` from `@backstage/plugin-catalog-backend` + * @deprecated Use `transformLegacyPolicyToProcessor` from `@backstage/plugin-catalog-backend` * and `CatalogProcessingExtensionPoint.addProcessor` instead. */ addEntityPolicies( From 0eda4bc6299a2110d2b32016a1d5326a4f2d5c7f Mon Sep 17 00:00:00 2001 From: Aramis Date: Mon, 12 Feb 2024 08:50:00 -0500 Subject: [PATCH 13/26] remove support for adding entity policies entirely Signed-off-by: Aramis --- .changeset/thirty-bags-try.md | 29 ++++++++++++++++++- .../src/service/CatalogPlugin.ts | 10 ------- plugins/catalog-node/src/extensions.ts | 10 +------ 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/.changeset/thirty-bags-try.md b/.changeset/thirty-bags-try.md index 7a8156e44c..f04d0160bf 100644 --- a/.changeset/thirty-bags-try.md +++ b/.changeset/thirty-bags-try.md @@ -2,4 +2,31 @@ '@backstage/plugin-catalog-backend': minor --- -Add support for defining entity policies in the new backend. +Adds support for supplying field validators to the new backend's catalog plugin. If you're using entity policies, you should use the new `transformLegacyPolicyToProcessor` function to install them as processors instead. + +```ts +import { + catalogProcessingExtensionPoint, + catalogModelExtensionPoint, +} from '@backstage/plugin-catalog-node/alpha'; +import {myPolicy} from './my-policy'; + +export const catalogModulePolicyProvider = createBackendModule({ + pluginId: 'catalog', + moduleId: 'internal-policy-provider', + register(reg) { + reg.registerInit({ + deps: { + modelExtensions: catalogModelExtensionPoint, + processingExtensions: catalogProcessingExtensionPoint, + }, + async init({ modelExtensions, processingExtensions }) { + modelExtensions.setFieldValidators({ + ... + }); + processingExtensions.addProcessors(transformLegacyPolicyToProcessor(myPolicy)) + }, + }); + }, +}); +``` diff --git a/plugins/catalog-backend/src/service/CatalogPlugin.ts b/plugins/catalog-backend/src/service/CatalogPlugin.ts index 1fca11e482..114dbe6f61 100644 --- a/plugins/catalog-backend/src/service/CatalogPlugin.ts +++ b/plugins/catalog-backend/src/service/CatalogPlugin.ts @@ -128,21 +128,12 @@ class CatalogPermissionExtensionPointImpl } class CatalogModelExtensionPointImpl implements CatalogModelExtensionPoint { - #entityPolicies = new Array(); #fieldValidators: Partial = {}; - addEntityPolicies(...policies: Array>) { - this.#entityPolicies.push(...policies.flat()); - } - setFieldValidators(validators: Partial): void { merge(this.#fieldValidators, validators); } - get entityPolicies() { - return this.#entityPolicies; - } - get fieldValidators() { return this.#fieldValidators; } @@ -219,7 +210,6 @@ export const catalogPlugin = createBackendPlugin({ ); builder.addLocationAnalyzers(...analysisExtensions.locationAnalyzers); builder.addPermissionRules(...permissionExtensions.permissionRules); - builder.addEntityPolicy(...modelExtensions.entityPolicies); builder.setFieldFormatValidators(modelExtensions.fieldValidators); const { processingEngine, router } = await builder.build(); diff --git a/plugins/catalog-node/src/extensions.ts b/plugins/catalog-node/src/extensions.ts index df1b8b0d57..763e934176 100644 --- a/plugins/catalog-node/src/extensions.ts +++ b/plugins/catalog-node/src/extensions.ts @@ -15,7 +15,7 @@ */ import { createExtensionPoint } from '@backstage/backend-plugin-api'; -import { Entity, EntityPolicy, Validators } from '@backstage/catalog-model'; +import { Entity, Validators } from '@backstage/catalog-model'; import { CatalogProcessor, EntitiesSearchFilter, @@ -47,14 +47,6 @@ export interface CatalogProcessingExtensionPoint { /** @alpha */ export interface CatalogModelExtensionPoint { - /** - * @deprecated Use `transformLegacyPolicyToProcessor` from `@backstage/plugin-catalog-backend` - * and `CatalogProcessingExtensionPoint.addProcessor` instead. - */ - addEntityPolicies( - ...policies: Array> - ): void; - /** * Sets the validator function to use for one or more special fields of an * entity. This is useful if the default rules for formatting of fields are From 3d1954f80dae8f4ae327ae0813177dc0363731f0 Mon Sep 17 00:00:00 2001 From: Aramis Date: Mon, 12 Feb 2024 09:34:33 -0500 Subject: [PATCH 14/26] fix api report Signed-off-by: Aramis Signed-off-by: aramissennyeydd --- plugins/catalog-node/api-report-alpha.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/plugins/catalog-node/api-report-alpha.md b/plugins/catalog-node/api-report-alpha.md index 9243fa98fe..e43d1bd49f 100644 --- a/plugins/catalog-node/api-report-alpha.md +++ b/plugins/catalog-node/api-report-alpha.md @@ -7,7 +7,6 @@ import { CatalogApi } from '@backstage/catalog-client'; import { CatalogProcessor } from '@backstage/plugin-catalog-node'; import { EntitiesSearchFilter } from '@backstage/plugin-catalog-node'; import { Entity } from '@backstage/catalog-model'; -import { EntityPolicy } from '@backstage/catalog-model'; import { EntityProvider } from '@backstage/plugin-catalog-node'; import { ExtensionPoint } from '@backstage/backend-plugin-api'; import { PermissionRule } from '@backstage/plugin-permission-node'; @@ -28,10 +27,6 @@ export const catalogAnalysisExtensionPoint: ExtensionPoint> - ): void; setFieldValidators(validators: Partial): void; } From 5182f5bfdde84756b3d87de0765525ce8d2689c2 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Fri, 16 Feb 2024 19:51:51 -0500 Subject: [PATCH 15/26] merge fixes and update changeset Signed-off-by: aramissennyeydd --- .changeset/thirty-bags-try.md | 1 + plugins/catalog-backend/src/service/CatalogPlugin.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.changeset/thirty-bags-try.md b/.changeset/thirty-bags-try.md index f04d0160bf..98d6c5620b 100644 --- a/.changeset/thirty-bags-try.md +++ b/.changeset/thirty-bags-try.md @@ -1,5 +1,6 @@ --- '@backstage/plugin-catalog-backend': minor +'@backstage/plugin-catalog-node': minor --- Adds support for supplying field validators to the new backend's catalog plugin. If you're using entity policies, you should use the new `transformLegacyPolicyToProcessor` function to install them as processors instead. diff --git a/plugins/catalog-backend/src/service/CatalogPlugin.ts b/plugins/catalog-backend/src/service/CatalogPlugin.ts index 114dbe6f61..255df84d58 100644 --- a/plugins/catalog-backend/src/service/CatalogPlugin.ts +++ b/plugins/catalog-backend/src/service/CatalogPlugin.ts @@ -17,7 +17,7 @@ import { createBackendPlugin, coreServices, } from '@backstage/backend-plugin-api'; -import { Entity, EntityPolicy, Validators } from '@backstage/catalog-model'; +import { Entity, Validators } from '@backstage/catalog-model'; import { CatalogBuilder, CatalogPermissionRuleInput } from './CatalogBuilder'; import { CatalogAnalysisExtensionPoint, From 1771120a72c118f0fea7a76926bd392464d4e0d3 Mon Sep 17 00:00:00 2001 From: JinoArch <39610834+JinoArch@users.noreply.github.com> Date: Sat, 17 Feb 2024 17:59:25 -0500 Subject: [PATCH 16/26] fix: update field props and field validation library Signed-off-by: JinoArch <39610834+JinoArch@users.noreply.github.com> --- .../software-templates/writing-custom-field-extensions.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/features/software-templates/writing-custom-field-extensions.md b/docs/features/software-templates/writing-custom-field-extensions.md index f22cecd22c..d53b8bdf2f 100644 --- a/docs/features/software-templates/writing-custom-field-extensions.md +++ b/docs/features/software-templates/writing-custom-field-extensions.md @@ -30,7 +30,8 @@ As an example, we will create a component that validates whether a string is in ```tsx //packages/app/src/scaffolder/ValidateKebabCase/ValidateKebabCaseExtension.tsx import React from 'react'; -import { FieldProps, FieldValidation } from '@rjsf/core'; +import { FieldExtensionComponentProps } from '@backstage/plugin-scaffolder-react'; +import type { FieldValidation } from '@rjsf/utils'; import FormControl from '@material-ui/core/FormControl'; /* This is the actual component that will get rendered in the form @@ -40,7 +41,7 @@ export const ValidateKebabCase = ({ rawErrors, required, formData, -}: FieldProps) => { +}: FieldExtensionComponentProps) => { return ( Date: Mon, 19 Feb 2024 10:55:40 +0100 Subject: [PATCH 17/26] fix(catalog-backend-module-azure): Use slash in branch name for tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ragnar Halldórsson --- .../src/lib/azure.test.ts | 4 ++-- .../AzureDevOpsDiscoveryProcessor.test.ts | 23 +++++++++++++++---- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/plugins/catalog-backend-module-azure/src/lib/azure.test.ts b/plugins/catalog-backend-module-azure/src/lib/azure.test.ts index 21fc77e7f7..ef00610dc0 100644 --- a/plugins/catalog-backend-module-azure/src/lib/azure.test.ts +++ b/plugins/catalog-backend-module-azure/src/lib/azure.test.ts @@ -251,7 +251,7 @@ describe('azure', () => { $skip: 0, $top: 1000, filters: { - Branch: ['development'], + Branch: ['topic/catalog-info'], }, }); return res(ctx.json(response)); @@ -272,7 +272,7 @@ describe('azure', () => { 'engineering', 'backstage', '/catalog-info.yaml', - 'development', + 'topic/catalog-info', ), ).resolves.toEqual(response.results); }); diff --git a/plugins/catalog-backend-module-azure/src/processors/AzureDevOpsDiscoveryProcessor.test.ts b/plugins/catalog-backend-module-azure/src/processors/AzureDevOpsDiscoveryProcessor.test.ts index 0817465d83..6df555f190 100644 --- a/plugins/catalog-backend-module-azure/src/processors/AzureDevOpsDiscoveryProcessor.test.ts +++ b/plugins/catalog-backend-module-azure/src/processors/AzureDevOpsDiscoveryProcessor.test.ts @@ -66,7 +66,7 @@ describe('AzureDevOpsDiscoveryProcessor', () => { expect( parseUrl( - 'https://azuredevops.mycompany.com/spotify/engineering/_git/backstage?path=/src/*/catalog.yaml&version=GBdevelopment', + 'https://azuredevops.mycompany.com/spotify/engineering/_git/backstage?path=/src/*/catalog.yaml&version=GBtopic/catalog-info', ), ).toEqual({ baseUrl: 'https://azuredevops.mycompany.com', @@ -74,7 +74,20 @@ describe('AzureDevOpsDiscoveryProcessor', () => { project: 'engineering', repo: 'backstage', catalogPath: '/src/*/catalog.yaml', - branch: 'development', + branch: 'topic/catalog-info', + }); + + expect( + parseUrl( + 'https://azuredevops.mycompany.com/spotify/engineering/_git/backstage?path=/src/*/catalog.yaml&version=GBtopic%2Fcatalog-info', + ), + ).toEqual({ + baseUrl: 'https://azuredevops.mycompany.com', + org: 'spotify', + project: 'engineering', + repo: 'backstage', + catalogPath: '/src/*/catalog.yaml', + branch: 'topic/catalog-info', }); }); @@ -249,7 +262,7 @@ describe('AzureDevOpsDiscoveryProcessor', () => { const location: LocationSpec = { type: 'azure-discovery', target: - 'https://dev.azure.com/shopify/engineering/_git/backstage?path=/catalog-info.yaml&version=GBdevelopment', + 'https://dev.azure.com/shopify/engineering/_git/backstage?path=/catalog-info.yaml&version=GBtopic/catalog-info', }; mockCodeSearch.mockResolvedValueOnce([ { @@ -274,7 +287,7 @@ describe('AzureDevOpsDiscoveryProcessor', () => { 'engineering', 'backstage', '/catalog-info.yaml', - 'development', + 'topic/catalog-info', ); expect(emitter).toHaveBeenCalledTimes(1); expect(emitter).toHaveBeenCalledWith({ @@ -282,7 +295,7 @@ describe('AzureDevOpsDiscoveryProcessor', () => { location: { type: 'url', target: - 'https://dev.azure.com/shopify/engineering/_git/backstage?path=/catalog-info.yaml&version=GBdevelopment', + 'https://dev.azure.com/shopify/engineering/_git/backstage?path=/catalog-info.yaml&version=GBtopic/catalog-info', presence: 'optional', }, }); From 49c759e1c1d020a4ebbb74f7aa12fedee882d653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ragnar=20Halld=C3=B3rsson?= Date: Mon, 19 Feb 2024 11:01:28 +0100 Subject: [PATCH 18/26] chore(catalog-backend-module-azure): Apply suggestion from code review and improve some wording in README MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ragnar Halldórsson --- docs/integrations/azure/discovery.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/integrations/azure/discovery.md b/docs/integrations/azure/discovery.md index ad454c0ed1..7fbba5228d 100644 --- a/docs/integrations/azure/discovery.md +++ b/docs/integrations/azure/discovery.md @@ -93,6 +93,8 @@ _Note:_ 5. In the window that appears, enter the name of the branch you want to add and click "Add". 6. The added branch will now appear in the "Searchable branches" list. +It may take some time before the branch is indexed and searchable. + As this provider is not one of the default providers, you will first need to install the Azure catalog plugin: @@ -162,12 +164,12 @@ catalog: target: https://dev.azure.com/myorg/myproject/_git/*?path=/src/*/catalog-info.yaml # And optionally provide a specific branch name using the version parameter - type: azure-discovery - target: https://dev.azure.com/myorg/myproject/_git/*?path=/catalog-info.yaml&version=GBdevelopment + target: https://dev.azure.com/myorg/myproject/_git/*?path=/catalog-info.yaml&version=GBtopic/catalog-info ``` Note the `azure-discovery` type, as this is not a regular `url` processor. -When using a custom pattern, the target is composed of six parts: +When using a custom pattern, the target is composed of these parts: - The base instance URL, `https://dev.azure.com` in this case - The organization name which is required, `myorg` in this case @@ -178,4 +180,4 @@ When using a custom pattern, the target is composed of six parts: - The path within each repository to find the catalog YAML file. This will usually be `/catalog-info.yaml`, `/src/*/catalog-info.yaml` or a similar variation for catalog files stored in the root directory of each repository. -- The repository branch to scan which is optional, `development` in this case. The `GB` prefix is mandatory, as this is how Azure DevOps identifies the version as a branch. If omitted, the repo's default branch will be scanned. +- The repository branch to scan which is optional, `topic/catalog-info` in this case. If omitted, the repo's default branch will be scanned. The `GB` prefix is required, as this is how Azure DevOps identifies the version as a branch. From 922b80d77cb3f6ae2981ee5df65f7befc54a909a Mon Sep 17 00:00:00 2001 From: cmoulliard Date: Mon, 19 Feb 2024 12:04:11 +0100 Subject: [PATCH 19/26] Add the missing break within the while statement to exit when the gitea repository exists. #23049 Signed-off-by: cmoulliard --- .../templates/quarkus-application/skeletons/hello-world/pom.xml | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 qshift/templates/quarkus-application/skeletons/hello-world/pom.xml diff --git a/qshift/templates/quarkus-application/skeletons/hello-world/pom.xml b/qshift/templates/quarkus-application/skeletons/hello-world/pom.xml new file mode 100644 index 0000000000..e69de29bb2 From ef0f44e423c89d721d9b5c22e3b085810ce8572e Mon Sep 17 00:00:00 2001 From: cmoulliard Date: Mon, 19 Feb 2024 12:11:02 +0100 Subject: [PATCH 20/26] Add change set and code change Signed-off-by: cmoulliard --- .changeset/weak-flowers-repeat.md | 5 +++++ plugins/scaffolder-backend-module-gitea/src/actions/gitea.ts | 3 +++ 2 files changed, 8 insertions(+) create mode 100644 .changeset/weak-flowers-repeat.md diff --git a/.changeset/weak-flowers-repeat.md b/.changeset/weak-flowers-repeat.md new file mode 100644 index 0000000000..4d0d4ce2b7 --- /dev/null +++ b/.changeset/weak-flowers-repeat.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend-module-gitea': patch +--- + +Add missing break within the while statement to exit from the while maxDuration statement when the gitea repository has been created and exists diff --git a/plugins/scaffolder-backend-module-gitea/src/actions/gitea.ts b/plugins/scaffolder-backend-module-gitea/src/actions/gitea.ts index e7f62f51da..875e6188a9 100644 --- a/plugins/scaffolder-backend-module-gitea/src/actions/gitea.ts +++ b/plugins/scaffolder-backend-module-gitea/src/actions/gitea.ts @@ -178,6 +178,9 @@ async function checkAvailabilityGiteaRepository( if (response.status !== 200) { // Repository is not yet available/accessible ... await sleep(1000); + } else { + // Gitea repository exists ! + break; } } } From 40de43d14d1295ff99e63cb0632c9eb8c7cd299e Mon Sep 17 00:00:00 2001 From: cmoulliard Date: Mon, 19 Feb 2024 12:50:14 +0100 Subject: [PATCH 21/26] Changed the wording to warn the user to get a certificate when using gitea locally and clean the template example Signed-off-by: cmoulliard --- .../scaffolder-backend-module-gitea/README.md | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/plugins/scaffolder-backend-module-gitea/README.md b/plugins/scaffolder-backend-module-gitea/README.md index f13602f2d8..a1282634e0 100644 --- a/plugins/scaffolder-backend-module-gitea/README.md +++ b/plugins/scaffolder-backend-module-gitea/README.md @@ -28,8 +28,8 @@ integrations: password: '' ``` -**NOTE**: As backstage will issue HTTPS/TLS requests to the gitea instance, it is needed to configure `gitea` with a valid certificate or at least with a -self-signed certificate `gitea cert --host localhost -ca`. Don't forget to set the env var `NODE_EXTRA_CA_CERTS` to point to the CA file before launching backstage ! +**Important**: As backstage will issue HTTPS/TLS requests to the gitea instance, it is needed to configure `gitea` with a valid certificate or at least with a +self-signed certificate `gitea cert --host localhost -ca` trusted by a CA authority. Don't forget to set the env var `NODE_EXTRA_CA_CERTS` to point to the CA file before launching backstage ! When done, you can create a template which: @@ -69,18 +69,8 @@ spec: action: fetch:template input: url: ./skeleton - copyWithoutTemplating: - - .github/workflows/* values: - component_id: ${{ parameters.component_id }} - namespace: ${{ parameters.component_id }}-dev - description: ${{ parameters.description }} - group_id: ${{ parameters.group_id }} - artifact_id: ${{ parameters.artifact_id }} - java_package_name: ${{ parameters.java_package_name }} - owner: ${{ parameters.owner }} - destination: ${{ (parameters.repoUrl | parseRepoUrl).owner }}/${{ (parameters.repoUrl | parseRepoUrl).repo } - port: 8080 + name: ${{ parameters.name }} - id: publish name: Publishing to a gitea git repository From 7ae570490302b0b6a748b8ef174aa6b9cdfa8902 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 19 Feb 2024 13:07:43 +0100 Subject: [PATCH 22/26] backend-app-api: filter internal errors Signed-off-by: Patrik Oldsberg --- .changeset/heavy-cameras-provide.md | 5 ++ .../src/http/MiddlewareFactory.test.ts | 30 ++++++++++ .../src/http/MiddlewareFactory.ts | 10 +++- .../src/http/applyInternalErrorFilter.ts | 56 +++++++++++++++++++ 4 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 .changeset/heavy-cameras-provide.md create mode 100644 packages/backend-app-api/src/http/applyInternalErrorFilter.ts diff --git a/.changeset/heavy-cameras-provide.md b/.changeset/heavy-cameras-provide.md new file mode 100644 index 0000000000..1d86f20592 --- /dev/null +++ b/.changeset/heavy-cameras-provide.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +Updated the default error handling middleware to filter out certain known error types that should never be returned in responses. The errors are instead logged along with a correlation ID, which is also returned in the response. Initially only PostgreSQL protocol errors from the `pg-protocol` package are filtered out. diff --git a/packages/backend-app-api/src/http/MiddlewareFactory.test.ts b/packages/backend-app-api/src/http/MiddlewareFactory.test.ts index 5dbb63ec59..f0506f11de 100644 --- a/packages/backend-app-api/src/http/MiddlewareFactory.test.ts +++ b/packages/backend-app-api/src/http/MiddlewareFactory.test.ts @@ -192,6 +192,36 @@ describe('MiddlewareFactory', () => { ); }); + it('should filter out internal errors', async () => { + const app = express(); + + class DatabaseError extends Error {} + const thrownError = new DatabaseError('some error'); + + app.use('/breaks', () => { + throw thrownError; + }); + app.use(middleware.error()); + + await request(app).get('/breaks'); + + expect(childLogger.error).toHaveBeenCalledTimes(2); + expect(childLogger.error).toHaveBeenCalledWith( + 'Request failed with status 500', + expect.objectContaining({ + message: expect.stringMatching( + /^An internal error occurred logId=[0-9a-f]+$/, + ), + }), + ); + expect(childLogger.error).toHaveBeenCalledWith( + expect.stringMatching( + /^Filtered internal error with logId=[0-9a-f]+ from response$/, + ), + thrownError, + ); + }); + it('does not log 400 errors', async () => { const app = express(); diff --git a/packages/backend-app-api/src/http/MiddlewareFactory.ts b/packages/backend-app-api/src/http/MiddlewareFactory.ts index bd78d77045..77136fd7e5 100644 --- a/packages/backend-app-api/src/http/MiddlewareFactory.ts +++ b/packages/backend-app-api/src/http/MiddlewareFactory.ts @@ -43,6 +43,7 @@ import { serializeError, } from '@backstage/errors'; import { NotImplementedError } from '@backstage/errors'; +import { applyInternalErrorFilter } from './applyInternalErrorFilter'; /** * Options used to create a {@link MiddlewareFactory}. @@ -209,7 +210,14 @@ export class MiddlewareFactory { type: 'errorHandler', }); - return (error: Error, req: Request, res: Response, next: NextFunction) => { + return ( + rawError: Error, + req: Request, + res: Response, + next: NextFunction, + ) => { + const error = applyInternalErrorFilter(rawError, logger); + const statusCode = getStatusCode(error); if (options.logAllErrors || statusCode >= 500) { logger.error(`Request failed with status ${statusCode}`, error); diff --git a/packages/backend-app-api/src/http/applyInternalErrorFilter.ts b/packages/backend-app-api/src/http/applyInternalErrorFilter.ts new file mode 100644 index 0000000000..e2e2f60e73 --- /dev/null +++ b/packages/backend-app-api/src/http/applyInternalErrorFilter.ts @@ -0,0 +1,56 @@ +/* + * Copyright 2024 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 { LoggerService } from '@backstage/backend-plugin-api'; +import { assertError } from '@backstage/errors'; +import { randomBytes } from 'crypto'; + +function handleBadError(error: Error, logger: LoggerService) { + const logId = randomBytes(10).toString('hex'); + logger.error( + `Filtered internal error with logId=${logId} from response`, + error, + ); + const newError = new Error(`An internal error occurred logId=${logId}`); + delete newError.stack; // Trim the stack since it's not particularly useful + return newError; +} + +/** + * Filters out certain known error types that should never be returned in responses. + * + * @internal + */ +export function applyInternalErrorFilter( + error: unknown, + logger: LoggerService, +): Error { + try { + assertError(error); + } catch (assertionError: unknown) { + assertError(assertionError); + return handleBadError(assertionError, logger); + } + + const constructorName = error.constructor.name; + + // DatabaseError are thrown by the pg-protocol module + if (constructorName === 'DatabaseError') { + return handleBadError(error, logger); + } + + return error; +} From 3fffd8a2cbcd3ea45f1ec2ed7ebb6ed13f695a0b Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 19 Feb 2024 13:24:48 +0100 Subject: [PATCH 23/26] backend-app-api: include logId in filtered error log Signed-off-by: Patrik Oldsberg --- .../src/http/MiddlewareFactory.test.ts | 15 +++++++++++---- .../src/http/applyInternalErrorFilter.ts | 7 +++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/backend-app-api/src/http/MiddlewareFactory.test.ts b/packages/backend-app-api/src/http/MiddlewareFactory.test.ts index f0506f11de..418d170365 100644 --- a/packages/backend-app-api/src/http/MiddlewareFactory.test.ts +++ b/packages/backend-app-api/src/http/MiddlewareFactory.test.ts @@ -195,6 +195,11 @@ describe('MiddlewareFactory', () => { it('should filter out internal errors', async () => { const app = express(); + const grandChildLogger = { + error: jest.fn(), + }; + childLogger.child.mockReturnValue(grandChildLogger); + class DatabaseError extends Error {} const thrownError = new DatabaseError('some error'); @@ -205,18 +210,20 @@ describe('MiddlewareFactory', () => { await request(app).get('/breaks'); - expect(childLogger.error).toHaveBeenCalledTimes(2); + const [{ logId }] = childLogger.child.mock.calls[0]; + + expect(logId).toMatch(/^[0-9a-f]+$/); expect(childLogger.error).toHaveBeenCalledWith( 'Request failed with status 500', expect.objectContaining({ message: expect.stringMatching( - /^An internal error occurred logId=[0-9a-f]+$/, + `An internal error occurred logId=${logId}`, ), }), ); - expect(childLogger.error).toHaveBeenCalledWith( + expect(grandChildLogger.error).toHaveBeenCalledWith( expect.stringMatching( - /^Filtered internal error with logId=[0-9a-f]+ from response$/, + `Filtered internal error with logId=${logId} from response`, ), thrownError, ); diff --git a/packages/backend-app-api/src/http/applyInternalErrorFilter.ts b/packages/backend-app-api/src/http/applyInternalErrorFilter.ts index e2e2f60e73..d1d1e0e4d9 100644 --- a/packages/backend-app-api/src/http/applyInternalErrorFilter.ts +++ b/packages/backend-app-api/src/http/applyInternalErrorFilter.ts @@ -20,10 +20,9 @@ import { randomBytes } from 'crypto'; function handleBadError(error: Error, logger: LoggerService) { const logId = randomBytes(10).toString('hex'); - logger.error( - `Filtered internal error with logId=${logId} from response`, - error, - ); + logger + .child({ logId }) + .error(`Filtered internal error with logId=${logId} from response`, error); const newError = new Error(`An internal error occurred logId=${logId}`); delete newError.stack; // Trim the stack since it's not particularly useful return newError; From 72ab2dcd58b9883113722e6ec893db9450acef93 Mon Sep 17 00:00:00 2001 From: cmoulliard Date: Mon, 19 Feb 2024 14:13:34 +0100 Subject: [PATCH 24/26] Reviewing the change wording. Add the root cause of the error of: checkGiteaOrg to allow to know if there is a certificate error Signed-off-by: cmoulliard --- .changeset/weak-flowers-repeat.md | 3 ++- plugins/scaffolder-backend-module-gitea/src/actions/gitea.ts | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.changeset/weak-flowers-repeat.md b/.changeset/weak-flowers-repeat.md index 4d0d4ce2b7..ed71acd332 100644 --- a/.changeset/weak-flowers-repeat.md +++ b/.changeset/weak-flowers-repeat.md @@ -2,4 +2,5 @@ '@backstage/plugin-scaffolder-backend-module-gitea': patch --- -Add missing break within the while statement to exit from the while maxDuration statement when the gitea repository has been created and exists +Fix issue for infinite loop when repository already exists +Log the root cause of error reported by checkGiteaOrg diff --git a/plugins/scaffolder-backend-module-gitea/src/actions/gitea.ts b/plugins/scaffolder-backend-module-gitea/src/actions/gitea.ts index 875e6188a9..089f9b2669 100644 --- a/plugins/scaffolder-backend-module-gitea/src/actions/gitea.ts +++ b/plugins/scaffolder-backend-module-gitea/src/actions/gitea.ts @@ -80,7 +80,9 @@ const checkGiteaOrg = async ( getOptions, ); } catch (e) { - throw new Error(`Unable to get the Organization: ${owner}, ${e}`); + throw new Error( + `Unable to get the Organization: ${owner}; Error cause: ${e.cause.message}`, + ); } if (response.status !== 200) { throw new Error( From be0202bc8d850cc27bca7b5c9bbd6b98c8d5e630 Mon Sep 17 00:00:00 2001 From: cmoulliard Date: Mon, 19 Feb 2024 16:39:48 +0100 Subject: [PATCH 25/26] Remove file pushed BUT non needed Signed-off-by: cmoulliard --- .../templates/quarkus-application/skeletons/hello-world/pom.xml | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 qshift/templates/quarkus-application/skeletons/hello-world/pom.xml diff --git a/qshift/templates/quarkus-application/skeletons/hello-world/pom.xml b/qshift/templates/quarkus-application/skeletons/hello-world/pom.xml deleted file mode 100644 index e69de29bb2..0000000000 From 6a670a4e167c5811f993239c6d99df94a05c9b36 Mon Sep 17 00:00:00 2001 From: Charles Moulliard Date: Mon, 19 Feb 2024 16:40:36 +0100 Subject: [PATCH 26/26] Update .changeset/weak-flowers-repeat.md Co-authored-by: Ben Lambert Signed-off-by: Charles Moulliard --- .changeset/weak-flowers-repeat.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/weak-flowers-repeat.md b/.changeset/weak-flowers-repeat.md index ed71acd332..d1705343bc 100644 --- a/.changeset/weak-flowers-repeat.md +++ b/.changeset/weak-flowers-repeat.md @@ -2,5 +2,5 @@ '@backstage/plugin-scaffolder-backend-module-gitea': patch --- -Fix issue for infinite loop when repository already exists -Log the root cause of error reported by checkGiteaOrg +- Fix issue for infinite loop when repository already exists +- Log the root cause of error reported by `checkGiteaOrg`