Merge rust-bitcoin/rust-bitcoin#3028: ci: fix semver-checks master
6ab8f99731
ci: fix semver-checks master (Jose Storopoli) Pull request description: Ok now I think I've nailed this! 🚀 Closes #3018. ## Summary This PR fix the permissions by adding a secondary job, `semver-checks-pr-label`, that runs only if the `semver-checks` completes and does not fail, i.e. we have either Semver API breaks. This is a secure job according to internal discussions and https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_run ## Implementation notes We split the semver checks into the actual check and labeling. This is due to the fact that we were having several permission issues and the original implementation was insecure. Also it is currently failing on several PRs. 1. `semver-checks.yml`: checks for API semver breaks. Upon finding API semver-checks, it creates a file named `semver-break` that has the PR number inside. If this file is present we upload an artifact representing that the API semver was broken with the PR number. 2. `semver-checks-pr-label.yml`: runs after the `semver-checks.yml` is completed. It uses `actions/github-script` (in JavaScript) to list the previous workflow artifacts and, if finds that an API semver break is present, adds a corresponding label and a comment to the PR that is inside the file `semver-break`. We sanitize the hell out of the thing to be a number only, both in BASH but also in JavaScript. ## How I tested this? 1. I merged this PR to my `master` branch in my `storopoli/rust-bitcoin` repo. 2. I've set the following settings in the repo settings to mimic what we have for `rust-bitcoin`:  3. I've made a PR (cherry-picked from Toby) that breaks semver, see https://github.com/storopoli/rust-bitcoin/pull/10. It correctly labelled as `API break` and added a comment to the PR  ACKs for top commit: Kixunil: ACK6ab8f99731
apoelstra: ACK6ab8f99731
thanks for sticking with this\!\! Tree-SHA512: 27385d6d4a40b5ffc3b95f00ad34010fabe5f1f9442d258b4cb433807bc663fc70b98c7c632048f05929cad073fd81ac6217f72016740b72db7575dac9d3939b
This commit is contained in:
commit
59c806996c
|
@ -0,0 +1,85 @@
|
|||
on: # yamllint disable-line rule:truthy
|
||||
workflow_run:
|
||||
workflows: [Check semver breaks]
|
||||
types: [completed]
|
||||
|
||||
name: Check semver breaks - Label and Comment PR
|
||||
|
||||
jobs:
|
||||
Download:
|
||||
name: Download, Unzip and Add Labels/Comments
|
||||
runs-on: ubuntu-latest
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: write
|
||||
# only run if CI passes on the "Check semver breaks" workflow
|
||||
if: ${{ github.event.workflow_run.conclusion == 'success' }}
|
||||
steps:
|
||||
- name: "Download artifact"
|
||||
uses: actions/github-script@v7
|
||||
with:
|
||||
script: |
|
||||
// get all artifacts from the workflow run
|
||||
let allArtifacts = await github.rest.actions.listWorkflowRunArtifacts({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
run_id: context.payload.workflow_run.id,
|
||||
});
|
||||
|
||||
// find the artifact that starts with 'semver-break'
|
||||
let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => {
|
||||
return artifact.name.startsWith('semver-break');
|
||||
})[0];
|
||||
|
||||
// if no artifact found, exit
|
||||
if (!matchArtifact) {
|
||||
console.log('No semver-break artifact found');
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
// otherwise download the artifact
|
||||
let download = await github.rest.actions.downloadArtifact({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
artifact_id: matchArtifact.id,
|
||||
archive_format: 'zip',
|
||||
});
|
||||
|
||||
// write the artifact to the workspace
|
||||
let fs = require('fs');
|
||||
fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/semver-break.zip`, Buffer.from(download.data));
|
||||
- name: "Unzip artifact"
|
||||
if: ${{ hashFiles('semver-break.zip') != '' }}
|
||||
run: unzip semver-break.zip
|
||||
- name: "Comment and add label on PR - Semver break"
|
||||
uses: actions/github-script@v7
|
||||
if: ${{ hashFiles('semver-break') != '' }}
|
||||
with:
|
||||
github-token: ${{ secrets.GITHUB_TOKEN }}
|
||||
script: |
|
||||
// sanitize and get the PR number from the semver-break file
|
||||
const fs = require('fs');
|
||||
let issue_number = parseInt(fs.readFileSync('semver-break', 'utf8'), 10);
|
||||
|
||||
// assure that is not NaN using Number.isNaN
|
||||
// since does not coerce the value to a number like isNaN
|
||||
if (Number.isNaN(issue_number)) {
|
||||
console.log('PR_NUMBER is not a number');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
// comment on the PR
|
||||
await github.rest.issues.createComment({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
issue_number: issue_number,
|
||||
body: ':rotating_light: API BREAKING CHANGE DETECTED'
|
||||
});
|
||||
|
||||
// add the label to the PR
|
||||
await github.rest.issues.addLabels({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
issue_number: issue_number,
|
||||
labels: ['API break']
|
||||
});
|
|
@ -22,15 +22,20 @@ jobs:
|
|||
run: cargo binstall cargo-semver-checks --no-confirm
|
||||
- name: "Run semver checker script"
|
||||
run: ./contrib/check-semver.sh
|
||||
- name: "Add PR label to breaking changes"
|
||||
uses: actions-ecosystem/action-add-labels@v1
|
||||
- name: Save PR number
|
||||
if: ${{ hashFiles('semver-break') != '' }}
|
||||
with:
|
||||
labels: "API break"
|
||||
- name: Comment PR
|
||||
uses: thollander/actions-comment-pull-request@v2
|
||||
env:
|
||||
PR_NUMBER: ${{ github.event.number }}
|
||||
run: |
|
||||
# check if PR_NUMBER is a number
|
||||
if ! [[ "$PR_NUMBER" =~ ^-?[0-9]+$ ]]; then
|
||||
echo "$PR_NUMBER is not a number."
|
||||
exit 1
|
||||
fi
|
||||
echo "$PR_NUMBER" > ./semver-break
|
||||
- name: "Save breaking state"
|
||||
if: ${{ hashFiles('semver-break') != '' }}
|
||||
uses: actions/upload-artifact@v4
|
||||
with:
|
||||
message: |
|
||||
:rotating_light: API BREAKING CHANGE DETECTED
|
||||
|
||||
name: semver-break
|
||||
path: semver-break
|
||||
|
|
Loading…
Reference in New Issue