From f64345108a0e870107e0c27c93298c3a0e1ca7e6 Mon Sep 17 00:00:00 2001 From: Andreas Berger Date: Tue, 21 Feb 2023 15:54:23 +0100 Subject: [PATCH 1/4] Add new config option for GitlabDiscoveryEntityProvider to distinguish between `branch` and `fallbackBranch` * A new config option `fallbackBranch` is added to the GitlabDiscoveryEntityProvider, to define the branch to use , if there is no default branch configured at the repository * The existing `branch` configuration is now used to explicitly set the branch to look up the catalog-info resolves #13587 Signed-off-by: Andreas Berger --- .changeset/fresh-schools-fly.md | 11 ++ docs/integrations/gitlab/discovery.md | 1 + .../src/lib/types.ts | 5 +- .../GitlabDiscoveryEntityProvider.test.ts | 107 ++++++++++++++++++ .../GitlabDiscoveryEntityProvider.ts | 10 +- .../src/providers/config.test.ts | 4 + .../src/providers/config.ts | 11 +- 7 files changed, 136 insertions(+), 13 deletions(-) create mode 100644 .changeset/fresh-schools-fly.md diff --git a/.changeset/fresh-schools-fly.md b/.changeset/fresh-schools-fly.md new file mode 100644 index 0000000000..52502a4ad8 --- /dev/null +++ b/.changeset/fresh-schools-fly.md @@ -0,0 +1,11 @@ +--- +'@backstage/plugin-catalog-backend-module-gitlab': major +--- + +The configuration of the `GitlabDiscoveryEntityProvider` has changed as follows: + +- The configuration key `branch` is now used to define the branch from which the catalog-info should be discovered. +- The old configuration key `branch` is now called `fallbackBranch`. This value specifies which branch should be used + if no default branch is defined on the project itself. + +To migrate to the new configuration value, rename `branch` to `fallbackBranch` diff --git a/docs/integrations/gitlab/discovery.md b/docs/integrations/gitlab/discovery.md index 6d0bb7e2e2..48383c4275 100644 --- a/docs/integrations/gitlab/discovery.md +++ b/docs/integrations/gitlab/discovery.md @@ -21,6 +21,7 @@ catalog: gitlab: yourProviderId: host: gitlab-host # Identifies one of the hosts set up in the integrations + branch: main # Optional. Used to discover on a concrete branch fallbackBranch: main # Optional. Fallback to be used if there is no default branch configured at the Gitlab repository. It is only used, if `branch` is undefined. Uses `master` as default group: example-group # Optional. Group and subgroup (if needed) to look for repositories. If not present the whole instance will be scanned entityFilename: catalog-info.yaml # Optional. Defaults to `catalog-info.yaml` diff --git a/plugins/catalog-backend-module-gitlab/src/lib/types.ts b/plugins/catalog-backend-module-gitlab/src/lib/types.ts index b4480efdc2..c27007861b 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/types.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/types.ts @@ -70,11 +70,12 @@ export type GitlabProviderConfig = { group: string; id: string; /** - * @deprecated use `fallbackBranch` instead + * The name of the branch to be used, to discover catalog files. */ branch?: string; /** - * If there is no default branch defined at the project, this fallback is used to discover catalog files. + * If no `branch` is configured and there is no default branch defined at the project as well, this fallback is used + * to discover catalog files. * Defaults to: `master` */ fallbackBranch: string; diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.test.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.test.ts index 42fa1b471c..75a4c6414c 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.test.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.test.ts @@ -449,4 +449,111 @@ describe('GitlabDiscoveryEntityProvider', () => { 'GitlabDiscoveryEntityProvider:test-id', ); }); + + it('should filter found projects based on the branch', async () => { + const config = new ConfigReader({ + integrations: { + gitlab: [ + { + host: 'test-gitlab', + apiBaseUrl: 'https://api.gitlab.example/api/v4', + token: '1234', + }, + ], + }, + catalog: { + providers: { + gitlab: { + 'test-id': { + host: 'test-gitlab', + branch: 'test', + }, + }, + }, + }, + }); + const schedule = new PersistingTaskRunner(); + const entityProviderConnection: EntityProviderConnection = { + applyMutation: jest.fn(), + refresh: jest.fn(), + }; + const provider = GitlabDiscoveryEntityProvider.fromConfig(config, { + logger, + schedule, + })[0]; + + server.use( + rest.get( + `https://api.gitlab.example/api/v4/projects`, + (_req, res, ctx) => { + const response = [ + { + id: 123, + default_branch: 'master', + archived: false, + last_activity_at: new Date().toString(), + web_url: 'https://api.gitlab.example/test-group/test-repo', + path_with_namespace: 'test-group/test-repo', + }, + { + id: 124, + default_branch: 'master', + archived: false, + last_activity_at: new Date().toString(), + web_url: 'https://api.gitlab.example/john/example', + path_with_namespace: 'john/example', + }, + ]; + return res(ctx.json(response)); + }, + ), + rest.head( + 'https://api.gitlab.example/api/v4/projects/test-group%2Ftest-repo/repository/files/catalog-info.yaml', + (_, res, ctx) => { + return res(ctx.status(404, 'Not Found')); + }, + ), + rest.head( + 'https://api.gitlab.example/api/v4/projects/john%2Fexample/repository/files/catalog-info.yaml', + (req, res, ctx) => { + if (req.url.searchParams.get('ref') === 'test') { + return res(ctx.status(200)); + } + return res(ctx.status(404, 'Not Found')); + }, + ), + ); + + await provider.connect(entityProviderConnection); + + await provider.refresh(logger); + + expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ + type: 'full', + entities: [ + { + entity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Location', + metadata: { + annotations: { + 'backstage.io/managed-by-location': + 'url:https://api.gitlab.example/john/example/-/blob/test/catalog-info.yaml', + 'backstage.io/managed-by-origin-location': + 'url:https://api.gitlab.example/john/example/-/blob/test/catalog-info.yaml', + }, + name: 'generated-232185d858fee049986d202c10316d634e76a3d1', + }, + spec: { + presence: 'optional', + target: + 'https://api.gitlab.example/john/example/-/blob/test/catalog-info.yaml', + type: 'url', + }, + }, + locationKey: 'GitlabDiscoveryEntityProvider:test-id', + }, + ], + }); + }); }); diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts index ece10ee8c6..2f5baa68f6 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts @@ -181,6 +181,7 @@ export class GitlabDiscoveryEntityProvider implements EntityProvider { } if ( + !this.config.branch && this.config.fallbackBranch === '*' && project.default_branch === undefined ) { @@ -188,7 +189,9 @@ export class GitlabDiscoveryEntityProvider implements EntityProvider { } const project_branch = - project.default_branch ?? this.config.fallbackBranch; + this.config.branch ?? + project.default_branch ?? + this.config.fallbackBranch; const projectHasFile: boolean = await client.hasFile( project.path_with_namespace ?? '', @@ -211,7 +214,10 @@ export class GitlabDiscoveryEntityProvider implements EntityProvider { } private createLocationSpec(project: GitLabProject): LocationSpec { - const project_branch = project.default_branch ?? this.config.fallbackBranch; + const project_branch = + this.config.branch ?? + project.default_branch ?? + this.config.fallbackBranch; return { type: 'url', target: `${project.web_url}/-/blob/${project_branch}/${this.config.catalogFile}`, diff --git a/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts b/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts index c9b35e78c2..87d9e5deb0 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts @@ -53,6 +53,7 @@ describe('config', () => { expect(r).toStrictEqual({ id: 'test', group: 'group', + fallbackBranch: undefined, fallbackBranch: 'master', host: 'host', catalogFile: 'catalog-info.yaml', @@ -74,6 +75,7 @@ describe('config', () => { group: 'group', host: 'host', branch: 'not-master', + fallbackBranch: 'main', entityFilename: 'custom-file.yaml', }, }, @@ -88,6 +90,7 @@ describe('config', () => { id: 'test', group: 'group', fallbackBranch: 'not-master', + fallbackBranch: 'main', host: 'host', catalogFile: 'custom-file.yaml', projectPattern: /[\s\S]*/, @@ -125,6 +128,7 @@ describe('config', () => { expect(r).toStrictEqual({ id: 'test', group: 'group', + fallbackBranch: undefined, fallbackBranch: 'master', host: 'host', catalogFile: 'catalog-info.yaml', diff --git a/plugins/catalog-backend-module-gitlab/src/providers/config.ts b/plugins/catalog-backend-module-gitlab/src/providers/config.ts index e5cf5a6298..c9b44f601f 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/config.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/config.ts @@ -36,15 +36,7 @@ function readGitlabConfig( const group = config.getOptionalString('group') ?? ''; const host = config.getString('host'); const branch = config.getOptionalString('branch'); - - if (branch) { - logger.warn( - 'The configuration key `branch` has been deprecated in favor of the configuration key `fallbackBranch`.', - ); - } - - const fallbackBranch = - config.getOptionalString('fallbackBranch') ?? branch ?? 'master'; + const fallbackBranch = config.getOptionalString('fallbackBranch') ?? 'master'; const catalogFile = config.getOptionalString('entityFilename') ?? 'catalog-info.yaml'; const projectPattern = new RegExp( @@ -65,6 +57,7 @@ function readGitlabConfig( return { id, group, + branch, fallbackBranch, host, catalogFile, From 359f9f796027e153e47fdf893ca8c2cbc24b85fa Mon Sep 17 00:00:00 2001 From: Andreas Berger Date: Wed, 22 Feb 2023 16:33:34 +0100 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Johan Haals Signed-off-by: Andreas Berger --- .changeset/fresh-schools-fly.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/fresh-schools-fly.md b/.changeset/fresh-schools-fly.md index 52502a4ad8..bde325b574 100644 --- a/.changeset/fresh-schools-fly.md +++ b/.changeset/fresh-schools-fly.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-catalog-backend-module-gitlab': major +'@backstage/plugin-catalog-backend-module-gitlab': minor --- The configuration of the `GitlabDiscoveryEntityProvider` has changed as follows: From 259abbdc884ea993fb138d3dd6d67e5aef2e192c Mon Sep 17 00:00:00 2001 From: Andreas Berger Date: Sat, 18 Mar 2023 17:42:11 +0100 Subject: [PATCH 3/4] cleanup after rebase Signed-off-by: Andreas Berger --- .../GitlabDiscoveryEntityProvider.ts | 5 +---- .../GitlabOrgDiscoveryEntityProvider.ts | 5 +---- .../src/providers/config.test.ts | 21 ++++++++---------- .../src/providers/config.ts | 22 +++---------------- 4 files changed, 14 insertions(+), 39 deletions(-) diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts index 2f5baa68f6..6001a32a4a 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts @@ -61,10 +61,7 @@ export class GitlabDiscoveryEntityProvider implements EntityProvider { throw new Error('Either schedule or scheduler must be provided.'); } - const providerConfigs = readGitlabConfigs( - config, - options.logger.child({ target: 'GitlabDiscoveryEntityProvider' }), - ); + const providerConfigs = readGitlabConfigs(config); const integrations = ScmIntegrations.fromConfig(config).gitlab; const providers: GitlabDiscoveryEntityProvider[] = []; diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts index 12cdf40e0c..1fe607afbb 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts @@ -72,10 +72,7 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider { throw new Error('Either schedule or scheduler must be provided.'); } - const providerConfigs = readGitlabConfigs( - config, - options.logger.child({ target: 'GitlabOrgDiscoveryEntityProvider' }), - ); + const providerConfigs = readGitlabConfigs(config); const integrations = ScmIntegrations.fromConfig(config).gitlab; const providers: GitlabOrgDiscoveryEntityProvider[] = []; diff --git a/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts b/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts index 87d9e5deb0..1035538939 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts @@ -17,9 +17,6 @@ import { ConfigReader } from '@backstage/config'; import { Duration } from 'luxon'; import { readGitlabConfigs } from './config'; -import { getVoidLogger } from '@backstage/backend-common'; - -const logger = getVoidLogger(); describe('config', () => { it('empty gitlab config', () => { @@ -29,7 +26,7 @@ describe('config', () => { }, }); - const result = readGitlabConfigs(config, logger); + const result = readGitlabConfigs(config); expect(result).toHaveLength(0); }); @@ -47,13 +44,13 @@ describe('config', () => { }, }); - const result = readGitlabConfigs(config, logger); + const result = readGitlabConfigs(config); expect(result).toHaveLength(1); result.forEach(r => expect(r).toStrictEqual({ id: 'test', group: 'group', - fallbackBranch: undefined, + branch: undefined, fallbackBranch: 'master', host: 'host', catalogFile: 'catalog-info.yaml', @@ -83,13 +80,13 @@ describe('config', () => { }, }); - const result = readGitlabConfigs(config, logger); + const result = readGitlabConfigs(config); expect(result).toHaveLength(1); result.forEach(r => expect(r).toStrictEqual({ id: 'test', group: 'group', - fallbackBranch: 'not-master', + branch: 'not-master', fallbackBranch: 'main', host: 'host', catalogFile: 'custom-file.yaml', @@ -122,13 +119,13 @@ describe('config', () => { }, }); - const result = readGitlabConfigs(config, logger); + const result = readGitlabConfigs(config); expect(result).toHaveLength(1); result.forEach(r => expect(r).toStrictEqual({ id: 'test', group: 'group', - fallbackBranch: undefined, + branch: undefined, fallbackBranch: 'master', host: 'host', catalogFile: 'catalog-info.yaml', @@ -162,7 +159,7 @@ describe('config', () => { }, }); - expect(() => readGitlabConfigs(config, logger)).toThrow( + expect(() => readGitlabConfigs(config)).toThrow( "Missing required config value at 'catalog.providers.gitlab.test.host'", ); }); @@ -181,7 +178,7 @@ describe('config', () => { }, }); - const result = readGitlabConfigs(config, logger); + const result = readGitlabConfigs(config); expect(result).toHaveLength(1); expect(result[0].group).toEqual(''); }); diff --git a/plugins/catalog-backend-module-gitlab/src/providers/config.ts b/plugins/catalog-backend-module-gitlab/src/providers/config.ts index c9b44f601f..cd15c4825c 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/config.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/config.ts @@ -17,7 +17,6 @@ import { readTaskScheduleDefinitionFromConfig } from '@backstage/backend-tasks'; import { Config } from '@backstage/config'; import { GitlabProviderConfig } from '../lib'; -import { Logger } from 'winston'; /** * Extracts the gitlab config from a config object @@ -26,13 +25,8 @@ import { Logger } from 'winston'; * * @param id - The provider key * @param config - The config object to extract from - * @param logger - The logger */ -function readGitlabConfig( - id: string, - config: Config, - logger: Logger, -): GitlabProviderConfig { +function readGitlabConfig(id: string, config: Config): GitlabProviderConfig { const group = config.getOptionalString('group') ?? ''; const host = config.getString('host'); const branch = config.getOptionalString('branch'); @@ -75,12 +69,8 @@ function readGitlabConfig( * @public * * @param config - The config object to extract from - * @param logger - The logger */ -export function readGitlabConfigs( - config: Config, - logger: Logger, -): GitlabProviderConfig[] { +export function readGitlabConfigs(config: Config): GitlabProviderConfig[] { const configs: GitlabProviderConfig[] = []; const providerConfigs = config.getOptionalConfig('catalog.providers.gitlab'); @@ -90,13 +80,7 @@ export function readGitlabConfigs( } for (const id of providerConfigs.keys()) { - configs.push( - readGitlabConfig( - id, - providerConfigs.getConfig(id), - logger.child({ target: id }), - ), - ); + configs.push(readGitlabConfig(id, providerConfigs.getConfig(id))); } return configs; From c11f33b8429e21eb5ce876138f85874994320377 Mon Sep 17 00:00:00 2001 From: Andreas Berger Date: Mon, 20 Mar 2023 11:57:00 +0100 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Johan Haals Signed-off-by: Andreas Berger --- .changeset/fresh-schools-fly.md | 2 +- docs/integrations/gitlab/discovery.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/fresh-schools-fly.md b/.changeset/fresh-schools-fly.md index bde325b574..e364e08d43 100644 --- a/.changeset/fresh-schools-fly.md +++ b/.changeset/fresh-schools-fly.md @@ -2,7 +2,7 @@ '@backstage/plugin-catalog-backend-module-gitlab': minor --- -The configuration of the `GitlabDiscoveryEntityProvider` has changed as follows: +**BREAKING**: The configuration of the `GitlabDiscoveryEntityProvider` has changed as follows: - The configuration key `branch` is now used to define the branch from which the catalog-info should be discovered. - The old configuration key `branch` is now called `fallbackBranch`. This value specifies which branch should be used diff --git a/docs/integrations/gitlab/discovery.md b/docs/integrations/gitlab/discovery.md index 48383c4275..45991b2cf5 100644 --- a/docs/integrations/gitlab/discovery.md +++ b/docs/integrations/gitlab/discovery.md @@ -21,7 +21,7 @@ catalog: gitlab: yourProviderId: host: gitlab-host # Identifies one of the hosts set up in the integrations - branch: main # Optional. Used to discover on a concrete branch + branch: main # Optional. Used to discover on a specific branch fallbackBranch: main # Optional. Fallback to be used if there is no default branch configured at the Gitlab repository. It is only used, if `branch` is undefined. Uses `master` as default group: example-group # Optional. Group and subgroup (if needed) to look for repositories. If not present the whole instance will be scanned entityFilename: catalog-info.yaml # Optional. Defaults to `catalog-info.yaml`