From 06cc084b9d1671880a95844581ef064890b20345 Mon Sep 17 00:00:00 2001 From: Hghtwr Date: Fri, 13 Sep 2024 21:50:51 +0200 Subject: [PATCH 1/4] Add 'includeUsersWithoutSeat' flag to enable ingestion of non-paid users from Gitlab Signed-off-by: Hghtwr --- .changeset/spotty-jokes-boil.md | 5 + docs/integrations/gitlab/org.md | 5 +- .../api-report.md | 1 + .../src/__testUtils__/mocks.ts | 141 ++++++++++++++++++ .../src/lib/client.ts | 14 +- .../src/lib/types.ts | 7 + .../GitlabOrgDiscoveryEntityProvider.test.ts | 34 +++++ .../GitlabOrgDiscoveryEntityProvider.ts | 7 +- .../src/providers/config.test.ts | 6 + .../src/providers/config.ts | 4 + 10 files changed, 221 insertions(+), 3 deletions(-) create mode 100644 .changeset/spotty-jokes-boil.md diff --git a/.changeset/spotty-jokes-boil.md b/.changeset/spotty-jokes-boil.md new file mode 100644 index 0000000000..b401ef7148 --- /dev/null +++ b/.changeset/spotty-jokes-boil.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend-module-gitlab': patch +--- + +Added includeUsersWithoutSeat allows to import users without paid seat, e.g. for Gitlab Free on SaaS. Defaults to false diff --git a/docs/integrations/gitlab/org.md b/docs/integrations/gitlab/org.md index aa1b49b41c..1d9da336ce 100644 --- a/docs/integrations/gitlab/org.md +++ b/docs/integrations/gitlab/org.md @@ -231,6 +231,8 @@ of the top-level group for the configured group path will be ingested. In both cases (SaaS & self hosted), you can limit the ingested users to users directly assigned to the group defined in your `app-config.yaml` by setting the configuration key `restrictUsersToGroup: true`. This is especially useful when you have a large user base that you don't want to import by default. +On SaaS, you can choose to include users to be ingested that do not have a paid seat. This can be useful when using a free version of Gitlab, or when you use Guest Users on Gitlab Ultimate. Unfortunately, this will also lead to some technical users that might be imported into your user base. While project & group access tokens are filtered, service accounts will remain. [Learn more about Billable Users](https://docs.gitlab.com/ee/subscriptions/self_managed/index.html#billable-users). + ```yaml catalog: providers: @@ -239,7 +241,8 @@ catalog: host: gitlab.com ## Could also be self hosted. orgEnabled: true group: org/teams # Required for gitlab.com when `orgEnabled: true`. Optional for self managed. Must not end with slash. Accepts only groups under the provided path (which will be stripped) - restrictUsersToGroup: true # Backstage will ingest only users directly assigned to org/teams. + restrictUsersToGroup: true # Optional: Backstage will ingest only users directly assigned to org/teams. + includeUsersWithoutSeat: true # Optional: Include users without paid seat, only valid for SaaS ``` ### Limiting `User` and `Group` entity ingestion in the provider diff --git a/plugins/catalog-backend-module-gitlab/api-report.md b/plugins/catalog-backend-module-gitlab/api-report.md index 0e6f39cae4..b2c71c787b 100644 --- a/plugins/catalog-backend-module-gitlab/api-report.md +++ b/plugins/catalog-backend-module-gitlab/api-report.md @@ -112,6 +112,7 @@ export type GitlabProviderConfig = { schedule?: SchedulerServiceTaskScheduleDefinition; skipForkedRepos?: boolean; excludeRepos?: string[]; + includeUsersWithoutSeat?: boolean; }; // @public diff --git a/plugins/catalog-backend-module-gitlab/src/__testUtils__/mocks.ts b/plugins/catalog-backend-module-gitlab/src/__testUtils__/mocks.ts index 4a173c6b50..3087d10f11 100644 --- a/plugins/catalog-backend-module-gitlab/src/__testUtils__/mocks.ts +++ b/plugins/catalog-backend-module-gitlab/src/__testUtils__/mocks.ts @@ -696,6 +696,31 @@ export const config_org_group_restrictUsers_true_saas = { }, }; +export const config_org_group_includeUsersWithoutSeat_true_saas = { + integrations: { + gitlab: [ + { + host: 'gitlab.com', + apiBaseUrl: 'https://gitlab.com/api/v4', + token: '1234', + }, + ], + }, + catalog: { + providers: { + gitlab: { + 'test-id': { + host: 'gitlab.com', + group: 'group1', + orgEnabled: true, + skipForkedRepos: true, + includeUsersWithoutSeat: true, + }, + }, + }, + }, +}; + export const config_org_group_selfHosted = { integrations: { gitlab: [ @@ -937,6 +962,40 @@ export const all_saas_users_response: MockObject[] = [ is_using_seat: false, membership_state: 'active', }, + { + access_level: 50, + created_at: '2023-07-15T08:58:34.984Z', + expires_at: '2023-10-26', + id: 54, + username: 'project_100_bot_23dc8057bef66e05181f39be4652577c', + name: 'Token Bot', + state: 'active', + avatar_url: 'https://secure.gravatar.com/', + web_url: + 'https://gitlab.com/project_100_bot_23dc8057bef66e05181f39be4652577c', + group_saml_identity: null, + is_using_seat: false, + membership_state: 'active', + }, + { + access_level: 30, + created_at: '2023-07-19T08:58:34.984Z', + expires_at: null, + id: 34, + username: 'testuser3', + name: 'Test User 3', + state: 'active', + avatar_url: 'https://secure.gravatar.com/', + web_url: 'https://gitlab.com/testuser3', + email: 'testuser3@example.com', + group_saml_identity: { + provider: 'group_saml', + extern_uid: '53', + saml_provider_id: 1, + }, + is_using_seat: false, + membership_state: 'active', + }, ]; export const all_groups_response: GitLabGroup[] = [ @@ -2050,6 +2109,88 @@ export const expected_full_org_scan_entities_saas: MockObject[] = [ }, ]; +export const expected_full_org_scan_entities_includeUsersWithoutSeat_saas: MockObject[] = + [ + { + entity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'User', + metadata: { + annotations: { + 'backstage.io/managed-by-location': + 'url:https://gitlab.com/testuser1', + 'backstage.io/managed-by-origin-location': + 'url:https://gitlab.com/testuser1', + 'gitlab.com/user-login': 'https://gitlab.com/testuser1', + 'gitlab.com/saml-external-uid': '51', + }, + name: 'testuser1', + }, + spec: { + memberOf: [], + profile: { + displayName: 'Test User 1', + email: 'testuser1@example.com', + picture: 'https://secure.gravatar.com/', + }, + }, + }, + locationKey: 'GitlabOrgDiscoveryEntityProvider:test-id', + }, + { + entity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'User', + metadata: { + annotations: { + 'backstage.io/managed-by-location': + 'url:https://gitlab.com/testuser2', + 'backstage.io/managed-by-origin-location': + 'url:https://gitlab.com/testuser2', + 'gitlab.com/user-login': 'https://gitlab.com/testuser2', + 'gitlab.com/saml-external-uid': '52', + }, + name: 'testuser2', + }, + spec: { + memberOf: [], + profile: { + displayName: 'Test User 2', + email: 'testuser2@example.com', + picture: 'https://secure.gravatar.com/', + }, + }, + }, + locationKey: 'GitlabOrgDiscoveryEntityProvider:test-id', + }, + { + entity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'User', + metadata: { + annotations: { + 'backstage.io/managed-by-location': + 'url:https://gitlab.com/testuser3', + 'backstage.io/managed-by-origin-location': + 'url:https://gitlab.com/testuser3', + 'gitlab.com/user-login': 'https://gitlab.com/testuser3', + 'gitlab.com/saml-external-uid': '53', + }, + name: 'testuser3', + }, + spec: { + memberOf: [], + profile: { + displayName: 'Test User 3', + email: 'testuser3@example.com', + picture: 'https://secure.gravatar.com/', + }, + }, + }, + locationKey: 'GitlabOrgDiscoveryEntityProvider:test-id', + }, + ]; + export const subgroup_saas_users_response: MockObject[] = [ { access_level: 30, diff --git a/plugins/catalog-backend-module-gitlab/src/lib/client.ts b/plugins/catalog-backend-module-gitlab/src/lib/client.ts index b5a2b69131..401011f2dc 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/client.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/client.ts @@ -137,12 +137,24 @@ export class GitLabClient { async listSaaSUsers( groupPath: string, options?: CommonListOptions, + includeUsersWithoutSeat?: boolean | false, ): Promise> { return this.listGroupMembers(groupPath, { ...options, + active: true, // Users with seat are always active but for users without seat we need to filter show_seat_info: true, }).then(resp => { - resp.items = resp.items.filter(user => user.is_using_seat); + // Filter is optional to allow to import Gitlab Free users without seats + // https://github.com/backstage/backstage/issues/26438 + // Filter out API tokens https://docs.gitlab.com/ee/user/project/settings/project_access_tokens.html#bot-users-for-projects + if (includeUsersWithoutSeat) { + const regex = /^(?:project|group)_(\w+)_bot_(\w+)$/; + resp.items = resp.items.filter(user => { + return !regex.test(user.username); + }); + } else { + resp.items = resp.items.filter(user => user.is_using_seat); + } return resp; }); } diff --git a/plugins/catalog-backend-module-gitlab/src/lib/types.ts b/plugins/catalog-backend-module-gitlab/src/lib/types.ts index ffd33ca5d9..423f3ff8b5 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/types.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/types.ts @@ -237,6 +237,13 @@ export type GitlabProviderConfig = { * Paths should not start or end with a slash. */ excludeRepos?: string[]; + + /** + * If true, users without a seat will be included in the catalog. + * Group/Application Access Tokens are still filtered out but you might find service accounts or other users without a seat. + * Defaults to `false` + */ + includeUsersWithoutSeat?: boolean; }; /** diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.test.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.test.ts index a080948d65..a15894b236 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.test.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.test.ts @@ -400,6 +400,40 @@ describe('GitlabOrgDiscoveryEntityProvider - refresh', () => { }); }); + // This needs to return all users, including those without a seat but filter out the bot users + it('SaaS: should get users without a seat if includeUsersWithoutSeat true', async () => { + const config = new ConfigReader( + mock.config_org_group_includeUsersWithoutSeat_true_saas, + ); + const schedule = new PersistingTaskRunner(); + const entityProviderConnection: EntityProviderConnection = { + applyMutation: jest.fn(), + refresh: jest.fn(), + }; + const provider = GitlabOrgDiscoveryEntityProvider.fromConfig(config, { + logger, + schedule, + })[0]; + expect(provider.getProviderName()).toEqual( + 'GitlabOrgDiscoveryEntityProvider:test-id', + ); + + await provider.connect(entityProviderConnection); + + const taskDef = schedule.getTasks()[0]; + expect(taskDef.id).toEqual( + 'GitlabOrgDiscoveryEntityProvider:test-id:refresh', + ); + await (taskDef.fn as () => Promise)(); + + expect(entityProviderConnection.applyMutation).toHaveBeenCalledTimes(1); + expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ + type: 'full', + entities: + mock.expected_full_org_scan_entities_includeUsersWithoutSeat_saas, // + }); + }); + // This should return all members of the self-hosted instance regardless of the group set -> expected_full_members_group_org_scan_entities // All instance members, but only the group entities below the config.group it('Self-hosted: should get all instance users when restrictUsersToGroup is not set', async () => { diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts index d242f90e68..277a5a8c51 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts @@ -400,7 +400,12 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider { ? rootGroupSplit[rootGroupSplit.length - 1] : rootGroupSplit[0]; users = paginated( - options => this.gitLabClient.listSaaSUsers(rootGroup, options), + options => + this.gitLabClient.listSaaSUsers( + rootGroup, + options, + this.config.includeUsersWithoutSeat, + ), { page: 1, per_page: 100, 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 bb493278c9..4b7c73ec80 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/config.test.ts @@ -63,6 +63,7 @@ describe('config', () => { skipForkedRepos: false, excludeRepos: [], restrictUsersToGroup: false, + includeUsersWithoutSeat: false, }), ); }); @@ -78,6 +79,7 @@ describe('config', () => { branch: 'not-master', fallbackBranch: 'main', entityFilename: 'custom-file.yaml', + includeUsersWithoutSeat: true, }, }, }, @@ -104,6 +106,7 @@ describe('config', () => { skipForkedRepos: false, excludeRepos: [], restrictUsersToGroup: false, + includeUsersWithoutSeat: true, }), ); }); @@ -146,6 +149,7 @@ describe('config', () => { restrictUsersToGroup: false, excludeRepos: [], skipForkedRepos: true, + includeUsersWithoutSeat: false, }), ); }); @@ -189,6 +193,7 @@ describe('config', () => { restrictUsersToGroup: false, skipForkedRepos: false, excludeRepos: ['foo/bar', 'quz/qux'], + includeUsersWithoutSeat: false, }), ); }); @@ -232,6 +237,7 @@ describe('config', () => { skipForkedRepos: false, restrictUsersToGroup: false, excludeRepos: [], + includeUsersWithoutSeat: false, schedule: { frequency: { minutes: 30 }, timeout: { diff --git a/plugins/catalog-backend-module-gitlab/src/providers/config.ts b/plugins/catalog-backend-module-gitlab/src/providers/config.ts index f1883213da..dd1f6d67d7 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/config.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/config.ts @@ -60,6 +60,9 @@ function readGitlabConfig(id: string, config: Config): GitlabProviderConfig { const restrictUsersToGroup = config.getOptionalBoolean('restrictUsersToGroup') ?? false; + const includeUsersWithoutSeat = + config.getOptionalBoolean('includeUsersWithoutSeat') ?? false; + return { id, group, @@ -77,6 +80,7 @@ function readGitlabConfig(id: string, config: Config): GitlabProviderConfig { skipForkedRepos, excludeRepos, restrictUsersToGroup, + includeUsersWithoutSeat, }; } From 2c810e221f8fdd356275994d36b8ff76328eb7ec Mon Sep 17 00:00:00 2001 From: Hghtwr Date: Fri, 13 Sep 2024 22:02:46 +0200 Subject: [PATCH 2/4] remove unnecessary literal Signed-off-by: Hghtwr --- plugins/catalog-backend-module-gitlab/src/lib/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/catalog-backend-module-gitlab/src/lib/client.ts b/plugins/catalog-backend-module-gitlab/src/lib/client.ts index 401011f2dc..6ff39fc708 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/client.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/client.ts @@ -137,7 +137,7 @@ export class GitLabClient { async listSaaSUsers( groupPath: string, options?: CommonListOptions, - includeUsersWithoutSeat?: boolean | false, + includeUsersWithoutSeat?: boolean, ): Promise> { return this.listGroupMembers(groupPath, { ...options, From 77bfb298cd4e8e0fd44662cd80fde6af009c2f8a Mon Sep 17 00:00:00 2001 From: Hghtwr Date: Mon, 16 Sep 2024 18:57:57 +0200 Subject: [PATCH 3/4] fix: Improve docs clarity, move filterRegex out of pagedRequest loop Signed-off-by: Hghtwr --- docs/integrations/gitlab/org.md | 2 +- plugins/catalog-backend-module-gitlab/src/lib/client.ts | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/integrations/gitlab/org.md b/docs/integrations/gitlab/org.md index 1d9da336ce..5fa2e229eb 100644 --- a/docs/integrations/gitlab/org.md +++ b/docs/integrations/gitlab/org.md @@ -242,7 +242,7 @@ catalog: orgEnabled: true group: org/teams # Required for gitlab.com when `orgEnabled: true`. Optional for self managed. Must not end with slash. Accepts only groups under the provided path (which will be stripped) restrictUsersToGroup: true # Optional: Backstage will ingest only users directly assigned to org/teams. - includeUsersWithoutSeat: true # Optional: Include users without paid seat, only valid for SaaS + includeUsersWithoutSeat: false # Optional: Set to true to include users without paid seat, only applicable for SaaS ``` ### Limiting `User` and `Group` entity ingestion in the provider diff --git a/plugins/catalog-backend-module-gitlab/src/lib/client.ts b/plugins/catalog-backend-module-gitlab/src/lib/client.ts index 6ff39fc708..f2ce3d18f8 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/client.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/client.ts @@ -139,6 +139,8 @@ export class GitLabClient { options?: CommonListOptions, includeUsersWithoutSeat?: boolean, ): Promise> { + const botFilterRegex = /^(?:project|group)_(\w+)_bot_(\w+)$/; + return this.listGroupMembers(groupPath, { ...options, active: true, // Users with seat are always active but for users without seat we need to filter @@ -148,9 +150,8 @@ export class GitLabClient { // https://github.com/backstage/backstage/issues/26438 // Filter out API tokens https://docs.gitlab.com/ee/user/project/settings/project_access_tokens.html#bot-users-for-projects if (includeUsersWithoutSeat) { - const regex = /^(?:project|group)_(\w+)_bot_(\w+)$/; resp.items = resp.items.filter(user => { - return !regex.test(user.username); + return !botFilterRegex.test(user.username); }); } else { resp.items = resp.items.filter(user => user.is_using_seat); From 45376d66fe718f2acb5c36a2dbed24fa13bd6065 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Tue, 17 Sep 2024 10:46:56 +0200 Subject: [PATCH 4/4] Update .changeset/spotty-jokes-boil.md Signed-off-by: Johan Haals Signed-off-by: Johan Haals --- .changeset/spotty-jokes-boil.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/spotty-jokes-boil.md b/.changeset/spotty-jokes-boil.md index b401ef7148..e09f2d8d41 100644 --- a/.changeset/spotty-jokes-boil.md +++ b/.changeset/spotty-jokes-boil.md @@ -2,4 +2,4 @@ '@backstage/plugin-catalog-backend-module-gitlab': patch --- -Added includeUsersWithoutSeat allows to import users without paid seat, e.g. for Gitlab Free on SaaS. Defaults to false +Added a `includeUsersWithoutSeat` config option that allow import of users without a paid seat, e.g. for Gitlab Free on SaaS. Defaults to false