From 2a4aded07efd0297198037e1932de8142330d38a Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Thu, 6 Mar 2025 00:49:41 +0100 Subject: [PATCH 01/12] sign artefact and metadata file and upload signatures --- README.md | 21 ++++++++ app.cfg.example | 20 +++++++ eessi_bot_event_handler.py | 1 + scripts/eessi-upload-to-staging | 94 ++++++++++++++++++++++++++++++--- tasks/deploy.py | 39 ++++++++++++-- tools/__init__.py | 12 +++-- tools/config.py | 4 ++ tools/job_metadata.py | 4 ++ 8 files changed, 180 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 95f482f7..d254e4ba 100644 --- a/README.md +++ b/README.md @@ -508,6 +508,27 @@ artefact_upload_script = PATH_TO_EESSI_BOT/scripts/eessi-upload-to-staging ``` `artefact_upload_script` provides the location for the script used for uploading built software packages to an S3 bucket. +``` +signing = { REPO_ID: + { "script": PATH_TO_SIGN_SCRIPT, + "key": PATH_TO_KEY_FILE, + "container_runtime": PATH_TO_CONTAINER_RUNTIME }, ... } +``` +`signing` provides a setting for signing artefacts. The value uses a JSON-like format +with `REPO_ID` being the repository ID. Repository IDs are defined in a file +`repos.cfg` (see setting `repos_cfG_dir`), `script` provides the location of the +script that is used to sign a file. If the location is a relative path, the script +must reside in the checked out pull request of the target repository (e.g., +EESSI/software-layer). `key` points to the file of the key being used +for signing. The bot calls the script with the two arguments: + 1. private key (as provided by the attribute 'key') + 2. path to the file to be signed (the upload script will determine that) +NOTE, signing requires a recent installation of OpenSSH (8.2 or newer). If the +frontend where the event handler runs does not have that version installed, you +can specify a container runtime via the `container_runtime` attribute below. +For example, this could be Singularity or Apptainer. Others are not supported +as they would require different arguments and environment settings. + ``` endpoint_url = URL_TO_S3_SERVER ``` diff --git a/app.cfg.example b/app.cfg.example index 013accfd..0d660fed 100644 --- a/app.cfg.example +++ b/app.cfg.example @@ -185,6 +185,26 @@ endpoint_url = URL_TO_S3_SERVER # like: bucket_name = {"eessi-pilot-2023.06": "eessi-staging-pilot-2023.06", "eessi.io-2023.06": "software.eessi.io-2023.06"} bucket_name = eessi-staging +# settings for signing artefacts with JSON-like format +# REPO_ID: { "script": PATH_TO_SIGN_SCRIPT, "key": PATH_TO_KEY_FILE } +# If PATH_TO_SIGN_SCRIPT is a relative path, the script must reside in the +# checked out pull request of the target repository (e.g., +# EESSI/software-layer). +# The bot calls the script with the two arguments: +# 1. private key (as provided by the attribute 'key') +# 2. path to the file to be signed (the upload script will determine that) +# NOTE, signing requires a recent installation of OpenSSH (8.2 or newer). If the +# frontend where the event handler runs does not have that version installed, you +# can specify a container runtime via the 'container_runtime' attribute below. +# For example, this could be Singularity or Apptainer. Others are not supported +# as they would require different arguments and environment settings. +signing = { + "eessi.io-2023.06-software: { + "script": PATH_TO_SIGN_SCRIPT, + "key": PATH_TO_EESSI_BOT/config/user-site-system.key, + "container_runtime": PATH_TO_CONTAINER_RUNTIME, + }, +} # upload policy: defines what policy is used for uploading built artefacts # to an S3 bucket # 'all' ..: upload all artefacts (mulitple uploads of the same artefact possible) diff --git a/eessi_bot_event_handler.py b/eessi_bot_event_handler.py index a627c61a..5895fbfb 100644 --- a/eessi_bot_event_handler.py +++ b/eessi_bot_event_handler.py @@ -77,6 +77,7 @@ # config.DEPLOYCFG_SETTING_ENDPOINT_URL, # optional config.DEPLOYCFG_SETTING_METADATA_PREFIX, # (required) config.DEPLOYCFG_SETTING_NO_DEPLOY_PERMISSION_COMMENT, # required + # config.DEPLOYCFG_SETTING_SIGNING, # optional config.DEPLOYCFG_SETTING_UPLOAD_POLICY], # required config.SECTION_DOWNLOAD_PR_COMMENTS: [ config.DOWNLOAD_PR_COMMENTS_SETTING_CURL_FAILURE, # required diff --git a/scripts/eessi-upload-to-staging b/scripts/eessi-upload-to-staging index b5e4482d..bf14dce7 100755 --- a/scripts/eessi-upload-to-staging +++ b/scripts/eessi-upload-to-staging @@ -83,6 +83,9 @@ function display_help echo " ingestion procedure" >&2 echo " -l | --list-variables - list variables that are available" >&2 echo " for expansion" >&2 + echo " -k | --sign-key SCRIPT_KEY - specify location of the key to be" >&2 + echo " used to sign artefacts and metadata" >&2 + echo " files [default: don't sign]" >&2 echo " -m | --metadata-prefix PREFIX - a directory to which the metadata" >&2 echo " file shall be uploaded; BASH variable" >&2 echo " expansion will be applied; arg '-l'" >&2 @@ -93,6 +96,13 @@ function display_help echo " link the upload to a PR" >&2 echo " -r | --repository FULL_NAME - a repository name ACCOUNT/REPONAME;" >&2 echo " used to link the upload to a PR" >&2 + echo " -s | --sign-script SCRIPT_PATH - path to script that is used to sign" >&2 + echo " artefacts and metadata files. The" >&2 + echo " script is called with two arguments:" >&2 + echo " KEY file_to_sign. The KEY is the one" >&2 + echo " provided via option --sign-key. The" >&2 + echo " latter is determined by this script." >&2 + echo " [default: don't sign]" >&2 } if [[ $# -lt 1 ]]; then @@ -120,6 +130,8 @@ endpoint_url= pr_comment_id="none" pull_request_number="none" github_repository="EESSI/software-layer" +sign_key= +sign_script= # provided via options in the bot's config file app.cfg and/or command line argument metadata_prefix= @@ -155,6 +167,14 @@ while [[ $# -gt 0 ]]; do pr_comment_id="$2" shift 2 ;; + -k|--sign-key) + sign_key=$2 + if [[ ! -r "${sign_key}" ]]; then + echo "Error: SSH key '${sign_key}' to be used for signing doesn't exist or cannot be read" >&2 + exit 1 + fi + shift 2 + ;; -m|--metadata-prefix) metadata_prefix="$2" shift 2 @@ -171,6 +191,14 @@ while [[ $# -gt 0 ]]; do github_repository="$2" shift 2 ;; + -s|--sign-script) + sign_script=$2 + if [[ ! -x "${sign_script}" ]]; then + echo "Error: Script '${sign_script}' to be used for signing doesn't exist or is not executable" >&2 + exit 1 + fi + shift 2 + ;; -*|--*) echo "Error: Unknown option: $1" >&2 exit 1 @@ -185,6 +213,21 @@ done # restore potentially parsed filename(s) into $* 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 +elif [[ -n "${sign_key}" ]]; then + sign=0 + 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 + 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 +fi + # infer bucket_base: # if endpoint_url is not set (assume AWS S3 is used), # bucket_base=https://${bucket_name}.s3.amazonaws.com/ @@ -217,6 +260,32 @@ for file in "$*"; do aws_path=$(envsubst <<< "${artefact_prefix}") fi aws_file=$(basename ${file}) + # 1st sign artefact, and upload signature + if [[ "${sign} = "1" ]]; then + # sign artefact + ${sign_script} ${sign_key_path} ${file} + # TODO check if signing worked (just check exit code == 0) + aws_sig_file=${aws_file}.sig + + # uploading signature + echo " store artefact signature at ${aws_path}/${aws_sig_file}" + upload_to_staging_bucket \ + "${aws_sig_file}" \ + "${bucket_name}" \ + "${aws_path}/${aws_sig_file}" \ + "${endpoint_url}" + else + echo "no signing method defined; not signing artefact" + fi + + echo Uploading to "${url}" + echo " store artefact at ${aws_path}/${aws_file}" + upload_to_staging_bucket \ + "${file}" \ + "${bucket_name}" \ + "${aws_path}/${aws_file}" \ + "${endpoint_url}" + echo "Creating metadata file" url="${bucket_base}/${aws_path}/${aws_file}" echo "create_metadata_file file=${file} \ @@ -229,17 +298,10 @@ for file in "$*"; do "${github_repository}" \ "${pull_request_number}" \ "${pr_comment_id}") + # TODO check that creating the metadata file succeeded echo "metadata:" cat ${metadata_file} - echo Uploading to "${url}" - echo " store artefact at ${aws_path}/${aws_file}" - upload_to_staging_bucket \ - "${file}" \ - "${bucket_name}" \ - "${aws_path}/${aws_file}" \ - "${endpoint_url}" - if [ -z ${metadata_prefix} ]; then aws_path=${legacy_aws_path} else @@ -247,6 +309,22 @@ for file in "$*"; do export github_repository aws_path=$(envsubst <<< "${metadata_prefix}") fi + # 2nd sign metadata file, and upload signature + if [[ "${sign} = "1" ]]; then + # sign metadata file + ${sign_script} ${sign_key_path} ${metadata_file} + # TODO check if signing worked (just check exit code == 0) + aws_sig_metadata_file=${aws_metadata_file}.sig + + echo " store metadata signature at ${aws_path}/${aws_sig_metadata_file}" + upload_to_staging_bucket \ + "${aws_sig_metadata_file}" \ + "${bucket_name}" \ + "${aws_path}/${aws_sig_metadata_file}" \ + "${endpoint_url}" + else + echo "no signing method defined; not signing metadata file" + fi echo " store metadata file at ${aws_path}/${aws_file}.meta.txt" upload_to_staging_bucket \ "${metadata_file}" \ diff --git a/tasks/deploy.py b/tasks/deploy.py index 32e7705f..43cce88a 100644 --- a/tasks/deploy.py +++ b/tasks/deploy.py @@ -265,6 +265,7 @@ def upload_artefact(job_dir, payload, timestamp, repo_name, pr_number, pr_commen bucket_spec = deploycfg.get(config.DEPLOYCFG_SETTING_BUCKET_NAME) metadata_prefix = deploycfg.get(config.DEPLOYCFG_SETTING_METADATA_PREFIX) artefact_prefix = deploycfg.get(config.DEPLOYCFG_SETTING_ARTEFACT_PREFIX) + signing = deploycfg.get(config.DEPLOYCFG_SETTING_SIGNING) or {} # if bucket_spec value looks like a dict, try parsing it as such if bucket_spec.lstrip().startswith('{'): @@ -334,11 +335,20 @@ def upload_artefact(job_dir, payload, timestamp, repo_name, pr_number, pr_commen return # run 'eessi-upload-to-staging {abs_path}' + # (1) construct command line + # (2) setup container environment (for signing artefacts ...) if needed + # (3) run command + # (1) construct command line # script assumes a few defaults: # bucket_name = 'eessi-staging' # if endpoint_url not set use EESSI S3 bucket - # (2) run command + do_signing = signing and target_repo_id in signing + sign_args = [] + if do_signing: + sign_args.extend(['--sign-key', signing[target_repo_id][config.DEPLOYCFG_SETTING_SIGNING_KEY]]) + sign_args.extend(['--sign-script', signing[target_repo_id][config.DEPLOYCFG_SETTING_SIGNING_SCRIPT]]) + cmd_args = [artefact_upload_script, ] if len(artefact_prefix_arg) > 0: cmd_args.extend(['--artefact-prefix', artefact_prefix_arg]) @@ -351,11 +361,32 @@ 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(sign_args) cmd_args.append(abs_path) upload_cmd = ' '.join(cmd_args) - - # run_cmd does all the logging we might need - out, err, ec = run_cmd(upload_cmd, 'Upload artefact to S3 bucket', raise_on_error=False) + # (2) setup container environment (for signing artefacts ...) if needed + # determine container to run (from job.cfg) + # determine container cache dir (from job.cfg) + # setup directory for temporary container storage (previous_tmp/upload_step) + # define miscellaneous args (--home ...) + run_in_container = + do_signing and + config.DEPLOYCFG_SETTING_SIGNING_CONTAINER_RUNTIME in signing[target_repo_id] + container_cmd = [] + my_env = {} + if run_in_container: + container = jobcfg[job_metadata.JOB_CFG_REPOSITORY_SECTION][job_metadata.JOB_CFG_REPOSITORY_CONTAINER] + cachedir = jobcfg[job_metadata.JOB_CFG_SITE_CONFIG_SECTION][job_metadata.JOB_CFG_SITE_CONFIG_CONTAINER_CACHEDIR] + upload_tmp_dir = os.path.join(job_dir, job_metadata.JOB_CFG_PREVIOUS_TMP, job_metadata.JOB_CFG_UPLOAD_STEP) + os.makedirs(upload_tmp_dir, exist_ok=True) + container_runtime = signing[target_repo_id][config.DEPLOYCFG_SETTING_SIGNING_CONTAINER_RUNTIME] + container_cmd = [container_runtime, ] + container_cmd.extend(['exec']) + container_cmd.extend(['--home', job_dir]) + container_cmd.extend([container]) + my_env = { 'SINGULARITY_CACHEDIR': cachedir, 'SINGULARITY_TMPDIR': upload_tmp_dir } + + out, err, ec = run_cmd(container_cmd + upload_cmd, my_env, 'Upload artefact to S3 bucket', raise_on_error=False) if ec == 0: # add file to 'job_dir/../uploaded.txt' diff --git a/tools/__init__.py b/tools/__init__.py index 640cae17..8e1dddab 100644 --- a/tools/__init__.py +++ b/tools/__init__.py @@ -23,12 +23,13 @@ # TODO do we really need two functions (run_cmd and run_subprocess) for # running a command? -def run_cmd(cmd, log_msg='', working_dir=None, log_file=None, raise_on_error=True): +def run_cmd(cmd, env=None, log_msg='', working_dir=None, log_file=None, raise_on_error=True): """ Runs a command in the shell and raises an error if one occurs. Args: cmd (string): command to run + env (dict): environment settings for running the command log_msg (string): message describing the purpose of the command working_dir (string): location of the job's working directory log_file (string): path to log file @@ -45,7 +46,7 @@ def run_cmd(cmd, log_msg='', working_dir=None, log_file=None, raise_on_error=Tru raise_on_error is True """ # TODO use common method for logging function name in log messages - stdout, stderr, exit_code = run_subprocess(cmd, log_msg, working_dir, log_file) + stdout, stderr, exit_code = run_subprocess(cmd, env, log_msg, working_dir, log_file) if exit_code != 0: error_msg = ( @@ -66,12 +67,13 @@ def run_cmd(cmd, log_msg='', working_dir=None, log_file=None, raise_on_error=Tru return stdout, stderr, exit_code -def run_subprocess(cmd, log_msg, working_dir, log_file): +def run_subprocess(cmd, env=None, log_msg='', working_dir=None, log_file=None): """ Runs a command in the shell. No error is raised if the command fails. Args: cmd (string): command to run + env (dict): environment settings for running the command log_msg (string): purpose of the command working_dir (string): location of the job's working directory log_file (string): path to log file @@ -91,7 +93,11 @@ def run_subprocess(cmd, log_msg, working_dir, log_file): else: log(f"run_subprocess(): Running '{cmd}' in directory '{working_dir}'", log_file=log_file) + my_env = os.environ.copy() + my_env.update(env) + result = subprocess.run(cmd, + env=my_env, cwd=working_dir, shell=True, encoding="UTF-8", diff --git a/tools/config.py b/tools/config.py index 64170059..5d0c6a7e 100644 --- a/tools/config.py +++ b/tools/config.py @@ -66,6 +66,10 @@ DEPLOYCFG_SETTING_ENDPOINT_URL = 'endpoint_url' DEPLOYCFG_SETTING_METADATA_PREFIX = 'metadata_prefix' DEPLOYCFG_SETTING_NO_DEPLOY_PERMISSION_COMMENT = 'no_deploy_permission_comment' +DEPLOYCFG_SETTING_SIGNING = 'signing' +DEPLOYCFG_SETTING_SIGNING_CONTAINER_RUNTIME = 'container_runtime' +DEPLOYCFG_SETTING_SIGNING_KEY = 'key' +DEPLOYCFG_SETTING_SIGNING_SCRIPT = 'script' DEPLOYCFG_SETTING_UPLOAD_POLICY = 'upload_policy' SECTION_DOWNLOAD_PR_COMMENTS = 'download_pr_comments' diff --git a/tools/job_metadata.py b/tools/job_metadata.py index 478665df..7b7b8d0a 100644 --- a/tools/job_metadata.py +++ b/tools/job_metadata.py @@ -29,6 +29,10 @@ JOB_CFG_DIRECTORY_NAME = "cfg" JOB_CFG_FILENAME = "job.cfg" +# job previous_tmp directory and sub directories +JOB_CFG_PREVIOUS_TMP = "previous_tmp" +JOB_CFG_UPLOAD_STEP = "upload_step" + # JWD/cfg/$JOB_CFG_FILENAME JOB_CFG_ARCHITECTURE_SECTION = "architecture" JOB_CFG_ARCHITECTURE_OS_TYPE = "os_type" From 808c057a7c306e164126ae5c44ad94ac44abc771 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Sat, 8 Mar 2025 20:13:24 +0100 Subject: [PATCH 02/12] allow to skip tests; useful to speed up development --- scripts/bot-build.slurm | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/scripts/bot-build.slurm b/scripts/bot-build.slurm index 270cfa77..7c1c7ec2 100755 --- a/scripts/bot-build.slurm +++ b/scripts/bot-build.slurm @@ -108,14 +108,18 @@ artefacts = EOF fi echo "check build step finished" -TEST_SCRIPT=bot/test.sh -if [ -f ${TEST_SCRIPT} ]; then - echo "${TEST_SCRIPT} script found in '${PWD}', so running it!" - ${TEST_SCRIPT} - echo "${TEST_SCRIPT} finished" -else - echo "could not find ${TEST_SCRIPT} script in '${PWD}'" >&2 -fi + +# SKIP_TESTS can be defined as export variable in the bot's config and then added to bot commands (export:SKIP_TESTS=yes) +if [[ "${SKIP_TESTS}" != "yes" ]]; then + TEST_SCRIPT=bot/test.sh + if [ -f ${TEST_SCRIPT} ]; then + echo "${TEST_SCRIPT} script found in '${PWD}', so running it!" + ${TEST_SCRIPT} + echo "${TEST_SCRIPT} finished" + else + echo "could not find ${TEST_SCRIPT} script in '${PWD}'" >&2 + fi + CHECK_TEST_SCRIPT=bot/check-test.sh if [ -f ${CHECK_TEST_SCRIPT} ]; then echo "${CHECK_TEST_SCRIPT} script found in '${PWD}', so running it!" From 7ad4dcefbb33ca63e8f97824bf9068a8613d3c7b Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Sat, 8 Mar 2025 20:20:04 +0100 Subject: [PATCH 03/12] fix types, missing args, wrongly used variables --- scripts/eessi-upload-to-staging | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/scripts/eessi-upload-to-staging b/scripts/eessi-upload-to-staging index bf14dce7..77988e55 100755 --- a/scripts/eessi-upload-to-staging +++ b/scripts/eessi-upload-to-staging @@ -261,16 +261,17 @@ for file in "$*"; do fi aws_file=$(basename ${file}) # 1st sign artefact, and upload signature - if [[ "${sign} = "1" ]]; then + if [[ "${sign}" = "1" ]]; then # sign artefact - ${sign_script} ${sign_key_path} ${file} + ${sign_script} sign ${sign_key_path} ${file} # TODO check if signing worked (just check exit code == 0) + sig_file=${file}.sig aws_sig_file=${aws_file}.sig # uploading signature echo " store artefact signature at ${aws_path}/${aws_sig_file}" upload_to_staging_bucket \ - "${aws_sig_file}" \ + "${sig_file}" \ "${bucket_name}" \ "${aws_path}/${aws_sig_file}" \ "${endpoint_url}" @@ -298,6 +299,7 @@ for file in "$*"; do "${github_repository}" \ "${pull_request_number}" \ "${pr_comment_id}") + aws_metadata_file=${aws_file}.meta.txt # TODO check that creating the metadata file succeeded echo "metadata:" cat ${metadata_file} @@ -310,15 +312,16 @@ for file in "$*"; do aws_path=$(envsubst <<< "${metadata_prefix}") fi # 2nd sign metadata file, and upload signature - if [[ "${sign} = "1" ]]; then + if [[ "${sign}" = "1" ]]; then # sign metadata file - ${sign_script} ${sign_key_path} ${metadata_file} + ${sign_script} sign ${sign_key_path} ${metadata_file} # 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}" upload_to_staging_bucket \ - "${aws_sig_metadata_file}" \ + "${sig_metadata_file}" \ "${bucket_name}" \ "${aws_path}/${aws_sig_metadata_file}" \ "${endpoint_url}" From cbd477ea4af5914313080ac03d37d0509b06d770 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Sat, 8 Mar 2025 20:30:01 +0100 Subject: [PATCH 04/12] various fixes - ensure the value of signing setting in app.cfg is read as json - convert all paths used to absolute paths - bind mount all paths used plus their realpath into the container (the latter is needed to make sure targets of symlinks that reside on a different file system prefix are still accessible inside the container) - put env arguments in run_cmd and run_subcommand at the end --- tasks/deploy.py | 52 ++++++++++++++++++++++++++++++++++++++++------- tools/__init__.py | 13 ++++++------ 2 files changed, 52 insertions(+), 13 deletions(-) diff --git a/tasks/deploy.py b/tasks/deploy.py index 43cce88a..8977600e 100644 --- a/tasks/deploy.py +++ b/tasks/deploy.py @@ -265,7 +265,12 @@ def upload_artefact(job_dir, payload, timestamp, repo_name, pr_number, pr_commen bucket_spec = deploycfg.get(config.DEPLOYCFG_SETTING_BUCKET_NAME) metadata_prefix = deploycfg.get(config.DEPLOYCFG_SETTING_METADATA_PREFIX) artefact_prefix = deploycfg.get(config.DEPLOYCFG_SETTING_ARTEFACT_PREFIX) - signing = deploycfg.get(config.DEPLOYCFG_SETTING_SIGNING) or {} + signing_str = deploycfg.get(config.DEPLOYCFG_SETTING_SIGNING) or '' + try: + signing = json.loads(signing_str) + except json.decoder.JSONDecodeError: + signing = {} + log(f"{funcname}(): error initialising signing from ({signing_str})") # if bucket_spec value looks like a dict, try parsing it as such if bucket_spec.lstrip().startswith('{'): @@ -346,8 +351,17 @@ def upload_artefact(job_dir, payload, timestamp, repo_name, pr_number, pr_commen do_signing = signing and target_repo_id in signing sign_args = [] if do_signing: - sign_args.extend(['--sign-key', signing[target_repo_id][config.DEPLOYCFG_SETTING_SIGNING_KEY]]) - sign_args.extend(['--sign-script', signing[target_repo_id][config.DEPLOYCFG_SETTING_SIGNING_SCRIPT]]) + sign_key_str = signing[target_repo_id][config.DEPLOYCFG_SETTING_SIGNING_KEY] + sign_key_path = os.path.abspath(sign_key_str) + sign_args.extend(['--sign-key', sign_key_path]) + sign_script_str = signing[target_repo_id][config.DEPLOYCFG_SETTING_SIGNING_SCRIPT] + # if script begins not with '/', assume its location is relative to the job directory + # (that's because the script is provided by the target repository) + if sign_script_str.startswith('/'): + sign_script_path = os.path.abspath(sign_script_str) + else: + sign_script_path = os.path.abspath(os.path.join(job_dir, sign_script_str)) + sign_args.extend(['--sign-script', sign_script_path]) cmd_args = [artefact_upload_script, ] if len(artefact_prefix_arg) > 0: @@ -363,15 +377,16 @@ def upload_artefact(job_dir, payload, timestamp, repo_name, pr_number, pr_commen cmd_args.extend(['--repository', repo_name]) cmd_args.extend(sign_args) cmd_args.append(abs_path) - upload_cmd = ' '.join(cmd_args) + # (2) setup container environment (for signing artefacts ...) if needed # determine container to run (from job.cfg) # determine container cache dir (from job.cfg) # setup directory for temporary container storage (previous_tmp/upload_step) # define miscellaneous args (--home ...) - run_in_container = + run_in_container = ( do_signing and config.DEPLOYCFG_SETTING_SIGNING_CONTAINER_RUNTIME in signing[target_repo_id] + ) container_cmd = [] my_env = {} if run_in_container: @@ -380,13 +395,36 @@ def upload_artefact(job_dir, payload, timestamp, repo_name, pr_number, pr_commen upload_tmp_dir = os.path.join(job_dir, job_metadata.JOB_CFG_PREVIOUS_TMP, job_metadata.JOB_CFG_UPLOAD_STEP) os.makedirs(upload_tmp_dir, exist_ok=True) container_runtime = signing[target_repo_id][config.DEPLOYCFG_SETTING_SIGNING_CONTAINER_RUNTIME] + + # determine (additional) bind mounts from paths used to call upload script and its arguments + # - assumes that all paths begin with '/' + bind_mounts = set() + # first add parent of job_dir and real path of the parent + job_parent_dir = os.path.dirname(job_dir) + bind_mounts.add(job_parent_dir) + real_job_parent_dir = os.path.realpath(job_parent_dir) + if job_parent_dir != real_job_parent_dir: + bind_mounts.add(real_job_parent_dir) + # now, process all args that begin with '/' + for arg in cmd_args: + if arg.startswith('/'): + arg_dir = os.path.dirname(arg) + bind_mounts.add(arg_dir) + # also, determine the real path for arg_dir and add it if it's different to arg_dir + real_dir = os.path.realpath(arg_dir) + if arg_dir != real_dir: + bind_mounts.add(real_dir) + container_cmd = [container_runtime, ] container_cmd.extend(['exec']) - container_cmd.extend(['--home', job_dir]) + for bind in bind_mounts: + container_cmd.extend(['--bind', bind]) container_cmd.extend([container]) my_env = { 'SINGULARITY_CACHEDIR': cachedir, 'SINGULARITY_TMPDIR': upload_tmp_dir } - out, err, ec = run_cmd(container_cmd + upload_cmd, my_env, 'Upload artefact to S3 bucket', raise_on_error=False) + cmd_and_args = ' '.join(container_cmd + cmd_args) + log(f"command to launch upload script: {cmd_and_args}") + out, err, ec = run_cmd(cmd_and_args, 'Upload artefact to S3 bucket', raise_on_error=False, env=my_env) if ec == 0: # add file to 'job_dir/../uploaded.txt' diff --git a/tools/__init__.py b/tools/__init__.py index 8e1dddab..0e7d2028 100644 --- a/tools/__init__.py +++ b/tools/__init__.py @@ -23,17 +23,17 @@ # TODO do we really need two functions (run_cmd and run_subprocess) for # running a command? -def run_cmd(cmd, env=None, log_msg='', working_dir=None, log_file=None, raise_on_error=True): +def run_cmd(cmd, log_msg='', working_dir=None, log_file=None, raise_on_error=True, env=None): """ Runs a command in the shell and raises an error if one occurs. Args: cmd (string): command to run - env (dict): environment settings for running the command log_msg (string): message describing the purpose of the command working_dir (string): location of the job's working directory log_file (string): path to log file raise_on_error (bool): if True raise an exception in case of error + env (dict): environment settings for running the command Returns: tuple of 3 elements containing @@ -46,7 +46,7 @@ def run_cmd(cmd, env=None, log_msg='', working_dir=None, log_file=None, raise_on raise_on_error is True """ # TODO use common method for logging function name in log messages - stdout, stderr, exit_code = run_subprocess(cmd, env, log_msg, working_dir, log_file) + stdout, stderr, exit_code = run_subprocess(cmd, log_msg, working_dir, log_file, env) if exit_code != 0: error_msg = ( @@ -67,16 +67,16 @@ def run_cmd(cmd, env=None, log_msg='', working_dir=None, log_file=None, raise_on return stdout, stderr, exit_code -def run_subprocess(cmd, env=None, log_msg='', working_dir=None, log_file=None): +def run_subprocess(cmd, log_msg='', working_dir=None, log_file=None, env=None): """ Runs a command in the shell. No error is raised if the command fails. Args: cmd (string): command to run - env (dict): environment settings for running the command log_msg (string): purpose of the command working_dir (string): location of the job's working directory log_file (string): path to log file + env (dict): environment settings for running the command Returns: tuple of 3 elements containing @@ -94,7 +94,8 @@ def run_subprocess(cmd, env=None, log_msg='', working_dir=None, log_file=None): log(f"run_subprocess(): Running '{cmd}' in directory '{working_dir}'", log_file=log_file) my_env = os.environ.copy() - my_env.update(env) + if env is not None: + my_env.update(env) result = subprocess.run(cmd, env=my_env, From db3cf9cd665ef1c5defc8b7aa2cc3e736292441d Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Sat, 8 Mar 2025 20:48:01 +0100 Subject: [PATCH 05/12] fix variable name typo --- scripts/eessi-upload-to-staging | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/eessi-upload-to-staging b/scripts/eessi-upload-to-staging index 77988e55..9e4b3e35 100755 --- a/scripts/eessi-upload-to-staging +++ b/scripts/eessi-upload-to-staging @@ -263,7 +263,7 @@ for file in "$*"; do # 1st sign artefact, and upload signature if [[ "${sign}" = "1" ]]; then # sign artefact - ${sign_script} sign ${sign_key_path} ${file} + ${sign_script} sign ${sign_key} ${file} # TODO check if signing worked (just check exit code == 0) sig_file=${file}.sig aws_sig_file=${aws_file}.sig @@ -314,7 +314,7 @@ for file in "$*"; do # 2nd sign metadata file, and upload signature if [[ "${sign}" = "1" ]]; then # sign metadata file - ${sign_script} sign ${sign_key_path} ${metadata_file} + ${sign_script} sign ${sign_key} ${metadata_file} # TODO check if signing worked (just check exit code == 0) sig_metadata_file=${metadata_file}.sig aws_sig_metadata_file=${aws_metadata_file}.sig From 0f8861c3976f99506c624eb5e41e44ac852e490b Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Sun, 9 Mar 2025 04:11:53 +0100 Subject: [PATCH 06/12] don't use $HOME implicitly --- tasks/deploy.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tasks/deploy.py b/tasks/deploy.py index 8977600e..2e7b9faa 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 that $HOME 'leaks' in due to system settings + container_cmd.extend(['--no-home']) for bind in bind_mounts: container_cmd.extend(['--bind', bind]) container_cmd.extend([container]) From 3eb4e12c75999d4e6986e2ff740611a82af4a396 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Sun, 9 Mar 2025 10:43:05 +0100 Subject: [PATCH 07/12] fix hound issues --- tasks/deploy.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tasks/deploy.py b/tasks/deploy.py index 2e7b9faa..2d36d24e 100644 --- a/tasks/deploy.py +++ b/tasks/deploy.py @@ -422,7 +422,10 @@ def upload_artefact(job_dir, payload, timestamp, repo_name, pr_number, pr_commen for bind in bind_mounts: container_cmd.extend(['--bind', bind]) container_cmd.extend([container]) - my_env = { 'SINGULARITY_CACHEDIR': cachedir, 'SINGULARITY_TMPDIR': upload_tmp_dir } + my_env = { + 'SINGULARITY_CACHEDIR': cachedir, + 'SINGULARITY_TMPDIR': upload_tmp_dir + } cmd_and_args = ' '.join(container_cmd + cmd_args) log(f"command to launch upload script: {cmd_and_args}") From 9a2196387ea444fa9ce44e2c9ed35e8cdb42c583 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Sun, 9 Mar 2025 10:49:38 +0100 Subject: [PATCH 08/12] fix missing 'fi' --- scripts/bot-build.slurm | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/bot-build.slurm b/scripts/bot-build.slurm index 7c1c7ec2..bfd1dad0 100755 --- a/scripts/bot-build.slurm +++ b/scripts/bot-build.slurm @@ -119,6 +119,7 @@ if [[ "${SKIP_TESTS}" != "yes" ]]; then else echo "could not find ${TEST_SCRIPT} script in '${PWD}'" >&2 fi +fi CHECK_TEST_SCRIPT=bot/check-test.sh if [ -f ${CHECK_TEST_SCRIPT} ]; then From 4d0d96b5150da345322035491bb96f03592c8c84 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Sun, 9 Mar 2025 11:24:02 +0100 Subject: [PATCH 09/12] clarified README.md and app.cfg.example with some notes --- README.md | 28 ++++++++++++++++++---------- app.cfg.example | 31 ++++++++++++++++++------------- 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index d254e4ba..c92e753f 100644 --- a/README.md +++ b/README.md @@ -509,25 +509,33 @@ artefact_upload_script = PATH_TO_EESSI_BOT/scripts/eessi-upload-to-staging `artefact_upload_script` provides the location for the script used for uploading built software packages to an S3 bucket. ``` -signing = { REPO_ID: - { "script": PATH_TO_SIGN_SCRIPT, - "key": PATH_TO_KEY_FILE, - "container_runtime": PATH_TO_CONTAINER_RUNTIME }, ... } +signing = + { + REPO_ID: { + "script": PATH_TO_SIGN_SCRIPT, + "key": PATH_TO_KEY_FILE, + "container_runtime": PATH_TO_CONTAINER_RUNTIME + }, ... + } ``` `signing` provides a setting for signing artefacts. The value uses a JSON-like format with `REPO_ID` being the repository ID. Repository IDs are defined in a file -`repos.cfg` (see setting `repos_cfG_dir`), `script` provides the location of the +`repos.cfg` (see setting `repos_cfg_dir`), `script` provides the location of the script that is used to sign a file. If the location is a relative path, the script must reside in the checked out pull request of the target repository (e.g., EESSI/software-layer). `key` points to the file of the key being used for signing. The bot calls the script with the two arguments: 1. private key (as provided by the attribute 'key') 2. path to the file to be signed (the upload script will determine that) -NOTE, signing requires a recent installation of OpenSSH (8.2 or newer). If the -frontend where the event handler runs does not have that version installed, you -can specify a container runtime via the `container_runtime` attribute below. -For example, this could be Singularity or Apptainer. Others are not supported -as they would require different arguments and environment settings. +NOTE (on `container_runtime`), signing requires a recent installation of OpenSSH +(8.2 or newer). If the frontend where the event handler runs does not have that +version installed, you can specify a container runtime via the `container_runtime` +attribute below. Currently, only Singularity or Apptainer are supported. +Note (on the key), make sure the file permissions are restricted to `0600` (only +readable+writable by the file owner, or the signing will likely fail. +Note (on json format), make sure no trailing commas are used after any elements +or parsing/loading the json will likely fail. Also, the whole value should start +at a new line and be indented as shown above. ``` endpoint_url = URL_TO_S3_SERVER diff --git a/app.cfg.example b/app.cfg.example index 0d660fed..15af005e 100644 --- a/app.cfg.example +++ b/app.cfg.example @@ -186,25 +186,30 @@ endpoint_url = URL_TO_S3_SERVER bucket_name = eessi-staging # settings for signing artefacts with JSON-like format -# REPO_ID: { "script": PATH_TO_SIGN_SCRIPT, "key": PATH_TO_KEY_FILE } +# REPO_ID: { "script": PATH_TO_SIGN_SCRIPT, "key": PATH_TO_KEY_FILE, "container_runtime": PATH_TO_CONTAINER_RUNTIME } # If PATH_TO_SIGN_SCRIPT is a relative path, the script must reside in the # checked out pull request of the target repository (e.g., # EESSI/software-layer). # The bot calls the script with the two arguments: # 1. private key (as provided by the attribute 'key') # 2. path to the file to be signed (the upload script will determine that) -# NOTE, signing requires a recent installation of OpenSSH (8.2 or newer). If the -# frontend where the event handler runs does not have that version installed, you -# can specify a container runtime via the 'container_runtime' attribute below. -# For example, this could be Singularity or Apptainer. Others are not supported -# as they would require different arguments and environment settings. -signing = { - "eessi.io-2023.06-software: { - "script": PATH_TO_SIGN_SCRIPT, - "key": PATH_TO_EESSI_BOT/config/user-site-system.key, - "container_runtime": PATH_TO_CONTAINER_RUNTIME, - }, -} +# NOTE (on "container_runtime"), signing requires a recent installation of OpenSSH +# (8.2 or newer). If the frontend where the event handler runs does not have that +# version installed, you can specify a container runtime via the 'container_runtime' +# attribute below. Currently, only Singularity or Apptainer are supported. +# NOTE (on the key), make sure the file permissions are restricted to `0600` (only +# readable+writable by the file owner, or the signing will likely fail. +# Note (on json format), make sure no trailing commas are used after any elements +# or parsing/loading the json will likely fail. Also, the whole value should start +# at a new line and be indented as shown below. +signing = + { + "eessi.io-2023.06-software: { + "script": PATH_TO_SIGN_SCRIPT, + "key": PATH_TO_EESSI_BOT/config/user-site-system.key, + "container_runtime": PATH_TO_CONTAINER_RUNTIME + } + } # upload policy: defines what policy is used for uploading built artefacts # to an S3 bucket # 'all' ..: upload all artefacts (mulitple uploads of the same artefact possible) From 9612c9d4dd3ff601056ea717d7b37b37072c17d9 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Sun, 9 Mar 2025 11:54:55 +0100 Subject: [PATCH 10/12] don't skip tests by default --- scripts/bot-build.slurm | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/bot-build.slurm b/scripts/bot-build.slurm index bfd1dad0..593bd158 100755 --- a/scripts/bot-build.slurm +++ b/scripts/bot-build.slurm @@ -23,6 +23,9 @@ # - the directory may contain any additional files references in job.cfg, # for example, repos.cfg and configuration file bundles for repositories +# set default for SKIP_TESTS (don't skip ReFrame tests) +SKIP_TESTS=no + echo "Starting bot-build.slurm" EXPORT_VARS_SCRIPT=cfg/export_vars.sh if [ -f ${EXPORT_VARS_SCRIPT} ]; then From b738989bf31878ebaab45cc121ac4b1f8246352e Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Sun, 9 Mar 2025 12:03:32 +0100 Subject: [PATCH 11/12] add information about SKIP_TESTS --- README.md | 7 +++++++ app.cfg.example | 6 +++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index c92e753f..77c9cdd1 100644 --- a/README.md +++ b/README.md @@ -477,6 +477,13 @@ variables) that are allowed to be specified in a PR command with the `exportvariable` filters must be used (one per variable). These variables will be exported into the build environment before running the bot/build.sh script. +The bot build script makes use of the variable `SKIP_TESTS` to determine if +ReFrame tests shall be skipped or not. Default is not to skip them. To allow the +use of the variable the setting could look like +``` +allowed_exportvars = ["SKIP_TESTS=yes", "SKIP_TESTS=no"] +``` + #### `[bot_control]` section diff --git a/app.cfg.example b/app.cfg.example index 15af005e..6be3c8c6 100644 --- a/app.cfg.example +++ b/app.cfg.example @@ -161,7 +161,11 @@ no_build_permission_comment = Label `bot:build` has been set by user `{build_lab allow_update_submit_opts = false # defines which name-value pairs (environment variables) are allowed to be -# exported into the build environment via `exportvariable` filters +# exported into the build environment via 'exportvariable' filters +# The bot build script makes use of the variable 'SKIP_TESTS' to determine if +# ReFrame tests shall be skipped or not. Default value is 'no'. If the value is +# 'yes' and the exportvariable filter is added to a bot build command +# ('export:SKIP_TESTS=yes'), ReFrame tests are skipped. allowed_exportvars = ["NAME1=value_1a", "NAME1=value_1b", "NAME2=value_2"] From 8a8963c9eeafb38b4a0ebc7f360f5d52b8f03f78 Mon Sep 17 00:00:00 2001 From: Thomas Roeblitz Date: Thu, 13 Mar 2025 09:18:40 +0100 Subject: [PATCH 12/12] improve app.cfg.example and help for upload script --- app.cfg.example | 7 ++++++- scripts/eessi-upload-to-staging | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app.cfg.example b/app.cfg.example index 6be3c8c6..f9b296f6 100644 --- a/app.cfg.example +++ b/app.cfg.example @@ -166,7 +166,12 @@ allow_update_submit_opts = false # ReFrame tests shall be skipped or not. Default value is 'no'. If the value is # 'yes' and the exportvariable filter is added to a bot build command # ('export:SKIP_TESTS=yes'), ReFrame tests are skipped. -allowed_exportvars = ["NAME1=value_1a", "NAME1=value_1b", "NAME2=value_2"] +# NOTE, the setting is optional and commented by default. If you want to enable +# this feature ('exportvariable' filters), uncomment the line below and define +# meaningful key-value pair(s). For example, to enable the use of +# 'exportvariable:SKIP_TESTS=yes' as a filter, the key-value pair would be +# "SKIP_TESTS=yes". +# allowed_exportvars = ["NAME1=value_1a", "NAME1=value_1b", "NAME2=value_2"] [deploycfg] diff --git a/scripts/eessi-upload-to-staging b/scripts/eessi-upload-to-staging index 9e4b3e35..25fd9675 100755 --- a/scripts/eessi-upload-to-staging +++ b/scripts/eessi-upload-to-staging @@ -85,7 +85,7 @@ function display_help echo " for expansion" >&2 echo " -k | --sign-key SCRIPT_KEY - specify location of the key to be" >&2 echo " used to sign artefacts and metadata" >&2 - echo " files [default: don't sign]" >&2 + echo " files [optional; default: don't sign]" >&2 echo " -m | --metadata-prefix PREFIX - a directory to which the metadata" >&2 echo " file shall be uploaded; BASH variable" >&2 echo " expansion will be applied; arg '-l'" >&2 @@ -102,7 +102,7 @@ function display_help echo " KEY file_to_sign. The KEY is the one" >&2 echo " provided via option --sign-key. The" >&2 echo " latter is determined by this script." >&2 - echo " [default: don't sign]" >&2 + echo " [optional; default: don't sign]" >&2 } if [[ $# -lt 1 ]]; then