github actions: Introduce kernel-build-and-test-multiarch-trigger.yml#993
github actions: Introduce kernel-build-and-test-multiarch-trigger.yml#993roxanan1996 wants to merge 1 commit intomainfrom
Conversation
Separate the workflow into 2: - trigger (that does not do much, appart from saving params and PR information metadata) - and the actual workflow that gets triggered automatically after this workflow is done. Needed so we can run this for forked repos. First workflow is "safer" because it is triggered in the PR context. The second workflow is triggered by workflow_run which always runs in the base repo context, and it has full access to secrets (APP_ID, APP_PRIVATE_KEY) The modified worklow kernel-build-and-test-multiarch.yml will be added later. Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
There was a problem hiding this comment.
Pull request overview
Adds a new reusable GitHub Actions workflow intended to act as a “safe” PR-context trigger that gathers/sanitizes PR metadata and publishes it as an artifact for a follow-up base-repo-context workflow (to enable fork support).
Changes:
- Introduces
.github/workflows/kernel-build-and-test-multiarch-trigger.ymlreusable workflow (workflow_call) to validate PR refs/metadata. - Clones base branch + fetches PR head to compute/validate
HEAD_SHAand ensure PR is rebased on the latest base. - Persists PR/build parameters into an uploaded artifact (
pr_metadata/+ checksums).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Use environment variables to prevent command injection | ||
| git fetch --depth=$((PR_COMMITS + 1)) "$HEAD_CLONE_URL" "$HEAD_REF" | ||
| HEAD_SHA=$(git rev-parse FETCH_HEAD) | ||
|
|
||
| # Validate SHA format (40 hex characters) | ||
| if ! [[ "$HEAD_SHA" =~ ^[0-9a-f]{40}$ ]]; then | ||
| echo "❌ Invalid SHA format: $HEAD_SHA" | ||
| exit 1 |
There was a problem hiding this comment.
git fetch --depth=$((PR_COMMITS + 1)) uses the PR commit count directly, which can be very large and lead to excessive network usage/time (resource exhaustion). Consider enforcing an upper bound for PR_COMMITS (or for the computed depth) and/or fetching the exact head SHA via the GitHub API/ref instead of scaling depth with commit count.
There was a problem hiding this comment.
I think this is pulled from PR commit processing. I don't think we need a dynamic range so depth=1 should be sufficient for build and test code right ?
Unless you're doing something with the state in the next workflow (but I think you still need to check it out again there?)
| trigger-kernelCI: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Validate and sanitize inputs | ||
| id: validate_inputs | ||
| env: | ||
| BASE_REF: ${{ github.base_ref }} | ||
| HEAD_REF: ${{ github.head_ref }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| PR_COMMITS: ${{ github.event.pull_request.commits }} | ||
| run: | | ||
| # Validate base branch name (alphanumeric, dots, slashes, dashes, underscores, curly braces) | ||
| # Note: hyphen must be at end of character class or escaped to be literal | ||
| if ! [[ "$BASE_REF" =~ ^[a-zA-Z0-9/_.{}-]+$ ]]; then | ||
| echo "❌ Invalid base branch name: $BASE_REF" | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Validate head branch name | ||
| if ! [[ "$HEAD_REF" =~ ^[a-zA-Z0-9/_.{}-]+$ ]]; then | ||
| echo "❌ Invalid head branch name: $HEAD_REF" | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Validate length (prevent resource exhaustion) | ||
| if [ ${#BASE_REF} -gt 255 ]; then | ||
| echo "❌ Base branch name too long" | ||
| exit 1 | ||
| fi | ||
|
|
||
| if [ ${#HEAD_REF} -gt 255 ]; then | ||
| echo "❌ Head branch name too long" | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Validate PR number is numeric | ||
| if ! [[ "$PR_NUMBER" =~ ^[0-9]+$ ]]; then | ||
| echo "❌ Invalid PR number: $PR_NUMBER" | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Validate commits count is numeric | ||
| if ! [[ "$PR_COMMITS" =~ ^[0-9]+$ ]]; then | ||
| echo "❌ Invalid commits count: $PR_COMMITS" | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Pass validated values to environment | ||
| echo "BASE_REF=$BASE_REF" >> "$GITHUB_ENV" | ||
| echo "HEAD_REF=$HEAD_REF" >> "$GITHUB_ENV" | ||
| echo "PR_NUMBER=$PR_NUMBER" >> "$GITHUB_ENV" | ||
| echo "PR_COMMITS=$PR_COMMITS" >> "$GITHUB_ENV" | ||
|
|
There was a problem hiding this comment.
The sanitization/clone pattern introduced here is duplicated in .github/workflows/validate-kernel-commits.yml. Because this is security-sensitive logic, consider extracting it into a shared composite action or reusable workflow so fixes (e.g., regex tweaks, fetch strategy changes) don’t have to be maintained in multiple places.
| trigger-kernelCI: | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Validate and sanitize inputs | |
| id: validate_inputs | |
| env: | |
| BASE_REF: ${{ github.base_ref }} | |
| HEAD_REF: ${{ github.head_ref }} | |
| PR_NUMBER: ${{ github.event.pull_request.number }} | |
| PR_COMMITS: ${{ github.event.pull_request.commits }} | |
| run: | | |
| # Validate base branch name (alphanumeric, dots, slashes, dashes, underscores, curly braces) | |
| # Note: hyphen must be at end of character class or escaped to be literal | |
| if ! [[ "$BASE_REF" =~ ^[a-zA-Z0-9/_.{}-]+$ ]]; then | |
| echo "❌ Invalid base branch name: $BASE_REF" | |
| exit 1 | |
| fi | |
| # Validate head branch name | |
| if ! [[ "$HEAD_REF" =~ ^[a-zA-Z0-9/_.{}-]+$ ]]; then | |
| echo "❌ Invalid head branch name: $HEAD_REF" | |
| exit 1 | |
| fi | |
| # Validate length (prevent resource exhaustion) | |
| if [ ${#BASE_REF} -gt 255 ]; then | |
| echo "❌ Base branch name too long" | |
| exit 1 | |
| fi | |
| if [ ${#HEAD_REF} -gt 255 ]; then | |
| echo "❌ Head branch name too long" | |
| exit 1 | |
| fi | |
| # Validate PR number is numeric | |
| if ! [[ "$PR_NUMBER" =~ ^[0-9]+$ ]]; then | |
| echo "❌ Invalid PR number: $PR_NUMBER" | |
| exit 1 | |
| fi | |
| # Validate commits count is numeric | |
| if ! [[ "$PR_COMMITS" =~ ^[0-9]+$ ]]; then | |
| echo "❌ Invalid commits count: $PR_COMMITS" | |
| exit 1 | |
| fi | |
| # Pass validated values to environment | |
| echo "BASE_REF=$BASE_REF" >> "$GITHUB_ENV" | |
| echo "HEAD_REF=$HEAD_REF" >> "$GITHUB_ENV" | |
| echo "PR_NUMBER=$PR_NUMBER" >> "$GITHUB_ENV" | |
| echo "PR_COMMITS=$PR_COMMITS" >> "$GITHUB_ENV" | |
| validate-kernel-commits: | |
| uses: ./.github/workflows/validate-kernel-commits.yml | |
| permissions: inherit | |
| trigger-kernelCI: | |
| needs: validate-kernel-commits | |
| runs-on: ubuntu-latest | |
| steps: |
There was a problem hiding this comment.
Fair point but if we do this I'd like to see us maintaining our own common actions
bmastbergen
left a comment
There was a problem hiding this comment.
LGTM except we might want some validation on ARCHITECTURES and MAYBE HEAD_REPO_FULL_NAME
| echo "$HEAD_REF" > pr_metadata/head_ref.txt | ||
| echo "$HEAD_SHA" > pr_metadata/head_sha.txt | ||
| echo "$HEAD_REPO_FULL_NAME" > pr_metadata/head_repo.txt | ||
| echo "$ARCHITECTURES" > pr_metadata/architectures.txt |
There was a problem hiding this comment.
HEAD_REPO_FULL_NAME and ARCHITECTURES don't receive any kind of validation. Whether or not that is bad depends on how they eventually get used in the next workflow, but some basic checks here couldn't hurt. HEAD_REPO_FULL_NAME may not be much of an issue since I'm guessing there are already limits on how you can name your repo.
"These get validated in the next workflow" is an acceptable answer. :)
There was a problem hiding this comment.
Good point, I have them validated on the other side.
But needed here too
|
Hi! Marking this under draft since I managed to trigger it at least for existing PRs. Having a common action for the metadata stuff and adding extra checks for all params. |
| - name: Verify PR branch isn't on stale base | ||
| run: | | ||
| if ! git merge-base --is-ancestor "$BASE_REF" "$HEAD_SHA"; then | ||
| echo "❌ PR branch must be rebased onto latest base branch commit" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
This is an annoying feature from the Comments Processing ... I would prefer if we drop this so it can run regardless if the PR isn't immediately pointed at head.
| jobs: | ||
| trigger-kernelCI: | ||
| runs-on: ubuntu-latest | ||
|
|
There was a problem hiding this comment.
There is a time out of 120m in https://github.com/ctrliq/kernel-src-tree/blob/main/.github/workflows/validate-kernel-commits.yml#L14, we could make it that or maybe 60m since we're not doing compute here. Just so its not a 360m default time out
PlaidCat
left a comment
There was a problem hiding this comment.
Just s a couple of minor things
Separate the workflow into 2:
Needed so we can run this for forked repos. First workflow is "safer" because it is triggered in the PR context. The second workflow is triggered by workflow_run which always runs in the base repo context, and it has full access to secrets (APP_ID, APP_PRIVATE_KEY)
The modified worklow kernel-build-and-test-multiarch.yml will be added later.
This is needed so I can test unfortunately, github limitation.