Skip to content

Commit b3cd1f1

Browse files
authored
Add PR approval check for specific directories (model-checking#31)
Adds process/CI workflow to check that the approvers for every PR are in a committee of recognized approvers. This is a much simplified version of the r?bot that rust-lang uses. ## Testing Scenarios ### Happy path What happens when 2 of the approvers are in the committee Run: https://github.com/jaisnan/rust-dev/pull/16/checks?check_run_id=26914353663 ## What if someone not in the list approves, and 1 from the committee approves? In this scenario, we have a committee that consists of someone called "jaisu-1". But if the approvals came from one ID that's recognized, and another called "Jaisu-1" (Note to the reader: "jaisu-1" != "Jaisu-1"), then the workflow fails and the PR merge is (rightfully) blocked. Run: https://github.com/jaisnan/rust-dev/pull/15/checks?check_run_id=26914179444 ## Call-Outs 1. We need to add a requirement through settings that at least 2 approvers are required before merging, and allow anyone to approve (if it's not already the setting). 2. Once the first iteration of a committee is finalized, we can merge this workflow to begin the approval checking. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.
1 parent f76a1fe commit b3cd1f1

File tree

1 file changed

+156
-0
lines changed

1 file changed

+156
-0
lines changed

.github/workflows/pr_approval.yml

+156
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
name: Check PR Approvals
2+
3+
# For now, the workflow gets triggered only when a review is submitted
4+
# This technically means, a PR with zero approvals can be merged by the rules of this workflow alone
5+
# To protect against that scenario, we can turn on number of approvals required to 2 in the github settings
6+
# of the repository
7+
on:
8+
pull_request_review:
9+
types: [submitted]
10+
workflow_dispatch:
11+
12+
jobs:
13+
check-approvals:
14+
if: github.event.review.state == 'APPROVED' || github.event_name == 'workflow_dispatch'
15+
runs-on: ubuntu-latest
16+
steps:
17+
- name: Checkout repository
18+
uses: actions/checkout@v2
19+
20+
- name: Install TOML parser
21+
run: npm install @iarna/toml
22+
23+
- name: Check PR Relevance and Approvals
24+
uses: actions/github-script@v6
25+
with:
26+
script: |
27+
const fs = require('fs');
28+
const toml = require('@iarna/toml');
29+
const { owner, repo } = context.repo;
30+
let pull_number;
31+
32+
if (github.event_name === 'workflow_dispatch') {
33+
const branch = github.ref.replace('refs/heads/', '');
34+
const prs = await github.rest.pulls.list({
35+
owner,
36+
repo,
37+
head: `${owner}:${branch}`,
38+
state: 'open'
39+
});
40+
if (prs.data.length === 0) {
41+
console.log('No open PR found for this branch.');
42+
return;
43+
}
44+
pull_number = prs.data[0].number;
45+
} else {
46+
pull_number = context.issue.number;
47+
}
48+
49+
// Get PR files
50+
const files = await github.rest.pulls.listFiles({
51+
owner,
52+
repo,
53+
pull_number
54+
});
55+
56+
const relevantPaths = ['library/', 'doc/src/challenges/'];
57+
const isRelevantPR = files.data.some(file =>
58+
relevantPaths.some(path => file.filename.startsWith(path))
59+
);
60+
61+
if (!isRelevantPR) {
62+
console.log('PR does not touch relevant paths. Exiting workflow.');
63+
return;
64+
}
65+
66+
// Get parsed data
67+
try {
68+
const tomlContent = fs.readFileSync('.github/pull_requests.toml', 'utf8');
69+
console.log('TOML content:', tomlContent);
70+
const tomlData = toml.parse(tomlContent);
71+
console.log('Parsed TOML data:', JSON.stringify(tomlData, null, 2));
72+
73+
if (!tomlData.committee || !Array.isArray(tomlData.committee.members)) {
74+
throw new Error('committee.members is not an array in the TOML file');
75+
}
76+
requiredApprovers = tomlData.committee.members;
77+
} catch (error) {
78+
console.error('Error reading or parsing TOML file:', error);
79+
core.setFailed('Failed to read required approvers list');
80+
return;
81+
}
82+
83+
// Get all reviews
84+
const reviews = await github.rest.pulls.listReviews({
85+
owner,
86+
repo,
87+
pull_number
88+
});
89+
90+
// Example: approvers = ["celina", "zyad"]
91+
const approvers = new Set(
92+
reviews.data
93+
.filter(review => review.state === 'APPROVED')
94+
.map(review => review.user.login)
95+
);
96+
97+
const requiredApprovals = 2;
98+
const requiredApproversCount = Array.from(approvers)
99+
.filter(approver => requiredApprovers.includes(approver))
100+
.length;
101+
102+
// TODO: Improve logging and messaging to the user
103+
console.log('PR Approvers:', Array.from(approvers));
104+
console.log('Required Approvers:', requiredApproversCount);
105+
106+
// Core logic that checks if the approvers are in the committee
107+
const checkName = 'PR Approval Status';
108+
const conclusion = (approvers.size >= requiredApprovals && requiredApproversCount >= 2) ? 'success' : 'failure';
109+
const output = {
110+
title: checkName,
111+
summary: `PR has ${approvers.size} total approvals and ${requiredApproversCount} required approvals.`,
112+
text: `Approvers: ${Array.from(approvers).join(', ')}\nRequired Approvers: ${requiredApprovers.join(', ')}`
113+
};
114+
115+
// Get PR details
116+
const pr = await github.rest.pulls.get({
117+
owner,
118+
repo,
119+
pull_number
120+
});
121+
122+
// Create or update check run
123+
const checkRuns = await github.rest.checks.listForRef({
124+
owner,
125+
repo,
126+
ref: pr.data.head.sha,
127+
check_name: checkName
128+
});
129+
130+
// Reuse the same workflow everytime there's a new review submitted
131+
// instead of creating new workflows. Better efficiency and readability
132+
// as the number of workflows is kept to a minimal number
133+
if (checkRuns.data.total_count > 0) {
134+
await github.rest.checks.update({
135+
owner,
136+
repo,
137+
check_run_id: checkRuns.data.check_runs[0].id,
138+
status: 'completed',
139+
conclusion,
140+
output
141+
});
142+
} else {
143+
await github.rest.checks.create({
144+
owner,
145+
repo,
146+
name: checkName,
147+
head_sha: pr.data.head.sha,
148+
status: 'completed',
149+
conclusion,
150+
output
151+
});
152+
}
153+
154+
if (conclusion === 'failure') {
155+
core.setFailed(`PR needs at least ${requiredApprovals} total approvals and 2 required approvals. Current approvals: ${approvers.size}, Required approvals: ${requiredApproversCount}`);
156+
}

0 commit comments

Comments
 (0)