From af1095f1e1160080f5b00c544d725821603f56fb Mon Sep 17 00:00:00 2001 From: Andreas Berger Date: Wed, 22 Feb 2023 16:52:58 +0100 Subject: [PATCH 1/3] Deprecate config key `branch` of GitlabDiscoveryEntityProvider in favor of new key `fallbackBranch` relates #13587, #16502 Signed-off-by: Andreas Berger --- .changeset/silver-lies-rest.md | 6 ++++++ docs/integrations/gitlab/discovery.md | 2 +- plugins/catalog-backend-module-gitlab/src/lib/types.ts | 10 +++++++++- .../src/providers/GitlabDiscoveryEntityProvider.ts | 10 +++++++--- .../src/providers/config.test.ts | 6 +++--- .../src/providers/config.ts | 7 +++++-- 6 files changed, 31 insertions(+), 10 deletions(-) create mode 100644 .changeset/silver-lies-rest.md diff --git a/.changeset/silver-lies-rest.md b/.changeset/silver-lies-rest.md new file mode 100644 index 0000000000..76fdba0b43 --- /dev/null +++ b/.changeset/silver-lies-rest.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-catalog-backend-module-gitlab': minor +--- + +The configuration key `branch` of the `GitlabDiscoveryEntityProvider` has been deprecated in favor of the configuration key `fallbackBranch`. +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 9a065e776a..4c448d3892 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. Uses `master` as default + 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` projectPattern: '[\s\S]*' # Optional. Filters found projects based on provided patter. Defaults to `[\s\S]*`, which means to not filter anything diff --git a/plugins/catalog-backend-module-gitlab/src/lib/types.ts b/plugins/catalog-backend-module-gitlab/src/lib/types.ts index 2b2b888851..c3fc8a4ede 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/types.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/types.ts @@ -61,7 +61,15 @@ export type GitlabProviderConfig = { host: string; group: string; id: string; - branch: string; + /** + * @deprecated use `fallbackBranch` instead + */ + branch?: string; + /** + * If there is no default branch defined at the project, this fallback is used to discover catalog files. + * Defaults to: `master` + */ + fallbackBranch: string; catalogFile: string; projectPattern: RegExp; userPattern: RegExp; diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts index 1cc818c811..1eafe2f637 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts @@ -177,11 +177,15 @@ export class GitlabDiscoveryEntityProvider implements EntityProvider { continue; } - if (this.config.branch === '*' && project.default_branch === undefined) { + if ( + this.config.fallbackBranch === '*' && + project.default_branch === undefined + ) { continue; } - const project_branch = project.default_branch ?? this.config.branch; + const project_branch = + project.default_branch ?? this.config.fallbackBranch; const projectHasFile: boolean = await client.hasFile( project.path_with_namespace ?? '', @@ -204,7 +208,7 @@ export class GitlabDiscoveryEntityProvider implements EntityProvider { } private createLocationSpec(project: GitLabProject): LocationSpec { - const project_branch = project.default_branch ?? this.config.branch; + const project_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 e72c38e9d1..a7ab354354 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts @@ -50,7 +50,7 @@ describe('config', () => { expect(r).toStrictEqual({ id: 'test', group: 'group', - branch: 'master', + fallbackBranch: 'master', host: 'host', catalogFile: 'catalog-info.yaml', projectPattern: /[\s\S]*/, @@ -84,7 +84,7 @@ describe('config', () => { expect(r).toStrictEqual({ id: 'test', group: 'group', - branch: 'not-master', + fallbackBranch: 'not-master', host: 'host', catalogFile: 'custom-file.yaml', projectPattern: /[\s\S]*/, @@ -122,7 +122,7 @@ describe('config', () => { expect(r).toStrictEqual({ id: 'test', group: 'group', - branch: 'master', + fallbackBranch: 'master', host: 'host', catalogFile: 'catalog-info.yaml', projectPattern: /[\s\S]*/, diff --git a/plugins/catalog-backend-module-gitlab/src/providers/config.ts b/plugins/catalog-backend-module-gitlab/src/providers/config.ts index cd856f97da..e64ff14cdd 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/config.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/config.ts @@ -29,7 +29,10 @@ import { GitlabProviderConfig } from '../lib'; function readGitlabConfig(id: string, config: Config): GitlabProviderConfig { const group = config.getOptionalString('group') ?? ''; const host = config.getString('host'); - const branch = config.getOptionalString('branch') ?? 'master'; + const fallbackBranch = + config.getOptionalString('fallbackBranch') ?? + config.getOptionalString('branch') ?? + 'master'; const catalogFile = config.getOptionalString('entityFilename') ?? 'catalog-info.yaml'; const projectPattern = new RegExp( @@ -50,7 +53,7 @@ function readGitlabConfig(id: string, config: Config): GitlabProviderConfig { return { id, group, - branch, + fallbackBranch, host, catalogFile, projectPattern, From b2fce0b01e1a5adfe00aa0b9cfaa803f963e35c7 Mon Sep 17 00:00:00 2001 From: Andreas Berger Date: Thu, 23 Feb 2023 12:12:53 +0100 Subject: [PATCH 2/3] Add warn log if config key `branch` is used Signed-off-by: Andreas Berger --- .../GitlabDiscoveryEntityProvider.ts | 5 ++- .../GitlabOrgDiscoveryEntityProvider.ts | 5 ++- .../src/providers/config.test.ts | 15 ++++---- .../src/providers/config.ts | 34 +++++++++++++++---- 4 files changed, 45 insertions(+), 14 deletions(-) diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts index 1eafe2f637..f0d88c34f5 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts @@ -61,7 +61,10 @@ export class GitlabDiscoveryEntityProvider implements EntityProvider { throw new Error('Either schedule or scheduler must be provided.'); } - const providerConfigs = readGitlabConfigs(config); + const providerConfigs = readGitlabConfigs( + config, + options.logger.child({ target: 'GitlabDiscoveryEntityProvider' }), + ); 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 f90a61f9e9..ad9d9dae79 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts @@ -72,7 +72,10 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider { throw new Error('Either schedule or scheduler must be provided.'); } - const providerConfigs = readGitlabConfigs(config); + const providerConfigs = readGitlabConfigs( + config, + options.logger.child({ target: 'GitlabOrgDiscoveryEntityProvider' }), + ); 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 a7ab354354..c9b35e78c2 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts @@ -17,6 +17,9 @@ 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', () => { @@ -26,7 +29,7 @@ describe('config', () => { }, }); - const result = readGitlabConfigs(config); + const result = readGitlabConfigs(config, logger); expect(result).toHaveLength(0); }); @@ -44,7 +47,7 @@ describe('config', () => { }, }); - const result = readGitlabConfigs(config); + const result = readGitlabConfigs(config, logger); expect(result).toHaveLength(1); result.forEach(r => expect(r).toStrictEqual({ @@ -78,7 +81,7 @@ describe('config', () => { }, }); - const result = readGitlabConfigs(config); + const result = readGitlabConfigs(config, logger); expect(result).toHaveLength(1); result.forEach(r => expect(r).toStrictEqual({ @@ -116,7 +119,7 @@ describe('config', () => { }, }); - const result = readGitlabConfigs(config); + const result = readGitlabConfigs(config, logger); expect(result).toHaveLength(1); result.forEach(r => expect(r).toStrictEqual({ @@ -155,7 +158,7 @@ describe('config', () => { }, }); - expect(() => readGitlabConfigs(config)).toThrow( + expect(() => readGitlabConfigs(config, logger)).toThrow( "Missing required config value at 'catalog.providers.gitlab.test.host'", ); }); @@ -174,7 +177,7 @@ describe('config', () => { }, }); - const result = readGitlabConfigs(config); + const result = readGitlabConfigs(config, logger); 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 e64ff14cdd..e5cf5a6298 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/config.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/config.ts @@ -17,6 +17,7 @@ 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 @@ -25,14 +26,25 @@ import { GitlabProviderConfig } from '../lib'; * * @param id - The provider key * @param config - The config object to extract from + * @param logger - The logger */ -function readGitlabConfig(id: string, config: Config): GitlabProviderConfig { +function readGitlabConfig( + id: string, + config: Config, + logger: Logger, +): GitlabProviderConfig { 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') ?? - config.getOptionalString('branch') ?? - 'master'; + config.getOptionalString('fallbackBranch') ?? branch ?? 'master'; const catalogFile = config.getOptionalString('entityFilename') ?? 'catalog-info.yaml'; const projectPattern = new RegExp( @@ -70,8 +82,12 @@ function readGitlabConfig(id: string, config: Config): GitlabProviderConfig { * @public * * @param config - The config object to extract from + * @param logger - The logger */ -export function readGitlabConfigs(config: Config): GitlabProviderConfig[] { +export function readGitlabConfigs( + config: Config, + logger: Logger, +): GitlabProviderConfig[] { const configs: GitlabProviderConfig[] = []; const providerConfigs = config.getOptionalConfig('catalog.providers.gitlab'); @@ -81,7 +97,13 @@ export function readGitlabConfigs(config: Config): GitlabProviderConfig[] { } for (const id of providerConfigs.keys()) { - configs.push(readGitlabConfig(id, providerConfigs.getConfig(id))); + configs.push( + readGitlabConfig( + id, + providerConfigs.getConfig(id), + logger.child({ target: id }), + ), + ); } return configs; From 3a8294aa963dc83480b5826448b2cae71ec5cb34 Mon Sep 17 00:00:00 2001 From: Andreas Berger Date: Fri, 24 Feb 2023 11:16:31 +0100 Subject: [PATCH 3/3] Improve changelog Signed-off-by: Andreas Berger --- .changeset/silver-lies-rest.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.changeset/silver-lies-rest.md b/.changeset/silver-lies-rest.md index 76fdba0b43..34b77f5375 100644 --- a/.changeset/silver-lies-rest.md +++ b/.changeset/silver-lies-rest.md @@ -1,6 +1,7 @@ --- -'@backstage/plugin-catalog-backend-module-gitlab': minor +'@backstage/plugin-catalog-backend-module-gitlab': patch --- The configuration key `branch` of the `GitlabDiscoveryEntityProvider` has been deprecated in favor of the configuration key `fallbackBranch`. +It will be reused in future release to enforce a concrete branch to be used in catalog file discovery. To migrate to the new configuration value, rename `branch` to `fallbackBranch`.