From 4d62dc15bb64b08682a55d7901780e67d942df30 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Thu, 26 Aug 2021 22:33:57 +0100 Subject: [PATCH 1/5] gh processor passes over repos without default branch Signed-off-by: Brian Fletcher --- .changeset/cuddly-impalas-wave.md | 5 +++++ .../ingestion/processors/GithubDiscoveryProcessor.ts | 12 +++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) create mode 100644 .changeset/cuddly-impalas-wave.md diff --git a/.changeset/cuddly-impalas-wave.md b/.changeset/cuddly-impalas-wave.md new file mode 100644 index 0000000000..1199ef1482 --- /dev/null +++ b/.changeset/cuddly-impalas-wave.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +GitHub discovery processor passes over repositories that do not have a default branch diff --git a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts index 567f6536d0..8bed5b4858 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts @@ -108,9 +108,15 @@ export class GithubDiscoveryProcessor implements CatalogProcessor { ); for (const repository of matching) { - const path = `/blob/${ - branch === '-' ? repository.defaultBranchRef.name : branch - }${catalogPath}`; + const branchName = branch === '-' ? repository.defaultBranchRef?.name : branch; + + if (!branchName) { + this.logger.info(`the repository ${repository.url} does not have a default branch, skipping`); + continue; + } + + const path = `/blob/${branchName}${catalogPath}`; + emit( results.location( { From 7ad2b4450af42bfa9994c7f510a7c591ef2a224d Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Thu, 26 Aug 2021 22:52:31 +0100 Subject: [PATCH 2/5] add a test for undefined default branch Signed-off-by: Brian Fletcher --- .../processors/GithubDiscoveryProcessor.test.ts | 6 ++++++ .../processors/GithubDiscoveryProcessor.ts | 13 ++++++++----- .../src/ingestion/processors/github/github.ts | 2 +- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts index f8647cb5d2..4d16b824a3 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts @@ -202,6 +202,12 @@ describe('GithubDiscoveryProcessor', () => { name: 'main', }, }, + { + name: 'techdocs-durp', + url: 'https://github.com/backstage/techdocs-durp', + isArchived: false, + defaultBranchRef: undefined, + }, ], }); const emitter = jest.fn(); diff --git a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts index 8bed5b4858..b104d87542 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.ts @@ -108,15 +108,18 @@ export class GithubDiscoveryProcessor implements CatalogProcessor { ); for (const repository of matching) { - const branchName = branch === '-' ? repository.defaultBranchRef?.name : branch; - + const branchName = + branch === '-' ? repository.defaultBranchRef?.name : branch; + if (!branchName) { - this.logger.info(`the repository ${repository.url} does not have a default branch, skipping`); + this.logger.info( + `the repository ${repository.url} does not have a default branch, skipping`, + ); continue; } - + const path = `/blob/${branchName}${catalogPath}`; - + emit( results.location( { diff --git a/plugins/catalog-backend/src/ingestion/processors/github/github.ts b/plugins/catalog-backend/src/ingestion/processors/github/github.ts index 9eb5073710..5ff3856f97 100644 --- a/plugins/catalog-backend/src/ingestion/processors/github/github.ts +++ b/plugins/catalog-backend/src/ingestion/processors/github/github.ts @@ -60,7 +60,7 @@ export type Repository = { name: string; url: string; isArchived: boolean; - defaultBranchRef: { + defaultBranchRef?: { name: string; }; }; From 977d67d656ffde8761609b3735b13f357db17837 Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 27 Aug 2021 07:47:47 +0100 Subject: [PATCH 3/5] changes default branch ref type to optionally null Also adds a couple of tests for the default branch feature. Signed-off-by: Brian Fletcher --- .../GithubDiscoveryProcessor.test.ts | 65 ++++++++++++++++++- .../src/ingestion/processors/github/github.ts | 4 +- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts index 4d16b824a3..8fb79bb20d 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts @@ -170,6 +170,68 @@ describe('GithubDiscoveryProcessor', () => { }); }); + it('output repositories with wildcards default branch option', async () => { + const location: LocationSpec = { + type: 'github-discovery', + target: 'https://github.com/backstage/techdocs-*/blob/-/catalog.yaml', + }; + mockGetOrganizationRepositories.mockResolvedValueOnce({ + repositories: [ + { + name: 'backstage', + url: 'https://github.com/backstage/backstage', + isArchived: false, + defaultBranchRef: { + name: 'master', + }, + }, + ], + }); + const emitter = jest.fn(); + + await processor.readLocation(location, false, emitter); + + expect(emitter).toHaveBeenCalledWith({ + type: 'location', + location: { + type: 'url', + target: 'https://github.com/backstage/blob/master/catalog.yaml', + }, + optional: true, + }); + }); + + it('output repositories with wildcards default branch option without catalog-info patch or branch match', async () => { + const location: LocationSpec = { + type: 'github-discovery', + target: 'https://github.com/backstage', + }; + mockGetOrganizationRepositories.mockResolvedValueOnce({ + repositories: [ + { + name: 'backstage', + url: 'https://github.com/backstage/backstage', + isArchived: false, + defaultBranchRef: { + name: 'master', + }, + }, + ], + }); + const emitter = jest.fn(); + + await processor.readLocation(location, false, emitter); + + expect(emitter).toHaveBeenCalledWith({ + type: 'location', + location: { + type: 'url', + target: 'https://github.com/backstage/blob/master/catalog-info.yaml', + }, + optional: true, + }); + }); + it('output repositories with wildcards', async () => { const location: LocationSpec = { type: 'github-discovery', @@ -206,7 +268,7 @@ describe('GithubDiscoveryProcessor', () => { name: 'techdocs-durp', url: 'https://github.com/backstage/techdocs-durp', isArchived: false, - defaultBranchRef: undefined, + defaultBranchRef: null, }, ], }); @@ -233,6 +295,7 @@ describe('GithubDiscoveryProcessor', () => { optional: true, }); }); + it('filter unrelated and archived repositories', async () => { const location: LocationSpec = { type: 'github-discovery', diff --git a/plugins/catalog-backend/src/ingestion/processors/github/github.ts b/plugins/catalog-backend/src/ingestion/processors/github/github.ts index 5ff3856f97..fbbe9b3470 100644 --- a/plugins/catalog-backend/src/ingestion/processors/github/github.ts +++ b/plugins/catalog-backend/src/ingestion/processors/github/github.ts @@ -60,9 +60,9 @@ export type Repository = { name: string; url: string; isArchived: boolean; - defaultBranchRef?: { + defaultBranchRef: { name: string; - }; + } | null; }; export type Connection = { From 0785cf54c35afc36b114755ac852116284b0dccc Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Fri, 27 Aug 2021 08:09:56 +0100 Subject: [PATCH 4/5] correct a small mistake I made in one test Signed-off-by: Brian Fletcher --- .../src/ingestion/processors/GithubDiscoveryProcessor.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts index 8fb79bb20d..d5173657b7 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts @@ -226,7 +226,8 @@ describe('GithubDiscoveryProcessor', () => { type: 'location', location: { type: 'url', - target: 'https://github.com/backstage/blob/master/catalog-info.yaml', + target: + 'https://github.com/backstage/backstage/blob/master/catalog-info.yaml', }, optional: true, }); From 391490d75e160cfda3253f9349c9b9a08cb30e7f Mon Sep 17 00:00:00 2001 From: Nicolas Arnold Date: Fri, 27 Aug 2021 09:59:29 +0200 Subject: [PATCH 5/5] Fixing broken test and adding one for the undefined case Signed-off-by: Nicolas Arnold --- .../GithubDiscoveryProcessor.test.ts | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts index d5173657b7..12d9e66659 100644 --- a/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/GithubDiscoveryProcessor.test.ts @@ -173,16 +173,16 @@ describe('GithubDiscoveryProcessor', () => { it('output repositories with wildcards default branch option', async () => { const location: LocationSpec = { type: 'github-discovery', - target: 'https://github.com/backstage/techdocs-*/blob/-/catalog.yaml', + target: 'https://github.com/backstage/*/blob/-/catalog.yaml', }; mockGetOrganizationRepositories.mockResolvedValueOnce({ repositories: [ { name: 'backstage', - url: 'https://github.com/backstage/backstage', + url: 'https://github.com/backstage/tech-docs', isArchived: false, defaultBranchRef: { - name: 'master', + name: 'main', }, }, ], @@ -195,12 +195,35 @@ describe('GithubDiscoveryProcessor', () => { type: 'location', location: { type: 'url', - target: 'https://github.com/backstage/blob/master/catalog.yaml', + target: + 'https://github.com/backstage/tech-docs/blob/main/catalog.yaml', }, optional: true, }); }); + it("doesn't output repositories as default branch returned is empty", async () => { + const location: LocationSpec = { + type: 'github-discovery', + target: 'https://github.com/backstage/blob/-/catalog.yaml', + }; + mockGetOrganizationRepositories.mockResolvedValueOnce({ + repositories: [ + { + name: 'backstage', + url: 'https://github.com/backstage/tech-docs', + isArchived: false, + defaultBranchRef: null, + }, + ], + }); + const emitter = jest.fn(); + + await processor.readLocation(location, false, emitter); + + expect(emitter).not.toHaveBeenCalled(); + }); + it('output repositories with wildcards default branch option without catalog-info patch or branch match', async () => { const location: LocationSpec = { type: 'github-discovery',