Push images and caches to github.repository_owner

[bennibbelink]

Should solve an issue discovered by @abachma2 in #184. Forks of cymetric do not have permission to push to the cyclus GHCR. Adding the ignore-error parameter to --cache-to should let the workflow continue without error when this happens. This should not have a significant impact the efficacy of our caching strategy since caches will be pushed whenever the workflow is run from the upstream repository cyclus/cymetric.

I intentionally did not include this parameter in publish_latest.yml and publish_release.yml. Those workflows need to push to GHCR to publish images. They will publish official images when run from the upstream repo, but any changes to the workflow should come from a branch off the upstream (NOT from a fork, otherwise this 403 forbidden will occur) because on a PR it will publish irrelevant images tagged :ci-image-cache to prove functionality of the workflow

[abachma2]

Do you plan to implement these updates to Cycamore and Cyclus as well? I know on Cyclus we have to have PRs from the cyclus repo because of pushing the images.

[bennibbelink]

Yes I plan to implement these updates in the other repos. They will need some additional changes because they perform downstream testing. I also need to figure out how to add the PR comments, I now see that those permissions are also scoped to the upstream repo. Working on it now, will move this PR out of draft when ready

[gonuke]

I should have flagged this in my review since we've seen it in other repos in the past, and maybe even the prior CI strategy here - any pushes to GHCR have to come from branches on the upstream repo.

What if we change the location for caching to be the author rather than the repo_owner? It pushes some CI load to the author's account, but for open repos that shouldn't matter? Some collaborators have been annoyed by this in the past, but it seems like the best choice....

[bennibbelink]

Do you suggest that we push :ci-image-cache images to the author's registry too or just load them locally on the github runner? (relevant to cycamore and cyclus, not cymetric)

EDIT: Answer to my own question - we will have to just load them locally since the Dockerfiles are written to pull cyclus/cyclus_...... If we pushed upstream images to github.repository_owner we wouldn't be able to test downstream projects using the correct image
EDIT AGAIN: I wasn't able to get the local loading working correctly. Instead I define a build-context so that the downstream build uses the collaborators image in place of cyclus' image

[bennibbelink]

Current implementation pushes all images/caches to the github.repository_owner container registry. If collaborators get annoyed by this we could just only cache to/from the cyclus container registry (and ignore-fail on cache-to) without much of an impact. I will leave this to your discretion.

Unfortunately getting PR comments to work correctly required a more sophisticated approach. PRs originating from a fork do not have permission to write to the upstream. One workaround I explored was using the pull_request_target trigger which runs the workflow from the target branch, however this has security implications. Thus the solution that is implemented is in the form of an additional workflow pr_comment.yml. build_test.yml (which DOES NOT have write permission) uploads the build statuses as artifacts which are then downloaded by the pr_comment.yml workflow (which DOES have write permisson) and this workflow comments on the PR. This is more complex than I would've liked, but this is exactly what the workflow_run trigger is intended for.

We won't see the workflows pass until this is merged to main, but here is an example PR on my fork showing what this will look like.
... on the bright side, this way we can consolidate all of the build statuses into a single comment instead of a comment for each matrix element

[abachma2]

Is this ready for review @bennibbelink ?

[gonuke]

This is certainly getting complicated!!!

[gonuke]

This all makes sense to me, so I'll merge and we'll see how it all rolls together.