diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 660ed2e2..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 @@ -38,9 +40,14 @@ jobs: python -m pip install pytest python -m pip install --upgrade flake8 - - name: Run test suite + - 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 -q --cov=$PWD - name: Run flake8 to verify PEP8-compliance of Python code run: | diff --git a/.github/workflows/tests_scripts.yml b/.github/workflows/tests_scripts.yml index 1f8d012b..88a473b6 100644 --- a/.github/workflows/tests_scripts.yml +++ b/.github/workflows/tests_scripts.yml @@ -4,36 +4,71 @@ 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: 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" - # Create an allowed_signers file based on the public key - echo -n "allowed_identity " > allowed_signers + ./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: | + valid_before=$(date --date='today+3days' +%Y%m%d) 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" + + - 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: | + 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 + + - 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 + + - 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 + 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 + # 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/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) -------------------------- 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/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 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/scripts/eessi-upload-to-staging b/scripts/eessi-upload-to-staging index 25fd9675..16047226 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 @@ -215,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: @@ -261,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 ${sign_key} ${file} + ${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 @@ -312,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 ${sign_key} ${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}" diff --git a/scripts/sign_verify_file_ssh.sh b/scripts/sign_verify_file_ssh.sh index 679ea7d6..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,25 +24,30 @@ # 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 + --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 + 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 @@ -50,10 +56,75 @@ CONVERSION_FAILURE=2 VALIDATION_FAILED=3 # Ensure minimum arguments -[ "$#" -lt 3 ] && usage +if [ "$#" -lt 3 ]; then + echo "Error: Missing required arguments." + usage +fi -MODE="$1" -FILE_TO_SIGN="$3" +# 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 + +# 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 @@ -61,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; } @@ -91,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; } @@ -102,55 +171,57 @@ 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" + 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 - done + 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 diff --git a/tasks/build.py b/tasks/build.py index 7a0b1c83..517c4077 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -28,12 +28,11 @@ # 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 # defaults (used if not specified via, eg, 'app.cfg') @@ -484,7 +483,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}") @@ -887,7 +886,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 @@ -896,7 +895,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: @@ -961,10 +959,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 @@ -1002,9 +997,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 = {} @@ -1013,7 +1005,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) @@ -1056,7 +1048,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 2d36d24e..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): @@ -266,6 +267,8 @@ def upload_artefact(job_dir, payload, timestamp, repo_name, pr_number, pr_commen metadata_prefix = deploycfg.get(config.DEPLOYCFG_SETTING_METADATA_PREFIX) artefact_prefix = deploycfg.get(config.DEPLOYCFG_SETTING_ARTEFACT_PREFIX) signing_str = deploycfg.get(config.DEPLOYCFG_SETTING_SIGNING) or '' + github = cfg[config.SECTION_GITHUB] + app_name = github.get(config.GITHUB_SETTING_APP_NAME) try: signing = json.loads(signing_str) except json.decoder.JSONDecodeError: @@ -375,6 +378,7 @@ def upload_artefact(job_dir, payload, timestamp, repo_name, pr_number, pr_commen cmd_args.extend(['--pr-comment-id', str(pr_comment_id)]) cmd_args.extend(['--pull-request-number', str(pr_number)]) cmd_args.extend(['--repository', repo_name]) + cmd_args.extend(['--bot-instance', app_name]) cmd_args.extend(sign_args) cmd_args.append(abs_path) @@ -417,6 +421,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: @@ -612,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/test.sh b/test.sh index 168d02bd..4d2b6c8a 100755 --- a/test.sh +++ b/test.sh @@ -7,4 +7,4 @@ # # license: GPLv2 # -PYTHONPATH=$PWD:$PYTHONPATH pytest -v -s +PYTHONPATH=$PWD:$PYTHONPATH pytest --capture=no "$@" 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 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..0585887f 100644 --- a/tools/pr_comments.py +++ b/tools/pr_comments.py @@ -9,13 +9,16 @@ # author: Hafsa Naeem (@hafsa-naeem) # author: Jonas Qvigstad (@jonas-lq) # author: Thomas Roeblitz (@trz42) +# author: Sam Moors (@smoors) # # license: GPLv2 # # Standard library imports 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 @@ -24,12 +27,21 @@ # 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" + INCOGNITO = 0 + 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 +49,31 @@ 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) + 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: " + f"chatlevel {ChatLevels[chatlevel].value} < required chatlevel {req_chatlevel.value}") + + return None def determine_issue_comment(pull_request, pr_comment_id, search_pattern=None):