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

Generate release builds with github actions #337

Merged
merged 9 commits into from
Mar 29, 2021

Conversation

passcod
Copy link
Contributor

@passcod passcod commented Mar 21, 2021

Submitted for consideration re #66

Obviously not end-to-end tested against this repo.

Some choices/questions:

  • I've named the Windows and macOS sections with their isa (x86-64) even though that's currently the only option, in the optic that perhaps there would eventually be builds for Windows ARM or Apple Silicon.

  • I've selected tar.gz as the archive format (zip on windows) as it's most common but it could be xz or zstd... as you wish

  • I've included the readme, changelog, and license files in the archive. Maybe that's not necessary? Or should the audit.toml.example file be included as well for even more of a batteries-included ux?

  • Should the build be done with --locked to respect the lockfile?

  • Should checksums be generated? (not sure how to do that due to the job layout / parallelism though)

  • Though this is a 1st party github service, some of the actions used are not (from action-rs and softprops). I'm sure it would be possible to avoid these, though that's beyond the level of effort I'm okay expending for this tbqh.

@tarcieri
Copy link
Member

Thank you! This definitely looks much closer to what I had originally proposed in #66 than any other attempt so far.

I'll review it in more depth when I have time.

@tarcieri
Copy link
Member

tarcieri commented Mar 22, 2021

To answer your questions:

  • 1,2,3: that's fine
  • Using --locked is an interesting question. I'm in favor because it provides the easiest audibility in terms of what dependencies were used in a particular release. If we have vulnerable dependencies we can bump them in Cargo.lock and cut another release
  • Can probably punt on a checksum for now. Unless checksums are signed or stored out-of-band they're not particularly useful, and if someone really wants them it seems like something that can be added as a follow-up
  • For 3rd party actions, I'd suggest pinning to the latest releases (i.e. v1.0.6 for actions-rs/toolchain, v0.1.5
    for softprops/action-gh-release) and then we can audit those releases before using them as part of the build, and then audit them again if we ever need to bump the versions

@passcod
Copy link
Contributor Author

passcod commented Mar 23, 2021

Looks like softprops' v1 is actually a tag (which is different from the 0.1.5 tag), but is not listed as a release. I've asked for clarification.

@passcod
Copy link
Contributor Author

passcod commented Mar 23, 2021

I've switched the naming scheme around (swapped version and target) so it's compatible out of the box with tools like cargo-binstall.

@tarcieri
Copy link
Member

Looks like softprops' v1 is actually a tag (which is different from the 0.1.5 tag), but is not listed as a release. I've asked for clarification.

Perhaps I've been misunderstanding how GitHub Actions work, but I thought releases were handled via the Actions Marketplace:

https://github.com/marketplace/actions/gh-release?version=v0.1.5

Are you saying that the tag in the repo may not correspond to what they released?

@passcod
Copy link
Contributor Author

passcod commented Mar 23, 2021

Oh, hmm. I'm not entirely sure how that works.

But the version that's set in the workflow currently (and which I also use elsewhere) is clearly v1, and yet the latest on the actions page is "0.1.5". The 0.1.5 tag is about two weeks before the v1 tag. I don't know precisely how github actions version resolution works, but this page seems to suggests it's based on tags and commits, not restricted to anything published via marketplace:

https://docs.github.com/en/actions/learn-github-actions/security-hardening-for-github-actions#using-third-party-actions

That page also recommends pinning via commit SHAs, I could do that

@tarcieri
Copy link
Member

That page also recommends pinning via commit SHAs, I could do that

Sounds good

@passcod
Copy link
Contributor Author

passcod commented Mar 24, 2021

I've pinned with SHAs so that's no longer a concern anyway, but for completeness softprops has confirmed that the v1 tag is an actual release (i.e. won't change to another commit).

Copy link
Member

@tarcieri tarcieri left a comment

Choose a reason for hiding this comment

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

In general I'd say this looks good and I'm interested in giving it a try.

I can try tagging a v0.14.1 release to test it out.

Edit: okay, spotted one small nit after I approved it 😉

@tarcieri
Copy link
Member

I'll more thoroughly review the actions-rs and softprops actions before merging.

We already extensively make use of actions-rs, but I am noticing that it appears to be decomposed into multiple repos in a way that could evade pinning as part of a software supply chain attack.

@tarcieri tarcieri merged commit 64ef899 into rustsec:main Mar 29, 2021
@passcod passcod deleted the ga-release branch March 29, 2021 19:25
This was referenced Apr 29, 2021
@tarcieri
Copy link
Member

@passcod well, it ran for the first time. Seems it had some trouble linking with OpenSSL on Linux:

https://github.com/RustSec/cargo-audit/runs/2467228358?check_suite_focus=true

run pkg_config fail: "`\"pkg-config\" \"--libs\" \"--cflags\" \"openssl\"` did not exit successfully: exit code: 1\n--- stderr\nPackage openssl was not found in the pkg-config search path.\nPerhaps you should add the directory containing `openssl.pc\'\nto the PKG_CONFIG_PATH environment variable\nNo package \'openssl\' found\n"

  --- stderr
  thread 'main' panicked at '

  Could not find directory of OpenSSL installation, and this `-sys` crate cannot
  proceed without this knowledge. If OpenSSL is installed and this crate had
  trouble finding it,  you can set the `OPENSSL_DIR` environment variable for the
  compilation process.

  Make sure you also have the development packages of openssl installed.
  For example, `libssl-dev` on Ubuntu or `openssl-devel` on Fedora.

I guess there's a tricky question here of how to link OpenSSL. I've never statically linked with the openssl-sys crate.

I'm guessing the root cause of this actual problem though is the libssl-dev package isn't installed?

@passcod
Copy link
Contributor Author

passcod commented Apr 29, 2021

No, it likely is installed, but not for cross compiling. The x86-64 linux gnu build has compiled openssl-sys just fine. Depends what you want to do, but perhaps vendoring openssl for the arm and musl targets is a good idea?

@tarcieri
Copy link
Member

Yes, that's fine

@tarcieri
Copy link
Member

@passcod seems there are still problems with this:
https://github.com/rustsec/rustsec/actions/runs/1222043962

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.

None yet

2 participants