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

Add GH workflow to build and release on PyPI (WIP) #1933

Closed
wants to merge 4 commits into from

Conversation

lukpueh
Copy link
Member

@lukpueh lukpueh commented Mar 30, 2022

Fixes #1550

This PR adds a workflow with two jobs to build and release on PyPI. The release job waits for the build job and uses a custom release environment, which can be configured to require review.

To share the build artifacts between the jobs and to make them available for intermediate review, we use actions/upload-artifact (in build) and actions/download-artifact (in release) (also see GitHub docs).

To upload the build artifacts to PyPI, we use pypa/gh-action-pypi-publish as recommend by PyPA.

A demo is available in my fork.

Caveat
The URL to grab the build artifacts created in a workflow run requires knowledge of the corresponding action ID and artifact ID, plus a login token (w/o special permissions). This makes it a bit cumbersome to fetch the artifacts with a script in order to compare them to a local build. It might be easier to instruct reviewer to download the artifacts manually using the GitHub UI, and tailor the local verification script accordingly.

TODO

  • Change repository_url to release on PyPI instead of Test PyPI and update the token
  • Require successful CI run for a pushed tag to trigger the workflow, instead of just a push (see workflow_run docs)
  • Integrate workflow with local verification (see caveat above)
  • Add "Release on GitHub" job to workflow
    Note, this could be akin to and in parallel with the "release-on-pypi" job, waiting for the same review approval, and uploading the same build artifacts as release assets. Alternatively, we could create the release as part of the build job and upload the build artifacts as release assets right away, bypassing the download/upload caveat from above. It would be nice if we could set the release as draft at first, but that has the same caveat, i.e. it requires a token and special knowledge to download the assets.
  • Update documentation

Add workflow with two jobs to build and publish on PyPI.
The release job waits for the build job and uses a custom release
environment, which can be configured to require review.

To share the build artifacts between the jobs and to make them
available for intermedieate review, they are stored using
'actions/upload-artifact' and 'actions/download-artifact'.
https://docs.github.com/en/actions/using-workflows/storing-workflow-data-as-artifacts

To upload the build artifacts to PyPI, the PyPA recommended
'pypa/gh-action-pypi-publish' is used.
https://packaging.python.org/en/latest/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/

**Caveat**
The URL to grab the artifacts, e.g. for review, requires knowledge
of action ID and artifact ID, and a login token (no special
permissions). This makes it a bit cumbersome to fetch the artifacts
with a script and compare them to a local build.
https://docs.github.com/en/actions/managing-workflow-runs/downloading-workflow-artifacts

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@coveralls
Copy link

coveralls commented Mar 30, 2022

Pull Request Test Coverage Report for Build 2076681489

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.001%) to 98.311%

Totals Coverage Status
Change from base Build 2057963481: -0.001%
Covered Lines: 1178
Relevant Lines: 1194

💛 - Coveralls

@lukpueh
Copy link
Member Author

lukpueh commented Mar 31, 2022

Re "Release on GitHub":
Unfortunately, the GitHub-native action/create-release is unmaintained. But there is a third-party softprops/action-gh-release, which seems popular. If we use that we'd have to make an exception in our "only allow actions created by GitHub"-policy.

@jku
Copy link
Member

jku commented Mar 31, 2022

The URL to grab the build artifacts created in a workflow run requires knowledge of the corresponding action ID and artifact ID, plus a login token

😮 wow. They really are making artifacts hidden from unauthenticated users, how disappointing of GitHub. This limits the number of people able to download the artifacts to a mere 70 million accounts: a security improvement indeed 🙄

This was so close to Just Working but then... not quite.

