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

[mikenairn]

@didierofrivia re. https://github.com/Kuadrant/kuadrant-operator/issues/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

[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 🤔

[KevFan]

Going to close this as it doesn't look like it's a priority and not sure is this worth the security risk given the scope that the pull_request_target has to secrets etc. and the mitigations that also need to be added to use this.

Will re-open if we want to explore this again.