Implement new runner service name convention (#193)

* Limit service name to 80 characters
* Add L0 tests
* New service name convention
* Make RepoOrOrgName a computed property
* Add service name sanitizing logic with L0 test

src/Runner.Common/ConfigurationStore.cs

@@ -1,6 +1,8 @@
 using GitHub.Runner.Common.Util;
 using GitHub.Runner.Sdk;
+using System;
 using System.IO;
+using System.Linq;
 using System.Runtime.Serialization;
 using System.Text;
 using System.Threading;
@@ -51,6 +53,34 @@ public sealed class RunnerSettings
 
         [DataMember(EmitDefaultValue = false)]
         public string MonitorSocketAddress { get; set; }
+
+        /// <summary>
+        // Computed property for convenience. Can either return:
+        // 1. If runner was configured at the repo level, returns something like: "myorg/myrepo"
+        // 2. If runner was configured at the org level, returns something like: "myorg"
+        /// </summary>
+        public string RepoOrOrgName
+        {
+            get
+            {
+                Uri accountUri = new Uri(this.ServerUrl);
+                string repoOrOrgName = string.Empty;
+
+                if (accountUri.Host.EndsWith(".githubusercontent.com", StringComparison.OrdinalIgnoreCase))
+                {
+                    Uri gitHubUrl = new Uri(this.GitHubUrl);
+
+                    // Use the "NWO part" from the GitHub URL path
+                    repoOrOrgName = gitHubUrl.AbsolutePath.Trim('/');
+                }
+                else
+                {
+                    repoOrOrgName = accountUri.AbsolutePath.Split('/', StringSplitOptions.RemoveEmptyEntries).FirstOrDefault();
+                }
+
+                return repoOrOrgName;
+            }
+        }
     }
 
     [DataContract]

src/Runner.Listener/Configuration/OsxServiceControlManager.cs

@@ -12,8 +12,8 @@ namespace GitHub.Runner.Listener.Configuration
     public class OsxServiceControlManager : ServiceControlManager, ILinuxServiceControlManager
     {
         // This is the name you would see when you do `systemctl list-units | grep runner`
-        private const string _svcNamePattern = "actions.runner.{0}.{1}.{2}";
-        private const string _svcDisplayPattern = "GitHub Actions Runner ({0}.{1}.{2})";
+        private const string _svcNamePattern = "actions.runner.{0}.{1}";
+        private const string _svcDisplayPattern = "GitHub Actions Runner ({0}.{1})";
         private const string _shTemplate = "darwin.svc.sh.template";
         private const string _svcShName = "svc.sh";
 

src/Runner.Listener/Configuration/ServiceControlManager.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Linq;
+using System.Text.RegularExpressions;
 using GitHub.Runner.Common;
 using GitHub.Runner.Common.Util;
 using GitHub.Runner.Sdk;
@@ -37,25 +38,38 @@ public void CalculateServiceName(RunnerSettings settings, string serviceNamePatt
             serviceName = string.Empty;
             serviceDisplayName = string.Empty;
 
-            Uri accountUri = new Uri(settings.ServerUrl);
-            string accountName = string.Empty;
-
-            if (accountUri.Host.EndsWith(".githubusercontent.com", StringComparison.OrdinalIgnoreCase))
-            {
-                accountName = accountUri.AbsolutePath.Split('/', StringSplitOptions.RemoveEmptyEntries).FirstOrDefault();
-            }
-            else
+            if (string.IsNullOrEmpty(settings.RepoOrOrgName))
             {
-                accountName = accountUri.Host.Split('.').FirstOrDefault();
+                throw new InvalidOperationException($"Cannot find GitHub repository/organization name from server url: '{settings.ServerUrl}'");
             }
 
-            if (string.IsNullOrEmpty(accountName))
+            // For the service name, replace any characters outside of the alpha-numeric set and ".", "_", "-" with "-"
+            Regex regex = new Regex(@"[^0-9a-zA-Z._\-]");
+            string repoOrOrgName = regex.Replace(settings.RepoOrOrgName, "-");
+
+            serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgName, settings.AgentName);
+
+            if (serviceName.Length > 80)
             {
-                throw new InvalidOperationException($"Cannot find GitHub organization name from server url: '{settings.ServerUrl}'");
+                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);
+
+                exceededCharLength -= repoOrOrgName.Length - repoOrOrgNameSubstring.Length;
+
+                string runnerNameSubstring = settings.AgentName;
+
+                // Only trim runner name if it's really necessary
+                if (exceededCharLength > 0)
+                {
+                    runnerNameSubstring = StringUtil.SubstringPrefix(settings.AgentName, settings.AgentName.Length - exceededCharLength);
+                }
+
+                serviceName = StringUtil.Format(serviceNamePattern, repoOrOrgNameSubstring, runnerNameSubstring);
             }
 
