Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughMultiple GitHub Actions workflows were updated to react to pull request "labeled" events, gate jobs to run only for non-PR events or when the label is Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/check_ci_coverage.yml (1)
5-21:⚠️ Potential issue | 🟠 MajorFork PRs cannot auto-remove the
ci:runlabel due to token restrictions.The gate condition depends on
.github/workflows/consume_ci_run_label.ymlremoving the label after each run, butpull_requestevents from forks receive a read-onlyGITHUB_TOKENregardless of thepermissionskey configuration. When the label removal step fails on fork PRs, the label remains stuck until manually removed, causing subsequent runs to skip.Switch the consumer workflow to
pull_request_targetto gain write access—but avoid checking out PR code in that context to prevent code injection attacks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/check_ci_coverage.yml around lines 5 - 21, The current gating logic in the check-ci-coverage workflow relies on .github/workflows/consume_ci_run_label.yml to remove the ci:run label, but that consumer cannot remove labels on forked pull requests because fork PRs get a read-only GITHUB_TOKEN; update the consumer workflow to use pull_request_target so it runs with write permissions and can remove labels, and ensure it does NOT check out or run untrusted PR code (remove or avoid actions/checkout with the PR head); also keep the conditional gating in check-ci-coverage (job name check-ci-coverage and its if: expression) unchanged so the label-removal step correctly enables subsequent runs..github/workflows/test_linting.yml (1)
20-30:⚠️ Potential issue | 🟠 MajorNon-
ci:runlabels trigger the workflow and create a skipped check that satisfies branch protection.When
pull_request.labeledfires for any label, this workflow starts and the lint job skips (due to theifcondition). Skipped jobs report as "Success" and satisfy required checks in branch protection. If the lint check is marked as required, any label can bypass the requirement without running linting.Either verify that the lint check is not required in branch protection, or route the workflow through a lightweight dispatcher so it only triggers for the
ci:runlabel.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test_linting.yml around lines 20 - 30, The current pull_request labeled trigger plus the job-level if (github.event_name != 'pull_request' || github.event.label.name == 'ci:run') causes the lint job to be skipped (reporting success) and can satisfy branch protection; fix by removing/avoiding a direct labeled trigger for the full lint workflow and instead implement a two-step flow: create a tiny "dispatcher" workflow that triggers on pull_request types: [labeled] and checks github.event.label.name == 'ci:run', and when that matches call/dispatch the real lint workflow via workflow_dispatch (or repository_dispatch); move the actual lint job (lint) into the workflow that is only triggered by workflow_dispatch and remove the ineffective job-level if so skipped jobs no longer count as required checks.
🧹 Nitpick comments (1)
.github/workflows/test_linting.yml (1)
20-30: Add the same concurrency gate here.This standalone workflow now participates in the
ci:rungate, but unlike the other updated workflows it still has noconcurrencyblock. Re-applyingci:runwhile a previous lint run is active will keep both runs on self-hosted capacity instead of canceling the stale one.Proposed fix
permissions: contents: read +concurrency: + group: ${{ github.workflow }}-${{ github.ref }}-${{ github.event_name == 'pull_request' && github.event.label.name != 'ci:run' && github.run_id || 'ci' }} + cancel-in-progress: true + jobs: lint:As per coding guidelines,
.github/workflows/**: Check workflow syntax, appropriate use of self-hosted vs ubuntu-latest runners, secret references, and concurrency settings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test_linting.yml around lines 20 - 30, The lint job lacks a concurrency gate, so add a concurrency block to the lint job (jobs.lint) to cancel in-progress runs when a new run is queued; specifically, under the lint job (the job keyed as "lint" with the if: ${{ github.event_name != 'pull_request' || github.event.label.name == 'ci:run' }}) add a concurrency stanza using a stable group expression (for example using github.workflow and github.ref or github.head_ref) and set cancel-in-progress: true so re-applying the ci:run label cancels the prior self-hosted lint run.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/consume_ci_run_label.yml:
- Around line 3-20: Change the workflow trigger from pull_request to
pull_request_target so the job "consume" runs with write permissions on forked
PRs; specifically replace the top-level "on:\n pull_request:\n types:
[labeled]" with "on:\n pull_request_target:\n types: [labeled]" (keep the
same job name "consume", steps and permissions), so gh issue edit can remove the
"ci:run" label even for fork PRs.
---
Outside diff comments:
In @.github/workflows/check_ci_coverage.yml:
- Around line 5-21: The current gating logic in the check-ci-coverage workflow
relies on .github/workflows/consume_ci_run_label.yml to remove the ci:run label,
but that consumer cannot remove labels on forked pull requests because fork PRs
get a read-only GITHUB_TOKEN; update the consumer workflow to use
pull_request_target so it runs with write permissions and can remove labels, and
ensure it does NOT check out or run untrusted PR code (remove or avoid
actions/checkout with the PR head); also keep the conditional gating in
check-ci-coverage (job name check-ci-coverage and its if: expression) unchanged
so the label-removal step correctly enables subsequent runs.
In @.github/workflows/test_linting.yml:
- Around line 20-30: The current pull_request labeled trigger plus the job-level
if (github.event_name != 'pull_request' || github.event.label.name == 'ci:run')
causes the lint job to be skipped (reporting success) and can satisfy branch
protection; fix by removing/avoiding a direct labeled trigger for the full lint
workflow and instead implement a two-step flow: create a tiny "dispatcher"
workflow that triggers on pull_request types: [labeled] and checks
github.event.label.name == 'ci:run', and when that matches call/dispatch the
real lint workflow via workflow_dispatch (or repository_dispatch); move the
actual lint job (lint) into the workflow that is only triggered by
workflow_dispatch and remove the ineffective job-level if so skipped jobs no
longer count as required checks.
---
Nitpick comments:
In @.github/workflows/test_linting.yml:
- Around line 20-30: The lint job lacks a concurrency gate, so add a concurrency
block to the lint job (jobs.lint) to cancel in-progress runs when a new run is
queued; specifically, under the lint job (the job keyed as "lint" with the if:
${{ github.event_name != 'pull_request' || github.event.label.name == 'ci:run'
}}) add a concurrency stanza using a stable group expression (for example using
github.workflow and github.ref or github.head_ref) and set cancel-in-progress:
true so re-applying the ci:run label cancels the prior self-hosted lint run.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c4c7fde9-009d-46a6-9393-539c85b28b16
📒 Files selected for processing (14)
.github/workflows/check_ci_coverage.yml.github/workflows/consume_ci_run_label.yml.github/workflows/kics.yml.github/workflows/test_elasticsearch_custom_certs.yml.github/workflows/test_elasticsearch_modules.yml.github/workflows/test_elasticsearch_upgrade.yml.github/workflows/test_full_stack.yml.github/workflows/test_linting.yml.github/workflows/test_plugins.yml.github/workflows/test_role_beats.yml.github/workflows/test_role_elasticsearch.yml.github/workflows/test_role_kibana.yml.github/workflows/test_role_logstash.yml.github/workflows/test_role_repos.yml
1 participant
Summary
ci:runlabelci:runlabel after it starts a CI run so follow-up pushes stay quietValidation
This lets review iteration continue without restarting the heavy CI set until the PR is ready.
Summary by CodeRabbit
New Features
ci:runlabel to a pull request.ci:runlabel after the run completes.Chores
ci:run.