From fdef95557017e324edb468c21c79d77c0288d2d3 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 12 Sep 2020 21:27:26 +0100 Subject: [PATCH 1/6] scaffolder: Add support for granting access on create --- .../__mocks__/@octokit/rest/index.ts | 4 ++ .../scaffolder/stages/publish/github.test.ts | 49 ++++++++++++++++++- .../src/scaffolder/stages/publish/github.ts | 22 +++++++++ .../components/TemplatePage/TemplatePage.tsx | 7 ++- 4 files changed, 80 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/__mocks__/@octokit/rest/index.ts b/plugins/scaffolder-backend/src/scaffolder/__mocks__/@octokit/rest/index.ts index a302bc5931..e0e9efa479 100644 --- a/plugins/scaffolder-backend/src/scaffolder/__mocks__/@octokit/rest/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/__mocks__/@octokit/rest/index.ts @@ -18,10 +18,14 @@ export const mockGithubClient = { repos: { createInOrg: jest.fn(), createForAuthenticatedUser: jest.fn(), + addCollaborator: jest.fn(), }, users: { getByUsername: jest.fn(), }, + teams: { + addOrUpdateRepoPermissionsInOrg: jest.fn(), + }, }; export class Octokit { diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.test.ts index 718c01438c..86effd0ea5 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.test.ts @@ -30,6 +30,7 @@ const { mockGithubClient } = require('@octokit/rest') as { mockGithubClient: { repos: jest.Mocked; users: jest.Mocked; + teams: jest.Mocked; }; }; @@ -76,6 +77,7 @@ describe('GitHub Publisher', () => { values: { storePath: 'blam/test', owner: 'bob', + access: 'blam/team', }, directory: '/tmp/test', }); @@ -84,6 +86,15 @@ describe('GitHub Publisher', () => { org: 'blam', name: 'test', }); + expect( + mockGithubClient.teams.addOrUpdateRepoPermissionsInOrg, + ).toHaveBeenCalledWith({ + org: 'blam', + team_slug: 'team', + owner: 'blam', + repo: 'test', + permission: 'admin', + }); }); it('should use octokit to create a repo in the authed user if the organisation property is not set', async () => { @@ -102,6 +113,7 @@ describe('GitHub Publisher', () => { values: { storePath: 'blam/test', owner: 'bob', + access: 'blam', }, directory: '/tmp/test', }); @@ -111,14 +123,49 @@ describe('GitHub Publisher', () => { ).toHaveBeenCalledWith({ name: 'test', }); + expect(mockGithubClient.repos.addCollaborator).not.toHaveBeenCalled(); + }); + }); + + it('should invite other user in the authed user', async () => { + mockGithubClient.repos.createForAuthenticatedUser.mockResolvedValue({ + data: { + clone_url: 'mockclone', + }, + } as OctokitResponse); + mockGithubClient.users.getByUsername.mockResolvedValue({ + data: { + type: 'User', + }, + } as OctokitResponse); + + await publisher.publish({ + values: { + storePath: 'blam/test', + owner: 'bob', + access: 'bob', + }, + directory: '/tmp/test', + }); + + expect( + mockGithubClient.repos.createForAuthenticatedUser, + ).toHaveBeenCalledWith({ + name: 'test', + }); + expect(mockGithubClient.repos.addCollaborator).toHaveBeenCalledWith({ + owner: 'blam', + repo: 'test', + username: 'bob', + permission: 'admin', }); }); describe('publish: createGitDirectory', () => { const values = { - isOrg: true, storePath: 'blam/test', owner: 'lols', + access: 'lols', }; const mockDir = '/tmp/test/dir'; diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts index f3306e3662..09cb720ebe 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts @@ -44,6 +44,7 @@ export class GithubPublisher implements PublisherBase { values: RequiredTemplateValues & Record, ) { const [owner, name] = values.storePath.split('/'); + const access = values.access as string; const user = await this.client.users.getByUsername({ username: owner }); @@ -54,6 +55,27 @@ export class GithubPublisher implements PublisherBase { const { data } = await repoCreationPromise; + if (access.match(new RegExp(`${owner}/.+`))) { + const [, team] = access.split('/'); + await this.client.teams.addOrUpdateRepoPermissionsInOrg({ + org: owner, + team_slug: team, + owner, + repo: name, + permission: 'admin', + }); + } else { + // no need to add access if it's the person who own's the personal account + if (access !== owner) { + await this.client.repos.addCollaborator({ + owner, + repo: name, + username: access, + permission: 'admin', + }); + } + } + return data?.clone_url; } diff --git a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx index 1ed0504894..60f74e4cbe 100644 --- a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx +++ b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx @@ -54,7 +54,7 @@ const useTemplate = ( const OWNER_REPO_SCHEMA = { $schema: 'http://json-schema.org/draft-07/schema#' as const, - required: ['storePath', 'owner'], + required: ['storePath', 'owner', 'access'], properties: { owner: { type: 'string' as const, @@ -67,6 +67,11 @@ const OWNER_REPO_SCHEMA = { title: 'Store path', description: 'GitHub store path in org/repo format', }, + access: { + type: 'string' as const, + title: 'Access', + description: 'Who should have access, in user or owner/team format', + }, }, }; From 7806decbc4e19f4338600695a177efdea24f1e87 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Wed, 16 Sep 2020 14:09:54 +0100 Subject: [PATCH 2/6] Make access optional --- .../src/scaffolder/stages/publish/github.ts | 4 ++-- .../scaffolder/src/components/TemplatePage/TemplatePage.tsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts index 09cb720ebe..b50d5730fb 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts @@ -55,7 +55,7 @@ export class GithubPublisher implements PublisherBase { const { data } = await repoCreationPromise; - if (access.match(new RegExp(`${owner}/.+`))) { + if (access && access.match(new RegExp(`${owner}/.+`))) { const [, team] = access.split('/'); await this.client.teams.addOrUpdateRepoPermissionsInOrg({ org: owner, @@ -66,7 +66,7 @@ export class GithubPublisher implements PublisherBase { }); } else { // no need to add access if it's the person who own's the personal account - if (access !== owner) { + if (access && access !== owner) { await this.client.repos.addCollaborator({ owner, repo: name, diff --git a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx index 60f74e4cbe..0f8ceb7a4e 100644 --- a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx +++ b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx @@ -54,7 +54,7 @@ const useTemplate = ( const OWNER_REPO_SCHEMA = { $schema: 'http://json-schema.org/draft-07/schema#' as const, - required: ['storePath', 'owner', 'access'], + required: ['storePath', 'owner'], properties: { owner: { type: 'string' as const, From a40012d974083eba26af6c8a1884681adef186e8 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Wed, 16 Sep 2020 14:14:46 +0100 Subject: [PATCH 3/6] Reword --- plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx index 0f8ceb7a4e..e68c795bd6 100644 --- a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx +++ b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx @@ -70,7 +70,7 @@ const OWNER_REPO_SCHEMA = { access: { type: 'string' as const, title: 'Access', - description: 'Who should have access, in user or owner/team format', + description: 'Who should have access, in org/team or user format', }, }, }; From 0c7acbe3cbc7f700fede8bad9ac057c08db2a052 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Wed, 16 Sep 2020 14:19:01 +0100 Subject: [PATCH 4/6] Update docs --- docs/features/software-templates/index.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/features/software-templates/index.md b/docs/features/software-templates/index.md index 86493c0120..645b498926 100644 --- a/docs/features/software-templates/index.md +++ b/docs/features/software-templates/index.md @@ -38,8 +38,10 @@ internally. After filling in these variables, you'll get some more fields to fill out which are required for backstage usage: the owner, (which is a `user` in the backstage -system), the `storePath` (which right now must be a GitHub Organisation), and a -non-existing github repository name in the format `organisation/reponame`. +system), the `storePath` (which right now must be a GitHub Organisation or +GitHub user), a non-existing github repository name in the format +`organisation/reponame`, and a GitHub team or user account which should be +granted admin access to the repository. ![Enter backstage vars](../../assets/software-templates/template-picked-2.png) From f93d7a15fc58fde39412a463d36384f9ddb33792 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Wed, 16 Sep 2020 14:36:52 +0100 Subject: [PATCH 5/6] Condense if to else if --- .../src/scaffolder/stages/publish/github.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts index b50d5730fb..ab1ffae80e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts @@ -64,16 +64,14 @@ export class GithubPublisher implements PublisherBase { repo: name, permission: 'admin', }); - } else { // no need to add access if it's the person who own's the personal account - if (access && access !== owner) { - await this.client.repos.addCollaborator({ - owner, - repo: name, - username: access, - permission: 'admin', - }); - } + } else if (access && access !== owner) { + await this.client.repos.addCollaborator({ + owner, + repo: name, + username: access, + permission: 'admin', + }); } return data?.clone_url; From 1b45fbc44289d0b06a68ae058eedc5a1983a4c71 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Thu, 17 Sep 2020 07:25:27 +0100 Subject: [PATCH 6/6] Simplify code --- .../src/scaffolder/stages/publish/github.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts index 57a1fca8d9..1d5c9e1947 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/publish/github.ts @@ -61,7 +61,6 @@ export class GithubPublisher implements PublisherBase { values: RequiredTemplateValues & Record, ) { const [owner, name] = values.storePath.split('/'); - const access = values.access as string; const user = await this.client.users.getByUsername({ username: owner }); @@ -80,7 +79,8 @@ export class GithubPublisher implements PublisherBase { const { data } = await repoCreationPromise; - if (access && access.match(new RegExp(`${owner}/.+`))) { + const access = values.access as string; + if (access?.startsWith(`${owner}/`)) { const [, team] = access.split('/'); await this.client.teams.addOrUpdateRepoPermissionsInOrg({ org: owner,