ci(oci): push PR test images to ghcr

[andrewazores]

Related to #1118
Closes #1124

[andrewazores]

@ebaron I think this approach is better than #1124 for various reasons, but this requires the action runner to have permission to push to ghcr.io/cryostat. The standard GITHUB_TOKEN doesn't have that, at least not when it's processing a pull request from a forked repository. Any thoughts on this? Seems like we would need to create some GitHub bot account, generate a PAT, and store that as a secret for the action runner to use.

[ebaron]

@ebaron I think this approach is better than #1124 for various reasons, but this requires the action runner to have permission to push to ghcr.io/cryostat. The standard GITHUB_TOKEN doesn't have that, at least not when it's processing a pull request from a forked repository. Any thoughts on this? Seems like we would need to create some GitHub bot account, generate a PAT, and store that as a secret for the action runner to use.

We could use a PAT, or maybe it's less trouble to just create a "cryostat-dev" (or similar) Quay organization where we push the PR images.

[andrewazores]

That's an option too. I think cleaning up the images after the PR is closed may be a little easier using ghcr and Actions, but it should still be doable on quay. One additional benefit of using a GitHub machine/bot account is I think the default visibility of those images would be to members of this cryostatio org only, so it they would be available for developer/reviewer ease of testing but wouldn't end up published somewhere visible for end users to pick up and use in unfinished states.

[andrewazores]

I'll try creating a very restricted PAT from my own account to add to a secret and update this CI config that way to see how it works out. If we decide to go that way then a separate machine account can be made for that purpose.

[ebaron]

I'll try creating a very restricted PAT from my own account to add to a secret and update this CI config that way to see how it works out. If we decide to go that way then a separate machine account can be made for that purpose.

Sounds good. A bot account would also be a good replacement for my PAT used to sync the web-client submodule.

[andrewazores]

I couldn't figure out the right combo of permissions to use with GitHub's new beta fine-grained PATs, but I was able to use a "classic" PAT to generate a limited token from my account that I can use to manually log in and push images to ghcr.io using the podman cli on my workstation. I added that as an org secret and updated the CI config to use that, and it's re-running now.

[andrewazores]

Gah.

Secrets are not passed to workflows that are triggered by a pull request from a fork.

That seems to kill the idea of using a PAT, even one from a machine account, to push to ghcr.io. It would only be possible when the triggering event is the PR being merged and a workflow run against the newly-updated main, not for each PR. Hmm...

[andrewazores]

There is this: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target

which is triggered when a PR is created and runs in the upstream repo's context rather than the fork's context, so it does have access to secrets. This is discouraged because it means that anyone who opens a PR can run an Action that has access to read our Secrets, so the PR could contain code that steals the secrets and records them elsewhere.

That's all true and a good security consideration, but we do also have the organization access settings configured so that outside collaborators (non-members of the cryostatio org) require approval for their Actions to run on every PR. If we maintain that standard and don't approve PRs from outside collaborators before reading them then we would be okay, but truthfully this means we have to be that careful about our human process to avoid leaking secrets.

[andrewazores]

There is the extra possibility that we can prevent the ghcr push job from running unless the PR has a specific label like safe-to-build applied to it. Only organization members can apply labels to PRs, so this would be an extra manual approval step from a reviewer before anyone's code can get built and run in the privileged context.

[andrewazores]

I think we can check for that label in CI, and also use Mergify to automatically apply that label if the PR author is a member of the cryostatio GH org. This should minimize or eliminate any workflow disruption for our usual contributors while also leaving safeguards in-place to prevent secret leaks from untrusted code in PRs from external collaborators.

[andrewazores]

Latest progress on my fork: https://github.com/andrewazores/cryostat/tree/ghcr-test-image , and andrewazores#6 as a demo of sorts

I modified the config there to run on my fork (linked branch is merged in my main for testing with these modifications) and push to my fork's container registry. The safe-to-test label must be applied for the container image to be run and published for testing, as well as for integration tests to be run. This label can only be applied by users with write access to the cryostatio org, so no outside collaborators. The Mergify config update should automatically apply this label to PRs if the PR author is a member of @cryostatio/reviewers. The ghcr.io/cryostatio/cryostat image also has internal visibility, so only people added to the cryostatio org can see or pull the test images.

Because the config now uses pull_request_target I don't think it will run in this PR:

https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target

This event runs in the context of the base of the pull request, rather than in the context of the merge commit, as the pull_request event does. This prevents execution of unsafe code from the head of the pull request that could alter your repository or steal any secrets you use in your workflow. This event allows your workflow to do things like label or comment on pull requests from forks. Avoid using this event if you need to build or run code from the pull request.