From e320e2e1837158e4aa675dfb8d99dc1634eb979e Mon Sep 17 00:00:00 2001 From: Alexandre Fournier Date: Sun, 17 Aug 2025 18:38:18 -0400 Subject: [PATCH] Revert change that introduce groupPatterns and reuse the config groupPattern insteand Signed-off-by: Alexandre Fournier --- .changeset/chubby-results-relax.md | 2 +- docs/integrations/gitlab/discovery.md | 2 +- .../catalog-backend-module-gitlab/config.d.ts | 2 +- .../report.api.md | 3 +- .../src/__testUtils__/handlers.ts | 7 + .../src/__testUtils__/mocks.ts | 14 +- .../src/lib/types.ts | 10 +- .../GitlabDiscoveryEntityProvider.test.ts | 155 +++++++++--------- .../GitlabDiscoveryEntityProvider.ts | 33 ++-- .../GitlabOrgDiscoveryEntityProvider.ts | 8 +- .../src/providers/config.test.ts | 10 -- .../src/providers/config.ts | 23 ++- 12 files changed, 140 insertions(+), 129 deletions(-) diff --git a/.changeset/chubby-results-relax.md b/.changeset/chubby-results-relax.md index a668570694..c20c79085f 100644 --- a/.changeset/chubby-results-relax.md +++ b/.changeset/chubby-results-relax.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-catalog-backend-module-gitlab': minor +'@backstage/plugin-catalog-backend-module-gitlab': patch --- Added support for multiple group patterns instead of a single one to increase flexibility when filtering groups from GitLab. diff --git a/docs/integrations/gitlab/discovery.md b/docs/integrations/gitlab/discovery.md index 5b76a9597b..0fce7b864e 100644 --- a/docs/integrations/gitlab/discovery.md +++ b/docs/integrations/gitlab/discovery.md @@ -151,7 +151,7 @@ catalog: skipForkedRepos: false # Optional. If the project is a fork, skip repository includeArchivedRepos: false # Optional. If project is archived, include repository group: example-group # Optional. Group and subgroup (if needed) to look for repositories. If not present the whole instance will be scanned - groupPatterns: # Optional. Filters for groups based on a list of RegEx. Default, no filters. + groupPattern: # Optional. Filters for groups based on a list of RegEx. Default, no filters. - '^somegroup$' - 'anothergroup' entityFilename: catalog-info.yaml # Optional. Defaults to `catalog-info.yaml` diff --git a/plugins/catalog-backend-module-gitlab/config.d.ts b/plugins/catalog-backend-module-gitlab/config.d.ts index 4cc7ebe844..8a2b6d8fa5 100644 --- a/plugins/catalog-backend-module-gitlab/config.d.ts +++ b/plugins/catalog-backend-module-gitlab/config.d.ts @@ -72,7 +72,7 @@ export interface Config { /** * (Optional) RegExp for the Group Name Pattern */ - groupPattern?: string; + groupPattern?: string | string[]; /** * Specifies the types of group membership relations that should be included when ingesting data. * diff --git a/plugins/catalog-backend-module-gitlab/report.api.md b/plugins/catalog-backend-module-gitlab/report.api.md index 896680b41f..f5048835c1 100644 --- a/plugins/catalog-backend-module-gitlab/report.api.md +++ b/plugins/catalog-backend-module-gitlab/report.api.md @@ -111,8 +111,7 @@ export type GitlabProviderConfig = { catalogFile: string; projectPattern: RegExp; userPattern: RegExp; - groupPattern: RegExp; - groupPatterns: RegExp[]; + groupPattern: RegExp | RegExp[]; allowInherited?: boolean; relations?: string[]; orgEnabled?: boolean; diff --git a/plugins/catalog-backend-module-gitlab/src/__testUtils__/handlers.ts b/plugins/catalog-backend-module-gitlab/src/__testUtils__/handlers.ts index 7549e6faca..9af00d2564 100644 --- a/plugins/catalog-backend-module-gitlab/src/__testUtils__/handlers.ts +++ b/plugins/catalog-backend-module-gitlab/src/__testUtils__/handlers.ts @@ -81,6 +81,13 @@ const httpHandlers = [ ); }), + rest.get(`${apiBaseUrl}/groups/group1`, (_, res, ctx) => { + return res( + ctx.set('x-next-page', ''), + ctx.json(all_groups_response.find(g => g.full_path === 'group1')), + ); + }), + rest.get(`${apiBaseUrl}/groups/42`, (_, res, ctx) => { return res(ctx.status(500), ctx.json({ error: 'Internal Server Error' })); }), diff --git a/plugins/catalog-backend-module-gitlab/src/__testUtils__/mocks.ts b/plugins/catalog-backend-module-gitlab/src/__testUtils__/mocks.ts index 768ccb416b..d64b266183 100644 --- a/plugins/catalog-backend-module-gitlab/src/__testUtils__/mocks.ts +++ b/plugins/catalog-backend-module-gitlab/src/__testUtils__/mocks.ts @@ -836,7 +836,7 @@ export const config_groupPatterns_only_noMatch = { gitlab: { 'test-id': { host: 'example.com', - groupPatterns: ['^group8$', '^group9$', '^group10$'], + groupPattern: ['^group8$', '^group9$', '^group10$'], }, }, }, @@ -858,7 +858,7 @@ export const config_groupPatterns_only_Match1Group = { gitlab: { 'test-id': { host: 'example.com', - groupPatterns: ['^group1$', '^group9$', '^group10$'], + groupPattern: ['^group1$', '^group9$', '^group10$'], }, }, }, @@ -880,7 +880,7 @@ export const config_groupPatterns_multiple_matches = { gitlab: { 'test-id': { host: 'example.com', - groupPatterns: ['^group1$', '^group2$', '^group10$'], + groupPattern: ['^group1$', '^group2$', '^group10$'], }, }, }, @@ -902,7 +902,7 @@ export const config_groupPatterns_duplicate_match = { gitlab: { 'test-id': { host: 'example.com', - groupPatterns: ['^group1$', 'subgroup1'], // Both patterns match the same group + groupPattern: ['^group1$', 'subgroup1'], // Both patterns match the same group }, }, }, @@ -1312,6 +1312,12 @@ export const all_groups_response: GitLabGroup[] = [ description: '', full_path: 'group1/subgroup2', }, + { + id: 8, + name: 'awsome-group', + description: '', + full_path: 'awsome-group', + }, ]; export const group_with_subgroups_response: GitLabGroup[] = [ diff --git a/plugins/catalog-backend-module-gitlab/src/lib/types.ts b/plugins/catalog-backend-module-gitlab/src/lib/types.ts index a51e129fbf..aa2f73b35d 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/types.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/types.ts @@ -186,17 +186,11 @@ export type GitlabProviderConfig = { userPattern: RegExp; /** - * Filters found groups based on provided pattern. + * Filters found groups based on provided patterns. * Defaults to `[\s\S]*`, which means to not filter anything * - * @deprecated Use groupPatterns that provide a list instead. */ - groupPattern: RegExp; - - /** - * Optional, provide a list of pattern for each can match a list of groups. - */ - groupPatterns: RegExp[]; + groupPattern: RegExp | RegExp[]; /** * If true, the provider will also add inherited (ascendant) users to the ingested groups. 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 7d7bb699f5..f052434035 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.test.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.test.ts @@ -557,14 +557,6 @@ describe('GitlabDiscoveryEntityProvider - events', () => { }); expect(entityProviderConnection.applyMutation).toHaveBeenCalledTimes(0); }); - // EventSupportChange: stop add tests >>> - - // add the following test: - // setup: 3 projects, 2 groups - // 1 test where none match --> OK - // 1 test where match only 1 group --> OK - // 1 test where match 2 groups - // 1 test where match 1 group with 2 identical regex it('should ignore projects when none of the groups regex patterns match', async () => { const config = new ConfigReader(mock.config_groupPatterns_only_noMatch); @@ -590,93 +582,96 @@ describe('GitlabDiscoveryEntityProvider - events', () => { expect(entityProviderConnection.applyMutation).toHaveBeenCalledTimes(1); // No entities should be applied }); -}); -it('should include only the project that matches one of the group regex patterns', async () => { - const config = new ConfigReader(mock.config_groupPatterns_only_Match1Group); - const schedule = new PersistingTaskRunner(); - const entityProviderConnection: EntityProviderConnection = { - applyMutation: jest.fn(), - refresh: jest.fn(), - }; - const provider = GitlabDiscoveryEntityProvider.fromConfig(config, { - logger, - schedule, - })[0]; + it('should include only the project that matches one of the group regex patterns', async () => { + const config = new ConfigReader(mock.config_groupPatterns_only_Match1Group); - await provider.connect(entityProviderConnection); + const schedule = new PersistingTaskRunner(); + const entityProviderConnection: EntityProviderConnection = { + applyMutation: jest.fn(), + refresh: jest.fn(), + }; + const provider = GitlabDiscoveryEntityProvider.fromConfig(config, { + logger, + schedule, + })[0]; - await provider.refresh(logger); + await provider.connect(entityProviderConnection); - expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ - type: 'full', - entities: mock.expected_location_entities_default_branch.filter( - entity => - entity.entity.metadata.annotations[ - 'backstage.io/managed-by-location' - ].includes('/group1/'), // Only projects in 'group2' should match - ), + await provider.refresh(logger); + + expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ + type: 'full', + entities: mock.expected_location_entities_default_branch.filter( + entity => + entity.entity.metadata.annotations[ + 'backstage.io/managed-by-location' + ].includes('/group1/'), // Only projects in 'group2' should match + ), + }); + + expect(entityProviderConnection.applyMutation).toHaveBeenCalledTimes(1); // Only one matching project should be applied }); - expect(entityProviderConnection.applyMutation).toHaveBeenCalledTimes(1); // Only one matching project should be applied -}); -it('should include projects that match multiple group regex patterns', async () => { - const config = new ConfigReader(mock.config_groupPatterns_multiple_matches); + it('should include projects that match multiple group regex patterns', async () => { + const config = new ConfigReader(mock.config_groupPatterns_multiple_matches); - const schedule = new PersistingTaskRunner(); - const entityProviderConnection: EntityProviderConnection = { - applyMutation: jest.fn(), - refresh: jest.fn(), - }; - const provider = GitlabDiscoveryEntityProvider.fromConfig(config, { - logger, - schedule, - })[0]; + const schedule = new PersistingTaskRunner(); + const entityProviderConnection: EntityProviderConnection = { + applyMutation: jest.fn(), + refresh: jest.fn(), + }; + const provider = GitlabDiscoveryEntityProvider.fromConfig(config, { + logger, + schedule, + })[0]; - await provider.connect(entityProviderConnection); + await provider.connect(entityProviderConnection); - await provider.refresh(logger); + await provider.refresh(logger); - expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ - type: 'full', - entities: mock.expected_location_entities_default_branch.filter( - entity => - entity.entity.metadata.annotations[ - 'backstage.io/managed-by-location' - ].includes('/group1/') || - entity.entity.metadata.annotations[ - 'backstage.io/managed-by-location' - ].includes('/group2/'), - ), + expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ + type: 'full', + entities: mock.expected_location_entities_default_branch.filter( + entity => + entity.entity.metadata.annotations[ + 'backstage.io/managed-by-location' + ].includes('/group1/') || + entity.entity.metadata.annotations[ + 'backstage.io/managed-by-location' + ].includes('/group2/'), + ), + }); + + expect(entityProviderConnection.applyMutation).toHaveBeenCalledTimes(1); // Projects from both groups should be applied }); - expect(entityProviderConnection.applyMutation).toHaveBeenCalledTimes(1); // Projects from both groups should be applied -}); -it('should not create duplicate locations when multiple groupPatterns match the same group', async () => { - const config = new ConfigReader(mock.config_groupPatterns_duplicate_match); + it('should not create duplicate locations when multiple groupPatterns match the same group', async () => { + const config = new ConfigReader(mock.config_groupPatterns_duplicate_match); - const schedule = new PersistingTaskRunner(); - const entityProviderConnection: EntityProviderConnection = { - applyMutation: jest.fn(), - refresh: jest.fn(), - }; - const provider = GitlabDiscoveryEntityProvider.fromConfig(config, { - logger, - schedule, - })[0]; + const schedule = new PersistingTaskRunner(); + const entityProviderConnection: EntityProviderConnection = { + applyMutation: jest.fn(), + refresh: jest.fn(), + }; + const provider = GitlabDiscoveryEntityProvider.fromConfig(config, { + logger, + schedule, + })[0]; - await provider.connect(entityProviderConnection); + await provider.connect(entityProviderConnection); - await provider.refresh(logger); + await provider.refresh(logger); - expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ - type: 'full', - entities: mock.expected_location_entities_default_branch.filter(entity => - entity.entity.metadata.annotations[ - 'backstage.io/managed-by-location' - ].includes('/group1/'), - ), + expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ + type: 'full', + entities: mock.expected_location_entities_default_branch.filter(entity => + entity.entity.metadata.annotations[ + 'backstage.io/managed-by-location' + ].includes('/group1/'), + ), + }); + + expect(entityProviderConnection.applyMutation).toHaveBeenCalledTimes(1); }); - - expect(entityProviderConnection.applyMutation).toHaveBeenCalledTimes(1); }); diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts index 2428c17b62..08bd498615 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts @@ -233,11 +233,22 @@ export class GitlabDiscoveryEntityProvider implements EntityProvider { }; const groupToProcess = new Map(); + let groupFilters; - if ( - this.config.groupPatterns !== undefined && - this.config.groupPatterns.length > 0 - ) { + if (this.config.groupPattern !== undefined) { + const patterns = Array.isArray(this.config.groupPattern) + ? this.config.groupPattern + : [this.config.groupPattern]; + + if (patterns.length === 1 && patterns[0].source === '[\\s\\S]*') { + // if the pattern is a catch-all, we don't need to filter groups + groupFilters = new Array(); + } else { + groupFilters = patterns; + } + } + + if (groupFilters && groupFilters.length > 0) { const groups = paginated( options => this.gitLabClient.listGroups(options), { @@ -246,14 +257,12 @@ export class GitlabDiscoveryEntityProvider implements EntityProvider { }, ); - for (const pattern of this.config.groupPatterns) { - for await (const group of groups) { - if ( - pattern.test(group.full_path) && - !groupToProcess.has(group.full_path) - ) { - groupToProcess.set(group.full_path, group); - } + for await (const group of groups) { + if ( + groupFilters.some(groupFilter => groupFilter.test(group.full_path)) && + !groupToProcess.has(group.full_path) + ) { + groupToProcess.set(group.full_path, group); } } diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts index 3eec377e73..3b8720b5f2 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts @@ -111,6 +111,7 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider { private groupEntitiesTransformer: GroupEntitiesTransformer; private groupNameTransformer: GroupNameTransformer; private readonly gitLabClient: GitLabClient; + private readonly groupPatterns: RegExp[]; static fromConfig( config: Config, @@ -198,6 +199,10 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider { this.groupNameTransformer = options.groupNameTransformer ?? defaultGroupNameTransformer; + this.groupPatterns = Array.isArray(this.config.groupPattern) + ? this.config.groupPattern + : [this.config.groupPattern]; + this.gitLabClient = new GitLabClient({ config: this.integration.config, logger: this.logger, @@ -464,7 +469,6 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider { for await (const group of groups) { groupRes.scanned++; - if (!this.shouldProcessGroup(group)) { logger.debug(`Skipped group: ${group.full_path}`); continue; @@ -798,7 +802,7 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider { private shouldProcessGroup(group: GitLabGroup): boolean { return ( - this.config.groupPattern.test(group.full_path) && + this.groupPatterns.some(pattern => pattern.test(group.full_path)) && (!this.config.group || group.full_path.startsWith(`${this.config.group}/`) || group.full_path === this.config.group) 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 eb7f86a8cb..8790a3fe53 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts @@ -55,7 +55,6 @@ describe('config', () => { catalogFile: 'catalog-info.yaml', projectPattern: /[\s\S]*/, groupPattern: /[\s\S]*/, - groupPatterns: [], userPattern: /[\s\S]*/, orgEnabled: false, allowInherited: false, @@ -102,7 +101,6 @@ describe('config', () => { catalogFile: 'custom-file.yaml', projectPattern: /[\s\S]*/, groupPattern: /[\s\S]*/, - groupPatterns: [], userPattern: /[\s\S]*/, orgEnabled: false, allowInherited: false, @@ -149,7 +147,6 @@ describe('config', () => { catalogFile: 'custom-file.yaml', projectPattern: /[\s\S]*/, groupPattern: /[\s\S]*/, - groupPatterns: [], userPattern: /[\s\S]*/, orgEnabled: false, allowInherited: false, @@ -196,7 +193,6 @@ describe('config', () => { catalogFile: 'custom-file.yaml', projectPattern: /[\s\S]*/, groupPattern: /[\s\S]*/, - groupPatterns: [], userPattern: /[\s\S]*/, orgEnabled: false, allowInherited: false, @@ -244,7 +240,6 @@ describe('config', () => { catalogFile: 'custom-file.yaml', projectPattern: /[\s\S]*/, groupPattern: /[\s\S]*/, - groupPatterns: [], userPattern: /[\s\S]*/, orgEnabled: false, allowInherited: false, @@ -293,7 +288,6 @@ describe('config', () => { catalogFile: 'catalog-info.yaml', projectPattern: /[\s\S]*/, groupPattern: /[\s\S]*/, - groupPatterns: [], userPattern: /[\s\S]*/, orgEnabled: false, allowInherited: false, @@ -385,7 +379,6 @@ describe('config', () => { catalogFile: 'custom-file.yaml', projectPattern: /[\s\S]*/, groupPattern: /[\s\S]*/, - groupPatterns: [], userPattern: /[\s\S]*/, orgEnabled: false, allowInherited: false, @@ -432,7 +425,6 @@ describe('config', () => { catalogFile: 'custom-file.yaml', projectPattern: /[\s\S]*/, groupPattern: /[\s\S]*/, - groupPatterns: [], userPattern: /[\s\S]*/, orgEnabled: false, allowInherited: false, @@ -479,7 +471,6 @@ describe('config', () => { catalogFile: 'custom-file.yaml', projectPattern: /[\s\S]*/, groupPattern: /[\s\S]*/, - groupPatterns: [], userPattern: /[\s\S]*/, orgEnabled: false, allowInherited: false, @@ -526,7 +517,6 @@ describe('config', () => { catalogFile: 'custom-file.yaml', projectPattern: /[\s\S]*/, groupPattern: /[\s\S]*/, - groupPatterns: [], userPattern: /[\s\S]*/, orgEnabled: false, allowInherited: false, diff --git a/plugins/catalog-backend-module-gitlab/src/providers/config.ts b/plugins/catalog-backend-module-gitlab/src/providers/config.ts index 639a1e1177..f635d4c432 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/config.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/config.ts @@ -39,13 +39,21 @@ function readGitlabConfig(id: string, config: Config): GitlabProviderConfig { const userPattern = new RegExp( config.getOptionalString('userPattern') ?? /[\s\S]*/, ); - const groupPattern = new RegExp( - config.getOptionalString('groupPattern') ?? /[\s\S]*/, - ); - const groupPatterns: Array = - config - .getOptionalStringArray('groupPatterns') - ?.map(pattern => new RegExp(pattern)) ?? []; + + const configValue = config.getOptional('groupPattern'); + let groupPattern; + + if ((configValue && typeof configValue === 'string') || !configValue) { + groupPattern = new RegExp( + config.getOptionalString('groupPattern') ?? /[\s\S]*/, + ); + } else if (configValue && Array.isArray(configValue)) { + const configPattern = config.getOptionalStringArray('groupPattern') ?? []; + groupPattern = configPattern.map(pattern => new RegExp(pattern)); + } else { + groupPattern = new RegExp(/[\s\S]*/); + } + const orgEnabled: boolean = config.getOptionalBoolean('orgEnabled') ?? false; const allowInherited: boolean = config.getOptionalBoolean('allowInherited') ?? false; @@ -85,7 +93,6 @@ function readGitlabConfig(id: string, config: Config): GitlabProviderConfig { projectPattern, userPattern, groupPattern, - groupPatterns, schedule, orgEnabled, allowInherited,