-            serviceName = StringUtil.Format(serviceNamePattern, accountName, settings.PoolName, settings.AgentName);
-            serviceDisplayName = StringUtil.Format(serviceDisplayNamePattern, accountName, settings.PoolName, settings.AgentName);
+            serviceDisplayName = StringUtil.Format(serviceDisplayNamePattern, repoOrOrgName, settings.AgentName);
 
             Trace.Info($"Service name '{serviceName}' display name '{serviceDisplayName}' will be used for service configuration.");
         }

src/Runner.Listener/Configuration/SystemdControlManager.cs

@@ -13,8 +13,8 @@ namespace GitHub.Runner.Listener.Configuration
     public class SystemDControlManager : ServiceControlManager, ILinuxServiceControlManager
     {
         // This is the name you would see when you do `systemctl list-units | grep runner`
-        private const string _svcNamePattern = "actions.runner.{0}.{1}.{2}.service";
-        private const string _svcDisplayPattern = "GitHub Actions Runner ({0}.{1}.{2})";
+        private const string _svcNamePattern = "actions.runner.{0}.{1}.service";
+        private const string _svcDisplayPattern = "GitHub Actions Runner ({0}.{1})";
         private const string _shTemplate = "systemd.svc.sh.template";
         private const string _shName = "svc.sh";
 

src/Runner.Listener/Configuration/WindowsServiceControlManager.cs

@@ -15,8 +15,8 @@ public class WindowsServiceControlManager : ServiceControlManager, IWindowsServi
     {
         public const string WindowsServiceControllerName = "RunnerService.exe";
 
-        private const string ServiceNamePattern = "actionsrunner.{0}.{1}.{2}";
-        private const string ServiceDisplayNamePattern = "GitHub Actions Runner ({0}.{1}.{2})";
+        private const string ServiceNamePattern = "actions.runner.{0}.{1}";
+        private const string ServiceDisplayNamePattern = "GitHub Actions Runner ({0}.{1})";
 
         private INativeWindowsServiceHelper _windowsServiceHelper;
         private ITerminal _term;

src/Runner.Sdk/Util/StringUtil.cs

@@ -122,5 +122,10 @@ private static string Format(CultureInfo culture, string format, params object[]
                 return format;
             }
         }
+
+        public static string SubstringPrefix(string value, int count)
+        {
+            return value?.Substring(0, Math.Min(value.Length, count));
+        }
     }
 }

src/Test/L0/ServiceControlManagerL0.cs

@@ -0,0 +1,175 @@
+using System;
+using System.Runtime.CompilerServices;
+using GitHub.Runner.Common;
+using GitHub.Runner.Listener.Configuration;
+using Xunit;
+
+namespace GitHub.Runner.Common.Tests
+{
+    public sealed class ServiceControlManagerL0
+    {
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Service")]
+        public void CalculateServiceName()
+        {
+            RunnerSettings settings = new RunnerSettings();
+
+            settings.AgentName = "thisiskindofalongrunnername1";
+            settings.ServerUrl = "https://example.githubusercontent.com/12345678901234567890123456789012345678901234567890";
+            settings.GitHubUrl = "https://github.com/myorganizationexample/myrepoexample";
+
+            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 name is 79 characters
+                Assert.Equal(79, serviceName.Length);
+
+                // Verify nothing has been shortened out
+                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]);
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Service")]
+        public void CalculateServiceName80Chars()
+        {
+            RunnerSettings settings = new RunnerSettings();
+
+            settings.AgentName = "thisiskindofalongrunnername12";
+            settings.ServerUrl = "https://example.githubusercontent.com/12345678901234567890123456789012345678901234567890";
+            settings.GitHubUrl = "https://github.com/myorganizationexample/myrepoexample";
+
+            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 is still equal to 80 characters
+                Assert.Equal(80, serviceName.Length);
+
+                var serviceNameParts = serviceName.Split('.');
+
+                // Verify nothing has been shortened out
+                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]);
+            }
+        }
+
+        [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())
+            {
+                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())
+            {
+                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]);
+            }
+        }
+
+        private TestHostContext CreateTestContext([CallerMemberName] string testName = "")
+        {
+            TestHostContext hc = new TestHostContext(this, testName);
+
+            return hc;
+        }
+    }
+}