Skip to content

Commit

Permalink
added Pull Request number as input (#94)
Browse files Browse the repository at this point in the history
This allows is to fetch the PR directly instead of obtaining it from the
event. It is an alternative solution for when we are using a dispatch
event.

This resolves #93

This also allows the use of `environment` because every event is being
triggered from the default branch.

I also updated dependencies.
  • Loading branch information
Bullrich authored Oct 17, 2023
1 parent a2988bc commit 3f4385d
Show file tree
Hide file tree
Showing 11 changed files with 793 additions and 678 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
# we may forget to set it back to main
- name: Validate that action points to main branch
run: |
BRANCH=$(yq '.jobs.review-approvals.steps[1].uses' $FILE_NAME | cut -d "@" -f2)
BRANCH=$(yq '.jobs.review-approvals.steps[2].uses' $FILE_NAME | cut -d "@" -f2)
# If the branch is not the main branch
if [ "$BRANCH" != "$GITHUB_BASE_REF" ]; then
echo "Action points to $BRANCH. It has to point to $GITHUB_BASE_REF instead!"
Expand Down
18 changes: 10 additions & 8 deletions .github/workflows/review-bot.yml
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
name: Review PR
on:
pull_request:
workflow_run:
workflows:
- Review-Trigger
types:
- opened
- reopened
- synchronize
- review_requested
- review_request_removed
- ready_for_review
pull_request_review:
- completed

permissions:
contents: read
Expand All @@ -18,6 +14,11 @@ jobs:
review-approvals:
runs-on: ubuntu-latest
steps:
- name: Extract content of artifact
id: number
uses: Bullrich/extract-text-from-artifact@main
with:
artifact-name: pr_number
- name: Generate token
id: team_token
uses: tibdex/github-app-token@v1
Expand All @@ -32,3 +33,4 @@ jobs:
repo-token: ${{ secrets.GITHUB_TOKEN }}
team-token: ${{ steps.team_token.outputs.token }}
checks-token: ${{ steps.team_token.outputs.token }}
pr-number: ${{ steps.number.outputs.content }}
31 changes: 31 additions & 0 deletions .github/workflows/review-trigger.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
name: Review-Trigger

on:
pull_request_target:
types:
- opened
- reopened
- synchronize
- review_requested
- review_request_removed
- ready_for_review
pull_request_review:

jobs:
trigger-review-bot:
runs-on: ubuntu-latest
name: trigger review bot
steps:
- name: Get PR number
env:
PR_NUMBER: ${{ github.event.pull_request.number }}
run: |
echo "Saving PR number: $PR_NUMBER"
mkdir -p ./pr
echo $PR_NUMBER > ./pr/pr_number
- uses: actions/upload-artifact@v3
name: Save PR number
with:
name: pr_number
path: pr/
retention-days: 5
54 changes: 48 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ If one user belongs to both teams, their review will satisfy both requirements.
With the `and-distinct` rule, you can request that *two distinct users must review that file*.

## Installation
The installation requires two files for it to work, we first need to create a file (`.github/review-bot.yml` by default).
The installation requires three files for it to work, we first need to create a file (`.github/review-bot.yml` by default).
```yaml
rules:
- name: General
Expand All @@ -39,10 +39,11 @@ rules:
- your-team-name-here
```
And then we must create a second file. This will be `.github/workflows/review-bot.yml`.
The second file is the triggering file. This will be `.github/workflows/review-trigger.yml`:
```yaml
name: Review Bot
on:
name: Review-Trigger
on:
pull_request_target:
types:
- opened
Expand All @@ -53,6 +54,36 @@ on:
- ready_for_review
pull_request_review:
jobs:
trigger-review-bot:
runs-on: ubuntu-latest
name: trigger review bot
steps:
- name: Get PR number
env:
PR_NUMBER: ${{ github.event.pull_request.number }}
run: |
echo "Saving PR number: $PR_NUMBER"
mkdir -p ./pr
echo $PR_NUMBER > ./pr/pr_number
- uses: actions/upload-artifact@v3
name: Save PR number
with:
name: pr_number
path: pr/
retention-days: 5
```

And then we must create a final file. This will be `.github/workflows/review-bot.yml`.
```yaml
name: Review Bot
on:
workflow_run:
workflows:
- Review-Trigger
types:
- completed
permissions:
contents: read
checks: write
Expand All @@ -61,13 +92,18 @@ jobs:
review-approvals:
runs-on: ubuntu-latest
steps:
- name: Extract content of artifact
id: number
uses: Bullrich/extract-text-from-artifact@main
with:
artifact-name: pr_number
- name: "Evaluates PR reviews"
uses: paritytech/review-bot@main
with:
repo-token: ${{ github.token }}
team-token: ${{ secrets.TEAM_TOKEN }}
checks-token: ${{ secrets.CHECKS_TOKEN }}
pr-number: ${{ steps.number.outputs.content }}
```
Create a new PR and see if it is working.

Expand All @@ -84,6 +120,10 @@ After this, go to your branch protection rules and make sure that you have the f
### Important
Use [`pull_request_target`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target) for the event, not `pull_request`.
- This is a security measure so that an attacker doesn’t have access to our secrets.

We use [`worflow_run`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_run) to let the action be triggered at all times (If we don’t use this, GitHub will stop it if it comes from a fork).

By chaining events we are able to safely execute our action without jeopardizing our secrets. You can even use [`environment`](https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment) in the final file if you want to have it extra secure.
## Workflow Configuration
Review bot has multiple configurations available. It has available inputs and outputs. It also has rule configurations, but you can find that in the [Rule Configuration](#rule-configuration-file) section.

Expand Down Expand Up @@ -136,8 +176,10 @@ Because this project is intended to be used with a token, we need to do an extra
repo-token: ${{ github.token }}
# The previous step generates a token which is used as the input for this action
team-token: ${{ steps.generate_token.outputs.token }
checks-token: ${{ steps.generate_token.outputs.token }
checks-token: ${{ steps.generate_token.outputs.token }}
pr-number: ${{ steps.number.outputs.content }}
```

### Outputs
Outputs are needed for your chained actions. If you want to use this information, remember to set an `id` field in the step, so you can access it.

Expand Down
3 changes: 3 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ inputs:
description: 'Location of the configuration file'
required: false
default: '.github/review-bot.yml'
pr-number:
description: 'The number of the pull request to review. Required if event is `workflow_run`'
required: false
outputs:
repo:
description: 'The name of the repo in owner/repo pattern'
Expand Down
18 changes: 9 additions & 9 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,20 @@
"homepage": "https://github.com/paritytech/review-bot#readme",
"devDependencies": {
"@eng-automation/js-style": "^2.3.0",
"@octokit/webhooks-types": "^7.2.0",
"@types/jest": "^29.5.4",
"@vercel/ncc": "^0.36.1",
"jest": "^29.6.4",
"@octokit/webhooks-types": "^7.3.1",
"@types/jest": "^29.5.5",
"@vercel/ncc": "^0.38.0",
"jest": "^29.7.0",
"jest-mock-extended": "^3.0.5",
"ts-jest": "^29.1.1",
"typescript": "^5.2.2"
},
"dependencies": {
"@actions/core": "^1.10.0",
"@actions/github": "^5.1.1",
"@actions/core": "^1.10.1",
"@actions/github": "^6.0.0",
"@eng-automation/js": "^1.0.2",
"@polkadot/api": "^10.9.1",
"joi": "^17.10.0",
"yaml": "^2.3.2"
"@polkadot/api": "^10.10.1",
"joi": "^17.11.0",
"yaml": "^2.3.3"
}
}
1 change: 0 additions & 1 deletion src/github/pullRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ export class PullRequestApi {
private readonly api: GitHubClient,
private readonly pr: PullRequest,
private readonly logger: ActionLogger,
private readonly detailsUrl: string,
) {
this.number = pr.number;
this.repoInfo = { owner: this.pr.base.repo.owner.login, repo: this.pr.base.repo.name };
Expand Down
51 changes: 34 additions & 17 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { PullRequest } from "@octokit/webhooks-types";
import { GitHubChecksApi } from "./github/check";
import { PullRequestApi } from "./github/pullRequest";
import { GitHubTeamsApi } from "./github/teams";
import { CheckData } from "./github/types";
import { PolkadotFellows } from "./polkadot/fellows";
import { ActionRunner } from "./runner";
import { generateCoreLogger } from "./util";
Expand All @@ -16,6 +17,8 @@ export interface Inputs {
repoToken: string;
/** A custom access token with the read:org access */
teamApiToken: string;
/** Number of the PR to analyze. Optional when it is triggered by `pull_request` event */
prNumber: number | null;
}

const getRepo = (ctx: Context) => {
Expand All @@ -36,43 +39,57 @@ const getInputs = (): Inputs => {
const configLocation = getInput("config-file");
const repoToken = getInput("repo-token", { required: true });
const teamApiToken = getInput("team-token", { required: true });
const prNumber = getInput("pr-number");

return { configLocation, repoToken, teamApiToken };
return { configLocation, repoToken, teamApiToken, prNumber: prNumber ? parseInt(prNumber) : null };
};

const repo = getRepo(context);

setOutput("repo", `${repo.owner}/${repo.repo}`);

if (!context.payload.pull_request) {
throw new Error("No pull request event");
}

debug("Got payload:" + JSON.stringify(context.payload.pull_request));

const inputs = getInputs();
const runAction = async (): Promise<Pick<CheckData, "conclusion">> => {
const inputs = getInputs();

const actionId = `${context.serverUrl}/${repo.owner}/${repo.repo}/actions/runs/${context.runId}`;

const actionInstance = getOctokit(inputs.repoToken);

const actionId = `${context.serverUrl}/${repo.owner}/${repo.repo}/actions/runs/${context.runId}`;
let pr: PullRequest;
if (context.payload.pull_request) {
pr = context.payload.pull_request as PullRequest;
} else if (inputs.prNumber) {
debug(`Fetching pull request number #${inputs.prNumber}`);
const { data } = await actionInstance.rest.pulls.get({ ...repo, pull_number: inputs.prNumber });
pr = data as PullRequest;
} else {
throw new Error("Payload is not `pull_request` and PR number wasn't provided");
}

const api = new PullRequestApi(actionInstance, pr, generateCoreLogger());

const pr = context.payload.pull_request as PullRequest;
const logger = generateCoreLogger();

const api = new PullRequestApi(getOctokit(inputs.repoToken), pr, generateCoreLogger(), actionId);
const teamApi = new GitHubTeamsApi(getOctokit(inputs.teamApiToken), repo.owner, logger);

const logger = generateCoreLogger();
const checks = new GitHubChecksApi(getOctokit(inputs.teamApiToken), pr, logger, actionId);

const teamApi = new GitHubTeamsApi(getOctokit(inputs.teamApiToken), repo.owner, logger);
const fellows = new PolkadotFellows(logger);

const checks = new GitHubChecksApi(getOctokit(inputs.teamApiToken), pr, logger, actionId);
const runner = new ActionRunner(api, teamApi, fellows, checks, logger);

const fellows = new PolkadotFellows(logger);
const result = await runner.runAction(inputs);

const runner = new ActionRunner(api, teamApi, fellows, checks, logger);
setOutput("report", JSON.stringify(result));

return result;
};

runner
.runAction(inputs)
runAction()
.then((result) => {
info(`Action run without problem. Evaluation result was '${result.conclusion}'`);
setOutput("report", JSON.stringify(result));
})
.catch((error) => {
console.error(error);
Expand Down
2 changes: 1 addition & 1 deletion src/test/github.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe("Pull Request API Tests", () => {
pr.number = 99;
pr.base.repo.owner.login = "org";

api = new PullRequestApi(client, pr, logger, "");
api = new PullRequestApi(client, pr, logger);
});

describe("Approvals", () => {
Expand Down
2 changes: 1 addition & 1 deletion src/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe("Integration testing", () => {
pr.number = 99;
pr.base.repo.owner.login = "org";

api = new PullRequestApi(client, pr, logger, "");
api = new PullRequestApi(client, pr, logger);
teams = new GitHubTeamsApi(client, "org", logger);
checks = new GitHubChecksApi(client, pr, logger, "example");
runner = new ActionRunner(api, teams, mock<TeamApi>(), checks, logger);
Expand Down
Loading

0 comments on commit 3f4385d

Please sign in to comment.