You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ignite.apache.org by "Anton Vinogradov (Jira)" <ji...@apache.org> on 2026/06/27 15:57:00 UTC

[jira] [Updated] (IGNITE-28827) Run all PR checks even when the PR conflicts with the base branch

     [ https://issues.apache.org/jira/browse/IGNITE-28827?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Anton Vinogradov updated IGNITE-28827:
--------------------------------------
    Description: 
h3. Problem

The GitHub Actions checks in .github/workflows/commit-check.yml (Code Style, Abandoned Tests,
Javadocs, .NET, ducktape) are not dispatched for a pull request that conflicts with the base
branch. The status checks never appear, so a contributor whose PR is temporarily in conflict
gets no CI feedback until the conflict is resolved.

h3. Root cause

These checks are triggered by the \{{pull_request}} event, which GitHub dispatches against the
*test-merge commit* (PR head merged into base). When the PR conflicts (mergeable state DIRTY),
GitHub cannot compute that merge commit, so no run is created. This is independent of what the
jobs check out — they already check out \{{pull_request.head.sha}}.

For comparison, the "Rolling Upgrade / Protected Classes" check runs fine on conflicting PRs
because it is triggered by \{{pull_request_target}}, which runs in the base-branch context and
does not depend on the test-merge commit.

h3. Proposed change

Switch commit-check.yml from \{{pull_request}} to \{{pull_request_target}}. That event is the only
PR-triggered event not gated on the merge commit, so it is the only way to dispatch these checks
on a conflicting PR. All jobs already check out \{{pull_request.head.sha}}.

h3. Security trade-off

These jobs build and *run* untrusted PR code (mvnw test, dotnet build, tox). Running untrusted
code under pull_request_target is sensitive, so the workflow is hardened to grant no more
privilege than the \{{pull_request}} event already did:
* the token is downscoped to \{{permissions: contents: read}} (no write);
* no \{{secrets.*}} are referenced;
* the workflow definition is taken from the base branch, so a PR cannot modify it.

A code comment documents this invariant so that secrets / write permissions are not added later
without revisiting the trade-off.

h3. Notes / follow-ups

* pull_request_target that builds and runs fork code goes against GitHub's default guidance for
  untrusted code; needs confirmation that ASF Infra policy permits it.
* Required checks / branch protection are unchanged (job names stay the same).

  was:
h3. Problem

The "Code Style" / checkstyle GitHub Actions check is not dispatched for a pull request that
has merge conflicts with the base branch. The status check simply never appears, so a
contributor whose PR is temporarily in conflict gets no code-style feedback until the conflict
is resolved.

h3. Root cause

The check lives in .github/workflows/commit-check.yml and is triggered by the \{{pull_request}}
event. For \{{pull_request}}, GitHub dispatches the run against the test-merge commit (the result
of merging the PR head into the base branch). When the PR conflicts with the base branch
(mergeable state DIRTY), GitHub cannot compute that merge commit, so no workflow run is created
at all. This is independent of what the job checks out (the job already checks out
{\{pull_request.head.sha}}).

For comparison, the "Rolling Upgrade / Protected Classes" check runs fine on conflicting PRs
because it is triggered by \{{pull_request_target}}, which executes in the base-branch context and
does not depend on the test-merge commit.

Checkstyle does not conceptually need a clean merge — it only needs the PR sources. The
limitation is inherited purely from the \{{pull_request}} event semantics.

h3. Proposed solution

Move the code-style + licenses checks into a dedicated workflow triggered by
{\{pull_request_target}} (plus \{{push}} for master/release branches, to keep coverage on pushes),
so the check is dispatched regardless of merge conflicts.

To keep this as safe as the current \{{pull_request}} event, the new workflow:
* downscopes the token to \{{permissions: contents: read}};
* references no repository secrets;
* still checks out \{{pull_request.head.sha}} and only *compiles* the PR sources
  (\{{mvnw test-compile ...}}) — it does not execute tests.

With write permissions and secrets removed, building untrusted fork code grants no privilege
beyond what the existing \{{pull_request}} job already allows, so no new attack surface is
introduced. The heavy steps that actually execute PR code (abandoned-tests check) stay on the
{\{pull_request}} event and are intentionally left untouched.

h3. Notes / follow-ups

* The status check name changes from "Check java code on JDK 17" to
  "Check java code style on JDK 17"; branch-protection required-checks must be updated by Infra.
* \{{pull_request_target}} that builds fork code is sensitive; worth confirming with ASF Infra.


> Run all PR checks even when the PR conflicts with the base branch
> -----------------------------------------------------------------
>
>                 Key: IGNITE-28827
>                 URL: https://issues.apache.org/jira/browse/IGNITE-28827
>             Project: Ignite
>          Issue Type: Task
>            Reporter: Anton Vinogradov
>            Assignee: Anton Vinogradov
>            Priority: Major
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> h3. Problem
> The GitHub Actions checks in .github/workflows/commit-check.yml (Code Style, Abandoned Tests,
> Javadocs, .NET, ducktape) are not dispatched for a pull request that conflicts with the base
> branch. The status checks never appear, so a contributor whose PR is temporarily in conflict
> gets no CI feedback until the conflict is resolved.
> h3. Root cause
> These checks are triggered by the \{{pull_request}} event, which GitHub dispatches against the
> *test-merge commit* (PR head merged into base). When the PR conflicts (mergeable state DIRTY),
> GitHub cannot compute that merge commit, so no run is created. This is independent of what the
> jobs check out — they already check out \{{pull_request.head.sha}}.
> For comparison, the "Rolling Upgrade / Protected Classes" check runs fine on conflicting PRs
> because it is triggered by \{{pull_request_target}}, which runs in the base-branch context and
> does not depend on the test-merge commit.
> h3. Proposed change
> Switch commit-check.yml from \{{pull_request}} to \{{pull_request_target}}. That event is the only
> PR-triggered event not gated on the merge commit, so it is the only way to dispatch these checks
> on a conflicting PR. All jobs already check out \{{pull_request.head.sha}}.
> h3. Security trade-off
> These jobs build and *run* untrusted PR code (mvnw test, dotnet build, tox). Running untrusted
> code under pull_request_target is sensitive, so the workflow is hardened to grant no more
> privilege than the \{{pull_request}} event already did:
> * the token is downscoped to \{{permissions: contents: read}} (no write);
> * no \{{secrets.*}} are referenced;
> * the workflow definition is taken from the base branch, so a PR cannot modify it.
> A code comment documents this invariant so that secrets / write permissions are not added later
> without revisiting the trade-off.
> h3. Notes / follow-ups
> * pull_request_target that builds and runs fork code goes against GitHub's default guidance for
>   untrusted code; needs confirmation that ASF Infra policy permits it.
> * Required checks / branch protection are unchanged (job names stay the same).



--
This message was sent by Atlassian Jira
(v8.20.10#820010)