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
Build vendor artifact #11764
base: main
Are you sure you want to change the base?
Build vendor artifact #11764
Conversation
@ndeloof here it is |
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.
LGTM but should be declared in the merge
workflow so this run for commits on main branch and tags
.github/workflows/ci.yml
Outdated
- | ||
name: Build vendor | ||
run: | | ||
go mod tidy |
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.
tidy
is not relevant here, as we enforce go.sum to be up-to-date as part of the PR workflow
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.
Fixed
Have I missed something? |
|
We should probably check if there's additional verification needed for these packages. The standard source tarballs on GitHub releases are produced from git, and therefore should always match the source code. This PR constructs a custom tar; while it's produced through GitHub actions, it's effectively still a "manual" upload, so we should check if there's ways to verify provenance and integrity. Recent events with the |
@thaJeztah this is one of root cause why I'm doing this. Right now OpenBSD port for So, the right way to do it is download from the same source where it sources is produced becuase this should be trusted source by defenition. Regarding the way: as soon as I know |
To keep things simpler I've added Otherwise I need to move and duplicate code inside |
Closes: docker#11760 Signed-off-by: Kirill A. Korinsky <kirill@korins.ky>
Yes, anyone producing binaries / artifacts for others to consume must have provenance. I think that goes beyond just these tars, and there would be many other ways for that build-environment / infra to be comprimised.
Correct; it won't protect against a compromised tarball that has The default tarballs produced by GitHub always match the git-commit of the release, but for other artifacts that's not the case, so some validation may be necessary for that. |
I haven't see any differences with producing Second point: GitHub and other git hosting sometimes changes produced |
just a ping |
tar czf bin/release/docker-compose-vendor.tar.gz vendor | ||
- | ||
name: Upload artifacts | ||
uses: actions/upload-artifact@v3 |
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.
Is there a reason that we use actions/upload-artifact@v3
instead of v4
? According to the action README, v3
will be deprecated in November.
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.
Speaking of v4
, some of the breaking changed in it relate to my other comment bellow about the aritfact name.
2. Uploading to the same named Artifact multiple times.
Due to how Artifacts are created in this new version, it is no longer possible to upload to the same named Artifact multiple times. You must either split the uploads into multiple Artifacts with different names, or only upload once. Otherwise you will encounter an error.
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.
aritfact name must be unique for a built commit, I don't think this has any impact for this workflow.
We still use upload-artifact@v3 on this repo as this breaking change require the whole workflow to be revisited to support the new behavior (see https://github.com/docker/compose/pull/11662/files)
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.
aritfact name must be unique for a built commit, I don't think this has any impact for this workflow.
Ah ok, then this should be fine.
Can you look into https://docs.github.com/en/actions/security-guides/using-artifact-attestations-to-establish-provenance-for-builds#generating-build-provenance-for-binaries so this additional artifact gets provenance attestation and you can safely rely on this for an offline build ? |
On Mon, 06 May 2024 13:54:21 +0100, Nicolas De loof ***@***.***> wrote:
Can you look into https://docs.github.com/en/actions/security-guides/using-artifact-attestations-to-establish-provenance-for-builds#generating-build-provenance-for-binaries so this additional artifact gets provenance attestation and you can safely rely on this for an offline build ?
Thanks for the link.
I've read it and I doesn't see how it can help me.
OpenBSD ports, for example, ships its own checksum of artifacts which should
be used to build port.
It is checked by build servers which downloads artifacts from github.com or
other plases and confirms that checksum holds.
…--
wbr, Kirill
|
Closes: #11760