Skip to content

Commit

Permalink
Extract checkout target commit to a composite action
Browse files Browse the repository at this point in the history
We already had to do this very sensitive part of the workflow in
three places, so it makes sense to extract it to a composite
action. This means that we need to do one more checkout from the
target branch first (otherwise the action would not be available)
but we avoid code duplication and fragility this way.
  • Loading branch information
potiuk committed Apr 2, 2024
1 parent ab5aabe commit ded8a29
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 134 deletions.
78 changes: 78 additions & 0 deletions .github/actions/checkout_target_commit/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
#
---
name: 'Checkout target commit'
description: >
Checks out target commit with the exception of .github scripts directories that come from the target branch
inputs:
target-commit-sha:
description: 'SHA of the target commit to checkout'
required: true
pull-request-target:
description: 'Whether the workflow is a pull request target workflow'
required: true
is-committer-build:
description: 'Whether the build is done by a committer'
required: true
runs:
using: "composite"
steps:
- name: "Checkout target commit"
uses: actions/checkout@v4
with:
ref: ${{ inputs.target-commit-sha }}
persist-credentials: false
####################################################################################################
# BE VERY CAREFUL HERE! THIS LINE AND THE END OF THE WARNING. IN PULL REQUEST TARGET WORKFLOW
# WE CHECK OUT THE TARGET COMMIT ABOVE TO BE ABLE TO BUILD THE IMAGE FROM SOURCES FROM THE
# INCOMING PR, RATHER THAN FROM TARGET BRANCH. THIS IS A SECURITY RISK, BECAUSE THE PR
# CAN CONTAIN ANY CODE AND WE EXECUTE IT HERE. THEREFORE, WE NEED TO BE VERY CAREFUL WHAT WE
# DO HERE. WE SHOULD NOT EXECUTE ANY CODE THAT COMES FROM THE PR. WE SHOULD NOT RUN ANY BREEZE
# COMMAND NOR SCRIPTS NOR COMPOSITE ACTIONS. WE SHOULD ONLY RUN CODE THAT IS EMBEDDED DIRECTLY IN
# THIS WORKFLOW - BECAUSE THIS IS THE ONLY CODE THAT WE CAN TRUST.
####################################################################################################
- name: Checkout target branch to 'target-airflow' folder to use ci/scripts and breeze from there.
uses: actions/checkout@v4
with:
path: "target-airflow"
ref: ${{ github.base_ref }}
persist-credentials: false
if: inputs.pull-request-target == 'true' && inputs.is-committer-build != 'true'
- name: >
Replace "scripts/ci", "dev", ".github/actions" and ".github/workflows" with the target branch
so that the those directories are not coming from the PR
shell: bash
run: |
echo
echo -e "\033[33m Replace scripts, dev, actions with target branch for non-committer builds!\033[0m"
echo
rm -rfv "scripts/ci"
rm -rfv "dev"
rm -rfv ".github/actions"
rm -rfv ".github/workflows"
mv -v "target-airflow/scripts/ci" "scripts"
mv -v "target-airflow/dev" "."
mv -v "target-airflow/.github/actions" "target-airflow/.github/workflows" ".github"
if: inputs.pull-request-target == 'true' && inputs.is-committer-build != 'true'
####################################################################################################
# AFTER IT'S SAFE. THE `dev`, `scripts/ci` AND `.github/actions` ARE NOW COMING FROM THE
# BASE_REF - WHICH IS THE TARGET BRANCH OF THE PR. WE CAN TRUST THAT THOSE SCRIPTS ARE SAFE TO RUN.
# ALL THE REST OF THE CODE COMES FROM THE PR, AND FOR EXAMPLE THE CODE IN THE `Dockerfile.ci` CAN
# BE RUN SAFELY AS PART OF DOCKER BUILD. BECAUSE IT RUNS INSIDE THE DOCKER CONTAINER AND IT IS
# ISOLATED FROM THE RUNNER.
####################################################################################################
51 changes: 7 additions & 44 deletions .github/workflows/ci-image-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -134,54 +134,17 @@ ${{ inputs.do-build == 'true' && inputs.image-tag || '' }}"
shell: bash
run: docker run -v "${GITHUB_WORKSPACE}:/workspace" -u 0:0 bash -c "rm -rf /workspace/*"
if: inputs.do-build == 'true'
- uses: actions/checkout@v4
- name: "Checkout target branch"
uses: actions/checkout@v4
with:
ref: ${{ inputs.target-commit-sha }}
persist-credentials: false
- name: "Checkout target commit"
uses: ./.github/actions/checkout_target_commit
if: inputs.do-build == 'true'
####################################################################################################
# BE VERY CAREFUL HERE! THIS LINE AND THE END OF THE WARNING. IN PULL REQUEST TARGET WORKFLOW
# WE CHECK OUT THE TARGET COMMIT ABOVE TO BE ABLE TO BUILD THE IMAGE FROM SOURCES FROM THE
# INCOMING PR, RATHER THAN FROM TARGET BRANCH. THIS IS A SECURITY RISK, BECAUSE THE PR
# CAN CONTAIN ANY CODE AND WE EXECUTE IT HERE. THEREFORE, WE NEED TO BE VERY CAREFUL WHAT WE
# DO HERE. WE SHOULD NOT EXECUTE ANY CODE THAT COMES FROM THE PR. WE SHOULD NOT RUN ANY BREEZE
# COMMAND NOR SCRIPTS NOR COMPOSITE ACTIONS. WE SHOULD ONLY RUN CODE THAT IS EMBEDDED DIRECTLY IN
# THIS WORKFLOW - BECAUSE THIS IS THE ONLY CODE THAT WE CAN TRUST.
####################################################################################################
- name: Checkout target branch to 'target-airflow' folder to use ci/scripts and breeze from there.
uses: actions/checkout@v4
with:
path: "target-airflow"
ref: ${{ github.base_ref }}
persist-credentials: false
if: >
inputs.do-build == 'true' && inputs.pull-request-target == 'true' &&
inputs.is-committer-build != 'true'
- name: >
Replace "scripts/ci", "dev", ".github/actions" and ".github/workflows" with the target branch
so that the those directories are not coming from the PR
shell: bash
run: |
echo
echo -e "\033[33m Replace scripts, dev, actions with target branch for non-committer builds!\033[0m"
echo
rm -rfv "scripts/ci"
rm -rfv "dev"
rm -rfv ".github/actions"
rm -rfv ".github/workflows"
mv -v "target-airflow/scripts/ci" "scripts"
mv -v "target-airflow/dev" "."
mv -v "target-airflow/.github/actions" "target-airflow/.github/workflows" ".github"
if: >
inputs.do-build == 'true' && inputs.pull-request-target == 'true' &&
inputs.is-committer-build != 'true'
####################################################################################################
# HERE IT'S A BIT SAFER. THE `dev`, `scripts/ci` AND `.github/actions` ARE NOW COMING FROM THE
# BASE_REF - WHICH IS THE TARGET BRANCH OF THE PR. WE CAN TRUST THAT THOSE SCRIPTS ARE SAVE TO RUN.
# ALL THE REST OF THE CODE COMES FROM THE PR, AND FOR EXAMPLE THE CODE IN THE `Dockerfile.ci` CAN
# BE RUN SAFELY AS PART OF DOCKER BUILD. BECAUSE IT RUNS INSIDE THE DOCKER CONTAINER AND IT IS
# ISOLATED FROM THE RUNNER.
####################################################################################################
target-commit-sha: ${{ inputs.target-commit-sha }}
pull-request-target: ${{ inputs.pull-request-target }}
is-committer-build: ${{ inputs.is-committer-build }}
- name: "Cleanup docker"
run: ./scripts/ci/cleanup_docker.sh
if: inputs.do-build == 'true'
Expand Down
106 changes: 16 additions & 90 deletions .github/workflows/prod-image-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -127,54 +127,17 @@ jobs:
shell: bash
run: docker run -v "${GITHUB_WORKSPACE}:/workspace" -u 0:0 bash -c "rm -rf /workspace/*"
if: inputs.do-build == 'true' && inputs.upload-package-artifact == 'true'
- uses: actions/checkout@v4
with:
ref: ${{ inputs.target-commit-sha }}
persist-credentials: false
if: inputs.do-build == 'true' && inputs.upload-package-artifact == 'true'
####################################################################################################
# BE VERY CAREFUL HERE! THIS LINE AND THE END OF THE WARNING. IN PULL REQUEST TARGET WORKFLOW
# WE CHECK OUT THE TARGET COMMIT ABOVE TO BE ABLE TO BUILD THE IMAGE FROM SOURCES FROM THE
# INCOMING PR, RATHER THAN FROM TARGET BRANCH. THIS IS A SECURITY RISK, BECAUSE THE PR
# CAN CONTAIN ANY CODE AND WE EXECUTE IT HERE. THEREFORE, WE NEED TO BE VERY CAREFUL WHAT WE
# DO HERE. WE SHOULD NOT EXECUTE ANY CODE THAT COMES FROM THE PR. WE SHOULD NOT RUN ANY BREEZE
# COMMAND NOR SCRIPTS NOR COMPOSITE ACTIONS. WE SHOULD ONLY RUN CODE THAT IS EMBEDDED DIRECTLY IN
# THIS WORKFLOW - BECAUSE THIS IS THE ONLY CODE THAT WE CAN TRUST.
####################################################################################################
- name: Checkout target branch to 'target-airflow' folder to use ci/scripts and breeze from there.
- name: "Checkout target branch"
uses: actions/checkout@v4
with:
path: "target-airflow"
ref: ${{ github.base_ref }}
persist-credentials: false
if: >
inputs.do-build == 'true' && inputs.pull-request-target == 'true' &&
inputs.is-committer-build != 'true' && inputs.upload-package-artifact == 'true'
- name: >
Replace "scripts/ci", "dev", ".github/actions" and ".github/workflows" with the target branch
so that the those directories are not coming from the PR
shell: bash
run: |
echo
echo -e "\033[33m Replace scripts, dev, actions with target branch for non-committer builds!\033[0m"
echo
rm -rfv "scripts/ci"
rm -rfv "dev"
rm -rfv ".github/actions"
rm -rfv ".github/workflows"
mv -v "target-airflow/scripts/ci" "scripts"
mv -v "target-airflow/dev" "."
mv -v "target-airflow/.github/actions" "target-airflow/.github/workflows" ".github"
if: >
inputs.do-build == 'true' && inputs.pull-request-target == 'true' &&
inputs.is-committer-build != 'true'
####################################################################################################
# HERE IT'S A BIT SAFER. THE `dev`, `scripts/ci` AND `.github/actions` ARE NOW COMING FROM THE
# BASE_REF - WHICH IS THE TARGET BRANCH OF THE PR. WE CAN TRUST THAT THOSE SCRIPTS ARE SAVE TO RUN.
# ALL THE REST OF THE CODE COMES FROM THE PR, AND FOR EXAMPLE THE CODE IN THE `Dockerfile.ci` CAN
# BE RUN SAFELY AS PART OF DOCKER BUILD. BECAUSE IT RUNS INSIDE THE DOCKER CONTAINER AND IT IS
# ISOLATED FROM THE RUNNER.
####################################################################################################
- name: "Checkout target commit"
uses: ./.github/actions/checkout_target_commit
with:
target-commit-sha: ${{ inputs.target-commit-sha }}
pull-request-target: ${{ inputs.pull-request-target }}
is-committer-build: ${{ inputs.is-committer-build }}
if: inputs.do-build == 'true' && inputs.upload-package-artifact == 'true'
- name: "Cleanup docker"
run: ./scripts/ci/cleanup_docker.sh
if: inputs.do-build == 'true' && inputs.upload-package-artifact == 'true'
Expand Down Expand Up @@ -263,54 +226,17 @@ ${{ inputs.do-build == 'true' && inputs.image-tag || '' }}"
shell: bash
run: docker run -v "${GITHUB_WORKSPACE}:/workspace" -u 0:0 bash -c "rm -rf /workspace/*"
if: inputs.do-build == 'true'
- uses: actions/checkout@v4
with:
ref: ${{ inputs.target-commit-sha }}
persist-credentials: false
if: inputs.do-build == 'true'
####################################################################################################
# BE VERY CAREFUL HERE! THIS LINE AND THE END OF THE WARNING. IN PULL REQUEST TARGET WORKFLOW
# WE CHECK OUT THE TARGET COMMIT ABOVE TO BE ABLE TO BUILD THE IMAGE FROM SOURCES FROM THE
# INCOMING PR, RATHER THAN FROM TARGET BRANCH. THIS IS A SECURITY RISK, BECAUSE THE PR
# CAN CONTAIN ANY CODE AND WE EXECUTE IT HERE. THEREFORE, WE NEED TO BE VERY CAREFUL WHAT WE
# DO HERE. WE SHOULD NOT EXECUTE ANY CODE THAT COMES FROM THE PR. WE SHOULD NOT RUN ANY BREEZE
# COMMAND NOR SCRIPTS NOR COMPOSITE ACTIONS. WE SHOULD ONLY RUN CODE THAT IS EMBEDDED DIRECTLY IN
# THIS WORKFLOW - BECAUSE THIS IS THE ONLY CODE THAT WE CAN TRUST.
####################################################################################################
- name: Checkout target branch to 'target-airflow' folder to use ci/scripts and breeze from there.
- name: "Checkout target branch"
uses: actions/checkout@v4
with:
path: "target-airflow"
ref: ${{ github.base_ref }}
persist-credentials: false
if: >
inputs.do-build == 'true' && inputs.pull-request-target == 'true' &&
inputs.is-committer-build != 'true'
- name: >
Replace "scripts/ci", "dev", ".github/actions" and ".github/workflows" with the target branch
so that the those directories are not coming from the PR
shell: bash
run: |
echo
echo -e "\033[33m Replace scripts, dev, actions with target branch for non-committer builds!\033[0m"
echo
rm -rfv "scripts/ci"
rm -rfv "dev"
rm -rfv ".github/actions"
rm -rfv ".github/workflows"
mv -v "target-airflow/scripts/ci" "scripts"
mv -v "target-airflow/dev" "."
mv -v "target-airflow/.github/actions" "target-airflow/.github/workflows" ".github"
if: >
inputs.do-build == 'true' && inputs.pull-request-target == 'true' &&
inputs.is-committer-build != 'true'
####################################################################################################
# HERE IT'S A BIT SAFER. THE `dev`, `scripts/ci` AND `.github/actions` ARE NOW COMING FROM THE
# BASE_REF - WHICH IS THE TARGET BRANCH OF THE PR. WE CAN TRUST THAT THOSE SCRIPTS ARE SAVE TO RUN.
# ALL THE REST OF THE CODE COMES FROM THE PR, AND FOR EXAMPLE THE CODE IN THE `Dockerfile.ci` CAN
# BE RUN SAFELY AS PART OF DOCKER BUILD. BECAUSE IT RUNS INSIDE THE DOCKER CONTAINER AND IT IS
# ISOLATED FROM THE RUNNER.
####################################################################################################
- name: "Checkout target commit"
uses: ./.github/actions/checkout_target_commit
with:
target-commit-sha: ${{ inputs.target-commit-sha }}
pull-request-target: ${{ inputs.pull-request-target }}
is-committer-build: ${{ inputs.is-committer-build }}
if: inputs.do-build == 'true'
- name: "Cleanup docker"
run: ./scripts/ci/cleanup_docker.sh
if: inputs.do-build == 'true'
Expand Down

0 comments on commit ded8a29

Please sign in to comment.