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

ci: harden workflows and cancel in-progress workflows whenever new changes are pushed up #318

Merged
merged 3 commits into from
Apr 21, 2024

Conversation

G-Rath
Copy link
Contributor

@G-Rath G-Rath commented Apr 19, 2024

This just applies some standard stuff ensuring jobs only have the permissions they actually need and that in-progress jobs are cancelled whenever new changes are pushed up to reduce CI spend.

@G-Rath G-Rath requested review from lukeify and nzlaura April 19, 2024 20:03
Copy link

@nzlaura nzlaura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good.

A non blocking question- were the permissions previously roo loose? I see you've updated to give the jobs read permissions 👍

@G-Rath
Copy link
Contributor Author

G-Rath commented Apr 21, 2024

were the permissions previously roo loose?

tl;dr: https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token

longer answer:

In the beginning, the default permission set was "permissive" which gave read/write access to most things - later GitHub introduced a "restricted" permission set which even later became the default for new repositories, but existing repositories had to manually switch to (since it could break existing workflows).

While technically it would be enough to just set that for each repository, it's not explicit in the workflow and requires us to have admin permissions on the repository (which we usually always do, but not always); so our convention is to always explicitly disable all permissions at the top level of a workflow and then explicitly configure the permissions required by each job.

In addition to ensuring jobs only have the access they actually need, it also makes things consistent - each job will always have the same permissions section (which is also why we place it as the very first property, since it'll always be there for every job), which you can audit at a glance.

@G-Rath G-Rath merged commit ebaa98b into main Apr 21, 2024
10 checks passed
@G-Rath G-Rath deleted the update-ci branch April 21, 2024 19:32
Copy link

🎉 This PR is included in version 3.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants