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

Update CodeQL actions to v3 #7966

Merged
merged 2 commits into from
Dec 31, 2024
Merged

Update CodeQL actions to v3 #7966

merged 2 commits into from
Dec 31, 2024

Conversation

rubo
Copy link
Contributor

@rubo rubo commented Dec 23, 2024

Changes

  • Updated CodeQL actions from the deprecated v2 to v3 as recommended by GitHub
  • Added security-and-quality suite
  • Added the newly announced community packs for analysis
  • Restored action versions over commit hashes as these actions are updated quite often, while the pinned hashes don't allow us to get the latest improvements and security updates for them
  • Removed disk space free-up action since disabling redundant submodules makes enough room

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

@rubo rubo requested a review from a team as a code owner December 23, 2024 22:54
Copy link
Member

@LukaszRozmej LukaszRozmej left a comment

Choose a reason for hiding this comment

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

wasn't it pinned to commit for security reasons?

@rubo
Copy link
Contributor Author

rubo commented Dec 27, 2024

wasn't it pinned to commit for security reasons?

It was but I don't think it works well for the above reasons. It's a controversial thing.

@LukaszRozmej
Copy link
Member

wasn't it pinned to commit for security reasons?

It was but I don't think it works well for the above reasons. It's a controversial thing.

I also think it might be paranoid, but it is a company level policy? @yevh

@yevh
Copy link
Contributor

yevh commented Dec 30, 2024

@rubo @LukaszRozmej The current codeql action already using v3 (this is tag version) and there are no any major changes between 2.19.0(pinned in the action) and 2.20.0 (latest). You can check it here: https://github.com/github/codeql-cli-binaries/releases

@rubo
Copy link
Contributor Author

rubo commented Dec 30, 2024

First, the version is not apparent from commit hashes, especially in this case, where the commit tag is v2 while GitHub recommendations refer to v3. This is a good example of how using commit hashes can be confusing and complicate maintenance. Second, we don't automatically get updates for actions, including bug fixes and security updates. There have been 14 updates to this GitHub action since the commit we hardcoded. This said, the disadvantages of using commit hashes outweigh the possible advantages if any.
IMO, using commit hashes may be justified for actions by some individuals that rarely get updates, while ones from big companies with a lot of attention and frequent update cycles are best with version numbers.

@yevh yevh closed this Dec 31, 2024
@yevh yevh reopened this Dec 31, 2024
@yevh
Copy link
Contributor

yevh commented Dec 31, 2024

@rubo the updates it's important. One of the option that maybe used here is updating the pinned action with Dependabot(it automatically create a PR). We are currently working on making security related action a shared one. I think this will also help solve the update issue. Anyway, this is not yet a policy, but more than a recommendation.

@rubo rubo merged commit 1a1e602 into master Dec 31, 2024
157 checks passed
@rubo rubo deleted the fix/codeql-update branch December 31, 2024 22:30
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.

3 participants