Skip to content
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

Jump to bottom

Lets allow up to 150 characters for services on linux/mac #1710

Merged
merged 9 commits into from
Feb 25, 2022
Merged

Lets allow up to 150 characters for services on linux/mac #1710

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
bda7355
Lets allow up to 150 characters on linux/mac, just to avoid some issu…
thboop Feb 24, 2022
e3ea8d9
Add 4 randomized digits on mac/linux
thboop Feb 24, 2022
d94cc09
fix pragma issue
thboop Feb 24, 2022
929b975
fix test
thboop Feb 24, 2022
42aa209
Address pr feedback
thboop Feb 24, 2022
ebd53b4
reduce complexity
thboop Feb 24, 2022
d50eb2d
lets make it cleaner!
thboop Feb 24, 2022
2288490
fix test
thboop Feb 24, 2022
8d77b8e
fix logic
thboop Feb 24, 2022
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
32 changes: 26 additions & 6 deletions src/Runner.Listener/Configuration/ServiceControlManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt

serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgName, settings.AgentName);

if (serviceName.Length > 80)
thboop marked this conversation as resolved.
Show resolved Hide resolved
if (serviceName.Length > MaxServiceNameLength)
{
Trace.Verbose($"Calculated service name is too long (> 80 chars). Trying again by calculating a shorter name.");
Trace.Verbose($"Calculated service name is too long (> {MaxServiceNameLength} chars). Trying again by calculating a shorter name.");

int exceededCharLength = serviceName.Length - 80;
string repoOrOrgNameSubstring = StringUtil.SubstringPrefix(repoOrOrgName, 45);
int exceededCharLength = serviceName.Length - MaxServiceNameLength;
string repoOrOrgNameSubstring = StringUtil.SubstringPrefix(repoOrOrgName, MaxRepoOrgCharacters);

exceededCharLength -= repoOrOrgName.Length - repoOrOrgNameSubstring.Length;

Expand All @@ -65,13 +65,33 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt
{
runnerNameSubstring = StringUtil.SubstringPrefix(settings.AgentName, settings.AgentName.Length - exceededCharLength);
}

serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgNameSubstring, runnerNameSubstring);
#pragma warning disable CS0162
if (AdditionalDigits > 0)
{
var random = new Random();
var num = random.Next((int)Math.Pow(10, AdditionalDigits-1),(int)Math.Pow(10, AdditionalDigits)).ToString();
thboop marked this conversation as resolved.
Show resolved Hide resolved
runnerNameSubstring +=$"-{num}";
serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgNameSubstring, runnerNameSubstring);
}
else
{
serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgNameSubstring, runnerNameSubstring);
}
#pragma warning restore CS0162
}

serviceDisplayName = StringUtil.Format(serviceDisplayNamePattern, repoOrOrgName, settings.AgentName);

Trace.Info($"Service name '{serviceName}' display name '{serviceDisplayName}' will be used for service configuration.");
}
#if (OS_LINUX || OS_OSX)
const int MaxServiceNameLength = 150;
const int MaxRepoOrgCharacters = 70;
const int AdditionalDigits = 4;
thboop marked this conversation as resolved.
Show resolved Hide resolved
#elif OS_WINDOWS
const int MaxServiceNameLength = 80;
const int MaxRepoOrgCharacters = 45;
const int AdditionalDigits = 0;
thboop marked this conversation as resolved.
Show resolved Hide resolved
#endif
}
}
188 changes: 115 additions & 73 deletions src/Test/L0/ServiceControlManagerL0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Runtime.CompilerServices;
using System.Text.RegularExpressions;
using GitHub.Runner.Common;
using GitHub.Runner.Listener.Configuration;
using Xunit;
Expand Down Expand Up @@ -86,84 +87,125 @@ public void CalculateServiceName80Chars()
}
}

