Revert change that introduce groupPatterns and reuse the config groupPattern insteand

Signed-off-by: Alexandre Fournier <vaceal@gmail.com>
This commit is contained in:
Alexandre Fournier
2025-08-17 18:38:18 -04:00
parent a715103301
commit e320e2e183
12 changed files with 140 additions and 129 deletions
+1 -1
View File
@@ -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.
+1 -1
View File
@@ -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`
+1 -1
View File
@@ -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.
*
@@ -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;
@@ -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' }));
}),
@@ -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[] = [
@@ -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.
@@ -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);
});
@@ -233,11 +233,22 @@ export class GitlabDiscoveryEntityProvider implements EntityProvider {
};
const groupToProcess = new Map<string, GitLabGroup>();
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<RegExp>();
} else {
groupFilters = patterns;
}
}
if (groupFilters && groupFilters.length > 0) {
const groups = paginated<GitLabGroup>(
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);
}
}
@@ -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)
@@ -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,
@@ -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<RegExp> =
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,