Skip to content

[release-1.8] ci: switch away from pull_request_target in the pre-commit workflow [RHIDP 11561] (#306) - #310

Merged
rm3l merged 1 commit into
redhat-developer:release-1.8from
rm3l:RHIDP-11561--update-gh-workflows--1.8
Jan 30, 2026
Merged

[release-1.8] ci: switch away from pull_request_target in the pre-commit workflow [RHIDP 11561] (#306)#310
rm3l merged 1 commit into
redhat-developer:release-1.8from
rm3l:RHIDP-11561--update-gh-workflows--1.8

Conversation

@rm3l

@rm3l rm3l commented Jan 30, 2026

Copy link
Copy Markdown
Member

Manual cherry-pick of #306

…ommit workflow [RHIDP 11561] (redhat-developer#306)

Co-authored-by: rhdh-qodo-merge[bot] <232573409+rhdh-qodo-merge[bot]@users.noreply.github.com>
@sonarqubecloud

Copy link
Copy Markdown

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Permissions

The workflow uses the Issues REST API (issues.listComments, issues.deleteComment, issues.createComment) but only grants pull-requests: write. Verify the token permissions are sufficient; you may need to add issues: write (and possibly issues: read) for comment listing/creation/deletion to work reliably.

permissions:
  pull-requests: write
Edge Case

The PR-number extraction step only sets an output when workflow_run.pull_requests is non-empty, but does not explicitly fail or log when it is empty. Confirm this workflow will always have an associated PR for your triggering conditions; otherwise consider handling the "no PR found" case to avoid silent no-op behavior.

- name: Get the PR number from the workflow run
  id: pr-number
  uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8
  with:
    script: |
      const prs = context.payload.workflow_run.pull_requests;
      if (prs.length > 0) {
        const num = prs[0].number;
        if (Number.isInteger(num)) {
          core.setOutput('number', num);
        } else {
          core.setFailed(`Invalid PR number detected: ${num}`);
        }
      }
📄 References
  1. redhat-developer/rhdh-chart/ct.yaml [1-10]
  2. redhat-developer/rhdh-chart/ct-install.yaml [1-11]
  3. redhat-developer/rhdh-chart/cr.yaml [1-2]
  4. redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/ci/upstream-values.yaml [1-27]
  5. redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/templates/openshift-pipelines/post-cleanup.yaml [0-2]
  6. redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/templates/openshift-gitops/post-cleanup.yaml [0-2]
  7. redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/templates/openshift-pipelines/subscription.yaml [0-2]
  8. redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/templates/openshift-gitops/subscription.yaml [0-2]

@rhdh-qodo-merge rhdh-qodo-merge Bot added the enhancement New feature or request label Jan 30, 2026
@rhdh-qodo-merge

Copy link
Copy Markdown

PR Type

Enhancement


Description

  • Switch pre-commit workflow from pull_request_target to pull_request trigger

  • Remove authorization job and manual approval requirement for external forks

  • Add separate workflow for posting pre-commit failure comments on PRs

  • Simplify pre-commit job by removing auto-commit and push functionality

  • Update pull request template with clearer pre-commit instructions


File Walkthrough

Relevant files
Documentation
pull_request_template.md
Update pre-commit instructions in PR template                       

.github/pull_request_template.md

  • Updated pre-commit command from pre-commit run -a to pre-commit run
    --all-files
  • Changed workflow behavior description from automatic application to
    enforcement with warnings
  • Clarified that users should push resulting changes manually
+1/-1     
Enhancement
pre-commit-comment.yaml
Add workflow for posting pre-commit failure comments         

.github/workflows/pre-commit-comment.yaml

  • New workflow triggered by pre-commit workflow completion via
    workflow_run event
  • Extracts PR number from workflow run and posts failure comments on PRs
  • Deletes previous pre-commit failure comments before posting new ones
  • Provides instructions for fixing pre-commit hook failures
+103/-0 
pre-commit.yaml
Simplify pre-commit workflow and remove auto-commit logic

.github/workflows/pre-commit.yaml

  • Changed trigger from pull_request_target to pull_request for improved
    security
  • Removed authorize job and environment-based approval requirement
  • Removed auto-commit and push functionality from pre-commit job
  • Simplified checkout step by removing fork repository and ref
    parameters
  • Added persist-credentials: false to prevent token leakage
  • Removed manual comment posting logic (now handled by separate
    workflow)
+6/-59   

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Use valid tool versions in setup actions

Correct the invalid python-version from 3.14 to a valid version like 3.12 and
update the go-version specifier to '1.x' for better clarity and compatibility.

.github/workflows/pre-commit.yaml [30-36]

 - uses: actions/setup-python@e797f83bcb11b83ae66e0230d6156d7c80228e7c # v6
   with:
-    python-version: 3.14
+    python-version: '3.12'
 
 - uses: actions/setup-go@44694675825211faa026b3c33043df3e48a5fa00 # v6
   with:
-    go-version: ^1
+    go-version: '1.x'
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that python-version: 3.14 is invalid and will cause the workflow to fail, which is a critical bug fix.

High
Handle cases where no PR is found

Add error handling to the script that retrieves the PR number to explicitly fail
the job if no pull request is found for the workflow run.

.github/workflows/pre-commit-comment.yaml [19-32]

 - name: Get the PR number from the workflow run
   id: pr-number
   uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8
   with:
     script: |
       const prs = context.payload.workflow_run.pull_requests;
       if (prs.length > 0) {
         const num = prs[0].number;
         if (Number.isInteger(num)) {
           core.setOutput('number', num);
         } else {
           core.setFailed(`Invalid PR number detected: ${num}`);
         }
+      } else {
+        core.setFailed('No pull requests found for this workflow run.');
       }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: This is a valid suggestion that improves the robustness of the workflow by handling an edge case where no pull request is associated with the workflow run, preventing silent failures.

Low
General
Simplify template indent

Simplify the multiline string creation for the comment body by un-indenting the
text and removing the .replace(/^ {12}/gm, '') call.

.github/workflows/pre-commit-comment.yaml [79-96]

 const body = `
-            ## ⚠️ Pre-commit hook failures
-            ...
-            `.trim().replace(/^ {12}/gm, '');
+## ⚠️ Pre-commit hook failures
 
+The pre-commit hooks detected issues that need to be fixed.
+
+**To fix these issues:**
+
+1. Install the required dependencies:
+   - [pre-commit](https://pre-commit.com/#install)
+   - [helm-docs](https://github.com/norwoodj/helm-docs#installation)
+
+2. Run pre-commit on all files:
+   \`\`\`bash
+   pre-commit run --all-files
+   \`\`\`
+
+3. Commit and push any resulting changes
+`.trim();
+

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 3

__

Why: This suggestion improves code readability and simplifies the string manipulation by removing an unnecessary regex replacement, although the functional impact is minimal.

Low
  • More

@rm3l
rm3l merged commit b280ec6 into redhat-developer:release-1.8 Jan 30, 2026
6 of 7 checks passed
@rm3l
rm3l deleted the RHIDP-11561--update-gh-workflows--1.8 branch January 30, 2026 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant