Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add cron job to pick up hanging prs #1217

Merged
merged 12 commits into from
Dec 23, 2020
313 changes: 227 additions & 86 deletions packages/merge-on-green/src/merge-on-green.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// limitations under the License.

// eslint-disable-next-line node/no-extraneous-import
import {Application, Context} from 'probot';
import {Application, ProbotOctokit, Context} from 'probot';
import {Datastore} from '@google-cloud/datastore';
import {mergeOnGreen} from './merge-logic';
import {logger} from 'gcf-utils';
Expand All @@ -39,12 +39,12 @@ interface WatchPR {
repo: string;
owner: string;
state: 'continue' | 'stop';
branchProtection: string[];
branchProtection?: string[];
label: string;
author: string;
url: string;
reactionId: number;
installationId: number;
reactionId?: number;
installationId?: number;
}

interface Label {
Expand Down Expand Up @@ -132,21 +132,94 @@ handler.cleanUpPullRequest = async function cleanUpPullRequest(
repo: string,
prNumber: number,
label: string,
reactionId: number,
github: Context['github']
reactionId: number | undefined,
github: InstanceType<typeof ProbotOctokit>
) {
await github.issues.removeLabel({
owner,
repo,
issue_number: prNumber,
name: label,
});
await github.reactions.deleteForIssue({
owner,
repo,
issue_number: prNumber,
reaction_id: reactionId,
});
if (reactionId) {
await github.reactions.deleteForIssue({
owner,
repo,
issue_number: prNumber,
reaction_id: reactionId,
});
}
};

/**
* Checks if a branch with a PR has branch protection, if not, comments on PR
* @param owner type string
* @param repo type string
* @param prNumber type number
* @param github type githup API surface from payload
*/
handler.checkForBranchProtection = async function checkForBranchProtection(
owner: string,
repo: string,
prNumber: number,
github: InstanceType<typeof ProbotOctokit>
): Promise<string[] | undefined> {
let branchProtection: string[] | undefined;

// Check to see if branch protection exists
try {
branchProtection = (
await github.repos.getBranchProtection({
owner,
repo,
branch: 'master',
})
).data.required_status_checks.contexts;
logger.info(
`checking branch protection for ${owner}/${repo}: ${branchProtection}`
);
// if branch protection doesn't exist, leave a comment on the PR;
} catch (err) {
err.message = `Error in getting branch protection\n\n${err.message}`;
await github.issues.createComment({
owner,
repo,
issue_number: prNumber,
body:
"Your PR doesn't have any required checks. Please add required checks to your master branch and then re-add the label. Learn more about enabling these checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks.",
});
logger.error(err);
}
return branchProtection;
};

/**
* Attempts to create a reaction on the PR, returns the reaction ID
* @param owner type string
* @param repo type string
* @param prNumber type number
* @param github type githup API surface from payload
*/
handler.createReaction = async function createReaction(
owner: string,
repo: string,
prNumber: number,
github: InstanceType<typeof ProbotOctokit>
): Promise<number | undefined> {
let reactionId: number | undefined;
try {
reactionId = (
await github.reactions.createForIssue({
owner,
repo,
issue_number: prNumber,
content: 'eyes',
})
).data.id;
} catch (err) {
logger.error(err);
}
return reactionId;
};

/**
Expand All @@ -156,16 +229,17 @@ handler.cleanUpPullRequest = async function cleanUpPullRequest(
* @param prNumber type number
* @param label type string
* @param reactionId type number or null
* @param url type string
* @param github type githup API surface from payload
*/
handler.checkIfPRIsInvalid = async function checkIfPRIsInvalid(
owner: string,
repo: string,
prNumber: number,
label: string,
reactionId: number,
reactionId: number | undefined,
url: string,
github: Context['github']
github: InstanceType<typeof ProbotOctokit>
) {
let pr;
let labels;
Expand Down Expand Up @@ -214,29 +288,56 @@ handler.checkIfPRIsInvalid = async function checkIfPRIsInvalid(
};

/**
* Adds a PR to datastore when an automerge label is added to a PR
* Adds a PR to datastore; first, checks if there's branch protection and adds a reaction Id
* @param url type string
* @param wp type Watch PR (owner, repo, pr number, state, url)
* @param github type githup API surface from payload
* @returns void
*/
handler.addPR = async function addPR(wp: WatchPR, url: string) {
const key = datastore.key([TABLE, url]);
const entity = {
key,
data: {
created: new Date().toJSON(),
owner: wp.owner,
repo: wp.repo,
number: wp.number,
branchProtection: wp.branchProtection,
label: wp.label,
author: wp.author,
reactionId: wp.reactionId,
installationId: wp.installationId,
},
method: 'upsert',
};
await datastore.save(entity);
handler.addPR = async function addPR(
wp: WatchPR,
url: string,
github: InstanceType<typeof ProbotOctokit>
) {
// Since a PR cannot be merged without required status checks, we'll check if they exist first
const branchProtection = await handler.checkForBranchProtection(
wp.owner,
wp.repo,
wp.number,
github
);
let reactionId: number | undefined;
// If the status checks exist, we'll try to react to the PR
if (branchProtection) {
try {
reactionId = await handler.createReaction(
wp.owner,
wp.repo,
wp.number,
github
);
} catch (err) {
logger.error(err);
}
//then, we'll add the PR to the Datastore table
const key = datastore.key([TABLE, url]);
const entity = {
key,
data: {
created: new Date().toJSON(),
owner: wp.owner,
repo: wp.repo,
number: wp.number,
branchProtection: branchProtection,
label: wp.label,
author: wp.author,
reactionId: wp.reactionId,
installationId: reactionId,
},
method: 'upsert',
};
await datastore.save(entity);
}
};

/**
Expand Down Expand Up @@ -300,7 +401,7 @@ handler.checkPRMergeability = async function checkPRMergeability(
wp.number,
[MERGE_ON_GREEN_LABEL, MERGE_ON_GREEN_LABEL_SECURE],
wp.state,
wp.branchProtection,
wp.branchProtection!,
wp.label,
wp.author,
github
Expand Down Expand Up @@ -331,6 +432,70 @@ handler.checkPRMergeability = async function checkPRMergeability(
}
};

//TODO: change the search query if expanding the allowlist to search for more authors
/**
* For a given repository, looks through all the PRs and checks to see if they have a MOG label
* @param owner the owner of the repo
* @param repo the repo name
* @param context the context of the webhook payload
* @returns void
*/
handler.scanForMissingPullRequests = async function scanForMissingPullRequests(
owner: string,
repo: string,
github: InstanceType<typeof ProbotOctokit>
) {
// Github does not support searching the labels with 'OR'.
// The searching for issues is considered to be an "AND" instead of an "OR" .
const [issuesAutomergeLabel, issuesAutomergeExactLabel] = await Promise.all([
github.paginate(github.search.issuesAndPullRequests, {
q: `is:open is:pr user:googleapis user:GoogleCloudPlatform label:"${MERGE_ON_GREEN_LABEL}"`,
}),
github.paginate(github.search.issuesAndPullRequests, {
q: `is:open is:pr user:googleapis user:GoogleCloudPlatform label:"${MERGE_ON_GREEN_LABEL_SECURE}"`,
}),
]);
for (const issue of issuesAutomergeLabel) {
logger.info('before getting PR');
const pullRequestInDatastore: WatchPR = await handler.getPR(issue.html_url);
if (!pullRequestInDatastore) {
logger.info('before adding PR');
await handler.addPR(
{
number: issue.number,
owner,
repo,
state: 'continue',
url: issue.html_url,
label: MERGE_ON_GREEN_LABEL,
author: issue.user.login,
},
issue.html_url,
github
);
}
}

for (const issue of issuesAutomergeExactLabel) {
const pullRequestInDatastore: WatchPR = await handler.getPR(issue.html_url);
if (!pullRequestInDatastore) {
await handler.addPR(
{
number: issue.number,
owner,
repo,
state: 'continue',
url: issue.html_url,
label: MERGE_ON_GREEN_LABEL_SECURE,
author: issue.user.login,
},
issue.html_url,
github
);
}
}
};

// TODO: refactor into multiple function exports, this will take some work in
// gcf-utils.

Expand All @@ -350,6 +515,22 @@ function handler(app: Application) {
await handler.cleanDatastoreTable(watchedPRs, app, context);
return;
}

//because we're searching for the PRs, and not getting the installation ID, we have to use
//the bot's installation ID to call the API. So, we need to make sure it matches the repo owner
if (context.payload.cron_org) {
logger.info('Entering job to pick up any hanging PRs');
const owner = context.payload.organization.login;
const repo = context.payload.repository.name;

if (context.payload.cron_org !== owner) {
logger.info(`skipping run for ${context.payload.cron_org}`);
return;
}
await handler.scanForMissingPullRequests(owner, repo, context.github);
return;
}

const start = Date.now();
await handler.checkPRMergeability(watchedPRs, app, context);
logger.info(`mergeOnGreen check took ${Date.now() - start}ms`);
Expand Down Expand Up @@ -396,60 +577,20 @@ function handler(app: Application) {
return;
}

// check to see if the owner has branch protection rules
let branchProtection: string[] | undefined;
try {
branchProtection = (
await context.github.repos.getBranchProtection({
owner,
repo,
branch: 'master',
})
).data.required_status_checks.contexts;
logger.info(
`checking branch protection for ${owner}/${repo}: ${branchProtection}`
);
} catch (err) {
err.message = `Error in getting branch protection\n\n${err.message}`;
await context.github.issues.createComment({
await handler.addPR(
{
number: prNumber,
owner,
repo,
issue_number: prNumber,
body:
"Your PR doesn't have any required checks. Please add required checks to your master branch and then re-add the label. Learn more about enabling these checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks.",
});
logger.error(err);
}

// if the owner has branch protection set up, add this PR to the Datastore table
if (branchProtection) {
// try to create reaction. Save this reaction in the Datastore table since I don't (think)
// it is on the pull_request payload.
const reactionId = (
await context.github.reactions.createForIssue({
owner,
repo,
issue_number: prNumber,
content: 'eyes',
})
).data.id;

await handler.addPR(
{
number: prNumber,
owner,
repo,
state: 'continue',
url: context.payload.pull_request.html_url,
branchProtection: branchProtection,
label: label.name,
author,
reactionId,
installationId,
},
context.payload.pull_request.html_url
);
}
state: 'continue',
url: context.payload.pull_request.html_url,
label: label.name,
author,
installationId,
},
context.payload.pull_request.html_url,
context.github
);
});

app.on(['pull_request.unlabeled'], async context => {
Expand Down