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

Re-opened PRs can potentially merge requirements #1143

Open
klutchell opened this issue Jul 31, 2024 · 0 comments
Open

Re-opened PRs can potentially merge requirements #1143

klutchell opened this issue Jul 31, 2024 · 0 comments

Comments

@klutchell
Copy link
Collaborator

klutchell commented Jul 31, 2024

Discussion: https://balena.zulipchat.com/#narrow/stream/348930-balena-io.2Fflowzone/topic/Flowzone.20skipped.20almost.20everything/near/455074162

Here is the suspected order of events where a problem may occur:

  1. PR is closed manually for any number of reasons (unmerged)
  2. PR close event triggers the workflow and runs the Custom Clean job, and maybe some others
  3. At some point in the future, the PR is reopened manually or by Renovate
  4. Flowzone workflows are not currently triggered by reopen event types so nothing happens
  5. The last successful workflow run at this point was on the previous close event, so the All jobs required step has a result of skipped
  6. Jobs with a skipped result are unusually considered to be passing in branch protections, so at this point the PR can be merged manually by clicking Merge

The possible solutions vary:

  • Run All jobs on all event types, but mark as failed if the PR is not open
  • Run workflow on reopen events (this requires changing the calling workflows in 100s of repositories)
  • ???

The core of this issue seems to be that Github PR triggers do not include closed (including merged) events by default, and they must be added. Combine this with the fact that Github treats skipped jobs as meeting merge requirements puts us in this situation.

@klutchell klutchell changed the title Re-opened PRs can potentially bypass merge requirements Re-opened PRs can potentially merge requirements Jul 31, 2024
klutchell added a commit that referenced this issue Jul 31, 2024
This job is used for branch requirements, and
we don't want it to show as skipped in case the PR
is ever reopened.

See: #1143

Change-type: minor
Signed-off-by: Kyle Harding <[email protected]>
klutchell added a commit that referenced this issue Jul 31, 2024
This job is used for branch requirements, and
we don't want it to show as skipped in case the PR
is ever reopened.

See: #1143

Change-type: minor
Signed-off-by: Kyle Harding <[email protected]>
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

No branches or pull requests

1 participant