Skip to content

Clone specific software-layer-commit and implement CI to check merged status - #1353

Open
casparvl wants to merge 34 commits into
EESSI:mainfrom
casparvl:improve_software_layer_scripts_workflow
Open

Clone specific software-layer-commit and implement CI to check merged status#1353
casparvl wants to merge 34 commits into
EESSI:mainfrom
casparvl:improve_software_layer_scripts_workflow

Conversation

@casparvl

@casparvl casparvl commented Jan 13, 2026

Copy link
Copy Markdown
Collaborator

This PR is an initial step in creating a workflow where we can use PRs to software-layer-scripts directly, and then once they are merged, just update the SHA checksum to a (Github-signed) merge commit, rebuild, and be done :)

Edit: better description...

This PR contains three changes:

  1. bot/build.sh now clones a particular commit, which is specified in bot/commit_sha
  2. CI job Verify software-layer-scripts / check_bot_build_checksum verifies the checksum of bot/build.sh against a reference hardcoded in the workflow file. This way, a malicious contributor would have to modify both bot/build.sh and the workflow file, which would (hopefully) stand out to a reviewer.
  3. CI job Verify software-layer-scripts / check_software_layer_scripts_commit which check that bot/commit_sha is a commit that is part of the upstream https://github.com/EESSI/software-layer-scripts , is on the main branch (i.e. has been merged), is a merge commit, and is signed with the public Github GPG for the web interface.

We might need to update the commit_sha already (not sure if there have been more merges to software-layer-scripts since I started this) before we merge this to make sure this PR doesn't actually revert us to using an older version.

@casparvl

Copy link
Copy Markdown
Collaborator Author

Let's do a small test build to see if the new build.sh works, i.e. if it correctly clones the software-layer-scripts repo from a given commit.

@casparvl

Copy link
Copy Markdown
Collaborator Author

Perfect. CI run on 2cd6082 shows

Commit c0a3ff09a3a38737af5a922fdf581aa7b2dd6c88 is NOT merged into origin/main.
Error: Process completed with exit code 1.

as expected, since this commit is on a feature branch but is not merged. Then, using a merge commit as in 6d954c4 the CI now passes.

@casparvl

Copy link
Copy Markdown
Collaborator Author

TODO: I guess in this same PR we should still add a check that verifies that the SHA-checksum of bot/build.sh itself has remained unchanged (there should be no reason to change it, since the sha-checksum is external to this file).

@casparvl

Copy link
Copy Markdown
Collaborator Author

Perfect. As expected, after changing the bot/build.sh in f1fdcca and fixing a typo in the workflow in c4b1f9a I get:

Computed checksum: bb805939ae22f3ca2e6fc85d13613aeb9b3fc81974a2e1ef3bfc85a7f3ae8a0f
Reference checksum: 9d33368cac2e38e10147eeb0aafc321651ebaa5912387ecef97683570906773a
ERROR: Checksum mismatch! The file bot/build.sh has been modified.

Changing the bot/build.sh back to it's original version in 0494884 and having the CI run on a subsequent merge commit 72fbb29 I now get

Computed checksum: 9d33368cac2e38e10147eeb0aafc321651ebaa5912387ecef97683570906773a
Reference checksum: 9d33368cac2e38e10147eeb0aafc321651ebaa5912387ecef97683570906773a
Checksum for bot/build.sh matches the reference value

@casparvl

Copy link
Copy Markdown
Collaborator Author

Ok, as expected, both Verify software-layer-scripts / check-bot_build_checksum and Verify software-layer-scripts / check_software_layer_scripts_commit pass on bce9bbc

Let's test again by changing the sha checksum...

@casparvl

Copy link
Copy Markdown
Collaborator Author

Ok, on bee1d29 we again have the expected failure:

Commit c0a3ff09a3a38737af5a922fdf581aa7b2dd6c88 is NOT merged into origin/main.
Error: Process completed with exit code 1.

Let's change the sha checksum back, and now change something in bot/build.sh to test the other CI job.

…f this causes the associated CI job to fail
@casparvl

Copy link
Copy Markdown
Collaborator Author

Again, we get the expected failure:

