diff --git a/.changeset/ripe-comics-sip.md b/.changeset/ripe-comics-sip.md new file mode 100644 index 0000000000..b5fb1e7f2b --- /dev/null +++ b/.changeset/ripe-comics-sip.md @@ -0,0 +1,5 @@ +--- +'@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. 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.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 62b997d7e2..a1c6a17209 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); } } } @@ -799,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); } }