smee icon indicating copy to clipboard operation
smee copied to clipboard

Enable CI & push to registry for forks

Open mmlb opened this issue 3 years ago • 1 comments

Description

Enable using our GitHub Actions workflows from forks.

Why is this needed

I was developing a couple of changes to boots and trying it out in production along with @ScottGarman. The code lived in my fork and the images were being pushed up to my personal quay repository. At one point I committed a fix and left for the day, but forgot to manually build the image and push. This left @ScottGarman stuck and needing to work around my lack of push. Why must I manually push when GitHub can do it for me?

I've been after this for a while now but failed because I was trying to push the "merged" code GitHub creates for the pull_request events. This happens in the target repository's context and thus secrets are not present for PRs from forks. I had some ideas to get around this by using the pull_request_target event but that ran against some lack of features in buildx and I burned out on that effort. This is a nice compromise though. Since the build happens for the push event, this actually takes place from the source's context and can use the secrets for the fork if present. If branches are kept up to date with upstream's main then that is just as good as the pull_request's fake-merge.

How Has This Been Tested?

Tested from my fork :tada:! https://github.com/mmlb/tinkerbell-boots/actions?query=branch%3Aenable-push-to-registry-for-forks

How are existing users impacted? What migration steps/scripts do we need?

Allows for testing of code using "upstream test procedure" before opening PRs for forks. Allows for pushing to a container image registry from a fork if secrets are defined, which makes for easier dev testing.

Checklist:

I have:

  • [ ] updated the documentation and/or roadmap (if required)
  • [ ] added unit or e2e tests
  • [ ] provided instructions on how to upgrade

mmlb avatar Feb 14 '22 14:02 mmlb

Codecov Report

Merging #235 (1788eae) into main (ac346cb) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 1788eae differs from pull request most recent head 80c523a. Consider uploading reports for the commit 80c523a to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #235   +/-   ##
=======================================
  Coverage   39.70%   39.70%           
=======================================
  Files          39       39           
  Lines        2758     2758           
=======================================
  Hits         1095     1095           
  Misses       1574     1574           
  Partials       89       89           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ac346cb...80c523a. Read the comment docs.

codecov[bot] avatar Feb 14 '22 15:02 codecov[bot]

Code base has moved on quite a bit. Gonna close this out, feel free to reopen.

jacobweinstock avatar Apr 04 '23 15:04 jacobweinstock