diff --git a/.changeset/ten-nails-hear.md b/.changeset/ten-nails-hear.md new file mode 100644 index 0000000000..8d3c4d59c3 --- /dev/null +++ b/.changeset/ten-nails-hear.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-scaffolder-backend-module-github': patch +--- + +Adds `requireLastPushApproval` input property to configure Branch Protection Settings in `github:publish` action + +Adds `requireLastPushApproval` input property to configure Branch Protection Settings in `github:repo:push` action diff --git a/plugins/scaffolder-backend-module-github/api-report.md b/plugins/scaffolder-backend-module-github/api-report.md index 94b642bdbf..79c2394e5c 100644 --- a/plugins/scaffolder-backend-module-github/api-report.md +++ b/plugins/scaffolder-backend-module-github/api-report.md @@ -257,6 +257,7 @@ export function createGithubRepoPushAction(options: { sourcePath?: string | undefined; token?: string | undefined; requiredCommitSigning?: boolean | undefined; + requireLastPushApproval?: boolean | undefined; }, JsonObject >; @@ -329,6 +330,7 @@ export function createPublishGithubAction(options: { requiredStatusCheckContexts?: string[] | undefined; requireBranchesToBeUpToDate?: boolean | undefined; requiredConversationResolution?: boolean | undefined; + requireLastPushApproval?: boolean | undefined; repoVisibility?: 'internal' | 'private' | 'public' | undefined; collaborators?: | ( diff --git a/plugins/scaffolder-backend-module-github/src/actions/gitHelpers.ts b/plugins/scaffolder-backend-module-github/src/actions/gitHelpers.ts index ea92201eec..9989a80724 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/gitHelpers.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/gitHelpers.ts @@ -38,6 +38,7 @@ type BranchProtectionOptions = { }; requireBranchesToBeUpToDate?: boolean; requiredConversationResolution?: boolean; + requireLastPushApproval: boolean; defaultBranch?: string; enforceAdmins?: boolean; dismissStaleReviews?: boolean; @@ -56,6 +57,7 @@ export const enableBranchProtectionOnDefaultRepoBranch = async ({ requiredStatusCheckContexts = [], requireBranchesToBeUpToDate = true, requiredConversationResolution = false, + requireLastPushApproval = false, defaultBranch = 'master', enforceAdmins = true, dismissStaleReviews = false, @@ -88,6 +90,7 @@ export const enableBranchProtectionOnDefaultRepoBranch = async ({ require_code_owner_reviews: requireCodeOwnerReviews, bypass_pull_request_allowances: bypassPullRequestAllowances, dismiss_stale_reviews: dismissStaleReviews, + require_last_push_approval: requireLastPushApproval, }, required_conversation_resolution: requiredConversationResolution, }); diff --git a/plugins/scaffolder-backend-module-github/src/actions/github.test.ts b/plugins/scaffolder-backend-module-github/src/actions/github.test.ts index 9eda9a2eb0..31ec7ecc90 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/github.test.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/github.test.ts @@ -1010,174 +1010,6 @@ describe('publish:github', () => { ); }); - it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of requireCodeOwnerReviews', async () => { - mockOctokit.rest.users.getByUsername.mockResolvedValue({ - data: { type: 'User' }, - }); - - mockOctokit.rest.repos.createForAuthenticatedUser.mockResolvedValue({ - data: { - name: 'repo', - }, - }); - - await action.handler(mockContext); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - requireCodeOwnerReviews: true, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: true, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - requireCodeOwnerReviews: false, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - }); - - it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of enforceAdmins', async () => { - mockOctokit.rest.users.getByUsername.mockResolvedValue({ - data: { type: 'User' }, - }); - - mockOctokit.rest.repos.createForAuthenticatedUser.mockResolvedValue({ - data: { - name: 'repo', - }, - }); - - await action.handler(mockContext); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - protectEnforceAdmins: false, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: false, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - protectEnforceAdmins: true, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - }); - it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of requiredStatusCheckContexts and requireBranchesToBeUpToDate', async () => { mockOctokit.rest.users.getByUsername.mockResolvedValue({ data: { type: 'User' }, @@ -1204,6 +1036,7 @@ describe('publish:github', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, @@ -1232,6 +1065,7 @@ describe('publish:github', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, @@ -1259,6 +1093,7 @@ describe('publish:github', () => { requiredStatusCheckContexts: ['statusCheck'], requireBranchesToBeUpToDate: false, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, @@ -1285,6 +1120,7 @@ describe('publish:github', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, @@ -1346,255 +1182,6 @@ describe('publish:github', () => { }); }); - it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of dismissStaleReviews', async () => { - mockOctokit.rest.users.getByUsername.mockResolvedValue({ - data: { type: 'User' }, - }); - - mockOctokit.rest.repos.createForAuthenticatedUser.mockResolvedValue({ - data: { - name: 'repo', - }, - }); - - await action.handler(mockContext); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - dismissStaleReviews: true, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: true, - requiredCommitSigning: false, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - dismissStaleReviews: false, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - }); - it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of requiredConversationResolution', async () => { - mockOctokit.rest.users.getByUsername.mockResolvedValue({ - data: { type: 'User' }, - }); - - mockOctokit.rest.repos.createForAuthenticatedUser.mockResolvedValue({ - data: { - name: 'repo', - }, - }); - - await action.handler(mockContext); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - requiredConversationResolution: true, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: true, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - requiredConversationResolution: false, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - }); - it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of requiredApprovingReviewCount', async () => { - mockOctokit.rest.users.getByUsername.mockResolvedValue({ - data: { type: 'User' }, - }); - - mockOctokit.rest.repos.createForAuthenticatedUser.mockResolvedValue({ - data: { - name: 'repo', - }, - }); - - await action.handler(mockContext); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - requiredApprovingReviewCount: 2, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 2, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - requiredApprovingReviewCount: 0, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 0, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - }); it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of restrictions', async () => { mockOctokit.rest.users.getByUsername.mockResolvedValue({ data: { type: 'User' }, @@ -1621,6 +1208,7 @@ describe('publish:github', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, @@ -1653,6 +1241,7 @@ describe('publish:github', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, @@ -1685,6 +1274,7 @@ describe('publish:github', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, @@ -1719,6 +1309,7 @@ describe('publish:github', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, @@ -1753,6 +1344,7 @@ describe('publish:github', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, @@ -1787,94 +1379,12 @@ describe('publish:github', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, }); }); - it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of requiredCommitSigning', async () => { - mockOctokit.rest.users.getByUsername.mockResolvedValue({ - data: { type: 'User' }, - }); - - mockOctokit.rest.repos.createForAuthenticatedUser.mockResolvedValue({ - data: { - name: 'repo', - }, - }); - - await action.handler(mockContext); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - requiredCommitSigning: false, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: false, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - requiredCommitSigning: true, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repo', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - restrictions: undefined, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - requiredCommitSigning: true, - }); - }); it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of bypassPullRequestAllowances', async () => { mockOctokit.rest.users.getByUsername.mockResolvedValue({ data: { type: 'User' }, @@ -1901,6 +1411,7 @@ describe('publish:github', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, @@ -1931,6 +1442,7 @@ describe('publish:github', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, @@ -1961,6 +1473,7 @@ describe('publish:github', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, @@ -1991,6 +1504,7 @@ describe('publish:github', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, @@ -2025,6 +1539,7 @@ describe('publish:github', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, @@ -2059,9 +1574,145 @@ describe('publish:github', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, requiredCommitSigning: false, }); }); + + it.each([ + { + inputProperty: 'dismissStaleReviews', + defaultValue: false, + overrideValue: true, + }, + { + inputProperty: 'requiredConversationResolution', + defaultValue: false, + overrideValue: true, + }, + { + inputProperty: 'requireLastPushApproval', + defaultValue: false, + overrideValue: true, + }, + { + inputProperty: 'requiredApprovingReviewCount', + defaultValue: 1, + overrideValue: 2, + }, + { + inputProperty: 'requiredCommitSigning', + defaultValue: false, + overrideValue: true, + }, + { + inputProperty: 'protectEnforceAdmins', + defaultValue: true, + overrideValue: false, + octokitParameter: 'enforceAdmins', + }, + { + inputProperty: 'requireCodeOwnerReviews', + defaultValue: false, + overrideValue: true, + }, + ])( + 'should call enableBranchProtectionOnDefaultRepoBranch with the correct values of $inputProperty', + async ({ + inputProperty, + defaultValue, + overrideValue, + octokitParameter, + }) => { + mockOctokit.rest.users.getByUsername.mockResolvedValue({ + data: { type: 'User' }, + }); + + mockOctokit.rest.repos.createForAuthenticatedUser.mockResolvedValue({ + data: { + name: 'repo', + }, + }); + + await action.handler(mockContext); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repo', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + bypassPullRequestAllowances: undefined, + requiredApprovingReviewCount: 1, + restrictions: undefined, + requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, + requiredConversationResolution: false, + requireLastPushApproval: false, + enforceAdmins: true, + dismissStaleReviews: false, + requiredCommitSigning: false, + [octokitParameter || inputProperty]: defaultValue, + }); + + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + [inputProperty]: overrideValue, + }, + }); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repo', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + bypassPullRequestAllowances: undefined, + requiredApprovingReviewCount: 1, + restrictions: undefined, + requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, + requiredConversationResolution: false, + requireLastPushApproval: false, + enforceAdmins: true, + dismissStaleReviews: false, + requiredCommitSigning: false, + [octokitParameter || inputProperty]: overrideValue, + }); + + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + [inputProperty]: defaultValue, + }, + }); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repo', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + bypassPullRequestAllowances: undefined, + requiredApprovingReviewCount: 1, + restrictions: undefined, + requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, + requiredConversationResolution: false, + requireLastPushApproval: false, + enforceAdmins: true, + dismissStaleReviews: false, + requiredCommitSigning: false, + [octokitParameter || inputProperty]: defaultValue, + }); + }, + ); }); diff --git a/plugins/scaffolder-backend-module-github/src/actions/github.ts b/plugins/scaffolder-backend-module-github/src/actions/github.ts index f32b65bc9a..c8b22de7e7 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/github.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/github.ts @@ -86,6 +86,7 @@ export function createPublishGithubAction(options: { requiredStatusCheckContexts?: string[]; requireBranchesToBeUpToDate?: boolean; requiredConversationResolution?: boolean; + requireLastPushApproval?: boolean; repoVisibility?: 'private' | 'internal' | 'public'; collaborators?: Array< | { @@ -137,6 +138,7 @@ export function createPublishGithubAction(options: { requireBranchesToBeUpToDate: inputProps.requireBranchesToBeUpToDate, requiredConversationResolution: inputProps.requiredConversationResolution, + requireLastPushApproval: inputProps.requireLastPushApproval, repoVisibility: inputProps.repoVisibility, defaultBranch: inputProps.defaultBranch, protectDefaultBranch: inputProps.protectDefaultBranch, @@ -187,6 +189,7 @@ export function createPublishGithubAction(options: { requiredStatusCheckContexts = [], requireBranchesToBeUpToDate = true, requiredConversationResolution = false, + requireLastPushApproval = false, repoVisibility = 'private', defaultBranch = 'master', protectDefaultBranch = true, @@ -274,6 +277,7 @@ export function createPublishGithubAction(options: { requiredStatusCheckContexts, requireBranchesToBeUpToDate, requiredConversationResolution, + requireLastPushApproval, config, ctx.logger, gitCommitMessage, diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubRepoPush.test.ts b/plugins/scaffolder-backend-module-github/src/actions/githubRepoPush.test.ts index 4d16a25774..d0ee4bfa12 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubRepoPush.test.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubRepoPush.test.ts @@ -305,168 +305,6 @@ describe('github:repo:push', () => { ); }); - it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of requireCodeOwnerReviews', async () => { - mockOctokit.rest.repos.get.mockResolvedValue({ - data: { - clone_url: 'https://github.com/clone/url.git', - html_url: 'https://github.com/html/url', - }, - }); - - await action.handler(mockContext); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repository', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - requiredCommitSigning: false, - restrictions: undefined, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - requireCodeOwnerReviews: true, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repository', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: true, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - requiredCommitSigning: false, - restrictions: undefined, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - requireCodeOwnerReviews: false, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repository', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - requiredCommitSigning: false, - restrictions: undefined, - }); - }); - - it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of enforceAdmins', async () => { - mockOctokit.rest.repos.get.mockResolvedValue({ - data: { - clone_url: 'https://github.com/clone/url.git', - html_url: 'https://github.com/html/url', - }, - }); - - await action.handler(mockContext); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repository', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - requiredCommitSigning: false, - restrictions: undefined, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - protectEnforceAdmins: true, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repository', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - requiredCommitSigning: false, - restrictions: undefined, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - protectEnforceAdmins: false, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repository', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: false, - dismissStaleReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - requiredCommitSigning: false, - restrictions: undefined, - }); - }); - it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of requiredStatusCheckContexts and requireBranchesToBeUpToDate', async () => { mockOctokit.rest.repos.get.mockResolvedValue({ data: { @@ -487,6 +325,7 @@ describe('github:repo:push', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, bypassPullRequestAllowances: undefined, @@ -501,7 +340,6 @@ describe('github:repo:push', () => { ...mockContext.input, requiredStatusCheckContexts: ['statusCheck'], requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, }, }); @@ -515,6 +353,7 @@ describe('github:repo:push', () => { requiredStatusCheckContexts: ['statusCheck'], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, bypassPullRequestAllowances: undefined, @@ -542,6 +381,7 @@ describe('github:repo:push', () => { requiredStatusCheckContexts: ['statusCheck'], requireBranchesToBeUpToDate: false, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, bypassPullRequestAllowances: undefined, @@ -556,7 +396,6 @@ describe('github:repo:push', () => { ...mockContext.input, requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, }, }); @@ -570,6 +409,7 @@ describe('github:repo:push', () => { requiredStatusCheckContexts: [], requireBranchesToBeUpToDate: true, requiredConversationResolution: false, + requireLastPushApproval: false, enforceAdmins: true, dismissStaleReviews: false, bypassPullRequestAllowances: undefined, @@ -598,165 +438,135 @@ describe('github:repo:push', () => { expect(enableBranchProtectionOnDefaultRepoBranch).not.toHaveBeenCalled(); }); - it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of dismissStaleReviews', async () => { - mockOctokit.rest.repos.get.mockResolvedValue({ - data: { - clone_url: 'https://github.com/clone/url.git', - html_url: 'https://github.com/html/url', - }, - }); + it.each([ + { + inputProperty: 'dismissStaleReviews', + defaultValue: false, + overrideValue: true, + }, + { + inputProperty: 'requiredConversationResolution', + defaultValue: false, + overrideValue: true, + }, + { + inputProperty: 'requireLastPushApproval', + defaultValue: false, + overrideValue: true, + }, + { + inputProperty: 'requiredApprovingReviewCount', + defaultValue: 1, + overrideValue: 2, + }, + { + inputProperty: 'requiredCommitSigning', + defaultValue: false, + overrideValue: true, + }, + { + inputProperty: 'protectEnforceAdmins', + defaultValue: true, + overrideValue: false, + octokitParameter: 'enforceAdmins', + }, + { + inputProperty: 'requireCodeOwnerReviews', + defaultValue: false, + overrideValue: true, + }, + ])( + 'should call enableBranchProtectionOnDefaultRepoBranch with the correct values of $inputProperty', + async ({ + inputProperty, + defaultValue, + overrideValue, + octokitParameter, + }) => { + mockOctokit.rest.repos.get.mockResolvedValue({ + data: { + clone_url: 'https://github.com/clone/url.git', + html_url: 'https://github.com/html/url', + }, + }); - await action.handler(mockContext); + await action.handler(mockContext); - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repository', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - requiredCommitSigning: false, - restrictions: undefined, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - dismissStaleReviews: true, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repository', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: true, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - requiredCommitSigning: false, - restrictions: undefined, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - dismissStaleReviews: false, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repository', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - requiredCommitSigning: false, - restrictions: undefined, - }); - }); - - it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of requiredConversationResolution', async () => { - mockOctokit.rest.repos.get.mockResolvedValue({ - data: { - clone_url: 'https://github.com/clone/url.git', - html_url: 'https://github.com/html/url', - }, - }); - - await action.handler(mockContext); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repository', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - requiredCommitSigning: false, - restrictions: undefined, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, - requiredConversationResolution: true, - }, - }); - - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repository', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: true, - enforceAdmins: true, - dismissStaleReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - requiredCommitSigning: false, - restrictions: undefined, - }); - - await action.handler({ - ...mockContext, - input: { - ...mockContext.input, + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repository', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, requiredConversationResolution: false, - }, - }); + requireLastPushApproval: false, + enforceAdmins: true, + dismissStaleReviews: false, + bypassPullRequestAllowances: undefined, + requiredApprovingReviewCount: 1, + requiredCommitSigning: false, + restrictions: undefined, + [octokitParameter || inputProperty]: defaultValue, + }); - expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ - owner: 'owner', - client: mockOctokit, - repoName: 'repository', - logger: mockContext.logger, - defaultBranch: 'master', - requireCodeOwnerReviews: false, - requiredStatusCheckContexts: [], - requireBranchesToBeUpToDate: true, - requiredConversationResolution: false, - enforceAdmins: true, - dismissStaleReviews: false, - bypassPullRequestAllowances: undefined, - requiredApprovingReviewCount: 1, - requiredCommitSigning: false, - restrictions: undefined, - }); - }); + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + [inputProperty]: overrideValue, + }, + }); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repository', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, + requiredConversationResolution: false, + requireLastPushApproval: false, + enforceAdmins: true, + dismissStaleReviews: false, + bypassPullRequestAllowances: undefined, + requiredApprovingReviewCount: 1, + requiredCommitSigning: false, + restrictions: undefined, + [octokitParameter || inputProperty]: overrideValue, + }); + + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + [inputProperty]: defaultValue, + }, + }); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repository', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, + requiredConversationResolution: false, + requireLastPushApproval: false, + enforceAdmins: true, + dismissStaleReviews: false, + bypassPullRequestAllowances: undefined, + requiredApprovingReviewCount: 1, + requiredCommitSigning: false, + restrictions: undefined, + [octokitParameter || inputProperty]: defaultValue, + }); + }, + ); }); diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubRepoPush.ts b/plugins/scaffolder-backend-module-github/src/actions/githubRepoPush.ts index 5b9db78f48..a4fa7673c6 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubRepoPush.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubRepoPush.ts @@ -75,6 +75,7 @@ export function createGithubRepoPushAction(options: { sourcePath?: string; token?: string; requiredCommitSigning?: boolean; + requireLastPushApproval?: boolean; }>({ id: 'github:repo:push', description: @@ -95,6 +96,7 @@ export function createGithubRepoPushAction(options: { requireBranchesToBeUpToDate: inputProps.requireBranchesToBeUpToDate, requiredConversationResolution: inputProps.requiredConversationResolution, + requireLastPushApproval: inputProps.requireLastPushApproval, defaultBranch: inputProps.defaultBranch, protectDefaultBranch: inputProps.protectDefaultBranch, protectEnforceAdmins: inputProps.protectEnforceAdmins, @@ -132,6 +134,7 @@ export function createGithubRepoPushAction(options: { requiredStatusCheckContexts = [], requireBranchesToBeUpToDate = true, requiredConversationResolution = false, + requireLastPushApproval = false, token: providedToken, requiredCommitSigning = false, } = ctx.input; @@ -174,6 +177,7 @@ export function createGithubRepoPushAction(options: { requiredStatusCheckContexts, requireBranchesToBeUpToDate, requiredConversationResolution, + requireLastPushApproval, config, ctx.logger, gitCommitMessage, diff --git a/plugins/scaffolder-backend-module-github/src/actions/helpers.ts b/plugins/scaffolder-backend-module-github/src/actions/helpers.ts index dd2f4a9f76..eec6f73f92 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/helpers.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/helpers.ts @@ -360,6 +360,7 @@ export async function initRepoPushAndProtect( requiredStatusCheckContexts: string[], requireBranchesToBeUpToDate: boolean, requiredConversationResolution: boolean, + requireLastPushApproval: boolean, config: Config, logger: any, gitCommitMessage?: string, @@ -408,6 +409,7 @@ export async function initRepoPushAndProtect( requiredStatusCheckContexts, requireBranchesToBeUpToDate, requiredConversationResolution, + requireLastPushApproval, enforceAdmins: protectEnforceAdmins, dismissStaleReviews: dismissStaleReviews, requiredCommitSigning: requiredCommitSigning, diff --git a/plugins/scaffolder-backend-module-github/src/actions/inputProperties.ts b/plugins/scaffolder-backend-module-github/src/actions/inputProperties.ts index 585526fe94..f6fc592b72 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/inputProperties.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/inputProperties.ts @@ -64,6 +64,11 @@ const requiredConversationResolution = { 'Requires all conversations on code to be resolved before a pull request can be merged into this branch', type: 'boolean', }; +const requireLastPushApproval = { + title: 'Require last push approval', + type: 'boolean', + description: `Whether the most recent push to a PR must be approved by someone other than the person who pushed it. The default value is 'false'`, +}; const repoVisibility = { title: 'Repository Visibility', type: 'string', @@ -326,6 +331,7 @@ export { dismissStaleReviews }; export { requiredStatusCheckContexts }; export { requireBranchesToBeUpToDate }; export { requiredConversationResolution }; +export { requireLastPushApproval }; export { hasProjects }; export { hasIssues }; export { hasWiki };