Merge pull request #16545 from Andy2003/feature/13587-gitlab-discovery-branch-handling-pre-migrate

Deprecate config key `branch` of GitlabDiscoveryEntityProvider in favor of new key `fallbackBranch`

.changeset/silver-lies-rest.md

@@ -0,0 +1,7 @@
+---
+'@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`.

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

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;

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[] = [];
 
@@ -177,11 +180,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 +211,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}`,

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[] = [];
 

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,13 +47,13 @@ describe('config', () => {
       },
     });
 
-    const result = readGitlabConfigs(config);
+    const result = readGitlabConfigs(config, logger);
     expect(result).toHaveLength(1);
     result.forEach(r =>
       expect(r).toStrictEqual({
         id: 'test',
         group: 'group',
-        branch: 'master',
+        fallbackBranch: 'master',
         host: 'host',
         catalogFile: 'catalog-info.yaml',
         projectPattern: /[\s\S]*/,
@@ -78,13 +81,13 @@ describe('config', () => {
       },
     });
 
-    const result = readGitlabConfigs(config);
+    const result = readGitlabConfigs(config, logger);
     expect(result).toHaveLength(1);
     result.forEach(r =>
       expect(r).toStrictEqual({
         id: 'test',
         group: 'group',
-        branch: 'not-master',
+        fallbackBranch: 'not-master',
         host: 'host',
         catalogFile: 'custom-file.yaml',
         projectPattern: /[\s\S]*/,
@@ -116,13 +119,13 @@ describe('config', () => {
       },
     });
 
-    const result = readGitlabConfigs(config);
+    const result = readGitlabConfigs(config, logger);
     expect(result).toHaveLength(1);
     result.forEach(r =>
       expect(r).toStrictEqual({
         id: 'test',
         group: 'group',
-        branch: 'master',
+        fallbackBranch: 'master',
         host: 'host',
         catalogFile: 'catalog-info.yaml',
         projectPattern: /[\s\S]*/,
@@ -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('');
   });

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,11 +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') ?? 'master';
+  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 catalogFile =
     config.getOptionalString('entityFilename') ?? 'catalog-info.yaml';
   const projectPattern = new RegExp(
@@ -50,7 +65,7 @@ function readGitlabConfig(id: string, config: Config): GitlabProviderConfig {
   return {
     id,
     group,
-    branch,
+    fallbackBranch,
     host,
     catalogFile,
     projectPattern,
@@ -67,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');
@@ -78,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;