Fix user ingestion for GitlabOrgDiscoveryEntityProvider for GitLab.com #18889

sbarrypoppulo · 2023-08-01T14:42:40Z

Hey, I just made a Pull Request!

Fixes an issue with user ingestion for GitLab.com by making the entity provider aware of the group config parameter and scoping the provider to this group. Previously, the Gitlab Org data entity provider attempted to fetch all users from Gitlab.com via the /users REST endpoint, which was undesirable.

Switched the user fetching for Gitlab.com to GraphQL API, scoped to the root Gitlab group for the Org, or via existing /users REST API for Gitlab Self-Managed.

Groups are fetched via GraphQL API for Gitlab.com and via existing /groups REST API for Gitlab Self-Managed.

Group members are fetched via existing GraphQL API for both instance types

Example provider configuration used to test with:

catalog:
  providers:
    gitlab:
      gitlabSaasOrgData:
        host: gitlab.com
        orgEnabled: true
        group: org/teams
        rules:
          - allow: [Group, User]

Relates to:

🚀 Feature: scope Gitlab Org Data integration to a single group #16025

✔️ Checklist

A changeset describing the change and affected packages. (more info)
Added or updated documentation
Tests for new functionality and regression tests for bug fixes
Screenshots attached (for UI changes)
All your commits have a Signed-off-by line in the message. (more info)

Signed-off-by: angom1 <angom@poppulo.com>

Signed-off-by: Stephen Barry <sbarry@poppulo.com>

… to SaaS full mutation test Signed-off-by: Stephen Barry <sbarry@poppulo.com>

Signed-off-by: Stephen Barry <sbarry@poppulo.com>

…sers-groups-ingestion

Signed-off-by: Stephen Barry <sbarry@poppulo.com>

backstage-goalie · 2023-08-01T14:43:15Z

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@backstage/plugin-catalog-backend-module-gitlab	plugins/catalog-backend-module-gitlab	minor	`v0.2.7-next.2`

github-actions · 2023-08-01T15:01:46Z

Uffizzi Preview deployment-33585 was deleted.

sbarrypoppulo · 2023-08-02T08:52:47Z

Tested on GitLab Self-Managed (16.1.1) without `group` parameter:

All User entities ingested from GitLab instance (all users have email on profile)
All Group entities ingested from GitLab instance

Tested on GitLab.com w/ `group` parameter configured

User entities ingested for users who are direct members of descendant GitLab groups of the configured group
Group entities ingested for descendant GitLab groups of the configured group

github-actions · 2023-08-09T16:18:32Z

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

sbarrypoppulo · 2023-08-10T06:31:26Z

@freben @alde Would appreciate some reviews here, and if you can remove the stale label 🙏

I'm not 100% sure on the changeset version for this change. My gut feeling is minor, but will await your feedback. Thanks!

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

Co-authored-by: Jamie Klassen <jklassen@vmware.com> Signed-off-by: sbarrypoppulo <101636916+sbarrypoppulo@users.noreply.github.com>

…endantGroupsResponse Signed-off-by: Stephen Barry <sbarry@poppulo.com>

…sers-groups-ingestion

sbarrypoppulo · 2023-08-22T09:44:34Z

Thanks once again for the review @jamieklassen and the very helpful suggesstions 💪

I think we're very close here now, if you can take another look and hopefully over to @alde / @freben

We're hoping we can get this in for 1.18 release, as it's preventing us from enabling sign-in resolvers for user identity, which is blocking various options and plugins on out adoption journey 🤞

jamieklassen

awesome

cthtrifork · 2023-08-24T15:29:34Z

Very nice work! Also hoping for a 1.18.x release!

mickfeech · 2023-08-24T15:47:06Z

Great work! Anxiously waiting for the merge to use it!

sbarrypoppulo · 2023-08-28T10:09:06Z

@freben Any chance of a review here? Seems a few folks are awaiting this fix 😄

