Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci(github): fix checklist PR comment #12180

Merged
merged 3 commits into from
Dec 6, 2024
Merged

ci(github): fix checklist PR comment #12180

merged 3 commits into from
Dec 6, 2024

Conversation

lahabana
Copy link
Contributor

@lahabana lahabana commented Dec 5, 2024

Motivation

Implementation information

To be able to leave a comment on a pull request we need pull_request: write permissions. Unfortunately github won't allow this level of permissions on PRs coming from forks.
The way to get the permissions is to use pull_request_target as a triggering event.
However, there are a lot of possible attack vectors when checking out code when using this event.
Therefore we inline commitlint.config.js inside the action to not have to checkout any code and be able to run this action safely.

Supporting documentation

xrel: #11654 #11666
superseeds: #11674

part of: https://github.com/Kong/team-mesh/issues/302

@lahabana lahabana requested a review from a team as a code owner December 5, 2024 17:14
@lahabana lahabana requested review from slonka and bartsmykla and removed request for a team December 5, 2024 17:14
This is necessary in order to safely run with pull_request_target
we simply don't checkout any code so this workflow can execute safely on
forks

Signed-off-by: Charly Molter <[email protected]>
@lahabana lahabana added the ci/skip-test PR: Don't run unit and e2e tests (maybe this is just a doc change) label Dec 5, 2024
Copy link
Contributor

@slonka slonka left a comment

Choose a reason for hiding this comment

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

Before merging please post a successful run from a branch if possible.

Signed-off-by: Charly Molter <[email protected]>
@lahabana
Copy link
Contributor Author

lahabana commented Dec 6, 2024

Here's an example:

lahabana/sandbox#13

just checking both files are the same:

➜  sandbox git:(main) curl -s https://raw.githubusercontent.com/lahabana/sandbox/refs/heads/main/.github/workflows/check.yaml  | sha256sum
91e5f3309d9ffbdf08b7b29a96c2766d020b66fad43fa3f64489c1790d1c964b  -
➜  sandbox git:(main) curl -s https://raw.githubusercontent.com/kumahq/kuma/refs/heads/fixPR/.github/workflows/check.yaml  | sha256sum
91e5f3309d9ffbdf08b7b29a96c2766d020b66fad43fa3f64489c1790d1c964b  -

@lahabana lahabana enabled auto-merge (squash) December 6, 2024 10:31
@lahabana lahabana merged commit ff16ad0 into master Dec 6, 2024
10 checks passed
@lahabana lahabana deleted the fixPR branch December 6, 2024 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/skip-test PR: Don't run unit and e2e tests (maybe this is just a doc change)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants