Feature/should not start if specified using service discovery but no provider registered (#584)

* #580 added failing test

* #580 added failing test for dynamic reroutes

* #580 added failing test for validator

* #580 validation tests passing

* #580 acceptance tests passing

* #580 got rid of the list in sdp factory

* +semver: breaking #580 service discovery provider returned by delegate must be the same as name in config, this is a breaking change because you have to specify consul now

* #580 removed use servide discovery property from file config, we dont need it, just use the service name as indicator the user wants to use service discovery for the given reroute

src/Ocelot/Configuration/Creator/FileInternalConfigurationCreator.cs

@@ -224,6 +224,8 @@ namespace Ocelot.Configuration.Creator
 
             var lbOptions = CreateLoadBalancerOptions(fileReRoute.LoadBalancerOptions);
 
+            var useServiceDiscovery = !string.IsNullOrEmpty(fileReRoute.ServiceName);
+
             var reRoute = new DownstreamReRouteBuilder()
                 .WithKey(fileReRoute.Key)
                 .WithDownstreamPathTemplate(fileReRoute.DownstreamPathTemplate)
@@ -249,7 +251,7 @@ namespace Ocelot.Configuration.Creator
                 .WithRateLimitOptions(rateLimitOption)
                 .WithHttpHandlerOptions(httpHandlerOptions)
                 .WithServiceName(fileReRoute.ServiceName)
-                .WithUseServiceDiscovery(fileReRoute.UseServiceDiscovery)
+                .WithUseServiceDiscovery(useServiceDiscovery)
                 .WithUpstreamHeaderFindAndReplace(hAndRs.Upstream)
                 .WithDownstreamHeaderFindAndReplace(hAndRs.Downstream)
                 .WithUpstreamHost(fileReRoute.UpstreamHost)

src/Ocelot/Configuration/File/FileGlobalConfiguration.cs

@@ -1,30 +1,30 @@
-namespace Ocelot.Configuration.File
-{
-    public class FileGlobalConfiguration
-    {
-        public FileGlobalConfiguration()
-        {
-            ServiceDiscoveryProvider = new FileServiceDiscoveryProvider();
-            RateLimitOptions = new FileRateLimitOptions();
-            LoadBalancerOptions = new FileLoadBalancerOptions();
-            QoSOptions = new FileQoSOptions();
-            HttpHandlerOptions = new FileHttpHandlerOptions();
-        }
-
-        public string RequestIdKey { get; set; }
-
-        public FileServiceDiscoveryProvider ServiceDiscoveryProvider { get;set; }
-
-        public FileRateLimitOptions RateLimitOptions { get; set; }
-
-        public FileQoSOptions QoSOptions { get; set; }
-
-        public string BaseUrl { get ;set; }
-
-        public FileLoadBalancerOptions LoadBalancerOptions { get; set; }
-
-        public string DownstreamScheme { get; set; }
-
-        public FileHttpHandlerOptions HttpHandlerOptions { get; set; }
-    }
-}
+namespace Ocelot.Configuration.File
+{
+    public class FileGlobalConfiguration
+    {
+        public FileGlobalConfiguration()
+        {
+            ServiceDiscoveryProvider = new FileServiceDiscoveryProvider();
+            RateLimitOptions = new FileRateLimitOptions();
+            LoadBalancerOptions = new FileLoadBalancerOptions();
+            QoSOptions = new FileQoSOptions();
+            HttpHandlerOptions = new FileHttpHandlerOptions();
+        }
+
+        public string RequestIdKey { get; set; }
+
+        public FileServiceDiscoveryProvider ServiceDiscoveryProvider { get;set; }
+
+        public FileRateLimitOptions RateLimitOptions { get; set; }
+
+        public FileQoSOptions QoSOptions { get; set; }
+
+        public string BaseUrl { get ;set; }
+
+        public FileLoadBalancerOptions LoadBalancerOptions { get; set; }
+
+        public string DownstreamScheme { get; set; }
+
+        public FileHttpHandlerOptions HttpHandlerOptions { get; set; }
+    }
+}

src/Ocelot/Configuration/File/FileReRoute.cs

@@ -43,7 +43,6 @@ namespace Ocelot.Configuration.File
         public FileRateLimitRule RateLimitOptions { get; set; }
         public FileAuthenticationOptions AuthenticationOptions { get; set; }
         public FileHttpHandlerOptions HttpHandlerOptions { get; set; }
-        public bool UseServiceDiscovery { get;set; }
         public List<FileHostAndPort> DownstreamHostAndPorts {get;set;}
         public string UpstreamHost { get; set; }
         public string Key { get;set; }

src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs

@@ -11,15 +11,19 @@ namespace Ocelot.Configuration.Validator
 {
     using System;
     using Microsoft.Extensions.DependencyInjection;
+    using Ocelot.ServiceDiscovery;
     using Requester;
 
     public class FileConfigurationFluentValidator : AbstractValidator<FileConfiguration>, IConfigurationValidator
     {
         private readonly QosDelegatingHandlerDelegate _qosDelegatingHandlerDelegate;
-
+        private readonly List<ServiceDiscoveryFinderDelegate> _serviceDiscoveryFinderDelegates;
         public FileConfigurationFluentValidator(IAuthenticationSchemeProvider authenticationSchemeProvider, IServiceProvider provider)
         {
             _qosDelegatingHandlerDelegate = provider.GetService<QosDelegatingHandlerDelegate>();
+            _serviceDiscoveryFinderDelegates = provider
+                .GetServices<ServiceDiscoveryFinderDelegate>()
+                .ToList();
 
             RuleFor(configuration => configuration.ReRoutes)
                 .SetCollectionValidator(new ReRouteFluentValidator(authenticationSchemeProvider, _qosDelegatingHandlerDelegate));
@@ -31,6 +35,14 @@ namespace Ocelot.Configuration.Validator
                 .Must((config, reRoute) => IsNotDuplicateIn(reRoute, config.ReRoutes))
                 .WithMessage((config, reRoute) => $"{nameof(reRoute)} {reRoute.UpstreamPathTemplate} has duplicate");
 
+            RuleForEach(configuration => configuration.ReRoutes)
+                .Must((config, reRoute) => HaveServiceDiscoveryProviderRegitered(reRoute, config.GlobalConfiguration.ServiceDiscoveryProvider))
+                .WithMessage((config, reRoute) => $"Unable to start Ocelot, errors are: Unable to start Ocelot because either a ReRoute or GlobalConfiguration are using ServiceDiscoveryOptions but no ServiceDiscoveryFinderDelegate has been registered in dependency injection container. Are you missing a package like Ocelot.Provider.Consul and services.AddConsul() or Ocelot.Provider.Eureka and services.AddEureka()?");
+
+            RuleFor(configuration => configuration.GlobalConfiguration.ServiceDiscoveryProvider)
+                .Must((config) => HaveServiceDiscoveryProviderRegitered(config))
+                .WithMessage((config, reRoute) => $"Unable to start Ocelot, errors are: Unable to start Ocelot because either a ReRoute or GlobalConfiguration are using ServiceDiscoveryOptions but no ServiceDiscoveryFinderDelegate has been registered in dependency injection container. Are you missing a package like Ocelot.Provider.Consul and services.AddConsul() or Ocelot.Provider.Eureka and services.AddEureka()?");
+
             RuleForEach(configuration => configuration.ReRoutes)
                 .Must((config, reRoute) => IsNotDuplicateIn(reRoute, config.Aggregates))
                 .WithMessage((config, reRoute) => $"{nameof(reRoute)} {reRoute.UpstreamPathTemplate} has duplicate aggregate");
@@ -48,6 +60,41 @@ namespace Ocelot.Configuration.Validator
                 .WithMessage((config, aggregateReRoute) => $"{nameof(aggregateReRoute)} {aggregateReRoute.UpstreamPathTemplate} contains ReRoute with specific RequestIdKey, this is not possible with Aggregates");
         }
 
+        private bool HaveServiceDiscoveryProviderRegitered(FileReRoute reRoute, FileServiceDiscoveryProvider serviceDiscoveryProvider)
+        {
+            if (string.IsNullOrEmpty(reRoute.ServiceName))
+            {
+                return true;
+            }
+
+            if (serviceDiscoveryProvider?.Type?.ToLower() == "servicefabric")
+            {
+                return true;
+            }
+
+            return _serviceDiscoveryFinderDelegates.Any();
+        }
+
+        private bool HaveServiceDiscoveryProviderRegitered(FileServiceDiscoveryProvider serviceDiscoveryProvider)
+        {
+            if(serviceDiscoveryProvider == null)
+            {
+                return true;
+            }
+            
+            if (serviceDiscoveryProvider?.Type?.ToLower() == "servicefabric")
+            {
+                return true;
+            }
+
+            if(string.IsNullOrEmpty(serviceDiscoveryProvider.Type))
+            {
+                return true;
+            }
+
+            return _serviceDiscoveryFinderDelegates.Any();
+        }
+
         public async Task<Response<ConfigurationValidationResult>> IsValid(FileConfiguration configuration)
         {
             var validateResult = await ValidateAsync(configuration);

src/Ocelot/Configuration/Validator/ReRouteFluentValidator.cs

@@ -55,17 +55,12 @@
                 .MustAsync(IsSupportedAuthenticationProviders)
                 .WithMessage("{PropertyValue} is unsupported authentication provider");
 
-            When(reRoute => reRoute.UseServiceDiscovery, () => {
-                RuleFor(r => r.ServiceName).NotEmpty()
-                    .WithMessage("ServiceName cannot be empty or null when using service discovery or Ocelot cannot look up your service!");
-                });
-
-            When(reRoute => !reRoute.UseServiceDiscovery, () => {
+            When(reRoute => string.IsNullOrEmpty(reRoute.ServiceName), () => {
                 RuleFor(r => r.DownstreamHostAndPorts).NotEmpty()
                     .WithMessage("When not using service discovery DownstreamHostAndPorts must be set and not empty or Ocelot cannot find your service!");
             });
 
-            When(reRoute => !reRoute.UseServiceDiscovery, () => {
+            When(reRoute => string.IsNullOrEmpty(reRoute.ServiceName), () => {
                 RuleFor(reRoute => reRoute.DownstreamHostAndPorts)
                     .SetCollectionValidator(new HostAndPortValidator());
             });

src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs

@@ -122,7 +122,7 @@ namespace Ocelot.DownstreamUrlCreator.Middleware
 
         private static bool ServiceFabricRequest(DownstreamContext context)
         {
-            return context.Configuration.ServiceProviderConfiguration.Type == "ServiceFabric" && context.DownstreamReRoute.UseServiceDiscovery;
+            return context.Configuration.ServiceProviderConfiguration.Type?.ToLower() == "servicefabric" && context.DownstreamReRoute.UseServiceDiscovery;
         }
     }
 }

src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerFactory.cs

@@ -1,10 +1,11 @@
-using System.Threading.Tasks;
-using Ocelot.Configuration;
-
-namespace Ocelot.LoadBalancer.LoadBalancers
+namespace Ocelot.LoadBalancer.LoadBalancers
 {
+    using System.Threading.Tasks;
+    using Ocelot.Configuration;
+    using Ocelot.Responses;
+
     public interface ILoadBalancerFactory
     {
-        Task<ILoadBalancer> Get(DownstreamReRoute reRoute, ServiceProviderConfiguration config);
+        Task<Response<ILoadBalancer>> Get(DownstreamReRoute reRoute, ServiceProviderConfiguration config);
     }
 }

src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs

@@ -1,6 +1,7 @@
 using System.Threading.Tasks;
 using Ocelot.Configuration;
 using Ocelot.Infrastructure;
+using Ocelot.Responses;
 using Ocelot.ServiceDiscovery;
 
 namespace Ocelot.LoadBalancer.LoadBalancers
@@ -14,22 +15,29 @@ namespace Ocelot.LoadBalancer.LoadBalancers
             _serviceProviderFactory = serviceProviderFactory;
         }
 
-        public async Task<ILoadBalancer> Get(DownstreamReRoute reRoute, ServiceProviderConfiguration config)
+        public async Task<Response<ILoadBalancer>> Get(DownstreamReRoute reRoute, ServiceProviderConfiguration config)
         {            
-            var serviceProvider = _serviceProviderFactory.Get(config, reRoute);
+            var response = _serviceProviderFactory.Get(config, reRoute);
+
+            if(response.IsError)
+            {
+                return new ErrorResponse<ILoadBalancer>(response.Errors);
+            }
+
+            var serviceProvider = response.Data;
 
             switch (reRoute.LoadBalancerOptions?.Type)
             {
                 case nameof(RoundRobin):
-                    return new RoundRobin(async () => await serviceProvider.Get());
+                    return new OkResponse<ILoadBalancer>(new RoundRobin(async () => await serviceProvider.Get()));
                 case nameof(LeastConnection):
-                    return new LeastConnection(async () => await serviceProvider.Get(), reRoute.ServiceName);
+                    return new OkResponse<ILoadBalancer>(new LeastConnection(async () => await serviceProvider.Get(), reRoute.ServiceName));
                 case nameof(CookieStickySessions):
                     var loadBalancer = new RoundRobin(async () => await serviceProvider.Get());
                     var bus = new InMemoryBus<StickySession>();
-                    return new CookieStickySessions(loadBalancer, reRoute.LoadBalancerOptions.Key, reRoute.LoadBalancerOptions.ExpiryInMs, bus);
+                    return new OkResponse<ILoadBalancer>(new CookieStickySessions(loadBalancer, reRoute.LoadBalancerOptions.Key, reRoute.LoadBalancerOptions.ExpiryInMs, bus));
                 default:
-                    return new NoLoadBalancer(async () => await serviceProvider.Get());
+                    return new OkResponse<ILoadBalancer>(new NoLoadBalancer(async () => await serviceProvider.Get()));
             }
         }
     }

src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerHouse.cs

@@ -22,24 +22,36 @@ namespace Ocelot.LoadBalancer.LoadBalancers
         {
             try
             {
-                if(_loadBalancers.TryGetValue(reRoute.LoadBalancerKey, out var loadBalancer))
+                Response<ILoadBalancer> result;
+
+                if (_loadBalancers.TryGetValue(reRoute.LoadBalancerKey, out var loadBalancer))
                 {
                     loadBalancer = _loadBalancers[reRoute.LoadBalancerKey];
 
-                    if(reRoute.LoadBalancerOptions.Type != loadBalancer.GetType().Name)
+                    if (reRoute.LoadBalancerOptions.Type != loadBalancer.GetType().Name)
                     {
-                        loadBalancer = await _factory.Get(reRoute, config);
+                        result = await _factory.Get(reRoute, config);
+                        if (result.IsError)
+                        {
+                            return new ErrorResponse<ILoadBalancer>(result.Errors);
+                        }
+                        loadBalancer = result.Data;
                         AddLoadBalancer(reRoute.LoadBalancerKey, loadBalancer);
                     }
 
                     return new OkResponse<ILoadBalancer>(loadBalancer);
                 }
 
-                loadBalancer = await _factory.Get(reRoute, config);
+                result = await _factory.Get(reRoute, config);
+                if (result.IsError)
+                {
+                    return new ErrorResponse<ILoadBalancer>(result.Errors);
+                }
+                loadBalancer = result.Data;
                 AddLoadBalancer(reRoute.LoadBalancerKey, loadBalancer);
                 return new OkResponse<ILoadBalancer>(loadBalancer);
             }
-            catch(Exception ex)
+            catch (Exception ex)
             {
                 return new ErrorResponse<ILoadBalancer>(new List<Ocelot.Errors.Error>()
                 {

src/Ocelot/ServiceDiscovery/IServiceDiscoveryProviderFactory.cs

@@ -1,10 +1,11 @@
-using Ocelot.Configuration;
-using Ocelot.ServiceDiscovery.Providers;
-
-namespace Ocelot.ServiceDiscovery
-{
-    public interface IServiceDiscoveryProviderFactory
-    {
-        IServiceDiscoveryProvider Get(ServiceProviderConfiguration serviceConfig, DownstreamReRoute reRoute);
-    }
-}
+namespace Ocelot.ServiceDiscovery
+{
+    using Ocelot.Configuration;
+    using Ocelot.Responses;
+    using Ocelot.ServiceDiscovery.Providers;
+
+    public interface IServiceDiscoveryProviderFactory
+    {
+        Response<IServiceDiscoveryProvider> Get(ServiceProviderConfiguration serviceConfig, DownstreamReRoute reRoute);
+    }
+}

src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs

@@ -9,23 +9,22 @@ namespace Ocelot.ServiceDiscovery
     using System;
     using System.Linq;
     using Microsoft.Extensions.DependencyInjection;
+    using Ocelot.Responses;
 
     public class ServiceDiscoveryProviderFactory : IServiceDiscoveryProviderFactory
     {
         private readonly IOcelotLoggerFactory _factory;
-        private readonly List<ServiceDiscoveryFinderDelegate> _delegates;
+        private readonly ServiceDiscoveryFinderDelegate _delegates;
         private readonly IServiceProvider _provider;
 
         public ServiceDiscoveryProviderFactory(IOcelotLoggerFactory factory, IServiceProvider provider)
         {
             _factory = factory;
             _provider = provider;
-            _delegates = provider
-                .GetServices<ServiceDiscoveryFinderDelegate>()
-                .ToList();
+            _delegates = provider.GetService<ServiceDiscoveryFinderDelegate>();
         }
 
-        public IServiceDiscoveryProvider Get(ServiceProviderConfiguration serviceConfig, DownstreamReRoute reRoute)
+        public Response<IServiceDiscoveryProvider> Get(ServiceProviderConfiguration serviceConfig, DownstreamReRoute reRoute)
         {
             if (reRoute.UseServiceDiscovery)
             {
@@ -37,31 +36,32 @@ namespace Ocelot.ServiceDiscovery
             foreach (var downstreamAddress in reRoute.DownstreamAddresses)
             {
                 var service = new Service(reRoute.ServiceName, new ServiceHostAndPort(downstreamAddress.Host, downstreamAddress.Port), string.Empty, string.Empty, new string[0]);
-                
+
                 services.Add(service);
             }
 
-            return new ConfigurationServiceProvider(services);
+            return new OkResponse<IServiceDiscoveryProvider>(new ConfigurationServiceProvider(services));
         }
 
-        private IServiceDiscoveryProvider GetServiceDiscoveryProvider(ServiceProviderConfiguration config, string key)
+        private Response<IServiceDiscoveryProvider> GetServiceDiscoveryProvider(ServiceProviderConfiguration config, string key)
         {
             if (config.Type?.ToLower() == "servicefabric")
             {
                 var sfConfig = new ServiceFabricConfiguration(config.Host, config.Port, key);
-                return new ServiceFabricServiceDiscoveryProvider(sfConfig);
+                return new OkResponse<IServiceDiscoveryProvider>(new ServiceFabricServiceDiscoveryProvider(sfConfig));
             }
 
-            foreach (var serviceDiscoveryFinderDelegate in _delegates)
+            if (_delegates != null)
             {
-                var provider = serviceDiscoveryFinderDelegate?.Invoke(_provider, config, key);
-                if (provider != null)
+                var provider = _delegates?.Invoke(_provider, config, key);
+
+                if(provider.GetType().Name.ToLower() == config.Type.ToLower())
                 {
-                    return provider;
+                    return new OkResponse<IServiceDiscoveryProvider>(provider);
                 }
             }
 
-            return null;
+            return new ErrorResponse<IServiceDiscoveryProvider>(new UnableToFindServiceDiscoveryProviderError($"Unable to find service discovery provider for type: {config.Type}"));
         }
     }
 }

src/Ocelot/ServiceDiscovery/UnableToFindServiceDiscoveryProviderError.cs

@@ -0,0 +1,11 @@
+namespace Ocelot.ServiceDiscovery
+{
+    using Ocelot.Errors;
+
+    public class UnableToFindServiceDiscoveryProviderError : Error
+    {
+        public UnableToFindServiceDiscoveryProviderError(string message) : base(message, OcelotErrorCode.UnableToFindServiceDiscoveryProviderError)
+        {
+        }
+    }
+}