From d69009b73074973484b1602011dbb0c730f013bf Mon Sep 17 00:00:00 2001
From: Ryan Nowak <nowakra@gmail.com>
Date: Wed, 11 Mar 2020 18:48:09 -0700
Subject: [PATCH] Auto-assign ports when inferred from launch settings

This tries to add some auto-detect magic to make common scenarios work
without any extra configuration, which avoiding doing obtuse stuff.

- We ignore the ports in launch settings (avoid conflict in the
  hello world multi-project case)
- When we infer bindings based on launch settings we set them to
  auto-select a port
- The user setting a port via tye.yaml will always win
- Bindings that are not inferred from launch settings still work
  the same way (we don't auto-auto-select a port)
- When we deploy we ignore https

The HTTPS behavior might need discussion, but we haven't started looking
at a recipe for HTTPs inside the cluster in k8s, and it's not clear
whether we even want to.

One could also make the argument that we should skip TLS locally because
we're not doing it inside the cluster. Someone writing an app today that
assumes TLS in their code (using https URLs, or gRPC + TLS) will have
a non-trivial path to k8s for now anyway. So, skipping https bindings
for k8s really just makes us more honest.
---
 samples/frontend-backend/tye.yaml             |  4 --
 src/Tye.Core/CombineStep.cs                   | 19 +++++++
 src/Tye.Core/DockerContainerBuilder.cs        |  2 +-
 src/Tye.Core/KubernetesManifestGenerator.cs   | 51 +++++++++++--------
 src/Tye.Hosting/ProxyService.cs               |  6 +--
 src/Tye.Hosting/TyeHost.cs                    |  2 +
 src/tye/GenerateHost.cs                       |  2 +
 src/tye/Program.DeployCommand.cs              | 22 ++++++--
 test/E2ETest/TyeRunTests.cs                   | 33 +++++-------
 .../testassets/generate/frontend-backend.yaml | 24 +++++----
 .../testassets/generate/multi-project.yaml    | 12 ++++-
 .../generate/single-project-noregistry.yaml   | 13 ++---
 .../testassets/generate/single-project.yaml   | 13 ++---
 13 files changed, 121 insertions(+), 82 deletions(-)

diff --git a/samples/frontend-backend/tye.yaml b/samples/frontend-backend/tye.yaml
index 74569f0a..8868a09a 100644
--- a/samples/frontend-backend/tye.yaml
+++ b/samples/frontend-backend/tye.yaml
@@ -4,9 +4,5 @@ name: frontend-backend
 services:
 - name: backend
   project: backend/backend.csproj
-  bindings:
-  - port: 5050
 - name: frontend
   project: frontend/frontend.csproj
-  bindings:
-  - port: 5051
diff --git a/src/Tye.Core/CombineStep.cs b/src/Tye.Core/CombineStep.cs
index c482c3fc..88849358 100644
--- a/src/Tye.Core/CombineStep.cs
+++ b/src/Tye.Core/CombineStep.cs
@@ -20,6 +20,18 @@ namespace Tye
                 return Task.CompletedTask;
             }
 
+            // Compute ASPNETCORE_URLS based on the bindings exposed by *this* project.
+            foreach (var binding in service.Service.Bindings)
+            {
+                if (binding.Protocol == "http" || binding.Protocol == null)
+                {
+                    var port = binding.Port ?? 80;
+                    var urls = $"http://*{(port == 80 ? "" : (":" + port.ToString()))}";
+                    service.Service.Environment.Add("ASPNETCORE_URLS", urls);
+                    break;
+                }
+            }
+
             // Process bindings and turn them into environment variables and secrets. There's
             // some duplication with the code in m8s (Application.cs) for populating environments.
             //
@@ -47,6 +59,13 @@ namespace Tye
                             bindings.Bindings.Add(new EnvironmentVariableInputBinding($"CONNECTIONSTRING__{configName}", binding.ConnectionString));
                         }
 
+                        if (binding.Protocol == "https")
+                        {
+                            // We skip https for now in deployment, because the E2E requires certificates
+                            // and we haven't done those features yet.
+                            continue;
+                        }
+
                         if (!string.IsNullOrEmpty(binding.Protocol))
                         {
                             bindings.Bindings.Add(new EnvironmentVariableInputBinding($"SERVICE__{configName}__PROTOCOL", binding.Protocol));
diff --git a/src/Tye.Core/DockerContainerBuilder.cs b/src/Tye.Core/DockerContainerBuilder.cs
index c26a23df..d4bf23b1 100644
--- a/src/Tye.Core/DockerContainerBuilder.cs
+++ b/src/Tye.Core/DockerContainerBuilder.cs
@@ -86,7 +86,7 @@ namespace Tye
                 throw new CommandException("'docker build' failed.");
             }
 
