diff --git a/docs/features/servicediscovery.rst b/docs/features/servicediscovery.rst index e6cb3695..d115b9c2 100644 --- a/docs/features/servicediscovery.rst +++ b/docs/features/servicediscovery.rst @@ -63,7 +63,7 @@ You services need to be added to Consul something like below (c# style but hopef is not to add http or https to the Address field. I have been contacted before about not accepting scheme in Address and accepting scheme in address. After reading `this `_ I don't think the scheme should be in there. -.. code-block: json +.. code-block: csharp new AgentService() { @@ -73,6 +73,17 @@ in address. After reading `this ID = "some-id", } +Or + +.. code-block:: json + + "Service": { + "ID": "some-id", + "Service": "some-service-name", + "Address": "localhost", + "Port": 8080 + } + ACL Token --------- diff --git a/src/Ocelot/ServiceDiscovery/Errors/UnableToFindServiceDiscoveryProviderError.cs b/src/Ocelot/ServiceDiscovery/Errors/UnableToFindServiceDiscoveryProviderError.cs deleted file mode 100644 index a31ed2ee..00000000 --- a/src/Ocelot/ServiceDiscovery/Errors/UnableToFindServiceDiscoveryProviderError.cs +++ /dev/null @@ -1,12 +0,0 @@ -using Ocelot.Errors; - -namespace Ocelot.ServiceDiscovery.Errors -{ - public class UnableToFindServiceDiscoveryProviderError : Error - { - public UnableToFindServiceDiscoveryProviderError(string message) - : base(message, OcelotErrorCode.UnableToFindServiceDiscoveryProviderError) - { - } - } -} diff --git a/src/Ocelot/ServiceDiscovery/Providers/ConsulServiceDiscoveryProvider.cs b/src/Ocelot/ServiceDiscovery/Providers/ConsulServiceDiscoveryProvider.cs index 88bcc5d5..e8abe460 100644 --- a/src/Ocelot/ServiceDiscovery/Providers/ConsulServiceDiscoveryProvider.cs +++ b/src/Ocelot/ServiceDiscovery/Providers/ConsulServiceDiscoveryProvider.cs @@ -59,7 +59,7 @@ namespace Ocelot.ServiceDiscovery.Providers private bool IsValid(ServiceEntry serviceEntry) { - if (serviceEntry.Service.Address.Contains("http://") || serviceEntry.Service.Address.Contains("https://") || serviceEntry.Service.Port <= 0) + if (string.IsNullOrEmpty(serviceEntry.Service.Address) || serviceEntry.Service.Address.Contains("http://") || serviceEntry.Service.Address.Contains("https://") || serviceEntry.Service.Port <= 0) { return false; } diff --git a/test/Ocelot.UnitTests/ServiceDiscovery/ConsulServiceDiscoveryProviderTests.cs b/test/Ocelot.UnitTests/ServiceDiscovery/ConsulServiceDiscoveryProviderTests.cs index b60330d8..4bc05efb 100644 --- a/test/Ocelot.UnitTests/ServiceDiscovery/ConsulServiceDiscoveryProviderTests.cs +++ b/test/Ocelot.UnitTests/ServiceDiscovery/ConsulServiceDiscoveryProviderTests.cs @@ -31,7 +31,7 @@ namespace Ocelot.UnitTests.ServiceDiscovery private readonly Mock _factory; private readonly Mock _logger; private string _receivedToken; - private IConsulClientFactory _clientFactory; + private readonly IConsulClientFactory _clientFactory; public ConsulServiceDiscoveryProviderTests() { @@ -134,6 +134,41 @@ namespace Ocelot.UnitTests.ServiceDiscovery .BDDfy(); } + [Fact] + public void should_not_return_services_with_empty_address() + { + var serviceEntryOne = new ServiceEntry() + { + Service = new AgentService() + { + Service = _serviceName, + Address = "", + Port = 50881, + ID = Guid.NewGuid().ToString(), + Tags = new string[0] + }, + }; + + var serviceEntryTwo = new ServiceEntry() + { + Service = new AgentService() + { + Service = _serviceName, + Address = null, + Port = 50888, + ID = Guid.NewGuid().ToString(), + Tags = new string[0] + }, + }; + + this.Given(x => GivenThereIsAFakeConsulServiceDiscoveryProvider(_fakeConsulServiceDiscoveryUrl, _serviceName)) + .And(x => GivenTheServicesAreRegisteredWithConsul(serviceEntryOne, serviceEntryTwo)) + .When(x => WhenIGetTheServices()) + .Then(x => ThenTheCountIs(0)) + .And(x => ThenTheLoggerHasBeenCalledCorrectlyForEmptyAddress()) + .BDDfy(); + } + [Fact] public void should_not_return_services_with_invalid_port() { @@ -182,6 +217,19 @@ namespace Ocelot.UnitTests.ServiceDiscovery Times.Once); } + private void ThenTheLoggerHasBeenCalledCorrectlyForEmptyAddress() + { + _logger.Verify( + x => x.LogWarning( + "Unable to use service Address: and Port: 50881 as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"), + Times.Once); + + _logger.Verify( + x => x.LogWarning( + "Unable to use service Address: and Port: 50888 as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"), + Times.Once); + } + private void ThenTheLoggerHasBeenCalledCorrectlyForInvalidPorts() { _logger.Verify(