So, assuming we do want to integrate the GitHub release into the process and also want to have the local verification as automatic as possible, a potential solution for both is:

  • add code to do the GH release (either using a 3rd party action or just adding the code, it doesn't look horribly bad for our simple case, 1 call to create release, 1 call for each asset)
  • do the GH release during the "build" job (as in before the maintainer deploy permission), but name the release like "1.1.0-rc" or something
  • now verify_release would be able to check that the release (candidate) matches a local build
  • after maintainer approval, the "deploy" job would take the release artifacts and push them to pypi. GH release can be renamed to "1.1.0"

It's definitely more work than I expected but maybe doable? We would not need to handle edge cases since we could just say e.g. that maintainers need to delete failed -rc releases before trying to re-release

@jku
Copy link
Member

jku commented Mar 31, 2022

I think using the pypa/gh-action-pypi-publish action seems fine... but also it literally just calls twine check and twine upload and that's it. We could do the same manually (and this would allow running twine check in the build phase instead of failing in deploy)

@jku
Copy link
Member

jku commented Mar 31, 2022

release environment, which can be configured to require review.

To clarify, this means configured in GitHub project/environment settings, not in the workflow?

Are we defining that all maintainers can deploy? I think this might be fine: Current pypi.org project maintainers is a subset of all maintainers but that makes sense as you can upload anything with pypi.org access. With deploy permission you can only release from git sources...

@lukpueh
Copy link
Member Author

lukpueh commented Mar 31, 2022

😮 wow. They really are making artifacts hidden from unauthenticated users, how disappointing of GitHub. This limits the number of people able to download the artifacts to a mere 70 million accounts: a security improvement indeed 🙄

Yep, others too have expressed their doubt about the security benefit of this restriction.

This was so close to Just Working but then... not quite.

Yep, I was really excited when I discovered the download/upload feature... and then quickly somewhat disappointed.

So, assuming we do want to integrate the GitHub release into the process and also want to have the local verification as automatic as possible, a potential solution for both is:
[...]

Thanks for thinking this through with me! That's one of the approaches I had in mind. I'll explore in that direction then.

@lukpueh
Copy link
Member Author

lukpueh commented Mar 31, 2022

I think using the pypa/gh-action-pypi-publish action seems fine... but also it literally just calls twine check and twine upload and that's it. We could do the same manually (and this would allow running twine check in the build phase instead of failing in deploy)

Right, I forgot to mention that this is a 3rd-party action too. I was less worried since it seems endorsed by the pypa. But yeah, if it just calls twine, we might as call it ourselves.

I'll prepare a POC using pypa/gh-action-pypi-publish and softprops/action-gh-release, and in a next iteration will try to replace them with custom code.

@lukpueh
Copy link
Member Author

lukpueh commented Mar 31, 2022

release environment, which can be configured to require review.

To clarify, this means configured in GitHub project/environment settings, not in the workflow?

Yes! Environments and protection rules like "requires review" are configured in the GitHub repo settings (see GH docs).

Are we defining that all maintainers can deploy? I think this might be fine: Current pypi.org project maintainers is a subset of all maintainers but that makes sense as you can upload anything with pypi.org access. With deploy permission you can only release from git sources...

I haven't thought this far. I probably would have started with requiring a review from one of the maintainers who currently also has pypi.org access. I am not sure if the git sources constraint is a good argument for broadening the subset, because git sources can be anything.

@jku
Copy link
Member

jku commented Mar 31, 2022

One more thing (that is probably future work as well): is signing the release still relevant?

I'm not sure if anyone uses our signatures for anything... but we could:

@lukpueh
Copy link
Member Author

lukpueh commented Apr 1, 2022

One more thing (that is probably future work as well): is signing the release still relevant?

I'm not sure if anyone uses our signatures for anything... but we could:

I'm in favor of the latter, GPG isn't relevant for PyPI anymore, and IIUC this is also the right direction towards generating in-toto attestations for our supply chain (I really need to catch up on the latest in-toto/sigstore developments).

@jku
Copy link
Member

jku commented Apr 1, 2022

Even more future improvements (there's a lot of details in this one PR/discussion, no need to fix everything in one go):

For standard auditability reasons, the build log should include the hashes (and/or signatures ) of the artifacts.

- Publish GitHub release candiate as part of the build process
  -> makes integration with local verify_release before review
     easier
- Add --skip-pypi flag to verify_release script for pre-release
  check between build and release job (before review approval)
- Tweak verify_release to work with my fork
- Comment out release on (test) pypi step in release job, because
  verify_release currently only works with vX.Y.Z releases, and
  I don't want to push them to test pypi for ongoing prototyping,
  because we can't reuse a version later when we want to test an
  an actual release.
- Add finalize GitHub release step to release job (does not work
  yet, error: release already exists)
@lukpueh lukpueh closed this Apr 1, 2022
@lukpueh lukpueh deleted the auto-release branch April 1, 2022 10:57
@lukpueh
Copy link
Member Author

lukpueh commented Apr 1, 2022

Damn, I think can't re-open the PR after having deleted the remote branch in my fork, even though I have re-pushed it. Will need to check. Otherwise, I'll create a new one migrating the key points in the discussion here.

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.

Automated release builds
3 participants