-            output.WriteInfoLine($"Created Docker Image: {container.ImageName}:{container.ImageTag}");
+            output.WriteInfoLine($"Created Docker Image: '{container.ImageName}:{container.ImageTag}'");
             service.Outputs.Add(new DockerImageOutput(container.ImageName!, container.ImageTag!));
         }
     }
diff --git a/src/Tye.Core/KubernetesManifestGenerator.cs b/src/Tye.Core/KubernetesManifestGenerator.cs
index 262f1f8c..d73fcc9b 100644
--- a/src/Tye.Core/KubernetesManifestGenerator.cs
+++ b/src/Tye.Core/KubernetesManifestGenerator.cs
@@ -61,13 +61,23 @@ namespace Tye
             // We figure out the port based on bindings
             foreach (var binding in service.Service.Bindings)
             {
-                var port = new YamlMappingNode();
-                ports.Add(port);
+                if (binding.Protocol == "https")
+                {
+                    // We skip https for now in deployment, because the E2E requires certificates
+                    // and we haven't done those features yet.
+                    continue;
+                }
 
-                port.Add("name", binding.Name ?? "web");
-                port.Add("protocol", "TCP"); // we use assume TCP. YOLO
-                port.Add("port", binding.Port?.ToString() ?? "80");
-                port.Add("targetPort", binding.Port?.ToString() ?? "80");
+                if (binding.Port != null)
+                {
+                    var port = new YamlMappingNode();
+                    ports.Add(port);
+
+                    port.Add("name", binding.Name ?? binding.Protocol ?? "http");
+                    port.Add("protocol", "TCP"); // we use assume TCP. YOLO
+                    port.Add("port", binding.Port.Value.ToString());
+                    port.Add("targetPort", binding.Port.Value.ToString());
+                }
             }
 
             return new KubernetesServiceOutput(service.Service.Name, new YamlDocument(root));
@@ -170,19 +180,6 @@ namespace Tye
                         });
                     }
 
-                    foreach (var binding in service.Service.Bindings)
-                    {
-                        if (binding.Protocol == "http" || binding.Protocol == null)
-                        {
-                            var port = binding.Port ?? 80;
-                            env.Add(new YamlMappingNode()
-                            {
-                                { "name", "ASPNETCORE_URLS" },
-                                { "value", $"http://*{(binding.Port == 80 ? "" : (":" + binding.Port.ToString()))}" },
-                            });
-                        }
-                    }
-
                     if (bindings is object)
                     {
                         foreach (var binding in bindings.Bindings.OfType<EnvironmentVariableInputBinding>())
@@ -219,9 +216,19 @@ namespace Tye
 
                     foreach (var binding in service.Service.Bindings)
                     {
-                        var containerPort = new YamlMappingNode();
-                        ports.Add(containerPort);
-                        containerPort.Add("containerPort", binding.Port?.ToString() ?? "80");
+                        if (binding.Protocol == "https")
+                        {
+                            // We skip https for now in deployment, because the E2E requires certificates
+                            // and we haven't done those features yet.
+                            continue;
+                        }
+
+                        if (binding.Port != null)
+                        {
+                            var containerPort = new YamlMappingNode();
+                            ports.Add(containerPort);
+                            containerPort.Add("containerPort", binding.Port.Value.ToString());
+                        }
                     }
                 }
             }
diff --git a/src/Tye.Hosting/ProxyService.cs b/src/Tye.Hosting/ProxyService.cs
index e2d3a61b..a8e29950 100644
--- a/src/Tye.Hosting/ProxyService.cs
+++ b/src/Tye.Hosting/ProxyService.cs
@@ -86,9 +86,9 @@ namespace Tye.Hosting
                                     }
 
                                     _logger.LogInformation(
-                                        "Mapping external port {ExternalPort} to internal port(s) {InternalPorts} for {ServiceName} binding {BindingName}", 
-                                        binding.Port, 
-                                        string.Join(", ", ports.Select(p => p.ToString())), 
+                                        "Mapping external port {ExternalPort} to internal port(s) {InternalPorts} for {ServiceName} binding {BindingName}",
+                                        binding.Port,
+                                        string.Join(", ", ports.Select(p => p.ToString())),
                                         service.Description.Name,
                                         binding.Name ?? binding.Protocol);
 
