From 4801592be9546b207cae9ec6030b2a2d35f5098c Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Wed, 25 Jan 2023 08:57:40 -0800 Subject: [PATCH] feat: introduce CommitSigner interface (#442) * feat: introduce CommitSigner interface * chore: fix lint * ignore eslint config * address review --- .eslintrc.json | 3 +- owlbot.py | 1 + src/github/commit-and-push.ts | 12 +++- src/github/create-commit.ts | 24 +++++++- src/index.ts | 4 +- src/types.ts | 21 ++++++- test/commit-and-push.ts | 108 ++++++++++++++++++++++++++++++++-- 7 files changed, 160 insertions(+), 13 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 78215349..3e8d97cc 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -1,3 +1,4 @@ { - "extends": "./node_modules/gts" + "extends": "./node_modules/gts", + "root": true } diff --git a/owlbot.py b/owlbot.py index 6321d5fe..84b3293f 100644 --- a/owlbot.py +++ b/owlbot.py @@ -20,5 +20,6 @@ '.github/release-please.yml', 'renovate.json', '.eslintignore', + '.eslintrc.json', '.prettierignore', ]) diff --git a/src/github/commit-and-push.ts b/src/github/commit-and-push.ts index b3d31c6e..e2e91123 100644 --- a/src/github/commit-and-push.ts +++ b/src/github/commit-and-push.ts @@ -21,7 +21,7 @@ import { } from '../types'; import {Octokit} from '@octokit/rest'; import {logger} from '../logger'; -import {createCommit} from './create-commit'; +import {createCommit, CreateCommitOptions} from './create-commit'; import {CommitError} from '../errors'; const DEFAULT_FILES_PER_COMMIT = 100; @@ -142,6 +142,10 @@ export async function updateRef( } } +interface CommitAndPushOptions extends CreateCommitOptions { + filesPerCommit?: number; +} + /** * Given a set of changes, apply the commit(s) on top of the given branch's head and upload it to GitHub * Rejects if GitHub V3 API fails with the GitHub error response @@ -162,8 +166,9 @@ export async function commitAndPush( originBranch: BranchDomain, commitMessage: string, force: boolean, - filesPerCommit: number = DEFAULT_FILES_PER_COMMIT + options?: CommitAndPushOptions ) { + const filesPerCommit = options?.filesPerCommit ?? DEFAULT_FILES_PER_COMMIT; const tree = generateTreeObjects(changes); for (const treeGroup of inGroupsOf(tree, filesPerCommit)) { const treeSha = await createTree(octokit, originBranch, refHead, treeGroup); @@ -172,7 +177,8 @@ export async function commitAndPush( originBranch, refHead, treeSha, - commitMessage + commitMessage, + options ); } await updateRef(octokit, originBranch, refHead, force); diff --git a/src/github/create-commit.ts b/src/github/create-commit.ts index a3afb4be..acb01b1f 100644 --- a/src/github/create-commit.ts +++ b/src/github/create-commit.ts @@ -13,10 +13,16 @@ // limitations under the License. import {Octokit} from '@octokit/rest'; -import {RepoDomain} from '../types'; +import {RepoDomain, CommitSigner, UserData} from '../types'; import {logger} from '../logger'; import {CommitError} from '../errors'; +export interface CreateCommitOptions { + signer?: CommitSigner; + author?: UserData; + committer?: UserData; +} + /** * Create a commit with a repo snapshot SHA on top of the reference HEAD * and resolves with the SHA of the commit. @@ -27,15 +33,26 @@ import {CommitError} from '../errors'; * @param {string} treeSha the tree SHA that this commit will point to * @param {string} message the message of the new commit * @returns {Promise} the new commit SHA + * @see https://docs.github.com/en/rest/git/commits?apiVersion=2022-11-28#create-a-commit */ export async function createCommit( octokit: Octokit, origin: RepoDomain, refHead: string, treeSha: string, - message: string + message: string, + options: CreateCommitOptions = {} ): Promise { try { + const signature = options.signer + ? await options.signer.generateSignature({ + message, + tree: treeSha, + parents: [refHead], + author: options.author, + committer: options.committer, + }) + : undefined; const { data: {sha, url}, } = await octokit.git.createCommit({ @@ -44,6 +61,9 @@ export async function createCommit( message, tree: treeSha, parents: [refHead], + signature, + author: options.author, + committer: options.committer, }); logger.info(`Successfully created commit. See commit at ${url}`); return sha; diff --git a/src/index.ts b/src/index.ts index ef522310..9c5894f3 100644 --- a/src/index.ts +++ b/src/index.ts @@ -22,7 +22,7 @@ import { FileDiffContent, CreateReviewCommentUserOptions, } from './types'; -export {Changes} from './types'; +export {Changes, CommitData, CommitSigner} from './types'; import {Octokit} from '@octokit/rest'; import {logger, setupLogger} from './logger'; import { @@ -192,7 +192,7 @@ async function createPullRequest( originBranch, gitHubConfigs.message, gitHubConfigs.force, - gitHubConfigs.filesPerCommit + options ); const description: Description = { diff --git a/src/types.ts b/src/types.ts index 446634fc..46c8e35c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +import {CreateCommitOptions} from './github/create-commit'; + export type FileMode = '100644' | '100755' | '040000' | '160000' | '120000'; /** @@ -72,7 +74,7 @@ export interface Description { /** * The user options for creating GitHub PRs */ -export interface CreatePullRequestUserOptions { +export interface CreatePullRequestUserOptions extends CreateCommitOptions { // the owner of the target fork repository upstreamOwner: string; // the name of the target fork repository @@ -200,3 +202,20 @@ export interface Logger { debug: LogFn; trace: LogFn; } + +export interface UserData { + name: string; + email: string; +} + +export interface CommitData { + message: string; + tree: string; + parents: string[]; + author?: UserData; + committer?: UserData; +} + +export interface CommitSigner { + generateSignature(commit: CommitData): Promise; +} diff --git a/test/commit-and-push.ts b/test/commit-and-push.ts index 34184bb5..4099c3fe 100644 --- a/test/commit-and-push.ts +++ b/test/commit-and-push.ts @@ -21,7 +21,14 @@ import * as sinon from 'sinon'; import {GetResponseTypeFromEndpointMethod} from '@octokit/types'; import * as handler from '../src/github/commit-and-push'; import * as createCommitModule from '../src/github/create-commit'; -import {Changes, FileData, TreeObject, RepoDomain} from '../src/types'; +import { + Changes, + FileData, + TreeObject, + RepoDomain, + CommitData, + CommitSigner, +} from '../src/types'; import {createCommit} from '../src/github/create-commit'; import {CommitError} from '../src/errors'; @@ -37,6 +44,17 @@ type CreateCommitResponse = GetResponseTypeFromEndpointMethod< typeof octokit.git.createCommit >; +class FakeCommitSigner implements CommitSigner { + signature: string; + constructor(signature: string) { + this.signature = signature; + } + /* eslint-disable @typescript-eslint/no-unused-vars */ + async generateSignature(_commit: CommitData): Promise { + return this.signature; + } +} + before(() => { setup(); }); @@ -199,7 +217,7 @@ describe('Commit', () => { // tests const sha = await createCommit(octokit, origin, head, treeSha, message); assert.strictEqual(sha, createCommitResponse.data.sha); - sandbox.assert.calledOnceWithExactly(stubCreateCommit, { + sandbox.assert.calledOnceWithMatch(stubCreateCommit, { owner: origin.owner, repo: origin.repo, message, @@ -324,12 +342,85 @@ describe('Commit and push function', async () => { ], base_tree: getCommitResponse.data.tree.sha, }); - sandbox.assert.calledOnceWithExactly(stubCreateCommit, { + sandbox.assert.calledOnceWithMatch(stubCreateCommit, { + owner: origin.owner, + repo: origin.repo, + message, + tree: createTreeResponse.data.sha, + parents: [oldHeadSha], + }); + sandbox.assert.calledOnceWithExactly(stubUpdateRef, { + owner: origin.owner, + repo: origin.repo, + sha: createCommitResponse.data.sha, + ref: 'heads/test-branch-name', + force: true, + }); + }); + it('allows configuring a commit signer', async () => { + changes.set('foo.txt', { + mode: '100755', + content: 'some file content', + }); + // setup + const stubGetCommit = sandbox + .stub(octokit.git, 'getCommit') + .resolves(getCommitResponse); + const stubCreateTree = sandbox + .stub(octokit.git, 'createTree') + .resolves(createTreeResponse); + const stubCreateCommit = sandbox + .stub(octokit.git, 'createCommit') + .resolves(createCommitResponse); + const stubUpdateRef = sandbox.stub(octokit.git, 'updateRef'); + const fakeSigner = new FakeCommitSigner('fake-signature'); + const options = { + author: { + name: 'Test Committer', + email: 'test-committer@example.com', + }, + signer: fakeSigner, + }; + const signatureSpy = sandbox.spy(fakeSigner, 'generateSignature'); + // tests + await handler.commitAndPush( + octokit, + oldHeadSha, + changes, + {branch: branchName, ...origin}, + message, + true, + options + ); + sandbox.assert.calledOnceWithExactly(stubGetCommit, { + owner: origin.owner, + repo: origin.repo, + commit_sha: oldHeadSha, + }); + sandbox.assert.calledWithExactly(stubCreateTree, { + owner: origin.owner, + repo: origin.repo, + tree: [ + { + path: 'foo.txt', + mode: '100755', + type: 'blob', + content: 'some file content', + }, + ], + base_tree: getCommitResponse.data.tree.sha, + }); + sandbox.assert.calledOnceWithMatch(stubCreateCommit, { owner: origin.owner, repo: origin.repo, message, tree: createTreeResponse.data.sha, parents: [oldHeadSha], + signature: 'fake-signature', + author: { + name: 'Test Committer', + email: 'test-committer@example.com', + }, }); sandbox.assert.calledOnceWithExactly(stubUpdateRef, { owner: origin.owner, @@ -338,6 +429,15 @@ describe('Commit and push function', async () => { ref: 'heads/test-branch-name', force: true, }); + sandbox.assert.calledOnceWithMatch(signatureSpy, { + message, + tree: createTreeResponse.data.sha, + parents: [oldHeadSha], + author: { + name: 'Test Committer', + email: 'test-committer@example.com', + }, + }); }); it('Forwards GitHub error if getCommit fails', async () => { const error = new Error('Error committing'); @@ -396,7 +496,7 @@ describe('Commit and push function', async () => { {branch: branchName, ...origin}, message, true, - 6 + {filesPerCommit: 6} ); sinon.assert.calledTwice(createTreeStub);