make rate limiting whitelist a function so users can override with dynamic behaviour

* Make rate-limiting client whitelist dynamic
* Refactor `RateLimitOptions.ClientWhiteList`
* Fix typo in variable `enbleRateLimiting`
* Fix case in variable `clientIdheader`
author Taiwo Otubamowo <totubamowo@deloitte.co.uk>

* fix 1045

* #492 log 500 with error log level, acceptance test, unit test

* #492 minor changes

* initial commit for new feature #1077

allow to limit the number of concurrent tcp connection to a downstream service

* protect code against value not in accurate range

add unit test

* Do not crash host on Dispose

* Add test

* Pin GitVersion.CommandLine package version

* #683 validate if there are duplicated placeholders in UpstreamPathTemplate

* Use registered scheme from Eureka (#1087)

* extra test

* very brief mention MaxConnectionsPerServer in docs

* build develop like a PR

* more docs

* test

Co-authored-by: Taiwo O. <44668623+totubamowo@users.noreply.github.com>
Co-authored-by: Catcher Wong <catcher_hwq@outlook.com>
Co-authored-by: jlukawska <56401969+jlukawska@users.noreply.github.com>
Co-authored-by: buretjph <58700930+buretjph@users.noreply.github.com>
Co-authored-by: Jonathan Mezach <jonathanmezach@gmail.com>
Co-authored-by: 彭伟 <pengweiqhca@sina.com>

Ocelot.sln

@@ -9,23 +9,13 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
 	ProjectSection(SolutionItems) = preProject
 		.dockerignore = .dockerignore
 		.gitignore = .gitignore
-		build-and-release-unstable.ps1 = build-and-release-unstable.ps1
-		build-and-run-tests.ps1 = build-and-run-tests.ps1
 		build.cake = build.cake
 		build.ps1 = build.ps1
 		codeanalysis.ruleset = codeanalysis.ruleset
-		docker-compose.yaml = docker-compose.yaml
-		Dockerfile = Dockerfile
 		GitVersion.yml = GitVersion.yml
-		global.json = global.json
 		LICENSE.md = LICENSE.md
 		README.md = README.md
-		release.ps1 = release.ps1
 		ReleaseNotes.md = ReleaseNotes.md
-		run-acceptance-tests.ps1 = run-acceptance-tests.ps1
-		run-benchmarks.ps1 = run-benchmarks.ps1
-		run-unit-tests.ps1 = run-unit-tests.ps1
-		version.ps1 = version.ps1
 	EndProjectSection
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{5B401523-36DA-4491-B73A-7590A26E420B}"

samples/OcelotBasic/Properties/launchSettings.json

@@ -0,0 +1,27 @@
+{
+  "iisSettings": {
+    "windowsAuthentication": false,
+    "anonymousAuthentication": true,
+    "iisExpress": {
+      "applicationUrl": "http://localhost:55029/",
+      "sslPort": 44390
+    }
+  },
+  "profiles": {
+    "IIS Express": {
+      "commandName": "IISExpress",
+      "launchBrowser": true,
+      "environmentVariables": {
+        "ASPNETCORE_ENVIRONMENT": "Development"
+      }
+    },
+    "OcelotBasic": {
+      "commandName": "Project",
+      "launchBrowser": true,
+      "environmentVariables": {
+        "ASPNETCORE_ENVIRONMENT": "Development"
+      },
+      "applicationUrl": "https://localhost:5001;http://localhost:5000"
+    }
+  }
+}

src/Ocelot/Configuration/Builder/RateLimitOptionsBuilder.cs

@@ -1,4 +1,5 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 
 namespace Ocelot.Configuration.Builder
 {
@@ -7,6 +8,7 @@ namespace Ocelot.Configuration.Builder
         private bool _enableRateLimiting;
         private string _clientIdHeader;
         private List<string> _clientWhitelist;
+        private Func<List<string>> _getClientWhitelist;
         private bool _disableRateLimitHeaders;
         private string _quotaExceededMessage;
         private string _rateLimitCounterPrefix;
@@ -19,15 +21,15 @@ namespace Ocelot.Configuration.Builder
             return this;
         }
 
-        public RateLimitOptionsBuilder WithClientIdHeader(string clientIdheader)
+        public RateLimitOptionsBuilder WithClientIdHeader(string clientIdHeader)
         {
-            _clientIdHeader = clientIdheader;
+            _clientIdHeader = clientIdHeader;
             return this;
         }
 
-        public RateLimitOptionsBuilder WithClientWhiteList(List<string> clientWhitelist)
+        public RateLimitOptionsBuilder WithClientWhiteList(Func<List<string>> getClientWhitelist)
         {
-            _clientWhitelist = clientWhitelist;
+            _getClientWhitelist = getClientWhitelist;
             return this;
         }
 
@@ -63,7 +65,7 @@ namespace Ocelot.Configuration.Builder
 
         public RateLimitOptions Build()
         {
-            return new RateLimitOptions(_enableRateLimiting, _clientIdHeader, _clientWhitelist,
+            return new RateLimitOptions(_enableRateLimiting, _clientIdHeader, _getClientWhitelist,
                 _disableRateLimitHeaders, _quotaExceededMessage, _rateLimitCounterPrefix,
                 _rateLimitRule, _httpStatusCode);
         }

src/Ocelot/Configuration/Creator/RateLimitOptionsCreator.cs

@@ -11,7 +11,7 @@ namespace Ocelot.Configuration.Creator
             {
                 return new RateLimitOptionsBuilder()
                     .WithClientIdHeader(globalConfiguration.RateLimitOptions.ClientIdHeader)
-                    .WithClientWhiteList(fileRateLimitRule.ClientWhitelist)
+                    .WithClientWhiteList(() => fileRateLimitRule.ClientWhitelist)
                     .WithDisableRateLimitHeaders(globalConfiguration.RateLimitOptions.DisableRateLimitHeaders)
                     .WithEnableRateLimiting(fileRateLimitRule.EnableRateLimiting)
                     .WithHttpStatusCode(globalConfiguration.RateLimitOptions.HttpStatusCode)

src/Ocelot/Configuration/RateLimitOptions.cs

@@ -1,4 +1,5 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 
 namespace Ocelot.Configuration
 {
@@ -7,12 +8,14 @@ namespace Ocelot.Configuration
     /// </summary>
     public class RateLimitOptions
     {
-        public RateLimitOptions(bool enbleRateLimiting, string clientIdHeader, List<string> clientWhitelist, bool disableRateLimitHeaders,
+        private readonly Func<List<string>> _getClientWhitelist;
+
+        public RateLimitOptions(bool enableRateLimiting, string clientIdHeader, Func<List<string>> getClientWhitelist, bool disableRateLimitHeaders,
             string quotaExceededMessage, string rateLimitCounterPrefix, RateLimitRule rateLimitRule, int httpStatusCode)
         {
-            EnableRateLimiting = enbleRateLimiting;
+            EnableRateLimiting = enableRateLimiting;
             ClientIdHeader = clientIdHeader;
-            ClientWhitelist = clientWhitelist ?? new List<string>();
+            _getClientWhitelist = getClientWhitelist;
             DisableRateLimitHeaders = disableRateLimitHeaders;
             QuotaExceededMessage = quotaExceededMessage;
             RateLimitCounterPrefix = rateLimitCounterPrefix;
@@ -22,7 +25,10 @@ namespace Ocelot.Configuration
 
         public RateLimitRule RateLimitRule { get; private set; }
 
-        public List<string> ClientWhitelist { get; private set; }
+        /// <summary>
+        /// Gets the list of white listed clients
+        /// </summary>
+        public List<string> ClientWhitelist { get => _getClientWhitelist(); }
 
         /// <summary>
         /// Gets or sets the HTTP header that holds the client identifier, by default is X-ClientId

src/Ocelot/Requester/HttpExeptionToErrorMapper.cs

@@ -1,6 +1,6 @@
 namespace Ocelot.Requester
 {
-    using Errors;
+    using Ocelot.Errors;
     using Microsoft.Extensions.DependencyInjection;
     using System;
     using System.Collections.Generic;
@@ -23,7 +23,7 @@ namespace Ocelot.Requester
                 return _mappers[type](exception);
             }
 
-            if (type == typeof(OperationCanceledException))
+            if (type == typeof(OperationCanceledException) || type.IsSubclassOf(typeof(OperationCanceledException)))
             {
                 return new RequestCanceledError(exception.Message);
             }

src/Ocelot/Requester/Middleware/HttpRequesterMiddleware.cs

@@ -1,6 +1,9 @@
+using System.Net;
+using System.Net.Http;
 using Ocelot.Logging;
 using Ocelot.Middleware;
 using System.Threading.Tasks;
+using Ocelot.Responses;
 
 namespace Ocelot.Requester.Middleware
 {
@@ -22,6 +25,8 @@ namespace Ocelot.Requester.Middleware
         {
             var response = await _requester.GetResponse(context);
 
+            CreateLogBasedOnResponse(response);
+
             if (response.IsError)
             {
                 Logger.LogDebug("IHttpRequester returned an error, setting pipeline error");
@@ -36,5 +41,19 @@ namespace Ocelot.Requester.Middleware
 
             await _next.Invoke(context);
         }
+
+        private void CreateLogBasedOnResponse(Response<HttpResponseMessage> response)
+        {
+            if (response.Data?.StatusCode <= HttpStatusCode.BadRequest)
+            {
+                Logger.LogInformation(
+                    $"{(int)response.Data.StatusCode} ({response.Data.ReasonPhrase}) status code, request uri: {response.Data.RequestMessage?.RequestUri}");
+            } 
+            else if (response.Data?.StatusCode >= HttpStatusCode.BadRequest)
+            {
+                Logger.LogWarning(
+                    $"{(int) response.Data.StatusCode} ({response.Data.ReasonPhrase}) status code, request uri: {response.Data.RequestMessage?.RequestUri}");
+            }
+        }
     }
 }

test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <VersionPrefix>0.0.0-dev</VersionPrefix>
     <TargetFramework>netcoreapp3.1</TargetFramework>
@@ -40,6 +40,7 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.AspNetCore.TestHost" Version="3.0.0" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.3.0" />
+    <PackageReference Include="Moq" Version="4.13.0" />
     <PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.66">
       <PrivateAssets>all</PrivateAssets>
     </PackageReference>

test/Ocelot.AcceptanceTests/ResponseCodeTests.cs

@@ -34,7 +34,7 @@ namespace Ocelot.AcceptanceTests
                                 new FileHostAndPort
                                 {
                                     Host = "localhost",
-                                    Port = 50092,
+                                    Port = 51092,
                                 }
                             },
                             UpstreamPathTemplate = "/{everything}",
@@ -43,7 +43,7 @@ namespace Ocelot.AcceptanceTests
                     }
             };
 
-            this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:50092", "/inline.132.bundle.js", 304))
+            this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:51092", "/inline.132.bundle.js", 304))
                 .And(x => _steps.GivenThereIsAConfiguration(configuration))
                 .And(x => _steps.GivenOcelotIsRunning())
                 .When(x => _steps.WhenIGetUrlOnTheApiGateway("/inline.132.bundle.js"))

test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs

@@ -21,6 +21,7 @@
         [Fact]
         public void should_return_internal_server_error_if_downstream_service_returns_internal_server_error()
         {
+
             var configuration = new FileConfiguration
             {
                 ReRoutes = new List<FileReRoute>
@@ -51,6 +52,39 @@
                 .BDDfy();
         }
 
+        [Fact]
+        public void should_log_warning_if_downstream_service_returns_internal_server_error()
+        {
+            var configuration = new FileConfiguration
+            {
+                ReRoutes = new List<FileReRoute>
+                    {
+                        new FileReRoute
+                        {
+                            DownstreamPathTemplate = "/",
+                            UpstreamPathTemplate = "/",
+                            UpstreamHttpMethod = new List<string> { "Get" },
+                            DownstreamHostAndPorts = new List<FileHostAndPort>
+                            {
+                                new FileHostAndPort
+                                {
+                                    Host = "localhost",
+                                    Port = 53876,
+                                },
+                            },
+                            DownstreamScheme = "http",
+                        },
+                    },
+            };
+
+            this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:53876"))
+                .And(x => _steps.GivenThereIsAConfiguration(configuration))
+                .And(x => _steps.GivenOcelotIsRunningWithLogger())
+                .When(x => _steps.WhenIGetUrlOnTheApiGateway("/"))
+                .Then(x => _steps.ThenWarningShouldBeLogged())
+                .BDDfy();
+        }
+
         private void GivenThereIsAServiceRunningOn(string url)
         {
             _serviceHandler.GivenThereIsAServiceRunningOn(url, context => throw new Exception("BLAMMMM"));

test/Ocelot.AcceptanceTests/Steps.cs

@@ -10,12 +10,14 @@
     using Microsoft.Extensions.Configuration;
     using Microsoft.Extensions.DependencyInjection;
     using Microsoft.Extensions.Logging;
+    using Moq;
     using Newtonsoft.Json;
     using Ocelot.Cache.CacheManager;
     using Ocelot.Configuration.Creator;
     using Ocelot.Configuration.File;
     using Ocelot.DependencyInjection;
     using Ocelot.Infrastructure;
+    using Ocelot.Logging;
     using Ocelot.Middleware;
     using Ocelot.Middleware.Multiplexer;
     using Ocelot.Provider.Consul;
@@ -1120,5 +1122,60 @@
 
             _ocelotClient = _ocelotServer.CreateClient();
         }
+
+        public void GivenOcelotIsRunningWithLogger()
+        {
+            _webHostBuilder = new WebHostBuilder();
+
+            _webHostBuilder
+                .ConfigureAppConfiguration((hostingContext, config) =>
+                {
+                    config.SetBasePath(hostingContext.HostingEnvironment.ContentRootPath);
+                    var env = hostingContext.HostingEnvironment;
+                    config.AddJsonFile("appsettings.json", optional: true, reloadOnChange: false)
+                        .AddJsonFile($"appsettings.{env.EnvironmentName}.json", optional: true, reloadOnChange: false);
+                    config.AddJsonFile("ocelot.json", false, false);
+                    config.AddEnvironmentVariables();
+                })
+                .ConfigureServices(s =>
+                {
+                    s.AddOcelot();
+                    s.AddSingleton<IOcelotLoggerFactory, MockLoggerFactory>();
+                })
+                .Configure(app =>
+                {
+                    app.UseOcelot().Wait();
+                });
+
+            _ocelotServer = new TestServer(_webHostBuilder);
+
+            _ocelotClient = _ocelotServer.CreateClient();
+        }
+
+        public void ThenWarningShouldBeLogged()
+        {
+            MockLoggerFactory loggerFactory = (MockLoggerFactory)_ocelotServer.Host.Services.GetService<IOcelotLoggerFactory>();
+            loggerFactory.Verify();
+        }
+
+        internal class MockLoggerFactory : IOcelotLoggerFactory
+        {
+            private Mock<IOcelotLogger> _logger;
+
+            public IOcelotLogger CreateLogger<T>()
+            {
+                if (_logger == null)
+                {
+                    _logger = new Mock<IOcelotLogger>();
+                    _logger.Setup(x => x.LogWarning(It.IsAny<string>())).Verifiable();
+                }
+                return _logger.Object;
+            }
+
+            public void Verify()
+            {
+                _logger.Verify(x => x.LogWarning(It.IsAny<string>()), Times.Once);
+            }
+        }
     }
 }

test/Ocelot.UnitTests/Configuration/RateLimitOptionsCreatorTests.cs

@@ -50,7 +50,7 @@ namespace Ocelot.UnitTests.Configuration
             };
             var expected = new RateLimitOptionsBuilder()
                 .WithClientIdHeader("ClientIdHeader")
-                .WithClientWhiteList(fileReRoute.RateLimitOptions.ClientWhitelist)
+                .WithClientWhiteList(() => fileReRoute.RateLimitOptions.ClientWhitelist)
                 .WithDisableRateLimitHeaders(true)
                 .WithEnableRateLimiting(true)
                 .WithHttpStatusCode(200)

test/Ocelot.UnitTests/RateLimit/ClientRateLimitMiddlewareTests.cs

@@ -54,7 +54,7 @@ namespace Ocelot.UnitTests.RateLimit
 
             var downstreamReRoute = new DownstreamReRouteBuilder()
                 .WithEnableRateLimiting(true)
-                .WithRateLimitOptions(new RateLimitOptions(true, "ClientId", new List<string>(), false, "", "", new RateLimitRule("1s", 100, 3), 429))
+                .WithRateLimitOptions(new RateLimitOptions(true, "ClientId", () => new List<string>(), false, "", "", new RateLimitRule("1s", 100, 3), 429))
                 .WithUpstreamHttpMethod(new List<string> { "Get" })
                 .WithUpstreamPathTemplate(upstreamTemplate)
                 .Build();
@@ -82,7 +82,7 @@ namespace Ocelot.UnitTests.RateLimit
                      .WithDownstreamReRoute(new DownstreamReRouteBuilder()
                          .WithEnableRateLimiting(true)
                          .WithRateLimitOptions(
-                             new Ocelot.Configuration.RateLimitOptions(true, "ClientId", new List<string>() { "ocelotclient2" }, false, "", "", new RateLimitRule("1s", 100, 3), 429))
+                             new Ocelot.Configuration.RateLimitOptions(true, "ClientId", () => new List<string>() { "ocelotclient2" }, false, "", "", new RateLimitRule("1s", 100, 3), 429))
                          .WithUpstreamHttpMethod(new List<string> { "Get" })
                          .Build())
                      .WithUpstreamHttpMethod(new List<string> { "Get" })

test/Ocelot.UnitTests/Requester/HttpExeptionToErrorMapperTests.cs

@@ -38,6 +38,14 @@
             error.ShouldBeOfType<RequestCanceledError>();
         }
 