diff --git a/src/Tye.Hosting/TyeHost.cs b/src/Tye.Hosting/TyeHost.cs
index ca805822..33a75c59 100644
--- a/src/Tye.Hosting/TyeHost.cs
+++ b/src/Tye.Hosting/TyeHost.cs
@@ -41,6 +41,8 @@ namespace Tye.Hosting
             _args = args;
         }
 
+        public Tye.Hosting.Model.Application Application => _application;
+
         public WebApplication? DashboardWebApplication { get; set; }
 
         // An additional sink that output will be piped to. Useful for testing.
diff --git a/src/tye/GenerateHost.cs b/src/tye/GenerateHost.cs
index fb6382f7..ce070a0c 100644
--- a/src/tye/GenerateHost.cs
+++ b/src/tye/GenerateHost.cs
@@ -47,6 +47,8 @@ namespace Tye
             var outputFilePath = Path.GetFullPath(Path.Combine(application.RootDirectory, $"{applicationName}-generate-{environment}.yaml"));
             output.WriteInfoLine($"Writing output to '{outputFilePath}'.");
             {
+                File.Delete(outputFilePath);
+
                 using var stream = File.OpenWrite(outputFilePath);
                 using var writer = new StreamWriter(stream, Encoding.UTF8, leaveOpen: true);
 
diff --git a/src/tye/Program.DeployCommand.cs b/src/tye/Program.DeployCommand.cs
index 05a27b4f..54ab7771 100644
--- a/src/tye/Program.DeployCommand.cs
+++ b/src/tye/Program.DeployCommand.cs
@@ -102,13 +102,29 @@ namespace Tye
 
                     foreach (var configBinding in configService.Bindings)
                     {
-                        service.Bindings.Add(new ServiceBinding(configBinding.Name ?? service.Name)
+                        var binding = new ServiceBinding(configBinding.Name ?? service.Name)
                         {
                             ConnectionString = configBinding.ConnectionString,
                             Host = configBinding.Host,
                             Port = configBinding.Port,
                             Protocol = configBinding.Protocol,
-                        });
+                        };
+
+                        binding.Protocol ??= "http";
+
+                        if (binding.Port == null && configBinding.AutoAssignPort)
+                        {
+                            if (binding.Protocol == "http" || binding.Protocol == null)
+                            {
+                                binding.Port = 80;
+                            }
+                            else if (binding.Protocol == "https")
+                            {
+                                binding.Port = 443;
+                            }
+                        }
+
+                        service.Bindings.Add(binding);
                     }
 
                     var serviceEntry = new ServiceEntry(service, configService.Name);
@@ -117,7 +133,7 @@ namespace Tye
 
                     var container = new ContainerInfo()
                     {
-                        UseMultiphaseDockerfile = false,
+                        UseMultiphaseDockerfile = true,
                     };
                     service.GeneratedAssets.Container = container;
                     services.Add(serviceEntry);
diff --git a/test/E2ETest/TyeRunTests.cs b/test/E2ETest/TyeRunTests.cs
index 540d9baa..542c4df1 100644
--- a/test/E2ETest/TyeRunTests.cs
+++ b/test/E2ETest/TyeRunTests.cs
@@ -3,7 +3,6 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
-using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Net;
@@ -36,9 +35,7 @@ namespace E2ETest
             DirectoryCopy.Copy(projectDirectory.FullName, tempDirectory.DirectoryPath);
 
             var projectFile = new FileInfo(Path.Combine(tempDirectory.DirectoryPath, "test-project.csproj"));
-
-            var application = ConfigFactory.FromFile(projectFile);
-            using var host = new TyeHost(application.ToHostingApplication(), Array.Empty<string>())
+            using var host = new TyeHost(ConfigFactory.FromFile(projectFile).ToHostingApplication(), Array.Empty<string>())
             {
                 Sink = sink,
             };
@@ -60,8 +57,8 @@ namespace E2ETest
                 var dashboardResponse = await client.GetStringAsync(dashboardUri);
 
                 // Only one service for single application.
-                var service = application.Services.First();
-                var binding = service.Bindings.First();
+                var service = host.Application.Services.First().Value;
+                var binding = service.Description.Bindings.First();
 
                 var protocol = binding.Protocol?.Length != 0 ? binding.Protocol : "http";
                 var hostName = binding.Host != null && binding.Host.Length != 0 ? binding.Host : "localhost";
