diff --git a/src/Runner.Listener/Configuration/ServiceControlManager.cs b/src/Runner.Listener/Configuration/ServiceControlManager.cs index e0562ddfb..9349484c5 100644 --- a/src/Runner.Listener/Configuration/ServiceControlManager.cs +++ b/src/Runner.Listener/Configuration/ServiceControlManager.cs @@ -48,13 +48,12 @@ namespace GitHub.Runner.Listener.Configuration string repoOrOrgName = regex.Replace(settings.RepoOrOrgName, "-"); serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgName, settings.AgentName); - - if (serviceName.Length > 80) + if (serviceName.Length > MaxServiceNameLength) { - Trace.Verbose($"Calculated service name is too long (> 80 chars). Trying again by calculating a shorter name."); - - int exceededCharLength = serviceName.Length - 80; - string repoOrOrgNameSubstring = StringUtil.SubstringPrefix(repoOrOrgName, 45); + Trace.Verbose($"Calculated service name is too long (> {MaxServiceNameLength} chars). Trying again by calculating a shorter name."); + // Add 5 to add -xxxx random number on the end + int exceededCharLength = serviceName.Length - MaxServiceNameLength + 5; + string repoOrOrgNameSubstring = StringUtil.SubstringPrefix(repoOrOrgName, MaxRepoOrgCharacters); exceededCharLength -= repoOrOrgName.Length - repoOrOrgNameSubstring.Length; @@ -66,6 +65,10 @@ namespace GitHub.Runner.Listener.Configuration runnerNameSubstring = StringUtil.SubstringPrefix(settings.AgentName, settings.AgentName.Length - exceededCharLength); } + // Lets add a suffix with a random number to reduce the chance of collisions between runner names once we truncate + var random = new Random(); + var num = random.Next(1000, 9999).ToString(); + runnerNameSubstring +=$"-{num}"; serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgNameSubstring, runnerNameSubstring); } @@ -73,5 +76,12 @@ namespace GitHub.Runner.Listener.Configuration 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; + #elif OS_WINDOWS + const int MaxServiceNameLength = 80; + const int MaxRepoOrgCharacters = 45; + #endif } } diff --git a/src/Test/L0/ServiceControlManagerL0.cs b/src/Test/L0/ServiceControlManagerL0.cs index 947e420b5..c7fe84cda 100644 --- a/src/Test/L0/ServiceControlManagerL0.cs +++ b/src/Test/L0/ServiceControlManagerL0.cs @@ -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; @@ -15,7 +16,7 @@ namespace GitHub.Runner.Common.Tests { RunnerSettings settings = new RunnerSettings(); - settings.AgentName = "thisiskindofalongrunnername1"; + settings.AgentName = "thisiskindofalongrunnerabcde"; settings.ServerUrl = "https://example.githubusercontent.com/12345678901234567890123456789012345678901234567890"; settings.GitHubUrl = "https://github.com/myorganizationexample/myrepoexample"; @@ -43,7 +44,7 @@ namespace GitHub.Runner.Common.Tests Assert.Equal("actions", serviceNameParts[0]); Assert.Equal("runner", serviceNameParts[1]); Assert.Equal("myorganizationexample-myrepoexample", serviceNameParts[2]); // '/' has been replaced with '-' - Assert.Equal("thisiskindofalongrunnername1", serviceNameParts[3]); + Assert.Equal("thisiskindofalongrunnerabcde", serviceNameParts[3]); } } @@ -54,7 +55,7 @@ namespace GitHub.Runner.Common.Tests { RunnerSettings settings = new RunnerSettings(); - settings.AgentName = "thisiskindofalongrunnername12"; + settings.AgentName = "thisiskindofalongrunnernabcde"; settings.ServerUrl = "https://example.githubusercontent.com/12345678901234567890123456789012345678901234567890"; settings.GitHubUrl = "https://github.com/myorganizationexample/myrepoexample"; @@ -82,88 +83,129 @@ namespace GitHub.Runner.Common.Tests Assert.Equal("actions", serviceNameParts[0]); Assert.Equal("runner", serviceNameParts[1]); Assert.Equal("myorganizationexample-myrepoexample", serviceNameParts[2]); // '/' has been replaced with '-' - Assert.Equal("thisiskindofalongrunnername12", serviceNameParts[3]); + Assert.Equal("thisiskindofalongrunnernabcde", serviceNameParts[3]); // should not have random numbers unless we exceed 80 } } - [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(); + RunnerSettings settings = new RunnerSettings(); - scm.Initialize(hc); - scm.CalculateServiceName( - settings, - serviceNamePattern, - serviceDisplayNamePattern, - out string serviceName, - out string serviceDisplayName); + settings.AgentName = "thisisareallyreallylongbutstillvalidagentname"; + settings.ServerUrl = "https://example.githubusercontent.com/12345678901234567890123456789012345678901234567890"; + settings.GitHubUrl = "https://github.com/myreallylongorganizationexample/myreallylongrepoexample"; - // Verify name has been shortened to 80 characters - Assert.Equal(80, serviceName.Length); + string serviceNamePattern = "actions.runner.{0}.{1}"; + string serviceDisplayNamePattern = "GitHub Actions Runner ({0}.{1})"; - var serviceNameParts = serviceName.Split('.'); + using (TestHostContext hc = CreateTestContext()) + { + ServiceControlManager scm = new ServiceControlManager(); - // 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 + 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.Matches(@"^(thisisareallyr-[0-9]{4})$", serviceNameParts[3]); // Remainder of unused chars, 4 random numbers added at the end + } } - } - // 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(); + RunnerSettings settings = new RunnerSettings(); - scm.Initialize(hc); - scm.CalculateServiceName( - settings, - serviceNamePattern, - serviceDisplayNamePattern, - out string serviceName, - out string serviceDisplayName); + settings.AgentName = "name"; + settings.ServerUrl = "https://example.githubusercontent.com/12345678901234567890123456789012345678901234567890"; + settings.GitHubUrl = "https://github.com/org!@$*+[]()/repo!@$*+[]()"; - var serviceNameParts = serviceName.Split('.'); + string serviceNamePattern = "actions.runner.{0}.{1}"; + string serviceDisplayNamePattern = "GitHub Actions Runner ({0}.{1})"; - // 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]); + 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 + Assert.Equal(150, 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(@"^(thisisareallyreallylongbutstillvalidagentnameiamusingforthi-[0-9]{4})$", serviceNameParts[3]); + } + } + #endif private TestHostContext CreateTestContext([CallerMemberName] string testName = "") {