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

Pin versions & make other changes to CI workflows for best practices #224

Merged
merged 3 commits into from
Mar 4, 2025

Conversation

mhucka
Copy link
Contributor

@mhucka mhucka commented Mar 3, 2025

Google's security practices for GitHub Actions states the following:

When using a third-party action (one not hosted in a Google-managed org), a fixed version of the action MUST be used by specifying a specific commit, rather than a branch like "main", or a tagged release, which can be overwritten by any maintainer of the action.

This commit adds sha hash numbers to all uses: directives that didn't have one already. This was done by running the tool frizbee over the files.

In addition, it does the following other best-practices changes:

  • pin versions of runners instead of using *-latest
  • set default permissions for workflow
  • set timeouts on jobs, to prevent running for 6 hrs (the default) in case of hangs or loops

This should also fix the following code scanning alerts:

mhucka added 2 commits March 3, 2025 10:12
oogle's security practices for GitHub Actions states the following:

> When using a third-party action (one not hosted in a [Google-managed org](http://go/github/orgs)), a fixed version of the action MUST be used by [specifying a specific commit](https://help.github.com/en/github/automating-your-workflow-with-github-actions/workflow-syntax-for-github-actions#jobsjob_idstepsuses), rather than a branch like "main", or a tagged release, which can be overwritten by any maintainer of the action.

This commit adds sha hash numbers to all `uses:` directives that didn't have one already. This was done by running the tool [frizbee](https://github.com/stacklok/frizbee?README) over the files.

This also specifies particular versions of runners, rather than
ubuntu, to follow Google guidance.
@mhucka mhucka marked this pull request as ready for review March 3, 2025 18:16
@mhucka mhucka requested a review from dstrain115 March 3, 2025 18:16
Google's best practices suggestions for GitHub Actions workflows is to
set default permissions for workflows explicitly.
@mhucka mhucka changed the title Pin versions & add timeouts to GitHub Actions workflows Pin versions & make other changes to CI workflows for best practices Mar 3, 2025
jobs:
pytest:
name: Pytest
runs-on: ubuntu-latest
runs-on: ubuntu-24.04
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we want to pin the versions? I am worried that this will become a maintenance burden to maintain these versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why do we want to pin the versions? I am worried that this will become a maintenance burden to maintain these versions.

I agree this raises questions. The Google open-source guidance does list this particular thing as a "should" recommendation, not a "must" (unlike pinning action versions to SHAs, which is listed as a "must" condition for Google projects to use GHA workflows), so we technically don't have to do this. Nevertheless, my conclusion is that on balance it'll be a net positive:

  1. Ubuntu's release cycle says that version 24.04 will not reach end of life until 2029.

  2. GitHub only recently stopped supporting Ubuntu 20.04, and made 24.04 the version you get with ubuntu-latest. Version 20.04 reaches end of life this April, so we can be reasonably confident they won't make ubuntu-24.04 unusable until 2029.

  3. Pinning the versions is actually a good thing for reproducibility: the next time that ubuntu-latest is changed by GitHub, who knows what obscure, hard-to-track-down failures might occur in workflows.

Points 1 & 2 suggest we won't have to change the version used by workflows anytime soon, so the maintenance burden seems fairly low. Point 3 suggests there are advantages to changing versions deliberately, on our schedule, instead of on GitHub's schedule.

However, if you guys have experience that argues otherwise, then we could stick with ubuntu-latest.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, that seems reasonable then. How often do the workflow SHAs change?

Copy link
Contributor Author

@mhucka mhucka Mar 3, 2025

Choose a reason for hiding this comment

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

Ok, that seems reasonable then. How often do the workflow SHAs change?

Oh, good question! It's up to the individual developers of course, and since unitary uses only a few actions, it's possible to guess based on their histories:

  • actions/checkout: now at 4.2.2, v4.0.0 in 2023, v3.0.0 in mid 2022
  • actions/setup-python: now at v5.4.0, v5.0.0 in late 2023, v4.0.0 in mid 2022
  • psf/black: now at v25.1.0, v24.1.0 in late 2023

It's worth noting that unitary was until now working with actions/checkout v2, which suggests problems due to old versions are rare, which in turn suggests we could track major versions only, or even just not do anything until something breaks.

However, there's a better way! Dependabot can be configured to check versions of GitHub Actions workflows and open PRs to update the versions, including new pin SHAs, which will minimize the maintenance burden of this. I was going to suggest we enable this on for unitary, but didn't want to do it without checking first because it will probably produce an annoying flurry of PRs for Python package updates at first, until we tune it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, SGTM.

@mhucka mhucka merged commit b12d753 into quantumlib:main Mar 4, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants