Repository navigation
CI: switch to kjanat/actionlint v1.17.0 #259
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this should use
v1.17.0(instead of v1.17, which may be a rolling tag?)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact it's better to use v1.17 here. The v1.17 tag points to the commit right after the release (722799f, "pin the action image of v1.17.0 to its digest"), which pins the image by digest. That is what the upstream README recommends ("For an immutable action reference with a pinned image, use the full commit SHA resolved from a floating tag").
Since we pin by SHA anyway, the rolling nature of v1.17 doesn't matter here — the comment just says which tag the SHA was resolved from.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, but v1.17.0 is an immutable tag, and signed.
If the floating v1.17 tag is updated, there's no way to verify if the commit we picked was ever matching that release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically, for the immutable tags, we wouldn't even need to pin to a sha (they can't be updated, unless github itself is compromised)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See, the issue here is, if we use v1.17.0 (or its sha, doesn't matter), the docker image used by the action is not pinned to a sha (which is what kjanat/actionlint@722799f fixes).
Which is understandable: the author sets a tag, builds a docker image, uploads it and only after that they can pin docker image to a sha. Chicken-and-egg problem here and the solution is rolling tag (which we pin to a sha so it's not a problem).
If you're OK with using unpinned docker image from an action, AND zizmor knows about immutable tags and thus won't complan about v1.17.0, I'm happy to change the above to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait, but does the action run an image, or the action itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh! the action is just a wrapper for an image that runs the actual work 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So now when we figured this out they've moved away from docker image (in kjanat/actionlint#185) so for the next version (1.18 or 2.0, I dunno) we will need a specific non-floating tag (and if the tag is immutable, we can drop sha, hope zizmor knows about immutable tags).
So this can be merged as is for now and I'm not adding a comment about v1.17.0 vs v1.17.