+        [Fact]
+        public void should_return_request_canceled_for_subtype()
+        {
+            var error = _mapper.Map(new SomeException());
+
+            error.ShouldBeOfType<RequestCanceledError>();
+        }
+
         [Fact]
         public void should_return_error_from_mapper()
         {
@@ -56,5 +64,8 @@
 
             error.ShouldBeOfType<AnyError>();
         }
+
+        private class SomeException : OperationCanceledException
+        { }
     }
 }

test/Ocelot.UnitTests/Requester/HttpRequesterMiddlewareTests.cs

@@ -41,9 +41,10 @@ namespace Ocelot.UnitTests.Requester
         public void should_call_services_correctly()
         {
             this.Given(x => x.GivenTheRequestIs())
-                .And(x => x.GivenTheRequesterReturns(new OkResponse<HttpResponseMessage>(new HttpResponseMessage())))
+                .And(x => x.GivenTheRequesterReturns(new OkResponse<HttpResponseMessage>(new HttpResponseMessage(System.Net.HttpStatusCode.OK))))
                 .When(x => x.WhenICallTheMiddleware())
                 .Then(x => x.ThenTheDownstreamResponseIsSet())
+                .Then(x => InformationIsLogged())
                 .BDDfy();
         }
 
@@ -57,6 +58,17 @@ namespace Ocelot.UnitTests.Requester
                 .BDDfy();
         }
 
