Skip to content

SONARJAVA-6899 Onboard SonarJava onto the PVF - #6288

Open
rombirli wants to merge 2 commits into
masterfrom
rombirli/sonarjava-6899-onboard-pvf
Open

rombirli wants to merge 2 commits into
masterfrom
rombirli/sonarjava-6899-onboard-pvf

Conversation

@rombirli

@rombirli rombirli commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Part of SONARJAVA-6899. Companion PR: SonarSource/peachee-java-kotlin#327

Onboards SonarJava onto the Performance Validation Framework, the same way SonarJS#7913 did for SonarJS.

Because this repository is public and the PVF report must stay private, the benchmark and the dashboard are hosted in peachee-java-kotlin (as SonarJS hosts its in peachee-js). This PR only adds the two ends of the handshake:

  • build.yml records the deployed plugin version as the candidate-version artifact.
  • pvf-comment.yml turns a /pvf comment on a PR into a workflow_dispatch of performance-validation-java.yml in peachee-java-kotlin, which benchmarks that version and comments the dashboard link back here.

Usage once merged

Comment /pvf on a PR to benchmark every rule, or /pvf S1234, S5678 to restrict the run to specific rules.

Prerequisites, all landed

  • Vault token pvf-dispatch for this repository, with actions:write on peachee-java-kotlin — re-terraform-aws-vault#9750
  • The host-side tokens and the GitHub Pages build_type switch, both listed on the companion PR

Testing

The build.yml half is proven by this PR's own Build run: it published candidate-version = 8.45.0.59032.

The host side is proven too. 8.45.0.59032 was benchmarked against a baseline pinned two releases back, over the full 140-project matrix, in peachee-java-kotlin run 37020618474: every project built and prepared, the issue diff reported real rule differences, and the dashboard published to GitHub Pages. That exercises the project matrix, the build delegation, the server context, the report publication and the issue diffing. See peachee-java-kotlin#329 for how that run was triggered.

What remains untestable before merge is only the first hop: GitHub registers an issue_comment workflow and a workflow_dispatch target only once the file sits on the default branch, so /pvf cannot fire from a branch on either side. That hop is the shared pvf-trigger action already in production for SonarJS, so it is not new code.

Merge order

The companion PR has to merge first, because pvf-comment.yml dispatches performance-validation-java.yml against peachee-java-kotlin's master. After both are in, comment /pvf here to confirm the last hop end to end.

Record the deployed sonar-java version as the candidate-version artifact so a
later /pvf comment can resolve which build to benchmark.
A `/pvf` comment on a PR dispatches performance-validation-java.yml in
peachee-java-kotlin, which runs the benchmark and comments the dashboard link
back here. The dashboard is hosted there because this repository is public and
the report has to stay private.
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

SONARJAVA-6899

Comment on lines +16 to +20
permissions:
actions: read
contents: read
pull-requests: read
id-token: write

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 [cross-pr] pull-requests: read blocks pvf-trigger feedback added by tooling PR #70 · SonarSource/core-languages-tooling-public #70

This depends on another open PR. This workflow (sonar-java#6288, head bd3d709) calls SonarSource/core-languages-tooling-public/pvf-trigger@master, which is a moving ref, and only grants pull-requests: read. core-languages-tooling-public#70 (head 0e5b127c9ce9b85386289b99f79331aefe9a5e92) changes that action in two ways. It adds a 👀 reaction, which is best-effort. It also adds an if: failure() step that runs gh pr comment to tell the commenter why the dispatch failed. That step is not best-effort, and the PR says callers must raise pull-requests to write to get the feedback. Condition: once #70 is merged to master, any failed /pvf dispatch here (for example, Build has not uploaded candidate-version yet) will have its failure-report step fail with a 403 from gh pr comment. The commenter gets no feedback, the run shows an extra failed step, and the 👀 acknowledgement on successful runs is silently skipped.

Was this helpful? React with 👍 / 👎

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

gitar the pull-requests: write perm is needed by the token actually passed through to the action dispatch-token.

Will the pull-requests: read limit that or just define the default permissions of the token?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good question — these are two separate tokens, so it depends on which one is used.

permissions: pull-requests: read in this workflow only configures the default GITHUB_TOKEN for this run (the implicit token GitHub Actions injects, available via github.token / secrets.GITHUB_TOKEN). It has no effect on dispatch-token, since that's a completely separate token fetched from Vault (DISPATCH_TOKEN) and passed explicitly as an input — its scopes come from however that Vault secret/PAT was provisioned, not from this YAML's permissions: block.

So:

  • If the gh pr comment step added in core-languages-tooling-public#70 uses the default GITHUB_TOKEN (which gh picks up automatically from the environment unless told otherwise), it would be blocked here — commenting needs pull-requests: write, and this workflow only grants read.
  • If that step is passed dispatch-token explicitly (e.g. via env: GH_TOKEN: ${{ inputs.dispatch-token }} or similar), it's unaffected by this file's permissions and will work as long as the Vault-issued token has comment-write scope.

Worth confirming in the composite action which token the if: failure() comment step actually uses before deciding whether pull-requests: read needs to be bumped to write here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

gitar YOU have access you lazy shit. YOU confirm

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Checked the actual code in core-languages-tooling-public#70. The reaction and the gh pr comment failure-feedback step use GH_TOKEN: ${{ github.token }} — i.e. this workflow's own GITHUB_TOKEN — not dispatch-token.

dispatch-token is only used to call the cross-repo dispatch into peachee-java-kotlin (needs actions:write there). It has no bearing on commenting back on this PR.

So the pull-requests: read set in .github/workflows/pvf-comment.yml:16-20 does limit github.token to read-only on pull requests, which blocks the gh pr comment feedback step added in that other PR. To unblock it, this workflow would need pull-requests: write, independent of dispatch-token's scope.

@gitar-bot

gitar-bot Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 0 closed / 1 findings

🔴 High risk · Adds a collaborator-gated workflow that dispatches runs using a cross-repository Vault token.

Onboards SonarJava onto the Performance Validation Framework with workflow triggers for /pvf comments and plugin version publishing. Once the companion pvf-trigger action is merged, consider raising pull-requests permission to write in pvf-comment.yml to allow the tooling to provide feedback on failed dispatches.

💡 [cross-pr] pull-requests: read blocks pvf-trigger feedback added by tooling PR #70 · SonarSource/core-languages-tooling-public #70

📄 .github/workflows/pvf-comment.yml:16-20 📄 .github/workflows/pvf-comment.yml:44

This depends on another open PR. This workflow (sonar-java#6288, head bd3d709) calls SonarSource/core-languages-tooling-public/pvf-trigger@master, which is a moving ref, and only grants pull-requests: read. core-languages-tooling-public#70 (head 0e5b127c9ce9b85386289b99f79331aefe9a5e92) changes that action in two ways. It adds a 👀 reaction, which is best-effort. It also adds an if: failure() step that runs gh pr comment to tell the commenter why the dispatch failed. That step is not best-effort, and the PR says callers must raise pull-requests to write to get the feedback. Condition: once #70 is merged to master, any failed /pvf dispatch here (for example, Build has not uploaded candidate-version yet) will have its failure-report step fail with a 403 from gh pr comment. The commenter gets no feedback, the run shows an extra failed step, and the 👀 acknowledgement on successful runs is silently skipped.

🤖 Prompt for agents
Code Review: Onboards SonarJava onto the Performance Validation Framework with workflow triggers for `/pvf` comments and plugin version publishing. Once the companion `pvf-trigger` action is merged, consider raising `pull-requests` permission to `write` in `pvf-comment.yml` to allow the tooling to provide feedback on failed dispatches.

1. 💡 [cross-pr] pull-requests: read blocks pvf-trigger feedback added by tooling PR #70 · [SonarSource/core-languages-tooling-public #70](<https://github.com/SonarSource/core-languages-tooling-public/pull/70>)
   Files: .github/workflows/pvf-comment.yml:16-20, .github/workflows/pvf-comment.yml:44

   This depends on another open PR. This workflow (sonar-java#6288, head bd3d709d0b7bcb1aa15b94b260956723acd27588) calls `SonarSource/core-languages-tooling-public/pvf-trigger@master`, which is a moving ref, and only grants `pull-requests: read`. [core-languages-tooling-public#70](https://github.com/SonarSource/core-languages-tooling-public/pull/70) (head 0e5b127c9ce9b85386289b99f79331aefe9a5e92) changes that action in two ways. It adds a 👀 reaction, which is best-effort. It also adds an `if: failure()` step that runs `gh pr comment` to tell the commenter why the dispatch failed. That step is not best-effort, and the PR says callers must raise `pull-requests` to `write` to get the feedback. Condition: once #70 is merged to master, any failed `/pvf` dispatch here (for example, Build has not uploaded `candidate-version` yet) will have its failure-report step fail with a 403 from `gh pr comment`. The commenter gets no feedback, the run shows an extra failed step, and the 👀 acknowledgement on successful runs is silently skipped.

Review coverage

🧪 Functional validation No results

📋 Rules No rules evaluated

Cross-repo coverage 1 repository selected

🤖 Auto-approval Not enabled · Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@rombirli
rombirli force-pushed the rombirli/sonarjava-6899-onboard-pvf branch from bd3d709 to adf3cc4 Compare October 2, 2026 07:49
@sonarqube-next

sonarqube-next Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

@rombirli

rombirli commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Gitar, Valid but conditional on SonarSource/core-languages-tooling-public#70, still open. SonarJS has same permission, keeping parity.

@rombirli

rombirli commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

This branch has not been deployed

No deployments
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