@@ -84,11 +81,11 @@ namespace E2ETest
                 {
                     // If we failed, there's a good chance the service isn't running. Let's get the logs either way and put
                     // them in the output.
-                    var request = new HttpRequestMessage(HttpMethod.Get, new Uri(dashboardUri, $"/api/v1/logs/{service.Name}"));
+                    var request = new HttpRequestMessage(HttpMethod.Get, new Uri(dashboardUri, $"/api/v1/logs/{service.Description.Name}"));
                     var response = await client.SendAsync(request);
                     var text = await response.Content.ReadAsStringAsync();
 
-                    output.WriteLine($"Logs for service: {service.Name}");
+                    output.WriteLine($"Logs for service: {service.Description.Name}");
                     output.WriteLine(text);
                 }
             }
@@ -106,9 +103,7 @@ namespace E2ETest
             DirectoryCopy.Copy(projectDirectory.FullName, tempDirectory.DirectoryPath);
 
             var projectFile = new FileInfo(Path.Combine(tempDirectory.DirectoryPath, "tye.yaml"));
-
-            var application = ConfigFactory.FromFile(projectFile);
-            using var host = new TyeHost(application.ToHostingApplication(), Array.Empty<string>())
+            using var host = new TyeHost(ConfigFactory.FromFile(projectFile).ToHostingApplication(), Array.Empty<string>())
             {
                 Sink = sink,
             };
@@ -127,8 +122,8 @@ namespace E2ETest
                 var dashboardUri = new Uri(host.DashboardWebApplication!.Addresses.First());
                 var dashboardResponse = await client.GetStringAsync(dashboardUri);
 
-                await CheckServiceIsUp(application, client, "backend", dashboardUri);
-                await CheckServiceIsUp(application, client, "frontend", dashboardUri);
+                await CheckServiceIsUp(host.Application, client, "backend", dashboardUri);
+                await CheckServiceIsUp(host.Application, client, "frontend", dashboardUri);
             }
             finally
             {
@@ -136,11 +131,11 @@ namespace E2ETest
             }
         }
 
