Tye should validate that service names are valid DNS names (#480)

.gitignore

@@ -1,7 +1,7 @@
 .dotnet/
 artifacts/
 .tye/
-
+**/.vscode/
 
 ## Ignore Visual Studio temporary files, build results, and
 ## files generated by popular Visual Studio add-ons.

docs/reference/schema.md

@@ -133,7 +133,12 @@ services:
 
 #### `name` (string) *required*
 
-The service name. Each service must have a name, and it must be a legal DNS name: (`a-z` + `_`).
+The service name. Each service must have a name, and it must be a legal DNS name: (`a-z` + `-`). Specifically, the service name must:
+
+- Contain at most 63 characters
+- Contain only alphanumeric characters or ‘-’
+- Start with an alphanumeric character
+- End with an alphanumeric character
 
 #### `project` (string)
 

samples/apps-with-ingress/tye.yaml

@@ -17,8 +17,7 @@ ingress:
       - host: a.example.com
         service: app-a
       - host: b.example.com
-        service: app-b 
-
+        service: app-b
 services:
 - name: app-a
   project: ApplicationA/ApplicationA.csproj

src/Microsoft.Tye.Core/ConfigModel/ConfigFactory.cs

@@ -5,6 +5,7 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Text.RegularExpressions;
 using Microsoft.Tye.Serialization;
 using Tye.Serialization;
 
@@ -42,7 +43,7 @@ namespace Microsoft.Tye.ConfigModel
 
             var service = new ConfigService()
             {
-                Name = Path.GetFileNameWithoutExtension(file.Name).ToLowerInvariant(),
+                Name = NormalizeServiceName(Path.GetFileNameWithoutExtension(file.Name)),
                 Project = file.FullName.Replace('\\', '/'),
             };
 
@@ -73,7 +74,7 @@ namespace Microsoft.Tye.ConfigModel
                 {
                     var service = new ConfigService()
                     {
-                        Name = Path.GetFileNameWithoutExtension(projectFile.Name).ToLowerInvariant(),
+                        Name = NormalizeServiceName(Path.GetFileNameWithoutExtension(projectFile.Name)),
                         Project = projectFile.FullName.Replace('\\', '/'),
                     };
 
@@ -97,5 +98,8 @@ namespace Microsoft.Tye.ConfigModel
             using var parser = new YamlParser(file);
             return parser.ParseConfigApplication();
         }
+
+        private static string NormalizeServiceName(string name)
+            => Regex.Replace(name.ToLowerInvariant(), "[^0-9A-Za-z-]+", "-");
     }
 }

src/Microsoft.Tye.Core/ConfigModel/ConfigService.cs

@@ -4,13 +4,21 @@
 
 using System.Collections.Generic;
 using System.ComponentModel.DataAnnotations;
