Skip to content

Commit 1f4505d

Browse files
ci: switch away from pull_request_target in the pre-commit workflow [RHIDP 11561] (#306)
Co-authored-by: rhdh-qodo-merge[bot] <232573409+rhdh-qodo-merge[bot]@users.noreply.github.com>
1 parent 25a5d37 commit 1f4505d

3 files changed

Lines changed: 110 additions & 60 deletions

File tree

.github/pull_request_template.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ Detailed instructions may help reviewers test this PR quickly and provide quicke
2424
## Checklist
2525

2626
- [ ] For each Chart updated, version bumped in the corresponding `Chart.yaml` according to [Semantic Versioning](http://semver.org/).
27-
- [ ] For each Chart updated, variables are documented in the `values.yaml` and added to the corresponding README.md. The [pre-commit](https://pre-commit.com/) utility can be used to generate the necessary content. Use `pre-commit run -a` to apply changes. The [pre-commit Workflow](./workflows/pre-commit.yaml) will do this automatically for you if needed.
27+
- [ ] For each Chart updated, variables are documented in the `values.yaml` and added to the corresponding README.md. The [pre-commit](https://pre-commit.com/) utility can be used to generate the necessary content. Run `pre-commit run --all-files` to run the hooks and then push any resulting changes. The [pre-commit Workflow](./workflows/pre-commit.yaml) will enforce this and warn you if needed.
2828
- [ ] JSON Schema template updated and re-generated the raw schema via the `pre-commit` hook.
2929
- [ ] Tests pass using the [Chart Testing](https://github.com/helm/chart-testing) tool and the `ct lint` command.
3030
- [ ] If you updated the [orchestrator-infra](../charts/orchestrator-infra) chart, make sure the versions of the [Knative CRDs](../charts/orchestrator-infra/crds) are aligned with the versions of the CRDs installed by the OpenShift Serverless operators declared in the [values.yaml](../charts/orchestrator-infra/values.yaml) file. See [Installing Knative Eventing and Knative Serving CRDs](../charts/orchestrator-infra/README.md#installing-knative-eventing-and-knative-serving-crds) for more details.
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
name: Pre-commit comment
2+
3+
on:
4+
workflow_run:
5+
workflows: ["Pre-commit"]
6+
types:
7+
- completed
8+
9+
jobs:
10+
comment:
11+
name: Comment on PR
12+
runs-on: ubuntu-latest
13+
if: github.event.workflow_run.event == 'pull_request'
14+
permissions:
15+
pull-requests: write
16+
17+
steps:
18+
19+
- name: Get the PR number from the workflow run
20+
id: pr-number
21+
uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8
22+
with:
23+
script: |
24+
const prs = context.payload.workflow_run.pull_requests;
25+
if (prs.length > 0) {
26+
const num = prs[0].number;
27+
if (Number.isInteger(num)) {
28+
core.setOutput('number', num);
29+
} else {
30+
core.setFailed(`Invalid PR number detected: ${num}`);
31+
}
32+
}
33+
34+
- name: Delete previous pre-commit failure comments
35+
if: steps.pr-number.outputs.number
36+
uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8
37+
env:
38+
PR_NUMBER: ${{ steps.pr-number.outputs.number }}
39+
with:
40+
script: |
41+
const prNumber = parseInt(process.env.PR_NUMBER, 10);
42+
43+
// Get all comments on the PR
44+
const { data: comments } = await github.rest.issues.listComments({
45+
owner: context.repo.owner,
46+
repo: context.repo.repo,
47+
issue_number: prNumber,
48+
});
49+
50+
console.log(`Found ${comments.length} total comments on PR #${prNumber}`);
51+
52+
const botComments = comments.filter(comment => {
53+
return comment.user.login === 'github-actions[bot]' &&
54+
comment.body &&
55+
comment.body.includes('⚠️ Pre-commit hook failures');
56+
});
57+
58+
for (const comment of botComments) {
59+
try {
60+
await github.rest.issues.deleteComment({
61+
owner: context.repo.owner,
62+
repo: context.repo.repo,
63+
comment_id: comment.id,
64+
});
65+
} catch (error) {
66+
console.warn(`Failed to delete comment ${comment.id}:`, error.message);
67+
}
68+
}
69+
70+
- name: Post comment
71+
if: steps.pr-number.outputs.number && github.event.workflow_run.conclusion == 'failure'
72+
uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8
73+
env:
74+
PR_NUMBER: ${{ steps.pr-number.outputs.number }}
75+
with:
76+
script: |
77+
const prNumber = parseInt(process.env.PR_NUMBER, 10);
78+
79+
const body = `
80+
## ⚠️ Pre-commit hook failures
81+
82+
The pre-commit hooks detected issues that need to be fixed.
83+
84+
**To fix these issues:**
85+
86+
1. Install the required dependencies:
87+
- [pre-commit](https://pre-commit.com/#install)
88+
- [helm-docs](https://github.com/norwoodj/helm-docs#installation)
89+
90+
2. Run pre-commit on all files:
91+
\`\`\`bash
92+
pre-commit run --all-files
93+
\`\`\`
94+
95+
3. Commit and push any resulting changes
96+
`.trim().replace(/^ {12}/gm, '');
97+
98+
await github.rest.issues.createComment({
99+
issue_number: prNumber,
100+
owner: context.repo.owner,
101+
repo: context.repo.repo,
102+
body: body
103+
});

.github/workflows/pre-commit.yaml

Lines changed: 6 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,31 @@
11
name: Pre-commit
22

33
on:
4-
# pull_request_target needed to be able to commit and push pre-commit diffs to external fork PRs.
5-
# But we included a manual authorization safeguard to prevent PWN requests. See the 'authorize' job below.
6-
pull_request_target:
4+
pull_request:
75
branches:
86
- main
9-
- rhdh-1.[0-9]+
10-
- 1.[0-9]+.x
117
- release-1.[0-9]+
128

139
concurrency:
1410
group: ${{ github.workflow }}-${{ github.event.number }}
1511
cancel-in-progress: true
1612

17-
jobs:
18-
authorize:
19-
# The 'external' environment is configured with the maintainers team as required reviewers.
20-
# All the subsequent jobs in this workflow 'need' this job, which will require manual approval for PRs coming from external forks.
21-
# see list of approvers in OWNERS file
22-
environment:
23-
${{ (github.event.pull_request.head.repo.full_name == github.repository ||
24-
contains(fromJSON('["gazarenkov","kadel","nickboldt","rm3l","kim-tsao","Fortune-Ndlovu","subhashkhileri","zdrapela","openshift-cherrypick-robot", "OpinionatedHeron"]'), github.event.pull_request.user.login)) && 'internal' || 'external' }}
25-
runs-on: ubuntu-latest
26-
steps:
27-
- name: approved
28-
run: echo "✓"
13+
# Revoke all permissions by default, then grant only what is needed
14+
permissions:
15+
contents: read
2916

17+
jobs:
3018
pre-commit:
3119
name: Pre-commit
3220
runs-on: ubuntu-latest
33-
needs: authorize
34-
permissions:
35-
contents: write
36-
pull-requests: write
3721
env:
3822
GO111MODULE: on
3923
steps:
4024
- name: Checkout
4125
uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6
4226
with:
4327
fetch-depth: 0
44-
repository: ${{github.event.pull_request.head.repo.full_name}}
45-
ref: ${{ github.event.pull_request.head.sha }}
28+
persist-credentials: false # Avoid token leakage to hooks
4629

4730
- uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6
4831
with:
@@ -57,41 +40,5 @@ jobs:
5740

5841
- name: Run pre-commit
5942
uses: pre-commit/action@2c7b3805fd2a0fd8c1884dcaebf91fc102a13ecd # v3.0.1
60-
continue-on-error: true # Don't fail immediately; we'll handle it below
6143
with:
6244
extra_args: --verbose --all-files --show-diff-on-failure
63-
64-
- name: Check for changes after pre-commit
65-
id: diff-checker
66-
run: |
67-
echo "CHANGED=$(if git diff --quiet; then echo "false"; else echo "true"; fi)" >> $GITHUB_OUTPUT
68-
69-
- name: Commit and push any manifest changes
70-
if: ${{ steps.diff-checker.outputs.CHANGED == 'true' }}
71-
run: |
72-
git remote add fork "https://github.com/${{ github.event.pull_request.head.repo.full_name }}.git"
73-
git fetch fork ${{ github.event.pull_request.head.ref }}
74-
git checkout -B pr-branch fork/${{ github.event.pull_request.head.ref }}
75-
76-
git config user.name 'github-actions[bot]'
77-
git config user.email 'github-actions[bot]@users.noreply.github.com'
78-
79-
git add -A .
80-
git commit \
81-
-m "chore(pre-commit): Auto-fix hooks" \
82-
-m "Co-authored-by: $GITHUB_ACTOR <$GITHUB_ACTOR@users.noreply.github.com>"
83-
84-
git push fork pr-branch:${{ github.event.pull_request.head.ref }}
85-
86-
- name: Comment on PR if manifests were updated
87-
uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8
88-
if: ${{ !cancelled() && steps.diff-checker.outputs.CHANGED == 'true' }}
89-
continue-on-error: true
90-
with:
91-
script: |
92-
await github.rest.issues.createComment({
93-
issue_number: context.issue.number,
94-
owner: context.repo.owner,
95-
repo: context.repo.repo,
96-
body: '⚠️ <b>Files changed after running the pre-commit hooks</b><br/><br/>Those changes should have been pushed automatically to your PR branch.<br/><br/><b>NOTE: </b>If the PR checks are stuck after this additional commit, manually close the PR and immediately reopen it to trigger the checks again.'
97-
})

0 commit comments

Comments
 (0)