-        private async Task CheckServiceIsUp(ConfigApplication application, HttpClient client, string serviceName, Uri dashboardUri)
+        private async Task CheckServiceIsUp(Tye.Hosting.Model.Application application, HttpClient client, string serviceName, Uri dashboardUri)
         {
             // make sure backend is up before frontend
-            var service = application.Services.Where(a => a.Name == serviceName).First();
-            var binding = service.Bindings.First();
+            var service = application.Services.Where(a => a.Value.Description.Name == serviceName).First().Value;
+            var binding = service.Description.Bindings.First();
 
             var protocol = binding.Protocol != null && binding.Protocol.Length != 0 ? binding.Protocol : "http";
             var hostName = binding.Host != null && binding.Host.Length != 0 ? binding.Host : "localhost";
@@ -168,13 +163,13 @@ namespace E2ETest
             {
                 // If we failed, there's a good chance the service isn't running. Let's get the logs either way and put
                 // them in the output.
-                foreach (var s in application.Services)
+                foreach (var s in application.Services.Values)
                 {
-                    var request = new HttpRequestMessage(HttpMethod.Get, new Uri(dashboardUri, $"/api/v1/logs/{s.Name}"));
+                    var request = new HttpRequestMessage(HttpMethod.Get, new Uri(dashboardUri, $"/api/v1/logs/{s.Description.Name}"));
                     var response = await client.SendAsync(request);
                     var text = await response.Content.ReadAsStringAsync();
 
-                    output.WriteLine($"Logs for service: {s.Name}");
+                    output.WriteLine($"Logs for service: {s.Description.Name}");
                     output.WriteLine(text);
                 }
             }
diff --git a/test/E2ETest/testassets/generate/frontend-backend.yaml b/test/E2ETest/testassets/generate/frontend-backend.yaml
index 5cef23ad..5b2ef157 100644
--- a/test/E2ETest/testassets/generate/frontend-backend.yaml
+++ b/test/E2ETest/testassets/generate/frontend-backend.yaml
@@ -22,13 +22,15 @@ spec:
         imagePullPolicy: Always
         env:
         - name: ASPNETCORE_URLS
-          value: http://*:5050
+          value: 'http://*'
+        - name: SERVICE__FRONTEND__PROTOCOL
+          value: 'http'
         - name: SERVICE__FRONTEND__PORT
-          value: '5051'
+          value: '80'
         - name: SERVICE__FRONTEND__HOST
           value: 'frontend'
         ports:
-        - containerPort: 5050
+        - containerPort: 80
 ...
 ---
 kind: Service
@@ -45,8 +47,8 @@ spec:
   ports:
   - name: backend
     protocol: TCP
-    port: 5050
-    targetPort: 5050
+    port: 80
+    targetPort: 80
 ...
 ---
 kind: Deployment
@@ -73,13 +75,15 @@ spec:
         imagePullPolicy: Always
         env:
         - name: ASPNETCORE_URLS
-          value: http://*:5051
+          value: 'http://*'
+        - name: SERVICE__BACKEND__PROTOCOL
+          value: 'http'
         - name: SERVICE__BACKEND__PORT
-          value: '5050'
+          value: '80'
         - name: SERVICE__BACKEND__HOST
           value: 'backend'
         ports:
-        - containerPort: 5051
+        - containerPort: 80
 ...
 ---
 kind: Service
@@ -96,6 +100,6 @@ spec:
   ports:
   - name: frontend
     protocol: TCP
-    port: 5051
-    targetPort: 5051
+    port: 80
+    targetPort: 80
 ...
diff --git a/test/E2ETest/testassets/generate/multi-project.yaml b/test/E2ETest/testassets/generate/multi-project.yaml
index 224dac61..630dc259 100644
--- a/test/E2ETest/testassets/generate/multi-project.yaml
+++ b/test/E2ETest/testassets/generate/multi-project.yaml
@@ -22,7 +22,9 @@ spec:
         imagePullPolicy: Always
         env:
         - name: ASPNETCORE_URLS
-          value: http://*:7000
+          value: 'http://*:7000'
+        - name: SERVICE__FRONTEND__PROTOCOL
+          value: 'http'
         - name: SERVICE__FRONTEND__PORT
           value: '8000'
         - name: SERVICE__FRONTEND__HOST
@@ -84,7 +86,9 @@ spec:
         imagePullPolicy: Always
         env:
         - name: ASPNETCORE_URLS
-          value: http://*:8000
+          value: 'http://*:8000'
+        - name: SERVICE__BACKEND__PROTOCOL
+          value: 'http'
         - name: SERVICE__BACKEND__PORT
           value: '7000'
         - name: SERVICE__BACKEND__HOST
@@ -145,10 +149,14 @@ spec:
         image: test/worker:1.0.0
         imagePullPolicy: Always
         env:
+        - name: SERVICE__BACKEND__PROTOCOL
+          value: 'http'
         - name: SERVICE__BACKEND__PORT
           value: '7000'
         - name: SERVICE__BACKEND__HOST
           value: 'backend'
+        - name: SERVICE__FRONTEND__PROTOCOL
+          value: 'http'
         - name: SERVICE__FRONTEND__PORT
           value: '8000'
         - name: SERVICE__FRONTEND__HOST
diff --git a/test/E2ETest/testassets/generate/single-project-noregistry.yaml b/test/E2ETest/testassets/generate/single-project-noregistry.yaml
index 72598a15..f5d5d0ec 100644
--- a/test/E2ETest/testassets/generate/single-project-noregistry.yaml
+++ b/test/E2ETest/testassets/generate/single-project-noregistry.yaml
@@ -22,10 +22,9 @@ spec:
         imagePullPolicy: Always
         env:
         - name: ASPNETCORE_URLS
-          value: http://*:5000
+          value: 'http://*'
         ports:
-        - containerPort: 5001
-        - containerPort: 5000
+        - containerPort: 80
 ...
 ---
 kind: Service
@@ -42,10 +41,6 @@ spec:
   ports:
   - name: test-project
     protocol: TCP
-    port: 5001
-    targetPort: 5001
-  - name: test-project
-    protocol: TCP
-    port: 5000
-    targetPort: 5000
+    port: 80
+    targetPort: 80
 ...
diff --git a/test/E2ETest/testassets/generate/single-project.yaml b/test/E2ETest/testassets/generate/single-project.yaml
index bab5a1d2..0717d032 100644
--- a/test/E2ETest/testassets/generate/single-project.yaml
+++ b/test/E2ETest/testassets/generate/single-project.yaml
@@ -22,10 +22,9 @@ spec:
         imagePullPolicy: Always
         env:
         - name: ASPNETCORE_URLS
-          value: http://*:5000
+          value: 'http://*'
         ports:
-        - containerPort: 5001
-        - containerPort: 5000
+        - containerPort: 80
 ...
 ---
 kind: Service
@@ -42,10 +41,6 @@ spec:
   ports:
   - name: test-project
     protocol: TCP
-    port: 5001
-    targetPort: 5001
-  - name: test-project
-    protocol: TCP
-    port: 5000
-    targetPort: 5000
+    port: 80
+    targetPort: 80
 ...