diff --git a/.changeset/serious-items-walk.md b/.changeset/serious-items-walk.md new file mode 100644 index 0000000000..c4cdfe34c4 --- /dev/null +++ b/.changeset/serious-items-walk.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +Stripped entity types and namespace before passing to GitHub API diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoCreate.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoCreate.test.ts index a9ef8c3daa..4f23839fca 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoCreate.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoCreate.test.ts @@ -28,6 +28,7 @@ import { import { when } from 'jest-when'; import { PassThrough } from 'stream'; import { createGithubRepoCreateAction } from './githubRepoCreate'; +import { entityRefToName } from '../helpers'; const mockOctokit = { rest: { @@ -83,15 +84,17 @@ describe('github:repo:create', () => { }; beforeEach(() => { - jest.resetAllMocks(); githubCredentialsProvider = DefaultGithubCredentialsProvider.fromIntegrations(integrations); action = createGithubRepoCreateAction({ integrations, githubCredentialsProvider, }); + (entityRefToName as jest.Mock).mockImplementation((s: string) => s); }); + afterEach(jest.resetAllMocks); + it('should call the githubApis with the correct values for createInOrg', async () => { mockOctokit.rest.users.getByUsername.mockResolvedValue({ data: { type: 'Organization' }, diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts index 75c8f25d8b..57be72bc73 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts @@ -29,6 +29,7 @@ import { initRepoAndPush, } from '../helpers'; import { getRepoSourceDirectory, parseRepoUrl } from '../publish/util'; +import { entityRefToName } from '../../builtin/helpers'; const DEFAULT_TIMEOUT_MS = 60_000; @@ -218,13 +219,13 @@ export async function createGithubRepoWithCollaboratorsAndTopics( await client.rest.repos.addCollaborator({ owner, repo, - username: collaborator.user, + username: entityRefToName(collaborator.user), permission: collaborator.access, }); } else if ('team' in collaborator) { await client.rest.teams.addOrUpdateRepoPermissionsInOrg({ org: owner, - team_slug: collaborator.team, + team_slug: entityRefToName(collaborator.team), owner, repo, permission: collaborator.access, diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.test.ts index b5a375d83d..239f7c7351 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.test.ts @@ -15,7 +15,7 @@ */ import { Git, getVoidLogger } from '@backstage/backend-common'; -import { commitAndPushRepo, initRepoAndPush } from './helpers'; +import { commitAndPushRepo, entityRefToName, initRepoAndPush } from './helpers'; jest.mock('@backstage/backend-common', () => ({ Git: { @@ -301,3 +301,17 @@ describe('commitAndPushRepo', () => { }); }); }); + +describe('entityRefToName', () => { + it.each([ + 'user:default/catpants', + 'group:default/catpants', + 'user:catpants', + 'default/catpants', + 'user:custom/catpants', + 'group:custom/catpants', + 'catpants', + ])('should parse: "%s"', (entityName: string) => { + expect(entityRefToName(entityName)).toEqual('catpants'); + }); +}); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts index 8bed279a1b..c1630d0bf7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts @@ -296,3 +296,7 @@ export function getGitCommitMessage( ? gitCommitMessage : config.getOptionalString('scaffolder.defaultCommitMessage'); } + +export function entityRefToName(name: string): string { + return name.replace(/^.*[:/]/g, ''); +} diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts index 94b51f3e94..8c6b02c1c3 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts @@ -29,6 +29,7 @@ import { when } from 'jest-when'; import { PassThrough } from 'stream'; import { enableBranchProtectionOnDefaultRepoBranch, + entityRefToName, initRepoAndPush, } from '../helpers'; import { createPublishGithubAction } from './github'; @@ -68,6 +69,8 @@ describe('publish:github', () => { }, }); + const { entityRefToName: realFamiliarizeEntityName } = + jest.requireActual('../helpers'); const integrations = ScmIntegrations.fromConfig(config); let githubCredentialsProvider: GithubCredentialsProvider; let action: TemplateAction; @@ -87,7 +90,6 @@ describe('publish:github', () => { }; beforeEach(() => { - jest.resetAllMocks(); githubCredentialsProvider = DefaultGithubCredentialsProvider.fromIntegrations(integrations); action = createPublishGithubAction({ @@ -95,8 +97,15 @@ describe('publish:github', () => { config, githubCredentialsProvider, }); + + // restore real implmentation + (entityRefToName as jest.Mock).mockImplementation( + realFamiliarizeEntityName, + ); }); + afterEach(jest.resetAllMocks); + it('should fail to create if the team is not found in the org', async () => { mockOctokit.rest.users.getByUsername.mockResolvedValue({ data: { type: 'Organization' }, @@ -651,6 +660,59 @@ describe('publish:github', () => { }); }); + it('should familiarize entity names while adding collaborators', async () => { + mockOctokit.rest.users.getByUsername.mockResolvedValue({ + data: { type: 'User' }, + }); + + mockOctokit.rest.repos.createForAuthenticatedUser.mockResolvedValue({ + data: { + clone_url: 'https://github.com/clone/url.git', + html_url: 'https://github.com/html/url', + }, + }); + + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + collaborators: [ + { + access: 'pull', + user: 'user:robot-1', + }, + { + access: 'push', + team: 'group:default/robot-2', + }, + ], + }, + }); + + const commonProperties = { + owner: 'owner', + repo: 'repo', + }; + + expect(mockOctokit.rest.repos.addCollaborator).toHaveBeenCalledWith({ + ...commonProperties, + username: 'robot-1', + permission: 'pull', + }); + + expect( + mockOctokit.rest.teams.addOrUpdateRepoPermissionsInOrg, + ).toHaveBeenCalledWith({ + ...commonProperties, + org: 'owner', + team_slug: 'robot-2', + permission: 'push', + }); + + expect(entityRefToName).toHaveBeenCalledWith('user:robot-1'); + expect(entityRefToName).toHaveBeenCalledWith('group:default/robot-2'); + }); + it('should ignore failures when adding multiple collaborators', async () => { mockOctokit.rest.users.getByUsername.mockResolvedValue({ data: { type: 'User' },