Computed checksum: 93705d4ae3517d9dfcac79d4e7a113e62977d9187b1af9bd4797c03367a9cdfb
Reference checksum: 9d33368cac2e38e10147eeb0aafc321651ebaa5912387ecef97683570906773a
ERROR: Checksum mismatch! The file bot/build.sh has been modified.
Error: Process completed with exit code 1.

# This workflow verifies that the correct version of software-layer-scripts is used.
#
# First, check_bot_build_checksums checks if the bot/build.sh code that clones software-layer-scripts is untouched,
# as this normally shouldn't change (a change could mean a contributor is trying to inject something

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be toned down, changing the bot/build.sh script doesn't necessarily imply that something malicious is going on.

It's weird/unusual, sure, but let's not brand every possible change to bot/build.sh script as malicious up front

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's especially ironic that this very PR is also making changes to bot/build.sh ;-)

@casparvl casparvl Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think this is the first change since we've split off software-layer-scripts from software-layer, well over a year ago. Plus, the wording is 'could', it doesn't say it is malicious.

The point is: there is very little reason to modify bot/build.sh going forward. Builds from feature branches are already possible, as long as they are open PRs to EESSI/software-layer-scripts. If bot/build.sh is modified, we want it to stand out really well, since it is a potential risk to do builds on a PR that uses some arbitrary bot/build.sh.

I know never to say never, so it can still happen - and that's ok. It'll just require a checksum update in the CI as well :)

with:
fetch-depth: 1 # We only need the current revision to read bot/commit_sha
- name: Checkout software-layer-scripts (full history)
uses: actions/checkout@v4

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should also use specific commit, as above

uses: actions/checkout@v4
with:
repository: EESSI/software-layer-scripts
path: upstream-scripts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why make it difficult, just call this EESSI-software-layer-scripts ?

Comment thread bot/build.sh Outdated
TOPDIR=$(dirname $(realpath $0))

# Clone a the commit from software-layer-script that corresponds to `bot/commit_sha`
commit_sha=$(cat ${TOPDIR}/commit_sha)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please rename this file so it's clear from the filename what this refers to:

Suggested change
commit_sha=$(cat ${TOPDIR}/commit_sha)
software_layer_scripts_commit=$(cat ${TOPDIR}/software_layer_scripts_commit)

@boegel

boegel commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

Besides all the necessary changes in software-layer-scripts and eessi-bot-software-layer, would this PR mean that one has to change the commit_sha every time software-layer-scripts contents change?

Yes. If you need a newer version of software-layer-scripts to be used, you have to be explicit about it - that's the whole point, we want it to be explicit about which version of software-layer-scripts was used, so we have to tell it. But note that the commit_sha in software-layer main branch will always point to the tip of software-layer-scripts's main branch. So a naive user who doesn't know about this won't have to be bothered to change / update the commit_sha.

This is the critical point for me: as long as the approach we're taking here doesn't affect most contributors, it's OK imho.

And I would say that's indeed the case here: only if you need updated hooks (or other scripts), you need to take action.

I like the approach being proposed here, as it removes a great deal of uncertainty: no more guessing which commit of the scripts was used, it's crystal clear.

I wouldn't hurt to make bot/build.sh print out which commit of software-layer-scripts it's using BTW.

We should also update our docs to reflect these changes, with a specific subsection that outlines the procedure if changes to hooks are required.

@laraPPr

laraPPr commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

@casparvl Where is this stuck?

@casparvl

casparvl commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator Author

At a technical level, I don't know if it is even stuck - I think this worked. But back then I felt we lacked the commitment to actually adopt this approach - though it keeps popping up that our existing workflow is annoying. I would like to get back to this, so please leave it open.

I think at a technical level, the one thing that would be needed is an accompanying bot PR that makes the bot print which checksum was used. I think we discussed at some point how to do this in a generic way, and the plan - if I remember correctly - was to have a configurable item in app.cfg that points to a file in the repo that - if it exists - is read, and the information is added to the bot's feedback in the PR (and then also to the summary table).

@casparvl

casparvl commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

