Skip to content

Commit

Permalink
Move remaining pr-bot logic to GitHub Script (#1727)
Browse files Browse the repository at this point in the history 
* Update command handling

Assume that a first line starting with a slash is intended
to be treated as a command and output help if not recognised

* Remove previous help output from yml

* Use core for logging, validate outputs are set correctly

* Simplify test context setup

* Move to using GitHub script outputs for pr details

* Remove redundant step

* Add reply comment when user cannot run commands

* Switch to using output for command

The linter was reporting failures when referencing additional outputs
from the GitHub script action.
Previously, we set the command as the action result and set the
result type as string.
This change switches to setting an output for the command explicitly,
and reinstates the result type as object

* Move to later super-linter to get actionlint update

* Revert "Move to later super-linter to get actionlint update"

This reverts commit 93a09b454733a87b924baea04f0674962c5b8301.

It seems that the latest version of the super-linter action still
pulls the v4.9.2 container which doesn't have the latest actionlint

* Add actionlint to build script tests

* Temporarily disable GitHub action linting in super-linter

* Add missing awaits on github SDK calls

* Move "running tests" comment to GitHub script

* Handle docs-only check in GitHub script

Bonus: allows for including 'mkdocs.yml' as a docs file

* Use addActionComment for show-help

Add more consistency around pr-bot messages

* Add comment on /test-force-approve

* Include commit SHA in /test-force-approve comment

* Include refid (part of RG name) in comment for /test

* Add pointer to run-test.sh
  • Loading branch information
@stuartleeks
stuartleeks authored Apr 23, 2022
1 parent 76cd703 commit 7f6e292
Show file tree
Hide file tree
Showing 5 changed files with 535 additions and 225 deletions.
185 changes: 143 additions & 42 deletions .github/scripts/build.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,47 @@ async function getCommandFromComment({ core, context, github }) {
const repoParts = repoFullName.split("/");
const repoOwner = repoParts[0];
const repoName = repoParts[1];
const prNumber = context.payload.issue.number;
const commentLink = context.payload.comment.html_url;
const runId = context.runId;

// only allow actions for users with write access
if (!await userHasWriteAccessToRepo({ github }, commentUsername, repoOwner, repoName)) {
console.log("Command: none [user doesn't have write permission]");
if (!await userHasWriteAccessToRepo({ core, github }, commentUsername, repoOwner, repoName)) {
core.notice("Command: none - user doesn't have write permission]");
await github.rest.issues.createComment({
owner: repoOwner,
repo: repoName,
issue_number: prNumber,
body: `Sorry, @${commentUsername}, only users with write access to the repo can run pr-bot commands.`
});
return "none";
}

// Determine PR SHA etc
const prNumber = context.payload.issue.number;
const pr = (await github.rest.pulls.get({ owner: repoOwner, repo: repoName, pull_number: prNumber })).data;
console.log("==========================================================================================");
console.log(pr);
console.log("==========================================================================================");
const ciGitRef = getRefForPr(prNumber);
logAndSetOutput(core, "ciGitRef", ciGitRef);

const prRefId = getRefIdForPr(prNumber);
console.log(`prRefId: ${prRefId}`);
core.setOutput("prRefId", prRefId);
logAndSetOutput(core, "prRefId", prRefId);

const pr = (await github.rest.pulls.get({ owner: repoOwner, repo: repoName, pull_number: prNumber })).data;

console.log(`Using head ref: ${pr.head.ref}`)
const branchRefId = getRefIdForBranch(pr.head.ref);
console.log(`branchRefId: ${branchRefId}`);
if (repoFullName === pr.head.repo.full_name) {
core.info(`Using head ref: ${pr.head.ref}`)
const branchRefId = getRefIdForBranch(pr.head.ref);
logAndSetOutput(core, "branchRefId", branchRefId);
} else {
core.info("Skipping branchRefId as PR is from a fork")
}

const potentialMergeCommit = pr.merge_commit_sha;
console.log(`potentialMergeCommit: ${potentialMergeCommit}`);
core.setOutput("potentialMergeCommit", potentialMergeCommit);
logAndSetOutput(core, "prRef", potentialMergeCommit);

const prHeadSha = pr.head.sha;;
console.log(`prHeadSha: ${prHeadSha}`);
const prHeadSha = pr.head.sha;
logAndSetOutput(core, "prHeadSha", prHeadSha);

const gotNonDocChanges = await prContainsNonDocChanges(github, repoOwner, repoName, prNumber);
logAndSetOutput(core, "nonDocsChanges", gotNonDocChanges.toString());

//
// Determine what action to take
Expand All @@ -43,35 +55,82 @@ async function getCommandFromComment({ core, context, github }) {
const commentBody = context.payload.comment.body;
const commentFirstLine = commentBody.split("\n")[0];
let command = "none";
switch (commentFirstLine.trim()) {
case "/test":
command = "run-tests";
break;
case "/test-extended":
command = "run-tests-extended";
break;
case "/test-force-approve":
command = "test-force-approve";
break;
case "/test-destroy-env":
command = "test-destroy-env";
break;
case "/help":
command = "show-help";
break;
const trimmedFirstLine = commentFirstLine.trim();
if (trimmedFirstLine[0] === "/") {
switch (trimmedFirstLine) {
case "/test":
{
if (gotNonDocChanges) {
command = "run-tests";
const message = `:runner: Running tests: https://github.com/${repoFullName}/actions/runs/${runId} (with refid \`${prRefId}\`)`;
await addActionComment({ github }, repoOwner, repoName, prNumber, commentUsername, commentLink, message);
} else {
command = "test-force-approve";
const message = `:white_check_mark: PR only contains docs changes - marking tests as complete`;
await addActionComment({ github }, repoOwner, repoName, prNumber, commentUsername, commentLink, message);
}
break;
}

case "/test-extended":
{
command = "run-tests-extended";
const message = `:runner: Running extended tests: https://github.com/${repoFullName}/actions/runs/${runId} (with refid \`${prRefId}\`)`;
await addActionComment({ github }, repoOwner, repoName, prNumber, commentUsername, commentLink, message);
break;
}

case "/test-force-approve":
{
command = "test-force-approve";
const message = `:white_check_mark: Marking tests as complete (for commit ${prHeadSha})`;
await addActionComment({ github }, repoOwner, repoName, prNumber, commentUsername, commentLink, message);
break;
}

case "/test-destroy-env":
command = "test-destroy-env";
break;

case "/help":
showHelp({ github }, repoOwner, repoName, prNumber, commentUsername, commentLink, null);
command = "none"; // command has been handled, so don't need to return a value for future steps
break;

default:
core.warning(`'${trimmedFirstLine}' not recognised as a valid command`);
await showHelp({ github }, repoOwner, repoName, prNumber, commentUsername, commentLink, trimmedFirstLine);
command = "none";
break;
}
}
console.log(`Command: ${command}`);
logAndSetOutput(core, "command", command);
return command;
}

async function prContainsNonDocChanges(github, repoOwner, repoName, prNumber) {
const prFilesResponse = await github.paginate(github.rest.pulls.listFiles, {
owner: repoOwner,
repo: repoName,
pull_number: prNumber
});
const prFiles = prFilesResponse.map(file => file.filename);
// Regexes describing allowed filenames
// If a filename matches any regex in the array then it is considered a doc
// Currently, match all `.md` files and `mkdocs.yml` in the root
const docsRegexes = [/\.md$/, /^mkdocs.yml$/];
const gotNonDocChanges = prFiles.some(file => docsRegexes.every(regex => !regex.test(file)));
return gotNonDocChanges;
}

async function labelAsExternalIfAuthorDoesNotHaveWriteAccess({ core, context, github }) {
const username = context.payload.pull_request.user.login;
const owner = context.repo.owner;
const repo = context.repo.repo;

if (!await userHasWriteAccessToRepo({ github }, username, owner, repo)) {
console.log("Adding external label to PR " + context.payload.pull_request.number)
github.rest.issues.addLabels({
if (!await userHasWriteAccessToRepo({ core, github }, username, owner, repo)) {
core.info("Adding external label to PR " + context.payload.pull_request.number)
await github.rest.issues.addLabels({
owner,
repo,
issue_number: context.payload.pull_request.number,
Expand All @@ -80,15 +139,15 @@ async function labelAsExternalIfAuthorDoesNotHaveWriteAccess({ core, context, gi
}
}

async function userHasWriteAccessToRepo({ github }, username, repoOwner, repoName) {
async function userHasWriteAccessToRepo({ core, github }, username, repoOwner, repoName) {
// Previously, we attempted to use github.event.comment.author_association to check for OWNER or COLLABORATOR
// Unfortunately, that always shows MEMBER if you are in the microsoft org and have that set to publicly visible
// (Can check via https://github.com/orgs/microsoft/people?query=<username>)

// https://docs.github.com/en/rest/reference/collaborators#check-if-a-user-is-a-repository-collaborator
let userHasWriteAccess = false;
try {
console.log(`Checking if user "${username}" has write access to ${repoOwner}/${repoName} ...`)
core.info(`Checking if user "${username}" has write access to ${repoOwner}/${repoName} ...`)
const result = await github.request('GET /repos/{owner}/{repo}/collaborators/{username}', {
owner: repoOwner,
repo: repoName,
Expand All @@ -97,18 +156,60 @@ async function userHasWriteAccessToRepo({ github }, username, repoOwner, repoNam
userHasWriteAccess = result.status === 204;
} catch (err) {
if (err.status === 404) {
console.log("User not found in collaborators");
core.info("User not found in collaborators");
} else {
console.log(`Error checking if user has write access: ${err.status} (${err.response.data.message}) `)
core.error(`Error checking if user has write access: ${err.status} (${err.response.data.message}) `)
}
}
console.log("User has write access: " + userHasWriteAccess);
core.info("User has write access: " + userHasWriteAccess);
return userHasWriteAccess
}

async function showHelp({ github }, repoOwner, repoName, prNumber, commentUser, commentLink, invalidCommand) {
const leadingContent = invalidCommand ? `\`${invalidCommand}\` is not recognised as a valid command.` : "Hello!";

const body = `${leadingContent}
You can use the following commands:
&nbsp;&nbsp;&nbsp;&nbsp;/test - build, deploy and run smoke tests on a PR
&nbsp;&nbsp;&nbsp;&nbsp;/test-extended - build, deploy and run smoke & extended tests on a PR
&nbsp;&nbsp;&nbsp;&nbsp;/test-force-approve - force approval of the PR tests (i.e. skip the deployment checks)
&nbsp;&nbsp;&nbsp;&nbsp;/test-destroy-env - delete the validation environment for a PR (e.g. to enable testing a deployment from a clean start after previous tests)
&nbsp;&nbsp;&nbsp;&nbsp;/help - show this help`;

await addActionComment({github}, repoOwner, repoName, prNumber, commentUser, commentLink, body);

}
async function addActionComment({ github }, repoOwner, repoName, prNumber, commentUser, commentLink, message) {

const body = `:robot: pr-bot :robot:
${message}
(in response to [this comment](${commentLink}) from @${commentUser})
`;

await github.rest.issues.createComment({
owner: repoOwner,
repo: repoName,
issue_number: prNumber,
body: body
});

}

function logAndSetOutput(core, name, value) {
core.info(`Setting output '${name}: ${value}`);
core.setOutput(name, value);
}

function getRefForPr(prNumber) {
return `refs/pull/${prNumber}/merge`;
}
function getRefIdForPr(prNumber) {
const prRef = getRefForPr(prNumber);
// Trailing newline is for compatibility with previous bash SHA calculation
return createShortHash(`refs/pull/${prNumber}/merge\n`);
return createShortHash(`${prRef}\n`);
}
function getRefIdForBranch(branchName) {
// Trailing newline is for compatibility with previous bash SHA calculation
Expand Down
Loading

0 comments on commit 7f6e292

Please sign in to comment.