+        [Fact]
+        public void should_log_downstream_internal_server_error()
+        {
+            this.Given(x => x.GivenTheRequestIs())
+                    .And(x => x.GivenTheRequesterReturns(
+                        new OkResponse<HttpResponseMessage>(new HttpResponseMessage(System.Net.HttpStatusCode.InternalServerError))))
+                .When(x => x.WhenICallTheMiddleware())
+                .Then(x => x.WarningIsLogged())
+                .BDDfy();
+        }
+
         private void ThenTheErrorIsSet()
         {
             _downstreamContext.IsError.ShouldBeTrue();
@@ -98,5 +110,23 @@ namespace Ocelot.UnitTests.Requester
             _downstreamContext.DownstreamResponse.Content.ShouldBe(_response.Data.Content);
             _downstreamContext.DownstreamResponse.StatusCode.ShouldBe(_response.Data.StatusCode);
         }
+
+        private void WarningIsLogged()
+        {
+            _logger.Verify(
+                x => x.LogWarning(                 
+                    It.IsAny<string>()
+                   ),
+                Times.Once);
+        }
+
+        private void InformationIsLogged()
+        {
+            _logger.Verify(
+                x => x.LogInformation(
+                    It.IsAny<string>()
+                ),
+                Times.Once);
+        }
     }
 }