Skip to content

Pin versions of 3rd-party actions and GitHub runners#41

Merged
NoureldinYosri merged 8 commits intoquantumlib:mainfrom
mhucka:mh-pin-versions-in-ci
Feb 18, 2025
Merged

Pin versions of 3rd-party actions and GitHub runners#41
NoureldinYosri merged 8 commits intoquantumlib:mainfrom
mhucka:mh-pin-versions-in-ci

Conversation

@mhucka
Copy link
Copy Markdown
Contributor

@mhucka mhucka commented Feb 15, 2025

Google's terms for allowing the use of GitHub Actions on Google-owned repositories requires that third-party actions be referenced using a specific commit, not a tagged release or a branch name. They also recommend that GitHub-hosted runners be referenced by fixed versions and not "-latest". (Internal doc link: go/github-actions#actions)

The SHAs for GitHub Actions in this commit were obtained using frizbee. The runner versions equivalent to the "-latest" runners are based on the table at https://github.com/actions/runner-images

Google security best practices recommend the use of specific runner
operating system versions instead of "-latest". (Internal
documentation link: go/github-actions#actions)
Google's terms for allowing the use of GitHub Actions on Google-owned
repositories requires that third-party actions be referenced using a
specific commit, not a tagged release or a branch name. (Internal doc
link: go/github-actions#actions)

The version numbers in this commit were obtained using
[frizbee](https://github.com/stacklok/frizbee).
@mhucka mhucka enabled auto-merge February 16, 2025 03:32
Copy link
Copy Markdown
Collaborator

@NoureldinYosri NoureldinYosri left a comment

Choose a reason for hiding this comment

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

thanks @mhucka, but the changes seem to make the CI unable to start

@mhucka
Copy link
Copy Markdown
Contributor Author

mhucka commented Feb 18, 2025

I think I see the problem: ci.yml doesn't have a trigger condition for pull requests:

name: Build and Test

on: [push]

I'm surprised this hasn't come up before. For CI checks, CI workflows normally need pull_request. A push event only happens when (1) an actual push is done on the branch and (2) when the merge event happens (because a PR merge is a commit pushed to the target branch). So, if using a push trigger only, the workflow won't get triggered when someone does a PR from another branch, at least until the PR is merged.

If using merge queues, it also needs a merge_group trigger. Also, I found that with merge queues, it works better to remove push altogether because it ends up causing 2 runs of the workflow when the merge finally happens.

Here's what I've been using for the other repos:

on:
  pull_request:
    types: [opened, synchronize]
    branches:
      - master

  merge_group:
    types:
      - checks_requested

  # Allow manual invocation.
  workflow_dispatch:

The workflow_dispatch at the end is optional; it lets you run the workflow manually from the GUI interface on GitHub, and is quite handy at times (especially when troubleshooting).

If you like these settings, I can either modify this PR or make a new one just for the trigger changes. (Let me know which approach you prefer.)

Copy link
Copy Markdown
Collaborator

@NoureldinYosri NoureldinYosri left a comment

Choose a reason for hiding this comment

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

thanks @mhucka , the ci triggered for every PR before this one including your last PR #13 so I don't know what's special about this one

anyway I applied your suggestions

@NoureldinYosri NoureldinYosri merged commit af7fa09 into quantumlib:main Feb 18, 2025
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