freben · 2023-08-21T07:26:06Z

docs/integrations/gitlab/org.md

+order to limit the ingestion to a group within your organisation. `Group`
+entities will only be ingested for the configured group, or it's descendant groups,
+but not any ancestor groups higher than the configured group path. Only groups
+which contain members will be ingested.


Was this a conscious choice or "for technical reasons"? Seems like an unnecessary limitation.

As for the level within the hierarchy, it made sense that if you configure a group path, then the only groups ingested would be at that group level, or lower. Rather than in the other direction, higher up the hierarchy. Regarding ingestion of only groups with members, this was my understanding of how this always worked, but let me double check on that assumption.

freben · 2023-08-21T07:26:16Z

docs/integrations/gitlab/org.md

+
+For gitlab.com, when `orgEnabled: true`, the `group` parameter is required in
+order to limit the ingestion to a group within your organisation. `Group`
+entities will only be ingested for the configured group, or it's descendant groups,


Suggested change

entities will only be ingested for the configured group, or it's descendant groups,

entities will only be ingested for the configured group, or its descendant groups,

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

freben · 2023-08-29T08:02:10Z

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

@@ -136,16 +236,26 @@ export class GitLabClient {
        continue;
      }

-      memberIds.push(
-        ...response.data.group.groupMembers.nodes
-          .filter(n => n.user)


I want to call out the deletion of this one. I vaguely remember it being added for very good reason - it could be the case that the user was null sometimes in the response, for some corner cases. You may want to retain that check, right

Let me take another look here. This rings a bell, think I brought up this issue in #17735 and it slipped my mind

Hi @freben, would adding this check here work ?739b927

Almost. I think it needs to be group?.id and user.user?.id or so to not throw an error

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

Apply suggestion Co-authored-by: Fredrik Adelöw <freben@gmail.com> Signed-off-by: sbarrypoppulo <101636916+sbarrypoppulo@users.noreply.github.com>

Signed-off-by: angom1 <angom@poppulo.com>

freben · 2023-08-31T14:44:25Z

Should we hold off waiting for the publicEmail question, or is this ready to go from your POV?

Signed-off-by: angom1 <angom@poppulo.com>

sbarrypoppulo · 2023-09-01T13:25:45Z

Should we hold off waiting for the publicEmail question, or is this ready to go from your POV?

We've switched to commitEmail it's is similarly not guaranteed to be populated or returned from the GraphQL API, but seems like a more sane default attribute in hindsight

Further details in the comment here - #18889 (comment)

Signed-off-by: angom1 <angom@poppulo.com>

freben · 2023-09-12T10:15:26Z

See above about group?.id and user.user?.id - besides that I think we are ready to 🚢

Signed-off-by: angom1 <angom@poppulo.com>

github-actions · 2023-09-12T11:28:28Z

Thank you for contributing to Backstage! The changes in this pull request will be part of the 1.18.0 release, scheduled for Tue, 19 Sep 2023.

matteosilv · 2023-09-14T08:24:38Z

Greate work guys!!!

angom1 and others added 17 commits July 14, 2023 11:43

Add users and groups ingestion for gitlab.com

a0b80e7

Signed-off-by: angom1 <angom@poppulo.com>

Update error handling and documentation

d2f1349

Signed-off-by: angom1 <angom@poppulo.com>

Add test when group parameter is not set

1d2d469

Signed-off-by: angom1 <angom@poppulo.com>

Fix host and url

a446d9e

Signed-off-by: angom1 <angom@poppulo.com>

Add test for listSaasUsers function

8f8391b

Signed-off-by: angom1 <angom@poppulo.com>

fix key names

80a049c

Signed-off-by: angom1 <angom@poppulo.com>

Add tests for listSaasGroups

3be6da7

Signed-off-by: angom1 <angom@poppulo.com>

Fix name in test

79dbbc0

Signed-off-by: angom1 <angom@poppulo.com>

Add draft test

096e2c9

Signed-off-by: angom1 <angom@poppulo.com>

Update draft test

f0b82fc

Signed-off-by: angom1 <angom@poppulo.com>

Update client tests

f85f673

Signed-off-by: angom1 <angom@poppulo.com>

Switch from commitEmail to publicEmail for email attribute

0b56923

Signed-off-by: Stephen Barry <sbarry@poppulo.com>

Update tests after switch to publicEmail and add additional test data…

74f7fa6

… to SaaS full mutation test Signed-off-by: Stephen Barry <sbarry@poppulo.com>

Fetch users from root SaaS group

017e9e2

Signed-off-by: Stephen Barry <sbarry@poppulo.com>

Merge branch 'master' of github.com:backstage/backstage into GD-294-u…

a4d9b6a

…sers-groups-ingestion

Update Gitlab Org Data doc

86e2f78

Signed-off-by: Stephen Barry <sbarry@poppulo.com>

Add changeset

3d73baf

Signed-off-by: Stephen Barry <sbarry@poppulo.com>

sbarrypoppulo requested review from a team as code owners August 1, 2023 14:42

sbarrypoppulo requested review from freben and alde August 1, 2023 14:42

github-actions bot added documentation Improvements or additions to documentation area:catalog Related to the Catalog Project Area labels Aug 1, 2023

github-actions bot added the stale label Aug 9, 2023

github-actions bot removed the stale label Aug 10, 2023

jamieklassen reviewed Aug 22, 2023

View reviewed changes

plugins/catalog-backend-module-gitlab/src/lib/client.ts Outdated Show resolved Hide resolved

sbarrypoppulo and others added 3 commits August 22, 2023 08:26

Apply suggestions from code review

8bc5141

Co-authored-by: Jamie Klassen <jklassen@vmware.com> Signed-off-by: sbarrypoppulo <101636916+sbarrypoppulo@users.noreply.github.com>

Apply prettier lint and rename GitlabSaasGroupsResponse -> GitlabDesc…

880c08c

…endantGroupsResponse Signed-off-by: Stephen Barry <sbarry@poppulo.com>

Merge branch 'master' of github.com:backstage/backstage into GD-294-u…

68032af

…sers-groups-ingestion

sbarrypoppulo requested a review from jamieklassen August 22, 2023 09:44

jamieklassen approved these changes Aug 22, 2023

View reviewed changes

freben reviewed Aug 29, 2023

View reviewed changes

matteosilv reviewed Aug 29, 2023

View reviewed changes

plugins/catalog-backend-module-gitlab/src/lib/client.ts Outdated Show resolved Hide resolved

sbarrypoppulo and others added 2 commits August 29, 2023 10:53

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

d740314

Apply suggestion Co-authored-by: Fredrik Adelöw <freben@gmail.com> Signed-off-by: sbarrypoppulo <101636916+sbarrypoppulo@users.noreply.github.com>

Simplify for loops

69405c5

Signed-off-by: angom1 <angom@poppulo.com>

angom1 requested a review from a team as a code owner August 31, 2023 11:29

angom1 requested a review from tudi2d August 31, 2023 11:29

Switch to commitEmail

1d8b7a5

Signed-off-by: angom1 <angom@poppulo.com>

angom1 added 2 commits September 5, 2023 10:50

Add check for null response

739b927

Signed-off-by: angom1 <angom@poppulo.com>

Update query to commitEmail

0ccb00a

Signed-off-by: angom1 <angom@poppulo.com>

Update filter syntax

2c0ac5c

Signed-off-by: angom1 <angom@poppulo.com>

freben approved these changes Sep 12, 2023

View reviewed changes

freben merged commit 6b1a6e7 into backstage:master Sep 12, 2023
28 checks passed

vinzscam mentioned this pull request Sep 21, 2023

🚀 Feature: scope Gitlab Org Data integration to a single group #16025

Closed

2 tasks

freben mentioned this pull request Sep 25, 2023

email field and external uid annotation for GitlabOrgDiscoveryEntityProvider Enterprise users #20145

Merged

jamieklassen mentioned this pull request Sep 29, 2023

🐛 Bug Report: GitlabOrgDiscoveryEntityProvider only supports one group since 1.18.0 #20274

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix user ingestion for GitlabOrgDiscoveryEntityProvider for GitLab.com #18889

Fix user ingestion for GitlabOrgDiscoveryEntityProvider for GitLab.com #18889

sbarrypoppulo commented Aug 1, 2023 •

edited

backstage-goalie bot commented Aug 1, 2023 •

edited

github-actions bot commented Aug 1, 2023 •

edited

sbarrypoppulo commented Aug 2, 2023

github-actions bot commented Aug 9, 2023

sbarrypoppulo commented Aug 10, 2023 •

edited

sbarrypoppulo commented Aug 22, 2023

jamieklassen left a comment

cthtrifork commented Aug 24, 2023

mickfeech commented Aug 24, 2023

sbarrypoppulo commented Aug 28, 2023 •

edited

freben Aug 21, 2023

sbarrypoppulo Aug 29, 2023

freben Aug 21, 2023

freben Aug 29, 2023

sbarrypoppulo Aug 29, 2023

angom1 Sep 8, 2023

freben Sep 12, 2023

freben commented Aug 31, 2023

sbarrypoppulo commented Sep 1, 2023

freben commented Sep 12, 2023

github-actions bot commented Sep 12, 2023

matteosilv commented Sep 14, 2023

	entities will only be ingested for the configured group, or it's descendant groups,
	entities will only be ingested for the configured group, or its descendant groups,

Fix user ingestion for GitlabOrgDiscoveryEntityProvider for GitLab.com #18889

Fix user ingestion for GitlabOrgDiscoveryEntityProvider for GitLab.com #18889

Conversation

sbarrypoppulo commented Aug 1, 2023 • edited

Hey, I just made a Pull Request!

✔️ Checklist

backstage-goalie bot commented Aug 1, 2023 • edited

Changed Packages

github-actions bot commented Aug 1, 2023 • edited

sbarrypoppulo commented Aug 2, 2023

Tested on GitLab Self-Managed (16.1.1) without group parameter:

Tested on GitLab.com w/ group parameter configured

github-actions bot commented Aug 9, 2023

sbarrypoppulo commented Aug 10, 2023 • edited

sbarrypoppulo commented Aug 22, 2023

jamieklassen left a comment

Choose a reason for hiding this comment

cthtrifork commented Aug 24, 2023

mickfeech commented Aug 24, 2023

sbarrypoppulo commented Aug 28, 2023 • edited

freben Aug 21, 2023

Choose a reason for hiding this comment

sbarrypoppulo Aug 29, 2023

Choose a reason for hiding this comment

freben Aug 21, 2023

Choose a reason for hiding this comment

freben Aug 29, 2023

Choose a reason for hiding this comment

sbarrypoppulo Aug 29, 2023

Choose a reason for hiding this comment

angom1 Sep 8, 2023

Choose a reason for hiding this comment

freben Sep 12, 2023

Choose a reason for hiding this comment

freben commented Aug 31, 2023

sbarrypoppulo commented Sep 1, 2023

freben commented Sep 12, 2023

github-actions bot commented Sep 12, 2023

matteosilv commented Sep 14, 2023

sbarrypoppulo commented Aug 1, 2023 •

edited

backstage-goalie bot commented Aug 1, 2023 •

edited

github-actions bot commented Aug 1, 2023 •

edited

Tested on GitLab Self-Managed (16.1.1) without `group` parameter:

Tested on GitLab.com w/ `group` parameter configured

sbarrypoppulo commented Aug 10, 2023 •

edited

sbarrypoppulo commented Aug 28, 2023 •

edited