From 17ff77154c189eb50d77a35be8a9c4b170973a80 Mon Sep 17 00:00:00 2001 From: martincostello Date: Wed, 5 Oct 2022 13:55:53 +0100 Subject: [PATCH] feat: Allow configuring branch up-to-date-ness Allow users to configure whether GitHub branches from pull requests are required to be up to date when merging when configuring branch protections. Signed-off-by: martincostello --- .changeset/little-hats-yell.md | 6 ++++ plugins/scaffolder-backend/api-report.md | 3 ++ .../builtin/github/githubRepoCreate.ts | 2 ++ .../builtin/github/githubRepoPush.test.ts | 34 ++++++++++++++++++- .../actions/builtin/github/githubRepoPush.ts | 4 +++ .../actions/builtin/github/helpers.ts | 2 ++ .../actions/builtin/github/inputProperties.ts | 6 ++++ .../src/scaffolder/actions/builtin/helpers.ts | 4 ++- .../actions/builtin/publish/github.test.ts | 33 +++++++++++++++++- .../actions/builtin/publish/github.ts | 4 +++ 10 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 .changeset/little-hats-yell.md diff --git a/.changeset/little-hats-yell.md b/.changeset/little-hats-yell.md new file mode 100644 index 0000000000..6063459d8d --- /dev/null +++ b/.changeset/little-hats-yell.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +Update the `github:publish` action to allow passing whether pull +requests must be up to date with the default branch before merging. diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index ca5f68294b..df00d95f36 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -199,6 +199,7 @@ export function createGithubRepoCreateAction(options: { allowAutoMerge?: boolean | undefined; requireCodeOwnerReviews?: boolean | undefined; requiredStatusCheckContexts?: string[] | undefined; + requireBranchesToBeUpToDate?: boolean | undefined; repoVisibility?: 'internal' | 'private' | 'public' | undefined; collaborators?: | ( @@ -236,6 +237,7 @@ export function createGithubRepoPushAction(options: { gitAuthorEmail?: string | undefined; requireCodeOwnerReviews?: boolean | undefined; requiredStatusCheckContexts?: string[] | undefined; + requireBranchesToBeUpToDate?: boolean | undefined; sourcePath?: string | undefined; token?: string | undefined; }>; @@ -366,6 +368,7 @@ export function createPublishGithubAction(options: { sourcePath?: string | undefined; requireCodeOwnerReviews?: boolean | undefined; requiredStatusCheckContexts?: string[] | undefined; + requireBranchesToBeUpToDate?: boolean | undefined; repoVisibility?: 'internal' | 'private' | 'public' | undefined; collaborators?: | ( diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoCreate.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoCreate.ts index 32443a924e..9348526b20 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoCreate.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoCreate.ts @@ -53,6 +53,7 @@ export function createGithubRepoCreateAction(options: { allowAutoMerge?: boolean; requireCodeOwnerReviews?: boolean; requiredStatusCheckContexts?: string[]; + requireBranchesToBeUpToDate?: boolean; repoVisibility?: 'private' | 'internal' | 'public'; collaborators?: Array< | { @@ -85,6 +86,7 @@ export function createGithubRepoCreateAction(options: { access: inputProps.access, requireCodeOwnerReviews: inputProps.requireCodeOwnerReviews, requiredStatusCheckContexts: inputProps.requiredStatusCheckContexts, + requireBranchesToBeUpToDate: inputProps.requireBranchesToBeUpToDate, repoVisibility: inputProps.repoVisibility, deleteBranchOnMerge: inputProps.deleteBranchOnMerge, allowMergeCommit: inputProps.allowMergeCommit, diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.test.ts index 20755ad781..fc39e03cbc 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.test.ts @@ -283,6 +283,7 @@ describe('github:repo:push', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: true, }); @@ -302,6 +303,7 @@ describe('github:repo:push', () => { defaultBranch: 'master', requireCodeOwnerReviews: true, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: true, }); @@ -321,6 +323,7 @@ describe('github:repo:push', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: true, }); }); @@ -343,6 +346,7 @@ describe('github:repo:push', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: true, }); @@ -362,6 +366,7 @@ describe('github:repo:push', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: true, }); @@ -381,11 +386,12 @@ describe('github:repo:push', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: false, }); }); - it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of requiredStatusCheckContexts', async () => { + it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of requiredStatusCheckContexts and requireBranchesToBeUpToDate', async () => { mockOctokit.rest.repos.get.mockResolvedValue({ data: { clone_url: 'https://github.com/clone/url.git', @@ -403,6 +409,7 @@ describe('github:repo:push', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: true, }); @@ -411,6 +418,7 @@ describe('github:repo:push', () => { input: { ...mockContext.input, requiredStatusCheckContexts: ['statusCheck'], + requireBranchesToBeUpToDate: true, }, }); @@ -422,6 +430,28 @@ describe('github:repo:push', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: ['statusCheck'], + requireBranchesToBeUpToDate: true, + enforceAdmins: true, + }); + + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + requiredStatusCheckContexts: ['statusCheck'], + requireBranchesToBeUpToDate: false, + }, + }); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repository', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + requiredStatusCheckContexts: ['statusCheck'], + requireBranchesToBeUpToDate: false, enforceAdmins: true, }); @@ -430,6 +460,7 @@ describe('github:repo:push', () => { input: { ...mockContext.input, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, }, }); @@ -441,6 +472,7 @@ describe('github:repo:push', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: true, }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.ts index b80502a80c..b02a475698 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.ts @@ -50,6 +50,7 @@ export function createGithubRepoPushAction(options: { gitAuthorEmail?: string; requireCodeOwnerReviews?: boolean; requiredStatusCheckContexts?: string[]; + requireBranchesToBeUpToDate?: boolean; sourcePath?: string; token?: string; }>({ @@ -64,6 +65,7 @@ export function createGithubRepoPushAction(options: { repoUrl: inputProps.repoUrl, requireCodeOwnerReviews: inputProps.requireCodeOwnerReviews, requiredStatusCheckContexts: inputProps.requiredStatusCheckContexts, + requireBranchesToBeUpToDate: inputProps.requireBranchesToBeUpToDate, defaultBranch: inputProps.defaultBranch, protectDefaultBranch: inputProps.protectDefaultBranch, protectEnforceAdmins: inputProps.protectEnforceAdmins, @@ -93,6 +95,7 @@ export function createGithubRepoPushAction(options: { gitAuthorEmail, requireCodeOwnerReviews = false, requiredStatusCheckContexts = [], + requireBranchesToBeUpToDate = true, token: providedToken, } = ctx.input; @@ -129,6 +132,7 @@ export function createGithubRepoPushAction(options: { repo, requireCodeOwnerReviews, requiredStatusCheckContexts, + requireBranchesToBeUpToDate, config, ctx.logger, gitCommitMessage, 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 65c838e590..2264da1dac 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts @@ -248,6 +248,7 @@ export async function initRepoPushAndProtect( repo: string, requireCodeOwnerReviews: boolean, requiredStatusCheckContexts: string[], + requireBranchesToBeUpToDate: boolean, config: Config, logger: any, gitCommitMessage?: string, @@ -290,6 +291,7 @@ export async function initRepoPushAndProtect( defaultBranch, requireCodeOwnerReviews, requiredStatusCheckContexts, + requireBranchesToBeUpToDate, enforceAdmins: protectEnforceAdmins, }); } catch (e) { diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/inputProperties.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/inputProperties.ts index ef9eaead3c..b00d5fa132 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/inputProperties.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/inputProperties.ts @@ -47,6 +47,11 @@ const requiredStatusCheckContexts = { type: 'string', }, }; +const requireBranchesToBeUpToDate = { + title: 'Require Branches To Be Up To Date?', + description: `Require branches to be up to date before merging. The default value is 'true'`, + type: 'boolean', +}; const repoVisibility = { title: 'Repository Visibility', type: 'string', @@ -173,6 +178,7 @@ export { repoUrl }; export { repoVisibility }; export { requireCodeOwnerReviews }; export { requiredStatusCheckContexts }; +export { requireBranchesToBeUpToDate }; export { sourcePath }; export { token }; export { topics }; diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts index d0cdcd627a..be77d27a9a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts @@ -185,6 +185,7 @@ type BranchProtectionOptions = { logger: Logger; requireCodeOwnerReviews: boolean; requiredStatusCheckContexts?: string[]; + requireBranchesToBeUpToDate?: boolean; defaultBranch?: string; enforceAdmins?: boolean; }; @@ -196,6 +197,7 @@ export const enableBranchProtectionOnDefaultRepoBranch = async ({ logger, requireCodeOwnerReviews, requiredStatusCheckContexts = [], + requireBranchesToBeUpToDate = true, defaultBranch = 'master', enforceAdmins = true, }: BranchProtectionOptions): Promise => { @@ -216,7 +218,7 @@ export const enableBranchProtectionOnDefaultRepoBranch = async ({ repo: repoName, branch: defaultBranch, required_status_checks: { - strict: true, + strict: requireBranchesToBeUpToDate, contexts: requiredStatusCheckContexts, }, restrictions: null, 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 499c4b0276..cebc968669 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 @@ -669,6 +669,7 @@ describe('publish:github', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: true, }); @@ -688,6 +689,7 @@ describe('publish:github', () => { defaultBranch: 'master', requireCodeOwnerReviews: true, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: true, }); @@ -707,6 +709,7 @@ describe('publish:github', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: true, }); }); @@ -732,6 +735,7 @@ describe('publish:github', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: true, }); @@ -751,6 +755,7 @@ describe('publish:github', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: false, }); @@ -770,11 +775,12 @@ describe('publish:github', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: true, }); }); - it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of requiredStatusCheckContexts', async () => { + it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of requiredStatusCheckContexts and requireBranchesToBeUpToDate', async () => { mockOctokit.rest.users.getByUsername.mockResolvedValue({ data: { type: 'User' }, }); @@ -795,6 +801,7 @@ describe('publish:github', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: true, }); @@ -803,6 +810,7 @@ describe('publish:github', () => { input: { ...mockContext.input, requiredStatusCheckContexts: ['statusCheck'], + requireBranchesToBeUpToDate: true, }, }); @@ -814,6 +822,28 @@ describe('publish:github', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: ['statusCheck'], + requireBranchesToBeUpToDate: true, + enforceAdmins: true, + }); + + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + requiredStatusCheckContexts: ['statusCheck'], + requireBranchesToBeUpToDate: false, + }, + }); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repo', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + requiredStatusCheckContexts: ['statusCheck'], + requireBranchesToBeUpToDate: false, enforceAdmins: true, }); @@ -833,6 +863,7 @@ describe('publish:github', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, enforceAdmins: true, }); }); 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 ef9c61929a..c1f8e8b5b6 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts @@ -61,6 +61,7 @@ export function createPublishGithubAction(options: { sourcePath?: string; requireCodeOwnerReviews?: boolean; requiredStatusCheckContexts?: string[]; + requireBranchesToBeUpToDate?: boolean; repoVisibility?: 'private' | 'internal' | 'public'; collaborators?: Array< | { @@ -94,6 +95,7 @@ export function createPublishGithubAction(options: { access: inputProps.access, requireCodeOwnerReviews: inputProps.requireCodeOwnerReviews, requiredStatusCheckContexts: inputProps.requiredStatusCheckContexts, + requireBranchesToBeUpToDate: inputProps.requireBranchesToBeUpToDate, repoVisibility: inputProps.repoVisibility, defaultBranch: inputProps.defaultBranch, protectDefaultBranch: inputProps.protectDefaultBranch, @@ -128,6 +130,7 @@ export function createPublishGithubAction(options: { access, requireCodeOwnerReviews = false, requiredStatusCheckContexts = [], + requireBranchesToBeUpToDate = true, repoVisibility = 'private', defaultBranch = 'master', protectDefaultBranch = true, @@ -193,6 +196,7 @@ export function createPublishGithubAction(options: { repo, requireCodeOwnerReviews, requiredStatusCheckContexts, + requireBranchesToBeUpToDate, config, ctx.logger, gitCommitMessage,