Workflow: PR Conditional Build and Push Image to Quay.io Repo

[dlaw4608]

Closes: #241

To solve the issue of pull requests (PRs) coming from forks not being able to trigger the build and push of images, a new workflow has been implemented.

Summary:

This workflow leverages the pull_request_target event and performs the build and push only if the forked_image_approved label is applied to the PR. This ensures that only approved changes can trigger the image build and push process, maintaining security for secrets.

name: PR Conditional Build and Push Image to Quay.io Repo

on:
  pull_request_target:
    types: [labeled, opened, synchronize, reopened]

jobs:
  workflow-build:
    if: ${{ github.event_name == 'pull_request_target' && contains(github.event.pull_request.labels.*.name, 'forked_image_approved') }}
    name: Calls build-images-base workflow
    uses: ./.github/workflows/build-images-base.yaml
    secrets: inherit
    with:
      kuadrantOperatorVersion: ${{ github.event.pull_request.user.login }}-${{ github.event.pull_request.number }}
      kuadrantOperatorTag: ${{ github.event.pull_request.user.login }}-${{ github.event.pull_request.number }} 

Verification Steps

1. Create a Pull Request from a Fork:

• Fork the repository and make changes that affect the Docker image build.
• Create a pull request (PR) from the forked repository to the base repository.

2. Apply the forked_image_approved Label:

• As a maintainer, review the incoming PR.
• If the changes are approved, apply the forked_image_approved label to the PR.

3. Check Workflow Execution:

• Verify that the PR Conditional Build and Push Image to Quay.io Repo workflow is triggered upon applying the label.
• Ensure the workflow runs successfully, building and pushing the Docker images.

4. Verify Image Push:

• After the workflow completes, check the Quay.io repository.
• Confirm that the image is built and pushed with the tags corresponding to the PR (e.g., username-PRnumber).

Should look like this:

(https://github.com/user-attachments/assets/563717f2-9442-47fb-afbe-e0a83333e8cd)

[eguzki]

This is solving only one part of the #241

We might want to create another github issue for Images overflow in our quay.io repo

[KevFan]

nitpick: lets keep this line break so that the PR doesn't touch this file since it's doesn't change anything other than removing a line break

[KevFan]

Suggested change

	if: ${{ github.event_name == 'pull_request_target' && contains(github.event.pull_request.labels.*.name, 'forked_image_approved') }}
	if: contains(github.event.pull_request.labels.*.name, 'forked_image_approved')

I think we can simplify to this instead since we should only get events from pull_request_target defined in the on previously already

[mikenairn]

@didierofrivia re. #241, why do we want to trigger this job on pull requests?

[eguzki]

rigger this job on pull requ

In other words, why do we want to push images to quay for forked repos? Build images make sense as it is yet another test

[KevFan]

I think we should also consider to always remove the forked_image_approved label regardless of whether the image build was successfull or not as an additional security measure - https://github.com/actions-ecosystem/action-remove-labels.

And also possibly disallow multiples of this workflow from running concurrently (i.e. I'm thinking someone might first get the label and then quickly push something that could leak our secrets so we should try to prevent/mitigate this from happening) 🤔 - https://docs.github.com/en/actions/using-jobs/using-concurrency

[didierofrivia]

@eguzki @mikenairn I'd imagine for some testing during the review... would you reckon it's better to not build at all on PRs?

[eguzki]

@eguzki @mikenairn I'd imagine for some testing during the review... would you reckon it's better to not build at all on PRs?

I like building. It is "cheap" and I see it as another type of testing which needs to succeed.

I do not think we should push to quay.io kuadrant org the builds from forks. What is the value?

[KevFan]

I see this as a way for some maintainers (such as me) that typically make PRs from forks rather than branches to run the image build since the image build flow is not run these "external" PRs. If QE or anyone want to build the image they can as it can serve as another type of test as @eguzki mentioned. Downside is this does allow for a security risk given the scope that the pull_request_target has to secrets etc. Not sure is this worth the risk 🤷

It can also potentially replace the build-images-branches.yaml if we choose to only want to build at the point where the PR is made and a maintainer / QE deems it as necessary through a label. Personally I don't see always building the images on branch on every commit as necessary since I don't think it's used that often in general other than to serve as a build test, unless people actually do pull the branch builds regulary for testing 🤔