From aa4aee0e567f1b068c4eaaca93e648d266cd2ce7 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Thu, 28 Jun 2018 08:45:28 +0200
Subject: [PATCH] feat: check pr commit author against gitAuthor (#2170)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
If gitAuthor is configured, checks that a PR’s commit matches. If not, it is assumed that someone else force pushed to the repo and we should not rebase it.
Closes #2169
---
lib/platform/github/index.js | 39 ++++++-
.../github/__snapshots__/index.spec.js.snap | 51 ++++++++
test/platform/github/index.spec.js | 110 ++++++++++++++++++
3 files changed, 195 insertions(+), 5 deletions(-)
diff --git a/lib/platform/github/index.js b/lib/platform/github/index.js
index 8e578561fa..57ec2ea20b 100644
--- a/lib/platform/github/index.js
+++ b/lib/platform/github/index.js
@@ -915,16 +915,45 @@ async function getPr(prNo) {
pr.canMerge = true;
}
if (pr.mergeable_state === 'dirty') {
- logger.debug('PR state is dirty so unmergeable');
+ logger.debug({ prNo }, 'PR state is dirty so unmergeable');
pr.isUnmergeable = true;
}
if (pr.commits === 1) {
- // Only one commit was made - must have been renovate
- logger.debug('Only 1 commit in PR so rebase is possible');
- pr.canRebase = true;
+ if (config.gitAuthor) {
+ // Check against gitAuthor
+ logger.debug('Checking all commits');
+ try {
+ const commitAuthorEmail = (await get(
+ `repos/${config.parentRepo ||
+ config.repository}/pulls/${prNo}/commits`
+ )).body[0].commit.author.email;
+ if (commitAuthorEmail === config.gitAuthor.address) {
+ logger.debug(
+ { prNo },
+ '1 commit matches configured gitAuthor so can rebase'
+ );
+ pr.canRebase = true;
+ } else {
+ logger.debug(
+ { prNo },
+ '1 commit and not by configured gitAuthor so cannot rebase'
+ );
+ pr.canRebase = false;
+ }
+ } catch (err) {
+ logger.warn({ prNo }, 'Error checking branch commits for rebase');
+ pr.canRebase = false;
+ }
+ } else {
+ logger.debug(
+ { prNo },
+ '1 commit and no configured gitAuthor so can rebase'
+ );
+ pr.canRebase = true;
+ }
} else {
// Check if only one author of all commits
- logger.debug('Checking all commits');
+ logger.debug({ prNo }, 'Checking all commits');
const prCommits = (await get(
`repos/${config.parentRepo || config.repository}/pulls/${prNo}/commits`
)).body;
diff --git a/test/platform/github/__snapshots__/index.spec.js.snap b/test/platform/github/__snapshots__/index.spec.js.snap
index bc03550a8c..dc96b8188a 100644
--- a/test/platform/github/__snapshots__/index.spec.js.snap
+++ b/test/platform/github/__snapshots__/index.spec.js.snap
@@ -370,6 +370,40 @@ Object {
}
`;
+exports[`platform/github getPr(prNo) should return a not rebaseable PR if gitAuthor and error 1`] = `
+Object {
+ "base": Object {
+ "sha": "1234",
+ },
+ "branchName": undefined,
+ "canRebase": false,
+ "commits": 1,
+ "displayNumber": "Pull Request #1",
+ "isUnmergeable": true,
+ "mergeable_state": "dirty",
+ "number": 1,
+ "sha": undefined,
+ "state": "open",
+}
+`;
+
+exports[`platform/github getPr(prNo) should return a not rebaseable PR if gitAuthor does not match 1 commit 1`] = `
+Object {
+ "base": Object {
+ "sha": "1234",
+ },
+ "branchName": undefined,
+ "canRebase": false,
+ "commits": 1,
+ "displayNumber": "Pull Request #1",
+ "isUnmergeable": true,
+ "mergeable_state": "dirty",
+ "number": 1,
+ "sha": undefined,
+ "state": "open",
+}
+`;
+
exports[`platform/github getPr(prNo) should return a rebaseable PR despite multiple commits 1`] = `
Object {
"base": Object {
@@ -387,6 +421,23 @@ Object {
}
`;
+exports[`platform/github getPr(prNo) should return a rebaseable PR if gitAuthor matches 1 commit 1`] = `
+Object {
+ "base": Object {
+ "sha": "1234",
+ },
+ "branchName": undefined,
+ "canRebase": true,
+ "commits": 1,
+ "displayNumber": "Pull Request #1",
+ "isUnmergeable": true,
+ "mergeable_state": "dirty",
+ "number": 1,
+ "sha": undefined,
+ "state": "open",
+}
+`;
+
exports[`platform/github getPr(prNo) should return a rebaseable PR if web-flow is second author 1`] = `
Object {
"base": Object {
diff --git a/test/platform/github/index.spec.js b/test/platform/github/index.spec.js
index a6417cd652..5d0e4cd20b 100644
--- a/test/platform/github/index.spec.js
+++ b/test/platform/github/index.spec.js
@@ -1348,6 +1348,116 @@ describe('platform/github', () => {
expect(pr.canRebase).toBe(true);
expect(pr).toMatchSnapshot();
});
+ it('should return a rebaseable PR if gitAuthor matches 1 commit', async () => {
+ await initRepo({
+ repository: 'some/repo',
+ token: 'token',
+ gitAuthor: 'Renovate Bot <bot@renovateapp.com>',
+ });
+ get.mockImplementationOnce(() => ({
+ body: {
+ number: 1,
+ state: 'open',
+ mergeable_state: 'dirty',
+ base: { sha: '1234' },
+ commits: 1,
+ },
+ }));
+ get.mockImplementationOnce(() => ({
+ body: [
+ {
+ commit: {
+ author: {
+ email: 'bot@renovateapp.com',
+ },
+ },
+ },
+ ],
+ }));
+ // getBranchCommit
+ get.mockImplementationOnce(() => ({
+ body: {
+ object: {
+ sha: '1234',
+ },
+ },
+ }));
+ const pr = await github.getPr(1234);
+ expect(pr.canRebase).toBe(true);
+ expect(pr).toMatchSnapshot();
+ });
+ it('should return a not rebaseable PR if gitAuthor does not match 1 commit', async () => {
+ await initRepo({
+ repository: 'some/repo',
+ token: 'token',
+ gitAuthor: 'Renovate Bot <bot@renovateapp.com>',
+ });
+ get.mockImplementationOnce(() => ({
+ body: {
+ number: 1,
+ state: 'open',
+ mergeable_state: 'dirty',
+ base: { sha: '1234' },
+ commits: 1,
+ },
+ }));
+ get.mockImplementationOnce(() => ({
+ body: [
+ {
+ commit: {
+ author: {
+ email: 'foo@bar.com',
+ },
+ },
+ },
+ ],
+ }));
+ // getBranchCommit
+ get.mockImplementationOnce(() => ({
+ body: {
+ object: {
+ sha: '1234',
+ },
+ },
+ }));
+ const pr = await github.getPr(1234);
+ expect(pr.canRebase).toBe(false);
+ expect(pr).toMatchSnapshot();
+ });
+ it('should return a not rebaseable PR if gitAuthor and error', async () => {
+ await initRepo({
+ repository: 'some/repo',
+ token: 'token',
+ gitAuthor: 'Renovate Bot <bot@renovateapp.com>',
+ });
+ get.mockImplementationOnce(() => ({
+ body: {
+ number: 1,
+ state: 'open',
+ mergeable_state: 'dirty',
+ base: { sha: '1234' },
+ commits: 1,
+ },
+ }));
+ get.mockImplementationOnce(() => ({
+ body: [
+ {
+ commit: {},
+ },
+ ],
+ }));
+ // getBranchCommit
+ get.mockImplementationOnce(() => ({
+ body: {
+ object: {
+ sha: '1234',
+ },
+ },
+ }));
+ const pr = await github.getPr(1234);
+ expect(pr.canRebase).toBe(false);
+ expect(pr).toMatchSnapshot();
+ });
});
describe('getPrFiles()', () => {
it('should return empty if no prNo is passed', async () => {
--
GitLab