From b3ad0c3014c3447f12d8f13a5cc1291d93f78c7d Mon Sep 17 00:00:00 2001 From: Michael Vasseur Date: Sun, 2 Jun 2024 00:17:22 +0200 Subject: [PATCH 1/5] Annotate GitHub actions log The current build fails and the log is very hard to read --- .../workflows/build-domjudge-container-PR.yml | 2 + docker/build.sh | 50 +++++++++++++++++-- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build-domjudge-container-PR.yml b/.github/workflows/build-domjudge-container-PR.yml index 49141223..1f607233 100644 --- a/.github/workflows/build-domjudge-container-PR.yml +++ b/.github/workflows/build-domjudge-container-PR.yml @@ -66,10 +66,12 @@ jobs: - name: Build and push run: | for IMG in domserver judgehost default-judgehost-chroot; do + echo "::group::$IMG" IMAGE_NAME="${GITHUB_REPOSITORY_OWNER@L}/$IMG:${{ env.DOMJUDGE_VERSION }}" docker image tag "$IMAGE_NAME" ghcr.io/${GITHUB_REPOSITORY_OWNER@L}/$IMG:${{ env.PR_TAG }} docker image tag "$IMAGE_NAME" ${GITHUB_REPOSITORY_OWNER@L}/$IMG:${{ env.PR_TAG }} docker push ghcr.io/${GITHUB_REPOSITORY_OWNER@L}/$IMG:${{ env.PR_TAG }} + echo "::endgroup::" done - name: Check for wrong permisions diff --git a/docker/build.sh b/docker/build.sh index 525576ec..1079e330 100755 --- a/docker/build.sh +++ b/docker/build.sh @@ -1,9 +1,43 @@ -#!/bin/sh -eu +#!/bin/sh -eux -if [ -n "${CI+}" ] +# Placeholders to annotate the Github actions logs +trace_on () { true; } +trace_off () { true; } +section_start () { true; } +section_end () { true; } + +if [ -n "${CI+x}" ] then + if [ -n "${GITHUB_ACTION+x}" ] set -x - export PS4='(${0}:${LINENO}): - [$?] $ ' + then + # Functions to annotate the Github actions logs + trace_on () { set -x; } + trace_off () { + { set +x; } 2>/dev/null + } + + section_start_internal () { + echo "::group::$1" + trace_on + } + + section_end_internal () { + echo "::endgroup::" + trace_on + } + + section_start () { + trace_off + section_start_internal "$@" + } + section_end () { + trace_off + section_end_internal + } + else + export PS4='(${0}:${LINENO}): - [$?] $ ' + fi fi if [ "$#" -eq 0 ] || [ "$#" -gt 2 ] @@ -24,6 +58,7 @@ fi URL=https://www.domjudge.org/releases/domjudge-${VERSION}.tar.gz FILE=domjudge.tar.gz +section_start "Download DOMjudge tarball" echo "[..] Downloading DOMjudge version ${VERSION}..." if ! wget --quiet "${URL}" -O ${FILE} @@ -33,19 +68,27 @@ then fi echo "[ok] DOMjudge version ${VERSION} downloaded as domjudge.tar.gz"; echo +section_end +section_start "Build domserver container" echo "[..] Building Docker image for domserver..." ./build-domjudge.sh "${NAMESPACE}/domserver:${VERSION}" echo "[ok] Done building Docker image for domserver" +section_end +section_start "Build judgehost container (with intermediate image)" echo "[..] Building Docker image for judgehost using intermediate build image..." ./build-judgehost.sh "${NAMESPACE}/judgehost:${VERSION}" echo "[ok] Done building Docker image for judgehost" +section_end +section_start "Build judgehost container (judging chroot)" echo "[..] Building Docker image for judgehost chroot..." docker build -t "${NAMESPACE}/default-judgehost-chroot:${VERSION}" -f judgehost/Dockerfile.chroot . echo "[ok] Done building Docker image for judgehost chroot" +section_end +section_start "Push instructions" echo "All done. Image ${NAMESPACE}/domserver:${VERSION} and ${NAMESPACE}/judgehost:${VERSION} created" echo "If you are a DOMjudge maintainer with access to the domjudge organization on Docker Hub, you can now run the following command to push them to Docker Hub:" echo "$ docker push ${NAMESPACE}/domserver:${VERSION} && docker push ${NAMESPACE}/judgehost:${VERSION} && docker push $NAMESPACE}/default-judgehost-chroot:${VERSION}" @@ -54,3 +97,4 @@ echo "$ docker tag ${NAMESPACE}/domserver:${VERSION} ${NAMESPACE}/domserver:late docker tag ${NAMESPACE}/judgehost:${VERSION} ${NAMESPACE}/judgehost:latest && \ docker tag ${NAMESPACE}/default-judgehost-chroot:${VERSION} ${NAMESPACE}/default-judgehost-chroot:latest && \ docker push ${NAMESPACE}/domserver:latest && docker push ${NAMESPACE}/judgehost:latest && docker push ${NAMESPACE}/default-judgehost-chroot:latest" +section_end From c1e3d42e83b940cc8ae548d189e275fa4a45002e Mon Sep 17 00:00:00 2001 From: Michael Vasseur Date: Sun, 2 Jun 2024 14:28:01 +0200 Subject: [PATCH 2/5] GitHub recommends to not use `pull_request_target` See: https://runs-on.com/github-actions/pull-request-vs-pull-request-target/ --- .github/workflows/build-contributor-container-PR.yml | 4 ++-- .github/workflows/build-domjudge-container-PR.yml | 4 ++-- .github/workflows/build-gitlab-container-PR.yml | 4 ++-- .github/workflows/shellcheck.yml | 3 --- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build-contributor-container-PR.yml b/.github/workflows/build-contributor-container-PR.yml index a45a8251..90fcc573 100644 --- a/.github/workflows/build-contributor-container-PR.yml +++ b/.github/workflows/build-contributor-container-PR.yml @@ -4,7 +4,7 @@ name: 'Build Contributor container (PR)' on: push: - pull_request_target: + pull_request: branches: - main @@ -13,7 +13,7 @@ jobs: if: ${{ !(github.repository == 'domjudge/domjudge-packaging' && github.ref == 'refs/heads/main') && !contains(github.ref, 'gh-readonly-queue') && - (github.event_name == 'pull_request_target' || + (github.event_name == 'pull_request' || github.event.pull_request.head.repo.full_name != github.repository) }} runs-on: ubuntu-latest steps: diff --git a/.github/workflows/build-domjudge-container-PR.yml b/.github/workflows/build-domjudge-container-PR.yml index 1f607233..80a170c5 100644 --- a/.github/workflows/build-domjudge-container-PR.yml +++ b/.github/workflows/build-domjudge-container-PR.yml @@ -2,7 +2,7 @@ name: 'Build domjudge container (PR)' on: push: - pull_request_target: + pull_request: branches: - main @@ -14,7 +14,7 @@ jobs: if: ${{ !(github.repository == 'domjudge/domjudge-packaging' && github.ref == 'refs/heads/main') && !contains(github.ref, 'gh-readonly-queue') && - (github.event_name == 'pull_request_target' || + (github.event_name == 'pull_request' || github.event.pull_request.head.repo.full_name != github.repository) }} runs-on: ubuntu-latest steps: diff --git a/.github/workflows/build-gitlab-container-PR.yml b/.github/workflows/build-gitlab-container-PR.yml index b2e78778..0b02c3c2 100644 --- a/.github/workflows/build-gitlab-container-PR.yml +++ b/.github/workflows/build-gitlab-container-PR.yml @@ -2,7 +2,7 @@ name: 'Build GitLab CI container (PR)' on: push: - pull_request_target: + pull_request: branches: - main @@ -11,7 +11,7 @@ jobs: if: ${{ !(github.repository == 'domjudge/domjudge-packaging' && github.ref == 'refs/heads/main') && !contains(github.ref, 'gh-readonly-queue') && - (github.event_name == 'pull_request_target' || + (github.event_name == 'pull_request' || github.event.pull_request.head.repo.full_name != github.repository) }} name: PR GitLab image runs-on: ubuntu-latest diff --git a/.github/workflows/shellcheck.yml b/.github/workflows/shellcheck.yml index 99cb18cd..c6fd65e9 100644 --- a/.github/workflows/shellcheck.yml +++ b/.github/workflows/shellcheck.yml @@ -7,9 +7,6 @@ on: pull_request: branches: - main - pull_request_target: - branches: - - main jobs: shellcheck: From 02e8da0fed8a2694a23d0bbb572d8fd0f618c65d Mon Sep 17 00:00:00 2001 From: Michael Vasseur Date: Sun, 2 Jun 2024 16:13:48 +0200 Subject: [PATCH 3/5] Don't store the resulting image for PRs Moving this out of the security scope of the repository would make that we need to store this for the `github.author`. As we never used this before it's now taken out. --- .github/workflows/build-contributor-container-PR.yml | 2 +- .github/workflows/build-domjudge-container-PR.yml | 11 +++++------ .github/workflows/build-gitlab-container-PR.yml | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build-contributor-container-PR.yml b/.github/workflows/build-contributor-container-PR.yml index 90fcc573..889466c1 100644 --- a/.github/workflows/build-contributor-container-PR.yml +++ b/.github/workflows/build-contributor-container-PR.yml @@ -41,7 +41,7 @@ jobs: with: context: docker-contributor platforms: linux/amd64,linux/arm64 - push: true + push: false tags: ${{ steps.meta.outputs.tags }} labels: ${{ steps.meta.outputs.labels }} diff --git a/.github/workflows/build-domjudge-container-PR.yml b/.github/workflows/build-domjudge-container-PR.yml index 80a170c5..ae4e8a4f 100644 --- a/.github/workflows/build-domjudge-container-PR.yml +++ b/.github/workflows/build-domjudge-container-PR.yml @@ -58,11 +58,9 @@ jobs: run: | cd docker set -x - sh ./build.sh "${{ env.DOMJUDGE_VERSION }}" ${{ github.actor }} + sh ./build.sh "${{ env.DOMJUDGE_VERSION }}" set +x - - run: docker image list - - name: Build and push run: | for IMG in domserver judgehost default-judgehost-chroot; do @@ -70,18 +68,19 @@ jobs: IMAGE_NAME="${GITHUB_REPOSITORY_OWNER@L}/$IMG:${{ env.DOMJUDGE_VERSION }}" docker image tag "$IMAGE_NAME" ghcr.io/${GITHUB_REPOSITORY_OWNER@L}/$IMG:${{ env.PR_TAG }} docker image tag "$IMAGE_NAME" ${GITHUB_REPOSITORY_OWNER@L}/$IMG:${{ env.PR_TAG }} - docker push ghcr.io/${GITHUB_REPOSITORY_OWNER@L}/$IMG:${{ env.PR_TAG }} echo "::endgroup::" done + - run: docker image list + - name: Check for wrong permisions run: | docker image list set -x for IMG in domserver judgehost; do - files=$(docker run --rm --pull=never "${{ github.repository_owner }}/$IMG:${{ env.PR_TAG }}" find / -xdev -perm -o+w ! -type l ! \( -type d -a -perm -+t \) ! -type c) + files=$(docker run --rm --pull=never "domjudge/$IMG:${{ env.PR_TAG }}" find / -xdev -perm -o+w ! -type l ! \( -type d -a -perm -+t \) ! -type c) if [ -n "$files" ]; then - echo "error: image ${{ github.repository_owner }}/$IMG:${{ env.PR_TAG }} contains world-writable files:" >&2 + echo "error: image domjudge/$IMG:${{ env.PR_TAG }} contains world-writable files:" >&2 printf "%s\n" "$files" >&2 exit 1 fi diff --git a/.github/workflows/build-gitlab-container-PR.yml b/.github/workflows/build-gitlab-container-PR.yml index 0b02c3c2..ffdb67f9 100644 --- a/.github/workflows/build-gitlab-container-PR.yml +++ b/.github/workflows/build-gitlab-container-PR.yml @@ -42,7 +42,7 @@ jobs: uses: docker/build-push-action@v5 with: context: "./docker-gitlabci" - push: true + push: false tags: ${{ steps.meta.outputs.tags }} labels: ${{ steps.meta.outputs.labels }} From bff923b5043e3311aed8edc436bee96082237a93 Mon Sep 17 00:00:00 2001 From: Michael Vasseur Date: Sun, 2 Jun 2024 11:25:32 +0200 Subject: [PATCH 4/5] Actually trigger the workflow for PRs The workflows did not always trigger for PRs versus normal pushes. --- .github/workflows/build-contributor-container-PR.yml | 12 +++++++----- .github/workflows/build-domjudge-container-PR.yml | 12 +++++++----- .github/workflows/build-gitlab-container-PR.yml | 12 +++++++----- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/.github/workflows/build-contributor-container-PR.yml b/.github/workflows/build-contributor-container-PR.yml index 889466c1..df6e2466 100644 --- a/.github/workflows/build-contributor-container-PR.yml +++ b/.github/workflows/build-contributor-container-PR.yml @@ -10,11 +10,13 @@ on: jobs: pr-contributor: - if: ${{ !(github.repository == 'domjudge/domjudge-packaging' && - github.ref == 'refs/heads/main') && - !contains(github.ref, 'gh-readonly-queue') && - (github.event_name == 'pull_request' || - github.event.pull_request.head.repo.full_name != github.repository) }} + # Stop processing if this is a merge-queue + # Stop processing if this is not against our repo + # Always run if this PR is not from our organization + # Or run if this PR is not `main` (So notQueue && ourRepo && (notPROurOrg || notMain)) + if : ${{ !contains(github.ref, 'gh-readonly-queue') && + github.repository == 'domjudge/domjudge-packaging' && + !(github.event.pull_request.head.repo.full_name == 'domjudge/domjudge-packaging' && github.ref == 'main') }} runs-on: ubuntu-latest steps: - name: Checkout diff --git a/.github/workflows/build-domjudge-container-PR.yml b/.github/workflows/build-domjudge-container-PR.yml index ae4e8a4f..315916f8 100644 --- a/.github/workflows/build-domjudge-container-PR.yml +++ b/.github/workflows/build-domjudge-container-PR.yml @@ -11,11 +11,13 @@ env: jobs: pr-domjudge: - if: ${{ !(github.repository == 'domjudge/domjudge-packaging' && - github.ref == 'refs/heads/main') && - !contains(github.ref, 'gh-readonly-queue') && - (github.event_name == 'pull_request' || - github.event.pull_request.head.repo.full_name != github.repository) }} + # Stop processing if this is a merge-queue + # Stop processing if this is not against our repo + # Always run if this PR is not from our organization + # Or run if this PR is not `main` (So notQueue && ourRepo && (notPROurOrg || notMain)) + if : ${{ !contains(github.ref, 'gh-readonly-queue') && + github.repository == 'domjudge/domjudge-packaging' && + !(github.event.pull_request.head.repo.full_name == 'domjudge/domjudge-packaging' && github.ref == 'main') }} runs-on: ubuntu-latest steps: - name: Checkout diff --git a/.github/workflows/build-gitlab-container-PR.yml b/.github/workflows/build-gitlab-container-PR.yml index ffdb67f9..fb2aed72 100644 --- a/.github/workflows/build-gitlab-container-PR.yml +++ b/.github/workflows/build-gitlab-container-PR.yml @@ -8,11 +8,13 @@ on: jobs: pr-gitlab: - if: ${{ !(github.repository == 'domjudge/domjudge-packaging' && - github.ref == 'refs/heads/main') && - !contains(github.ref, 'gh-readonly-queue') && - (github.event_name == 'pull_request' || - github.event.pull_request.head.repo.full_name != github.repository) }} + # Stop processing if this is a merge-queue + # Stop processing if this is not against our repo + # Always run if this PR is not from our organization + # Or run if this PR is not `main` (So notQueue && ourRepo && (notPROurOrg || notMain)) + if : ${{ !contains(github.ref, 'gh-readonly-queue') && + github.repository == 'domjudge/domjudge-packaging' && + !(github.event.pull_request.head.repo.full_name == 'domjudge/domjudge-packaging' && github.ref == 'main') }} name: PR GitLab image runs-on: ubuntu-latest permissions: From a2f2f96709525600a2461d9dc50c7ddd3888ded7 Mon Sep 17 00:00:00 2001 From: Michael Vasseur Date: Sun, 2 Jun 2024 19:59:05 +0200 Subject: [PATCH 5/5] Only run when a file has changed --- .github/workflows/build-contributor-container-PR.yml | 6 ++++++ .github/workflows/build-domjudge-container-PR.yml | 6 ++++++ .github/workflows/build-gitlab-container-PR.yml | 8 ++++++++ 3 files changed, 20 insertions(+) diff --git a/.github/workflows/build-contributor-container-PR.yml b/.github/workflows/build-contributor-container-PR.yml index df6e2466..ecd2e2c5 100644 --- a/.github/workflows/build-contributor-container-PR.yml +++ b/.github/workflows/build-contributor-container-PR.yml @@ -4,9 +4,15 @@ name: 'Build Contributor container (PR)' on: push: + paths: + - docker-contributor + - .github/workflows/build-contributor-container-PR.yml pull_request: branches: - main + paths: + - docker-contributor + - .github/workflows/build-contributor-container-PR.yml jobs: pr-contributor: diff --git a/.github/workflows/build-domjudge-container-PR.yml b/.github/workflows/build-domjudge-container-PR.yml index 315916f8..b40a3779 100644 --- a/.github/workflows/build-domjudge-container-PR.yml +++ b/.github/workflows/build-domjudge-container-PR.yml @@ -2,9 +2,15 @@ name: 'Build domjudge container (PR)' on: push: + paths: + - .github/workflows/build-domjudge-container-PR.yml + - docker pull_request: branches: - main + paths: + - .github/workflows/build-domjudge-container-PR.yml + - docker env: DOMJUDGE_VERSION: M.m.p diff --git a/.github/workflows/build-gitlab-container-PR.yml b/.github/workflows/build-gitlab-container-PR.yml index fb2aed72..776830c1 100644 --- a/.github/workflows/build-gitlab-container-PR.yml +++ b/.github/workflows/build-gitlab-container-PR.yml @@ -2,9 +2,17 @@ name: 'Build GitLab CI container (PR)' on: push: + paths: + - docker-contributor/php-config + - docker-gitlabci + - .github/workflows/build-gitlab-container-PR.yml pull_request: branches: - main + paths: + - docker-contributor/php-config + - docker-gitlabci + - .github/workflows/build-gitlab-container-PR.yml jobs: pr-gitlab: