diff --git a/docs/features/routing.rst b/docs/features/routing.rst index 0359d917..4878b904 100644 --- a/docs/features/routing.rst +++ b/docs/features/routing.rst @@ -24,7 +24,7 @@ the following. "DownstreamPathTemplate": "/api/posts/{postId}", "DownstreamScheme": "https", "DownstreamPort": 80, - "DownstreamHost" "localhost", + "DownstreamHost":"localhost", "UpstreamPathTemplate": "/posts/{postId}", "UpstreamHttpMethod": [ "Put", "Delete" ] } @@ -46,7 +46,7 @@ You can also do a catch all type of ReRoute e.g. "DownstreamPathTemplate": "/api/{everything}", "DownstreamScheme": "https", "DownstreamPort": 80, - "DownstreamHost" "localhost", + "DownstreamHost":"localhost", "UpstreamPathTemplate": "/{everything}", "UpstreamHttpMethod": [ "Get", "Post" ] } @@ -75,7 +75,7 @@ Ocelot's routing also supports a catch all style routing where the user can spec "DownstreamPathTemplate": "/{url}", "DownstreamScheme": "https", "DownstreamPort": 80, - "DownstreamHost" "localhost", + "DownstreamHost":"localhost", "UpstreamPathTemplate": "/{url}", "UpstreamHttpMethod": [ "Get" ] } @@ -88,7 +88,7 @@ The catch all has a lower priority than any other ReRoute. If you also have the "DownstreamPathTemplate": "/", "DownstreamScheme": "https", "DownstreamPort": 80, - "DownstreamHost" "10.0.10.1", + "DownstreamHost":"10.0.10.1", "UpstreamPathTemplate": "/", "UpstreamHttpMethod": [ "Get" ] - } \ No newline at end of file + } diff --git a/src/Ocelot/Configuration/Validator/ReRouteFluentValidator.cs b/src/Ocelot/Configuration/Validator/ReRouteFluentValidator.cs index b386890f..e8088dab 100644 --- a/src/Ocelot/Configuration/Validator/ReRouteFluentValidator.cs +++ b/src/Ocelot/Configuration/Validator/ReRouteFluentValidator.cs @@ -19,6 +19,14 @@ namespace Ocelot.Configuration.Validator .Must(path => path.StartsWith("/")) .WithMessage("{PropertyName} {PropertyValue} doesnt start with forward slash"); + RuleFor(reRoute => reRoute.UpstreamPathTemplate) + .Must(path => !path.Contains("//")) + .WithMessage("{PropertyName} {PropertyValue} contains double forward slash, Ocelot does not support this at the moment. Please raise an issue in GitHib if you need this feature."); + + RuleFor(reRoute => reRoute.DownstreamPathTemplate) + .Must(path => !path.Contains("//")) + .WithMessage("{PropertyName} {PropertyValue} contains double forward slash, Ocelot does not support this at the moment. Please raise an issue in GitHib if you need this feature."); + RuleFor(reRoute => reRoute.UpstreamPathTemplate) .Must(path => path.StartsWith("/")) .WithMessage("{PropertyName} {PropertyValue} doesnt start with forward slash"); diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs index 0e136b4a..0afec16b 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs @@ -123,7 +123,7 @@ namespace Ocelot.LoadBalancer.LoadBalancers foreach (var service in services) { - var exists = _leases.FirstOrDefault(l => l.HostAndPort.ToString() == service.HostAndPort.ToString()); + var exists = _leases.FirstOrDefault(l => l.HostAndPort.DownstreamHost == service.HostAndPort.DownstreamHost && l.HostAndPort.DownstreamPort == service.HostAndPort.DownstreamPort); if (exists == null) { diff --git a/test/Ocelot.AcceptanceTests/ServiceDiscoveryTests.cs b/test/Ocelot.AcceptanceTests/ServiceDiscoveryTests.cs index 2c7b6b56..d6e9d862 100644 --- a/test/Ocelot.AcceptanceTests/ServiceDiscoveryTests.cs +++ b/test/Ocelot.AcceptanceTests/ServiceDiscoveryTests.cs @@ -32,6 +32,74 @@ namespace Ocelot.AcceptanceTests [Fact] public void should_use_service_discovery_and_load_balance_request() + { + var consulPort = 8502; + var serviceName = "product"; + var downstreamServiceOneUrl = "http://localhost:50881"; + var downstreamServiceTwoUrl = "http://localhost:50882"; + var fakeConsulServiceDiscoveryUrl = $"http://localhost:{consulPort}"; + var serviceEntryOne = new ServiceEntry() + { + Service = new AgentService() + { + Service = serviceName, + Address = "localhost", + Port = 50881, + ID = Guid.NewGuid().ToString(), + Tags = new string[0] + }, + }; + var serviceEntryTwo = new ServiceEntry() + { + Service = new AgentService() + { + Service = serviceName, + Address = "localhost", + Port = 50882, + ID = Guid.NewGuid().ToString(), + Tags = new string[0] + }, + }; + + var configuration = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/", + DownstreamScheme = "http", + UpstreamPathTemplate = "/", + UpstreamHttpMethod = new List { "Get" }, + ServiceName = serviceName, + LoadBalancer = "LeastConnection", + UseServiceDiscovery = true, + } + }, + GlobalConfiguration = new FileGlobalConfiguration() + { + ServiceDiscoveryProvider = new FileServiceDiscoveryProvider() + { + Host = "localhost", + Port = consulPort + } + } + }; + + this.Given(x => x.GivenProductServiceOneIsRunning(downstreamServiceOneUrl, 200)) + .And(x => x.GivenProductServiceTwoIsRunning(downstreamServiceTwoUrl, 200)) + .And(x => x.GivenThereIsAFakeConsulServiceDiscoveryProvider(fakeConsulServiceDiscoveryUrl)) + .And(x => x.GivenTheServicesAreRegisteredWithConsul(serviceEntryOne, serviceEntryTwo)) + .And(x => _steps.GivenThereIsAConfiguration(configuration)) + .And(x => _steps.GivenOcelotIsRunning()) + .When(x => _steps.WhenIGetUrlOnTheApiGatewayMultipleTimes("/", 50)) + .Then(x => x.ThenTheTwoServicesShouldHaveBeenCalledTimes(50)) + .And(x => x.ThenBothServicesCalledRealisticAmountOfTimes(24, 26)) + .BDDfy(); + } + + [Fact] + public void should_send_request_to_service_after_it_becomes_available() { var consulPort = 8501; var serviceName = "product"; @@ -92,16 +160,47 @@ namespace Ocelot.AcceptanceTests .And(x => x.GivenTheServicesAreRegisteredWithConsul(serviceEntryOne, serviceEntryTwo)) .And(x => _steps.GivenThereIsAConfiguration(configuration)) .And(x => _steps.GivenOcelotIsRunning()) - .When(x => _steps.WhenIGetUrlOnTheApiGatewayMultipleTimes("/", 50)) - .Then(x => x.ThenTheTwoServicesShouldHaveBeenCalledTimes(50)) - .And(x => x.ThenBothServicesCalledRealisticAmountOfTimes()) + .And(x => _steps.WhenIGetUrlOnTheApiGatewayMultipleTimes("/", 10)) + .And(x => x.ThenTheTwoServicesShouldHaveBeenCalledTimes(10)) + .And(x => x.ThenBothServicesCalledRealisticAmountOfTimes(4, 6)) + .And(x => WhenIRemoveAService(serviceEntryTwo)) + .And(x => GivenIResetCounters()) + .And(x => _steps.WhenIGetUrlOnTheApiGatewayMultipleTimes("/", 10)) + .And(x => ThenOnlyOneServiceHasBeenCalled()) + .And(x => WhenIAddAServiceBackIn(serviceEntryTwo)) + .And(x => GivenIResetCounters()) + .When(x => _steps.WhenIGetUrlOnTheApiGatewayMultipleTimes("/", 10)) + .Then(x => x.ThenTheTwoServicesShouldHaveBeenCalledTimes(10)) + .And(x => x.ThenBothServicesCalledRealisticAmountOfTimes(4, 6)) .BDDfy(); } - private void ThenBothServicesCalledRealisticAmountOfTimes() + private void WhenIAddAServiceBackIn(ServiceEntry serviceEntryTwo) { - _counterOne.ShouldBeInRange(24,26); - _counterOne.ShouldBeInRange(24,26); + _serviceEntries.Add(serviceEntryTwo); + } + + private void ThenOnlyOneServiceHasBeenCalled() + { + _counterOne.ShouldBe(10); + _counterTwo.ShouldBe(0); + } + + private void WhenIRemoveAService(ServiceEntry serviceEntryTwo) + { + _serviceEntries.Remove(serviceEntryTwo); + } + + private void GivenIResetCounters() + { + _counterOne = 0; + _counterTwo = 0; + } + + private void ThenBothServicesCalledRealisticAmountOfTimes(int bottom, int top) + { + _counterOne.ShouldBeInRange(bottom, top); + _counterOne.ShouldBeInRange(bottom, top); } private void ThenTheTwoServicesShouldHaveBeenCalledTimes(int expected) diff --git a/test/Ocelot.UnitTests/Configuration/ConfigurationFluentValidationTests.cs b/test/Ocelot.UnitTests/Configuration/ConfigurationFluentValidationTests.cs index 90475a27..ea476ca8 100644 --- a/test/Ocelot.UnitTests/Configuration/ConfigurationFluentValidationTests.cs +++ b/test/Ocelot.UnitTests/Configuration/ConfigurationFluentValidationTests.cs @@ -46,9 +46,11 @@ namespace Ocelot.UnitTests.Configuration .Then(x => x.ThenTheResultIsNotValid()) .Then(x => x.ThenTheErrorIs()) .And(x => x.ThenTheErrorMessageAtPositionIs(0, "Downstream Path Template http://www.bbc.co.uk/api/products/{productId} doesnt start with forward slash")) - .And(x => x.ThenTheErrorMessageAtPositionIs(1, "Upstream Path Template http://asdf.com doesnt start with forward slash")) - .And(x => x.ThenTheErrorMessageAtPositionIs(2, "Downstream Path Template http://www.bbc.co.uk/api/products/{productId} contains scheme")) - .And(x => x.ThenTheErrorMessageAtPositionIs(3, "Upstream Path Template http://asdf.com contains scheme")) + .And(x => x.ThenTheErrorMessageAtPositionIs(1, "Upstream Path Template http://asdf.com contains double forward slash, Ocelot does not support this at the moment. Please raise an issue in GitHib if you need this feature.")) + .And(x => x.ThenTheErrorMessageAtPositionIs(2, "Downstream Path Template http://www.bbc.co.uk/api/products/{productId} contains double forward slash, Ocelot does not support this at the moment. Please raise an issue in GitHib if you need this feature.")) + .And(x => x.ThenTheErrorMessageAtPositionIs(3, "Upstream Path Template http://asdf.com doesnt start with forward slash")) + .And(x => x.ThenTheErrorMessageAtPositionIs(4, "Downstream Path Template http://www.bbc.co.uk/api/products/{productId} contains scheme")) + .And(x => x.ThenTheErrorMessageAtPositionIs(5, "Upstream Path Template http://asdf.com contains scheme")) .BDDfy(); } @@ -112,6 +114,50 @@ namespace Ocelot.UnitTests.Configuration .BDDfy(); } + [Fact] + public void configuration_is_invalid_if_upstream_url_contains_forward_slash_then_another_forward_slash() + { + this.Given(x => x.GivenAConfiguration(new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/api/products/", + UpstreamPathTemplate = "//api/prod/", + DownstreamHost = "bbc.co.uk", + DownstreamPort = 80 + } + } + })) + .When(x => x.WhenIValidateTheConfiguration()) + .Then(x => x.ThenTheResultIsNotValid()) + .And(x => x.ThenTheErrorMessageAtPositionIs(0, "Upstream Path Template //api/prod/ contains double forward slash, Ocelot does not support this at the moment. Please raise an issue in GitHib if you need this feature.")) + .BDDfy(); + } + + [Fact] + public void configuration_is_invalid_if_downstream_url_contains_forward_slash_then_another_forward_slash() + { + this.Given(x => x.GivenAConfiguration(new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "//api/products/", + UpstreamPathTemplate = "/api/prod/", + DownstreamHost = "bbc.co.uk", + DownstreamPort = 80 + } + } + })) + .When(x => x.WhenIValidateTheConfiguration()) + .Then(x => x.ThenTheResultIsNotValid()) + .And(x => x.ThenTheErrorMessageAtPositionIs(0, "Downstream Path Template //api/products/ contains double forward slash, Ocelot does not support this at the moment. Please raise an issue in GitHib if you need this feature.")) + .BDDfy(); + } + [Fact] public void configuration_is_valid_with_valid_authentication_provider() { diff --git a/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionTests.cs index 936a57e8..be1bd108 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionTests.cs @@ -47,6 +47,53 @@ namespace Ocelot.UnitTests.LoadBalancer Task.WaitAll(tasks); } + [Fact] + public void should_handle_service_returning_to_available() + { + var serviceName = "products"; + + var availableServices = new List + { + new Service(serviceName, new HostAndPort("127.0.0.1", 80), string.Empty, string.Empty, new string[0]), + new Service(serviceName, new HostAndPort("127.0.0.2", 80), string.Empty, string.Empty, new string[0]), + }; + + _leastConnection = new LeastConnection(() => Task.FromResult(availableServices), serviceName); + + var hostAndPortOne = _leastConnection.Lease().Result; + hostAndPortOne.Data.DownstreamHost.ShouldBe("127.0.0.1"); + var hostAndPortTwo = _leastConnection.Lease().Result; + hostAndPortTwo.Data.DownstreamHost.ShouldBe("127.0.0.2"); + _leastConnection.Release(hostAndPortOne.Data); + _leastConnection.Release(hostAndPortTwo.Data); + + availableServices = new List + { + new Service(serviceName, new HostAndPort("127.0.0.1", 80), string.Empty, string.Empty, new string[0]), + }; + + hostAndPortOne = _leastConnection.Lease().Result; + hostAndPortOne.Data.DownstreamHost.ShouldBe("127.0.0.1"); + hostAndPortTwo = _leastConnection.Lease().Result; + hostAndPortTwo.Data.DownstreamHost.ShouldBe("127.0.0.1"); + _leastConnection.Release(hostAndPortOne.Data); + _leastConnection.Release(hostAndPortTwo.Data); + + availableServices = new List + { + new Service(serviceName, new HostAndPort("127.0.0.1", 80), string.Empty, string.Empty, new string[0]), + new Service(serviceName, new HostAndPort("127.0.0.2", 80), string.Empty, string.Empty, new string[0]), + }; + + hostAndPortOne = _leastConnection.Lease().Result; + hostAndPortOne.Data.DownstreamHost.ShouldBe("127.0.0.1"); + hostAndPortTwo = _leastConnection.Lease().Result; + hostAndPortTwo.Data.DownstreamHost.ShouldBe("127.0.0.2"); + _leastConnection.Release(hostAndPortOne.Data); + _leastConnection.Release(hostAndPortTwo.Data); + + } + private async Task LeaseDelayAndRelease() { var hostAndPort = await _leastConnection.Lease();