Skip to content

Add GitHub Enterprise support #3720

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.

Sign up for GitHub

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

Open
arhowe00 wants to merge 3 commits into
base: master
Choose a base branch
from
+53 −8
Open

Add GitHub Enterprise support #3720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7485892
Add GitHub Enterprise Server support
arhowe00 May 29, 2026
000d859
Guard createOrUpdateCheck with try/catch; inspired by getRepositorySe…
arhowe00 Jun 2, 2026
2928d72
Fix PHPStan warnings
arhowe00 Jun 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions app/Http/Submission/Handlers/DoneHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,9 @@ public function endElement($parser, $name): void
$this->Build->UpdateBuild($this->Build->Id, -1, -1);
$this->Build->MarkAsDone(true);

// Should we re-run any checks that were previously marked
// as pending?
if ($pendingSubmissionsModel !== null && $pendingSubmissionsModel->recheck) {
$revision = \App\Models\Build::findOrFail((int) $this->Build->Id)->updateStep->revision ?? '';
// Create or update the GitHub check for this commit.
$revision = \App\Models\Build::findOrFail((int) $this->Build->Id)->updateStep->revision ?? '';
if ($revision !== '') {
Comment on lines -66 to +68
Copy link
Copy Markdown
Collaborator

@williamjallen williamjallen Jun 2, 2026

Choose a reason for hiding this comment

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

What's the thought process here?

Repository::createOrUpdateCheck($revision);
}

Expand Down
29 changes: 28 additions & 1 deletion app/Utils/RepositoryUtils.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,19 @@

class RepositoryUtils
{
private static function isGitHubUrl(string $url): bool
{
if (str_contains($url, 'github.com')) {
return true;
}
$enterpriseUrl = config('cdash.github_enterprise_url');
if (is_string($enterpriseUrl)) {
$host = parse_url($enterpriseUrl, PHP_URL_HOST);
Copy link
Copy Markdown
Collaborator

@williamjallen williamjallen Jun 2, 2026

Choose a reason for hiding this comment

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

You can use Laravel's Uri facade to do this more eloquently (note: untested).

Apply throughout.

Suggested change
$host = parse_url($enterpriseUrl, PHP_URL_HOST);
$host = Uri::of($enterpriseUrl)->host();

return is_string($host) && str_contains($url, $host);
}
return false;
}
Comment on lines +17 to +28
Copy link
Copy Markdown
Collaborator

@williamjallen williamjallen Jun 2, 2026

Choose a reason for hiding this comment

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

This is a duplicate of isGitHubUrl() in app/cdash/app/Lib/Repository/GitHub.php. Let's remove the one in app/cdash/app/Lib/Repository/GitHub.php.


/** Return the GitHub diff URL */
public static function get_github_diff_url($projecturl, $directory, $file, $revision)
{
Expand Down Expand Up @@ -146,6 +159,20 @@ public static function post_pull_request_comment($projectid, $pull_request, $com
/** Convert GitHub repository viewer URL into corresponding API URL. */
public static function get_github_api_url($github_url): string
{
$enterpriseUrl = config('cdash.github_enterprise_url');
if (is_string($enterpriseUrl)) {
$host = parse_url($enterpriseUrl, PHP_URL_HOST);
if (is_string($host)) {
$idx = strpos($github_url, $host);
if ($idx !== false) {
// For GHE, ...://<host>/<user>/<repo> becomes ...://<host>/api/v3/repos/<user>/<repo>
$idx2 = $idx + strlen($host) + 1;
$api_url = substr($github_url, 0, $idx) . $host . '/api/v3/repos/';
$api_url .= substr($github_url, $idx2);
return $api_url;
}
}
}
Comment on lines +162 to +175
Copy link
Copy Markdown
Collaborator

@williamjallen williamjallen Jun 2, 2026

Choose a reason for hiding this comment

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

A few questions/comments:

  1. Can you please add a comment explaining what's going on here?
  2. Does ...<host>/api/v3... work for GitHub.com too? i.e., are we strictly required to have a GHE-specific code path here?
  3. Assuming we need a GHE-specific code path, can you wrap the following code in an else so it's clear that there are two branches? It took a second for me to understand what was going on here.
  4. There are probably Laravel string facade methods which could make this cleaner.

/*
* For a URL of the form:
* ...://github.com/<user>/<repo>
Expand All @@ -166,7 +193,7 @@ public static function post_github_pull_request_comment(Project $project, $pull_
$repo = null;
$repositories = $project->GetRepositories();
foreach ($repositories as $repository) {
if (str_contains($repository['url'], 'github.com')) {
if (self::isGitHubUrl($repository['url'])) {
$repo = $repository;
break;
}
Expand Down
18 changes: 16 additions & 2 deletions app/cdash/app/Lib/Repository/GitHub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public function __construct(Project $project)

$repositories = $this->project->GetRepositories();
foreach ($repositories as $repo) {
if (str_contains($repo['url'], 'github.com')) {
if ($this->isGitHubUrl($repo['url'])) {
$this->installationId = $repo['username'];
break;
}
Expand All @@ -93,7 +93,8 @@ public function setApiClient(GitHubClient $client): void
protected function initializeApiClient(): void
{
$builder = new GitHubBuilder();
$apiClient = new GitHubClient($builder, 'machine-man-preview');
$enterpriseUrl = config('cdash.github_enterprise_url');
$apiClient = new GitHubClient($builder, null, is_string($enterpriseUrl) ? $enterpriseUrl : null);
Copy link
Copy Markdown
Collaborator

@williamjallen williamjallen Jun 2, 2026

Choose a reason for hiding this comment

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

You should treat an empty string like a null here.

$this->setApiClient($apiClient);
}

Expand Down Expand Up @@ -662,6 +663,19 @@ public function getRepository(): string
return $this->repo;
}

private function isGitHubUrl(string $url): bool
{
if (str_contains($url, 'github.com')) {
return true;
}
$enterpriseUrl = config('cdash.github_enterprise_url');
if (is_string($enterpriseUrl)) {
$host = parse_url($enterpriseUrl, PHP_URL_HOST);
return is_string($host) && str_contains($url, $host);
}
Comment on lines +671 to +675
Copy link
Copy Markdown
Collaborator

@williamjallen williamjallen Jun 2, 2026

Choose a reason for hiding this comment

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

You're referencing the config value multiple times in this file. It would be better to factor that out into a method which returns the enterprise URL or null.

return false;
}

protected function getRepositoryInformation(): void
{
$url = str_replace('//', '', $this->project->CvsUrl ?? '');
Expand Down
6 changes: 5 additions & 1 deletion app/cdash/app/Model/Repository.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,11 @@ public static function createOrUpdateCheck($sha): void
$project = new Project();
$project->Id = $projectid;
$project->Fill();
$repositoryInterface = self::getRepositoryInterface($project);
try {
$repositoryInterface = self::getRepositoryInterface($project);
} catch (Exception $e) {
return;
}
Comment on lines -87 to +91
Copy link
Copy Markdown
Collaborator

@williamjallen williamjallen Jun 2, 2026

Choose a reason for hiding this comment

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

Why is this necessary? This will destroy all exceptions within getRepositoryInterface(), preventing them from making it to the logs.

$repositoryInterface->createCheck($sha);
}

Expand Down
1 change: 1 addition & 0 deletions config/cdash.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
'delete_old_subprojects' => env('DELETE_OLD_SUBPROJECTS', true),
'github_always_pass' => env('GITHUB_ALWAYS_PASS', false),
'github_app_id' => env('GITHUB_APP_ID', null),
'github_enterprise_url' => env('GITHUB_ENTERPRISE_URL', null),
'github_private_key' => env('GITHUB_PRIVATE_KEY', null),
'github_webhook_secret' => env('GITHUB_WEBHOOK_SECRET', null),
'large_text_limit' => env('LARGE_TEXT_LIMIT', 0),
Expand Down