Skip to content

Conversation

@trz42
Copy link
Contributor

@trz42 trz42 commented Mar 5, 2025

Adds signing of a tarball (artefact) and the corresponding metadata file and uploading those to the S3 bucket too.

Uses a sign script that could be provided by the target repository, for example, see EESSI/software-layer#948

Because the latter uses the "recently" added option of ssh-keygen to sign files (option -Y requires OpenSSH v8.2 or newer), the bot may be configured to run the upload script in a container that provides a sufficiently recent version.

The PR adds the capability to skip the ReFrame tests at the end of a build job. This should not be used for production builds, but is useful to shorten development cycles.

The PR has been extensively tested via trz42/software-layer#89

  • As part of the testing three bot instances were used: one on our main AWS build cluster, one on JSC and one on eX3.
  • Where possible, signing was tested within and without a container.
  • Eventually, all tarballs, metadata files and their signatures were successfully uploaded to an S3 bucket.
  • A fix for a minor issue with the sign script (too open permissions for the converted key) has been suggested in Add a script that can sign a file based on an ssh key software-layer#948

truib added 8 commits March 8, 2025 20:13
- 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
@trz42 trz42 marked this pull request as ready for review March 9, 2025 10:38
@trz42 trz42 mentioned this pull request Mar 11, 2025
@Neves-P Neves-P self-requested a review March 12, 2025 14:25
Copy link
Member

@Neves-P Neves-P left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great and is quite clear! I made just a few suggestions in the help messages and a default, but these are minor and I have no issues if you disagree and would rather merge as is.

Feel free to commit the suggestions or reject them and I'm happy to merge when done either way.

Copy link
Member

@Neves-P Neves-P left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! 👍

@Neves-P Neves-P merged commit c963cf6 into EESSI:develop Mar 13, 2025
7 checks passed
if [[ -n "${sign_key}" ]] && [[ -n "${sign_script}" ]]; then
sign=1
elif [[ -n "${sign_key}" ]]; then
sign=0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking, can maybe be tackled in a follow-up PR: I would've used 'no' and 'yes' here, to avoid confusion.

0 is often a sign of success in bash scripts, here's it's a negative value, 'no' leaves no room for confusion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants