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 1/6] 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 2/6] 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 3/6] 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 4/6] 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 6d1c0b663bf09cf34672afaf652785938cb280b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ragnar=20Halld=C3=B3rsson?= Date: Mon, 19 Feb 2024 10:55:40 +0100 Subject: [PATCH 5/6] 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 6/6] 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.