+using Tye;
 using YamlDotNet.Serialization;
 
 namespace Microsoft.Tye.ConfigModel
 {
     public class ConfigService
     {
+        const string ErrorMessage = "A service name must consist of lower case alphanumeric characters or '-'," +
+            " start with an alphabetic character, and end with an alphanumeric character" +
+            " (e.g. 'my-name',  or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?').";
+        const string MaxLengthErrorMessage = "Name cannot be more that 63 characters long.";
+
         [Required]
+        [RegularExpression("[a-z]([-a-z0-9]*[a-z0-9])?", ErrorMessage = ErrorMessage)]
+        [MaxLength(63, ErrorMessage = MaxLengthErrorMessage)]
         public string Name { get; set; } = default!;
         public bool External { get; set; }
         public string? Image { get; set; }

src/Microsoft.Tye.Core/Serialization/ConfigIngressParser.cs

@@ -30,7 +30,7 @@ namespace Tye.Serialization
                 switch (key)
                 {
                     case "name":
-                        configIngress.Name = YamlParser.GetScalarValue(key, child.Value);
+                        configIngress.Name = YamlParser.GetScalarValue(key, child.Value).ToLowerInvariant();
                         break;
                     case "replicas":
                         if (!int.TryParse(YamlParser.GetScalarValue(key, child.Value), out var replicas))
@@ -91,7 +91,7 @@ namespace Tye.Serialization
                         rule.Path = YamlParser.GetScalarValue(key, child.Value);
                         break;
                     case "service":
-                        rule.Service = YamlParser.GetScalarValue(key, child.Value);
+                        rule.Service = YamlParser.GetScalarValue(key, child.Value).ToLowerInvariant();
                         break;
                     default:
                         throw new TyeYamlException(child.Key.Start, CoreStrings.FormatUnrecognizedKey(key));

src/Microsoft.Tye.Core/Serialization/ConfigServiceParser.cs

@@ -30,7 +30,7 @@ namespace Tye.Serialization
                 switch (key)
                 {
                     case "name":
-                        service.Name = YamlParser.GetScalarValue(key, child.Value);
+                        service.Name = YamlParser.GetScalarValue(key, child.Value).ToLowerInvariant();
                         break;
                     case "external":
                         if (!bool.TryParse(YamlParser.GetScalarValue(key, child.Value), out var external))

test/E2ETest/TyeGenerateTests.cs

@@ -183,7 +183,7 @@ namespace E2ETest
         public async Task Generate_DaprApplication()
         {
             var applicationName = "dapr_test_application";
-            var projectName = "dapr_test_project";
+            var projectName = "dapr-test-project";
             var environment = "production";
 
             await DockerAssert.DeleteDockerImagesAsync(output, projectName);
@@ -363,8 +363,8 @@ namespace E2ETest
             var applicationName = "apps-with-ingress";
             var environment = "production";
 
-            await DockerAssert.DeleteDockerImagesAsync(output, "app-a");
-            await DockerAssert.DeleteDockerImagesAsync(output, "app-b");
+            await DockerAssert.DeleteDockerImagesAsync(output, "appa");
+            await DockerAssert.DeleteDockerImagesAsync(output, "appa");
 
             using var projectDirectory = TestHelpers.CopyTestProjectDirectory(applicationName);
 
@@ -383,13 +383,13 @@ namespace E2ETest
 
                 YamlAssert.Equals(expectedContent, content, output);
 
-                await DockerAssert.AssertImageExistsAsync(output, "app-a");
-                await DockerAssert.AssertImageExistsAsync(output, "app-b");
+                await DockerAssert.AssertImageExistsAsync(output, "appa");
+                await DockerAssert.AssertImageExistsAsync(output, "appb");
             }
             finally
             {
-                await DockerAssert.DeleteDockerImagesAsync(output, "app-a");
-                await DockerAssert.DeleteDockerImagesAsync(output, "app-b");
+                await DockerAssert.DeleteDockerImagesAsync(output, "appa");
+                await DockerAssert.DeleteDockerImagesAsync(output, "appb");
             }
         }
     }

test/E2ETest/TyeInitTests.cs

@@ -74,5 +74,20 @@ namespace E2ETest
 
             YamlAssert.Equals(expectedContent, content);
         }
+
+
+        [Fact]
+        public void Console_Normalization_Service_Name()
+        {
+            using var projectDirectory = CopyTestProjectDirectory("Console.Normalization.svc.Name");
+
+            var projectFile = new FileInfo(Path.Combine(projectDirectory.DirectoryPath, "Console.Normalization.svc.Name.csproj"));
+
+            var (content, _) = InitHost.CreateTyeFileContent(projectFile, force: false);
+
+            var expectedContent = File.ReadAllText("testassets/init/console-normalization-svc-name.yaml");
+
+            YamlAssert.Equals(expectedContent, content);
+        }
     }
 }

test/E2ETest/TyeRunTests.cs

