-
Notifications
You must be signed in to change notification settings - Fork 990
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
Add support for ghe.com domain #2420
Conversation
src/Runner.Sdk/Util/UrlUtil.cs
Outdated
@@ -8,7 +8,8 @@ public static bool IsHostedServer(UriBuilder gitHubUrl) | |||
{ | |||
return string.Equals(gitHubUrl.Host, "github.com", StringComparison.OrdinalIgnoreCase) || | |||
string.Equals(gitHubUrl.Host, "www.github.com", StringComparison.OrdinalIgnoreCase) || | |||
string.Equals(gitHubUrl.Host, "github.localhost", StringComparison.OrdinalIgnoreCase); | |||
string.Equals(gitHubUrl.Host, "github.localhost", StringComparison.OrdinalIgnoreCase) || | |||
gitHubUrl.Host.EndsWith("ghe.com", StringComparison.OrdinalIgnoreCase); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EndsWith("ghe.com"
or EndsWith(".ghe.com"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we introduce an env to allow overwriting this behavior? for edge cases where the customer's GHES is on the same name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I'll fix that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would GHES ever be on ghe.com
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You never know what customers will try to use in their network.
d11d31b
to
fd622a6
Compare
src/Runner.Sdk/Util/UrlUtil.cs
Outdated
return string.Equals(gitHubUrl.Host, "github.com", StringComparison.OrdinalIgnoreCase) || | ||
string.Equals(gitHubUrl.Host, "www.github.com", StringComparison.OrdinalIgnoreCase) || | ||
string.Equals(gitHubUrl.Host, "github.localhost", StringComparison.OrdinalIgnoreCase); | ||
string.Equals(gitHubUrl.Host, "github.localhost", StringComparison.OrdinalIgnoreCase) || | ||
gitHubUrl.Host.EndsWith(".ghe.com", StringComparison.OrdinalIgnoreCase); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something like
var takeOverGhe = StringUtil.ConvertToBoolean(Environment.GetEnvironmentVariable("GITHUB_ACTIONS_RUNNER_TAKEOVER_GHE_DOTCOM"));
return string.Equals(gitHubUrl.Host, "github.com", StringComparison.OrdinalIgnoreCase) ||
string.Equals(gitHubUrl.Host, "www.github.com", StringComparison.OrdinalIgnoreCase) ||
string.Equals(gitHubUrl.Host, "github.localhost", StringComparison.OrdinalIgnoreCase) ||
(!takeOverGhe && gitHubUrl.Host.EndsWith(".ghe.com", StringComparison.OrdinalIgnoreCase));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in case GHES customers for whatever reason used .ghe.com
, they can export GITHUB_ACTIONS_RUNNER_TAKEOVER_GHE_DOTCOM=1
and make their things keep working.
fd622a6
to
9785521
Compare
src/Runner.Sdk/Util/UrlUtil.cs
Outdated
var forceGhes = StringUtil.ConvertToBoolean(Environment.GetEnvironmentVariable("GITHUB_ACTIONS_RUNNER_FORCE_GHES")); | ||
if (forceGhes) | ||
{ | ||
return false; | ||
} | ||
|
||
return | ||
string.Equals(gitHubUrl.Host, "github.com", StringComparison.OrdinalIgnoreCase) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var forceGhes = StringUtil.ConvertToBoolean(Environment.GetEnvironmentVariable("GITHUB_ACTIONS_RUNNER_FORCE_GHES")); | |
if (forceGhes) | |
{ | |
return false; | |
} | |
return | |
string.Equals(gitHubUrl.Host, "github.com", StringComparison.OrdinalIgnoreCase) || | |
if (StringUtil.ConvertToBoolean(Environment.GetEnvironmentVariable("GITHUB_ACTIONS_RUNNER_FORCE_GHES"))) | |
{ | |
return false; | |
} | |
return string.Equals(gitHubUrl.Host, "github.com", StringComparison.OrdinalIgnoreCase) || |
9785521
to
3fb58e8
Compare
string.Equals(gitHubUrl.Host, "github.localhost", StringComparison.OrdinalIgnoreCase) || | ||
gitHubUrl.Host.EndsWith(".ghe.com", StringComparison.OrdinalIgnoreCase); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be an equivalent ghe.localhost
check since we use that in github/github development?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a good point. I hadn't thought about local development. We can track keep tract of that and add it in. Thanks @mtodd!
cc @javs-perez
This PR adds support for treating domains ending in
ghe.com
as hosted.