Merge pull request #32184 from angeliski/update-internal-event-handler

This commit is contained in:
Fredrik Adelöw
2026-01-07 19:46:29 +01:00
committed by GitHub
3 changed files with 102 additions and 232 deletions
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-catalog-backend-module-github': patch
---
Improved efficiency of `GithubOrgEntityProvider` membership event handling and edit team. The provider now fetches only the specific user's teams instead of all organization users when processing membership events, and uses `addEntitiesOperation` instead of `replaceEntitiesOperation` to avoid unnecessary entity deletions.
@@ -840,6 +840,9 @@ describe('GithubOrgEntityProvider', () => {
name: {
from: 'mygroup with spaces',
},
description: {
from: 'old description',
},
},
team: {
node_id: 'xpto',
@@ -956,21 +959,21 @@ describe('GithubOrgEntityProvider', () => {
'url:https://github.com/orgs/backstage/teams/mygroup-with-spaces',
'backstage.io/managed-by-origin-location':
'url:https://github.com/orgs/backstage/teams/mygroup-with-spaces',
'github.com/team-slug': 'backstage/mygroup-with-spaces',
},
name: 'mygroup-with-spaces',
description: 'description-from-the-new-team',
description: 'old description',
},
apiVersion: 'backstage.io/v1alpha1',
kind: 'Group',
spec: {
type: 'team',
profile: {
displayName: 'Team',
picture: 'http://example.com/team.jpeg',
displayName: 'mygroup with spaces',
},
children: [],
parent: 'parent',
members: ['a', 'githubuser'],
parent: 'father-team',
members: [],
},
},
},
@@ -1012,6 +1015,7 @@ describe('GithubOrgEntityProvider', () => {
const mockClient = jest.fn();
mockClient
// getOrganizationTeam
.mockResolvedValueOnce({
organization: {
team: {
@@ -1032,31 +1036,7 @@ describe('GithubOrgEntityProvider', () => {
},
},
})
.mockResolvedValueOnce({
organization: {
membersWithRole: {
pageInfo: { hasNextPage: false },
nodes: [
{
login: 'a',
id: 'f',
name: 'b',
bio: 'c',
email: 'd',
avatarUrl: 'e',
},
{
login: 'githubuser',
id: 'githubuserId',
name: 'githubuser',
bio: 'githubuser',
email: 'd',
avatarUrl: 'e',
},
],
},
},
})
// getOrganizationTeamsForUser
.mockResolvedValueOnce({
organization: {
teams: {
@@ -1070,12 +1050,6 @@ describe('GithubOrgEntityProvider', () => {
avatarUrl: 'http://example.com/team.jpeg',
parentTeam: {
slug: 'parent',
combinedSlug: '',
members: { pageInfo: { hasNextPage: false }, nodes: [] },
},
members: {
pageInfo: { hasNextPage: false },
nodes: [{ login: 'a' }, { login: 'githubuser' }],
},
},
],
@@ -1101,6 +1075,10 @@ describe('GithubOrgEntityProvider', () => {
},
member: {
login: 'githubuser',
avatar_url: 'e',
email: 'd',
name: 'githubuser',
node_id: 'githubuserId',
},
organization: {
login: 'test-org',
@@ -1114,33 +1092,6 @@ describe('GithubOrgEntityProvider', () => {
expect(entityProviderConnection.applyMutation).toHaveBeenCalledTimes(1);
expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({
added: [
{
locationKey: 'github-org-provider:my-id',
entity: {
metadata: {
annotations: {
'backstage.io/managed-by-location':
'url:https://github.com/githubuser',
'backstage.io/managed-by-origin-location':
'url:https://github.com/githubuser',
'github.com/user-login': 'githubuser',
'github.com/user-id': 'githubuserId',
},
name: 'githubuser',
description: 'githubuser',
},
apiVersion: 'backstage.io/v1alpha1',
kind: 'User',
spec: {
profile: {
displayName: 'githubuser',
email: 'd',
picture: 'e',
},
memberOf: ['team'],
},
},
},
{
locationKey: 'github-org-provider:my-id',
entity: {
@@ -1169,8 +1120,6 @@ describe('GithubOrgEntityProvider', () => {
},
},
},
],
removed: [
{
locationKey: 'github-org-provider:my-id',
entity: {
@@ -1184,7 +1133,6 @@ describe('GithubOrgEntityProvider', () => {
'github.com/user-id': 'githubuserId',
},
name: 'githubuser',
description: 'githubuser',
},
apiVersion: 'backstage.io/v1alpha1',
kind: 'User',
@@ -1198,35 +1146,8 @@ describe('GithubOrgEntityProvider', () => {
},
},
},
{
locationKey: 'github-org-provider:my-id',
entity: {
metadata: {
annotations: {
'backstage.io/managed-by-location':
'url:https://github.com/orgs/backstage/teams/team',
'backstage.io/managed-by-origin-location':
'url:https://github.com/orgs/backstage/teams/team',
'github.com/team-slug': 'blah/team',
},
name: 'team',
description: 'The one and only team',
},
apiVersion: 'backstage.io/v1alpha1',
kind: 'Group',
spec: {
type: 'team',
profile: {
displayName: 'Team',
picture: 'http://example.com/team.jpeg',
},
children: [],
parent: 'parent',
members: ['a', 'githubuser'],
},
},
},
],
removed: [],
type: 'delta',
});
});
@@ -1264,6 +1185,7 @@ describe('GithubOrgEntityProvider', () => {
const mockClient = jest.fn();
mockClient
// getOrganizationTeam
.mockResolvedValueOnce({
organization: {
team: {
@@ -1284,23 +1206,7 @@ describe('GithubOrgEntityProvider', () => {
},
},
})
.mockResolvedValueOnce({
organization: {
membersWithRole: {
pageInfo: { hasNextPage: false },
nodes: [
{
login: 'githubuser',
id: 'githubuserId',
name: 'githubuser',
bio: 'githubuser',
email: 'd',
avatarUrl: 'e',
},
],
},
},
})
// getOrganizationTeamsForUser - returns empty teams since user was removed
.mockResolvedValueOnce({
organization: {
teams: {
@@ -1328,6 +1234,10 @@ describe('GithubOrgEntityProvider', () => {
},
member: {
login: 'githubuser',
avatar_url: 'e',
email: 'd',
name: 'githubuser',
node_id: 'githubuserId',
},
organization: {
login: 'test-org',
@@ -1341,33 +1251,6 @@ describe('GithubOrgEntityProvider', () => {
expect(entityProviderConnection.applyMutation).toHaveBeenCalledTimes(1);
expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({
added: [
{
locationKey: 'github-org-provider:my-id',
entity: {
metadata: {
annotations: {
'backstage.io/managed-by-location':
'url:https://github.com/githubuser',
'backstage.io/managed-by-origin-location':
'url:https://github.com/githubuser',
'github.com/user-login': 'githubuser',
'github.com/user-id': 'githubuserId',
},
name: 'githubuser',
description: 'githubuser',
},
apiVersion: 'backstage.io/v1alpha1',
kind: 'User',
spec: {
profile: {
displayName: 'githubuser',
email: 'd',
picture: 'e',
},
memberOf: [],
},
},
},
{
locationKey: 'github-org-provider:my-id',
entity: {
@@ -1396,8 +1279,6 @@ describe('GithubOrgEntityProvider', () => {
},
},
},
],
removed: [
{
locationKey: 'github-org-provider:my-id',
entity: {
@@ -1411,7 +1292,6 @@ describe('GithubOrgEntityProvider', () => {
'github.com/user-id': 'githubuserId',
},
name: 'githubuser',
description: 'githubuser',
},
apiVersion: 'backstage.io/v1alpha1',
kind: 'User',
@@ -1425,35 +1305,8 @@ describe('GithubOrgEntityProvider', () => {
},
},
},
{
locationKey: 'github-org-provider:my-id',
entity: {
metadata: {
annotations: {
'backstage.io/managed-by-location':
'url:https://github.com/orgs/backstage/teams/team',
'backstage.io/managed-by-origin-location':
'url:https://github.com/orgs/backstage/teams/team',
'github.com/team-slug': 'blah/team',
},
name: 'team',
description: 'The one and only team',
},
apiVersion: 'backstage.io/v1alpha1',
kind: 'Group',
spec: {
type: 'team',
profile: {
displayName: 'Team',
picture: 'http://example.com/team.jpeg',
},
children: [],
parent: 'parent',
members: ['a'],
},
},
},
],
removed: [],
type: 'delta',
});
});
@@ -18,7 +18,7 @@ import {
LoggerService,
SchedulerServiceTaskRunner,
} from '@backstage/backend-plugin-api';
import { Entity, isGroupEntity } from '@backstage/catalog-model';
import { Entity, isGroupEntity, isUserEntity } from '@backstage/catalog-model';
import { Config } from '@backstage/config';
import {
DefaultGithubCredentialsProvider,
@@ -52,18 +52,22 @@ import {
createAddEntitiesOperation,
createGraphqlClient,
createRemoveEntitiesOperation,
createReplaceEntitiesOperation,
DEFAULT_PAGE_SIZES,
DeferredEntitiesBuilder,
getOrganizationTeam,
getOrganizationTeams,
getOrganizationTeamsForUser,
getOrganizationTeamsFromUsers,
getOrganizationUsers,
GithubPageSizes,
GithubTeam,
} from '../lib/github';
import { areGroupEntities, areUserEntities } from '../lib/guards';
import { assignGroupsToUsers, buildOrgHierarchy } from '../lib/org';
import {
assignGroupsToUser,
assignGroupsToUsers,
buildOrgHierarchy,
} from '../lib/org';
import { parseGithubOrgUrl } from '../lib/util';
import { withLocations } from '../lib/withLocations';
@@ -311,11 +315,6 @@ export class GithubOrgEntityProvider implements EntityProvider {
this.options.gitHubConfig.host,
);
const replaceEntitiesOperation = createReplaceEntitiesOperation(
this.options.id,
this.options.gitHubConfig.host,
);
// handle change users in the org
// https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#organization
if (params.topic.includes('organization')) {
@@ -346,7 +345,8 @@ export class GithubOrgEntityProvider implements EntityProvider {
} else if (teamEvent.action === 'edited') {
await this.onTeamEditedInOrganization(
teamEvent,
replaceEntitiesOperation,
addEntitiesOperation,
removeEntitiesOperation,
);
}
}
@@ -355,9 +355,9 @@ export class GithubOrgEntityProvider implements EntityProvider {
// https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#membership
if (params.topic.includes('membership')) {
const membershipEvent = params.eventPayload as MembershipEvent;
this.onMembershipChangedInOrganization(
await this.onMembershipChangedInOrganization(
membershipEvent,
replaceEntitiesOperation,
addEntitiesOperation,
);
}
@@ -366,7 +366,8 @@ export class GithubOrgEntityProvider implements EntityProvider {
private async onTeamEditedInOrganization(
event: TeamEditedEvent,
createDeltaOperation: DeferredEntitiesBuilder,
addEntitiesOperation: DeferredEntitiesBuilder,
removeEntitiesOperation: DeferredEntitiesBuilder,
) {
if (!this.connection) {
throw new Error('Not initialized');
@@ -424,25 +425,33 @@ export class GithubOrgEntityProvider implements EntityProvider {
}
}
const oldName = event.changes.name?.from || event.team.name;
const teamTransformer =
this.options.teamTransformer || defaultOrganizationTeamTransformer;
const oldName = event.changes.name?.from || '';
const oldSlug = oldName.toLowerCase().replaceAll(/\s/gi, '-');
const oldDescription =
event.changes.description?.from || event.team.description;
const oldDescriptionSlug = oldDescription
?.toLowerCase()
.replaceAll(/\s/gi, '-');
const { removed } = createDeltaOperation(org, [
const oldGroup = (await teamTransformer(
{
...team,
metadata: {
name: oldSlug,
description: oldDescriptionSlug,
},
name: event.changes.name?.from,
slug: oldSlug,
combinedSlug: `${org}/${oldSlug}`,
description: event.changes.description?.from,
parentTeam: event.team?.parent?.slug
? ({ slug: event.team.parent.slug } as GithubTeam)
: undefined,
// entity will be removed
members: [],
},
]);
const { added } = createDeltaOperation(org, [...usersToRebuild, ...teams]);
{
org,
client,
query: '',
},
)) as Entity;
// Remove the old group entity in case the entity ref is now different
const { removed } = removeEntitiesOperation(org, [oldGroup]);
const { added } = addEntitiesOperation(org, [...usersToRebuild, team]);
await this.connection.applyMutation({
type: 'delta',
removed,
@@ -467,11 +476,9 @@ export class GithubOrgEntityProvider implements EntityProvider {
}
const teamSlug = event.team.slug;
const userLogin = event.member.login;
const { headers, type: tokenType } =
await this.credentialsProvider.getCredentials({
url: this.options.orgUrl,
});
const { headers } = await this.credentialsProvider.getCredentials({
url: this.options.orgUrl,
});
const client = graphql.defaults({
baseUrl: this.options.gitHubConfig.apiBaseUrl,
headers,
@@ -484,43 +491,48 @@ export class GithubOrgEntityProvider implements EntityProvider {
org,
teamSlug,
this.options.teamTransformer,
);
const { users } = await getOrganizationUsers(
client,
org,
tokenType,
this.options.userTransformer,
pageSizes,
this.options.excludeSuspendedUsers,
);
const usersToRebuild = users.filter(u => u.metadata.name === userLogin);
const { teams } = await getOrganizationTeamsFromUsers(
client,
org,
[userLogin],
this.options.teamTransformer,
pageSizes,
);
// we include group because the removed event need to update the old group too
if (!teams.some(t => t.metadata.name === team.metadata.name)) {
teams.push(team);
}
const userTransformer =
this.options.userTransformer || defaultUserTransformer;
const { name, avatar_url: avatarUrl, email, login, node_id } = event.member;
const user = await userTransformer(
{
name,
avatarUrl,
login,
email: email ?? undefined,
id: node_id,
},
{
org,
client,
query: '',
},
);
if (areGroupEntities(teams)) {
buildOrgHierarchy(teams);
if (areUserEntities(usersToRebuild)) {
assignGroupsToUsers(usersToRebuild, teams);
const mutationEntities: Entity[] = [team];
if (user && isUserEntity(user)) {
const teamTransformer =
this.options.teamTransformer || defaultOrganizationTeamTransformer;
const { teams } = await getOrganizationTeamsForUser(
client,
org,
login,
teamTransformer,
pageSizes,
);
if (areGroupEntities(teams)) {
assignGroupsToUser(user, teams);
}
mutationEntities.push(user);
}
const { added, removed } = createDeltaOperation(org, [
...usersToRebuild,
...teams,
]);
const { added, removed } = createDeltaOperation(org, mutationEntities);
await this.connection.applyMutation({
type: 'delta',
removed,