From bda7355a81c063fcb34359c2bc686a2ad37a402c Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Wed, 23 Feb 2022 19:34:57 -0500 Subject: [PATCH 1/9] Lets allow up to 150 characters on linux/mac, just to avoid some issues with runner naming --- .../Configuration/ServiceControlManager.cs | 15 +- src/Test/L0/ServiceControlManagerL0.cs | 187 +++++++++++------- 2 files changed, 125 insertions(+), 77 deletions(-) diff --git a/src/Runner.Listener/Configuration/ServiceControlManager.cs b/src/Runner.Listener/Configuration/ServiceControlManager.cs index e0562ddfb48..532295a9564 100644 --- a/src/Runner.Listener/Configuration/ServiceControlManager.cs +++ b/src/Runner.Listener/Configuration/ServiceControlManager.cs @@ -49,12 +49,12 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt 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."); + 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; @@ -73,5 +73,12 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt 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 947e420b56e..f3f221850e0 100644 --- a/src/Test/L0/ServiceControlManagerL0.cs +++ b/src/Test/L0/ServiceControlManagerL0.cs @@ -86,84 +86,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 characters + 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.Equal("thisisareallyreallylongbutstillvalidagentnameiamusingforthisexam", serviceNameParts[3]); // Remainder of unused chars + } + } + #endif private TestHostContext CreateTestContext([CallerMemberName] string testName = "") { From e3ea8d93c98da6ce1dc8f0ef24db05d8f7950a18 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Thu, 24 Feb 2022 16:48:53 -0500 Subject: [PATCH 2/9] Add 4 randomized digits on mac/linux --- .../Configuration/ServiceControlManager.cs | 16 +++++++++++++++- src/Test/L0/ServiceControlManagerL0.cs | 8 +++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/Runner.Listener/Configuration/ServiceControlManager.cs b/src/Runner.Listener/Configuration/ServiceControlManager.cs index 532295a9564..44768c10a9c 100644 --- a/src/Runner.Listener/Configuration/ServiceControlManager.cs +++ b/src/Runner.Listener/Configuration/ServiceControlManager.cs @@ -66,7 +66,19 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt runnerNameSubstring = StringUtil.SubstringPrefix(settings.AgentName, settings.AgentName.Length - exceededCharLength); } - serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgNameSubstring, runnerNameSubstring); + if (AdditionalDigits > 0) + { + var random = new Random(); + var num = random.Next((int)Math.Pow(10, AdditionalDigits-1),(int)Math.Pow(10, AdditionalDigits)).ToString(); + runnerNameSubstring +=$"-{num}"; + serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgNameSubstring, runnerNameSubstring); + } + #pragma warning disable CS0162 + else + { + serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgNameSubstring, runnerNameSubstring); + } + #pragma warning restore CS0162 } serviceDisplayName = StringUtil.Format(serviceDisplayNamePattern, repoOrOrgName, settings.AgentName); @@ -76,9 +88,11 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt #if (OS_LINUX || OS_OSX) const int MaxServiceNameLength = 150; const int MaxRepoOrgCharacters = 70; + const int AdditionalDigits = 4; #elif OS_WINDOWS const int MaxServiceNameLength = 80; const int MaxRepoOrgCharacters = 45; + const int AdditionalDigits = 0; #endif } } diff --git a/src/Test/L0/ServiceControlManagerL0.cs b/src/Test/L0/ServiceControlManagerL0.cs index f3f221850e0..0dee8567cc2 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; @@ -192,8 +193,8 @@ public void CalculateServiceNameLimitsServiceNameTo150Chars() out string serviceName, out string serviceDisplayName); - // Verify name has been shortened to 150 characters - Assert.Equal(150, serviceName.Length); + // Verify name has been shortened to 150 + 5 randomized characters + Assert.Equal(155, serviceName.Length); var serviceNameParts = serviceName.Split('.'); @@ -201,7 +202,8 @@ public void CalculateServiceNameLimitsServiceNameTo150Chars() 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.Equal("thisisareallyreallylongbutstillvalidagentnameiamusingforthisexam", serviceNameParts[3]); // Remainder of unused chars + Regex regex = new Regex(@"^(thisisareallyreallylongbutstillvalidagentnameiamusingforthisexam[0-9]{4})$"); + Assert.Matches(@"^(thisisareallyreallylongbutstillvalidagentnameiamusingforthisexam[0-9]{4})$", serviceNameParts[3]); } } #endif From d94cc099c910a3a98adf414ff7d1b71c1647d0e2 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Thu, 24 Feb 2022 16:50:14 -0500 Subject: [PATCH 3/9] fix pragma issue --- src/Runner.Listener/Configuration/ServiceControlManager.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Runner.Listener/Configuration/ServiceControlManager.cs b/src/Runner.Listener/Configuration/ServiceControlManager.cs index 44768c10a9c..a1bc5a91b32 100644 --- a/src/Runner.Listener/Configuration/ServiceControlManager.cs +++ b/src/Runner.Listener/Configuration/ServiceControlManager.cs @@ -65,7 +65,7 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt { runnerNameSubstring = StringUtil.SubstringPrefix(settings.AgentName, settings.AgentName.Length - exceededCharLength); } - + #pragma warning disable CS0162 if (AdditionalDigits > 0) { var random = new Random(); @@ -73,7 +73,6 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt runnerNameSubstring +=$"-{num}"; serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgNameSubstring, runnerNameSubstring); } - #pragma warning disable CS0162 else { serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgNameSubstring, runnerNameSubstring); From 929b975930888fe75feec8eac07edcc07c587476 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Thu, 24 Feb 2022 16:55:55 -0500 Subject: [PATCH 4/9] fix test --- src/Test/L0/ServiceControlManagerL0.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Test/L0/ServiceControlManagerL0.cs b/src/Test/L0/ServiceControlManagerL0.cs index 0dee8567cc2..ac9542662b8 100644 --- a/src/Test/L0/ServiceControlManagerL0.cs +++ b/src/Test/L0/ServiceControlManagerL0.cs @@ -202,8 +202,7 @@ public void CalculateServiceNameLimitsServiceNameTo150Chars() 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 '-' - Regex regex = new Regex(@"^(thisisareallyreallylongbutstillvalidagentnameiamusingforthisexam[0-9]{4})$"); - Assert.Matches(@"^(thisisareallyreallylongbutstillvalidagentnameiamusingforthisexam[0-9]{4})$", serviceNameParts[3]); + Assert.Matches(@"^(thisisareallyreallylongbutstillvalidagentnameiamusingforthisexam-[0-9]{4})$", serviceNameParts[3]); } } #endif From 42aa2094155959c412c59f6bc065a2036176bccf Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Thu, 24 Feb 2022 17:13:17 -0500 Subject: [PATCH 5/9] Address pr feedback --- .../Configuration/ServiceControlManager.cs | 23 +++++++------------ src/Test/L0/ServiceControlManagerL0.cs | 2 +- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/src/Runner.Listener/Configuration/ServiceControlManager.cs b/src/Runner.Listener/Configuration/ServiceControlManager.cs index a1bc5a91b32..e9ada02613f 100644 --- a/src/Runner.Listener/Configuration/ServiceControlManager.cs +++ b/src/Runner.Listener/Configuration/ServiceControlManager.cs @@ -65,19 +65,12 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt { runnerNameSubstring = StringUtil.SubstringPrefix(settings.AgentName, settings.AgentName.Length - exceededCharLength); } - #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(); - runnerNameSubstring +=$"-{num}"; - serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgNameSubstring, runnerNameSubstring); - } - else - { - serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgNameSubstring, runnerNameSubstring); - } - #pragma warning restore CS0162 + + // 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((int)Math.Pow(10, AdditionalDigits-1),(int)Math.Pow(10, AdditionalDigits)).ToString(); + runnerNameSubstring +=$"-{num}"; + serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgNameSubstring, runnerNameSubstring); } serviceDisplayName = StringUtil.Format(serviceDisplayNamePattern, repoOrOrgName, settings.AgentName); @@ -89,9 +82,9 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt const int MaxRepoOrgCharacters = 70; const int AdditionalDigits = 4; #elif OS_WINDOWS - const int MaxServiceNameLength = 80; + const int MaxServiceNameLength = 75; const int MaxRepoOrgCharacters = 45; - const int AdditionalDigits = 0; + const int AdditionalDigits = 4; #endif } } diff --git a/src/Test/L0/ServiceControlManagerL0.cs b/src/Test/L0/ServiceControlManagerL0.cs index ac9542662b8..8e70f1e9bea 100644 --- a/src/Test/L0/ServiceControlManagerL0.cs +++ b/src/Test/L0/ServiceControlManagerL0.cs @@ -123,7 +123,7 @@ public void CalculateServiceNameLimitsServiceNameTo80Chars() 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 + Assert.Matches(@"^(thisisareallyr-[0-9]{4})$", serviceNameParts[3]); // Remainder of unused chars } } From ebd53b43ccf58e5af3af110872258e8e97370146 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Thu, 24 Feb 2022 17:16:33 -0500 Subject: [PATCH 6/9] reduce complexity --- src/Runner.Listener/Configuration/ServiceControlManager.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Runner.Listener/Configuration/ServiceControlManager.cs b/src/Runner.Listener/Configuration/ServiceControlManager.cs index e9ada02613f..91ebfff0b2b 100644 --- a/src/Runner.Listener/Configuration/ServiceControlManager.cs +++ b/src/Runner.Listener/Configuration/ServiceControlManager.cs @@ -68,7 +68,7 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt // 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((int)Math.Pow(10, AdditionalDigits-1),(int)Math.Pow(10, AdditionalDigits)).ToString(); + var num = random.Next(1000, 9999).ToString(); runnerNameSubstring +=$"-{num}"; serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgNameSubstring, runnerNameSubstring); } @@ -80,11 +80,9 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt #if (OS_LINUX || OS_OSX) const int MaxServiceNameLength = 150; const int MaxRepoOrgCharacters = 70; - const int AdditionalDigits = 4; #elif OS_WINDOWS const int MaxServiceNameLength = 75; const int MaxRepoOrgCharacters = 45; - const int AdditionalDigits = 4; #endif } } From d50eb2dc056e82cd44ab22b36effbe45bac7f6d0 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Thu, 24 Feb 2022 17:30:37 -0500 Subject: [PATCH 7/9] lets make it cleaner! --- .../Configuration/ServiceControlManager.cs | 7 +++---- src/Test/L0/ServiceControlManagerL0.cs | 10 +++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Runner.Listener/Configuration/ServiceControlManager.cs b/src/Runner.Listener/Configuration/ServiceControlManager.cs index 91ebfff0b2b..b96531ff900 100644 --- a/src/Runner.Listener/Configuration/ServiceControlManager.cs +++ b/src/Runner.Listener/Configuration/ServiceControlManager.cs @@ -48,12 +48,11 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt string repoOrOrgName = regex.Replace(settings.RepoOrOrgName, "-"); serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgName, settings.AgentName); - if (serviceName.Length > MaxServiceNameLength) { Trace.Verbose($"Calculated service name is too long (> {MaxServiceNameLength} chars). Trying again by calculating a shorter name."); - - int exceededCharLength = serviceName.Length - MaxServiceNameLength; + // Subtract 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; @@ -81,7 +80,7 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt const int MaxServiceNameLength = 150; const int MaxRepoOrgCharacters = 70; #elif OS_WINDOWS - const int MaxServiceNameLength = 75; + const int MaxServiceNameLength = 80; const int MaxRepoOrgCharacters = 45; #endif } diff --git a/src/Test/L0/ServiceControlManagerL0.cs b/src/Test/L0/ServiceControlManagerL0.cs index 8e70f1e9bea..147cabf9a94 100644 --- a/src/Test/L0/ServiceControlManagerL0.cs +++ b/src/Test/L0/ServiceControlManagerL0.cs @@ -16,7 +16,7 @@ public void CalculateServiceName() { RunnerSettings settings = new RunnerSettings(); - settings.AgentName = "thisiskindofalongrunnername1"; + settings.AgentName = "thisiskindofalongrunnerabcde"; settings.ServerUrl = "https://example.githubusercontent.com/12345678901234567890123456789012345678901234567890"; settings.GitHubUrl = "https://github.com/myorganizationexample/myrepoexample"; @@ -44,7 +44,7 @@ public void CalculateServiceName() 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]); } } @@ -55,7 +55,7 @@ public void CalculateServiceName80Chars() { RunnerSettings settings = new RunnerSettings(); - settings.AgentName = "thisiskindofalongrunnername12"; + settings.AgentName = "thisiskindofalongrunnernabcde"; settings.ServerUrl = "https://example.githubusercontent.com/12345678901234567890123456789012345678901234567890"; settings.GitHubUrl = "https://github.com/myorganizationexample/myrepoexample"; @@ -83,7 +83,7 @@ public void CalculateServiceName80Chars() 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 } } @@ -123,7 +123,7 @@ public void CalculateServiceNameLimitsServiceNameTo80Chars() 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 + Assert.Matches(@"^(thisisareallyr-[0-9]{4})$", serviceNameParts[3]); // Remainder of unused chars, 4 random numbers added at the end } } From 2288490227d08a4a0d6965cfed7d49c8c4a37191 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Thu, 24 Feb 2022 17:31:23 -0500 Subject: [PATCH 8/9] fix test --- src/Test/L0/ServiceControlManagerL0.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Test/L0/ServiceControlManagerL0.cs b/src/Test/L0/ServiceControlManagerL0.cs index 147cabf9a94..c7fe84cda87 100644 --- a/src/Test/L0/ServiceControlManagerL0.cs +++ b/src/Test/L0/ServiceControlManagerL0.cs @@ -193,8 +193,8 @@ public void CalculateServiceNameLimitsServiceNameTo150Chars() out string serviceName, out string serviceDisplayName); - // Verify name has been shortened to 150 + 5 randomized characters - Assert.Equal(155, serviceName.Length); + // Verify name has been shortened to 150 + Assert.Equal(150, serviceName.Length); var serviceNameParts = serviceName.Split('.'); @@ -202,7 +202,7 @@ public void CalculateServiceNameLimitsServiceNameTo150Chars() 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]); + Assert.Matches(@"^(thisisareallyreallylongbutstillvalidagentnameiamusingforthi-[0-9]{4})$", serviceNameParts[3]); } } #endif From 8d77b8ec0391c1f2ad5a9ee23ba033c3eeb5283b Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Thu, 24 Feb 2022 17:36:16 -0500 Subject: [PATCH 9/9] fix logic --- src/Runner.Listener/Configuration/ServiceControlManager.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Runner.Listener/Configuration/ServiceControlManager.cs b/src/Runner.Listener/Configuration/ServiceControlManager.cs index b96531ff900..9349484c5ee 100644 --- a/src/Runner.Listener/Configuration/ServiceControlManager.cs +++ b/src/Runner.Listener/Configuration/ServiceControlManager.cs @@ -51,8 +51,8 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt if (serviceName.Length > MaxServiceNameLength) { Trace.Verbose($"Calculated service name is too long (> {MaxServiceNameLength} chars). Trying again by calculating a shorter name."); - // Subtract 5 to add -xxxx random number on the end - int exceededCharLength = serviceName.Length - MaxServiceNameLength - 5; + // 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;