diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index dd3f251c65..ec25b44b98 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -260,6 +260,8 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $source = data_get($this->application, 'source'); if ($source) { $this->source = $source->getMorphClass()::where('id', $this->application->source->id)->first(); + // Share one source instance so the job reuses its in-memory GitHub installation token. + $this->application->setRelation('source', $this->source); } $this->server = Server::find($this->application_deployment_queue->server_id); $this->timeout = $this->server->settings->dynamic_timeout; diff --git a/app/Jobs/ProcessGithubPullRequestWebhook.php b/app/Jobs/ProcessGithubPullRequestWebhook.php index 362ad0a2d8..8abef9a486 100644 --- a/app/Jobs/ProcessGithubPullRequestWebhook.php +++ b/app/Jobs/ProcessGithubPullRequestWebhook.php @@ -14,6 +14,7 @@ use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; +use Illuminate\Support\Facades\Log; use Throwable; class ProcessGithubPullRequestWebhook implements ShouldBeEncrypted, ShouldQueue @@ -127,21 +128,34 @@ class ProcessGithubPullRequestWebhook implements ShouldBeEncrypted, ShouldQueue } } - // Get changed files for watch path filtering - $changed_files = collect(); + // Get changed files for watch path filtering (null means the full list is unknown) + $changed_files = []; if ($this->action === 'synchronize' && $this->beforeSha && $this->afterSha) { // For synchronize events, get files changed between before and after commits - $changed_files = collect(getGithubCommitRangeFiles($githubApp, $owner, $repo, $this->beforeSha, $this->afterSha)); + $changed_files = getGithubCommitRangeFiles($githubApp, $owner, $repo, $this->beforeSha, $this->afterSha); } elseif ($this->action === 'opened' || $this->action === 'reopened') { // For opened/reopened events, get all files in the PR - $changed_files = collect(getGithubPullRequestFiles($githubApp, $owner, $repo, $this->pullRequestId)); + $changed_files = getGithubPullRequestFiles($githubApp, $owner, $repo, $this->pullRequestId); } // Apply watch path filtering - $is_watch_path_triggered = $application->isWatchPathsTriggered($changed_files); - if (! $is_watch_path_triggered && ! blank($application->watch_paths)) { - return; + if (! blank($application->watch_paths)) { + $logContext = [ + 'application_uuid' => $application->uuid, + 'pull_request_id' => $this->pullRequestId, + 'action' => $this->action, + ]; + + if (is_null($changed_files)) { + Log::warning('Could not get the full list of changed files from GitHub. Deploying the pull request preview without the watch paths check.', $logContext); + } elseif (! $application->isWatchPathsTriggered(collect($changed_files))) { + Log::info('Skipped GitHub pull request preview deployment because no changed file matches the watch paths.', $logContext + [ + 'changed_files_count' => count($changed_files), + ]); + + return; + } } // Create ApplicationPreview if not exists diff --git a/app/Models/GithubApp.php b/app/Models/GithubApp.php index 0052232fb1..c7e5385fff 100644 --- a/app/Models/GithubApp.php +++ b/app/Models/GithubApp.php @@ -57,6 +57,34 @@ class GithubApp extends BaseModel 'webhook_secret', ]; + /** + * Installation token kept in memory, so one job does not request a new token for each GitHub call. + * + * @var array{credentials: string, token: string, expires_at: int}|null + */ + private ?array $installationTokenInMemory = null; + + public function installationToken(): string + { + $credentials = implode('|', [$this->api_url, $this->app_id, $this->installation_id, $this->private_key_id]); + $cached = $this->installationTokenInMemory; + + if ($cached && $cached['credentials'] === $credentials && $cached['expires_at'] > now()->getTimestamp()) { + return $cached['token']; + } + + $token = generateGithubToken($this, 'installation'); + + // GitHub installation tokens expire after one hour; renew them well before that. + $this->installationTokenInMemory = [ + 'credentials' => $credentials, + 'token' => $token, + 'expires_at' => now()->addMinutes(50)->getTimestamp(), + ]; + + return $token; + } + protected static function booted(): void { static::deleting(function (GithubApp $github_app) { diff --git a/bootstrap/helpers/github.php b/bootstrap/helpers/github.php index d66ffd88b8..46ab731e94 100644 --- a/bootstrap/helpers/github.php +++ b/bootstrap/helpers/github.php @@ -216,7 +216,7 @@ function generateGithubToken(GithubApp $source, string $type) function generateGithubInstallationToken(GithubApp $source) { - return generateGithubToken($source, 'installation'); + return $source->installationToken(); } function generateGithubJwt(GithubApp $source) @@ -397,28 +397,29 @@ function loadRepositoryByPage(GithubApp $source, string $token, int $page) 'repositories' => $json['repositories'], ]; } -function getGithubCommitRangeFiles(?GithubApp $source, string $owner, string $repo, string $beforeSha, string $afterSha): array +/** + * Returns null when the full list of changed files is unknown (API error or truncated list). + */ +function getGithubCommitRangeFiles(?GithubApp $source, string $owner, string $repo, string $beforeSha, string $afterSha): ?array { try { if (! $source) { // Manual webhooks don't have GitHub App authentication - // Return empty array so watch paths are ignored (current behavior) return []; } $endpoint = "/repos/{$owner}/{$repo}/compare/{$beforeSha}...{$afterSha}"; - $response = githubApi($source, $endpoint, 'get', null, false); - - if (! $response) { - return []; - } - + $response = githubApi($source, $endpoint); $files = collect(data_get($response, 'data.files', [])); + // The compare API returns at most 300 files and cannot paginate them. + if ($files->count() >= 300) { + return null; + } + return $files->pluck('filename')->filter()->values()->toArray(); } catch (Exception $e) { - - return []; + return null; } } @@ -444,27 +445,34 @@ function getGithubCommitMessage(?GithubApp $source, string $owner, string $repo, } } -function getGithubPullRequestFiles(?GithubApp $source, string $owner, string $repo, int $pullRequestId): array +/** + * Returns null when the full list of changed files is unknown (API error or truncated list). + */ +function getGithubPullRequestFiles(?GithubApp $source, string $owner, string $repo, int $pullRequestId): ?array { try { if (! $source) { // Manual webhooks don't have GitHub App authentication - // Return empty array so watch paths are ignored (current behavior) return []; } - $endpoint = "/repos/{$owner}/{$repo}/pulls/{$pullRequestId}/files"; - $response = githubApi($source, $endpoint, 'get', null, false); + // GitHub returns at most 100 files per page and 3000 files per pull request. + $perPage = 100; + $maxPages = 30; + $files = collect(); - if (! $response) { - return []; + for ($page = 1; $page <= $maxPages; $page++) { + $endpoint = "/repos/{$owner}/{$repo}/pulls/{$pullRequestId}/files?per_page={$perPage}&page={$page}"; + $pageFiles = collect(data_get(githubApi($source, $endpoint), 'data', [])); + $files = $files->merge($pageFiles->pluck('filename')); + + if ($pageFiles->count() < $perPage) { + return $files->filter()->values()->toArray(); + } } - $files = collect(data_get($response, 'data', [])); - - return $files->pluck('filename')->filter()->values()->toArray(); + return null; } catch (Exception $e) { - - return []; + return null; } } diff --git a/tests/Feature/GithubAppInstallationTokenTest.php b/tests/Feature/GithubAppInstallationTokenTest.php new file mode 100644 index 0000000000..bb83df0636 --- /dev/null +++ b/tests/Feature/GithubAppInstallationTokenTest.php @@ -0,0 +1,79 @@ +team = Team::factory()->create(); + + openssl_pkey_export(openssl_pkey_new([ + 'private_key_bits' => 2048, + 'private_key_type' => OPENSSL_KEYTYPE_RSA, + ]), $pemKey); + + $this->githubApp = GithubApp::create([ + 'name' => 'Private GitHub App', + 'api_url' => 'https://api.github.com', + 'html_url' => 'https://github.com', + 'app_id' => 12345, + 'installation_id' => 67890, + 'private_key_id' => PrivateKey::create([ + 'name' => 'GitHub App Key', + 'private_key' => $pemKey, + 'team_id' => $this->team->id, + ])->id, + 'is_public' => false, + 'team_id' => $this->team->id, + ]); + + Http::fake([ + 'https://api.github.com/zen' => fn () => Http::response('Keep it logically awesome.', 200, [ + 'Date' => now()->toRfc7231String(), + ]), + 'https://api.github.com/app/installations/*/access_tokens' => Http::sequence() + ->push(['token' => 'first-token'], 201) + ->push(['token' => 'second-token'], 201), + ]); +}); + +function githubInstallationTokenRequests(): int +{ + return Http::recorded(fn (Request $request): bool => str_ends_with($request->url(), '/access_tokens'))->count(); +} + +it('reuses the installation token of the same GitHub App instance', function () { + expect(generateGithubInstallationToken($this->githubApp))->toBe('first-token') + ->and(generateGithubInstallationToken($this->githubApp))->toBe('first-token') + ->and(githubInstallationTokenRequests())->toBe(1); +}); + +it('requests a new installation token after the in-memory token expires', function () { + generateGithubInstallationToken($this->githubApp); + + $this->travel(51)->minutes(); + + expect(generateGithubInstallationToken($this->githubApp))->toBe('second-token') + ->and(githubInstallationTokenRequests())->toBe(2); +}); + +it('requests a new installation token when the GitHub App installation changes', function () { + generateGithubInstallationToken($this->githubApp); + + $this->githubApp->installation_id = 11111; + + expect(generateGithubInstallationToken($this->githubApp))->toBe('second-token') + ->and(githubInstallationTokenRequests())->toBe(2); +}); + +it('does not share the installation token between GitHub App instances', function () { + generateGithubInstallationToken($this->githubApp); + + expect(generateGithubInstallationToken(GithubApp::find($this->githubApp->id)))->toBe('second-token') + ->and(githubInstallationTokenRequests())->toBe(2); +}); diff --git a/tests/Feature/ProcessGithubPullRequestWebhookTest.php b/tests/Feature/ProcessGithubPullRequestWebhookTest.php index 667a7643f0..1b5c3023c1 100644 --- a/tests/Feature/ProcessGithubPullRequestWebhookTest.php +++ b/tests/Feature/ProcessGithubPullRequestWebhookTest.php @@ -8,6 +8,7 @@ use App\Models\ApplicationPreview; use App\Models\Environment; use App\Models\GithubApp; use App\Models\InstanceSettings; +use App\Models\PrivateKey; use App\Models\Project; use App\Models\Server; use App\Models\StandaloneDocker; @@ -204,5 +205,227 @@ it('skips a GitHub pull request preview when the opened or reopened head commit ->and(ApplicationDeploymentQueue::where('application_id', $this->application->id)->where('pull_request_id', 42)->exists())->toBeFalse(); Http::assertSent(fn (Request $request): bool => $request->url() === 'https://api.github.com/repos/example/repo/commits/head-sha'); - Http::assertNotSent(fn (Request $request): bool => $request->url() === 'https://api.github.com/repos/example/repo/pulls/42/files'); + Http::assertNotSent(fn (Request $request): bool => str_contains($request->url(), '/repos/example/repo/pulls/42/files')); })->with(['opened', 'reopened']); + +it('deploys an opened GitHub pull request preview when a watched file is beyond the first page of changed files', function () { + Queue::fake(); + + $this->application->update(['watch_paths' => 'apps/app/**']); + $this->application->settings->update([ + 'is_preview_deployments_enabled' => true, + ]); + + $githubApp = GithubApp::create([ + 'name' => 'Public GitHub', + 'api_url' => 'https://api.github.com', + 'html_url' => 'https://github.com', + 'is_public' => true, + 'team_id' => $this->team->id, + ]); + + $firstPage = collect(range(1, 100))->map(fn (int $i): array => ['filename' => ".github/workflows/workflow-{$i}.yml"])->all(); + + Http::fake([ + 'https://api.github.com/repos/example/repo/commits/head-sha' => Http::response([ + 'commit' => ['message' => 'feat: add app'], + ]), + 'https://api.github.com/repos/example/repo/pulls/42/files?per_page=100&page=1' => Http::response($firstPage), + 'https://api.github.com/repos/example/repo/pulls/42/files?per_page=100&page=2' => Http::response([ + ['filename' => 'apps/app/index.ts'], + ]), + ]); + + $job = new ProcessGithubPullRequestWebhook( + applicationId: $this->application->id, + githubAppId: $githubApp->id, + action: 'opened', + pullRequestId: 42, + pullRequestHtmlUrl: 'https://github.com/example/repo/pull/42', + pullRequestTitle: 'Add feature', + beforeSha: null, + afterSha: null, + commitSha: 'head-sha', + authorAssociation: 'OWNER', + fullName: 'example/repo', + ); + + $job->handle(); + + expect(ApplicationPreview::where('application_id', $this->application->id)->where('pull_request_id', 42)->exists())->toBeTrue() + ->and(ApplicationDeploymentQueue::where('application_id', $this->application->id)->where('pull_request_id', 42)->exists())->toBeTrue(); + + Http::assertSentCount(3); +}); + +it('deploys a GitHub pull request preview when the full list of changed files is unknown', function (string $action, array $filesResponse) { + Queue::fake(); + + $this->application->update(['watch_paths' => 'apps/app/**']); + $this->application->settings->update([ + 'is_preview_deployments_enabled' => true, + ]); + + $githubApp = GithubApp::create([ + 'name' => 'Public GitHub', + 'api_url' => 'https://api.github.com', + 'html_url' => 'https://github.com', + 'is_public' => true, + 'team_id' => $this->team->id, + ]); + + Http::fake([ + 'https://api.github.com/repos/example/repo/commits/*' => Http::response([ + 'commit' => ['message' => 'feat: add app'], + ]), + ...$filesResponse, + ]); + + $job = new ProcessGithubPullRequestWebhook( + applicationId: $this->application->id, + githubAppId: $githubApp->id, + action: $action, + pullRequestId: 42, + pullRequestHtmlUrl: 'https://github.com/example/repo/pull/42', + pullRequestTitle: 'Add feature', + beforeSha: 'before-sha', + afterSha: 'after-sha', + commitSha: 'after-sha', + authorAssociation: 'OWNER', + fullName: 'example/repo', + ); + + $job->handle(); + + expect(ApplicationPreview::where('application_id', $this->application->id)->where('pull_request_id', 42)->exists())->toBeTrue() + ->and(ApplicationDeploymentQueue::where('application_id', $this->application->id)->where('pull_request_id', 42)->exists())->toBeTrue(); +})->with([ + 'opened with a GitHub API error' => fn () => ['opened', [ + 'https://api.github.com/repos/example/repo/pulls/42/files*' => Http::response(['message' => 'Server Error'], 500), + ]], + 'synchronize with a GitHub API error' => fn () => ['synchronize', [ + 'https://api.github.com/repos/example/repo/compare/*' => Http::response(['message' => 'Server Error'], 500), + ]], + 'synchronize with a truncated compare file list' => fn () => ['synchronize', [ + 'https://api.github.com/repos/example/repo/compare/*' => Http::response([ + 'files' => collect(range(1, 300))->map(fn (int $i): array => ['filename' => "docs/page-{$i}.md"])->all(), + ]), + ]], +]); + +it('skips an opened GitHub pull request preview when no changed file matches the watch paths', function () { + Queue::fake(); + + $this->application->update(['watch_paths' => 'apps/app/**']); + $this->application->settings->update([ + 'is_preview_deployments_enabled' => true, + ]); + + $githubApp = GithubApp::create([ + 'name' => 'Public GitHub', + 'api_url' => 'https://api.github.com', + 'html_url' => 'https://github.com', + 'is_public' => true, + 'team_id' => $this->team->id, + ]); + + Http::fake([ + 'https://api.github.com/repos/example/repo/commits/head-sha' => Http::response([ + 'commit' => ['message' => 'docs: update readme'], + ]), + 'https://api.github.com/repos/example/repo/pulls/42/files*' => Http::response([ + ['filename' => 'README.md'], + ]), + ]); + + $job = new ProcessGithubPullRequestWebhook( + applicationId: $this->application->id, + githubAppId: $githubApp->id, + action: 'opened', + pullRequestId: 42, + pullRequestHtmlUrl: 'https://github.com/example/repo/pull/42', + pullRequestTitle: 'Update docs', + beforeSha: null, + afterSha: null, + commitSha: 'head-sha', + authorAssociation: 'OWNER', + fullName: 'example/repo', + ); + + $job->handle(); + + expect(ApplicationPreview::where('application_id', $this->application->id)->where('pull_request_id', 42)->exists())->toBeFalse() + ->and(ApplicationDeploymentQueue::where('application_id', $this->application->id)->where('pull_request_id', 42)->exists())->toBeFalse(); +}); + +it('uses one GitHub App installation token for all GitHub API calls of a pull request job', function () { + Queue::fake(); + + $this->application->update(['watch_paths' => 'apps/app/**']); + $this->application->settings->update([ + 'is_preview_deployments_enabled' => true, + ]); + + openssl_pkey_export(openssl_pkey_new([ + 'private_key_bits' => 2048, + 'private_key_type' => OPENSSL_KEYTYPE_RSA, + ]), $pemKey); + + $githubApp = GithubApp::create([ + 'name' => 'Private GitHub App', + 'api_url' => 'https://api.github.com', + 'html_url' => 'https://github.com', + 'app_id' => 12345, + 'installation_id' => 67890, + 'private_key_id' => PrivateKey::create([ + 'name' => 'GitHub App Key', + 'private_key' => $pemKey, + 'team_id' => $this->team->id, + ])->id, + 'is_public' => false, + 'team_id' => $this->team->id, + ]); + + $firstPage = collect(range(1, 100))->map(fn (int $i): array => ['filename' => ".github/workflows/workflow-{$i}.yml"])->all(); + + Http::fake([ + 'https://api.github.com/zen' => Http::response('Keep it logically awesome.', 200, [ + 'Date' => now()->toRfc7231String(), + ]), + 'https://api.github.com/app/installations/67890/access_tokens' => Http::response([ + 'token' => 'fake-installation-token', + ], 201), + 'https://api.github.com/repos/example/repo/commits/head-sha' => Http::response([ + 'commit' => ['message' => 'feat: add app'], + ]), + 'https://api.github.com/repos/example/repo/pulls/42/files?per_page=100&page=1' => Http::response($firstPage), + 'https://api.github.com/repos/example/repo/pulls/42/files?per_page=100&page=2' => Http::response([ + ['filename' => 'apps/app/index.ts'], + ]), + ]); + + $job = new ProcessGithubPullRequestWebhook( + applicationId: $this->application->id, + githubAppId: $githubApp->id, + action: 'opened', + pullRequestId: 42, + pullRequestHtmlUrl: 'https://github.com/example/repo/pull/42', + pullRequestTitle: 'Add feature', + beforeSha: null, + afterSha: null, + commitSha: 'head-sha', + authorAssociation: 'OWNER', + fullName: 'example/repo', + ); + + $job->handle(); + + expect(ApplicationDeploymentQueue::where('application_id', $this->application->id)->where('pull_request_id', 42)->exists())->toBeTrue(); + + $tokenRequests = Http::recorded(fn (Request $request): bool => str_ends_with($request->url(), '/access_tokens')); + $repositoryRequests = Http::recorded(fn (Request $request): bool => str_contains($request->url(), '/repos/example/repo/')); + + expect($tokenRequests)->toHaveCount(1) + ->and($repositoryRequests)->toHaveCount(3) + ->and($repositoryRequests->every(fn (array $pair): bool => $pair[0]->hasHeader('Authorization', 'Bearer fake-installation-token')))->toBeTrue(); +});