Quick Deploy Webhooks do not get deleted on PR closure

bakerkretzmar / laravel-deploy-preview

A GitHub Action to deploy PR preview sites for Laravel apps.

MIT License

19 stars 6 forks source link

Quick Deploy Webhooks do not get deleted on PR closure #34

Closed GlitchWitch closed 3 months ago

GlitchWitch commented 5 months ago

Problem

When a PR is created a Github webhook is added to the given repository with a forge deploy URL like https://forge.laravel.com/github/push/[Token].

However when the PR is closed, this webhook is seemingly not always deleted. This results in a large number of unused webhooks remaining in the given repository, and eventually the following error:

Enabling Quick Deploy.
[
  "GitHub was unable to add the deployment hook to your repository.",
  "The \"push\" event cannot have more than 20 hooks"
]
Error: Forge API request failed with status code 422.

Proposed Solution

We should ensure that webhooks are deleted when a branch is closed

Additional Information

bakerkretzmar commented 5 months ago

Weird... I'm sure we can fix this but my first impression is that it's a technically a Forge issue. Forge creates those webhooks when enabling Quick Deploy, I would think it should delete them when deleting a site. We don't explicitly disable Quick Deploy before deleting the site but that would be the workaround I bet. I'll message Forge support about this too.

bakerkretzmar commented 4 months ago

Still discussing with Forge support. I can reproduce this intermittently in my test repo for this action, https://github.com/bakerkretzmar/laravel-deploy-preview-app. Forge does try to clean up these webhooks, so something on their side or GitHub's isn't always working as expected.

GlitchWitch commented 4 months ago

I'm glad to hear you were able to reproduce this, even if only intermittently.

I appreciate you working with the Forge team to investigate and relay updates, let me know if I can help!

bakerkretzmar commented 4 months ago

@GlitchWitch what server provider were you using here?

GlitchWitch commented 4 months ago

We self-host on colo'd/bare metal hardware across a couple locations.

Our forge deployments are all Ubuntu based LXC's, though I don't see why this would matter?

bakerkretzmar commented 4 months ago

Okay great, just wanted to make sure it wasn't somehow a DigitalOcean issue.

bakerkretzmar commented 3 months ago

@GlitchWitch can you confirm if v.2.5.0 fixes this?

GlitchWitch commented 3 months ago

@bakerkretzmar I'll do some testing and confirm this week!

GlitchWitch commented 3 months ago

After clearing out the old webhooks, it appears this is now fixed from my initial testing. Thanks for your help @bakerkretzmar !!