From 45321a7312349b7bafc1459d850ac28778811987 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Fri, 14 Mar 2025 15:38:53 +0100 Subject: [PATCH 01/27] avoid that host initialisation scripts are run --- tasks/deploy.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tasks/deploy.py b/tasks/deploy.py index 2d36d24e..0baa8ee4 100644 --- a/tasks/deploy.py +++ b/tasks/deploy.py @@ -417,6 +417,8 @@ def upload_artefact(job_dir, payload, timestamp, repo_name, pr_number, pr_commen container_cmd = [container_runtime, ] container_cmd.extend(['exec']) + # avoid execution of system level initialisation scripts + container_cmd.extend(['--contain']) # avoid that $HOME 'leaks' in due to system settings container_cmd.extend(['--no-home']) for bind in bind_mounts: From c6048c7d53df659947224a1de0c54f7dc0b3745d Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Fri, 14 Mar 2025 22:05:52 +0100 Subject: [PATCH 02/27] use bot's app_name as namespace when signing files --- scripts/eessi-upload-to-staging | 11 +++++-- scripts/sign_verify_file_ssh.sh | 52 ++++++++++++++++++--------------- tasks/deploy.py | 3 ++ 3 files changed, 40 insertions(+), 26 deletions(-) diff --git a/scripts/eessi-upload-to-staging b/scripts/eessi-upload-to-staging index 25fd9675..46557338 100755 --- a/scripts/eessi-upload-to-staging +++ b/scripts/eessi-upload-to-staging @@ -75,6 +75,8 @@ function display_help echo " expansion will be applied; arg '-l'" >&2 echo " lists variables that are defined at" >&2 echo " the time of expansion" >&2 + echo " -b | --bot-instance NAME - name of the bot instance that uploads" >&2 + echo " files to S3" >&2 echo " -e | --endpoint-url URL - endpoint url (needed for non AWS S3)" >&2 echo " -h | --help - display this usage information" >&2 echo " -i | --pr-comment-id - identifier of a PR comment; may be" >&2 @@ -134,6 +136,7 @@ sign_key= sign_script= # provided via options in the bot's config file app.cfg and/or command line argument +bot_instance= metadata_prefix= artefact_prefix= @@ -147,6 +150,10 @@ while [[ $# -gt 0 ]]; do artefact_prefix="$2" shift 2 ;; + -b|--bot-instance) + bot_instance="$2" + shift 2 + ;; -e|--endpoint-url) endpoint_url="$2" shift 2 @@ -263,7 +270,7 @@ for file in "$*"; do # 1st sign artefact, and upload signature if [[ "${sign}" = "1" ]]; then # sign artefact - ${sign_script} sign ${sign_key} ${file} + ${sign_script} sign ${sign_key} ${file} ${bot_instance} # TODO check if signing worked (just check exit code == 0) sig_file=${file}.sig aws_sig_file=${aws_file}.sig @@ -314,7 +321,7 @@ for file in "$*"; do # 2nd sign metadata file, and upload signature if [[ "${sign}" = "1" ]]; then # sign metadata file - ${sign_script} sign ${sign_key} ${metadata_file} + ${sign_script} sign ${sign_key} ${metadata_file} ${bot_instance} # TODO check if signing worked (just check exit code == 0) sig_metadata_file=${metadata_file}.sig aws_sig_metadata_file=${aws_metadata_file}.sig diff --git a/scripts/sign_verify_file_ssh.sh b/scripts/sign_verify_file_ssh.sh index 679ea7d6..c79e94c0 100755 --- a/scripts/sign_verify_file_ssh.sh +++ b/scripts/sign_verify_file_ssh.sh @@ -25,13 +25,14 @@ usage() { cat < + $0 sign $0 verify [signature_file] Options: sign: - : Path to SSH private key (use KEY_PASSPHRASE env for passphrase) - : File to sign + - : Additional information to limit scope of the signature verify: - : Path to the allowed signers file @@ -39,7 +40,7 @@ Options: - [signature_file]: Optional, defaults to '.sig' Example allowed signers format: - identity_1 + identity_1 namespaces="namespace",valid-before="last-valid-day" EOF exit 9 } @@ -48,12 +49,14 @@ EOF FILE_PROBLEM=1 CONVERSION_FAILURE=2 VALIDATION_FAILED=3 +MISSING_NAMESPACE=4 # Ensure minimum arguments [ "$#" -lt 3 ] && usage MODE="$1" FILE_TO_SIGN="$3" +NAMESPACE="$4" # Ensure the target file exists if [ ! -f "$FILE_TO_SIGN" ]; then @@ -102,19 +105,24 @@ if [ "$MODE" == "sign" ]; then convert_private_key "$PRIVATE_KEY" "$TEMP_KEY" echo "Signing the file..." - ssh-keygen -Y sign -f "$TEMP_KEY" -P "${KEY_PASSPHRASE:-}" -n file "$FILE_TO_SIGN" - - [ ! -f "$SIG_FILE" ] && { echo "Error: Signing failed."; exit $FILE_PROBLEM; } - echo "Signature created: $SIG_FILE" - - cat < Date: Fri, 14 Mar 2025 22:20:17 +0100 Subject: [PATCH 03/27] adjust and extend CI for sign/verify script --- .github/workflows/tests_scripts.yml | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests_scripts.yml b/.github/workflows/tests_scripts.yml index 1f8d012b..9c76744a 100644 --- a/.github/workflows/tests_scripts.yml +++ b/.github/workflows/tests_scripts.yml @@ -24,16 +24,30 @@ jobs: echo "Very important stuff" > out.txt export FILE_TO_SIGN="out.txt" # Sign the file - ./scripts/sign_verify_file_ssh.sh sign id_rsa.pem "$FILE_TO_SIGN" + ./scripts/sign_verify_file_ssh.sh sign id_rsa.pem "$FILE_TO_SIGN" ci # Create an allowed_signers file based on the public key - echo -n "allowed_identity " > allowed_signers + valid_before=$(date --date='today+3days' +%Y%m%d) + echo -n 'allowed_identity namespaces="ci",valid-before="'$valid_before'" ' > allowed_signers cat id_rsa.pem.pub >> allowed_signers # Verify the signature ./scripts/sign_verify_file_ssh.sh verify allowed_signers "$FILE_TO_SIGN" # Make a new signature that does not appear in the allowed signers file ssh-keygen -t rsa -b 4096 -m PEM -f id_rsa.alt.pem -N "" # Replace the allowed signers file - echo -n "disallowed_identity " > allowed_signers + echo -n 'disallowed_identity namespaces="ci",valid-before="'$valid_before'" ' > allowed_signers cat id_rsa.alt.pem.pub >> allowed_signers # Make sure signature checking fails in this case ./scripts/sign_verify_file_ssh.sh verify allowed_signers "$FILE_TO_SIGN" && exit 1 || echo "Expected failure for unknown identity" + # Check if wrong namespace let verification fail + # Replace the allowed signers file + echo -n 'wrong_namespace_identity namespaces="CI",valid-before="'$valid_before'" ' > allowed_signers + cat id_rsa.pem.pub >> allowed_signers + # Make sure signature checking fails in this case + ./scripts/sign_verify_file_ssh.sh verify allowed_signers "$FILE_TO_SIGN" && exit 2 || echo "Expected failure for wrong namespace" + # Check if expired key let verification fail + # Replace the allowed signers file + valid_expired=$(date --date='today-3days' +%Y%m%d) + echo -n 'expired_key_identity namespaces="ci",valid-before="'$valid_expired'" ' > allowed_signers + cat id_rsa.pem.pub >> allowed_signers + # Make sure signature checking fails in this case + ./scripts/sign_verify_file_ssh.sh verify allowed_signers "$FILE_TO_SIGN" && exit 3 || echo "Expected failure for expired key" From 0f67582d6c4edf1f85d0fe7185cec653edc3e1a6 Mon Sep 17 00:00:00 2001 From: Alan O'Cais Date: Mon, 17 Mar 2025 11:28:36 +0100 Subject: [PATCH 04/27] Use named arguments and parsing in ssh signing script --- .github/workflows/tests_scripts.yml | 62 +++++++----- scripts/eessi-upload-to-staging | 4 +- scripts/sign_verify_file_ssh.sh | 143 ++++++++++++++++++++-------- 3 files changed, 146 insertions(+), 63 deletions(-) diff --git a/.github/workflows/tests_scripts.yml b/.github/workflows/tests_scripts.yml index 9c76744a..a4ca010f 100644 --- a/.github/workflows/tests_scripts.yml +++ b/.github/workflows/tests_scripts.yml @@ -13,41 +13,57 @@ jobs: build: runs-on: ubuntu-24.04 steps: - - name: checkout - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + - name: Checkout repository + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 - - name: test sign_verify_file_ssh.sh script - run: | - # Create a PEM format ssh identity + - name: Prepare SSH key pair, file and signature + run: | ssh-keygen -t rsa -b 4096 -m PEM -f id_rsa.pem -N "" - # Create a file to sign echo "Very important stuff" > out.txt export FILE_TO_SIGN="out.txt" - # Sign the file - ./scripts/sign_verify_file_ssh.sh sign id_rsa.pem "$FILE_TO_SIGN" ci - # Create an allowed_signers file based on the public key + ./scripts/sign_verify_file_ssh.sh --sign --private-key id_rsa.pem --file "$FILE_TO_SIGN" --namespace ci + + - name: Create allowed signers file and verify + run: | valid_before=$(date --date='today+3days' +%Y%m%d) echo -n 'allowed_identity namespaces="ci",valid-before="'$valid_before'" ' > allowed_signers cat id_rsa.pem.pub >> allowed_signers - # Verify the signature - ./scripts/sign_verify_file_ssh.sh verify allowed_signers "$FILE_TO_SIGN" - # Make a new signature that does not appear in the allowed signers file + ./scripts/sign_verify_file_ssh.sh --verify --allowed-signers-file allowed_signers --file out.txt + + - name: Replace allowed signers with disallowed identity + run: | ssh-keygen -t rsa -b 4096 -m PEM -f id_rsa.alt.pem -N "" - # Replace the allowed signers file echo -n 'disallowed_identity namespaces="ci",valid-before="'$valid_before'" ' > allowed_signers cat id_rsa.alt.pem.pub >> allowed_signers - # Make sure signature checking fails in this case - ./scripts/sign_verify_file_ssh.sh verify allowed_signers "$FILE_TO_SIGN" && exit 1 || echo "Expected failure for unknown identity" - # Check if wrong namespace let verification fail - # Replace the allowed signers file + + - name: Ensure verification fails for unknown identity + run: | + ./scripts/sign_verify_file_ssh.sh --verify --allowed-signers-file allowed_signers --file out.txt && exit 1 || echo "Expected failure for unknown identity" + + - name: Replace allowed signers with wrong namespace + run: | echo -n 'wrong_namespace_identity namespaces="CI",valid-before="'$valid_before'" ' > allowed_signers cat id_rsa.pem.pub >> allowed_signers - # Make sure signature checking fails in this case - ./scripts/sign_verify_file_ssh.sh verify allowed_signers "$FILE_TO_SIGN" && exit 2 || echo "Expected failure for wrong namespace" - # Check if expired key let verification fail - # Replace the allowed signers file + + - name: Ensure verification fails for wrong namespace + run: | + ./scripts/sign_verify_file_ssh.sh --verify --allowed-signers-file allowed_signers --file out.txt && exit 2 || echo "Expected failure for wrong namespace" + + - name: Replace allowed signers with expired key + run: | valid_expired=$(date --date='today-3days' +%Y%m%d) echo -n 'expired_key_identity namespaces="ci",valid-before="'$valid_expired'" ' > allowed_signers cat id_rsa.pem.pub >> allowed_signers - # Make sure signature checking fails in this case - ./scripts/sign_verify_file_ssh.sh verify allowed_signers "$FILE_TO_SIGN" && exit 3 || echo "Expected failure for expired key" + + - name: Ensure verification fails for expired key + run: | + ./scripts/sign_verify_file_ssh.sh --verify --allowed-signers-file allowed_signers --file out.txt && exit 3 || echo "Expected failure for expired key" + + - name: Ensure verification when looping through allowed signers file + run: | + # Add the approved identity to the end + echo -n 'listed_identity namespaces="ci",valid-before="'$valid_before'" ' >> allowed_signers + cat id_rsa.pem.pub >> allowed_signers + ../scripts/sign_verify_file_ssh.sh --verify --allowed-signers-file allowed_signers --file out.txt + # Make sure we get exactly what we want in terse mode + ../scripts/sign_verify_file_ssh.sh --verify --allowed-signers-file allowed_signers --file out.txt --terse | grep -q '{\"identity\": \"listed_identity\", \"namespace\": \"ci\"}' diff --git a/scripts/eessi-upload-to-staging b/scripts/eessi-upload-to-staging index 46557338..fa3d7f57 100755 --- a/scripts/eessi-upload-to-staging +++ b/scripts/eessi-upload-to-staging @@ -270,7 +270,7 @@ for file in "$*"; do # 1st sign artefact, and upload signature if [[ "${sign}" = "1" ]]; then # sign artefact - ${sign_script} sign ${sign_key} ${file} ${bot_instance} + ${sign_script} --sign --private-key ${sign_key} --file ${file} --namespace ${bot_instance} # TODO check if signing worked (just check exit code == 0) sig_file=${file}.sig aws_sig_file=${aws_file}.sig @@ -321,7 +321,7 @@ for file in "$*"; do # 2nd sign metadata file, and upload signature if [[ "${sign}" = "1" ]]; then # sign metadata file - ${sign_script} sign ${sign_key} ${metadata_file} ${bot_instance} + ${sign_script} --sign --private-key ${sign_key} --file ${metadata_file} --namespace ${bot_instance} # TODO check if signing worked (just check exit code == 0) sig_metadata_file=${metadata_file}.sig aws_sig_metadata_file=${aws_metadata_file}.sig diff --git a/scripts/sign_verify_file_ssh.sh b/scripts/sign_verify_file_ssh.sh index c79e94c0..2c5717d2 100755 --- a/scripts/sign_verify_file_ssh.sh +++ b/scripts/sign_verify_file_ssh.sh @@ -7,6 +7,7 @@ # Generates a signature file named `.sig` in the same directory. # # Author: Alan O'Cais +# Author: Thomas Roeblitz # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -23,40 +24,107 @@ # Usage message usage() { + local exit_code=${1:-9} cat < - $0 verify [signature_file] + $0 --sign --private-key --file [--namespace ] + $0 --verify --allowed-signers-file --file [--signature-file ] [--terse] Options: - sign: - - : Path to SSH private key (use KEY_PASSPHRASE env for passphrase) - - : File to sign - - : Additional information to limit scope of the signature + --sign: + --private-key : Path to SSH private key (use KEY_PASSPHRASE env for passphrase) + --file : File to sign + --namespace : Optional, defaults to "file" if not specified - verify: - - : Path to the allowed signers file - - : File to verify - - [signature_file]: Optional, defaults to '.sig' + --verify: + --allowed-signers-file : Path to the allowed signers file + --file : File to verify + --signature-file : Optional, defaults to '.sig' + --terse: If set, output only matching identity and namespace for verification in JSON format Example allowed signers format: identity_1 namespaces="namespace",valid-before="last-valid-day" + +If the private key has a passphrase, this can be provided via a 'KEY_PASSPHRASE' environment variable. EOF - exit 9 + exit "$exit_code" } # Error codes FILE_PROBLEM=1 CONVERSION_FAILURE=2 VALIDATION_FAILED=3 -MISSING_NAMESPACE=4 # Ensure minimum arguments -[ "$#" -lt 3 ] && usage +if [ "$#" -lt 3 ]; then + echo "Error: Missing required arguments." + usage +fi + +# Parse options +TERSE_MODE=false +while [[ "$#" -gt 0 ]]; do + case "$1" in + --sign) + MODE="sign" + shift + ;; + --verify) + MODE="verify" + shift + ;; + --private-key) + PRIVATE_KEY="$2" + shift 2 + ;; + --file) + FILE_TO_SIGN="$2" + shift 2 + ;; + --namespace) + NAMESPACE="$2" + shift 2 + ;; + --allowed-signers-file) + ALLOWED_SIGNERS_FILE="$2" + shift 2 + ;; + --signature-file) + SIG_FILE="$2" + shift 2 + ;; + --terse) + TERSE_MODE=true + shift + ;; + *) + echo "Error: Invalid argument: $1" + usage + ;; + esac +done + +# Set default namespace if not provided +if [ -z "$NAMESPACE" ]; then + NAMESPACE="file" +fi -MODE="$1" -FILE_TO_SIGN="$3" -NAMESPACE="$4" +# Ensure mode is set +if [ -z "$MODE" ]; then + echo "Error: Missing operation mode (either --sign or --verify)" + usage +fi + +# Ensure required arguments +if [ "$MODE" == "sign" ]; then + [ -z "$PRIVATE_KEY" ] && { echo "Error: --private-key not specified."; usage $FILE_PROBLEM; } + [ -z "$FILE_TO_SIGN" ] && { echo "Error: --file not specified."; usage $FILE_PROBLEM; } + SIG_FILE="${FILE_TO_SIGN}.sig" +elif [ "$MODE" == "verify" ]; then + [ -z "$ALLOWED_SIGNERS_FILE" ] && { echo "Error: --allowed-signers-file not specified."; usage $FILE_PROBLEM; } + [ -z "$FILE_TO_SIGN" ] && { echo "Error: --file not specified."; usage $FILE_PROBLEM; } + SIG_FILE="${SIG_FILE:-${FILE_TO_SIGN}.sig}" +fi # Ensure the target file exists if [ ! -f "$FILE_TO_SIGN" ]; then @@ -64,8 +132,8 @@ if [ ! -f "$FILE_TO_SIGN" ]; then exit $FILE_PROBLEM fi -# Use a very conservatuve umask throughout this script since we are dealing with sensitive things -umask 077 || { echo "Error: Failed to set 0177 umask."; exit $FILE_PROBLEM; } +# Use a very conservative umask throughout this script since we are dealing with sensitive things +umask 0077 || { echo "Error: Failed to set 0077 umask."; exit $FILE_PROBLEM; } # Create a restricted temporary directory and ensure cleanup on exit TEMP_DIR=$(mktemp -d) || { echo "Error: Failed to create temporary directory."; exit $FILE_PROBLEM; } @@ -94,9 +162,7 @@ convert_private_key() { # Sign mode if [ "$MODE" == "sign" ]; then - PRIVATE_KEY="$2" TEMP_KEY="$TEMP_DIR/converted_key" - SIG_FILE="${FILE_TO_SIGN}.sig" # Check for key and existing signature [ ! -f "$PRIVATE_KEY" ] && { echo "Error: Private key not found."; exit $FILE_PROBLEM; } @@ -105,18 +171,13 @@ if [ "$MODE" == "sign" ]; then convert_private_key "$PRIVATE_KEY" "$TEMP_KEY" echo "Signing the file..." - if [[ -n "${NAMESPACE}" ]]; then - ssh-keygen -Y sign -f "$TEMP_KEY" -P "${KEY_PASSPHRASE:-}" -n "${NAMESPACE}" "$FILE_TO_SIGN" - cat < /dev/null 2>&1; then + # Output in JSON format + echo "{\"identity\": \"$principal\", \"namespace\": \"$namespaces\"}" + exit 0 + fi + else + if ssh-keygen -Y verify -f "$ALLOWED_SIGNERS_FILE" -n "$namespaces" -I "$principal" -s "$SIG_FILE" < "$FILE_TO_SIGN"; then + echo "Signature is valid for principal: $principal and namespace: $namespaces" + exit 0 + else + echo + echo "Signature _not_ valid for principal: $principal and namespace: $namespaces" + fi fi done < "$ALLOWED_SIGNERS_FILE" echo "Error: No valid signature found." exit $VALIDATION_FAILED - else + echo "Error: Invalid operation mode. Use --sign or --verify." usage fi From 8da77b00bd5dfaccf1243b126e8b2e33cbfedbdf Mon Sep 17 00:00:00 2001 From: Alan O'Cais Date: Mon, 17 Mar 2025 11:34:53 +0100 Subject: [PATCH 05/27] Wrong path to script in final check --- .github/workflows/tests_scripts.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests_scripts.yml b/.github/workflows/tests_scripts.yml index a4ca010f..aab1d8e3 100644 --- a/.github/workflows/tests_scripts.yml +++ b/.github/workflows/tests_scripts.yml @@ -64,6 +64,6 @@ jobs: # Add the approved identity to the end echo -n 'listed_identity namespaces="ci",valid-before="'$valid_before'" ' >> allowed_signers cat id_rsa.pem.pub >> allowed_signers - ../scripts/sign_verify_file_ssh.sh --verify --allowed-signers-file allowed_signers --file out.txt + ./scripts/sign_verify_file_ssh.sh --verify --allowed-signers-file allowed_signers --file out.txt # Make sure we get exactly what we want in terse mode - ../scripts/sign_verify_file_ssh.sh --verify --allowed-signers-file allowed_signers --file out.txt --terse | grep -q '{\"identity\": \"listed_identity\", \"namespace\": \"ci\"}' + ./scripts/sign_verify_file_ssh.sh --verify --allowed-signers-file allowed_signers --file out.txt --terse | grep -q '{\"identity\": \"listed_identity\", \"namespace\": \"ci\"}' From 1813229ef3e574f02c96d926f6fa3a1e00f5577e Mon Sep 17 00:00:00 2001 From: Alan O'Cais Date: Mon, 17 Mar 2025 11:41:37 +0100 Subject: [PATCH 06/27] Also run the tests when the testing changes --- .github/workflows/tests_scripts.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/tests_scripts.yml b/.github/workflows/tests_scripts.yml index aab1d8e3..873cd13f 100644 --- a/.github/workflows/tests_scripts.yml +++ b/.github/workflows/tests_scripts.yml @@ -4,9 +4,11 @@ on: push: paths: - scripts/sign_verify_file_ssh.sh + - .github/workflows/tests_scripts.yml pull_request: paths: - scripts/sign_verify_file_ssh.sh + - .github/workflows/tests_scripts.yml permissions: contents: read # to fetch code (actions/checkout) jobs: From eefaec125df8dbd947bb1230476f02794456d5be Mon Sep 17 00:00:00 2001 From: Alan O'Cais Date: Mon, 17 Mar 2025 11:44:47 +0100 Subject: [PATCH 07/27] Fix test issue --- .github/workflows/tests_scripts.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests_scripts.yml b/.github/workflows/tests_scripts.yml index 873cd13f..6dfd99b8 100644 --- a/.github/workflows/tests_scripts.yml +++ b/.github/workflows/tests_scripts.yml @@ -64,6 +64,7 @@ jobs: - name: Ensure verification when looping through allowed signers file run: | # Add the approved identity to the end + valid_before=$(date --date='today+3days' +%Y%m%d) echo -n 'listed_identity namespaces="ci",valid-before="'$valid_before'" ' >> allowed_signers cat id_rsa.pem.pub >> allowed_signers ./scripts/sign_verify_file_ssh.sh --verify --allowed-signers-file allowed_signers --file out.txt From 7a2b0e24c2d36b81f0eb5c856e3b9220a85863f6 Mon Sep 17 00:00:00 2001 From: Alan O'Cais Date: Mon, 17 Mar 2025 11:57:09 +0100 Subject: [PATCH 08/27] Be more careful with variables between steps --- .github/workflows/tests_scripts.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/tests_scripts.yml b/.github/workflows/tests_scripts.yml index 6dfd99b8..88a473b6 100644 --- a/.github/workflows/tests_scripts.yml +++ b/.github/workflows/tests_scripts.yml @@ -34,6 +34,7 @@ jobs: - name: Replace allowed signers with disallowed identity run: | + valid_before=$(date --date='today+3days' +%Y%m%d) ssh-keygen -t rsa -b 4096 -m PEM -f id_rsa.alt.pem -N "" echo -n 'disallowed_identity namespaces="ci",valid-before="'$valid_before'" ' > allowed_signers cat id_rsa.alt.pem.pub >> allowed_signers @@ -44,6 +45,7 @@ jobs: - name: Replace allowed signers with wrong namespace run: | + valid_before=$(date --date='today+3days' +%Y%m%d) echo -n 'wrong_namespace_identity namespaces="CI",valid-before="'$valid_before'" ' > allowed_signers cat id_rsa.pem.pub >> allowed_signers From 772a6260a68f0c8eb40ed61a54d954e3cf2be99f Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 18 Mar 2025 21:42:01 +0100 Subject: [PATCH 09/27] delete pre-existing signature files + small fixes --- scripts/eessi-upload-to-staging | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/scripts/eessi-upload-to-staging b/scripts/eessi-upload-to-staging index fa3d7f57..16047226 100755 --- a/scripts/eessi-upload-to-staging +++ b/scripts/eessi-upload-to-staging @@ -222,17 +222,17 @@ set -- "${POSITIONAL_ARGS[@]}" # ensure that either none or both of $sign_key and $sign_script are defined if [[ -n "${sign_key}" ]] && [[ -n "${sign_script}" ]]; then - sign=1 + sign=yes elif [[ -n "${sign_key}" ]]; then - sign=0 + sign=no echo "Error: Signing requires a key (${sign_key}) AND a script (${sign_script}); likely the bot config is incomplete" >&2 exit 1 elif [[ -n "${sign_script}" ]]; then - sign=0 + sign=no echo "Error: Signing requires a key (${sign_key}) AND a script (${sign_script}); likely the bot config is incomplete" >&2 exit 1 else - sign=0 + sign=no fi # infer bucket_base: @@ -268,11 +268,16 @@ for file in "$*"; do fi aws_file=$(basename ${file}) # 1st sign artefact, and upload signature - if [[ "${sign}" = "1" ]]; then + if [[ "${sign}" = "yes" ]]; then + sig_file=${file}.sig + # delete sig file if it already exists + if [[ -f "${sig_file}" ]]; then + rm -f ${sig_file} + echo "INFO: removed existing signature file (${sig_file})" + fi # sign artefact ${sign_script} --sign --private-key ${sign_key} --file ${file} --namespace ${bot_instance} # TODO check if signing worked (just check exit code == 0) - sig_file=${file}.sig aws_sig_file=${aws_file}.sig # uploading signature @@ -319,11 +324,16 @@ for file in "$*"; do aws_path=$(envsubst <<< "${metadata_prefix}") fi # 2nd sign metadata file, and upload signature - if [[ "${sign}" = "1" ]]; then + if [[ "${sign}" = "yes" ]]; then + sig_metadata_file=${metadata_file}.sig + # delete sig file if it already exists + if [[ -f "${sig_metadata_file}" ]]; then + rm -f ${sig_metadata_file} + echo "INFO: removed existing signature file (${sig_metadata_file})" + fi # sign metadata file ${sign_script} --sign --private-key ${sign_key} --file ${metadata_file} --namespace ${bot_instance} # TODO check if signing worked (just check exit code == 0) - sig_metadata_file=${metadata_file}.sig aws_sig_metadata_file=${aws_metadata_file}.sig echo " store metadata signature at ${aws_path}/${aws_sig_metadata_file}" From bb146dd38bd09c23082ed9d14cd457205090c1df Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 1 Apr 2025 14:07:09 +0200 Subject: [PATCH 10/27] determine test coverage and upload report --- .github/workflows/tests.yaml | 7 +++++++ test.sh | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 660ed2e2..25746a90 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -36,12 +36,19 @@ jobs: python -m pip install --upgrade pip python -m pip install -r requirements.txt python -m pip install pytest + python -m pip install pytest-cov python -m pip install --upgrade flake8 - name: Run test suite run: | ./test.sh + - name: Upload coverage report + uses: actions/upload-artifact@3cea5372237819ed00197afe530f5a7ea3e805c8 # v3.1.0 + with: + name: coverage-report + path: coverage.html + - name: Run flake8 to verify PEP8-compliance of Python code run: | flake8 diff --git a/test.sh b/test.sh index 168d02bd..f425df8c 100755 --- a/test.sh +++ b/test.sh @@ -7,4 +7,4 @@ # # license: GPLv2 # -PYTHONPATH=$PWD:$PYTHONPATH pytest -v -s +PYTHONPATH=$PWD:$PYTHONPATH pytest --verbose -capture=no --cov=$PWD --cov-report=html From 8f43b22d21e161602cac44b573ca26434fd3bd16 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 1 Apr 2025 14:09:50 +0200 Subject: [PATCH 11/27] add missing dash --- test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test.sh b/test.sh index f425df8c..92770fec 100755 --- a/test.sh +++ b/test.sh @@ -7,4 +7,4 @@ # # license: GPLv2 # -PYTHONPATH=$PWD:$PYTHONPATH pytest --verbose -capture=no --cov=$PWD --cov-report=html +PYTHONPATH=$PWD:$PYTHONPATH pytest --verbose --capture=no --cov=$PWD --cov-report=html From 57d38f30f192dc61d2c08fe32184aadeeb20f77f Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 1 Apr 2025 14:17:49 +0200 Subject: [PATCH 12/27] just produce plain ASCII report and don't upload it --- .github/workflows/tests.yaml | 6 ------ test.sh | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 25746a90..822d6aa6 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -43,12 +43,6 @@ jobs: run: | ./test.sh - - name: Upload coverage report - uses: actions/upload-artifact@3cea5372237819ed00197afe530f5a7ea3e805c8 # v3.1.0 - with: - name: coverage-report - path: coverage.html - - name: Run flake8 to verify PEP8-compliance of Python code run: | flake8 diff --git a/test.sh b/test.sh index 92770fec..2913dd6d 100755 --- a/test.sh +++ b/test.sh @@ -7,4 +7,4 @@ # # license: GPLv2 # -PYTHONPATH=$PWD:$PYTHONPATH pytest --verbose --capture=no --cov=$PWD --cov-report=html +PYTHONPATH=$PWD:$PYTHONPATH pytest --verbose --capture=no --cov=$PWD From f40eefc00eb16e4519ae149ca525f4a84ef916ec Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 1 Apr 2025 14:30:19 +0200 Subject: [PATCH 13/27] fix newly reported global var issue --- connections/github.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/connections/github.py b/connections/github.py index 37cf205c..d6e9d959 100644 --- a/connections/github.py +++ b/connections/github.py @@ -101,7 +101,7 @@ def get_instance(): Returns: Instance of Github """ - global _gh, _token + global _gh # Check if PyGithub version is < 1.56 if hasattr(github, 'GithubRetry'): @@ -129,5 +129,4 @@ def token(): Returns: Token """ - global _token return _token From 47f8206565688d0bbe07c98f34d4354a71537e8f Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 1 Apr 2025 14:34:29 +0200 Subject: [PATCH 14/27] add extra step for coverage --- .github/workflows/tests.yaml | 8 ++++++-- test.sh | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 822d6aa6..ce6126d6 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -36,13 +36,17 @@ jobs: python -m pip install --upgrade pip python -m pip install -r requirements.txt python -m pip install pytest - python -m pip install pytest-cov python -m pip install --upgrade flake8 - - name: Run test suite + - name: Run test suite (without coverage) run: | ./test.sh + - name: Run test suite (with coverage) + run: | + python -m pip install pytest-cov + ./test.sh --cov=$PWD + - name: Run flake8 to verify PEP8-compliance of Python code run: | flake8 diff --git a/test.sh b/test.sh index 2913dd6d..e6836051 100755 --- a/test.sh +++ b/test.sh @@ -7,4 +7,4 @@ # # license: GPLv2 # -PYTHONPATH=$PWD:$PYTHONPATH pytest --verbose --capture=no --cov=$PWD +PYTHONPATH=$PWD:$PYTHONPATH pytest --verbose --capture=no "$@" From dd6ffed1ff5828fede2d3839dfe4b751cc292d67 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 1 Apr 2025 14:39:23 +0200 Subject: [PATCH 15/27] run steps with different options --- .github/workflows/tests.yaml | 4 ++-- test.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index ce6126d6..e925ede1 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -40,12 +40,12 @@ jobs: - name: Run test suite (without coverage) run: | - ./test.sh + ./test.sh --verbose - name: Run test suite (with coverage) run: | python -m pip install pytest-cov - ./test.sh --cov=$PWD + ./test.sh --quiet --cov=$PWD - name: Run flake8 to verify PEP8-compliance of Python code run: | diff --git a/test.sh b/test.sh index e6836051..4d2b6c8a 100755 --- a/test.sh +++ b/test.sh @@ -7,4 +7,4 @@ # # license: GPLv2 # -PYTHONPATH=$PWD:$PYTHONPATH pytest --verbose --capture=no "$@" +PYTHONPATH=$PWD:$PYTHONPATH pytest --capture=no "$@" From 581cf99b9981b04e4ac046503178c23ce399dc80 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 1 Apr 2025 15:03:01 +0200 Subject: [PATCH 16/27] use newer version of pytest-cov --- .github/workflows/tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index e925ede1..81a11af4 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -44,7 +44,7 @@ jobs: - name: Run test suite (with coverage) run: | - python -m pip install pytest-cov + python -m pip install pytest-cov>6.0.0 ./test.sh --quiet --cov=$PWD - name: Run flake8 to verify PEP8-compliance of Python code From 5a1f531b0324b472f8ddf99996d86a0ef1979f0a Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 1 Apr 2025 15:08:59 +0200 Subject: [PATCH 17/27] attempt to only show the coverage summary --- .github/workflows/tests.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 81a11af4..ea1e25d9 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -45,7 +45,8 @@ jobs: - name: Run test suite (with coverage) run: | python -m pip install pytest-cov>6.0.0 - ./test.sh --quiet --cov=$PWD + ./test.sh --quiet --cov=$PWD --cov-report=term:cov-report.txt > /dev/null 2>&1 + cat cov-report.txt - name: Run flake8 to verify PEP8-compliance of Python code run: | From 3dad2597c4fccb98c5603745bdc4b8c1b2f7cb0a Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 1 Apr 2025 15:12:51 +0200 Subject: [PATCH 18/27] debug step error --- .github/workflows/tests.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index ea1e25d9..81bde331 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -45,8 +45,8 @@ jobs: - name: Run test suite (with coverage) run: | python -m pip install pytest-cov>6.0.0 - ./test.sh --quiet --cov=$PWD --cov-report=term:cov-report.txt > /dev/null 2>&1 - cat cov-report.txt + ./test.sh -q --cov=$PWD --cov-report=term:cov-report.txt > /dev/null 2>&1 + # cat cov-report.txt - name: Run flake8 to verify PEP8-compliance of Python code run: | From ed703185c90321682932b5b36c713a12dc02ff13 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 1 Apr 2025 15:15:50 +0200 Subject: [PATCH 19/27] remove requirement for recent version of pytest-cov --- .github/workflows/tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 81bde331..7e47788d 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -44,7 +44,7 @@ jobs: - name: Run test suite (with coverage) run: | - python -m pip install pytest-cov>6.0.0 + python -m pip install pytest-cov ./test.sh -q --cov=$PWD --cov-report=term:cov-report.txt > /dev/null 2>&1 # cat cov-report.txt From f29ec64500403a0c7e9b99fe7cdaba6e01ae0b71 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Tue, 1 Apr 2025 15:19:42 +0200 Subject: [PATCH 20/27] cov-report=term does not support location --- .github/workflows/tests.yaml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 7e47788d..dadec66f 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -45,8 +45,7 @@ jobs: - name: Run test suite (with coverage) run: | python -m pip install pytest-cov - ./test.sh -q --cov=$PWD --cov-report=term:cov-report.txt > /dev/null 2>&1 - # cat cov-report.txt + ./test.sh -q --cov=$PWD - name: Run flake8 to verify PEP8-compliance of Python code run: | From 426b4a1fc36e52b1ff2834c247a69fc593e6cb11 Mon Sep 17 00:00:00 2001 From: sam Date: Sat, 3 May 2025 17:23:11 +0200 Subject: [PATCH 21/27] make bot chattiness configurable --- app.cfg.example | 2 ++ eessi_bot_event_handler.py | 60 ++++++++++++++++++++------------------ tasks/build.py | 6 ++-- tasks/deploy.py | 4 ++- tools/config.py | 1 + tools/pr_comments.py | 33 +++++++++++++++++---- 6 files changed, 69 insertions(+), 37 deletions(-) diff --git a/app.cfg.example b/app.cfg.example index f9b296f6..f0a4ec49 100644 --- a/app.cfg.example +++ b/app.cfg.example @@ -61,6 +61,8 @@ command_response_fmt = {comment_result} +# chattiness level of the bot in terms of writing comments into PRs (minimal, basic, or chatty) +chatlevel = basic [buildenv] # name of the job script used for building an EESSI stack diff --git a/eessi_bot_event_handler.py b/eessi_bot_event_handler.py index 5895fbfb..84128305 100644 --- a/eessi_bot_event_handler.py +++ b/eessi_bot_event_handler.py @@ -38,13 +38,14 @@ from tools.commands import EESSIBotCommand, EESSIBotCommandError, \ contains_any_bot_command, get_bot_command from tools.permissions import check_command_permission -from tools.pr_comments import create_comment +from tools.pr_comments import ChatLevels, create_comment REQUIRED_CONFIG = { config.SECTION_ARCHITECTURETARGETS: [ config.ARCHITECTURETARGETS_SETTING_ARCH_TARGET_MAP], # required config.SECTION_BOT_CONTROL: [ + # config.BOT_CONTROL_SETTING_CHATLEVEL, # optional config.BOT_CONTROL_SETTING_COMMAND_PERMISSION, # required config.BOT_CONTROL_SETTING_COMMAND_RESPONSE_FMT], # required config.SECTION_BUILDENV: [ @@ -193,6 +194,8 @@ def handle_issue_comment_event(self, event_info, log_file=None): return # at this point we know that we are handling a new comment + issue_comment = None + # check if comment does not contain a bot command if not contains_any_bot_command(comment_received): self.log("comment does not contain a bot comment; not processing it further") @@ -229,7 +232,7 @@ def handle_issue_comment_event(self, event_info, log_file=None): comment_response=comment_response, comment_result='' ) - issue_comment = create_comment(repo_name, pr_number, comment_body) + issue_comment = create_comment(repo_name, pr_number, comment_body, ChatLevels.CHATTY) else: self.log(f"account `{sender}` seems to be a bot instance itself, hence not creating a new PR comment") return @@ -263,6 +266,11 @@ def handle_issue_comment_event(self, event_info, log_file=None): # including a bot command; the logging should only be done when log # level is set to debug + if 'help' in (x.command for x in commands): + req_chatlevel = ChatLevels.MINIMAL + else: + req_chatlevel = ChatLevels.CHATTY + if comment_response == '': # no update to be added, just log and return self.log("comment response is empty") @@ -281,7 +289,7 @@ def handle_issue_comment_event(self, event_info, log_file=None): comment_response=comment_response, comment_result='' ) - issue_comment = create_comment(repo_name, pr_number, comment_body) + issue_comment = create_comment(repo_name, pr_number, comment_body, req_chatlevel) else: self.log(f"update '{comment_response}' is considered to contain bot command ... not creating PR comment") # TODO we may want to report this back to the PR on GitHub, e.g., @@ -306,7 +314,7 @@ def handle_issue_comment_event(self, event_info, log_file=None): continue except Exception as err: log(f"Unexpected err={err}, type(err)={type(err)}") - if comment_result: + if comment_result and issue_comment: comment_body = command_response_fmt.format( app_name=app_name, comment_response=comment_response, @@ -314,16 +322,18 @@ def handle_issue_comment_event(self, event_info, log_file=None): ) issue_comment.edit(comment_body) raise - # only update PR comment once, that is, a single call to - # issue_comment.edit is made in the entire function - comment_body = command_response_fmt.format( - app_name=app_name, - comment_response=comment_response, - comment_result=comment_result - ) - issue_comment.edit(comment_body) - self.log(f"issue_comment event (url {issue_url}) handled!") + if issue_comment: + # only update PR comment once, that is, a single call to + # issue_comment.edit is made in the entire function + comment_body = command_response_fmt.format( + app_name=app_name, + comment_response=comment_response, + comment_result=comment_result + ) + issue_comment.edit(comment_body) + + self.log(f"issue_comment event (url {issue_url}) handled!") def handle_installation_event(self, event_info, log_file=None): """ @@ -373,14 +383,14 @@ def handle_pull_request_labeled_event(self, event_info, pr): comment_response=msg, comment_result='' ) - create_comment(repo_name, pr_number, comment_body) + create_comment(repo_name, pr_number, comment_body, ChatLevels.BASIC) elif label == "bot:deploy": # run function to deploy built artefacts deploy_built_artefacts(pr, event_info) else: self.log("handle_pull_request_labeled_event: no handler for label '%s'", label) - def handle_pull_request_opened_event(self, event_info, pr): + def handle_pull_request_opened_event(self, event_info, pr, req_chatlevel=ChatLevels.CHATTY): """ Handle events of type pull_request with the action opened. Main action is to report for which architectures and repositories a bot instance is @@ -420,10 +430,7 @@ def handle_pull_request_opened_event(self, event_info, pr): # create comment to pull request repo_name = pr.base.repo.full_name - gh = github.get_instance() - repo = gh.get_repo(repo_name) - pull_request = repo.get_pull(pr.number) - issue_comment = pull_request.create_issue_comment(comment) + issue_comment = create_comment(repo_name, pr.number, comment, req_chatlevel) return issue_comment def handle_pull_request_event(self, event_info, log_file=None): @@ -554,8 +561,9 @@ def handle_bot_command_show_config(self, event_info, bot_command): repo_name = event_info['raw_request_body']['repository']['full_name'] pr_number = event_info['raw_request_body']['issue']['number'] pr = gh.get_repo(repo_name).get_pull(pr_number) - issue_comment = self.handle_pull_request_opened_event(event_info, pr) - return f"\n - added comment {issue_comment.html_url} to show configuration" + issue_comment = self.handle_pull_request_opened_event(event_info, pr, req_chatlevel=ChatLevels.MINIMAL) + if issue_comment: + return f"\n - added comment {issue_comment.html_url} to show configuration" def handle_bot_command_status(self, event_info, bot_command): """ @@ -571,7 +579,6 @@ def handle_bot_command_status(self, event_info, bot_command): PyGithub, not the github from the internal connections module) """ self.log("processing bot command 'status'") - gh = github.get_instance() repo_name = event_info['raw_request_body']['repository']['full_name'] pr_number = event_info['raw_request_body']['issue']['number'] status_table = request_bot_build_issue_comments(repo_name, pr_number) @@ -588,9 +595,7 @@ def handle_bot_command_status(self, event_info, bot_command): comment_status += f"{status_table['url'][x]}|" self.log(f"Overview of finished builds: comment '{comment_status}'") - repo = gh.get_repo(repo_name) - pull_request = repo.get_pull(pr_number) - issue_comment = pull_request.create_issue_comment(comment_status) + issue_comment = create_comment(repo_name, pr_number, comment_status, ChatLevels.MINIMAL) return issue_comment def start(self, app, port=3000): @@ -669,12 +674,9 @@ def handle_pull_request_closed_event(self, event_info, pr): # 4) report move to pull request repo_name = pr.base.repo.full_name - gh = github.get_instance() - repo = gh.get_repo(repo_name) - pull_request = repo.get_pull(pr.number) clean_up_comment = self.cfg[config.SECTION_CLEAN_UP][config.CLEAN_UP_SETTING_MOVED_JOB_DIRS_COMMENT] moved_comment = clean_up_comment.format(job_dirs=job_dirs, trash_bin_dir=trash_bin_dir) - issue_comment = pull_request.create_issue_comment(moved_comment) + issue_comment = create_comment(repo_name, pr.number, moved_comment, ChatLevels.CHATTY) return issue_comment diff --git a/tasks/build.py b/tasks/build.py index 7a0b1c83..edb08942 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -34,6 +34,7 @@ from connections import github from tools import config, cvmfs_repository, job_metadata, pr_comments, run_cmd import tools.filter as tools_filter +from tools.pr_comments import ChatLevels # defaults (used if not specified via, eg, 'app.cfg') @@ -484,7 +485,7 @@ def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_e f"\n{download_pr_comments_cfg[config.DOWNLOAD_PR_COMMENTS_SETTING_GIT_APPLY_TIP]}") download_comment = pr_comments.create_comment( - repo_name=base_repo_name, pr_number=pr.number, comment=download_comment + repo_name=base_repo_name, pr_number=pr.number, comment=download_comment, req_chatlevel=ChatLevels.MINIMAL ) if download_comment: log(f"{fn}(): created PR issue comment with id {download_comment.id}") @@ -1056,7 +1057,8 @@ def check_build_permission(pr, event_info): repo_name = event_info["raw_request_body"]["repository"]["full_name"] pr_comments.create_comment(repo_name, pr.number, - no_build_permission_comment.format(build_labeler=build_labeler)) + no_build_permission_comment.format(build_labeler=build_labeler), + ChatLevels.MINIMAL) return False else: log(f"{fn}(): GH account '{build_labeler}' is authorized to build") diff --git a/tasks/deploy.py b/tasks/deploy.py index 37ad3adf..f137b068 100644 --- a/tasks/deploy.py +++ b/tasks/deploy.py @@ -28,6 +28,7 @@ from connections import github from tasks.build import get_build_env_cfg from tools import config, job_metadata, pr_comments, run_cmd +from tools.pr_comments import ChatLevels def determine_job_dirs(pr_number): @@ -617,7 +618,8 @@ def deploy_built_artefacts(pr, event_info): repo_name = event_info["raw_request_body"]["repository"]["full_name"] pr_comments.create_comment(repo_name, pr.number, - no_deploy_permission_comment.format(deploy_labeler=labeler)) + no_deploy_permission_comment.format(deploy_labeler=labeler), + ChatLevels.CHATTY) return else: log(f"{funcname}(): GH account '{labeler}' is authorized to deploy") diff --git a/tools/config.py b/tools/config.py index 5d0c6a7e..6fe5982c 100644 --- a/tools/config.py +++ b/tools/config.py @@ -35,6 +35,7 @@ SECTION_BOT_CONTROL = 'bot_control' BOT_CONTROL_SETTING_COMMAND_PERMISSION = 'command_permission' BOT_CONTROL_SETTING_COMMAND_RESPONSE_FMT = 'command_response_fmt' +BOT_CONTROL_SETTING_CHATLEVEL = 'chatlevel' SECTION_BUILDENV = 'buildenv' BUILDENV_SETTING_ALLOWED_EXPORTVARS = 'allowed_exportvars' diff --git a/tools/pr_comments.py b/tools/pr_comments.py index f74bbbf2..98e81245 100644 --- a/tools/pr_comments.py +++ b/tools/pr_comments.py @@ -15,6 +15,7 @@ # Standard library imports from collections import namedtuple +from enum import Enum import re # Third party imports (anything installed into the local Python environment) @@ -24,12 +25,20 @@ # Local application imports (anything from EESSI/eessi-bot-software-layer) from connections import github +from tools import config PRComment = namedtuple('PRComment', ('repo_name', 'pr_number', 'pr_comment_id')) -def create_comment(repo_name, pr_number, comment): +class ChatLevels(Enum): + "chattiness levels" + MINIMAL = 1 + BASIC = 2 + CHATTY = 3 + + +def create_comment(repo_name, pr_number, comment, req_chatlevel): """ Create a comment to a pull request on GitHub @@ -37,15 +46,29 @@ def create_comment(repo_name, pr_number, comment): repo_name (string): name of the repository pr_number (int): number of the pull request within the repository comment (string): comment body + req_chatlevel (member of ChatLevels Enum): minimum required chattiness level for creating the PR comment Returns: github.IssueComment.IssueComment instance or None (note, github refers to PyGithub, not the github from the internal connections module) """ - gh = github.get_instance() - repo = gh.get_repo(repo_name) - pull_request = repo.get_pull(pr_number) - return pull_request.create_issue_comment(comment) + fn = sys._getframe().f_code.co_name + + cfg = config.read_config() + chatlevel = cfg[config.SECTION_BOT_CONTROL].get( + config.BOT_CONTROL_SETTING_CHATLEVEL, ChatLevels.BASIC.name).upper() + + if ChatLevels[chatlevel].value >= req_chatlevel.value: + gh = github.get_instance() + repo = gh.get_repo(repo_name) + pull_request = repo.get_pull(pr_number) + return pull_request.create_issue_comment(comment) + + else: + log(f"{fn}(): not creating PR comment: " + f"chatlevel {ChatLevels[chatlevel].value} < required chatlevel {req_chatlevel.value}") + + return None def determine_issue_comment(pull_request, pr_comment_id, search_pattern=None): From bfc6f27469e7370c73a4126dfb6cef6c1d520e3c Mon Sep 17 00:00:00 2001 From: sam Date: Sun, 4 May 2025 13:04:49 +0200 Subject: [PATCH 22/27] add missing import --- tools/pr_comments.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/pr_comments.py b/tools/pr_comments.py index 98e81245..8dcb6f7a 100644 --- a/tools/pr_comments.py +++ b/tools/pr_comments.py @@ -17,6 +17,7 @@ from collections import namedtuple from enum import Enum import re +import sys # Third party imports (anything installed into the local Python environment) from pyghee.utils import log From 4a7c8333c1f44e2d8f7c63b13b8a5f17b4fd000d Mon Sep 17 00:00:00 2001 From: sam Date: Wed, 7 May 2025 07:52:50 +0200 Subject: [PATCH 23/27] add incognito chatlevel --- tasks/build.py | 7 ++----- tools/pr_comments.py | 6 +++++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index edb08942..547e540c 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -34,7 +34,7 @@ from connections import github from tools import config, cvmfs_repository, job_metadata, pr_comments, run_cmd import tools.filter as tools_filter -from tools.pr_comments import ChatLevels +from tools.pr_comments import ChatLevels, create_comment # defaults (used if not specified via, eg, 'app.cfg') @@ -962,10 +962,7 @@ def create_pr_comment(job, job_id, app_name, pr, gh, symlink): # create comment to pull request repo_name = pr.base.repo.full_name - repo = gh.get_repo(repo_name) - pull_request = repo.get_pull(pr.number) - issue_comment = retry_call(pull_request.create_issue_comment, fargs=[job_comment], - exceptions=Exception, tries=3, delay=1, backoff=2, max_delay=10) + issue_comment = create_comment(repo_name, pr.number, job_comment, ChatLevels.MINIMAL) if issue_comment: log(f"{fn}(): created PR issue comment with id {issue_comment.id}") return issue_comment diff --git a/tools/pr_comments.py b/tools/pr_comments.py index 8dcb6f7a..0585887f 100644 --- a/tools/pr_comments.py +++ b/tools/pr_comments.py @@ -9,6 +9,7 @@ # author: Hafsa Naeem (@hafsa-naeem) # author: Jonas Qvigstad (@jonas-lq) # author: Thomas Roeblitz (@trz42) +# author: Sam Moors (@smoors) # # license: GPLv2 # @@ -34,6 +35,7 @@ class ChatLevels(Enum): "chattiness levels" + INCOGNITO = 0 MINIMAL = 1 BASIC = 2 CHATTY = 3 @@ -63,7 +65,9 @@ def create_comment(repo_name, pr_number, comment, req_chatlevel): gh = github.get_instance() repo = gh.get_repo(repo_name) pull_request = repo.get_pull(pr_number) - return pull_request.create_issue_comment(comment) + issue_comment = retry_call(pull_request.create_issue_comment, fargs=[comment], + exceptions=Exception, tries=3, delay=1, backoff=2, max_delay=10) + return issue_comment else: log(f"{fn}(): not creating PR comment: " From da8480d68d2c43e2fd45e0233c4961638e7c2c32 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 7 May 2025 14:21:49 +0200 Subject: [PATCH 24/27] use Ubuntu 24.04 in CI --- .github/workflows/tests.yaml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index dadec66f..fb4e1882 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -17,10 +17,12 @@ on: [push, pull_request] permissions: read-all jobs: test: - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 strategy: matrix: - python: [3.6, 3.7, 3.8, 3.9, '3.10', '3.11'] + # for now, only test with Python 3.9+ (since we're testing in Ubuntu 24.04) + #python: [3.6, 3.7, 3.8, 3.9, '3.10', '3.11'] + python: ['3.9', '3.10', '3.11'] fail-fast: false steps: - name: checkout From 5b2647eadb165535c18e42d2494316db0f61f255 Mon Sep 17 00:00:00 2001 From: sam Date: Thu, 8 May 2025 15:11:31 +0200 Subject: [PATCH 25/27] fix tests --- tasks/build.py | 8 ++------ tests/test_app.cfg | 2 ++ tests/test_task_build.py | 28 ++++++++++++++++++---------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 547e540c..2156ed1c 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -888,7 +888,7 @@ def submit_job(job, cfg): return job_id, symlink -def create_pr_comment(job, job_id, app_name, pr, gh, symlink): +def create_pr_comment(job, job_id, app_name, pr, symlink): """ Create a comment to the pull request for a newly submitted job @@ -897,7 +897,6 @@ def create_pr_comment(job, job_id, app_name, pr, gh, symlink): job_id (string): id of the submitted job app_name (string): name of the app pr (github.PullRequest.PullRequest): instance representing the pull request - gh (object): github instance symlink (string): symlink from main pr_ dir to job dir Returns: @@ -1000,9 +999,6 @@ def submit_build_jobs(pr, event_info, action_filter): log(f"{fn}(): no jobs ({len(jobs)}) to be submitted") return {} - # obtain handle to GitHub - gh = github.get_instance() - # process prepared jobs: submit, create metadata file and add comment to pull # request on GitHub job_id_to_comment_map = {} @@ -1011,7 +1007,7 @@ def submit_build_jobs(pr, event_info, action_filter): job_id, symlink = submit_job(job, cfg) # create pull request comment to report about the submitted job - pr_comment = create_pr_comment(job, job_id, app_name, pr, gh, symlink) + pr_comment = create_pr_comment(job, job_id, app_name, pr, symlink) job_id_to_comment_map[job_id] = pr_comment pr_comment = pr_comments.PRComment(pr.base.repo.full_name, pr.number, pr_comment.id) diff --git a/tests/test_app.cfg b/tests/test_app.cfg index 84161ba0..4f833bbf 100644 --- a/tests/test_app.cfg +++ b/tests/test_app.cfg @@ -31,3 +31,5 @@ awaits_lauch = job awaits launch by Slurm scheduler running_job = job `{job_id}` is running [finished_job_comments] + +[bot_control] diff --git a/tests/test_task_build.py b/tests/test_task_build.py index f432d768..1c289947 100644 --- a/tests/test_task_build.py +++ b/tests/test_task_build.py @@ -157,6 +157,9 @@ def get_repo(self, repo_name): repo = self.repos[repo_name] return repo + def get_instance(self): + return self + MockBase = namedtuple('MockBase', ['repo']) @@ -275,8 +278,9 @@ def no_sleep_after_create(delay): # returns !None --> create_pr_comment returns comment (with id == 1) @pytest.mark.repo_name("EESSI/software-layer") @pytest.mark.pr_number(1) -def test_create_pr_comment_succeeds(mocked_github, tmpdir): +def test_create_pr_comment_succeeds(monkeypatch, mocked_github, tmpdir): """Tests for function create_pr_comment.""" + monkeypatch.setattr('tools.pr_comments.github', mocked_github) shutil.copyfile("tests/test_app.cfg", "app.cfg") # creating a PR comment print("CREATING PR COMMENT") @@ -291,7 +295,7 @@ def test_create_pr_comment_succeeds(mocked_github, tmpdir): repo = mocked_github.get_repo(repo_name) pr = repo.get_pull(pr_number) symlink = "/symlink" - comment = create_pr_comment(job, job_id, app_name, pr, mocked_github, symlink) + comment = create_pr_comment(job, job_id, app_name, pr, symlink) assert comment.id == 1 # check if created comment includes jobid? print("VERIFYING PR COMMENT") @@ -304,8 +308,9 @@ def test_create_pr_comment_succeeds(mocked_github, tmpdir): @pytest.mark.repo_name("EESSI/software-layer") @pytest.mark.pr_number(1) @pytest.mark.create_fails(True) -def test_create_pr_comment_succeeds_none(mocked_github, tmpdir): +def test_create_pr_comment_succeeds_none(monkeypatch, mocked_github, tmpdir): """Tests for function create_pr_comment.""" + monkeypatch.setattr('tools.pr_comments.github', mocked_github) shutil.copyfile("tests/test_app.cfg", "app.cfg") # creating a PR comment print("CREATING PR COMMENT") @@ -320,7 +325,7 @@ def test_create_pr_comment_succeeds_none(mocked_github, tmpdir): repo = mocked_github.get_repo(repo_name) pr = repo.get_pull(pr_number) symlink = "/symlink" - comment = create_pr_comment(job, job_id, app_name, pr, mocked_github, symlink) + comment = create_pr_comment(job, job_id, app_name, pr, symlink) assert comment is None @@ -329,8 +334,9 @@ def test_create_pr_comment_succeeds_none(mocked_github, tmpdir): @pytest.mark.repo_name("EESSI/software-layer") @pytest.mark.pr_number(1) @pytest.mark.create_raises("1") -def test_create_pr_comment_raises_once_then_succeeds(mocked_github, tmpdir): +def test_create_pr_comment_raises_once_then_succeeds(monkeypatch, mocked_github, tmpdir): """Tests for function create_pr_comment.""" + monkeypatch.setattr('tools.pr_comments.github', mocked_github) shutil.copyfile("tests/test_app.cfg", "app.cfg") # creating a PR comment print("CREATING PR COMMENT") @@ -345,7 +351,7 @@ def test_create_pr_comment_raises_once_then_succeeds(mocked_github, tmpdir): repo = mocked_github.get_repo(repo_name) pr = repo.get_pull(pr_number) symlink = "/symlink" - comment = create_pr_comment(job, job_id, app_name, pr, mocked_github, symlink) + comment = create_pr_comment(job, job_id, app_name, pr, symlink) assert comment.id == 1 assert pr.create_call_count == 2 @@ -354,8 +360,9 @@ def test_create_pr_comment_raises_once_then_succeeds(mocked_github, tmpdir): @pytest.mark.repo_name("EESSI/software-layer") @pytest.mark.pr_number(1) @pytest.mark.create_raises("always_raise") -def test_create_pr_comment_always_raises(mocked_github, tmpdir): +def test_create_pr_comment_always_raises(monkeypatch, mocked_github, tmpdir): """Tests for function create_pr_comment.""" + monkeypatch.setattr('tools.pr_comments.github', mocked_github) shutil.copyfile("tests/test_app.cfg", "app.cfg") # creating a PR comment print("CREATING PR COMMENT") @@ -371,7 +378,7 @@ def test_create_pr_comment_always_raises(mocked_github, tmpdir): pr = repo.get_pull(pr_number) symlink = "/symlink" with pytest.raises(Exception) as err: - create_pr_comment(job, job_id, app_name, pr, mocked_github, symlink) + create_pr_comment(job, job_id, app_name, pr, symlink) assert err.type == CreateIssueCommentException assert pr.create_call_count == 3 @@ -380,8 +387,9 @@ def test_create_pr_comment_always_raises(mocked_github, tmpdir): @pytest.mark.repo_name("EESSI/software-layer") @pytest.mark.pr_number(1) @pytest.mark.create_raises("3") -def test_create_pr_comment_three_raises(mocked_github, tmpdir): +def test_create_pr_comment_three_raises(monkeypatch, mocked_github, tmpdir): """Tests for function create_pr_comment.""" + monkeypatch.setattr('tools.pr_comments.github', mocked_github) shutil.copyfile("tests/test_app.cfg", "app.cfg") # creating a PR comment print("CREATING PR COMMENT") @@ -397,7 +405,7 @@ def test_create_pr_comment_three_raises(mocked_github, tmpdir): pr = repo.get_pull(pr_number) symlink = "/symlink" with pytest.raises(Exception) as err: - create_pr_comment(job, job_id, app_name, pr, mocked_github, symlink) + create_pr_comment(job, job_id, app_name, pr, symlink) assert err.type == CreateIssueCommentException assert pr.create_call_count == 3 From 7604a70633496963b50235872b833de18171f84f Mon Sep 17 00:00:00 2001 From: sam Date: Thu, 8 May 2025 15:13:50 +0200 Subject: [PATCH 26/27] remove unused imports --- tasks/build.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 2156ed1c..517c4077 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -28,10 +28,8 @@ # Third party imports (anything installed into the local Python environment) from pyghee.utils import error, log -from retry.api import retry_call # Local application imports (anything from EESSI/eessi-bot-software-layer) -from connections import github from tools import config, cvmfs_repository, job_metadata, pr_comments, run_cmd import tools.filter as tools_filter from tools.pr_comments import ChatLevels, create_comment From ac5fd3bd3485c5ed08b62abc5f40942f653147c1 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Fri, 23 May 2025 12:09:39 +0200 Subject: [PATCH 27/27] release notes for v0.8.0 --- RELEASE_NOTES | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/RELEASE_NOTES b/RELEASE_NOTES index 5dc4bf33..eb5b4ebd 100644 --- a/RELEASE_NOTES +++ b/RELEASE_NOTES @@ -1,6 +1,25 @@ This file contains a description of the major changes to the EESSI build-and-deploy bot. For more detailed information, please see the git log. +v0.8.0 (23 May 2025) +-------------------------- + +This is a minor release of the EESSI build-and-deploy bot. + +Bug fixes: +* use Ubuntu 24.04 in CI (#316) +* delete pre-existing signature files (#309) + +Improvements: +* adding argument `--contain` when launching the build container (#307) +* use the bot instance's name as `namespaces` value in a signature (#308) +* determine test coverage (#311) +* support different levels for when a bot instance creates comments on GitHub (#315) + +Changes to 'app.cfg' settings (see README.md and app.cfg.example for details): +* NEW (optional) 'chatlevel' in section '[bot_control]' + + v0.7.0 (13 March 2025) --------------------------