From 9818112d12a3668579903f6c79eaccc3b07b43ca Mon Sep 17 00:00:00 2001 From: Chris Trombley Date: Thu, 14 Apr 2022 10:06:01 -0700 Subject: [PATCH 1/3] Allow definition of required status checks for new github repos Signed-off-by: Chris Trombley --- .changeset/curly-parrots-applaud.md | 5 ++ .../src/scaffolder/actions/builtin/helpers.ts | 7 ++- .../actions/builtin/publish/github.test.ts | 63 +++++++++++++++++++ .../actions/builtin/publish/github.ts | 12 ++++ 4 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 .changeset/curly-parrots-applaud.md diff --git a/.changeset/curly-parrots-applaud.md b/.changeset/curly-parrots-applaud.md new file mode 100644 index 0000000000..ddfd093cff --- /dev/null +++ b/.changeset/curly-parrots-applaud.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +Update the github:publish action to allow passing required status checks by name. diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts index 75e22522d5..a6e65a169e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts @@ -132,6 +132,7 @@ type BranchProtectionOptions = { repoName: string; logger: Logger; requireCodeOwnerReviews: boolean; + requiredStatusChecks: string[]; defaultBranch?: string; }; @@ -141,6 +142,7 @@ export const enableBranchProtectionOnDefaultRepoBranch = async ({ owner, logger, requireCodeOwnerReviews, + requiredStatusChecks = [], defaultBranch = 'master', }: BranchProtectionOptions): Promise => { const tryOnce = async () => { @@ -159,7 +161,10 @@ export const enableBranchProtectionOnDefaultRepoBranch = async ({ owner, repo: repoName, branch: defaultBranch, - required_status_checks: { strict: true, contexts: [] }, + required_status_checks: { + strict: true, + contexts: requiredStatusChecks, + }, restrictions: null, enforce_admins: true, required_pull_request_reviews: { 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 4e835657a3..80a3f77006 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 @@ -625,6 +625,7 @@ describe('publish:github', () => { logger: mockContext.logger, defaultBranch: 'master', requireCodeOwnerReviews: false, + requiredStatusChecks: [], }); await action.handler({ @@ -642,6 +643,7 @@ describe('publish:github', () => { logger: mockContext.logger, defaultBranch: 'master', requireCodeOwnerReviews: true, + requiredStatusChecks: [], }); await action.handler({ @@ -659,6 +661,67 @@ describe('publish:github', () => { logger: mockContext.logger, defaultBranch: 'master', requireCodeOwnerReviews: false, + requiredStatusChecks: [], + }); + }); + + it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of requireStatusChecks', async () => { + mockOctokit.rest.users.getByUsername.mockResolvedValue({ + data: { type: 'User' }, + }); + + mockOctokit.rest.repos.createForAuthenticatedUser.mockResolvedValue({ + data: { + name: 'repository', + }, + }); + + await action.handler(mockContext); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repository', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + requiredStatusChecks: [], + }); + + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + requiredStatusChecks: ['statusCheck'], + }, + }); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repository', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + requiredStatusChecks: ['statusCheck'], + }); + + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + requiredStatusChecks: [], + }, + }); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repository', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + requiredStatusChecks: [], }); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts index 7f1e7a1c93..0ef71bc6fa 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts @@ -52,6 +52,7 @@ export function createPublishGithubAction(options: { allowMergeCommit?: boolean; sourcePath?: string; requireCodeOwnerReviews?: boolean; + requiredStatusChecks?: string[]; repoVisibility?: 'private' | 'internal' | 'public'; collaborators?: Array<{ username: string; @@ -88,6 +89,15 @@ export function createPublishGithubAction(options: { 'Require an approved review in PR including files with a designated Code Owner', type: 'boolean', }, + requiredStatusChecks: { + title: 'Required Status Checks', + description: + 'The list of status checks to require in order to merge into this branch', + type: 'array', + items: { + type: 'string', + }, + }, repoVisibility: { title: 'Repository Visibility', type: 'string', @@ -178,6 +188,7 @@ export function createPublishGithubAction(options: { description, access, requireCodeOwnerReviews = false, + requiredStatusChecks = [], repoVisibility = 'private', defaultBranch = 'master', deleteBranchOnMerge = false, @@ -317,6 +328,7 @@ export function createPublishGithubAction(options: { logger: ctx.logger, defaultBranch, requireCodeOwnerReviews, + requiredStatusChecks, }); } catch (e) { assertError(e); From 8d624a7fd3dea7742be12b1db94fe8626b2fc311 Mon Sep 17 00:00:00 2001 From: Chris Trombley Date: Tue, 19 Apr 2022 08:48:22 -0700 Subject: [PATCH 2/3] refactor: make BranchProtectionOoptions.requiredStatusCheckContexts optional Signed-off-by: Chris Trombley --- .changeset/curly-parrots-applaud.md | 3 ++- .../src/scaffolder/actions/builtin/helpers.ts | 6 +++--- .../actions/builtin/publish/github.test.ts | 18 +++++++++--------- .../actions/builtin/publish/github.ts | 10 +++++----- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/.changeset/curly-parrots-applaud.md b/.changeset/curly-parrots-applaud.md index ddfd093cff..d9443415eb 100644 --- a/.changeset/curly-parrots-applaud.md +++ b/.changeset/curly-parrots-applaud.md @@ -2,4 +2,5 @@ '@backstage/plugin-scaffolder-backend': minor --- -Update the github:publish action to allow passing required status checks by name. +Update the `github:publish` action to allow passing required status check +contexts before merging to the main branch. diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts index a6e65a169e..529a504cba 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts @@ -132,7 +132,7 @@ type BranchProtectionOptions = { repoName: string; logger: Logger; requireCodeOwnerReviews: boolean; - requiredStatusChecks: string[]; + requiredStatusCheckContexts?: string[]; defaultBranch?: string; }; @@ -142,7 +142,7 @@ export const enableBranchProtectionOnDefaultRepoBranch = async ({ owner, logger, requireCodeOwnerReviews, - requiredStatusChecks = [], + requiredStatusCheckContexts = [], defaultBranch = 'master', }: BranchProtectionOptions): Promise => { const tryOnce = async () => { @@ -163,7 +163,7 @@ export const enableBranchProtectionOnDefaultRepoBranch = async ({ branch: defaultBranch, required_status_checks: { strict: true, - contexts: requiredStatusChecks, + contexts: requiredStatusCheckContexts, }, restrictions: null, enforce_admins: true, 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 80a3f77006..186189b259 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 @@ -625,7 +625,7 @@ describe('publish:github', () => { logger: mockContext.logger, defaultBranch: 'master', requireCodeOwnerReviews: false, - requiredStatusChecks: [], + requiredStatusCheckContexts: [], }); await action.handler({ @@ -643,7 +643,7 @@ describe('publish:github', () => { logger: mockContext.logger, defaultBranch: 'master', requireCodeOwnerReviews: true, - requiredStatusChecks: [], + requiredStatusCheckContexts: [], }); await action.handler({ @@ -661,11 +661,11 @@ describe('publish:github', () => { logger: mockContext.logger, defaultBranch: 'master', requireCodeOwnerReviews: false, - requiredStatusChecks: [], + requiredStatusCheckContexts: [], }); }); - it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of requireStatusChecks', async () => { + it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of requiredStatusCheckContexts', async () => { mockOctokit.rest.users.getByUsername.mockResolvedValue({ data: { type: 'User' }, }); @@ -685,14 +685,14 @@ describe('publish:github', () => { logger: mockContext.logger, defaultBranch: 'master', requireCodeOwnerReviews: false, - requiredStatusChecks: [], + requiredStatusCheckContexts: [], }); await action.handler({ ...mockContext, input: { ...mockContext.input, - requiredStatusChecks: ['statusCheck'], + requiredStatusCheckContexts: ['statusCheck'], }, }); @@ -703,14 +703,14 @@ describe('publish:github', () => { logger: mockContext.logger, defaultBranch: 'master', requireCodeOwnerReviews: false, - requiredStatusChecks: ['statusCheck'], + requiredStatusCheckContexts: ['statusCheck'], }); await action.handler({ ...mockContext, input: { ...mockContext.input, - requiredStatusChecks: [], + requiredStatusCheckContexts: [], }, }); @@ -721,7 +721,7 @@ describe('publish:github', () => { logger: mockContext.logger, defaultBranch: 'master', requireCodeOwnerReviews: false, - requiredStatusChecks: [], + requiredStatusCheckContexts: [], }); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts index 0ef71bc6fa..1d26206f75 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts @@ -52,7 +52,7 @@ export function createPublishGithubAction(options: { allowMergeCommit?: boolean; sourcePath?: string; requireCodeOwnerReviews?: boolean; - requiredStatusChecks?: string[]; + requiredStatusCheckContexts?: string[]; repoVisibility?: 'private' | 'internal' | 'public'; collaborators?: Array<{ username: string; @@ -89,8 +89,8 @@ export function createPublishGithubAction(options: { 'Require an approved review in PR including files with a designated Code Owner', type: 'boolean', }, - requiredStatusChecks: { - title: 'Required Status Checks', + requiredStatusCheckContexts: { + title: 'Required Status Check Contexts', description: 'The list of status checks to require in order to merge into this branch', type: 'array', @@ -188,7 +188,7 @@ export function createPublishGithubAction(options: { description, access, requireCodeOwnerReviews = false, - requiredStatusChecks = [], + requiredStatusCheckContexts = [], repoVisibility = 'private', defaultBranch = 'master', deleteBranchOnMerge = false, @@ -328,7 +328,7 @@ export function createPublishGithubAction(options: { logger: ctx.logger, defaultBranch, requireCodeOwnerReviews, - requiredStatusChecks, + requiredStatusCheckContexts, }); } catch (e) { assertError(e); From eb45134bf26b8f81226b032079261f2f39f887a0 Mon Sep 17 00:00:00 2001 From: Chris Trombley Date: Wed, 20 Apr 2022 08:56:38 -0700 Subject: [PATCH 3/3] chore: update api-report.md Signed-off-by: Chris Trombley --- plugins/scaffolder-backend/api-report.md | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 2c44004e5b..6ff44c7979 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -230,6 +230,7 @@ export function createPublishGithubAction(options: { allowMergeCommit?: boolean | undefined; sourcePath?: string | undefined; requireCodeOwnerReviews?: boolean | undefined; + requiredStatusCheckContexts?: string[] | undefined; repoVisibility?: 'internal' | 'private' | 'public' | undefined; collaborators?: | {