Made a PR for the bot to report the commit SHA of the software-layer clone EESSI/eessi-bot-software-layer#386 . Together with this PR (#1353) , which defines which commit is cloned from software-layer-scripts by means of the bot/software_layer_scripts_commit file, we get full traceability: the software-layer commit sha refers to a certain version of bot/software_layer_scripts_commit which defines which software-layer-scripts commit was checked out to do the build process.

@casparvl

casparvl commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

TODO:

  • Implement a CI that is only run after the deploy, which checks that the bot/software_layer_scripts_commit points to the current tip of the software-layer-scripts main branch. This is so that whoever creates a new feature branch does so based on the latest commit on software-layer-scripts' main.

This means that if a PR changed bot/software_layer_scripts_commit explicitely, it will have to be updated to the tip of the main software-layer-scripts right before the PR is merged, to ensure that the bot/software_layer_scripts_commit in the main branch of software-layer always points to the latest software-layer-scripts commit on main.

One caveat: if someone forks, and doesn't sync with main, builds will happen with a somewhat older version of software-layer-scripts. I think however that this is desirable: currently, we sometimes have builds in software-layer that are done 5 minutes apart, which use different versions of software-layer-scripts, just because someone has happened to deploy something from there in the meantime. That is currently completely invisible, and therefore completely undesirable. It is exactly the type of behavior we want to avoid...

Note that even after the above TODO item is tackled, it is still the reviewers that are responsible for checking that they only set a deploy label after the CI "Verify software-layer-scripts / check_software_layer_scripts_commit" passes, as this ensures that the builds are done from a merged software-layer-scripts commit. I won't be implementing a hard check on the bot side right now. We have discussed that having functionality in the bot for a pre-deploy check, that would check the status of certain CIs (for which the names are then configurable) would be nice. But that should not block adopting this new workflow: including the above TODO, it is already an improvement over the current workflow.

@casparvl

casparvl commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

I thought about it some more, and maybe we can do better.

  • We make a CI in software-layer that updates the SHA checksum to the tip of software-layer-scripts/main upon merges into main.

  • Additionally, we can make a CI in software-layer for which the trigger is a CI in software-layer-scripts that runs on that main branch, and sends the new SHA as a payload. According to my AI:

=======

Create trigger on repo A (software-layer):

  • Go to Settings > CI / CD > Pipeline triggers in repo A.
  • Click Add trigger → give it a name (e.g., update‑sha‑trigger).
  • Note the Trigger URL and the generated Trigger token.

You’ll use these in repo B to start a pipeline in repo A.

CI on repo B (software-layer-scripts):

stages:
  - notify

# Run on every push to the main branch
update_repo_a_sha:
  stage: notify
  only:
    - main
  script:
    # Optional: fetch the new SHA and pass it as a variable
    - NEW_SHA=$(git rev-parse HEAD)
    - echo "New SHA is $NEW_SHA"
    # Trigger repo A pipeline, passing the SHA as a CI variable
    - >
      curl --request POST
      --form token=TRIGGER_TOKEN   # ← replace with the token from repo A
      --form ref=main
      --form "variables[NEW_SHA]=$NEW_SHA"
      https://gitlab.com/api/v4/projects/PROJECT_ID/trigger/pipeline

CI on repo A (software-layer)

stages:
  - update_sha

update_sha_file:
  stage: update_sha
  only:
    - triggers        # run only when started by a trigger
  variables:
    GIT_STRATEGY: fetch   # ensure we have the full history
  script:
    # 1. Get the SHA passed from repo B (fallback: fetch via API)
    - |
      if [ -z "$NEW_SHA" ]; then
        # No variable – fetch the latest SHA from repo B via API
        B_PROJECT_ID=YOUR_B_PROJECT_ID
        NEW_SHA=$(curl --silent "https://gitlab.com/api/v4/projects/${B_PROJECT_ID}/repository/branches/main" \
                         --header "PRIVATE-TOKEN:${CI_JOB_TOKEN}" |
                         jq -r .commit.id)
      fi
      echo "Using SHA: $NEW_SHA"

    # 2. Replace the old SHA in the target file (e.g., src/commit_ref.txt)
    - FILE=path/to/your/file.txt
    - |
      # Assume the file contains only the SHA; otherwise adjust the sed pattern.
      echo "$NEW_SHA" > $FILE

    # 3. Commit and push the change back to repo A
    - git config user.email "ci-bot@example.com"
    - git config user.name "CI Bot"
    - git add $FILE
    - git commit -m "Update repo B SHA to $NEW_SHA [ci skip]"
    # Use a personal access token (PAT) with write_repo permission
    - git push "https://${CI_JOB_TOKEN}@gitlab.com/namespace/repo-a.git" HEAD:main

