From d421b16559f943f92f5401d237bec1a7270d5783 Mon Sep 17 00:00:00 2001 From: Scott Guymer Date: Thu, 10 Jul 2025 10:43:13 +0200 Subject: [PATCH 1/3] Reduce API calls made for single user events in GitHub Org Provider Currently the provider will load all users group and all members of the groups when responding to an event on a single user. This can cause excessive API calls to page through group members when this information is just discarded in this instance. This change introduces single user versions of the code that will not page through all membership. Signed-off-by: Scott Guymer --- .changeset/ripe-comics-sip.md | 5 ++ .../src/lib/github.test.ts | 74 +++++++++++++++++++ .../src/lib/github.ts | 53 +++++++++++++ .../src/lib/index.ts | 6 +- .../src/lib/org.test.ts | 43 ++++++++++- .../src/lib/org.ts | 23 ++++++ .../providers/GithubMultiOrgEntityProvider.ts | 8 +- 7 files changed, 207 insertions(+), 5 deletions(-) create mode 100644 .changeset/ripe-comics-sip.md diff --git a/.changeset/ripe-comics-sip.md b/.changeset/ripe-comics-sip.md new file mode 100644 index 0000000000..0566ff1147 --- /dev/null +++ b/.changeset/ripe-comics-sip.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend-module-github': minor +--- + +This change introduces single user versions of the user group resolution code in the multi org provider that will not page through all membership when updating a single user. diff --git a/plugins/catalog-backend-module-github/src/lib/github.test.ts b/plugins/catalog-backend-module-github/src/lib/github.test.ts index ef52007f50..09eeba3cc0 100644 --- a/plugins/catalog-backend-module-github/src/lib/github.test.ts +++ b/plugins/catalog-backend-module-github/src/lib/github.test.ts @@ -36,6 +36,7 @@ import { createRemoveEntitiesOperation, createReplaceEntitiesOperation, createGraphqlClient, + getOrganizationTeamsForUser, } from './github'; import { Octokit } from '@octokit/core'; import { throttling } from '@octokit/plugin-throttling'; @@ -53,6 +54,79 @@ describe('github', () => { const graphql = graphqlOctokit.defaults({}); + describe('getOrganizationTeamsForUser', () => { + const org = 'my-org'; + const userLogin = 'testuser'; + + it('returns teams for a user', async () => { + server.use( + graphqlMsw.query('teams', () => + HttpResponse.json({ + data: { + organization: { + teams: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [ + { + slug: 'team1', + combinedSlug: 'my-org/team1', + name: 'Team 1', + description: 'desc', + avatarUrl: '', + editTeamUrl: '', + parentTeam: null, + }, + ], + }, + }, + }, + }), + ), + ); + + const mockTransformer = jest.fn().mockImplementation(async team => ({ + kind: 'Group', + metadata: { name: team.slug }, + })); + + const { teams } = await getOrganizationTeamsForUser( + graphql as any, + org, + userLogin, + mockTransformer as any, + ); + expect(Array.isArray(teams)).toBe(true); + expect(teams[0]).toEqual({ kind: 'Group', metadata: { name: 'team1' } }); + expect(mockTransformer).toHaveBeenCalled(); + }); + + it('returns an empty array if no teams found', async () => { + server.use( + graphqlMsw.query('teams', () => + HttpResponse.json({ + data: { + organization: { + teams: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [], + }, + }, + }, + }), + ), + ); + const mockTransformer = jest.fn().mockResolvedValue(undefined); + const { teams } = await getOrganizationTeamsForUser( + graphql as any, + org, + userLogin, + mockTransformer as any, + ); + expect(Array.isArray(teams)).toBe(true); + expect(teams.length).toBe(0); + }); + }); + describe('getOrganizationUsers using defaultUserMapper', () => { it('reads members', async () => { const input: QueryResponse = { diff --git a/plugins/catalog-backend-module-github/src/lib/github.ts b/plugins/catalog-backend-module-github/src/lib/github.ts index 6d29782bad..5d51dd35cd 100644 --- a/plugins/catalog-backend-module-github/src/lib/github.ts +++ b/plugins/catalog-backend-module-github/src/lib/github.ts @@ -346,6 +346,59 @@ export async function getOrganizationTeamsFromUsers( return { teams }; } +export async function getOrganizationTeamsForUser( + client: typeof graphql, + org: string, + userLogin: string, + teamTransformer: TeamTransformer, +): Promise<{ teams: Entity[] }> { + const query = ` + query teams($org: String!, $cursor: String, $userLogins: [String!] = "") { + organization(login: $org) { + teams(first: 100, after: $cursor, userLogins: $userLogins) { + pageInfo { + hasNextPage + endCursor + } + nodes { + slug + combinedSlug + name + description + avatarUrl + editTeamUrl + parentTeam { + slug + } + } + } + } +}`; + + const materialisedTeams = async ( + item: GithubTeamResponse, + ctx: TransformerContext, + ): Promise => { + const team: GithubTeam = { + ...item, + members: [{ login: userLogin }], + }; + + return await teamTransformer(team, ctx); + }; + + const teams = await queryWithPaging( + client, + query, + org, + r => r.organization?.teams, + materialisedTeams, + { org, userLogins: [userLogin] }, + ); + + return { teams }; +} + export async function getOrganizationsFromUser( client: typeof graphql, user: string, diff --git a/plugins/catalog-backend-module-github/src/lib/index.ts b/plugins/catalog-backend-module-github/src/lib/index.ts index 14f78fdeee..f5da0f1c2b 100644 --- a/plugins/catalog-backend-module-github/src/lib/index.ts +++ b/plugins/catalog-backend-module-github/src/lib/index.ts @@ -30,5 +30,9 @@ export { defaultOrganizationTeamTransformer, type TransformerContext, } from './defaultTransformers'; -export { assignGroupsToUsers, buildOrgHierarchy } from './org'; +export { + assignGroupsToUser, + assignGroupsToUsers, + buildOrgHierarchy, +} from './org'; export { parseGithubOrgUrl } from './util'; diff --git a/plugins/catalog-backend-module-github/src/lib/org.test.ts b/plugins/catalog-backend-module-github/src/lib/org.test.ts index 1cdc55a4da..a2e379adc7 100644 --- a/plugins/catalog-backend-module-github/src/lib/org.test.ts +++ b/plugins/catalog-backend-module-github/src/lib/org.test.ts @@ -15,7 +15,12 @@ */ import { GroupEntity, UserEntity } from '@backstage/catalog-model'; -import { assignGroupsToUsers, buildMemberOf, buildOrgHierarchy } from './org'; +import { + assignGroupsToUsers, + assignGroupsToUser, + buildMemberOf, + buildOrgHierarchy, +} from './org'; function u(name: string, memberOf: string[] = []): UserEntity { return { @@ -115,6 +120,42 @@ describe('assignGroupsToUsers', () => { }); }); +describe('assignGroupsToUser', () => { + it('should assign the correct groups to a user', () => { + const user: UserEntity = u('u1'); + const groups: GroupEntity[] = [ + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Group', + metadata: { name: 'g1' }, + spec: { type: 'team', children: [], members: ['default/u1'] }, + }, + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Group', + metadata: { name: 'g2' }, + spec: { type: 'team', children: [], members: ['u2'] }, + }, + ]; + assignGroupsToUser(user, groups); + expect(user.spec.memberOf).toEqual(['g1']); + }); + + it('should not assign any groups if user is not a member', () => { + const user: UserEntity = u('u3'); + const groups: GroupEntity[] = [ + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Group', + metadata: { name: 'g1' }, + spec: { type: 'team', children: [], members: ['u1'] }, + }, + ]; + assignGroupsToUser(user, groups); + expect(user.spec.memberOf).toEqual([]); + }); +}); + describe('buildMemberOf', () => { it('fills indirect member of groups', () => { const a = g('a', undefined, []); diff --git a/plugins/catalog-backend-module-github/src/lib/org.ts b/plugins/catalog-backend-module-github/src/lib/org.ts index 4336f2d49c..3a0a30e340 100644 --- a/plugins/catalog-backend-module-github/src/lib/org.ts +++ b/plugins/catalog-backend-module-github/src/lib/org.ts @@ -91,6 +91,29 @@ export function assignGroupsToUsers( } } +// Assign all relevant groups to a single user if the user is a member of each group. +export function assignGroupsToUser(user: UserEntity, groups: GroupEntity[]) { + const userRef = stringifyEntityRef(user); + for (const group of groups) { + const groupKey = + group.metadata.namespace && group.metadata.namespace !== DEFAULT_NAMESPACE + ? `${group.metadata.namespace}/${group.metadata.name}` + : group.metadata.name; + const memberRefs = + group.spec.members?.map(m => + stringifyEntityRef(parseEntityRef(m, { defaultKind: 'user' })), + ) || []; + if (memberRefs.includes(userRef)) { + if (!user.spec.memberOf) { + user.spec.memberOf = []; + } + if (!user.spec.memberOf.includes(groupKey)) { + user.spec.memberOf.push(groupKey); + } + } + } +} + // Ensure that users have their transitive group memberships. Requires that // the groups were previously processed with buildOrgHierarchy() export function buildMemberOf(groups: GroupEntity[], users: UserEntity[]) { diff --git a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts index 62b997d7e2..6a0ecd299a 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts @@ -60,6 +60,7 @@ import { defaultOrganizationTeamTransformer, defaultUserTransformer, getOrganizationTeams, + assignGroupsToUser, getOrganizationUsers, GithubTeam, TeamTransformer, @@ -73,6 +74,7 @@ import { import { getOrganizationsFromUser, getOrganizationTeam, + getOrganizationTeamsForUser, getOrganizationTeamsFromUsers, } from '../lib/github'; import { splitTeamSlug } from '../lib/util'; @@ -556,15 +558,15 @@ export class GithubMultiOrgEntityProvider implements EntityProvider { headers: orgHeaders, }); - const { teams } = await getOrganizationTeamsFromUsers( + const { teams } = await getOrganizationTeamsForUser( orgClient, userOrg, - [login], + login, this.defaultMultiOrgTeamTransformer.bind(this), ); if (isUserEntity(user) && areGroupEntities(teams)) { - assignGroupsToUsers([user], teams); + assignGroupsToUser(user, teams); } } } From c518ecb15b3b87c756669a9245b4703cde553034 Mon Sep 17 00:00:00 2001 From: Scott Guymer Date: Tue, 15 Jul 2025 17:21:23 +0200 Subject: [PATCH 2/3] Updated to patch change This is non breaking so only a patch change is needed Signed-off-by: Scott Guymer --- .changeset/ripe-comics-sip.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/ripe-comics-sip.md b/.changeset/ripe-comics-sip.md index 0566ff1147..b5fb1e7f2b 100644 --- a/.changeset/ripe-comics-sip.md +++ b/.changeset/ripe-comics-sip.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-catalog-backend-module-github': minor +'@backstage/plugin-catalog-backend-module-github': patch --- This change introduces single user versions of the user group resolution code in the multi org provider that will not page through all membership when updating a single user. From 412c4660f0ebab7c9988014d768501c753cc8a91 Mon Sep 17 00:00:00 2001 From: Scott Guymer Date: Tue, 22 Jul 2025 17:32:19 +0200 Subject: [PATCH 3/3] Apply to onMembershipChangedInTeam Signed-off-by: Scott Guymer --- .../GithubMultiOrgEntityProvider.test.ts | 18 +----------------- .../providers/GithubMultiOrgEntityProvider.ts | 6 +++--- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts index 2186b11f50..1aa1af46d7 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts @@ -2306,23 +2306,7 @@ describe('GithubMultiOrgEntityProvider', () => { organization: { teams: { pageInfo: { hasNextPage: false }, - nodes: [ - { - slug: 'team', - combinedSlug: 'orgA/team', - name: 'TeamA', - description: 'The one and only team', - avatarUrl: 'http://example.com/team.jpeg', - editTeamUrl: 'https://example.com', - parentTeam: { - slug: 'parent', - }, - members: { - pageInfo: { hasNextPage: false }, - nodes: [], - }, - }, - ], + nodes: [], }, }, }) diff --git a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts index 6a0ecd299a..a1c6a17209 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts @@ -801,15 +801,15 @@ export class GithubMultiOrgEntityProvider implements EntityProvider { headers: orgHeaders, }); - const { teams } = await getOrganizationTeamsFromUsers( + const { teams } = await getOrganizationTeamsForUser( orgClient, userOrg, - [login], + login, this.defaultMultiOrgTeamTransformer.bind(this), ); if (areGroupEntities(teams)) { - assignGroupsToUsers([user], teams); + assignGroupsToUser(user, teams); } }