From dab9bcf2e792b68118549a7a3f71c0b5b20642b8 Mon Sep 17 00:00:00 2001 From: Michael Short Date: Fri, 15 Jul 2022 10:38:09 -0500 Subject: [PATCH] protection enforce admin Signed-off-by: Michael Short --- .changeset/late-poets-protect.md | 5 ++ .../builtin/github/githubRepoPush.test.ts | 66 ++++++++++++++++++ .../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 | 69 +++++++++++++++++++ .../actions/builtin/publish/github.ts | 4 ++ 8 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 .changeset/late-poets-protect.md diff --git a/.changeset/late-poets-protect.md b/.changeset/late-poets-protect.md new file mode 100644 index 0000000000..d72f63477c --- /dev/null +++ b/.changeset/late-poets-protect.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +Add enforceAdmins as scaffolder input to branch protection github config 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 f23e3dde9f..20755ad781 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: [], + enforceAdmins: true, }); await action.handler({ @@ -301,6 +302,7 @@ describe('github:repo:push', () => { defaultBranch: 'master', requireCodeOwnerReviews: true, requiredStatusCheckContexts: [], + enforceAdmins: true, }); await action.handler({ @@ -319,6 +321,67 @@ describe('github:repo:push', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + enforceAdmins: true, + }); + }); + + 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: [], + enforceAdmins: true, + }); + + 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: [], + enforceAdmins: true, + }); + + 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: [], + enforceAdmins: false, }); }); @@ -340,6 +403,7 @@ describe('github:repo:push', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + enforceAdmins: true, }); await action.handler({ @@ -358,6 +422,7 @@ describe('github:repo:push', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: ['statusCheck'], + enforceAdmins: true, }); await action.handler({ @@ -376,6 +441,7 @@ describe('github:repo:push', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + 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 942ebe578d..b80502a80c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.ts @@ -44,6 +44,7 @@ export function createGithubRepoPushAction(options: { description?: string; defaultBranch?: string; protectDefaultBranch?: boolean; + protectEnforceAdmins?: boolean; gitCommitMessage?: string; gitAuthorName?: string; gitAuthorEmail?: string; @@ -65,6 +66,7 @@ export function createGithubRepoPushAction(options: { requiredStatusCheckContexts: inputProps.requiredStatusCheckContexts, defaultBranch: inputProps.defaultBranch, protectDefaultBranch: inputProps.protectDefaultBranch, + protectEnforceAdmins: inputProps.protectEnforceAdmins, gitCommitMessage: inputProps.gitCommitMessage, gitAuthorName: inputProps.gitAuthorName, gitAuthorEmail: inputProps.gitAuthorEmail, @@ -85,6 +87,7 @@ export function createGithubRepoPushAction(options: { repoUrl, defaultBranch = 'master', protectDefaultBranch = true, + protectEnforceAdmins = true, gitCommitMessage = 'initial commit', gitAuthorName, gitAuthorEmail, @@ -120,6 +123,7 @@ export function createGithubRepoPushAction(options: { ctx.input.sourcePath, defaultBranch, protectDefaultBranch, + protectEnforceAdmins, owner, client, repo, 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 c8f508e4c0..b60794295a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts @@ -236,6 +236,7 @@ export async function initRepoPushAndProtect( sourcePath: string | undefined, defaultBranch: string, protectDefaultBranch: boolean, + protectEnforceAdmins: boolean, owner: string, client: Octokit, repo: string, @@ -283,6 +284,7 @@ export async function initRepoPushAndProtect( defaultBranch, requireCodeOwnerReviews, requiredStatusCheckContexts, + enforceAdmins: protectEnforceAdmins, }); } catch (e) { assertError(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 0aad402f1f..b0113a4485 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/inputProperties.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/inputProperties.ts @@ -128,6 +128,11 @@ const protectDefaultBranch = { type: 'boolean', description: `Protect the default branch after creating the repository. The default value is 'true'`, }; +const protectEnforceAdmins = { + title: 'Enforce Admins On Protected Branches', + type: 'boolean', + description: `Enforce admins to adhere to default branch protection. The default value is 'true'`, +}; const gitCommitMessage = { title: 'Git Commit Message', type: 'string', @@ -152,6 +157,7 @@ export { gitAuthorEmail }; export { gitAuthorName }; export { gitCommitMessage }; export { protectDefaultBranch }; +export { protectEnforceAdmins }; export { repoUrl }; export { repoVisibility }; export { requireCodeOwnerReviews }; diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts index 8753d44041..dec62eb4a2 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts @@ -135,6 +135,7 @@ type BranchProtectionOptions = { requireCodeOwnerReviews: boolean; requiredStatusCheckContexts?: string[]; defaultBranch?: string; + enforceAdmins?: boolean; }; export const enableBranchProtectionOnDefaultRepoBranch = async ({ @@ -145,6 +146,7 @@ export const enableBranchProtectionOnDefaultRepoBranch = async ({ requireCodeOwnerReviews, requiredStatusCheckContexts = [], defaultBranch = 'master', + enforceAdmins = true, }: BranchProtectionOptions): Promise => { const tryOnce = async () => { try { @@ -167,7 +169,7 @@ export const enableBranchProtectionOnDefaultRepoBranch = async ({ contexts: requiredStatusCheckContexts, }, restrictions: null, - enforce_admins: true, + enforce_admins: enforceAdmins, required_pull_request_reviews: { required_approving_review_count: 1, require_code_owner_reviews: requireCodeOwnerReviews, 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 036c5d66ee..a526e73019 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 @@ -623,6 +623,7 @@ describe('publish:github', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + enforceAdmins: true, }); await action.handler({ @@ -641,6 +642,7 @@ describe('publish:github', () => { defaultBranch: 'master', requireCodeOwnerReviews: true, requiredStatusCheckContexts: [], + enforceAdmins: true, }); await action.handler({ @@ -659,6 +661,70 @@ describe('publish:github', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + enforceAdmins: true, + }); + }); + + 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, + requiredStatusCheckContexts: [], + enforceAdmins: true, + }); + + 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, + requiredStatusCheckContexts: [], + enforceAdmins: 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, + requiredStatusCheckContexts: [], + enforceAdmins: true, }); }); @@ -683,6 +749,7 @@ describe('publish:github', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + enforceAdmins: true, }); await action.handler({ @@ -701,6 +768,7 @@ describe('publish:github', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: ['statusCheck'], + enforceAdmins: true, }); await action.handler({ @@ -719,6 +787,7 @@ describe('publish:github', () => { defaultBranch: 'master', requireCodeOwnerReviews: false, requiredStatusCheckContexts: [], + 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 0815a2cd00..eea006ae8f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts @@ -48,6 +48,7 @@ export function createPublishGithubAction(options: { access?: string; defaultBranch?: string; protectDefaultBranch?: boolean; + protectEnforceAdmins?: boolean; deleteBranchOnMerge?: boolean; gitCommitMessage?: string; gitAuthorName?: string; @@ -93,6 +94,7 @@ export function createPublishGithubAction(options: { repoVisibility: inputProps.repoVisibility, defaultBranch: inputProps.defaultBranch, protectDefaultBranch: inputProps.protectDefaultBranch, + protectEnforceAdmins: inputProps.protectEnforceAdmins, deleteBranchOnMerge: inputProps.deleteBranchOnMerge, gitCommitMessage: inputProps.gitCommitMessage, gitAuthorName: inputProps.gitAuthorName, @@ -124,6 +126,7 @@ export function createPublishGithubAction(options: { repoVisibility = 'private', defaultBranch = 'master', protectDefaultBranch = true, + protectEnforceAdmins = true, deleteBranchOnMerge = false, gitCommitMessage = 'initial commit', gitAuthorName, @@ -176,6 +179,7 @@ export function createPublishGithubAction(options: { ctx.input.sourcePath, defaultBranch, protectDefaultBranch, + protectEnforceAdmins, owner, client, repo,