=======

The downside is that this requires tokens that have direct write access to your main branch. It should be possible to do this in a controlled way in the GH CI space (masked & protected tokens), but if that makes us uncomfortable, the alternative would be:

  • A CI that is triggered when the bot:deploy label is set (typically all the building is done then, so we can freely update the SHA commit) , and which then either fails if the commit SHA is not pointing to the head of software-layer-scripts or even has write permission and proactively pushes the latest commit SHA to the feature branch. The advantage is that this the only happens on the feature branch, and is still 'in view' of a reviewer before actually merging it.
  • A second CI that is triggered on commits to software-layer-scripts/main that creates a PR to software-layer/main to update the SHA commit.

That solution only requires write tokens on feature branches.

@casparvl

casparvl commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

Some more interaction with my AI tells me that I need to use GITHUB_TOKEN instead of CI_JOB_TOKEN if we want it to have write access. Also, the branch protection rules should be modified in order for the GITHUB_TOKEN to be able to do this (.e.g '“Allow GitHub Actions to bypass branch protection rules”`). We need to discuss if we want this.

@casparvl

casparvl commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

Ok, to avoid stalling this over discussion, let's phase the approach:

First I'll implement:

  • A CI that runs only after the bot:deploy label has been added, and checks if the bot/software_layer_commit_sha points to the tip of software-layer-scripts/main
  • A Github Action that runs periodically (every few minutes or so) and checks if the bot/software_layer_commit_sha on software-layer/main still points to the tip of software-layer-scripts/main. And if not, creates a PR.

This should be totally safe, and only requires the 2nd GH action to have write access to a dedicated feature branch which I'll call ci-update-sha.

…against the latest commit on EESSI/software-layer-scripts's main branch, to see if it points to the latest commit
@casparvl casparvl added the force-ci-checks Force missing software CI checks for a PR label Aug 5, 2026
@casparvl

casparvl commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

Perfect, https://github.com/EESSI/software-layer/actions/runs/31018598477/job/92349130424?pr=1353 goes green immediately (and takes only 6s to run). Let me change the checksum to an older merge commit.

@casparvl

casparvl commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

Excellent, CI https://github.com/EESSI/software-layer/actions/runs/31018815032/job/92349881283?pr=1353 goes red immediately, reporting:

ERROR: bot/software_layer_scripts_commit does not match the latest commit on main of EESSI/software-layer-scripts.
Stored commit:  83169938a52e22849226308cf3010e496f267430
Latest commit:  62b8e332fb041a9ba598dbed3ed781a3416407c0
Please update bot/software_layer_scripts_commit to 62b8e332fb041a9ba598dbed3ed781a3416407c0 and push the change.
Error: Process completed with exit code 1.

Now, let's move to an unmerged commit, check that our other CI also goes red in that case.

@casparvl

casparvl commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

Perfect, the other CI also fails https://github.com/EESSI/software-layer/actions/runs/31019123923/job/92350942842?pr=1353 with:

Commit 89e670f2336a610827ef899ecad520af78b0a848 is NOT merged into origin/main.

Exactly as itended. This CI always runs, and provides reviewers with a red flag that this thing is NOT ready to deploy, as it's not pointing to a merge commit.

…supported CPU targets for that EESSI version. If there is a checksum, it'll verify that all checksums are the same for all tarballs
Comment thread .github/workflows/check_software_layer_scripts_commit.yml
Caspar van Leeuwen added 4 commits August 10, 2026 17:49
…. We don't want to bother contributors who don't know about this file that they have to update stuff - for that we will create a seperate GH Action to automtically create PRs that update this. With this CI, we only ensure that contributors that DO change the commit, ensure that we don't merge an older commit into our software-layer/main branch (even if they built from an older one, which is totally fine)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

force-ci-checks Force missing software CI checks for a PR ready-to-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants