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.

Sign up for GitHub

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

Jump to bottom

tools: use actions pinned by commit hash on coverage-linux.yml #46294

Closed
wants to merge 5 commits into from
Closed

tools: use actions pinned by commit hash on coverage-linux.yml #46294

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cbd8c8a
tools: use actions pinned by commit hash in coverage-linux
gabibguti Jan 20, 2023
f023b10
tools: Double space inline comments
gabibguti Jan 23, 2023
b8bfce1
tools: Add dependabot for GHA version updates
gabibguti Feb 1, 2023
0194f28
tools: Fix lint double quote error
gabibguti Feb 8, 2023
2bda6e1
tools: fix linter issues
gabibguti Feb 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/coverage-linux.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ jobs:
if: github.event.pull_request.draft == false
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it really make sense for Actions provided by GitHub? Since they're the ones who also provide the hardware, we have to trust them at some point, and keeping this updated seems more likely to happen if we're using a tag rather than a commit hash. +1 one for doing it for the codecov action though. wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with you it's debatable. After all, if GitHub itself gets hijacked we would have bigger problems. About keeping the hash updated it's a good point. Dependabot can take care of updating the hash and the version tag comment, which is more readable. But, yes, keeping the tag version or tag major can be a good option for GitHub actions if you folks prefer.

Copy link
Member

Choose a reason for hiding this comment

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

Dependabot can take care of updating the hash and the version tag comment, which is more readable.

To provide context: dependabot/dependabot-core#4691 (comment)

Copy link

@DanielRuf DanielRuf Jan 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Shall we resolve the linting warning?
I do this pinning also in my personal projects but have the comment above the actual uses line.

Edit: seems according to the dependabot link like dependabot/dependabot-core#4691 (comment) only this comment format is supported? Not sure if the linting issue will go away without doing any further changes.

Might be a problem with the linting rules of GH.

Copy link

@DanielRuf DanielRuf Jan 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

In my opinion it is good to pin all actions, also those of GitHub. We can not be sure if and who pushes what code. What if someone has bad intentions and joins GitHub just to get malicious code into such solutions?

Also at least I do not know which persons maintain the project, have push and write permissions and if I can trust every single one.

Some PRs also seem to be spam or malicious: https://github.com/actions/checkout/pulls

I doubt that the same persons who have access to the hardware at GH (totally different security restrictions) also work on the actions or have access to them.

Tags can also be changed / pointed to different commits at any time - at least in Git.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe I fixed the lint warning but kept the version comment inline, cause, in my understanding, dependabot can only update the version comment inline.

Copy link

Choose a reason for hiding this comment

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

Perfect, thank you very much. This is ok for me as long as dependabot can automatically handle this.

with:
persist-credentials: false
- name: Set up Python ${{ env.PYTHON_VERSION }}
uses: actions/setup-python@v4
uses: actions/setup-python@d27e3f3d7c64b4bbf8e4abfb9b63b83e846e0435 # v4.5.0
with:
python-version: ${{ env.PYTHON_VERSION }}
- name: Environment Information
Expand All @@ -64,6 +64,6 @@ jobs:
- name: Clean tmp
run: rm -rf coverage/tmp && rm -rf out
- name: Upload
uses: codecov/codecov-action@v3
uses: codecov/codecov-action@d9f34f8cd5cb3b3eb79b3e4b5dae3a16df499a70 # v3.1.1
with:
directory: ./coverage