From 1464b751b1598521da3f678b9f0de8ccbf764274 Mon Sep 17 00:00:00 2001 From: johandenhollander Date: Thu, 15 Oct 2026 22:55:06 +0200 Subject: [PATCH] fix(webhooks): clean up closed GitLab MRs after target branch changes (#12012) Co-authored-by: Claude Opus 5.5 --- app/Http/Controllers/Webhook/Gitlab.php | 10 +- .../GitlabMergeRequestWebhookRoutingTest.php | 155 ++++++++++++++++++ 2 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 tests/Feature/GitlabMergeRequestWebhookRoutingTest.php diff --git a/app/Http/Controllers/Webhook/Gitlab.php b/app/Http/Controllers/Webhook/Gitlab.php index c135aef328..3d4d22cd85 100644 --- a/app/Http/Controllers/Webhook/Gitlab.php +++ b/app/Http/Controllers/Webhook/Gitlab.php @@ -231,7 +231,10 @@ class Gitlab extends Controller 'skip_deploy' => $skip_deploy_pr, ] = $this->readMergeRequestPayload($payload); - $applications = $applications->where('git_branch', $base_branch)->get(); + if (! in_array($action, ['closed', 'close', 'merge'])) { + $applications->where('git_branch', $base_branch); + } + $applications = $applications->get(); foreach ($applications as $application) { if (! $application->destination->server->isFunctional()) { @@ -440,7 +443,10 @@ class Gitlab extends Controller } } if ($x_gitlab_event === 'merge_request') { - $applications = $this->manualWebhookApplications($applications->where('git_branch', $base_branch), $full_name); + if (! in_array($action, ['closed', 'close', 'merge'])) { + $applications->where('git_branch', $base_branch); + } + $applications = $this->manualWebhookApplications($applications, $full_name); if ($applications->isEmpty()) { return $this->unauthenticatedManualWebhookResponse($failure_key, $failure_attempt); } diff --git a/tests/Feature/GitlabMergeRequestWebhookRoutingTest.php b/tests/Feature/GitlabMergeRequestWebhookRoutingTest.php new file mode 100644 index 0000000000..4f55c86e4a --- /dev/null +++ b/tests/Feature/GitlabMergeRequestWebhookRoutingTest.php @@ -0,0 +1,155 @@ +create(); + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + $server = Server::factory()->create(['team_id' => $team->id]); + $server->settings->update([ + 'is_reachable' => true, + 'is_usable' => true, + 'force_disabled' => false, + ]); + $destination = $server->standaloneDockers()->firstOrFail(); + + return Application::create(array_merge([ + 'name' => 'gitlab-mr-webhook-app', + 'git_repository' => 'https://gitlab.com/test-org/test-repo', + 'git_branch' => 'main', + 'build_pack' => 'nixpacks', + 'ports_exposes' => '3000', + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ], $overrides)); +} + +function createGitlabMergeRequestWebhookSource(): GitlabApp +{ + return GitlabApp::create([ + 'name' => 'gitlab-app-webhook-test', + 'api_url' => 'https://gitlab.com/api/v4', + 'html_url' => 'https://gitlab.com', + 'webhook_token' => 'test-webhook-token', + 'team_id' => Team::factory()->create()->id, + ]); +} + +function gitlabMergeRequestPayload(string $action, string $targetBranch): array +{ + return [ + 'object_kind' => 'merge_request', + 'project' => [ + 'id' => 987654321, + 'path_with_namespace' => 'test-org/test-repo', + ], + 'object_attributes' => [ + 'action' => $action, + 'iid' => 42, + 'url' => 'https://gitlab.com/test-org/test-repo/-/merge_requests/42', + 'title' => 'Stacked change', + 'source_branch' => 'feature/child', + 'target_branch' => $targetBranch, + 'source_project_id' => 987654321, + 'target_project_id' => 987654321, + 'last_commit' => [ + 'id' => 'abc1234def5678', + 'message' => 'Stacked change', + ], + ], + ]; +} + +function expectGitlabPreviewCleanup(Application $application): void +{ + $preview = ApplicationPreview::create([ + 'application_id' => $application->id, + 'pull_request_id' => 42, + 'pull_request_html_url' => 'https://gitlab.com/test-org/test-repo/-/merge_requests/42', + 'git_type' => 'gitlab', + ]); + + CleanupPreviewDeployment::shouldRun() + ->once() + ->withArgs(fn (Application $cleanedApplication, int $pullRequestId, ApplicationPreview $cleanedPreview): bool => $cleanedApplication->is($application) + && $pullRequestId === 42 + && $cleanedPreview->is($preview)) + ->andReturn([ + 'cancelled_deployments' => 0, + 'killed_containers' => 0, + 'status' => 'success', + ]); +} + +it('cleans up the preview of a closed GitLab App merge request after the target branch changes', function (string $action) { + $gitlabApp = createGitlabMergeRequestWebhookSource(); + $application = createGitlabMergeRequestWebhookApplication([ + 'repository_project_id' => 987654321, + 'source_id' => $gitlabApp->id, + 'source_type' => GitlabApp::class, + ]); + expectGitlabPreviewCleanup($application); + + $response = $this->postJson('/webhooks/source/gitlab/events', gitlabMergeRequestPayload($action, 'feature/parent'), [ + 'X-Gitlab-Token' => 'test-webhook-token', + ]); + + $response->assertOk(); + expect($response->getContent())->toContain('Preview deployment closed.'); +})->with(['close', 'merge']); + +it('cleans up the preview of a closed manual merge request after the target branch changes', function (string $action) { + $application = createGitlabMergeRequestWebhookApplication(); + expectGitlabPreviewCleanup($application); + + $response = $this->postJson('/webhooks/source/gitlab/events/manual', gitlabMergeRequestPayload($action, 'feature/parent'), [ + 'X-Gitlab-Event' => 'Merge Request Hook', + 'X-Gitlab-Token' => $application->manual_webhook_secret_gitlab, + ]); + + $response->assertOk(); + expect($response->getContent())->toContain('Preview deployment closed.'); +})->with(['close', 'merge']); + +it('continues to filter opened merge requests by target branch', function (string $endpoint) { + Queue::fake(); + + if ($endpoint === 'app') { + $gitlabApp = createGitlabMergeRequestWebhookSource(); + $application = createGitlabMergeRequestWebhookApplication([ + 'repository_project_id' => 987654321, + 'source_id' => $gitlabApp->id, + 'source_type' => GitlabApp::class, + ]); + $application->settings->update(['is_preview_deployments_enabled' => true]); + + $response = $this->postJson('/webhooks/source/gitlab/events', gitlabMergeRequestPayload('open', 'feature/parent'), [ + 'X-Gitlab-Token' => 'test-webhook-token', + ]); + } else { + $application = createGitlabMergeRequestWebhookApplication(); + $application->settings->update(['is_preview_deployments_enabled' => true]); + + $response = $this->postJson('/webhooks/source/gitlab/events/manual', gitlabMergeRequestPayload('open', 'feature/parent'), [ + 'X-Gitlab-Event' => 'Merge Request Hook', + 'X-Gitlab-Token' => $application->manual_webhook_secret_gitlab, + ]); + } + + $response->assertOk(); + expect(ApplicationPreview::where('application_id', $application->id)->exists())->toBeFalse(); +})->with(['app', 'manual']);