From ba95c81bff16e2fc506148ddbf0858d7c30dd074 Mon Sep 17 00:00:00 2001 From: "Ruslan.Nasyrov" Date: Fri, 13 May 2022 14:55:05 +0500 Subject: [PATCH] feat/catalog-backend-module-gitlab: create url location only if file exists (fixes #1) Signed-off-by: Ruslan.Nasyrov --- .changeset/real-beers-type.md | 11 +++++++++- .../src/GitLabDiscoveryProcessor.test.ts | 20 +++++++++++++------ .../src/GitLabDiscoveryProcessor.ts | 11 +++++----- .../src/lib/client.ts | 7 ------- 4 files changed, 30 insertions(+), 19 deletions(-) diff --git a/.changeset/real-beers-type.md b/.changeset/real-beers-type.md index 106a97bffb..e3ab67cfca 100644 --- a/.changeset/real-beers-type.md +++ b/.changeset/real-beers-type.md @@ -2,4 +2,13 @@ '@backstage/plugin-catalog-backend-module-gitlab': patch --- -do not creating url location object if file with component definition do not exists in project, that decrease count of request to gitlab with 404 status code +do not create url location object if file with component definition do not exists in project, that decrease count of request to gitlab with 404 status code. Now we can create processor with new flag to enable this logic: + +```ts +const processor = GitLabDiscoveryProcessor.fromConfig(config, { + logger, + skipReposWithoutExactFileMatch: true, +}); +``` + +**WARNING:** This new functionality does not support globs in the repo filepath diff --git a/plugins/catalog-backend-module-gitlab/src/GitLabDiscoveryProcessor.test.ts b/plugins/catalog-backend-module-gitlab/src/GitLabDiscoveryProcessor.test.ts index 3f4534ad48..278efe8036 100644 --- a/plugins/catalog-backend-module-gitlab/src/GitLabDiscoveryProcessor.test.ts +++ b/plugins/catalog-backend-module-gitlab/src/GitLabDiscoveryProcessor.test.ts @@ -29,6 +29,7 @@ const SERVER_URL = `https://${DOMAIN}`; const API_URL = `${SERVER_URL}/api/v4`; const PROJECTS_URL = `${API_URL}/projects`; const GROUP_PROJECTS_URL = `${API_URL}/groups/group%2Fsubgroup/projects`; +const EXISTING_PROJECT_PATH = 'exist'; const PROJECT_LOCATION: LocationSpec = { type: 'gitlab-discovery', @@ -50,7 +51,7 @@ const GROUP_LOCATION_CUSTOM_BRANCH: LocationSpec = { function setupFakeServer( url: string, - list_projects_callback: (request: { + listProjectsCallback: (request: { page: number; include_subgroups: boolean; }) => { @@ -65,7 +66,7 @@ function setupFakeServer( } const page = req.url.searchParams.get('page'); const include_subgroups = req.url.searchParams.get('include_subgroups'); - const response = list_projects_callback({ + const response = listProjectsCallback({ page: parseInt(page!, 10), include_subgroups: include_subgroups === 'true', }); @@ -96,6 +97,11 @@ function setupFakeServer( if (ref === 'main' || ref === 'master') { return res(ctx.status(200)); } + + if (EXISTING_PROJECT_PATH === req.params.project_path) { + return res(ctx.status(200)); + } + return res(ctx.status(404)); }, ), @@ -345,7 +351,9 @@ describe('GitlabDiscoveryProcessor', () => { }); it('can filter based on file existing', async () => { - const processor = getProcessor({ options: { checkFileExistence: true } }); + const processor = getProcessor({ + options: { skipReposWithoutExactFileMatch: true }, + }); setupFakeServer(GROUP_PROJECTS_URL, request => { if (!request.include_subgroups) { throw new Error('include_subgroups should be set'); @@ -367,8 +375,8 @@ describe('GitlabDiscoveryProcessor', () => { archived: false, default_branch: 'main', last_activity_at: '2021-08-05T11:03:05.774Z', - web_url: 'https://gitlab.fake/g/2', - path_with_namespace: 'g/2', + web_url: `https://gitlab.fake/${EXISTING_PROJECT_PATH}`, + path_with_namespace: EXISTING_PROJECT_PATH, }, ], }; @@ -382,7 +390,7 @@ describe('GitlabDiscoveryProcessor', () => { result.push(e); }); // If everything was set up correctly, we should have received the fake repo specified above - expect(result).toHaveLength(0); + expect(result).toHaveLength(1); }); it('uses the previous scan timestamp to filter', async () => { diff --git a/plugins/catalog-backend-module-gitlab/src/GitLabDiscoveryProcessor.ts b/plugins/catalog-backend-module-gitlab/src/GitLabDiscoveryProcessor.ts index e5cb7b15db..73b8c52358 100644 --- a/plugins/catalog-backend-module-gitlab/src/GitLabDiscoveryProcessor.ts +++ b/plugins/catalog-backend-module-gitlab/src/GitLabDiscoveryProcessor.ts @@ -41,11 +41,11 @@ export class GitLabDiscoveryProcessor implements CatalogProcessor { private readonly integrations: ScmIntegrationRegistry; private readonly logger: Logger; private readonly cache: CacheClient; - private readonly checkFileExistence: boolean; + private readonly skipReposWithoutExactFileMatch: boolean; static fromConfig( config: Config, - options: { logger: Logger; checkFileExistence?: boolean }, + options: { logger: Logger; skipReposWithoutExactFileMatch?: boolean }, ): GitLabDiscoveryProcessor { const integrations = ScmIntegrations.fromConfig(config); const pluginCache = @@ -62,12 +62,13 @@ export class GitLabDiscoveryProcessor implements CatalogProcessor { integrations: ScmIntegrationRegistry; pluginCache: PluginCacheManager; logger: Logger; - checkFileExistence?: boolean; + skipReposWithoutExactFileMatch?: boolean; }) { this.integrations = options.integrations; this.cache = options.pluginCache.getClient(); this.logger = options.logger; - this.checkFileExistence = options.checkFileExistence || false; + this.skipReposWithoutExactFileMatch = + options.skipReposWithoutExactFileMatch || false; } getProcessorName(): string { @@ -120,7 +121,7 @@ export class GitLabDiscoveryProcessor implements CatalogProcessor { continue; } - if (this.checkFileExistence) { + if (this.skipReposWithoutExactFileMatch) { const project_branch = branch === '*' ? project.default_branch : branch; const projectHasFile: boolean = await client.hasFile( diff --git a/plugins/catalog-backend-module-gitlab/src/lib/client.ts b/plugins/catalog-backend-module-gitlab/src/lib/client.ts index aa8eb22c19..5c43db6e68 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/client.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/client.ts @@ -74,19 +74,12 @@ export class GitLabClient { const request = new URL(`${this.config.apiBaseUrl}${endpoint}`); request.searchParams.append('ref', branch); - this.logger.debug(`Fetching: ${request.toString()}`); - const response = await fetch(request.toString(), { headers: getGitLabRequestOptions(this.config).headers, method: 'HEAD', }); if (!response.ok) { - this.logger.debug( - `Unexpected response when fetching ${request.toString()}. Expected 200 but got ${ - response.status - } - ${response.statusText}`, - ); return false; }