Skip to content

github: workflows: Restore ok-package-test trigger behavior#11820

Merged
patrick-stephens merged 2 commits into
masterfrom
cosmo0920-fix-package-test-trigger
May 19, 2026
Merged

github: workflows: Restore ok-package-test trigger behavior#11820
patrick-stephens merged 2 commits into
masterfrom
cosmo0920-fix-package-test-trigger

Conversation

@cosmo0920
Copy link
Copy Markdown
Contributor

@cosmo0920 cosmo0920 commented May 19, 2026

On this patch, ok-package-test works as the maintainer-controlled switch for the full package workflow.

When someone adds the ok-package-test label to a PR, GitHub fires the pull_request_target workflow from the base repository. The first job checks two things before anything expensive runs:

contains(github.event.pull_request.labels.*.name, 'ok-package-test') &&
github.event.pull_request.head.repo.full_name == github.repository

So the workflow only proceeds when the label is present, and only for same-repository PR branches. That restores the label-triggered behavior while avoiding privileged execution for fork-controlled code.

The actual build jobs then pass:

ref: ${{ github.event.pull_request.head.sha }}

to the reusable Linux, Windows, macOS, and container build workflows. That detail matters: because pull_request_target runs in the base-repo context, github.ref would point at the target/base side, not the PR code. Using the PR head SHA makes the package builders check out and build the exact commit from the labeled PR.

Finally, the concurrency group is now:

${{ github.workflow }}-${{ github.event.pull_request.number }}

so reruns or new commits for the same PR cancel older package runs for that PR, but package builds for different PRs no longer cancel each other just because they all target master.


Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • Example configuration file for the change
  • Debug log output from testing the change
  • Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • Run local packaging test showing all targets (including any new ones) build.
  • Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • Documentation required for this feature

Backporting

  • Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Summary by CodeRabbit

  • Chores
    • CI workflow now uses a maintainer-gated pull request trigger and includes inline controls for maintainer execution when standard triggers lack permissions.
    • Concurrency tightened to a per-workflow/per-PR group with in-progress runs cancelled.
    • Downstream reusable build workflows now receive explicit PR number and head SHA and invoke builds with explicit package write permissions.

Review Change Stack

Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2b5b1fa3-8c17-47c2-84e4-fa32c5005c94

📥 Commits

Reviewing files that changed from the base of the PR and between b8088cc and 0504ab5.

📒 Files selected for processing (1)
  • .github/workflows/pr-package-tests.yaml

📝 Walkthrough

Walkthrough

Updated the PR package tests workflow to use label-gated pull_request_target, tightened concurrency to a PR-scoped group, and forwarded PR number and PR head SHA plus explicit permissions to downstream reusable container and platform build jobs.

Changes

PR Package Tests Workflow Hardening

Layer / File(s) Summary
Workflow trigger and security gating
.github/workflows/pr-package-tests.yaml
Switched workflow trigger from pull_request to label-gated pull_request_target with comments about maintainer-controlled execution when pull_request lacks permissions.
Concurrency control and job gating
.github/workflows/pr-package-tests.yaml
Replaced concurrency grouping based on github.ref with a PR-scoped group using ${{ github.workflow }}-${{ github.event.pull_request.number }}, retained cancel-in-progress: true, and expanded the metadata job if to require the ok-package-test label and that the PR head repo equals the current repo.
Build job inputs across all platforms
.github/workflows/pr-package-tests.yaml
In the container, Linux, Windows, and macOS reusable workflow calls, changed forwarded version to PR number and ref to PR head SHA; also set explicit permissions (including packages: write) for the container reusable workflow call.

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

ci

Suggested reviewers

  • niedbalski
  • patrick-stephens
  • celalettin1286

Poem

🐰 Labels and triggers neatly aligned,
PR-sha and numbers now clearly defined,
Concurrency scoped to the PR's own name,
Permissions set so workflows run the same,
A hop, a build, and order reclaimed.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: restoring the ok-package-test label as a trigger for the package workflow, which is the core objective of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cosmo0920-fix-package-test-trigger

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cosmo0920 cosmo0920 removed the ok-package-test Run PR packaging tests label May 19, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/pr-package-tests.yaml (1)

42-61: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Lock down token permissions on reusable jobs under pull_request_target.

These uses: jobs don't set explicit permissions. With pull_request_target, they inherit broader GITHUB_TOKEN scopes (repository-default dependent), which is risky when building PR-head code. Set least-privilege permissions per job (or at workflow level, then override only where needed).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/pr-package-tests.yaml around lines 42 - 61, The
pr-container-builds job currently omits explicit permissions when invoked under
pull_request_target, allowing the reusable job (uses:
./.github/workflows/call-build-images.yaml) to inherit broad GITHUB_TOKEN
scopes; add a least-privilege permissions block to the job (e.g., explicitly set
permissions: contents: read and packages: write or only the minimal scopes your
reusable workflow needs) or set workflow-level permissions and override as
necessary, ensuring the job’s inputs/secrets (version, ref, registry, token,
cosign_private_key, etc.) still work but do not grant excess repository
permissions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In @.github/workflows/pr-package-tests.yaml:
- Around line 42-61: The pr-container-builds job currently omits explicit
permissions when invoked under pull_request_target, allowing the reusable job
(uses: ./.github/workflows/call-build-images.yaml) to inherit broad GITHUB_TOKEN
scopes; add a least-privilege permissions block to the job (e.g., explicitly set
permissions: contents: read and packages: write or only the minimal scopes your
reusable workflow needs) or set workflow-level permissions and override as
necessary, ensuring the job’s inputs/secrets (version, ref, registry, token,
cosign_private_key, etc.) still work but do not grant excess repository
permissions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 16a17642-6bff-4afc-aa75-27290957cbb5

📥 Commits

Reviewing files that changed from the base of the PR and between 6474297 and b8088cc.

📒 Files selected for processing (1)
  • .github/workflows/pr-package-tests.yaml

Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
@cosmo0920 cosmo0920 added this to the Fluent Bit v5.0.6 milestone May 19, 2026
@patrick-stephens patrick-stephens merged commit 226c112 into master May 19, 2026
10 checks passed
@patrick-stephens patrick-stephens deleted the cosmo0920-fix-package-test-trigger branch May 19, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants