ci: switch away from pull_request_target in the pre-commit workflow [RHIDP 11561]

[rm3l]

Description of the change

This splits the pre-commit workflow into 2 different workflows to switch away from the unsafe pull_request_target trigger.
As a consequence, instead of automatically pushing the pre-commit diff to the PR branch, the user will just be made aware of the failures with clear instructions. It will then be up to the PR author or maintainers to run the pre-commit hooks and push the changes.

More context in https://issues.redhat.com/browse/RHIDP-11561

Which issue(s) does this PR fix or relate to

Fixes https://issues.redhat.com/browse/RHIDP-11561

How to test changes / Special notes to the reviewer

This takes inspiration from the upstream backstage/charts repo that we depend on: https://github.com/backstage/charts/blob/main/.github/workflows/pre-commit-comment.yaml

Example of comment in the PR:

rm3l#1 (comment)

Checklist

• For each Chart updated, version bumped in the corresponding Chart.yaml according to Semantic Versioning.
• For each Chart updated, variables are documented in the values.yaml and added to the corresponding README.md. The pre-commit utility can be used to generate the necessary content. Use pre-commit run -a to apply changes. The pre-commit Workflow will do this automatically for you if needed.
• JSON Schema template updated and re-generated the raw schema via the pre-commit hook.
• Tests pass using the Chart Testing tool and the ct lint command.
• If you updated the orchestrator-infra chart, make sure the versions of the Knative CRDs are aligned with the versions of the CRDs installed by the OpenShift Serverless operators declared in the values.yaml file. See Installing Knative Eventing and Knative Serving CRDs for more details.

[rhdh-qodo-merge]

PR Reviewer Guide 🔍

(Review updated until commit 983b410)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🔒 No security concerns identified

⚡ Recommended focus areas for review Permissions The workflow posts/deletes PR issue comments via github.rest.issues.*. Depending on repo/org defaults, pull-requests: write may not be sufficient for issue comments; consider explicitly granting issues: write (and keeping permissions minimal) to avoid silent failures when attempting to create/delete comments. permissions: pull-requests: write Comment matching Previous comments are identified by checking whether the body includes a specific string. This can be brittle (format changes, localization, user quoting, etc.). Consider adding a stable hidden marker (e.g., an HTML comment token) to the posted body and matching on that marker to avoid missing old comments or accidentally deleting unrelated ones. const botComments = comments.filter(comment => { return comment.user.login === 'github-actions[bot]' && comment.body && comment.body.includes('⚠️ Pre-commit hook failures'); });

📄 References redhat-developer/rhdh-chart/charts/orchestrator-software-templates/ci/upstream-values.yaml [1-16] redhat-developer/rhdh-chart/charts/backstage/ci/with-test-pod-disabled-values.yaml [1-12] redhat-developer/rhdh-chart/charts/backstage/ci/with-custom-dynamic-pvc-claim-spec-values.yaml [0-2] redhat-developer/rhdh-chart/charts/backstage/ci/with-orchestrator-values.yaml [1-21] redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/ci/upstream-values.yaml [1-27] redhat-developer/rhdh/scripts/rhdh-openshift-setup/values.yaml [238-252] redhat-developer/rhdh/scripts/rhdh-openshift-setup/values.yaml [103-106] redhat-developer/rhdh/scripts/rhdh-openshift-setup/values.yaml [107-121]

[rhdh-qodo-merge]

Branches

Removed authorization job and manual approval safeguard, Simplified checkout to use default PR context instead of fork repository

---

PR Type

(Describe updated until commit 983b410)

Enhancement

---

Description

• Split pre-commit workflow into 2 separate workflows to eliminate unsafe pull_request_target trigger

• Removed automatic commit/push functionality; users now receive PR comments with instructions

• Added new pre-commit-comment.yaml workflow to post failure notifications on PRs

• Updated PR template to reflect manual pre-commit execution requirement

---

File Walkthrough

	Relevant files
Enhancement	pre-commit.yaml Simplify pre-commit workflow and remove auto-commit            .github/workflows/pre-commit.yaml Changed trigger from pull_request_target to pull_request for improved security Removed authorize job that required manual approval for external fork PRs Removed automatic commit/push logic that modified PR branches Simplified checkout step to use default PR context Removed continue-on-error and post-failure comment logic +1/-59    pre-commit-comment.yaml New workflow to comment on pre-commit failures                      .github/workflows/pre-commit-comment.yaml New workflow triggered on completion of pre-commit workflow Retrieves PR number from workflow run event Deletes previous pre-commit failure comments to avoid duplicates Posts detailed comment with instructions when pre-commit fails Includes setup steps for dependencies and command to run hooks +98/-0
Documentation	pull_request_template.md Update PR template for manual pre-commit execution              .github/pull_request_template.md Updated pre-commit checklist item to reflect manual execution requirement Changed from pre-commit run -a to pre-commit run --all-files Updated description to indicate workflow enforces and warns instead of auto-fixing +1/-1

---

[rhdh-qodo-merge]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Grant write permission for issues API Grant issues: write permission to the workflow, as it is required for creating and deleting PR comments via the issues API. .github/workflows/pre-commit-comment.yaml [14-15] permissions: pull-requests: write + issues: write Apply / Chat Suggestion importance[1-10]: 10 __ Why: This is a critical fix; without issues: write permission, the workflow will fail to create or delete comments, rendering it non-functional.	High
General	✅ Optimize PR number retrieval logic Suggestion Impact: The workflow script was updated to use context.payload.workflow_run.pull_requests instead of an API call to pulls.list, and added logs for when a PR is found or not found. code diff: @@ -22,20 +22,13 @@ with: script: | // Get PR number from the workflow run event - const headRepo = context.payload.workflow_run.head_repository.owner.login; - const headBranch = context.payload.workflow_run.head_branch; - - console.log(`Looking for PR with head: ${headRepo}:${headBranch}`); - - const { data: pullRequests } = await github.rest.pulls.list({ - owner: context.repo.owner, - repo: context.repo.repo, - state: 'open', - head: `${headRepo}:${headBranch}`, - }); + const pullRequests = context.payload.workflow_run.pull_requests; if (pullRequests.length > 0) { core.setOutput('number', pullRequests[0].number); + console.log(`Found PR number: ${pullRequests[0].number}`); + } else { + console.log('No associated pull request found.'); } To improve efficiency and reliability, retrieve the pull request number directly from the context.payload.workflow_run.pull_requests array instead of making a separate API call to list all pull requests. .github/workflows/pre-commit-comment.yaml [19-39] - name: Get the PR number from the workflow run id: pr-number uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 with: script: | // Get PR number from the workflow run event - const headRepo = context.payload.workflow_run.head_repository.owner.login; - const headBranch = context.payload.workflow_run.head_branch; - - console.log(`Looking for PR with head: ${headRepo}:${headBranch}`); - - const { data: pullRequests } = await github.rest.pulls.list({ - owner: context.repo.owner, - repo: context.repo.repo, - state: 'open', - head: `${headRepo}:${headBranch}`, - }); + const pullRequests = context.payload.workflow_run.pull_requests; if (pullRequests.length > 0) { core.setOutput('number', pullRequests[0].number); + console.log(`Found PR number: ${pullRequests[0].number}`); + } else { + console.log('No associated pull request found.'); } [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a more efficient and robust way to retrieve the PR number by using the workflow_run event payload, thus avoiding an unnecessary API call.	Medium
Update

[rm3l]

/cherry-pick release-1.9
/cherry-pick release-1.8
/cherry-pick release-1.7

pull_request_target does not seem to be used in the other release branches.

[openshift-cherrypick-robot]

@rm3l: once the present PR merges, I will cherry-pick it on top of release-1.7, release-1.8, release-1.9 in new PRs and assign them to you.

Details

In response to this:

/cherry-pick release-1.9
/cherry-pick release-1.8
/cherry-pick release-1.7

pull_request_target does not seem to be used in the other release branches.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[rm3l]

/cc @kim-tsao

[rhdh-qodo-merge]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🔒 No security concerns identified

⚡ Recommended focus areas for review Permissions The workflow uses github.rest.issues.listComments, deleteComment, and createComment, which typically require issues: write permission (issue comments API), not just pull-requests: write. Please verify the current token permissions are sufficient; otherwise the comment/deletion steps may fail at runtime. comment: name: Comment on PR runs-on: ubuntu-latest if: github.event.workflow_run.event == 'pull_request' permissions: pull-requests: write steps: - name: Get the PR number from the workflow run id: pr-number uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 with: script: | // Get PR number from the workflow run event const pullRequests = context.payload.workflow_run.pull_requests; if (pullRequests.length > 0) { core.setOutput('number', pullRequests[0].number); console.log(`Found PR number: ${pullRequests[0].number}`); } else { console.log('No associated pull request found.'); } - name: Delete previous pre-commit failure comments if: steps.pr-number.outputs.number uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: PR_NUMBER: ${{ steps.pr-number.outputs.number }} with: script: | const prNumber = parseInt(process.env.PR_NUMBER, 10); // Get all comments on the PR const { data: comments } = await github.rest.issues.listComments({ owner: context.repo.owner, repo: context.repo.repo, issue_number: prNumber, }); console.log(`Found ${comments.length} total comments on PR #${prNumber}`); const botComments = comments.filter(comment => { return comment.user.login === 'github-actions[bot]' && comment.body && comment.body.includes('⚠️ Pre-commit hook failures'); }); for (const comment of botComments) { await github.rest.issues.deleteComment({ owner: context.repo.owner, repo: context.repo.repo, comment_id: comment.id, }); } - name: Post comment if: steps.pr-number.outputs.number && github.event.workflow_run.conclusion == 'failure' uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 Trigger Pattern The pull_request.branches entries look regex-like (e.g., release-1.[0-9]+) but GitHub Actions branch filters use glob-style patterns. Validate that the intended release branches actually match, otherwise the workflow may not run for expected PR targets. on: pull_request: branches: - main - release-1.[0-9]+

📄 References redhat-developer/rhdh-chart/charts/orchestrator-software-templates/ci/upstream-values.yaml [1-16] redhat-developer/rhdh-chart/charts/backstage/ci/with-test-pod-disabled-values.yaml [1-12] redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/ci/upstream-values.yaml [1-27] redhat-developer/rhdh/scripts/rhdh-openshift-setup/values.yaml [238-252] redhat-developer/rhdh/scripts/rhdh-openshift-setup/values.yaml [103-106] redhat-developer/rhdh/scripts/rhdh-openshift-setup/values.yaml [79-102] redhat-developer/rhdh-chart/charts/orchestrator-software-templates/templates/tekton/tekton-tasks.yaml [117-151] redhat-developer/rhdh-chart/charts/orchestrator-software-templates/templates/tekton/tekton-tasks.yaml [91-116]

[rhdh-qodo-merge]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	✅ Handle deletion errors gracefully Suggestion Impact: The comment deletion loop was updated to wrap github.rest.issues.deleteComment in a try/catch block and log a warning on failure, preventing a single deletion error from breaking the workflow. code diff: for (const comment of botComments) { - await github.rest.issues.deleteComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: comment.id, - }); + try { + await github.rest.issues.deleteComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: comment.id, + }); + } catch (error) { + console.warn(`Failed to delete comment ${comment.id}:`, error.message); + } } Make the comment deletion process more robust by wrapping the deleteComment API call in a try/catch block. This prevents a single failed deletion from causing the entire workflow step to fail. .github/workflows/pre-commit-comment.yaml [57-63] for (const comment of botComments) { - await github.rest.issues.deleteComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: comment.id, - }); + try { + await github.rest.issues.deleteComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: comment.id, + }); + } catch (error) { + console.warn(`Failed to delete comment ${comment.id}:`, error.message); + } } [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: This suggestion correctly identifies a potential point of failure and proposes adding try/catch blocks to make the comment deletion loop more robust against errors, preventing the entire job from failing.	Medium
	Optimize comment deletion using search API Optimize the comment deletion step by using the GitHub search API to directly find relevant comments, instead of fetching all comments and filtering them. This improves efficiency and reduces API usage. .github/workflows/pre-commit-comment.yaml [33-63] - name: Delete previous pre-commit failure comments if: steps.pr-number.outputs.number uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: PR_NUMBER: ${{ steps.pr-number.outputs.number }} with: script: | const prNumber = parseInt(process.env.PR_NUMBER, 10); - - // Get all comments on the PR - const { data: comments } = await github.rest.issues.listComments({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: prNumber, + const owner = context.repo.owner; + const repo = context.repo.repo; + + const { data: { items: comments } } = await github.rest.search.issuesAndPullRequests({ + q: `is:pr is:issue repo:${owner}/${repo} issue:${prNumber} author:app/github-actions "⚠️ Pre-commit hook failures" in:body`, }); - console.log(`Found ${comments.length} total comments on PR #${prNumber}`); + console.log(`Found ${comments.length} previous failure comments on PR #${prNumber}`); - const botComments = comments.filter(comment => { - return comment.user.login === 'github-actions[bot]' && - comment.body && - comment.body.includes('⚠️ Pre-commit hook failures'); - }); - - for (const comment of botComments) { + for (const comment of comments) { await github.rest.issues.deleteComment({ - owner: context.repo.owner, - repo: context.repo.repo, + owner: owner, + repo: repo, comment_id: comment.id, }); } Apply / Chat Suggestion importance[1-10]: 6 __ Why: This is a valid optimization that makes the workflow more efficient by using the search API instead of listing all comments and filtering, which is beneficial for PRs with many comments.	Low
More

[rm3l]

/review

[rhdh-qodo-merge]

Persistent review updated to latest commit 983b410

[kim-tsao]

/lgtm

[openshift-ci]

New changes are detected. LGTM label has been removed.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[openshift-cherrypick-robot]

@rm3l: new pull request created: #308

Details

In response to this:

/cherry-pick release-1.9
/cherry-pick release-1.8
/cherry-pick release-1.7

pull_request_target does not seem to be used in the other release branches.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[openshift-cherrypick-robot]

@rm3l: #306 failed to apply on top of branch "release-1.8":

Applying: fix: split the pre-commit workflow into 2 different workflows to switch away from the unsafe pull_request_target trigger
.git/rebase-apply/patch:71: trailing whitespace.
              return comment.user.login === 'github-actions[bot]' && 
.git/rebase-apply/patch:72: trailing whitespace.
                     comment.body && 
warning: 2 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	.github/workflows/pre-commit.yaml
Falling back to patching base and 3-way merge...
Auto-merging .github/workflows/pre-commit.yaml
CONFLICT (content): Merge conflict in .github/workflows/pre-commit.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 fix: split the pre-commit workflow into 2 different workflows to switch away from the unsafe pull_request_target trigger

Details

In response to this:

/cherry-pick release-1.9
/cherry-pick release-1.8
/cherry-pick release-1.7

pull_request_target does not seem to be used in the other release branches.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[openshift-cherrypick-robot]

@rm3l: #306 failed to apply on top of branch "release-1.7":

Applying: fix: split the pre-commit workflow into 2 different workflows to switch away from the unsafe pull_request_target trigger
.git/rebase-apply/patch:71: trailing whitespace.
              return comment.user.login === 'github-actions[bot]' && 
.git/rebase-apply/patch:72: trailing whitespace.
                     comment.body && 
warning: 2 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	.github/workflows/pre-commit.yaml
Falling back to patching base and 3-way merge...
Auto-merging .github/workflows/pre-commit.yaml
CONFLICT (content): Merge conflict in .github/workflows/pre-commit.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 fix: split the pre-commit workflow into 2 different workflows to switch away from the unsafe pull_request_target trigger

Details

In response to this:

/cherry-pick release-1.9
/cherry-pick release-1.8
/cherry-pick release-1.7

pull_request_target does not seem to be used in the other release branches.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.