diff --git a/.changeset/weak-tomatoes-dance.md b/.changeset/weak-tomatoes-dance.md new file mode 100644 index 0000000000..446051e16f --- /dev/null +++ b/.changeset/weak-tomatoes-dance.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend-module-github': patch +--- + +Properly support custom `userTransformer` returning `undefined` in `GithubMultiOrgEntityProvider` 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 2622b2483d..504e5772dd 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts @@ -1699,31 +1699,6 @@ describe('GithubMultiOrgEntityProvider', () => { expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ type: 'delta', added: [ - { - entity: { - apiVersion: 'backstage.io/v1alpha1', - kind: 'User', - metadata: { - annotations: { - 'backstage.io/managed-by-location': - 'url:https://github.com/a', - 'backstage.io/managed-by-origin-location': - 'url:https://github.com/a', - 'github.com/user-login': 'a', - }, - name: 'a', - }, - spec: { - memberOf: ['orga/team', 'orgb/team'], - profile: { - displayName: 'a', - email: 'user1@test.com', - picture: 'https://avatars.githubusercontent.com/u/83820368', - }, - }, - }, - locationKey: 'github-multi-org-provider:my-id', - }, { entity: { apiVersion: 'backstage.io/v1alpha1', @@ -1754,6 +1729,31 @@ describe('GithubMultiOrgEntityProvider', () => { }, locationKey: 'github-multi-org-provider:my-id', }, + { + entity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'User', + metadata: { + annotations: { + 'backstage.io/managed-by-location': + 'url:https://github.com/a', + 'backstage.io/managed-by-origin-location': + 'url:https://github.com/a', + 'github.com/user-login': 'a', + }, + name: 'a', + }, + spec: { + memberOf: ['orga/team', 'orgb/team'], + profile: { + displayName: 'a', + email: 'user1@test.com', + picture: 'https://avatars.githubusercontent.com/u/83820368', + }, + }, + }, + locationKey: 'github-multi-org-provider:my-id', + }, ], removed: [], }); @@ -1871,31 +1871,6 @@ describe('GithubMultiOrgEntityProvider', () => { expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ type: 'delta', added: [ - { - entity: { - apiVersion: 'backstage.io/v1alpha1', - kind: 'User', - metadata: { - annotations: { - 'backstage.io/managed-by-location': - 'url:https://github.com/a', - 'backstage.io/managed-by-origin-location': - 'url:https://github.com/a', - 'github.com/user-login': 'a', - }, - name: 'a', - }, - spec: { - memberOf: ['orgb/team'], - profile: { - displayName: 'a', - email: 'user1@test.com', - picture: 'https://avatars.githubusercontent.com/u/83820368', - }, - }, - }, - locationKey: 'github-multi-org-provider:my-id', - }, { entity: { apiVersion: 'backstage.io/v1alpha1', @@ -1926,6 +1901,31 @@ describe('GithubMultiOrgEntityProvider', () => { }, locationKey: 'github-multi-org-provider:my-id', }, + { + entity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'User', + metadata: { + annotations: { + 'backstage.io/managed-by-location': + 'url:https://github.com/a', + 'backstage.io/managed-by-origin-location': + 'url:https://github.com/a', + 'github.com/user-login': 'a', + }, + name: 'a', + }, + spec: { + memberOf: ['orgb/team'], + profile: { + displayName: 'a', + email: 'user1@test.com', + picture: 'https://avatars.githubusercontent.com/u/83820368', + }, + }, + }, + locationKey: 'github-multi-org-provider:my-id', + }, ], removed: [], }); diff --git a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts index 65a622890d..7e15c4ce68 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts @@ -21,9 +21,9 @@ import { DEFAULT_NAMESPACE, Entity, isGroupEntity, + isUserEntity, parseEntityRef, stringifyEntityRef, - UserEntity, } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import { @@ -433,31 +433,33 @@ export class GithubMultiOrgEntityProvider implements EntityProvider { this.defaultMultiOrgTeamTransformer.bind(this), ); - // Fetch group memberships of users in case they already exist and - // have memberships in groups from other applicable orgs - for (const userOrg of applicableOrgs) { - const { headers: orgHeaders } = - await this.options.githubCredentialsProvider.getCredentials({ - url: `${this.options.githubUrl}/${userOrg}`, + if (users.length) { + // Fetch group memberships of users in case they already exist and + // have memberships in groups from other applicable orgs + for (const userOrg of applicableOrgs) { + const { headers: orgHeaders } = + await this.options.githubCredentialsProvider.getCredentials({ + url: `${this.options.githubUrl}/${userOrg}`, + }); + const orgClient = graphql.defaults({ + baseUrl: this.options.gitHubConfig.apiBaseUrl, + headers: orgHeaders, }); - const orgClient = graphql.defaults({ - baseUrl: this.options.gitHubConfig.apiBaseUrl, - headers: orgHeaders, - }); - const { teams: userTeams } = await getOrganizationTeamsFromUsers( - orgClient, - userOrg, - users.map( - u => - u.metadata.annotations?.[ANNOTATION_GITHUB_USER_LOGIN] || - u.metadata.name, - ), - this.defaultMultiOrgTeamTransformer.bind(this), - ); + const { teams: userTeams } = await getOrganizationTeamsFromUsers( + orgClient, + userOrg, + users.map( + u => + u.metadata.annotations?.[ANNOTATION_GITHUB_USER_LOGIN] || + u.metadata.name, + ), + this.defaultMultiOrgTeamTransformer.bind(this), + ); - if (areGroupEntities(userTeams) && areUserEntities(users)) { - assignGroupsToUsers(users, userTeams); + if (areGroupEntities(userTeams) && areUserEntities(users)) { + assignGroupsToUsers(users, userTeams); + } } } @@ -518,7 +520,7 @@ export class GithubMultiOrgEntityProvider implements EntityProvider { updateMemberships = true; } - const user = (await userTransformer( + const user = await userTransformer( { name, avatarUrl, @@ -530,7 +532,11 @@ export class GithubMultiOrgEntityProvider implements EntityProvider { client, query: '', }, - )) as UserEntity; + ); + + if (!user) { + return; + } if (updateMemberships) { for (const userOrg of userApplicableOrgs) { @@ -550,7 +556,7 @@ export class GithubMultiOrgEntityProvider implements EntityProvider { this.defaultMultiOrgTeamTransformer.bind(this), ); - if (areGroupEntities(teams)) { + if (isUserEntity(user) && areGroupEntities(teams)) { assignGroupsToUsers([user], teams); } } @@ -655,30 +661,32 @@ export class GithubMultiOrgEntityProvider implements EntityProvider { usersFromChangedGroup.includes(stringifyEntityRef(u)), ); - // Update memberships of associated members of this group in case the group entity ref changed - for (const userOrg of applicableOrgs) { - const { headers: orgHeaders } = - await this.options.githubCredentialsProvider.getCredentials({ - url: `${this.options.githubUrl}/${userOrg}`, + if (usersToRebuild.length) { + // Update memberships of associated members of this group in case the group entity ref changed + for (const userOrg of applicableOrgs) { + const { headers: orgHeaders } = + await this.options.githubCredentialsProvider.getCredentials({ + url: `${this.options.githubUrl}/${userOrg}`, + }); + const orgClient = graphql.defaults({ + baseUrl: this.options.gitHubConfig.apiBaseUrl, + headers: orgHeaders, }); - const orgClient = graphql.defaults({ - baseUrl: this.options.gitHubConfig.apiBaseUrl, - headers: orgHeaders, - }); - const { teams } = await getOrganizationTeamsFromUsers( - orgClient, - userOrg, - usersToRebuild.map( - u => - u.metadata.annotations?.[ANNOTATION_GITHUB_USER_LOGIN] || - u.metadata.name, - ), - this.defaultMultiOrgTeamTransformer.bind(this), - ); + const { teams } = await getOrganizationTeamsFromUsers( + orgClient, + userOrg, + usersToRebuild.map( + u => + u.metadata.annotations?.[ANNOTATION_GITHUB_USER_LOGIN] || + u.metadata.name, + ), + this.defaultMultiOrgTeamTransformer.bind(this), + ); - if (areGroupEntities(teams) && areUserEntities(usersToRebuild)) { - assignGroupsToUsers(usersToRebuild, teams); + if (areGroupEntities(teams) && areUserEntities(usersToRebuild)) { + assignGroupsToUsers(usersToRebuild, teams); + } } } @@ -751,7 +759,7 @@ export class GithubMultiOrgEntityProvider implements EntityProvider { const userTransformer = this.options.userTransformer || defaultUserTransformer; const { name, avatar_url: avatarUrl, email, login } = event.member; - const user = (await userTransformer( + const user = await userTransformer( { name, avatarUrl, @@ -763,33 +771,40 @@ export class GithubMultiOrgEntityProvider implements EntityProvider { client, query: '', }, - )) as UserEntity; + ); - const { orgs } = await getOrganizationsFromUser(client, login); - const userApplicableOrgs = orgs.filter(o => applicableOrgs.includes(o)); - for (const userOrg of userApplicableOrgs) { - const { headers: orgHeaders } = - await this.options.githubCredentialsProvider.getCredentials({ - url: `${this.options.githubUrl}/${userOrg}`, + const mutationEntities = [team]; + + if (user && isUserEntity(user)) { + const { orgs } = await getOrganizationsFromUser(client, login); + const userApplicableOrgs = orgs.filter(o => applicableOrgs.includes(o)); + for (const userOrg of userApplicableOrgs) { + const { headers: orgHeaders } = + await this.options.githubCredentialsProvider.getCredentials({ + url: `${this.options.githubUrl}/${userOrg}`, + }); + const orgClient = graphql.defaults({ + baseUrl: this.options.gitHubConfig.apiBaseUrl, + headers: orgHeaders, }); - const orgClient = graphql.defaults({ - baseUrl: this.options.gitHubConfig.apiBaseUrl, - headers: orgHeaders, - }); - const { teams } = await getOrganizationTeamsFromUsers( - orgClient, - userOrg, - [login], - this.defaultMultiOrgTeamTransformer.bind(this), - ); + const { teams } = await getOrganizationTeamsFromUsers( + orgClient, + userOrg, + [login], + this.defaultMultiOrgTeamTransformer.bind(this), + ); - if (areGroupEntities(teams)) { - assignGroupsToUsers([user], teams); + if (areGroupEntities(teams)) { + assignGroupsToUsers([user], teams); + } } + + mutationEntities.push(user); } - const { added, removed } = this.createAddEntitiesOperation([user, team]); + const { added, removed } = + this.createAddEntitiesOperation(mutationEntities); await this.connection.applyMutation({ type: 'delta', removed,