Skip to content

Ci hardening - #608

Open
Saul-STFC wants to merge 4 commits into
stackabletech:mainfrom
Saul-STFC:CI-Hardening
Open

Ci hardening#608
Saul-STFC wants to merge 4 commits into
stackabletech:mainfrom
Saul-STFC:CI-Hardening

Conversation

@Saul-STFC

@Saul-STFC Saul-STFC commented Jul 14, 2026

Copy link
Copy Markdown

This PR introduces a collection of 'correctness fixes' and 'hygiene' improvements to the CI templates:

Including:

  • Fix failure condition to check actual job results e.g a skipped job will silently pass
  • Add missing job dependencies to notify and validate all required jobs in finished
  • Guard build-container-image, publish, provenance, preflight and notify jobs with github.repository_owner == 'stackabletech' so fork PRs no longer queue runners or attempt to push images
  • Pinned deps to latest hashes
  • Some other small hardening improvements

* ci: Bump actions/checkout to v7.0.0
* ci: Add explicit permissions to jobs
* ci: Conditionally install host dependencies without cache
* ci: Conditionally set rust-cache lookup-only
* ci: Skip publish jobs for forks
* ci: Remove checkout step from notify job
@Techassi
Techassi self-requested a review July 14, 2026 18:10
* Prevent notification on cancelled workflows
* Update failure condition to check job results
* Add missing job dependencies to notify job
* Validate required jobs in finished job
* Fix detect-changes globs to cover subdirectory Rust sources
* Match dependency versions across templates
* Guard publish/provenance jobs with stackabletech owner check
* Fix daily security workflow
@Saul-STFC
Saul-STFC marked this pull request as ready for review July 20, 2026 09:24
@sbernauer sbernauer moved this to Development: In Review in Stackable Engineering Aug 4, 2026

@Techassi Techassi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall looks good to me, thanks!

Just a few minor comments.

runs-on: ubuntu-latest
permissions:
contents: read
checks: write

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should leave a comment here explaining why we need this permission.

if [ "$EVENT_NAME" = "schedule" ]; then
AUTHOR="stackabletech/developers"
REASON="Weekly run triggered"
else

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We previously explicitly matched on schedule XOR workflow_dispatch. We should do the same here and fallback to exiting with non-0 if the trigger is neither of the two options.

--arg author "$AUTHOR" \
--arg reason "$REASON" \
--arg custom_pr_title "$CUSTOM_PR_TITLE" \
--arg base_dir "$(pwd)" \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Mhm, curious. This expands to nothing because the correct thing to do here would be ${PWD}.

name: Publish/Sign ${{ needs.build-container-image.outputs.operator-version }} Index
if: |
(github.event_name != 'merge_group')
github.repository_owner == 'stackabletech'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this replaces !github.event.pull_request.head.repo.fork below, right?

More of these would need to be changed below if that is the case.

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

Labels

None yet

Projects

Status: Development: In Review

Development

Successfully merging this pull request may close these issues.

3 participants