dont use dynamic routing unless service discovery provider explictly …

…set and log that we are going to use dynamic info (#437) * #428 dont use dynamic routing unless service discovery provider explictly set and log that we are going to use dynamic info * #428 fixed tests that were failing due to not maintaining backwards compat with config for service discovery
ThreeMammals · Jun 27, 2018 · 0f7aaa0 · 0f7aaa0
1 parent 9db4273
commit 0f7aaa0
Show file tree

Hide file tree

Showing 5 changed files with 128 additions and 92 deletions.
diff --git a/docs/features/servicediscovery.rst b/docs/features/servicediscovery.rst
@@ -18,7 +18,8 @@ will be used.
 
     "ServiceDiscoveryProvider": {
         "Host": "localhost",
-        "Port": 8500
+        "Port": 8500,
+        "Type": "Consul"
     }
 
 In the future we can add a feature that allows ReRoute specfic configuration. 
@@ -136,8 +137,8 @@ The config might look something like
             "RequestIdKey": null,
             "ServiceDiscoveryProvider": {
                 "Host": "localhost",
-                "Port": 8510,
-                "Type": null,
+                "Port": 8500,
+                "Type": "Consul",
                 "Token": null,
                 "ConfigurationKey": null
             },

diff --git a/src/Ocelot/Configuration/Creator/ServiceProviderConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/ServiceProviderConfigurationCreator.cs
@@ -8,13 +8,16 @@ public class ServiceProviderConfigurationCreator : IServiceProviderConfiguration
         public ServiceProviderConfiguration Create(FileGlobalConfiguration globalConfiguration)
         {
             var port = globalConfiguration?.ServiceDiscoveryProvider?.Port ?? 0;
-            var host = globalConfiguration?.ServiceDiscoveryProvider?.Host ?? "consul";
+            var host = globalConfiguration?.ServiceDiscoveryProvider?.Host ?? "localhost";
+            var type = !string.IsNullOrEmpty(globalConfiguration?.ServiceDiscoveryProvider?.Type) 
+                ? globalConfiguration?.ServiceDiscoveryProvider?.Type 
+                : "consul";
             var pollingInterval = globalConfiguration?.ServiceDiscoveryProvider?.PollingInterval ?? 0;
 
             return new ServiceProviderConfigurationBuilder()
                 .WithHost(host)
                 .WithPort(port)
-                .WithType(globalConfiguration?.ServiceDiscoveryProvider?.Type)
+                .WithType(type)
                 .WithToken(globalConfiguration?.ServiceDiscoveryProvider?.Token)
                 .WithConfigurationKey(globalConfiguration?.ServiceDiscoveryProvider?.ConfigurationKey)
                 .WithPollingInterval(pollingInterval)

diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteProviderFactory.cs b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteProviderFactory.cs
@@ -5,20 +5,24 @@
     using System.Linq;
     using Configuration;
     using Microsoft.Extensions.DependencyInjection;
+    using Ocelot.Logging;
 
     public class DownstreamRouteProviderFactory : IDownstreamRouteProviderFactory
     {
         private readonly Dictionary<string, IDownstreamRouteProvider> _providers;
+        private IOcelotLogger _logger;
 
-        public DownstreamRouteProviderFactory(IServiceProvider provider)
+        public DownstreamRouteProviderFactory(IServiceProvider provider, IOcelotLoggerFactory factory)
         {
+            _logger = factory.CreateLogger<DownstreamRouteProviderFactory>();
             _providers = provider.GetServices<IDownstreamRouteProvider>().ToDictionary(x => x.GetType().Name);
         }
 
         public IDownstreamRouteProvider Get(IInternalConfiguration config)
         {
             if(!config.ReRoutes.Any() && IsServiceDiscovery(config.ServiceProviderConfiguration))
             {
+                _logger.LogInformation($"Selected {nameof(DownstreamRouteCreator)} as DownstreamRouteProvider for this request");
                 return _providers[nameof(DownstreamRouteCreator)];
             }
 
@@ -27,7 +31,7 @@ public IDownstreamRouteProvider Get(IInternalConfiguration config)
 
         private bool IsServiceDiscovery(ServiceProviderConfiguration config)
         {
-            if(!string.IsNullOrEmpty(config?.Host) || config?.Port > 0)
+            if(!string.IsNullOrEmpty(config?.Host) && config?.Port > 0 && !string.IsNullOrEmpty(config.Type))
             {
                 return true;
             }

diff --git a/test/Ocelot.ManualTest/Program.cs b/test/Ocelot.ManualTest/Program.cs
@@ -1,61 +1,63 @@
-namespace Ocelot.ManualTest
-{
-    using System.IO;
-    using Microsoft.AspNetCore.Hosting;
-    using Microsoft.Extensions.Logging;
-    using Microsoft.Extensions.Configuration;
-    using Microsoft.Extensions.DependencyInjection;
-    using Ocelot.DependencyInjection;
-    using Ocelot.Middleware;
-
-    public class Program
-    {
-        public static void Main(string[] args)
-        {
-            new WebHostBuilder()
-                .UseKestrel()
-                .UseContentRoot(Directory.GetCurrentDirectory())
-                .ConfigureAppConfiguration((hostingContext, config) =>
-                {
-                    config
-                        .SetBasePath(hostingContext.HostingEnvironment.ContentRootPath)
-                        .AddJsonFile("appsettings.json", true, true)
-                        .AddJsonFile($"appsettings.{hostingContext.HostingEnvironment.EnvironmentName}.json", true, true)
-                        .AddJsonFile("ocelot.json")
-                        .AddEnvironmentVariables();
-                })
-                .ConfigureServices(s => {
-                     s.AddAuthentication()
-                        .AddJwtBearer("TestKey", x =>
-                        {
-                            x.Authority = "test";
-                            x.Audience = "test";
-                        });
-
-                    s.AddOcelot()
-                        .AddCacheManager(x =>
-                        {
-                            x.WithDictionaryHandle();
-                        })
-                      /*.AddOpenTracing(option =>
-                      {
-                          option.CollectorUrl = "http://localhost:9618";
-                          option.Service = "Ocelot.ManualTest";
-                      })*/
-                    .AddAdministration("/administration", "secret");
-                })
-                .ConfigureLogging((hostingContext, logging) =>
-                {
-                    logging.AddConfiguration(hostingContext.Configuration.GetSection("Logging"));
-                    logging.AddConsole();
-                })
-                .UseIISIntegration()
-                .Configure(app =>
-                {
-                    app.UseOcelot().Wait();
-                })
-                .Build()
-                .Run();                
-        }
-    }
-}
+namespace Ocelot.ManualTest
+{
+    using System.IO;
+    using Microsoft.AspNetCore.Hosting;
+    using Microsoft.Extensions.Logging;
+    using Microsoft.Extensions.Configuration;
+    using Microsoft.Extensions.DependencyInjection;
+    using Ocelot.DependencyInjection;
+    using Ocelot.Middleware;
+    using System;
+    using IdentityServer4.AccessTokenValidation;
+
+    public class Program
+    {
+        public static void Main(string[] args)
+        {
+            new WebHostBuilder()
+                .UseKestrel()
+                .UseContentRoot(Directory.GetCurrentDirectory())
+                .ConfigureAppConfiguration((hostingContext, config) =>
+                {
+                    config
+                        .SetBasePath(hostingContext.HostingEnvironment.ContentRootPath)
+                        .AddJsonFile("appsettings.json", true, true)
+                        .AddJsonFile($"appsettings.{hostingContext.HostingEnvironment.EnvironmentName}.json", true, true)
+                        .AddJsonFile("ocelot.json")
+                        .AddEnvironmentVariables();
+                })
+                .ConfigureServices(s => {
+                     s.AddAuthentication()
+                        .AddJwtBearer("TestKey", x =>
+                        {
+                            x.Authority = "test";
+                            x.Audience = "test";
+                        });
+
+                    s.AddOcelot()
+                        .AddCacheManager(x =>
+                        {
+                            x.WithDictionaryHandle();
+                        })
+                      /*.AddOpenTracing(option =>
+                      {
+                          option.CollectorUrl = "http://localhost:9618";
+                          option.Service = "Ocelot.ManualTest";
+                      })*/
+                    .AddAdministration("/administration", "secret");
+                })
+                .ConfigureLogging((hostingContext, logging) =>
+                {
+                    logging.AddConfiguration(hostingContext.Configuration.GetSection("Logging"));
+                    logging.AddConsole();
+                })
+                .UseIISIntegration()
+                .Configure(app =>
+                {
+                    app.UseOcelot().Wait();
+                })
+                .Build()
+                .Run();                
+        }
+    }
+}
diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteProviderFactoryTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteProviderFactoryTests.cs
@@ -1,26 +1,28 @@
-using System.Collections.Generic;
-using Microsoft.Extensions.DependencyInjection;
-using Moq;
-using Ocelot.Configuration;
-using Ocelot.Configuration.Builder;
-using Ocelot.DownstreamRouteFinder;
-using Ocelot.DownstreamRouteFinder.Finder;
-using Ocelot.DownstreamRouteFinder.UrlMatcher;
-using Ocelot.Responses;
-using Ocelot.Values;
-using Shouldly;
-using TestStack.BDDfy;
-using Xunit;
-
 namespace Ocelot.UnitTests.DownstreamRouteFinder
 {
+    using System.Collections.Generic;
+    using Microsoft.Extensions.DependencyInjection;
+    using Moq;
+    using Ocelot.Configuration;
+    using Ocelot.Configuration.Builder;
+    using Ocelot.DownstreamRouteFinder;
+    using Ocelot.DownstreamRouteFinder.Finder;
+    using Ocelot.DownstreamRouteFinder.UrlMatcher;
+    using Ocelot.Responses;
+    using Ocelot.Values;
+    using Shouldly;
+    using TestStack.BDDfy;
+    using Xunit;
     using Ocelot.Configuration.Creator;
+    using Ocelot.Logging;
 
     public class DownstreamRouteProviderFactoryTests
     {
         private readonly DownstreamRouteProviderFactory _factory;
         private IInternalConfiguration _config;
         private IDownstreamRouteProvider _result;
+        private Mock<IOcelotLogger> _logger;
+        private Mock<IOcelotLoggerFactory> _loggerFactory;
 
         public DownstreamRouteProviderFactoryTests()
         {
@@ -31,7 +33,10 @@ public DownstreamRouteProviderFactoryTests()
             services.AddSingleton<IDownstreamRouteProvider, Ocelot.DownstreamRouteFinder.Finder.DownstreamRouteFinder>();
             services.AddSingleton<IDownstreamRouteProvider, Ocelot.DownstreamRouteFinder.Finder.DownstreamRouteCreator>();
             var provider = services.BuildServiceProvider();
-            _factory = new DownstreamRouteProviderFactory(provider);
+            _logger = new Mock<IOcelotLogger>();
+            _loggerFactory = new Mock<IOcelotLoggerFactory>();
+            _loggerFactory.Setup(x => x.CreateLogger<DownstreamRouteProviderFactory>()).Returns(_logger.Object);
+            _factory = new DownstreamRouteProviderFactory(provider, _loggerFactory.Object);
         }
 
         [Fact]
@@ -49,12 +54,34 @@ public void should_return_downstream_route_finder()
         }
 
         [Fact]
-        public void should_return_downstream_route_finder_as_no_service_discovery()
+        public void should_return_downstream_route_finder_as_no_service_discovery_given_no_host()
         {
-            var spConfig = new ServiceProviderConfigurationBuilder().Build();
-            var reRoutes = new List<ReRoute>
-            {
-            };
+            var spConfig = new ServiceProviderConfigurationBuilder().WithHost("").WithPort(50).Build();
+            var reRoutes = new List<ReRoute>();
+
+            this.Given(_ => GivenTheReRoutes(reRoutes, spConfig))
+                .When(_ => WhenIGet())
+                .Then(_ => ThenTheResultShouldBe<Ocelot.DownstreamRouteFinder.Finder.DownstreamRouteFinder>())
+                .BDDfy();
+        }
+
+        [Fact]
+        public void should_return_downstream_route_finder_given_no_service_discovery_port()
+        {
+            var spConfig = new ServiceProviderConfigurationBuilder().WithHost("localhost").WithPort(0).Build();
+            var reRoutes = new List<ReRoute>();
+
+            this.Given(_ => GivenTheReRoutes(reRoutes, spConfig))
+                .When(_ => WhenIGet())
+                .Then(_ => ThenTheResultShouldBe<Ocelot.DownstreamRouteFinder.Finder.DownstreamRouteFinder>())
+                .BDDfy();
+        }
+
+        [Fact]
+        public void should_return_downstream_route_finder_given_no_service_discovery_type()
+        {
+            var spConfig = new ServiceProviderConfigurationBuilder().WithHost("localhost").WithPort(50).WithType("").Build();
+            var reRoutes = new List<ReRoute>();
 
             this.Given(_ => GivenTheReRoutes(reRoutes, spConfig))
                 .When(_ => WhenIGet())
@@ -65,10 +92,9 @@ public void should_return_downstream_route_finder_as_no_service_discovery()
         [Fact]
         public void should_return_downstream_route_creator()
         {
-            var spConfig = new ServiceProviderConfigurationBuilder().WithHost("test").WithPort(50).Build();
-            var reRoutes = new List<ReRoute>
-            {
-            };
+            var spConfig = new ServiceProviderConfigurationBuilder().WithHost("test").WithPort(50).WithType("test").Build();
+            var reRoutes = new List<ReRoute>();
+
             this.Given(_ => GivenTheReRoutes(reRoutes, spConfig))
                 .When(_ => WhenIGet())
                 .Then(_ => ThenTheResultShouldBe<Ocelot.DownstreamRouteFinder.Finder.DownstreamRouteCreator>())