Merge pull request #27575 from hghtwr/26554

extend filter logic to include parent group

.changeset/smooth-bikes-sparkle.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend-module-gitlab': minor
+---
+
+Implemented discovery for top-level groups defined in config.group or if undefined global top-level group in Gitlab

docs/integrations/gitlab/org.md

@@ -170,7 +170,7 @@ catalog:
       yourProviderId:
         host: gitlab.com
         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)
+        group: org/teams # Required for gitlab.com when `orgEnabled: true`. Optional for self managed. Must not end with slash. Accepts only this group and groups under the provided path (which will be stripped)
         relations: # Optional
           - INHERITED # Optional. Members of any ancestor groups will also be considered members of the current group.
           - DESCENDANTS # Optional. Members of any descendant groups will also be considered members of the current group.
@@ -240,7 +240,7 @@ catalog:
       yourProviderId:
         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)
+        group: org/teams # Required for gitlab.com when `orgEnabled: true`. Optional for self managed. Must not end with slash. Accepts only this group and groups under the provided path (which will be stripped)
         restrictUsersToGroup: true # Optional: Backstage will ingest only users directly assigned to org/teams.
         includeUsersWithoutSeat: false # Optional: Set to true to include users without paid seat, only applicable for SaaS
 ```

plugins/catalog-backend-module-gitlab/src/__testUtils__/handlers.ts

@@ -136,6 +136,22 @@ const httpHandlers = [
 
 // dynamic handlers
 
+// https://docs.gitlab.com/ee/api/groups.html#list-group-details supports encoded path and id
+const httpGroupFindByEncodedPathDynamic = all_groups_response.flatMap(group => [
+  // Handler for apiBaseUrl
+  rest.get(`${apiBaseUrl}/groups/${group.full_path}`, (_, res, ctx) => {
+    return res(
+      ctx.json(all_groups_response.find(g => g.full_path === group.full_path)),
+    );
+  }),
+  // Handler for apiSaaSBaseUrl
+  rest.get(`${apiBaseUrlSaas}/groups/${group.full_path}`, (_, res, ctx) => {
+    return res(
+      ctx.json(all_groups_response.find(g => g.full_path === group.full_path)),
+    );
+  }),
+]);
+
 const httpGroupFindByIdDynamic = all_groups_response.map(group => {
   return rest.get(`${apiBaseUrl}/groups/${group.id}`, (_, res, ctx) => {
     return res(ctx.json(all_groups_response.find(g => g.id === group.id)));
@@ -618,4 +634,5 @@ export const handlers = [
   ...httpGroupListDescendantProjectsById,
   ...httpGroupListDescendantProjectsByName,
   ...graphqlHandlers,
+  ...httpGroupFindByEncodedPathDynamic,
 ];

plugins/catalog-backend-module-gitlab/src/__testUtils__/mocks.ts

@@ -1041,7 +1041,7 @@ export const all_groups_response: GitLabGroup[] = [
   {
     id: 1,
     name: 'group1',
-    description: '',
+    description: 'description1',
     full_path: 'group1',
   },
   {
@@ -1804,6 +1804,7 @@ export const expected_group_user_entity: MockObject[] = [
             'url:https://example.com/group1',
           'example.com/team-path': 'group1',
         },
+        description: 'description1',
         name: 'group1',
       },
       spec: {
@@ -2090,6 +2091,7 @@ export const expected_full_org_scan_entities: MockObject[] = [
             'url:https://example.com/group1',
           'example.com/team-path': 'group1',
         },
+        description: 'description1',
         name: 'group1',
       },
       spec: {
@@ -2347,7 +2349,7 @@ export const expected_full_members_group_org_scan_entities: MockObject[] = [
         name: 'JohnDoe',
       },
       spec: {
-        memberOf: ['subgroup1'],
+        memberOf: ['group1', 'subgroup1'], // since #26554 also config.group needs to be here.
         profile: {
           displayName: 'John Doe',
           email: 'john.doe@company.com',
@@ -2431,6 +2433,30 @@ export const expected_full_members_group_org_scan_entities: MockObject[] = [
     },
     locationKey: 'GitlabOrgDiscoveryEntityProvider:test-id',
   },
+  {
+    entity: {
+      apiVersion: 'backstage.io/v1alpha1',
+      kind: 'Group',
+      metadata: {
+        annotations: {
+          'backstage.io/managed-by-location': 'url:https://example.com/group1',
+          'backstage.io/managed-by-origin-location':
+            'url:https://example.com/group1',
+          'example.com/team-path': 'group1',
+        },
+        description: 'description1',
+        name: 'group1',
+      },
+      spec: {
+        children: [],
+        profile: {
+          displayName: 'group1',
+        },
+        type: 'team',
+      },
+    },
+    locationKey: 'GitlabOrgDiscoveryEntityProvider:test-id',
+  },
   {
     entity: {
       apiVersion: 'backstage.io/v1alpha1',
@@ -2472,7 +2498,7 @@ export const expected_group_members_group_org_scan_entities: MockObject[] = [
         name: 'JohnDoe',
       },
       spec: {
-        memberOf: ['subgroup1'],
+        memberOf: ['subgroup1', 'group1'],
         profile: {
           displayName: 'John Doe',
           email: 'john.doe@company.com',
@@ -2507,6 +2533,30 @@ export const expected_group_members_group_org_scan_entities: MockObject[] = [
     },
     locationKey: 'GitlabOrgDiscoveryEntityProvider:test-id',
   },
+  {
+    entity: {
+      apiVersion: 'backstage.io/v1alpha1',
+      kind: 'Group',
+      metadata: {
+        annotations: {
+          'backstage.io/managed-by-location': 'url:https://example.com/group1',
+          'backstage.io/managed-by-origin-location':
+            'url:https://example.com/group1',
+          'example.com/team-path': 'group1',
+        },
+        name: 'group1',
+        description: 'description1',
+      },
+      spec: {
+        children: [],
+        profile: {
+          displayName: 'group1',
+        },
+        type: 'team',
+      },
+    },
+    locationKey: 'GitlabOrgDiscoveryEntityProvider:test-id',
+  },
 ];
 
 export const all_self_hosted_group1_members: MockObject[] = [

plugins/catalog-backend-module-gitlab/src/lib/client.ts

@@ -165,6 +165,15 @@ export class GitLabClient {
     return this.pagedRequest(`/groups`, options);
   }
 
+  // https://docs.gitlab.com/ee/api/groups.html#list-group-details
+  // id can either be group id or encoded full path
+  async getGroupByPath(
+    groupPath: string,
+    options?: CommonListOptions,
+  ): Promise<GitLabGroup> {
+    return this.nonPagedRequest(`/groups/${groupPath}`, options);
+  }
+
   async listDescendantGroups(
     groupPath: string,
   ): Promise<PagedResponse<GitLabGroup>> {

plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.test.ts

@@ -435,7 +435,7 @@ describe('GitlabOrgDiscoveryEntityProvider - refresh', () => {
   });
 
   // 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
+  // All instance members, but only the group entities of config.group and below (#26554)
   it('Self-hosted: should get all instance users when restrictUsersToGroup is not set', async () => {
     const config = new ConfigReader(mock.config_org_group_selfHosted);
     const schedule = new PersistingTaskRunner();

plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts

@@ -366,6 +366,7 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider {
     if (this.gitLabClient.isSelfManaged() && this.config.restrictUsersToGroup) {
       groups = (await this.gitLabClient.listDescendantGroups(this.config.group))
         .items;
+      groups.push(await this.gitLabClient.getGroupByPath(this.config.group)); // adds the parent group for #26554
       users = paginated<GitLabUser>(
         options =>
           this.gitLabClient.listGroupMembers(this.config.group, options), // calls /groups/<groupId>/members
@@ -395,10 +396,14 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider {
     else {
       groups = (await this.gitLabClient.listDescendantGroups(this.config.group))
         .items;
+
+      groups.push(await this.gitLabClient.getGroupByPath(this.config.group)); // adds the parent group for #26554
+
       const rootGroupSplit = this.config.group.split('/');
       const rootGroup = this.config.restrictUsersToGroup
         ? rootGroupSplit[rootGroupSplit.length - 1]
         : rootGroupSplit[0];
+
       users = paginated<GitLabUser>(
         options =>
           this.gitLabClient.listSaaSUsers(
@@ -775,7 +780,8 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider {
     return (
       this.config.groupPattern.test(group.full_path) &&
       (!this.config.group ||
-        group.full_path.startsWith(`${this.config.group}/`))
+        group.full_path.startsWith(`${this.config.group}/`) ||
+        group.full_path === this.config.group)
     );
   }