@@ -489,8 +489,8 @@ namespace E2ETest
             await RunHostingApplication(application, new HostOptions(), async (app, uri) =>
             {
                 var ingressUri = await GetServiceUrl(client, uri, "ingress");
-                var appAUri = await GetServiceUrl(client, uri, "app-a");
-                var appBUri = await GetServiceUrl(client, uri, "app-b");
+                var appAUri = await GetServiceUrl(client, uri, "appa");
+                var appBUri = await GetServiceUrl(client, uri, "appb");
 
                 var appAResponse = await client.GetAsync(appAUri);
                 var appBResponse = await client.GetAsync(appBUri);
@@ -538,8 +538,8 @@ namespace E2ETest
             await RunHostingApplication(application, new HostOptions(), async (app, uri) =>
             {
                 var nginxUri = await GetServiceUrl(client, uri, "nginx");
-                var appAUri = await GetServiceUrl(client, uri, "appA");
-                var appBUri = await GetServiceUrl(client, uri, "appB");
+                var appAUri = await GetServiceUrl(client, uri, "appa");
+                var appBUri = await GetServiceUrl(client, uri, "appb");
 
                 var nginxResponse = await client.GetAsync(nginxUri);
                 var appAResponse = await client.GetAsync(appAUri);

test/E2ETest/testassets/generate/apps-with-ingress.yaml

@@ -1,36 +1,36 @@
 kind: Deployment
 apiVersion: apps/v1
 metadata:
-  name: app-a
+  name: appa
   labels:
-    app.kubernetes.io/name: 'app-a'
+    app.kubernetes.io/name: 'appa'
     app.kubernetes.io/part-of: 'apps-with-ingress'
 spec:
   replicas: 2
   selector:
     matchLabels:
-      app.kubernetes.io/name: app-a
+      app.kubernetes.io/name: appa
   template:
     metadata:
       labels:
-        app.kubernetes.io/name: 'app-a'
+        app.kubernetes.io/name: 'appa'
         app.kubernetes.io/part-of: 'apps-with-ingress'
     spec:
       containers:
-      - name: app-a
-        image: app-a:1.0.0
+      - name: appa
+        image: appa:1.0.0
         imagePullPolicy: Always
         env:
         - name: ASPNETCORE_URLS
           value: 'http://*'
         - name: PORT
           value: '80'
-        - name: SERVICE__APP-B__PROTOCOL
+        - name: SERVICE__APPB__PROTOCOL
           value: 'http'
-        - name: SERVICE__APP-B__PORT
+        - name: SERVICE__APPB__PORT
           value: '80'
-        - name: SERVICE__APP-B__HOST
-          value: 'app-b'
+        - name: SERVICE__APPB__HOST
+          value: 'appb'
         ports:
         - containerPort: 80
 ...
@@ -38,13 +38,13 @@ spec:
 kind: Service
 apiVersion: v1
 metadata:
-  name: app-a
+  name: appa
   labels:
-    app.kubernetes.io/name: 'app-a'
+    app.kubernetes.io/name: 'appa'
     app.kubernetes.io/part-of: 'apps-with-ingress'
 spec:
   selector:
-    app.kubernetes.io/name: app-a
+    app.kubernetes.io/name: appa
   type: ClusterIP
   ports:
   - name: http
@@ -56,36 +56,36 @@ spec:
 kind: Deployment
 apiVersion: apps/v1
 metadata:
-  name: app-b
+  name: appb
   labels:
-    app.kubernetes.io/name: 'app-b'
+    app.kubernetes.io/name: 'appb'
     app.kubernetes.io/part-of: 'apps-with-ingress'
 spec:
   replicas: 2
   selector:
     matchLabels:
-      app.kubernetes.io/name: app-b
+      app.kubernetes.io/name: appb
   template:
     metadata:
       labels:
-        app.kubernetes.io/name: 'app-b'
+        app.kubernetes.io/name: 'appb'
         app.kubernetes.io/part-of: 'apps-with-ingress'
     spec:
       containers:
-      - name: app-b
-        image: app-b:1.0.0
+      - name: appb
+        image: appb:1.0.0
         imagePullPolicy: Always
         env:
         - name: ASPNETCORE_URLS
           value: 'http://*'
         - name: PORT
           value: '80'
-        - name: SERVICE__APP-A__PROTOCOL
+        - name: SERVICE__APPA__PROTOCOL
           value: 'http'
-        - name: SERVICE__APP-A__PORT
+        - name: SERVICE__APPA__PORT
           value: '80'
-        - name: SERVICE__APP-A__HOST
-          value: 'app-a'
+        - name: SERVICE__APPA__HOST
+          value: 'appa'
         ports:
         - containerPort: 80
 ...
@@ -93,13 +93,13 @@ spec:
 kind: Service
 apiVersion: v1
 metadata:
-  name: app-b
+  name: appb
   labels:
-    app.kubernetes.io/name: 'app-b'
+    app.kubernetes.io/name: 'appb'
     app.kubernetes.io/part-of: 'apps-with-ingress'
 spec:
   selector:
-    app.kubernetes.io/name: app-b
+    app.kubernetes.io/name: appb
   type: ClusterIP
   ports:
   - name: http
@@ -122,25 +122,25 @@ spec:
   - http:
       paths:
       - backend:
-          serviceName: app-a
+          serviceName: appa
           servicePort: 80
         path: /A(/|$)(.*)
       - backend:
-          serviceName: app-b
+          serviceName: appb
           servicePort: 80
         path: /B(/|$)(.*)
   - host: a.example.com
     http:
       paths:
       - backend:
-          serviceName: app-a
+          serviceName: appa
           servicePort: 80
         path: /()(.*)
   - host: b.example.com
     http:
       paths:
       - backend:
-          serviceName: app-b
+          serviceName: appb
           servicePort: 80
         path: /()(.*)
 ...

test/E2ETest/testassets/generate/dapr.yaml

@@ -1,36 +1,36 @@
 kind: Deployment
 apiVersion: apps/v1
 metadata:
-  name: dapr_test_project
+  name: dapr-test-project
   annotations:
     dapr.io/enabled: 'true'
-    dapr.io/id: 'dapr_test_project'
+    dapr.io/id: 'dapr-test-project'
     dapr.io/port: '80'
     dapr.io/config: 'tracing'
     dapr.io/log-level: 'debug'
   labels:
-    app.kubernetes.io/name: 'dapr_test_project'
+    app.kubernetes.io/name: 'dapr-test-project'
     app.kubernetes.io/part-of: 'dapr_test_application'
 spec:
   replicas: 1
   selector:
     matchLabels:
-      app.kubernetes.io/name: dapr_test_project
+      app.kubernetes.io/name: dapr-test-project
   template:
     metadata:
       annotations:
         dapr.io/enabled: 'true'
-        dapr.io/id: 'dapr_test_project'
+        dapr.io/id: 'dapr-test-project'
         dapr.io/port: '80'
         dapr.io/config: 'tracing'
         dapr.io/log-level: 'debug'
       labels:
-        app.kubernetes.io/name: 'dapr_test_project'
+        app.kubernetes.io/name: 'dapr-test-project'
         app.kubernetes.io/part-of: 'dapr_test_application'
     spec:
       containers:
-      - name: dapr_test_project
-        image: dapr_test_project:1.0.0
+      - name: dapr-test-project
+        image: dapr-test-project:1.0.0
         imagePullPolicy: Always
         env:
         - name: ASPNETCORE_URLS
@@ -44,13 +44,13 @@ spec:
 kind: Service
 apiVersion: v1
 metadata:
-  name: dapr_test_project
+  name: dapr-test-project
   labels:
-    app.kubernetes.io/name: 'dapr_test_project'
+    app.kubernetes.io/name: 'dapr-test-project'
     app.kubernetes.io/part-of: 'dapr_test_application'
 spec:
   selector:
-    app.kubernetes.io/name: dapr_test_project
+    app.kubernetes.io/name: dapr-test-project
   type: ClusterIP
   ports:
   - name: http

test/E2ETest/testassets/init/console-normalization-svc-name.yaml

@@ -0,0 +1,10 @@
+# tye application configuration file
+# read all about it at https://github.com/dotnet/tye
+#
+# when you've given us a try, we'd love to know what you think:
+#    https://aka.ms/AA7q20u
+#
+name: console.normalization.svc.name
+services:
+- name: console-normalization-svc-name
+  project: Console.Normalization.svc.Name.csproj

test/E2ETest/testassets/projects/Console.Normalization.svc.Name/Console.Normalization.svc.Name.csproj

@@ -0,0 +1,8 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <TargetFramework>netcoreapp3.1</TargetFramework>
+  </PropertyGroup>
+
+</Project>

test/E2ETest/testassets/projects/Console.Normalization.svc.Name/Program.cs

@@ -0,0 +1,12 @@
+using System;
+
+namespace Console.Normalization.svc.Name
+{
+    class Program
+    {
+        static void Main(string[] args)
+        {
+            Console.WriteLine("Hello World!");
+        }
+    }
+}

test/E2ETest/testassets/projects/apps-with-ingress/tye.yaml

@@ -11,18 +11,18 @@ ingress:
       - port: 8080
     rules:
       - path: /A
-        service: app-a
+        service: appA
       - path: /B
-        service: app-b
+        service: appB
       - host: a.example.com
-        service: app-a
+        service: appA
       - host: b.example.com
-        service: app-b 
+        service: appB 
 
 services:
-- name: app-a
+- name: appA
   project: ApplicationA/ApplicationA.csproj
   replicas: 2
-- name: app-b
+- name: appB
   project: ApplicationB/ApplicationB.csproj
   replicas: 2

test/E2ETest/testassets/projects/dapr/tye.yaml

@@ -4,5 +4,5 @@ extensions:
   config: tracing
   log-level: debug
 services:
-- name: dapr_test_project
+- name: dapr-test-project
   project: dapr.csproj

test/Test.Infrastructure/TyeAssert.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System;
 using System.Linq;
 using Microsoft.Tye.ConfigModel;
 using Xunit;
@@ -29,7 +30,7 @@ namespace Test.Infrastructure
                 {
                     var otherRule = otherIngress
                         .Rules
-                        .Where(o => o.Path == rule.Path && o.Host == rule.Host && o.Service == rule.Service)
+                        .Where(o => o.Path == rule.Path && o.Host == rule.Host && o.Service?.Equals(rule.Service, StringComparison.OrdinalIgnoreCase) == true)
                         .Single();
                     Assert.NotNull(otherRule);
                 }
@@ -49,7 +50,7 @@ namespace Test.Infrastructure
             {
                 var otherService = expected
                     .Services
-                    .Where(o => o.Name == service.Name)
+                    .Where(o => o.Name.Equals(service.Name, StringComparison.OrdinalIgnoreCase))
                     .Single();
                 Assert.NotNull(otherService);
                 Assert.Equal(otherService.Args, service.Args);

test/UnitTests/TyeDeserializationValidationTests.cs

@@ -201,10 +201,45 @@ services:
       - port: 8080
         protocol: http";
 
+            using var parser = new YamlParser(input);
+            var app = parser.ParseConfigApplication();
+            Assert.Throws<TyeYamlException>(() => app.Validate());
+        }
+
+        [Fact]
+        public void ValidateServiceNameThrowsException()
+        {
+            var input = @"
+services:
+  - name: app_
+    bindings:
+      - protocol: http
+        name: a
+      - protocol: https
+        name: b";
+            var errorMessage = "A service name must consist of lower case alphanumeric";
             using var parser = new YamlParser(input);
             var app = parser.ParseConfigApplication();
             var exception = Assert.Throws<TyeYamlException>(() => app.Validate());
-            Assert.Contains(CoreStrings.FormatProjectImageExecutableExclusive(a, b), exception.Message);
+            Assert.Contains(errorMessage, exception.Message);
+        }
+
+        [Fact]
+        public void ValidateServiceNameThrowsExceptionForMaxLength()
+        {
+            var input = @"
+services:
+  - name: appavalidateservicenamethrowsexceptionformaxlengthvalidateservicen
+    bindings:
+      - protocol: http
+        name: a
+      - protocol: https
+        name: b";
+            var errorMessage = "Name cannot be more that 63 characters long.";
+            using var parser = new YamlParser(input);
+            var app = parser.ParseConfigApplication();
+            var exception = Assert.Throws<TyeYamlException>(() => app.Validate());
+            Assert.Contains(errorMessage, exception.Message);
         }
     }
 }