fix: fix failing docker CI due to permission issues on PRs (#94)

Co-authored-by: Rootul P <[email protected]>
celestiaorg · Feb 7, 2024 · 813fe47 · 813fe47
1 parent b88ab2a
commit 813fe47
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 3 deletions.
diff --git a/.github/workflows/dockerfile_workflow_test.yaml b/.github/workflows/dockerfile_workflow_test.yaml
@@ -1,6 +1,16 @@
+#
+# This workflow is used to test the `reusable_dockerfile_pipeline` action used
+# to build and push the Docker image to the container registries.
+#
+# The reason this workflow targets the develop branch is so that we can test the
+# action in the PR. If we targeted main, we would need to merge changes into main
+# before being able to test them.
+#
 name: Build Using Reusable Workflow
 on: [push, pull_request]
 jobs:
+  # reusable-build tests calling the reusable_dockerfile_pipeline while
+  # providing a custom packageName
   reusable-build:
     permissions:
       contents: write
@@ -10,6 +20,9 @@ jobs:
       dockerfile: docker-action-test/Dockerfile
       packageName: docker-test
     secrets: inherit
+
+  # reusable-build-defaults tests calling the reusable_dockerfile_pipeline with
+  # the defaults
   reusable-build-defaults:
     permissions:
       contents: write

diff --git a/.github/workflows/reusable_dockerfile_pipeline.yml b/.github/workflows/reusable_dockerfile_pipeline.yml
@@ -172,7 +172,7 @@ jobs:
           severity: "CRITICAL,HIGH"
 
   docker-build:
-    name: docker-build (${{ matrix.registry.name }}; ${{ matrix.registry.registry-url }}/${{ matrix.registry.registry-owner }}/${{ needs.prepare-env.outputs.output_image_name }}:${{ needs.prepare-env.outputs.output_short_sha }})
+    name: docker-build (${{ matrix.registry.name }}; ${{ matrix.registry.registry-url }}/${{ matrix.registry.registry-owner }}/${{ needs.prepare-env.outputs.output_image_name }})
     runs-on: "ubuntu-latest"
     # wait until the jobs are finished.
     needs: ["prepare-env", "logic-check", "docker-security"]
@@ -181,35 +181,48 @@ jobs:
       packages: write
     strategy:
       matrix:
+        # run-on-pr is used to skip running registries that are expected to fail
+        # due to github permission issues with org wide secrets.
         registry:
           - name: DockerHub
             user-secret: DOCKERHUB_USERNAME
             token-secret: DOCKERHUB_TOKEN
             registry-url: docker.io
             registry-owner: celestiaorg
+            run-on-pr: "false"
           - name: GHCR
             user-secret: ${{ github.repository_owner }}
             token-secret: GITHUB_TOKEN
             registry-url: ghcr.io
             registry-owner: ${{ needs.prepare-env.outputs.repo_owner }}
+            run-on-pr: "true"
           - name: ScaleWay
             user-secret: SCALEWAY_USERNAME
             token-secret: SCW_SECRET_KEY
             registry-url: rg.fr-par.scw.cloud
             registry-owner: celestiaorg
+            run-on-pr: "false"
       fail-fast: false
     steps:
+      - name: Check run conditions
+        id: run_check
+        # We only want to run when the registry is able to run on PR or if it is a merge event
+        run: echo "run=${{ matrix.registry.run-on-pr == needs.prepare-env.outputs.build_for_pr || needs.prepare-env.outputs.build_for_merge == 'true'}}" >> "$GITHUB_OUTPUT"
+
       - name: Checkout
+        if: ${{ steps.run_check.outputs.run == 'true'}}
         uses: "actions/checkout@v4"
 
       - name: Login to ${{ matrix.registry.name }}
+        if: ${{ steps.run_check.outputs.run == 'true'}}
         uses: docker/login-action@v3
         with:
           registry: ${{ matrix.registry.registry-url }}
           username: ${{ matrix.registry.registry-url == env.GITHUB_REG && matrix.registry.user-secret || secrets[matrix.registry.user-secret] }}
           password: ${{ secrets[matrix.registry.token-secret] }}
 
       - name: Extract Docker Metadata
+        if: ${{ steps.run_check.outputs.run == 'true'}}
         id: meta
         uses: docker/metadata-action@v5
         env:
@@ -233,9 +246,11 @@ jobs:
           # yamllint enable
 
       - name: Set up QEMU
+        if: ${{ steps.run_check.outputs.run == 'true'}}
         uses: docker/setup-qemu-action@v3
 
       - name: Set up Docker Buildx
+        if: ${{ steps.run_check.outputs.run == 'true'}}
         uses: docker/setup-buildx-action@v3
 
       # Build and Publish images on main, master, and versioned branches.
@@ -245,7 +260,7 @@ jobs:
       # the amd64 image since building the arm64 image takes significantly
       # longer.
       - name: "Merge on Main Trigger: Build and Push All Docker Images"
-        if: ${{ needs.prepare-env.outputs.build_for_merge == 'true' }}
+        if: ${{ needs.prepare-env.outputs.build_for_merge == 'true' && steps.run_check.outputs.run == 'true'}}
         uses: docker/build-push-action@v5
         env:
           OUTPUT_SHORT_SHA: ${{ needs.prepare-env.outputs.output_short_sha }}
@@ -265,7 +280,7 @@ jobs:
       # forks can't push, we still want to try and build the image to catch
       # bugs. For testing purposes we only need an amd64 image.
       - name: "Pull Request Trigger: Build and Push amd64 Docker Image"
-        if: ${{ needs.prepare-env.outputs.build_for_pr == 'true' }}
+        if: ${{ needs.prepare-env.outputs.build_for_pr == 'true' && steps.run_check.outputs.run == 'true'}}
         uses: docker/build-push-action@v5
         env:
           OUTPUT_SHORT_SHA: ${{ needs.prepare-env.outputs.output_short_sha }}