[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Service")]
public void CalculateServiceNameLimitsServiceNameTo80Chars()
{
RunnerSettings settings = new RunnerSettings();

settings.AgentName = "thisisareallyreallylongbutstillvalidagentname";
settings.ServerUrl = "https://example.githubusercontent.com/12345678901234567890123456789012345678901234567890";
settings.GitHubUrl = "https://github.com/myreallylongorganizationexample/myreallylongrepoexample";

string serviceNamePattern = "actions.runner.{0}.{1}";
string serviceDisplayNamePattern = "GitHub Actions Runner ({0}.{1})";

using (TestHostContext hc = CreateTestContext())
#if OS_WINDOWS
[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Service")]
public void CalculateServiceNameLimitsServiceNameTo80Chars()
{
ServiceControlManager scm = new ServiceControlManager();

scm.Initialize(hc);
scm.CalculateServiceName(
settings,
serviceNamePattern,
serviceDisplayNamePattern,
out string serviceName,
out string serviceDisplayName);

// Verify name has been shortened to 80 characters
Assert.Equal(80, serviceName.Length);

var serviceNameParts = serviceName.Split('.');

// Verify that each component has been shortened to a sensible length
Assert.Equal("actions", serviceNameParts[0]); // Never shortened
Assert.Equal("runner", serviceNameParts[1]); // Never shortened
Assert.Equal("myreallylongorganizationexample-myreallylongr", serviceNameParts[2]); // First 45 chars, '/' has been replaced with '-'
Assert.Equal("thisisareallyreally", serviceNameParts[3]); // Remainder of unused chars
RunnerSettings settings = new RunnerSettings();

settings.AgentName = "thisisareallyreallylongbutstillvalidagentname";
settings.ServerUrl = "https://example.githubusercontent.com/12345678901234567890123456789012345678901234567890";
settings.GitHubUrl = "https://github.com/myreallylongorganizationexample/myreallylongrepoexample";

string serviceNamePattern = "actions.runner.{0}.{1}";
string serviceDisplayNamePattern = "GitHub Actions Runner ({0}.{1})";

using (TestHostContext hc = CreateTestContext())
{
ServiceControlManager scm = new ServiceControlManager();

scm.Initialize(hc);
scm.CalculateServiceName(
settings,
serviceNamePattern,
serviceDisplayNamePattern,
out string serviceName,
out string serviceDisplayName);

// Verify name has been shortened to 80 characters
Assert.Equal(80, serviceName.Length);

var serviceNameParts = serviceName.Split('.');

// Verify that each component has been shortened to a sensible length
Assert.Equal("actions", serviceNameParts[0]); // Never shortened
Assert.Equal("runner", serviceNameParts[1]); // Never shortened
Assert.Equal("myreallylongorganizationexample-myreallylongr", serviceNameParts[2]); // First 45 chars, '/' has been replaced with '-'
Assert.Equal("thisisareallyreally", serviceNameParts[3]); // Remainder of unused chars
}
}
}

// Special 'defensive' test that verifies we can gracefully handle creating service names
// in case GitHub.com changes its org/repo naming convention in the future,
// and some of these characters may be invalid for service names
// Not meant to test character set exhaustively -- it's just here to exercise the sanitizing logic
[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Service")]
public void CalculateServiceNameSanitizeOutOfRangeChars()
{
RunnerSettings settings = new RunnerSettings();

settings.AgentName = "name";
settings.ServerUrl = "https://example.githubusercontent.com/12345678901234567890123456789012345678901234567890";
settings.GitHubUrl = "https://github.com/org!@$*+[]()/repo!@$*+[]()";

string serviceNamePattern = "actions.runner.{0}.{1}";
string serviceDisplayNamePattern = "GitHub Actions Runner ({0}.{1})";

using (TestHostContext hc = CreateTestContext())
// Special 'defensive' test that verifies we can gracefully handle creating service names
// in case GitHub.com changes its org/repo naming convention in the future,
// and some of these characters may be invalid for service names
// Not meant to test character set exhaustively -- it's just here to exercise the sanitizing logic
[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Service")]
public void CalculateServiceNameSanitizeOutOfRangeChars()
{
ServiceControlManager scm = new ServiceControlManager();

scm.Initialize(hc);
scm.CalculateServiceName(
settings,
serviceNamePattern,
serviceDisplayNamePattern,
out string serviceName,
out string serviceDisplayName);

var serviceNameParts = serviceName.Split('.');

// Verify service name parts are sanitized correctly
Assert.Equal("actions", serviceNameParts[0]);
Assert.Equal("runner", serviceNameParts[1]);
Assert.Equal("org----------repo---------", serviceNameParts[2]); // Chars replaced with '-'
Assert.Equal("name", serviceNameParts[3]);
RunnerSettings settings = new RunnerSettings();

settings.AgentName = "name";
settings.ServerUrl = "https://example.githubusercontent.com/12345678901234567890123456789012345678901234567890";
settings.GitHubUrl = "https://github.com/org!@$*+[]()/repo!@$*+[]()";

string serviceNamePattern = "actions.runner.{0}.{1}";
string serviceDisplayNamePattern = "GitHub Actions Runner ({0}.{1})";

using (TestHostContext hc = CreateTestContext())
{
ServiceControlManager scm = new ServiceControlManager();

scm.Initialize(hc);
scm.CalculateServiceName(
settings,
serviceNamePattern,
serviceDisplayNamePattern,
out string serviceName,
out string serviceDisplayName);

var serviceNameParts = serviceName.Split('.');

// Verify service name parts are sanitized correctly
Assert.Equal("actions", serviceNameParts[0]);
Assert.Equal("runner", serviceNameParts[1]);
Assert.Equal("org----------repo---------", serviceNameParts[2]); // Chars replaced with '-'
Assert.Equal("name", serviceNameParts[3]);
}
}
}
#else
[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Service")]
public void CalculateServiceNameLimitsServiceNameTo150Chars()
{
RunnerSettings settings = new RunnerSettings();

settings.AgentName = "thisisareallyreallylongbutstillvalidagentnameiamusingforthisexampletotestverylongnamelimits";
settings.ServerUrl = "https://example.githubusercontent.com/12345678901234567890123456789012345678901234567890";
settings.GitHubUrl = "https://github.com/myreallylongorganizationexampleonlinux/myreallylongrepoexampleonlinux1234";

string serviceNamePattern = "actions.runner.{0}.{1}";
string serviceDisplayNamePattern = "GitHub Actions Runner ({0}.{1})";

using (TestHostContext hc = CreateTestContext())
{
ServiceControlManager scm = new ServiceControlManager();

scm.Initialize(hc);
scm.CalculateServiceName(
settings,
serviceNamePattern,
serviceDisplayNamePattern,
out string serviceName,
out string serviceDisplayName);

// Verify name has been shortened to 150 + 5 randomized characters
Assert.Equal(155, serviceName.Length);

var serviceNameParts = serviceName.Split('.');

// Verify that each component has been shortened to a sensible length
Assert.Equal("actions", serviceNameParts[0]); // Never shortened
Assert.Equal("runner", serviceNameParts[1]); // Never shortened
Assert.Equal("myreallylongorganizationexampleonlinux-myreallylongrepoexampleonlinux1", serviceNameParts[2]); // First 70 chars, '/' has been replaced with '-'
Assert.Matches(@"^(thisisareallyreallylongbutstillvalidagentnameiamusingforthisexam-[0-9]{4})$", serviceNameParts[3]);
}
}
#endif

private TestHostContext CreateTestContext([CallerMemberName] string testName = "")
{
Expand Down