From d7207e18e72f95f0431cf6964bebfb85085123e0 Mon Sep 17 00:00:00 2001 From: Zbynek Novotny Date: Fri, 28 Jul 2023 22:46:35 +0200 Subject: [PATCH 01/13] Implementation of support for routing based on values in designated upstream request headers. --- docs/features/routing.rst | 33 ++++ .../Configuration/Builder/RouteBuilder.cs | 14 +- .../IUpstreamHeaderRoutingOptionsCreator.cs | 9 + .../Configuration/Creator/RoutesCreator.cs | 11 +- .../UpstreamHeaderRoutingOptionsCreator.cs | 25 +++ src/Ocelot/Configuration/File/FileRoute.cs | 25 +-- .../File/FileUpstreamHeaderRoutingOptions.cs | 17 ++ src/Ocelot/Configuration/Route.cs | 5 +- .../UpstreamHeaderRoutingOptions.cs | 19 ++ .../UpstreamHeaderRoutingTriggerMode.cs | 8 + .../Configuration/UpstreamRoutingHeaders.cs | 70 +++++++ .../DependencyInjection/OcelotBuilder.cs | 1 + .../Finder/DownstreamRouteCreator.cs | 19 +- .../Finder/DownstreamRouteFinder.cs | 43 ++++- .../Finder/IDownstreamRouteProvider.cs | 11 +- .../DownstreamRouteFinderMiddleware.cs | 10 +- .../Configuration/RoutesCreatorTests.cs | 17 +- ...pstreamHeaderRoutingOptionsCreatorTests.cs | 71 +++++++ .../UpstreamRoutingHeadersTests.cs | 142 ++++++++++++++ .../DownstreamRouteCreatorTests.cs | 21 ++- .../DownstreamRouteFinderMiddlewareTests.cs | 2 +- .../DownstreamRouteFinderTests.cs | 173 +++++++++++++++++- 22 files changed, 687 insertions(+), 59 deletions(-) create mode 100644 src/Ocelot/Configuration/Creator/IUpstreamHeaderRoutingOptionsCreator.cs create mode 100644 src/Ocelot/Configuration/Creator/UpstreamHeaderRoutingOptionsCreator.cs create mode 100644 src/Ocelot/Configuration/File/FileUpstreamHeaderRoutingOptions.cs create mode 100644 src/Ocelot/Configuration/UpstreamHeaderRoutingOptions.cs create mode 100644 src/Ocelot/Configuration/UpstreamHeaderRoutingTriggerMode.cs create mode 100644 src/Ocelot/Configuration/UpstreamRoutingHeaders.cs create mode 100644 test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs create mode 100644 test/Ocelot.UnitTests/Configuration/UpstreamRoutingHeadersTests.cs diff --git a/docs/features/routing.rst b/docs/features/routing.rst index 6de03d8e2..546319db9 100644 --- a/docs/features/routing.rst +++ b/docs/features/routing.rst @@ -292,6 +292,39 @@ Here are two user scenarios. So, both ``{userId}`` placeholder and ``userId`` parameter **names are the same**! Finally, the ``userId`` parameter is removed. + +Upstream header-based routing +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +This feature was requested in `issue 360 `_ and `issue 624 `_. + +Ocelot allows you to define a Route with upstream headers, each of which may define a set of accepted values. If a Route has a set of upstream headers defined in it, it will no longer match a request's upstream path based solely on upstream path template. The request must also contain one or more headers required by the Route for a match. + +A sample configuration might look like the following: + +.. code-block:: json + + { + "Routes": [ + { + "UpstreamHeaderRoutingOptions": { + "Headers": { + "X-API-Version": [ "1", "2" ], + "X-Tennant-Id": [ "tennantId" ] + }, + "TriggerOn": "all" + } + } + ] + } + +The ``UpstreamHeaderRoutingOptions`` block defines two attributes -- the ``Headers`` block and the ``TriggerOn`` attribute. The ``Headers`` attribute defines required header names as keys and lists of acceptable header values as values. During route matching, both header names and values are matched in *case insensitive* manner. Please note that if a header has more than one acceptable value configured, presence of any of those values in a request is sufficient for a header to be a match. + +The second attribute, ``TriggerOn``, defines how the route finder will determine whether a particular header configuration in a request matches a Route's header configuration. The attribute accepts two values: + +* ``"Any"`` causes the route finder to match a Route if any value of any configured header is present in a request +* ``"All"`` causes the route finder to match a Route only if any value of *all* configured headers is present in a request + .. _routing-security-options: Security Options [#f3]_ diff --git a/src/Ocelot/Configuration/Builder/RouteBuilder.cs b/src/Ocelot/Configuration/Builder/RouteBuilder.cs index 8e7614a9c..287c1038f 100644 --- a/src/Ocelot/Configuration/Builder/RouteBuilder.cs +++ b/src/Ocelot/Configuration/Builder/RouteBuilder.cs @@ -1,5 +1,5 @@ -using Ocelot.Configuration.File; -using Ocelot.Values; +using Ocelot.Configuration.File; +using Ocelot.Values; namespace Ocelot.Configuration.Builder { @@ -11,6 +11,7 @@ public class RouteBuilder private List _downstreamRoutes; private List _downstreamRoutesConfig; private string _aggregator; + private UpstreamHeaderRoutingOptions _upstreamHeaderRoutingOptions; public RouteBuilder() { @@ -60,6 +61,12 @@ public RouteBuilder WithAggregator(string aggregator) return this; } + public RouteBuilder WithUpstreamHeaderRoutingOptions(UpstreamHeaderRoutingOptions routingOptions) + { + _upstreamHeaderRoutingOptions = routingOptions; + return this; + } + public Route Build() { return new Route( @@ -68,7 +75,8 @@ public Route Build() _upstreamHttpMethod, _upstreamTemplatePattern, _upstreamHost, - _aggregator + _aggregator, + _upstreamHeaderRoutingOptions ); } } diff --git a/src/Ocelot/Configuration/Creator/IUpstreamHeaderRoutingOptionsCreator.cs b/src/Ocelot/Configuration/Creator/IUpstreamHeaderRoutingOptionsCreator.cs new file mode 100644 index 000000000..8c965d08f --- /dev/null +++ b/src/Ocelot/Configuration/Creator/IUpstreamHeaderRoutingOptionsCreator.cs @@ -0,0 +1,9 @@ +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public interface IUpstreamHeaderRoutingOptionsCreator + { + UpstreamHeaderRoutingOptions Create(FileUpstreamHeaderRoutingOptions options); + } +} diff --git a/src/Ocelot/Configuration/Creator/RoutesCreator.cs b/src/Ocelot/Configuration/Creator/RoutesCreator.cs index 8c1f1de63..e74a0eedf 100644 --- a/src/Ocelot/Configuration/Creator/RoutesCreator.cs +++ b/src/Ocelot/Configuration/Creator/RoutesCreator.cs @@ -1,5 +1,5 @@ -using Ocelot.Cache; -using Ocelot.Configuration.Builder; +using Ocelot.Cache; +using Ocelot.Configuration.Builder; using Ocelot.Configuration.File; namespace Ocelot.Configuration.Creator @@ -21,6 +21,7 @@ public class RoutesCreator : IRoutesCreator private readonly IRouteKeyCreator _routeKeyCreator; private readonly ISecurityOptionsCreator _securityOptionsCreator; private readonly IVersionCreator _versionCreator; + private readonly IUpstreamHeaderRoutingOptionsCreator _upstreamHeaderRoutingOptionsCreator; public RoutesCreator( IClaimsToThingCreator claimsToThingCreator, @@ -37,7 +38,8 @@ public class RoutesCreator : IRoutesCreator ILoadBalancerOptionsCreator loadBalancerOptionsCreator, IRouteKeyCreator routeKeyCreator, ISecurityOptionsCreator securityOptionsCreator, - IVersionCreator versionCreator + IVersionCreator versionCreator, + IUpstreamHeaderRoutingOptionsCreator upstreamHeaderRoutingOptionsCreator ) { _routeKeyCreator = routeKeyCreator; @@ -56,6 +58,7 @@ IVersionCreator versionCreator _loadBalancerOptionsCreator = loadBalancerOptionsCreator; _securityOptionsCreator = securityOptionsCreator; _versionCreator = versionCreator; + _upstreamHeaderRoutingOptionsCreator = upstreamHeaderRoutingOptionsCreator; } public List Create(FileConfiguration fileConfiguration) @@ -151,12 +154,14 @@ private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConf private Route SetUpRoute(FileRoute fileRoute, DownstreamRoute downstreamRoutes) { var upstreamTemplatePattern = _upstreamTemplatePatternCreator.Create(fileRoute); + var upstreamHeaderRoutingOptions = _upstreamHeaderRoutingOptionsCreator.Create(fileRoute.UpstreamHeaderRoutingOptions); var route = new RouteBuilder() .WithUpstreamHttpMethod(fileRoute.UpstreamHttpMethod) .WithUpstreamPathTemplate(upstreamTemplatePattern) .WithDownstreamRoute(downstreamRoutes) .WithUpstreamHost(fileRoute.UpstreamHost) + .WithUpstreamHeaderRoutingOptions(upstreamHeaderRoutingOptions) .Build(); return route; diff --git a/src/Ocelot/Configuration/Creator/UpstreamHeaderRoutingOptionsCreator.cs b/src/Ocelot/Configuration/Creator/UpstreamHeaderRoutingOptionsCreator.cs new file mode 100644 index 000000000..730d0f654 --- /dev/null +++ b/src/Ocelot/Configuration/Creator/UpstreamHeaderRoutingOptionsCreator.cs @@ -0,0 +1,25 @@ +using System; +using System.Linq; +using System.Collections.Generic; +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public class UpstreamHeaderRoutingOptionsCreator : IUpstreamHeaderRoutingOptionsCreator + { + public UpstreamHeaderRoutingOptions Create(FileUpstreamHeaderRoutingOptions options) + { + UpstreamHeaderRoutingTriggerMode mode = UpstreamHeaderRoutingTriggerMode.Any; + if (options.TriggerOn.Length > 0) + { + mode = Enum.Parse(options.TriggerOn, true); + } + + Dictionary> headers = options.Headers.ToDictionary( + kv => kv.Key.ToLowerInvariant(), + kv => new HashSet(kv.Value.Select(v => v.ToLowerInvariant()))); + + return new UpstreamHeaderRoutingOptions(headers, mode); + } + } +} diff --git a/src/Ocelot/Configuration/File/FileRoute.cs b/src/Ocelot/Configuration/File/FileRoute.cs index 5823113ad..d055ed424 100644 --- a/src/Ocelot/Configuration/File/FileRoute.cs +++ b/src/Ocelot/Configuration/File/FileRoute.cs @@ -1,4 +1,4 @@ -namespace Ocelot.Configuration.File +namespace Ocelot.Configuration.File { public class FileRoute : IRoute, ICloneable { @@ -21,19 +21,20 @@ public FileRoute() RouteClaimsRequirement = new Dictionary(); SecurityOptions = new FileSecurityOptions(); UpstreamHeaderTransform = new Dictionary(); + UpstreamHeaderRoutingOptions = new FileUpstreamHeaderRoutingOptions(); UpstreamHttpMethod = new List(); - } - + } + public FileRoute(FileRoute from) { DeepCopy(from, this); } - public Dictionary AddClaimsToRequest { get; set; } + public Dictionary AddClaimsToRequest { get; set; } public Dictionary AddHeadersToRequest { get; set; } - public Dictionary AddQueriesToRequest { get; set; } + public Dictionary AddQueriesToRequest { get; set; } public FileAuthenticationOptions AuthenticationOptions { get; set; } - public Dictionary ChangeDownstreamPathTemplate { get; set; } + public Dictionary ChangeDownstreamPathTemplate { get; set; } public bool DangerousAcceptAnyServerCertificateValidator { get; set; } public List DelegatingHandlers { get; set; } public Dictionary DownstreamHeaderTransform { get; set; } @@ -42,7 +43,7 @@ public FileRoute(FileRoute from) public string DownstreamHttpVersion { get; set; } public string DownstreamPathTemplate { get; set; } public string DownstreamScheme { get; set; } - public FileCacheOptions FileCacheOptions { get; set; } + public FileCacheOptions FileCacheOptions { get; set; } public FileHttpHandlerOptions HttpHandlerOptions { get; set; } public string Key { get; set; } public FileLoadBalancerOptions LoadBalancerOptions { get; set; } @@ -51,13 +52,14 @@ public FileRoute(FileRoute from) public FileRateLimitRule RateLimitOptions { get; set; } public string RequestIdKey { get; set; } public Dictionary RouteClaimsRequirement { get; set; } - public bool RouteIsCaseSensitive { get; set; } + public bool RouteIsCaseSensitive { get; set; } public FileSecurityOptions SecurityOptions { get; set; } - public string ServiceName { get; set; } - public string ServiceNamespace { get; set; } + public string ServiceName { get; set; } + public string ServiceNamespace { get; set; } public int Timeout { get; set; } public Dictionary UpstreamHeaderTransform { get; set; } - public string UpstreamHost { get; set; } + public FileUpstreamHeaderRoutingOptions UpstreamHeaderRoutingOptions { get; set; } + public string UpstreamHost { get; set; } public List UpstreamHttpMethod { get; set; } public string UpstreamPathTemplate { get; set; } @@ -102,6 +104,7 @@ public static void DeepCopy(FileRoute from, FileRoute to) to.ServiceNamespace = from.ServiceNamespace; to.Timeout = from.Timeout; to.UpstreamHeaderTransform = new(from.UpstreamHeaderTransform); + to.UpstreamHeaderRoutingOptions = from.UpstreamHeaderRoutingOptions; to.UpstreamHost = from.UpstreamHost; to.UpstreamHttpMethod = new(from.UpstreamHttpMethod); to.UpstreamPathTemplate = from.UpstreamPathTemplate; diff --git a/src/Ocelot/Configuration/File/FileUpstreamHeaderRoutingOptions.cs b/src/Ocelot/Configuration/File/FileUpstreamHeaderRoutingOptions.cs new file mode 100644 index 000000000..23d6a32d5 --- /dev/null +++ b/src/Ocelot/Configuration/File/FileUpstreamHeaderRoutingOptions.cs @@ -0,0 +1,17 @@ +using System.Collections.Generic; + +namespace Ocelot.Configuration.File +{ + public class FileUpstreamHeaderRoutingOptions + { + public FileUpstreamHeaderRoutingOptions() + { + Headers = new(); + TriggerOn = string.Empty; + } + + public Dictionary> Headers { get; set; } + + public string TriggerOn { get; set; } + } +} diff --git a/src/Ocelot/Configuration/Route.cs b/src/Ocelot/Configuration/Route.cs index 8f9c0992f..602b227c1 100644 --- a/src/Ocelot/Configuration/Route.cs +++ b/src/Ocelot/Configuration/Route.cs @@ -10,7 +10,8 @@ public class Route List upstreamHttpMethod, UpstreamPathTemplate upstreamTemplatePattern, string upstreamHost, - string aggregator) + string aggregator, + UpstreamHeaderRoutingOptions upstreamHeaderRoutingOptions) { UpstreamHost = upstreamHost; DownstreamRoute = downstreamRoute; @@ -18,6 +19,7 @@ public class Route UpstreamHttpMethod = upstreamHttpMethod; UpstreamTemplatePattern = upstreamTemplatePattern; Aggregator = aggregator; + UpstreamHeaderRoutingOptions = upstreamHeaderRoutingOptions; } public UpstreamPathTemplate UpstreamTemplatePattern { get; } @@ -26,5 +28,6 @@ public class Route public List DownstreamRoute { get; } public List DownstreamRouteConfig { get; } public string Aggregator { get; } + public UpstreamHeaderRoutingOptions UpstreamHeaderRoutingOptions { get; } } } diff --git a/src/Ocelot/Configuration/UpstreamHeaderRoutingOptions.cs b/src/Ocelot/Configuration/UpstreamHeaderRoutingOptions.cs new file mode 100644 index 000000000..3496d86c4 --- /dev/null +++ b/src/Ocelot/Configuration/UpstreamHeaderRoutingOptions.cs @@ -0,0 +1,19 @@ +using System.Collections.Generic; + +namespace Ocelot.Configuration +{ + public class UpstreamHeaderRoutingOptions + { + public UpstreamHeaderRoutingOptions(Dictionary> headers, UpstreamHeaderRoutingTriggerMode mode) + { + Headers = new UpstreamRoutingHeaders(headers); + Mode = mode; + } + + public bool Enabled() => Headers.Any(); + + public UpstreamRoutingHeaders Headers { get; } + + public UpstreamHeaderRoutingTriggerMode Mode { get; } + } +} diff --git a/src/Ocelot/Configuration/UpstreamHeaderRoutingTriggerMode.cs b/src/Ocelot/Configuration/UpstreamHeaderRoutingTriggerMode.cs new file mode 100644 index 000000000..8596ae7f1 --- /dev/null +++ b/src/Ocelot/Configuration/UpstreamHeaderRoutingTriggerMode.cs @@ -0,0 +1,8 @@ +namespace Ocelot.Configuration +{ + public enum UpstreamHeaderRoutingTriggerMode + { + Any = 0, + All = 1, + } +} diff --git a/src/Ocelot/Configuration/UpstreamRoutingHeaders.cs b/src/Ocelot/Configuration/UpstreamRoutingHeaders.cs new file mode 100644 index 000000000..119155c1e --- /dev/null +++ b/src/Ocelot/Configuration/UpstreamRoutingHeaders.cs @@ -0,0 +1,70 @@ +using System.Collections.Generic; +using System.Linq; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Primitives; + +namespace Ocelot.Configuration +{ + public class UpstreamRoutingHeaders + { + public IReadOnlyDictionary> Headers { get; } + + public UpstreamRoutingHeaders(IReadOnlyDictionary> headers) + { + Headers = headers; + } + + public bool Any() => Headers.Any(); + + public bool HasAnyOf(IHeaderDictionary requestHeaders) + { + IHeaderDictionary lowerCaseHeaders = GetLowerCaseHeaders(requestHeaders); + foreach (KeyValuePair> h in Headers) + { + if (lowerCaseHeaders.TryGetValue(h.Key, out var values)) + { + HashSet requestHeaderValues = new(values); + if (h.Value.Overlaps(requestHeaderValues)) + { + return true; + } + } + } + + return false; + } + + public bool HasAllOf(IHeaderDictionary requestHeaders) + { + IHeaderDictionary lowerCaseHeaders = GetLowerCaseHeaders(requestHeaders); + foreach (KeyValuePair> h in Headers) + { + if (!lowerCaseHeaders.TryGetValue(h.Key, out var values)) + { + return false; + } + + HashSet requestHeaderValues = new(values); + if (!h.Value.Overlaps(requestHeaderValues)) + { + return false; + } + } + + return true; + } + + private static IHeaderDictionary GetLowerCaseHeaders(IHeaderDictionary headers) + { + IHeaderDictionary lowerCaseHeaders = new HeaderDictionary(); + foreach (KeyValuePair kv in headers) + { + string key = kv.Key.ToLowerInvariant(); + StringValues values = new(kv.Value.Select(v => v.ToLowerInvariant()).ToArray()); + lowerCaseHeaders.Add(key, values); + } + + return lowerCaseHeaders; + } + } +} diff --git a/src/Ocelot/DependencyInjection/OcelotBuilder.cs b/src/Ocelot/DependencyInjection/OcelotBuilder.cs index a72ec3cbf..2db70695e 100644 --- a/src/Ocelot/DependencyInjection/OcelotBuilder.cs +++ b/src/Ocelot/DependencyInjection/OcelotBuilder.cs @@ -75,6 +75,7 @@ public OcelotBuilder(IServiceCollection services, IConfiguration configurationRo Services.TryAddSingleton(); Services.TryAddSingleton(); Services.TryAddSingleton(); + Services.TryAddSingleton(); Services.TryAddSingleton(); Services.TryAddSingleton(); Services.TryAddSingleton(); diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteCreator.cs b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteCreator.cs index be4d5e32b..92e4b81bc 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteCreator.cs +++ b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteCreator.cs @@ -1,9 +1,10 @@ -using Ocelot.Configuration; -using Ocelot.Configuration.Builder; -using Ocelot.Configuration.Creator; +using Microsoft.AspNetCore.Http; +using Ocelot.Configuration; +using Ocelot.Configuration.Builder; +using Ocelot.Configuration.Creator; using Ocelot.DownstreamRouteFinder.UrlMatcher; -using Ocelot.LoadBalancer.LoadBalancers; -using Ocelot.Responses; +using Ocelot.LoadBalancer.LoadBalancers; +using Ocelot.Responses; namespace Ocelot.DownstreamRouteFinder.Finder { @@ -18,7 +19,13 @@ public DownstreamRouteCreator(IQoSOptionsCreator qoSOptionsCreator) _cache = new ConcurrentDictionary>(); } - public Response Get(string upstreamUrlPath, string upstreamQueryString, string upstreamHttpMethod, IInternalConfiguration configuration, string upstreamHost) + public Response Get( + string upstreamUrlPath, + string upstreamQueryString, + string upstreamHttpMethod, + IInternalConfiguration configuration, + string upstreamHost, + IHeaderDictionary requestHeaders) { var serviceName = GetServiceName(upstreamUrlPath); diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs index 59cf1f7b7..bec039365 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs +++ b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs @@ -1,6 +1,7 @@ -using Ocelot.Configuration; +using Microsoft.AspNetCore.Http; +using Ocelot.Configuration; using Ocelot.DownstreamRouteFinder.UrlMatcher; -using Ocelot.Responses; +using Ocelot.Responses; namespace Ocelot.DownstreamRouteFinder.Finder { @@ -15,12 +16,18 @@ public DownstreamRouteFinder(IUrlPathToUrlTemplateMatcher urlMatcher, IPlacehold _placeholderNameAndValueFinder = urlPathPlaceholderNameAndValueFinder; } - public Response Get(string upstreamUrlPath, string upstreamQueryString, string httpMethod, IInternalConfiguration configuration, string upstreamHost) + public Response Get( + string upstreamUrlPath, + string upstreamQueryString, + string httpMethod, + IInternalConfiguration configuration, + string upstreamHost, + IHeaderDictionary requestHeaders) { var downstreamRoutes = new List(); var applicableRoutes = configuration.Routes - .Where(r => RouteIsApplicableToThisRequest(r, httpMethod, upstreamHost)) + .Where(r => RouteIsApplicableToThisRequest(r, httpMethod, upstreamHost, requestHeaders)) .OrderByDescending(x => x.UpstreamTemplatePattern.Priority); foreach (var route in applicableRoutes) @@ -44,10 +51,32 @@ public Response Get(string upstreamUrlPath, string upstre return new ErrorResponse(new UnableToFindDownstreamRouteError(upstreamUrlPath, httpMethod)); } - private static bool RouteIsApplicableToThisRequest(Route route, string httpMethod, string upstreamHost) + private static bool RouteIsApplicableToThisRequest(Route route, string httpMethod, string upstreamHost, IHeaderDictionary requestHeaders) { - return (route.UpstreamHttpMethod.Count == 0 || route.UpstreamHttpMethod.Select(x => x.Method.ToLower()).Contains(httpMethod.ToLower())) && - (string.IsNullOrEmpty(route.UpstreamHost) || route.UpstreamHost == upstreamHost); + return (route.UpstreamHttpMethod.Count == 0 || RouteHasHttpMethod(route, httpMethod)) && + (string.IsNullOrEmpty(route.UpstreamHost) || route.UpstreamHost == upstreamHost) && + (route.UpstreamHeaderRoutingOptions == null || !route.UpstreamHeaderRoutingOptions.Enabled() || RouteHasRequiredUpstreamHeaders(route, requestHeaders)); + } + + private static bool RouteHasHttpMethod(Route route, string httpMethod) + { + return route.UpstreamHttpMethod.Select(x => x.Method.ToLower()).Contains(httpMethod.ToLower()); + } + + private static bool RouteHasRequiredUpstreamHeaders(Route route, IHeaderDictionary requestHeaders) + { + bool result = false; + switch (route.UpstreamHeaderRoutingOptions.Mode) + { + case UpstreamHeaderRoutingTriggerMode.Any: + result = route.UpstreamHeaderRoutingOptions.Headers.HasAnyOf(requestHeaders); + break; + case UpstreamHeaderRoutingTriggerMode.All: + result = route.UpstreamHeaderRoutingOptions.Headers.HasAllOf(requestHeaders); + break; + } + + return result; } private DownstreamRouteHolder GetPlaceholderNamesAndValues(string path, string query, Route route) diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/IDownstreamRouteProvider.cs b/src/Ocelot/DownstreamRouteFinder/Finder/IDownstreamRouteProvider.cs index ed2a657ef..891536cbe 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/IDownstreamRouteProvider.cs +++ b/src/Ocelot/DownstreamRouteFinder/Finder/IDownstreamRouteProvider.cs @@ -1,10 +1,17 @@ -using Ocelot.Configuration; +using Microsoft.AspNetCore.Http; +using Ocelot.Configuration; using Ocelot.Responses; namespace Ocelot.DownstreamRouteFinder.Finder { public interface IDownstreamRouteProvider { - Response Get(string upstreamUrlPath, string upstreamQueryString, string upstreamHttpMethod, IInternalConfiguration configuration, string upstreamHost); + Response Get( + string upstreamUrlPath, + string upstreamQueryString, + string upstreamHttpMethod, + IInternalConfiguration configuration, + string upstreamHost, + IHeaderDictionary requestHeaders); } } diff --git a/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs b/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs index 63c21b76c..f2adcc181 100644 --- a/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs +++ b/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs @@ -32,13 +32,21 @@ public async Task Invoke(HttpContext httpContext) ? hostHeader.Split(':')[0] : hostHeader; + var upstreamHeaders = httpContext.Request.Headers; + Logger.LogDebug(() => $"Upstream url path is {upstreamUrlPath}"); var internalConfiguration = httpContext.Items.IInternalConfiguration(); var provider = _factory.Get(internalConfiguration); - var response = provider.Get(upstreamUrlPath, upstreamQueryString, httpContext.Request.Method, internalConfiguration, upstreamHost); + var response = provider.Get( + upstreamUrlPath, + upstreamQueryString, + httpContext.Request.Method, + internalConfiguration, + upstreamHost, + upstreamHeaders); if (response.IsError) { diff --git a/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs index cf5f38e7b..754d6ffc6 100644 --- a/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs @@ -1,9 +1,9 @@ -using Ocelot.Cache; -using Ocelot.Configuration; -using Ocelot.Configuration.Builder; -using Ocelot.Configuration.Creator; -using Ocelot.Configuration.File; -using Ocelot.Values; +using Ocelot.Cache; +using Ocelot.Configuration; +using Ocelot.Configuration.Builder; +using Ocelot.Configuration.Creator; +using Ocelot.Configuration.File; +using Ocelot.Values; namespace Ocelot.UnitTests.Configuration { @@ -25,6 +25,7 @@ public class RoutesCreatorTests private readonly Mock _rrkCreator; private readonly Mock _soCreator; private readonly Mock _versionCreator; + private readonly Mock _uhroCreator; private FileConfiguration _fileConfig; private RouteOptions _rro; private string _requestId; @@ -59,6 +60,7 @@ public RoutesCreatorTests() _rrkCreator = new Mock(); _soCreator = new Mock(); _versionCreator = new Mock(); + _uhroCreator = new Mock(); _creator = new RoutesCreator( _cthCreator.Object, @@ -75,7 +77,8 @@ public RoutesCreatorTests() _lboCreator.Object, _rrkCreator.Object, _soCreator.Object, - _versionCreator.Object + _versionCreator.Object, + _uhroCreator.Object ); } diff --git a/test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs new file mode 100644 index 000000000..049bc7d75 --- /dev/null +++ b/test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs @@ -0,0 +1,71 @@ +using System.Collections.Generic; +using System.Linq; +using Ocelot.Configuration.File; +using Ocelot.Configuration.Creator; +using Ocelot.Configuration; +using Xunit; +using TestStack.BDDfy; +using Shouldly; + +namespace Ocelot.UnitTests.Configuration +{ + public class UpstreamHeaderRoutingOptionsCreatorTests + { + private FileUpstreamHeaderRoutingOptions _fileUpstreamHeaderRoutingOptions; + private IUpstreamHeaderRoutingOptionsCreator _creator; + private UpstreamHeaderRoutingOptions _upstreamHeaderRoutingOptions; + + public UpstreamHeaderRoutingOptionsCreatorTests() + { + _creator = new UpstreamHeaderRoutingOptionsCreator(); + } + + [Fact] + public void should_create_upstream_routing_header_options() + { + UpstreamHeaderRoutingOptions expected = new UpstreamHeaderRoutingOptions( + headers: new Dictionary>() + { + { "header1", new HashSet() { "value1", "value2" }}, + { "header2", new HashSet() { "value3" }}, + }, + mode: UpstreamHeaderRoutingTriggerMode.All + ); + + this.Given(_ => GivenTheseFileUpstreamHeaderRoutingOptions()) + .When(_ => WhenICreate()) + .Then(_ => ThenTheCreatedMatchesThis(expected)) + .BDDfy(); + } + + private void GivenTheseFileUpstreamHeaderRoutingOptions() + { + _fileUpstreamHeaderRoutingOptions = new FileUpstreamHeaderRoutingOptions() + { + Headers = new Dictionary>() + { + { "Header1", new List() { "Value1", "Value2" }}, + { "Header2", new List() { "Value3" }}, + }, + TriggerOn = "all", + }; + } + + private void WhenICreate() + { + _upstreamHeaderRoutingOptions = _creator.Create(_fileUpstreamHeaderRoutingOptions); + } + + private void ThenTheCreatedMatchesThis(UpstreamHeaderRoutingOptions expected) + { + _upstreamHeaderRoutingOptions.Headers.Headers.Count.ShouldBe(expected.Headers.Headers.Count); + foreach (KeyValuePair> pair in _upstreamHeaderRoutingOptions.Headers.Headers) + { + expected.Headers.Headers.TryGetValue(pair.Key, out var expectedValue).ShouldBe(true); + expectedValue.SetEquals(pair.Value).ShouldBe(true); + } + + _upstreamHeaderRoutingOptions.Mode.ShouldBe(expected.Mode); + } + } +} diff --git a/test/Ocelot.UnitTests/Configuration/UpstreamRoutingHeadersTests.cs b/test/Ocelot.UnitTests/Configuration/UpstreamRoutingHeadersTests.cs new file mode 100644 index 000000000..87eb5306a --- /dev/null +++ b/test/Ocelot.UnitTests/Configuration/UpstreamRoutingHeadersTests.cs @@ -0,0 +1,142 @@ +using System.Collections.Generic; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Primitives; +using Xunit; +using TestStack.BDDfy; +using Shouldly; +using Ocelot.Configuration; + +namespace Ocelot.UnitTests.Configuration +{ + public class UpstreamRoutingHeadersTests + { + private Dictionary> _headersDictionary; + private UpstreamRoutingHeaders _upstreamRoutingHeaders; + private IHeaderDictionary _requestHeaders; + + [Fact] + public void should_create_empty_headers() + { + this.Given(_ => GivenEmptyHeaderDictionary()) + .When(_ => WhenICreate()) + .Then(_ => ThenAnyIs(false)) + .BDDfy(); + } + + [Fact] + public void should_create_preset_headers() + { + this.Given(_ => GivenPresetHeaderDictionary()) + .When(_ => WhenICreate()) + .Then(_ => ThenAnyIs(true)) + .BDDfy(); + } + + [Fact] + public void should_not_match_mismatching_request_headers() + { + this.Given(_ => GivenPresetHeaderDictionary()) + .And(_ => AndGivenMismatchingRequestHeaders()) + .When(_ => WhenICreate()) + .Then(_ => ThenHasAnyOfIs(false)) + .And(_ => ThenHasAllOfIs(false)) + .BDDfy(); + } + + [Fact] + public void should_not_match_matching_header_with_mismatching_value() + { + this.Given(_ => GivenPresetHeaderDictionary()) + .And(_ => AndGivenOneMatchingHeaderWithMismatchingValue()) + .When(_ => WhenICreate()) + .Then(_ => ThenHasAnyOfIs(false)) + .And(_ => ThenHasAllOfIs(false)) + .BDDfy(); + } + + [Fact] + public void should_match_any_header_not_all() + { + this.Given(_ => GivenPresetHeaderDictionary()) + .And(_ => AndGivenOneMatchingHeaderWithMatchingValue()) + .When(_ => WhenICreate()) + .Then(_ => ThenHasAnyOfIs(true)) + .And(_ => ThenHasAllOfIs(false)) + .BDDfy(); + } + + [Fact] + public void should_match_any_and_all_headers() + { + this.Given(_ => GivenPresetHeaderDictionary()) + .And(_ => AndGivenTwoMatchingHeadersWithMatchingValues()) + .When(_ => WhenICreate()) + .Then(_ => ThenHasAnyOfIs(true)) + .And(_ => ThenHasAllOfIs(true)) + .BDDfy(); + } + + private void GivenEmptyHeaderDictionary() + { + _headersDictionary = new Dictionary>(); + } + + private void GivenPresetHeaderDictionary() + { + _headersDictionary = new Dictionary>() + { + { "testheader1", new HashSet() { "testheader1value1", "testheader1value2" } }, + { "testheader2", new HashSet() { "testheader1Value1", "testheader2value2" } }, + }; + } + + private void AndGivenMismatchingRequestHeaders() + { + _requestHeaders = new HeaderDictionary() { + { "someHeader", new StringValues(new []{ "someHeaderValue" })}, + }; + } + + private void AndGivenOneMatchingHeaderWithMismatchingValue() + { + _requestHeaders = new HeaderDictionary() { + { "testHeader1", new StringValues(new []{ "mismatchingValue" })}, + }; + } + + private void AndGivenOneMatchingHeaderWithMatchingValue() + { + _requestHeaders = new HeaderDictionary() { + { "testHeader1", new StringValues(new []{ "testHeader1Value1" })}, + }; + } + + private void AndGivenTwoMatchingHeadersWithMatchingValues() + { + _requestHeaders = new HeaderDictionary() { + { "testHeader1", new StringValues(new []{ "testHeader1Value1", "bogusValue" })}, + { "testHeader2", new StringValues(new []{ "bogusValue", "testHeader2Value2" })}, + }; + } + + private void WhenICreate() + { + _upstreamRoutingHeaders = new UpstreamRoutingHeaders(_headersDictionary); + } + + private void ThenAnyIs(bool expected) + { + _upstreamRoutingHeaders.Any().ShouldBe(expected); + } + + private void ThenHasAnyOfIs(bool expected) + { + _upstreamRoutingHeaders.HasAnyOf(_requestHeaders).ShouldBe(expected); + } + + private void ThenHasAllOfIs(bool expected) + { + _upstreamRoutingHeaders.HasAllOf(_requestHeaders).ShouldBe(expected); + } + } +} diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteCreatorTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteCreatorTests.cs index 0ead908aa..f6b76e799 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteCreatorTests.cs +++ b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteCreatorTests.cs @@ -1,9 +1,10 @@ -using Ocelot.Configuration; -using Ocelot.Configuration.Builder; -using Ocelot.Configuration.Creator; -using Ocelot.DownstreamRouteFinder.Finder; -using Ocelot.LoadBalancer.LoadBalancers; -using Ocelot.Responses; +using Microsoft.AspNetCore.Http; +using Ocelot.Configuration; +using Ocelot.Configuration.Builder; +using Ocelot.Configuration.Creator; +using Ocelot.DownstreamRouteFinder.Finder; +using Ocelot.LoadBalancer.LoadBalancers; +using Ocelot.Responses; namespace Ocelot.UnitTests.DownstreamRouteFinder { @@ -21,6 +22,7 @@ public class DownstreamRouteCreatorTests private readonly Mock _qosOptionsCreator; private Response _resultTwo; private readonly string _upstreamQuery; + private readonly IHeaderDictionary _upstreamHeaders; public DownstreamRouteCreatorTests() { @@ -31,8 +33,9 @@ public DownstreamRouteCreatorTests() _qosOptionsCreator .Setup(x => x.Create(It.IsAny(), It.IsAny(), It.IsAny>())) .Returns(_qoSOptions); - _creator = new DownstreamRouteCreator(_qosOptionsCreator.Object); + _creator = new DownstreamRouteCreator(_qosOptionsCreator.Object); _upstreamQuery = string.Empty; + _upstreamHeaders = new HeaderDictionary(); } [Fact] @@ -278,12 +281,12 @@ private void ThenTheHandlerOptionsAreSet() private void WhenICreate() { - _result = _creator.Get(_upstreamUrlPath, _upstreamQuery, _upstreamHttpMethod, _configuration, _upstreamHost); + _result = _creator.Get(_upstreamUrlPath, _upstreamQuery, _upstreamHttpMethod, _configuration, _upstreamHost, _upstreamHeaders); } private void WhenICreateAgain() { - _resultTwo = _creator.Get(_upstreamUrlPath, _upstreamQuery, _upstreamHttpMethod, _configuration, _upstreamHost); + _resultTwo = _creator.Get(_upstreamUrlPath, _upstreamQuery, _upstreamHttpMethod, _configuration, _upstreamHost, _upstreamHeaders); } private void ThenTheDownstreamRoutesAreTheSameReference() diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderMiddlewareTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderMiddlewareTests.cs index f743f6914..22addae7a 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderMiddlewareTests.cs @@ -74,7 +74,7 @@ private void GivenTheDownStreamRouteFinderReturns(DownstreamRouteHolder downstre { _downstreamRoute = new OkResponse(downstreamRoute); _finder - .Setup(x => x.Get(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Setup(x => x.Get(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns(_downstreamRoute); } diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs index 46654d8f4..85c4528a4 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs +++ b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs @@ -1,10 +1,13 @@ -using Ocelot.Configuration; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Primitives; +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 Ocelot.Values; namespace Ocelot.UnitTests.DownstreamRouteFinder { @@ -21,6 +24,8 @@ public class DownstreamRouteFinderTests private string _upstreamHttpMethod; private string _upstreamHost; private string _upstreamQuery; + private UpstreamHeaderRoutingOptions _upstreamHeaderRoutingOptions; + private IHeaderDictionary _requestHeaders; public DownstreamRouteFinderTests() { @@ -60,6 +65,7 @@ public void should_return_highest_priority_when_first() }, string.Empty, serviceProviderConfig)) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder(new List(), new RouteBuilder() @@ -106,6 +112,7 @@ public void should_return_highest_priority_when_lowest() }, string.Empty, serviceProviderConfig)) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder(new List(), new RouteBuilder() @@ -145,6 +152,7 @@ public void should_return_route() )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder( @@ -187,6 +195,7 @@ public void should_not_append_slash_to_upstream_url_path() )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder( @@ -230,6 +239,7 @@ public void should_return_route_if_upstream_path_and_upstream_template_are_the_s )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder(new List(), @@ -280,6 +290,7 @@ public void should_return_correct_route_for_http_verb() )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder(new List(), @@ -317,9 +328,9 @@ public void should_not_return_route() )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(false)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) - .Then( - x => x.ThenAnErrorResponseIsReturned()) + .Then(x => x.ThenAnErrorResponseIsReturned()) .And(x => x.ThenTheUrlMatcherIsCalledCorrectly()) .BDDfy(); } @@ -349,6 +360,7 @@ public void should_return_correct_route_for_http_verb_setting_multiple_upstream_ )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder(new List(), @@ -390,6 +402,7 @@ public void should_return_correct_route_for_http_verb_setting_all_upstream_http_ )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder(new List(), @@ -431,6 +444,7 @@ public void should_not_return_route_for_http_verb_not_setting_in_upstream_http_m )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenAnErrorResponseIsReturned()) .And(x => x.ThenTheUrlMatcherIsNotCalled()) @@ -463,6 +477,7 @@ public void should_return_route_when_host_matches() )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder( @@ -506,6 +521,7 @@ public void should_return_route_when_upstreamhost_is_null() )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder( @@ -558,6 +574,7 @@ public void should_not_return_route_when_host_doesnt_match() )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenAnErrorResponseIsReturned()) .And(x => x.ThenTheUrlMatcherIsNotCalled()) @@ -588,6 +605,7 @@ public void should_not_return_route_when_host_doesnt_match_with_empty_upstream_h )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenAnErrorResponseIsReturned()) .And(x => x.ThenTheUrlMatcherIsNotCalled()) @@ -618,6 +636,7 @@ public void should_return_route_when_host_does_match_with_empty_upstream_http_me )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .And(x => x.ThenTheUrlMatcherIsCalledCorrectly(1, 0)) .BDDfy(); @@ -658,9 +677,9 @@ public void should_return_route_when_host_matches_but_null_host_on_same_path_fir )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) - .Then( - x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder( + .Then(x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder( new List(), new RouteBuilder() .WithDownstreamRoute(new DownstreamRouteBuilder() @@ -677,6 +696,114 @@ public void should_return_route_when_host_matches_but_null_host_on_same_path_fir .BDDfy(); } + [Fact] + public void should_not_return_route_with_upstream_header_routing_options_enabled_and_no_request_headers() + { + var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); + + this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher/")) + .And(x => x.GivenTheTemplateVariableAndNameFinderReturns( + new OkResponse>( + new List()))) + .And(x => x.GivenUpstreamHeaderRoutingOptions()) + .And(x => x.GivenTheConfigurationIs(new List + { + new RouteBuilder() + .WithDownstreamRoute(new DownstreamRouteBuilder() + .WithDownstreamPathTemplate("someDownstreamPath") + .WithUpstreamHttpMethod(new List { "Get" }) + .WithUpstreamPathTemplate(new UpstreamPathTemplate("someUpstreamPath", 1, false, "someUpstreamPath")) + .Build()) + .WithUpstreamHttpMethod(new List { "Get" }) + .WithUpstreamPathTemplate(new UpstreamPathTemplate("someUpstreamPath", 1, false, "someUpstreamPath")) + .WithUpstreamHeaderRoutingOptions(_upstreamHeaderRoutingOptions) + .Build(), + }, string.Empty, serviceProviderConfig + )) + .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .And(x => x.GivenEmptyRequestHeaders()) + .When(x => x.WhenICallTheFinder()) + .Then(x => x.ThenAnErrorResponseIsReturned()) + .BDDfy(); + } + + [Fact] + public void should_not_return_route_with_upstream_header_routing_options_enabled_and_non_matching_request_headers() + { + var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); + + this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher/")) + .And(x => x.GivenTheTemplateVariableAndNameFinderReturns( + new OkResponse>( + new List()))) + .And(x => x.GivenUpstreamHeaderRoutingOptions()) + .And(x => x.GivenTheConfigurationIs(new List + { + new RouteBuilder() + .WithDownstreamRoute(new DownstreamRouteBuilder() + .WithDownstreamPathTemplate("someDownstreamPath") + .WithUpstreamHttpMethod(new List { "Get" }) + .WithUpstreamPathTemplate(new UpstreamPathTemplate("someUpstreamPath", 1, false, "someUpstreamPath")) + .Build()) + .WithUpstreamHttpMethod(new List { "Get" }) + .WithUpstreamPathTemplate(new UpstreamPathTemplate("someUpstreamPath", 1, false, "someUpstreamPath")) + .WithUpstreamHeaderRoutingOptions(_upstreamHeaderRoutingOptions) + .Build(), + }, string.Empty, serviceProviderConfig + )) + .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .And(x => x.GivenNonEmptyNonMatchingRequestHeaders()) + .When(x => x.WhenICallTheFinder()) + .Then(x => x.ThenAnErrorResponseIsReturned()) + .BDDfy(); + } + + [Fact] + public void should_return_route_with_upstream_header_routing_options_enabled_and_matching_request_headers() + { + var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); + + this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher/")) + .And(x => x.GivenTheTemplateVariableAndNameFinderReturns( + new OkResponse>( + new List()))) + .And(x => x.GivenUpstreamHeaderRoutingOptions()) + .And(x => x.GivenTheConfigurationIs(new List + { + new RouteBuilder() + .WithDownstreamRoute(new DownstreamRouteBuilder() + .WithDownstreamPathTemplate("someDownstreamPath") + .WithUpstreamHttpMethod(new List { "Get" }) + .WithUpstreamPathTemplate(new UpstreamPathTemplate("someUpstreamPath", 1, false, "someUpstreamPath")) + .Build()) + .WithUpstreamHttpMethod(new List { "Get" }) + .WithUpstreamPathTemplate(new UpstreamPathTemplate("someUpstreamPath", 1, false, "someUpstreamPath")) + .WithUpstreamHeaderRoutingOptions(_upstreamHeaderRoutingOptions) + .Build(), + }, string.Empty, serviceProviderConfig + )) + .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .And(x => x.GivenNonEmptyMatchingRequestHeaders()) + .When(x => x.WhenICallTheFinder()) + .Then(x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder( + new List(), + new RouteBuilder() + .WithDownstreamRoute(new DownstreamRouteBuilder() + .WithDownstreamPathTemplate("someDownstreamPath") + .WithUpstreamHttpMethod(new List { "Get" }) + .WithUpstreamPathTemplate(new UpstreamPathTemplate("someUpstreamPath", 1, false, "someUpstreamPath")) + .Build()) + .WithUpstreamHttpMethod(new List { "Get" }) + .WithUpstreamPathTemplate(new UpstreamPathTemplate("someUpstreamPath", 1, false, "someUpstreamPath")) + .Build() + ))) + .And(x => x.ThenTheUrlMatcherIsCalledCorrectly()) + .BDDfy(); + } + private void GivenTheUpstreamHostIs(string upstreamHost) { _upstreamHost = upstreamHost; @@ -694,6 +821,36 @@ private void GivenTheUpstreamHttpMethodIs(string upstreamHttpMethod) _upstreamHttpMethod = upstreamHttpMethod; } + private void GivenUpstreamHeaderRoutingOptions() + { + var headers = new Dictionary>() + { + { "header", new HashSet() { "value" }}, + }; + _upstreamHeaderRoutingOptions = new UpstreamHeaderRoutingOptions(headers, UpstreamHeaderRoutingTriggerMode.All); + } + + private void GivenEmptyRequestHeaders() + { + _requestHeaders = new HeaderDictionary(); + } + + private void GivenNonEmptyNonMatchingRequestHeaders() + { + _requestHeaders = new HeaderDictionary() + { + { "header", new StringValues(new []{ "mismatch" }) }, + }; + } + + private void GivenNonEmptyMatchingRequestHeaders() + { + _requestHeaders = new HeaderDictionary() + { + { "header", new StringValues(new []{ "value" }) }, + }; + } + private void ThenAnErrorResponseIsReturned() { _result.IsError.ShouldBeTrue(); @@ -739,13 +896,13 @@ private void GivenTheConfigurationIs(List routesConfig, string adminPath, private void GivenThereIsAnUpstreamUrlPath(string upstreamUrlPath) { - _upstreamUrlPath = upstreamUrlPath; + _upstreamUrlPath = upstreamUrlPath; _upstreamQuery = string.Empty; } private void WhenICallTheFinder() { - _result = _downstreamRouteFinder.Get(_upstreamUrlPath, _upstreamQuery, _upstreamHttpMethod, _config, _upstreamHost); + _result = _downstreamRouteFinder.Get(_upstreamUrlPath, _upstreamQuery, _upstreamHttpMethod, _config, _upstreamHost, _requestHeaders); } private void ThenTheFollowingIsReturned(DownstreamRouteHolder expected) From f257989886689226204efa91165e595fb247e2dd Mon Sep 17 00:00:00 2001 From: Zbynek Novotny Date: Sat, 5 Aug 2023 13:17:54 +0200 Subject: [PATCH 02/13] Added upstream header routing options as an additional criterion to consider when evaluating duplication between upstream path mappings. --- .../FileConfigurationFluentValidator.cs | 64 ++++++++++++++-- .../FileConfigurationFluentValidatorTests.cs | 75 ++++++++++--------- 2 files changed, 95 insertions(+), 44 deletions(-) diff --git a/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs b/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs index 064a4e0c9..eec9aa35c 100644 --- a/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs +++ b/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs @@ -1,9 +1,9 @@ -using FluentValidation; -using Microsoft.Extensions.DependencyInjection; -using Ocelot.Configuration.File; -using Ocelot.Errors; -using Ocelot.Responses; -using Ocelot.ServiceDiscovery; +using FluentValidation; +using Microsoft.Extensions.DependencyInjection; +using Ocelot.Configuration.File; +using Ocelot.Errors; +using Ocelot.Responses; +using Ocelot.ServiceDiscovery; namespace Ocelot.Configuration.Validator { @@ -26,7 +26,7 @@ public FileConfigurationFluentValidator(IServiceProvider provider, RouteFluentVa RuleForEach(configuration => configuration.Routes) .Must((config, route) => IsNotDuplicateIn(route, config.Routes)) - .WithMessage((_, route) => $"{nameof(route)} {route.UpstreamPathTemplate} has duplicate"); + .WithMessage((_, route) => $"{nameof(route)} {route.UpstreamPathTemplate} has duplicate upstream path or routing header mapping"); RuleForEach(configuration => configuration.Routes) .Must((config, route) => HaveServiceDiscoveryProviderRegistered(route, config.GlobalConfiguration.ServiceDiscoveryProvider)) @@ -115,7 +115,8 @@ private static bool IsPlaceholderNotDuplicatedIn(string upstreamPathTemplate) { var matchingRoutes = routes .Where(r => r.UpstreamPathTemplate == route.UpstreamPathTemplate - && r.UpstreamHost == route.UpstreamHost) + && r.UpstreamHost == route.UpstreamHost + && AreDuplicateUpstreamRoutingHeaders(route, r)) .ToList(); if (matchingRoutes.Count == 1) @@ -156,5 +157,52 @@ private static bool IsNotDuplicateIn(FileAggregateRoute route, IEnumerable r.UpstreamPathTemplate == route.UpstreamPathTemplate & r.UpstreamHost == route.UpstreamHost); return matchingRoutes.Count() <= 1; } + + private static bool AreDuplicateUpstreamRoutingHeaders(FileRoute first, FileRoute second) + { + if (!first.UpstreamHeaderRoutingOptions.Headers.Any() && !second.UpstreamHeaderRoutingOptions.Headers.Any()) + { + return true; + } + + if (first.UpstreamHeaderRoutingOptions.Headers.Any() ^ second.UpstreamHeaderRoutingOptions.Headers.Any()) + { + return false; + } + + ISet firstKeySet = first.UpstreamHeaderRoutingOptions.Headers.Keys + .Select(k => k.ToLowerInvariant()) + .ToHashSet(); + ISet secondKeySet = second.UpstreamHeaderRoutingOptions.Headers.Keys + .Select(k => k.ToLowerInvariant()) + .ToHashSet(); + if (!firstKeySet.Overlaps(secondKeySet)) + { + return false; + } + + foreach (var (key, values) in first.UpstreamHeaderRoutingOptions.Headers) + { + IDictionary> secondHeaders = second.UpstreamHeaderRoutingOptions.Headers; + if (!secondHeaders.TryGetValue(key, out List secondHeaderValues)) + { + continue; + } + + ISet firstHeaderValuesLowerCase = values + .Select(v => v.ToLowerInvariant()) + .ToHashSet(); + ISet secondHeaderValuesLowerCase = secondHeaderValues + .Select(v => v.ToLowerInvariant()) + .ToHashSet(); + + if (firstHeaderValuesLowerCase.Overlaps(secondHeaderValuesLowerCase)) + { + return true; + } + } + + return false; + } } } diff --git a/test/Ocelot.UnitTests/Configuration/Validation/FileConfigurationFluentValidatorTests.cs b/test/Ocelot.UnitTests/Configuration/Validation/FileConfigurationFluentValidatorTests.cs index 77d2bf791..a2b8ce903 100644 --- a/test/Ocelot.UnitTests/Configuration/Validation/FileConfigurationFluentValidatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/Validation/FileConfigurationFluentValidatorTests.cs @@ -1,4 +1,4 @@ -using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -14,7 +14,7 @@ using Ocelot.UnitTests.Requester; using Ocelot.Values; using System.Security.Claims; -using System.Text.Encodings.Web; +using System.Text.Encodings.Web; namespace Ocelot.UnitTests.Configuration.Validation { @@ -25,10 +25,10 @@ public class FileConfigurationFluentValidatorTests private Response _result; private IServiceProvider _provider; private readonly ServiceCollection _services; - private readonly Mock _authProvider; + private readonly Mock _authProvider; public FileConfigurationFluentValidatorTests() - { + { _services = new ServiceCollection(); _authProvider = new Mock(); _provider = _services.BuildServiceProvider(); @@ -39,7 +39,7 @@ public FileConfigurationFluentValidatorTests() [Fact] public void configuration_is_valid_if_service_discovery_options_specified_and_has_service_fabric_as_option() - { + { var route = GivenServiceDiscoveryRoute(); var configuration = GivenAConfiguration(route); configuration.GlobalConfiguration.ServiceDiscoveryProvider = GivenDefaultServiceDiscoveryProvider(); @@ -107,7 +107,7 @@ public void configuration_is_invalid_if_service_discovery_options_specified_dyna [Fact] public void configuration_is_invalid_if_service_discovery_options_specified_but_no_service_discovery_handler_with_matching_name() - { + { var route = GivenServiceDiscoveryRoute(); var configuration = GivenAConfiguration(route); configuration.GlobalConfiguration.ServiceDiscoveryProvider = GivenDefaultServiceDiscoveryProvider(); @@ -123,11 +123,11 @@ public void configuration_is_invalid_if_service_discovery_options_specified_but_ [Fact] public void configuration_is_valid_if_qos_options_specified_and_has_qos_handler() - { + { var route = GivenDefaultRoute("/laura", "/"); route.Key = "Laura"; route.QoSOptions = new FileQoSOptions - { + { TimeoutValue = 1, ExceptionsAllowedBeforeBreaking = 1, }; @@ -145,7 +145,7 @@ public void configuration_is_valid_if_qos_options_specified_globally_and_has_qos route.Key = "Laura"; var configuration = GivenAConfiguration(route); configuration.GlobalConfiguration.QoSOptions = new FileQoSOptions - { + { TimeoutValue = 1, ExceptionsAllowedBeforeBreaking = 1, }; @@ -162,7 +162,7 @@ public void configuration_is_invalid_if_qos_options_specified_but_no_qos_handler var route = GivenDefaultRoute("/laura", "/"); route.Key = "Laura"; route.QoSOptions = new FileQoSOptions - { + { TimeoutValue = 1, ExceptionsAllowedBeforeBreaking = 1, }; @@ -181,7 +181,7 @@ public void configuration_is_invalid_if_qos_options_specified_globally_but_no_qo route.Key = "Laura"; var configuration = GivenAConfiguration(route); configuration.GlobalConfiguration.QoSOptions = new FileQoSOptions - { + { TimeoutValue = 1, ExceptionsAllowedBeforeBreaking = 1, }; @@ -213,7 +213,7 @@ public void configuration_is_valid_if_aggregates_are_valid() "Laura", ], }, - }; + }; this.Given(x => x.GivenAConfiguration(configuration)) .When(x => x.WhenIValidateTheConfiguration()) .Then(x => x.ThenTheResultIsValid()) @@ -241,7 +241,7 @@ public void configuration_is_invalid_if_aggregates_are_duplicate_of_routes() "Laura", ], }, - }; + }; this.Given(x => x.GivenAConfiguration(configuration)) .When(x => x.WhenIValidateTheConfiguration()) .Then(x => x.ThenTheResultIsNotValid()) @@ -270,7 +270,7 @@ public void configuration_is_valid_if_aggregates_are_not_duplicate_of_routes() "Laura", ], }, - }; + }; this.Given(x => x.GivenAConfiguration(configuration)) .When(x => x.WhenIValidateTheConfiguration()) .Then(x => x.ThenTheResultIsValid()) @@ -279,7 +279,7 @@ public void configuration_is_valid_if_aggregates_are_not_duplicate_of_routes() [Fact] public void configuration_is_invalid_if_aggregates_are_duplicate_of_aggregates() - { + { var route = GivenDefaultRoute("/laura", "/"); route.Key = "Laura"; var route2 = GivenDefaultRoute("/lol", "/"); @@ -307,7 +307,7 @@ public void configuration_is_invalid_if_aggregates_are_duplicate_of_aggregates() "Laura", ], }, - }; + }; this.Given(x => x.GivenAConfiguration(configuration)) .When(x => x.WhenIValidateTheConfiguration()) .Then(x => x.ThenTheResultIsNotValid()) @@ -362,7 +362,7 @@ public void configuration_is_invalid_if_aggregate_has_routes_with_specific_reque "Laura", ], }, - }; + }; this.Given(x => x.GivenAConfiguration(configuration)) .When(x => x.WhenIValidateTheConfiguration()) .Then(x => x.ThenTheResultIsNotValid()) @@ -408,7 +408,7 @@ public void configuration_is_invalid_without_slash_prefix_downstream_path_templa [Fact] public void configuration_is_invalid_without_slash_prefix_upstream_path_template() - { + { this.Given(x => x.GivenAConfiguration(GivenDefaultRoute("api/prod/", "/api/products/"))) .When(x => x.WhenIValidateTheConfiguration()) .Then(x => x.ThenTheResultIsNotValid()) @@ -438,7 +438,7 @@ public void configuration_is_invalid_if_downstream_url_contains_forward_slash_th [Fact] public void configuration_is_valid_with_valid_authentication_provider() - { + { var route = GivenDefaultRoute(); route.AuthenticationOptions.AuthenticationProviderKey = "Test"; this.Given(x => x.GivenAConfiguration(route)) @@ -466,14 +466,15 @@ public void configuration_is_invalid_with_invalid_authentication_provider() [Fact] public void configuration_is_not_valid_with_duplicate_routes_all_verbs() - { + { var route = GivenDefaultRoute(); var duplicate = GivenDefaultRoute(); duplicate.DownstreamPathTemplate = "/www/test/"; this.Given(x => x.GivenAConfiguration(route, duplicate)) .When(x => x.WhenIValidateTheConfiguration()) .Then(x => x.ThenTheResultIsNotValid()) - .And(x => x.ThenTheErrorMessageAtPositionIs(0, "route /asdf/ has duplicate")) + .And(x => x.ThenTheErrorMessageAtPositionIs( + 0, "route /asdf/ has duplicate upstream path or routing header mapping")) .BDDfy(); } @@ -499,7 +500,8 @@ public void configuration_is_not_valid_with_duplicate_routes_specific_verbs() this.Given(x => x.GivenAConfiguration(route, duplicate)) .When(x => x.WhenIValidateTheConfiguration()) .Then(x => x.ThenTheResultIsNotValid()) - .And(x => x.ThenTheErrorMessageAtPositionIs(0, "route /asdf/ has duplicate")) + .And(x => x.ThenTheErrorMessageAtPositionIs( + 0, "route /asdf/ has duplicate upstream path or routing header mapping")) .BDDfy(); } @@ -529,7 +531,8 @@ public void configuration_is_not_valid_with_duplicate_routes_with_duplicated_ups this.Given(x => x.GivenAConfiguration(route, duplicate)) .When(x => x.WhenIValidateTheConfiguration()) .Then(x => x.ThenTheResultIsNotValid()) - .And(x => x.ThenTheErrorMessageAtPositionIs(0, "route /asdf/ has duplicate")) + .And(x => x.ThenTheErrorMessageAtPositionIs( + 0, "route /asdf/ has duplicate upstream path or routing header mapping")) .BDDfy(); } @@ -608,14 +611,14 @@ public void configuration_is_valid_with_using_service_discovery_and_service_name .Then(x => x.ThenTheResultIsValid()) .BDDfy(); } - + private const string Empty = ""; [Theory] [InlineData(null)] [InlineData(Empty)] public void configuration_is_invalid_when_not_using_service_discovery_and_host(string downstreamHost) - { + { var route = GivenDefaultRoute(); route.DownstreamHostAndPorts[0].Host = downstreamHost; this.Given(x => x.GivenAConfiguration(route)) @@ -623,8 +626,8 @@ public void configuration_is_invalid_when_not_using_service_discovery_and_host(s .Then(x => x.ThenTheResultIsNotValid()) .And(x => x.ThenTheErrorMessageAtPositionIs(0, "When not using service discovery Host must be set on DownstreamHostAndPorts if you are not using Route.Host or Ocelot cannot find your service!")) .BDDfy(); - } - + } + [Theory] [InlineData(null, true)] [InlineData(Empty, true)] @@ -651,14 +654,14 @@ public void HaveServiceDiscoveryProviderRegistered_RouteServiceName_Validated(st [InlineData(true, "type", false)] [InlineData(true, "servicefabric", true)] public void HaveServiceDiscoveryProviderRegistered_ServiceDiscoveryProvider_Validated(bool create, string type, bool valid) - { + { // Arrange var route = GivenServiceDiscoveryRoute(); var config = GivenAConfiguration(route); var provider = create ? GivenDefaultServiceDiscoveryProvider() : null; config.GlobalConfiguration.ServiceDiscoveryProvider = provider; if (create && provider != null) - { + { provider.Type = type; } @@ -757,8 +760,8 @@ public void configuration_is_invalid_when_placeholder_is_used_twice_in_upstream_ .Then(x => x.ThenTheResultIsNotValid()) .And(x => x.ThenTheErrorMessageAtPositionIs(0, "route /foo/bar/{everything}/{everything} has duplicated placeholder")) .BDDfy(); - } - + } + private FileRoute GivenDefaultRoute() => GivenDefaultRoute(null, null); private FileRoute GivenDefaultRoute(string upstreamPathTemplate, string downstreamPathTemplate) => new() @@ -784,7 +787,7 @@ private FileRoute GivenServiceDiscoveryRoute() => new() private void GivenAConfiguration(FileConfiguration fileConfiguration) { _fileConfiguration = fileConfiguration; - } + } private FileConfiguration GivenAConfiguration(params FileRoute[] routes) { @@ -793,7 +796,7 @@ private FileConfiguration GivenAConfiguration(params FileRoute[] routes) _fileConfiguration = config; return config; } - + private FileServiceDiscoveryProvider GivenDefaultServiceDiscoveryProvider() => new FileServiceDiscoveryProvider { Scheme = "https", @@ -859,7 +862,7 @@ private class FakeServiceDiscoveryProvider : IServiceDiscoveryProvider private class TestOptions : AuthenticationSchemeOptions { } private class TestHandler : AuthenticationHandler - { + { // https://learn.microsoft.com/en-us/dotnet/core/compatibility/aspnet-core/8.0/isystemclock-obsolete // .NET 8.0: TimeProvider is now a settable property on the Options classes for the authentication and identity components. // It can be set directly or by registering a provider in the dependency injection container. @@ -870,9 +873,9 @@ public TestHandler(IOptionsMonitor options, ILoggerFactory logger, #else public TestHandler(IOptionsMonitor options, ILoggerFactory logger, UrlEncoder encoder, ISystemClock clock) : base(options, logger, encoder, clock) { - } + } #endif - + protected override Task HandleAuthenticateAsync() { var principal = new ClaimsPrincipal(); From a70a27a1ecc57690bb8cd4d6885235c1d4b4f68d Mon Sep 17 00:00:00 2001 From: Zbynek Novotny Date: Sat, 5 Aug 2023 13:41:03 +0200 Subject: [PATCH 03/13] Integration of feedback by @RaynaldM in the PR. --- .../Configuration/File/FileUpstreamHeaderRoutingOptions.cs | 4 ++-- src/Ocelot/Configuration/UpstreamHeaderRoutingOptions.cs | 2 +- .../Validator/FileConfigurationFluentValidator.cs | 4 ++-- .../DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs | 3 ++- .../Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs | 2 +- 5 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Ocelot/Configuration/File/FileUpstreamHeaderRoutingOptions.cs b/src/Ocelot/Configuration/File/FileUpstreamHeaderRoutingOptions.cs index 23d6a32d5..7e6a63a5e 100644 --- a/src/Ocelot/Configuration/File/FileUpstreamHeaderRoutingOptions.cs +++ b/src/Ocelot/Configuration/File/FileUpstreamHeaderRoutingOptions.cs @@ -6,11 +6,11 @@ public class FileUpstreamHeaderRoutingOptions { public FileUpstreamHeaderRoutingOptions() { - Headers = new(); + Headers = new Dictionary>(); TriggerOn = string.Empty; } - public Dictionary> Headers { get; set; } + public IDictionary> Headers { get; set; } public string TriggerOn { get; set; } } diff --git a/src/Ocelot/Configuration/UpstreamHeaderRoutingOptions.cs b/src/Ocelot/Configuration/UpstreamHeaderRoutingOptions.cs index 3496d86c4..576dae05d 100644 --- a/src/Ocelot/Configuration/UpstreamHeaderRoutingOptions.cs +++ b/src/Ocelot/Configuration/UpstreamHeaderRoutingOptions.cs @@ -4,7 +4,7 @@ namespace Ocelot.Configuration { public class UpstreamHeaderRoutingOptions { - public UpstreamHeaderRoutingOptions(Dictionary> headers, UpstreamHeaderRoutingTriggerMode mode) + public UpstreamHeaderRoutingOptions(IReadOnlyDictionary> headers, UpstreamHeaderRoutingTriggerMode mode) { Headers = new UpstreamRoutingHeaders(headers); Mode = mode; diff --git a/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs b/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs index eec9aa35c..5de675b7d 100644 --- a/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs +++ b/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs @@ -183,8 +183,8 @@ private static bool AreDuplicateUpstreamRoutingHeaders(FileRoute first, FileRout foreach (var (key, values) in first.UpstreamHeaderRoutingOptions.Headers) { - IDictionary> secondHeaders = second.UpstreamHeaderRoutingOptions.Headers; - if (!secondHeaders.TryGetValue(key, out List secondHeaderValues)) + IDictionary> secondHeaders = second.UpstreamHeaderRoutingOptions.Headers; + if (!secondHeaders.TryGetValue(key, out IList secondHeaderValues)) { continue; } diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs index bec039365..498216267 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs +++ b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs @@ -60,7 +60,8 @@ private static bool RouteIsApplicableToThisRequest(Route route, string httpMetho private static bool RouteHasHttpMethod(Route route, string httpMethod) { - return route.UpstreamHttpMethod.Select(x => x.Method.ToLower()).Contains(httpMethod.ToLower()); + httpMethod = httpMethod.ToLower(); + return route.UpstreamHttpMethod.Select(x => x.Method.ToLower()).Contains(httpMethod); } private static bool RouteHasRequiredUpstreamHeaders(Route route, IHeaderDictionary requestHeaders) diff --git a/test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs index 049bc7d75..942be3843 100644 --- a/test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs @@ -42,7 +42,7 @@ private void GivenTheseFileUpstreamHeaderRoutingOptions() { _fileUpstreamHeaderRoutingOptions = new FileUpstreamHeaderRoutingOptions() { - Headers = new Dictionary>() + Headers = new Dictionary>() { { "Header1", new List() { "Value1", "Value2" }}, { "Header2", new List() { "Value3" }}, From 378ad627dd1e22e4268b00ef8b9be9902902b44d Mon Sep 17 00:00:00 2001 From: Zbynek Novotny Date: Sat, 19 Aug 2023 18:49:19 +0200 Subject: [PATCH 04/13] Integration of additional PR feedback --- docs/features/routing.rst | 32 ++- .../IUpstreamHeaderRoutingOptionsCreator.cs | 9 +- .../UpstreamHeaderRoutingOptionsCreator.cs | 32 +-- .../File/FileUpstreamHeaderRoutingOptions.cs | 17 +- .../UpstreamHeaderRoutingOptions.cs | 21 +- .../UpstreamHeaderRoutingTriggerMode.cs | 11 +- .../Configuration/UpstreamRoutingHeaders.cs | 88 +++---- .../FileConfigurationFluentValidator.cs | 40 ++- .../Finder/DownstreamRouteFinder.cs | 28 +- ...pstreamHeaderRoutingOptionsCreatorTests.cs | 97 ++++--- .../UpstreamRoutingHeadersTests.cs | 239 +++++++++--------- .../DownstreamRouteFinderTests.cs | 4 +- 12 files changed, 302 insertions(+), 316 deletions(-) diff --git a/docs/features/routing.rst b/docs/features/routing.rst index 546319db9..d39c5b6eb 100644 --- a/docs/features/routing.rst +++ b/docs/features/routing.rst @@ -307,13 +307,41 @@ A sample configuration might look like the following: { "Routes": [ { + "DownstreamPathTemplate": "/api/posts", + "DownstreamScheme": "http", + "DownstreamHostAndPorts": [ + { + "Host": "server1", + "Port": 80, + } + ], + "UpstreamPathTemplate": "/posts", + "UpstreamHttpMethod": [ "Put", "Delete" ] "UpstreamHeaderRoutingOptions": { "Headers": { - "X-API-Version": [ "1", "2" ], - "X-Tennant-Id": [ "tennantId" ] + "X-API-Version": [ "1" ], + "X-Tenant-Id": [ "tenantId" ] }, "TriggerOn": "all" } + }, + { + "DownstreamPathTemplate": "/api/posts", + "DownstreamScheme": "http", + "DownstreamHostAndPorts": [ + { + "Host": "server2", + "Port": 80, + } + ], + "UpstreamPathTemplate": "/posts", + "UpstreamHttpMethod": [ "Put", "Delete" ] + "UpstreamHeaderRoutingOptions": { + "Headers": { + "X-API-Version": [ "2" ] + }, + "TriggerOn": "any" + } } ] } diff --git a/src/Ocelot/Configuration/Creator/IUpstreamHeaderRoutingOptionsCreator.cs b/src/Ocelot/Configuration/Creator/IUpstreamHeaderRoutingOptionsCreator.cs index 8c965d08f..bd26ad130 100644 --- a/src/Ocelot/Configuration/Creator/IUpstreamHeaderRoutingOptionsCreator.cs +++ b/src/Ocelot/Configuration/Creator/IUpstreamHeaderRoutingOptionsCreator.cs @@ -1,9 +1,8 @@ using Ocelot.Configuration.File; -namespace Ocelot.Configuration.Creator +namespace Ocelot.Configuration.Creator; + +public interface IUpstreamHeaderRoutingOptionsCreator { - public interface IUpstreamHeaderRoutingOptionsCreator - { - UpstreamHeaderRoutingOptions Create(FileUpstreamHeaderRoutingOptions options); - } + UpstreamHeaderRoutingOptions Create(FileUpstreamHeaderRoutingOptions options); } diff --git a/src/Ocelot/Configuration/Creator/UpstreamHeaderRoutingOptionsCreator.cs b/src/Ocelot/Configuration/Creator/UpstreamHeaderRoutingOptionsCreator.cs index 730d0f654..4e9f766e5 100644 --- a/src/Ocelot/Configuration/Creator/UpstreamHeaderRoutingOptionsCreator.cs +++ b/src/Ocelot/Configuration/Creator/UpstreamHeaderRoutingOptionsCreator.cs @@ -1,25 +1,25 @@ -using System; -using System.Linq; -using System.Collections.Generic; using Ocelot.Configuration.File; -namespace Ocelot.Configuration.Creator +namespace Ocelot.Configuration.Creator; + +public class UpstreamHeaderRoutingOptionsCreator : IUpstreamHeaderRoutingOptionsCreator { - public class UpstreamHeaderRoutingOptionsCreator : IUpstreamHeaderRoutingOptionsCreator + public UpstreamHeaderRoutingOptions Create(FileUpstreamHeaderRoutingOptions options) { - public UpstreamHeaderRoutingOptions Create(FileUpstreamHeaderRoutingOptions options) + var mode = UpstreamHeaderRoutingTriggerMode.Any; + if (options.TriggerOn.Length > 0) { - UpstreamHeaderRoutingTriggerMode mode = UpstreamHeaderRoutingTriggerMode.Any; - if (options.TriggerOn.Length > 0) - { - mode = Enum.Parse(options.TriggerOn, true); - } + mode = Enum.Parse(options.TriggerOn, true); + } - Dictionary> headers = options.Headers.ToDictionary( - kv => kv.Key.ToLowerInvariant(), - kv => new HashSet(kv.Value.Select(v => v.ToLowerInvariant()))); + // Keys are converted to uppercase as apparently that is the preferred + // approach according to https://learn.microsoft.com/en-us/dotnet/standard/base-types/best-practices-strings + // Values are left untouched but value comparison at runtime is done in + // a case-insensitive manner by using the appropriate StringComparer. + var headers = options.Headers.ToDictionary( + kv => kv.Key.ToUpperInvariant(), + kv => kv.Value); - return new UpstreamHeaderRoutingOptions(headers, mode); - } + return new UpstreamHeaderRoutingOptions(headers, mode); } } diff --git a/src/Ocelot/Configuration/File/FileUpstreamHeaderRoutingOptions.cs b/src/Ocelot/Configuration/File/FileUpstreamHeaderRoutingOptions.cs index 7e6a63a5e..3aaa35320 100644 --- a/src/Ocelot/Configuration/File/FileUpstreamHeaderRoutingOptions.cs +++ b/src/Ocelot/Configuration/File/FileUpstreamHeaderRoutingOptions.cs @@ -1,17 +1,8 @@ -using System.Collections.Generic; +namespace Ocelot.Configuration.File; -namespace Ocelot.Configuration.File +public class FileUpstreamHeaderRoutingOptions { - public class FileUpstreamHeaderRoutingOptions - { - public FileUpstreamHeaderRoutingOptions() - { - Headers = new Dictionary>(); - TriggerOn = string.Empty; - } + public IDictionary> Headers { get; set; } = new Dictionary>(); - public IDictionary> Headers { get; set; } - - public string TriggerOn { get; set; } - } + public string TriggerOn { get; set; } = string.Empty; } diff --git a/src/Ocelot/Configuration/UpstreamHeaderRoutingOptions.cs b/src/Ocelot/Configuration/UpstreamHeaderRoutingOptions.cs index 576dae05d..eac9ad9ad 100644 --- a/src/Ocelot/Configuration/UpstreamHeaderRoutingOptions.cs +++ b/src/Ocelot/Configuration/UpstreamHeaderRoutingOptions.cs @@ -1,19 +1,16 @@ -using System.Collections.Generic; +namespace Ocelot.Configuration; -namespace Ocelot.Configuration +public class UpstreamHeaderRoutingOptions { - public class UpstreamHeaderRoutingOptions + public UpstreamHeaderRoutingOptions(IReadOnlyDictionary> headers, UpstreamHeaderRoutingTriggerMode mode) { - public UpstreamHeaderRoutingOptions(IReadOnlyDictionary> headers, UpstreamHeaderRoutingTriggerMode mode) - { - Headers = new UpstreamRoutingHeaders(headers); - Mode = mode; - } + Headers = new UpstreamRoutingHeaders(headers); + Mode = mode; + } - public bool Enabled() => Headers.Any(); + public bool Enabled() => Headers.Any(); - public UpstreamRoutingHeaders Headers { get; } + public UpstreamRoutingHeaders Headers { get; } - public UpstreamHeaderRoutingTriggerMode Mode { get; } - } + public UpstreamHeaderRoutingTriggerMode Mode { get; } } diff --git a/src/Ocelot/Configuration/UpstreamHeaderRoutingTriggerMode.cs b/src/Ocelot/Configuration/UpstreamHeaderRoutingTriggerMode.cs index 8596ae7f1..47a3e636e 100644 --- a/src/Ocelot/Configuration/UpstreamHeaderRoutingTriggerMode.cs +++ b/src/Ocelot/Configuration/UpstreamHeaderRoutingTriggerMode.cs @@ -1,8 +1,7 @@ -namespace Ocelot.Configuration +namespace Ocelot.Configuration; + +public enum UpstreamHeaderRoutingTriggerMode : byte { - public enum UpstreamHeaderRoutingTriggerMode - { - Any = 0, - All = 1, - } + Any, + All, } diff --git a/src/Ocelot/Configuration/UpstreamRoutingHeaders.cs b/src/Ocelot/Configuration/UpstreamRoutingHeaders.cs index 119155c1e..d3c973d9c 100644 --- a/src/Ocelot/Configuration/UpstreamRoutingHeaders.cs +++ b/src/Ocelot/Configuration/UpstreamRoutingHeaders.cs @@ -1,70 +1,62 @@ -using System.Collections.Generic; -using System.Linq; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Primitives; -namespace Ocelot.Configuration +namespace Ocelot.Configuration; + +public class UpstreamRoutingHeaders { - public class UpstreamRoutingHeaders - { - public IReadOnlyDictionary> Headers { get; } + public IReadOnlyDictionary> Headers { get; } - public UpstreamRoutingHeaders(IReadOnlyDictionary> headers) - { - Headers = headers; - } + public UpstreamRoutingHeaders(IReadOnlyDictionary> headers) + { + Headers = headers; + } - public bool Any() => Headers.Any(); + public bool Any() => Headers.Any(); - public bool HasAnyOf(IHeaderDictionary requestHeaders) + public bool HasAnyOf(IHeaderDictionary requestHeaders) + { + IHeaderDictionary normalizedHeaders = NormalizeHeaderNames(requestHeaders); + foreach (var h in Headers) { - IHeaderDictionary lowerCaseHeaders = GetLowerCaseHeaders(requestHeaders); - foreach (KeyValuePair> h in Headers) + if (normalizedHeaders.TryGetValue(h.Key, out var values) && + h.Value.Intersect(values, StringComparer.OrdinalIgnoreCase).Any()) { - if (lowerCaseHeaders.TryGetValue(h.Key, out var values)) - { - HashSet requestHeaderValues = new(values); - if (h.Value.Overlaps(requestHeaderValues)) - { - return true; - } - } + return true; } - - return false; } - public bool HasAllOf(IHeaderDictionary requestHeaders) + return false; + } + + public bool HasAllOf(IHeaderDictionary requestHeaders) + { + IHeaderDictionary normalizedHeaders = NormalizeHeaderNames(requestHeaders); + foreach (var h in Headers) { - IHeaderDictionary lowerCaseHeaders = GetLowerCaseHeaders(requestHeaders); - foreach (KeyValuePair> h in Headers) + if (!normalizedHeaders.TryGetValue(h.Key, out var values)) { - if (!lowerCaseHeaders.TryGetValue(h.Key, out var values)) - { - return false; - } - - HashSet requestHeaderValues = new(values); - if (!h.Value.Overlaps(requestHeaderValues)) - { - return false; - } + return false; } - return true; - } - - private static IHeaderDictionary GetLowerCaseHeaders(IHeaderDictionary headers) - { - IHeaderDictionary lowerCaseHeaders = new HeaderDictionary(); - foreach (KeyValuePair kv in headers) + if (!h.Value.Intersect(values, StringComparer.OrdinalIgnoreCase).Any()) { - string key = kv.Key.ToLowerInvariant(); - StringValues values = new(kv.Value.Select(v => v.ToLowerInvariant()).ToArray()); - lowerCaseHeaders.Add(key, values); + return false; } + } - return lowerCaseHeaders; + return true; + } + + private static IHeaderDictionary NormalizeHeaderNames(IHeaderDictionary headers) + { + var upperCaseHeaders = new HeaderDictionary(); + foreach (KeyValuePair kv in headers) + { + var key = kv.Key.ToUpperInvariant(); + upperCaseHeaders.Add(key, kv.Value); } + + return upperCaseHeaders; } } diff --git a/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs b/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs index 5de675b7d..2d9455aa5 100644 --- a/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs +++ b/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs @@ -116,7 +116,9 @@ private static bool IsPlaceholderNotDuplicatedIn(string upstreamPathTemplate) var matchingRoutes = routes .Where(r => r.UpstreamPathTemplate == route.UpstreamPathTemplate && r.UpstreamHost == route.UpstreamHost - && AreDuplicateUpstreamRoutingHeaders(route, r)) + && AreDuplicateUpstreamRoutingHeaders( + route.UpstreamHeaderRoutingOptions.Headers, + r.UpstreamHeaderRoutingOptions.Headers)) .ToList(); if (matchingRoutes.Count == 1) @@ -158,45 +160,39 @@ private static bool IsNotDuplicateIn(FileAggregateRoute route, IEnumerable> first, + IDictionary> second) { - if (!first.UpstreamHeaderRoutingOptions.Headers.Any() && !second.UpstreamHeaderRoutingOptions.Headers.Any()) + if (!first.Any() && !second.Any()) { return true; } - if (first.UpstreamHeaderRoutingOptions.Headers.Any() ^ second.UpstreamHeaderRoutingOptions.Headers.Any()) + if (first.Any() ^ second.Any()) { return false; } - ISet firstKeySet = first.UpstreamHeaderRoutingOptions.Headers.Keys - .Select(k => k.ToLowerInvariant()) - .ToHashSet(); - ISet secondKeySet = second.UpstreamHeaderRoutingOptions.Headers.Keys - .Select(k => k.ToLowerInvariant()) - .ToHashSet(); - if (!firstKeySet.Overlaps(secondKeySet)) + var firstKeySet = first.Keys + .Select(k => k.ToUpperInvariant()) + .ToArray(); + var secondKeySet = second.Keys + .Select(k => k.ToUpperInvariant()) + .ToArray(); + if (!firstKeySet.Intersect(secondKeySet).Any()) { return false; } - foreach (var (key, values) in first.UpstreamHeaderRoutingOptions.Headers) + foreach (var (key, firstValues) in first) { - IDictionary> secondHeaders = second.UpstreamHeaderRoutingOptions.Headers; - if (!secondHeaders.TryGetValue(key, out IList secondHeaderValues)) + if (!second.TryGetValue(key, out var secondValues)) { continue; } - ISet firstHeaderValuesLowerCase = values - .Select(v => v.ToLowerInvariant()) - .ToHashSet(); - ISet secondHeaderValuesLowerCase = secondHeaderValues - .Select(v => v.ToLowerInvariant()) - .ToHashSet(); - - if (firstHeaderValuesLowerCase.Overlaps(secondHeaderValuesLowerCase)) + if (firstValues.Intersect(secondValues, StringComparer.OrdinalIgnoreCase).Any()) { return true; } diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs index 498216267..cecca0a17 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs +++ b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs @@ -55,30 +55,16 @@ private static bool RouteIsApplicableToThisRequest(Route route, string httpMetho { return (route.UpstreamHttpMethod.Count == 0 || RouteHasHttpMethod(route, httpMethod)) && (string.IsNullOrEmpty(route.UpstreamHost) || route.UpstreamHost == upstreamHost) && - (route.UpstreamHeaderRoutingOptions == null || !route.UpstreamHeaderRoutingOptions.Enabled() || RouteHasRequiredUpstreamHeaders(route, requestHeaders)); + (!(route.UpstreamHeaderRoutingOptions?.Enabled() ?? false) || RouteHasRequiredUpstreamHeaders(route, requestHeaders)); } - private static bool RouteHasHttpMethod(Route route, string httpMethod) - { - httpMethod = httpMethod.ToLower(); - return route.UpstreamHttpMethod.Select(x => x.Method.ToLower()).Contains(httpMethod); - } + private static bool RouteHasHttpMethod(Route route, string httpMethod) => + route.UpstreamHttpMethod.Select(x => x.Method).Contains(httpMethod, StringComparer.OrdinalIgnoreCase); - private static bool RouteHasRequiredUpstreamHeaders(Route route, IHeaderDictionary requestHeaders) - { - bool result = false; - switch (route.UpstreamHeaderRoutingOptions.Mode) - { - case UpstreamHeaderRoutingTriggerMode.Any: - result = route.UpstreamHeaderRoutingOptions.Headers.HasAnyOf(requestHeaders); - break; - case UpstreamHeaderRoutingTriggerMode.All: - result = route.UpstreamHeaderRoutingOptions.Headers.HasAllOf(requestHeaders); - break; - } - - return result; - } + private static bool RouteHasRequiredUpstreamHeaders(Route route, IHeaderDictionary requestHeaders) => + route.UpstreamHeaderRoutingOptions.Mode == UpstreamHeaderRoutingTriggerMode.Any + ? route.UpstreamHeaderRoutingOptions.Headers.HasAnyOf(requestHeaders) + : route.UpstreamHeaderRoutingOptions.Headers.HasAllOf(requestHeaders); private DownstreamRouteHolder GetPlaceholderNamesAndValues(string path, string query, Route route) { diff --git a/test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs index 942be3843..178a83ac2 100644 --- a/test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs @@ -7,65 +7,64 @@ using TestStack.BDDfy; using Shouldly; -namespace Ocelot.UnitTests.Configuration +namespace Ocelot.UnitTests.Configuration; + +public class UpstreamHeaderRoutingOptionsCreatorTests { - public class UpstreamHeaderRoutingOptionsCreatorTests - { - private FileUpstreamHeaderRoutingOptions _fileUpstreamHeaderRoutingOptions; - private IUpstreamHeaderRoutingOptionsCreator _creator; - private UpstreamHeaderRoutingOptions _upstreamHeaderRoutingOptions; + private FileUpstreamHeaderRoutingOptions _fileUpstreamHeaderRoutingOptions; + private IUpstreamHeaderRoutingOptionsCreator _creator; + private UpstreamHeaderRoutingOptions _upstreamHeaderRoutingOptions; - public UpstreamHeaderRoutingOptionsCreatorTests() - { - _creator = new UpstreamHeaderRoutingOptionsCreator(); - } + public UpstreamHeaderRoutingOptionsCreatorTests() + { + _creator = new UpstreamHeaderRoutingOptionsCreator(); + } - [Fact] - public void should_create_upstream_routing_header_options() - { - UpstreamHeaderRoutingOptions expected = new UpstreamHeaderRoutingOptions( - headers: new Dictionary>() - { - { "header1", new HashSet() { "value1", "value2" }}, - { "header2", new HashSet() { "value3" }}, - }, - mode: UpstreamHeaderRoutingTriggerMode.All - ); + [Fact] + public void should_create_upstream_routing_header_options() + { + UpstreamHeaderRoutingOptions expected = new( + headers: new Dictionary>() + { + { "HEADER1", new[] { "Value1", "Value2" }}, + { "HEADER2", new[] { "Value3" }}, + }, + mode: UpstreamHeaderRoutingTriggerMode.All + ); - this.Given(_ => GivenTheseFileUpstreamHeaderRoutingOptions()) - .When(_ => WhenICreate()) - .Then(_ => ThenTheCreatedMatchesThis(expected)) - .BDDfy(); - } + this.Given(_ => GivenTheseFileUpstreamHeaderRoutingOptions()) + .When(_ => WhenICreate()) + .Then(_ => ThenTheCreatedMatchesThis(expected)) + .BDDfy(); + } - private void GivenTheseFileUpstreamHeaderRoutingOptions() + private void GivenTheseFileUpstreamHeaderRoutingOptions() + { + _fileUpstreamHeaderRoutingOptions = new FileUpstreamHeaderRoutingOptions() { - _fileUpstreamHeaderRoutingOptions = new FileUpstreamHeaderRoutingOptions() + Headers = new Dictionary>() { - Headers = new Dictionary>() - { - { "Header1", new List() { "Value1", "Value2" }}, - { "Header2", new List() { "Value3" }}, - }, - TriggerOn = "all", - }; - } + { "Header1", new[] { "Value1", "Value2" }}, + { "Header2", new[] { "Value3" }}, + }, + TriggerOn = "all", + }; + } - private void WhenICreate() - { - _upstreamHeaderRoutingOptions = _creator.Create(_fileUpstreamHeaderRoutingOptions); - } + private void WhenICreate() + { + _upstreamHeaderRoutingOptions = _creator.Create(_fileUpstreamHeaderRoutingOptions); + } - private void ThenTheCreatedMatchesThis(UpstreamHeaderRoutingOptions expected) + private void ThenTheCreatedMatchesThis(UpstreamHeaderRoutingOptions expected) + { + _upstreamHeaderRoutingOptions.Headers.Headers.Count.ShouldBe(expected.Headers.Headers.Count); + foreach (var pair in _upstreamHeaderRoutingOptions.Headers.Headers) { - _upstreamHeaderRoutingOptions.Headers.Headers.Count.ShouldBe(expected.Headers.Headers.Count); - foreach (KeyValuePair> pair in _upstreamHeaderRoutingOptions.Headers.Headers) - { - expected.Headers.Headers.TryGetValue(pair.Key, out var expectedValue).ShouldBe(true); - expectedValue.SetEquals(pair.Value).ShouldBe(true); - } - - _upstreamHeaderRoutingOptions.Mode.ShouldBe(expected.Mode); + expected.Headers.Headers.TryGetValue(pair.Key, out var expectedValue).ShouldBe(true); + expectedValue.ShouldBeEquivalentTo(pair.Value); } + + _upstreamHeaderRoutingOptions.Mode.ShouldBe(expected.Mode); } } diff --git a/test/Ocelot.UnitTests/Configuration/UpstreamRoutingHeadersTests.cs b/test/Ocelot.UnitTests/Configuration/UpstreamRoutingHeadersTests.cs index 87eb5306a..18bb115ad 100644 --- a/test/Ocelot.UnitTests/Configuration/UpstreamRoutingHeadersTests.cs +++ b/test/Ocelot.UnitTests/Configuration/UpstreamRoutingHeadersTests.cs @@ -6,137 +6,136 @@ using Shouldly; using Ocelot.Configuration; -namespace Ocelot.UnitTests.Configuration +namespace Ocelot.UnitTests.Configuration; + +public class UpstreamRoutingHeadersTests { - public class UpstreamRoutingHeadersTests + private IReadOnlyDictionary> _headersDictionary; + private UpstreamRoutingHeaders _upstreamRoutingHeaders; + private IHeaderDictionary _requestHeaders; + + [Fact] + public void should_create_empty_headers() { - private Dictionary> _headersDictionary; - private UpstreamRoutingHeaders _upstreamRoutingHeaders; - private IHeaderDictionary _requestHeaders; + this.Given(_ => GivenEmptyHeaderDictionary()) + .When(_ => WhenICreate()) + .Then(_ => ThenAnyIs(false)) + .BDDfy(); + } - [Fact] - public void should_create_empty_headers() - { - this.Given(_ => GivenEmptyHeaderDictionary()) - .When(_ => WhenICreate()) - .Then(_ => ThenAnyIs(false)) - .BDDfy(); - } - - [Fact] - public void should_create_preset_headers() - { - this.Given(_ => GivenPresetHeaderDictionary()) - .When(_ => WhenICreate()) - .Then(_ => ThenAnyIs(true)) - .BDDfy(); - } - - [Fact] - public void should_not_match_mismatching_request_headers() - { - this.Given(_ => GivenPresetHeaderDictionary()) - .And(_ => AndGivenMismatchingRequestHeaders()) - .When(_ => WhenICreate()) - .Then(_ => ThenHasAnyOfIs(false)) - .And(_ => ThenHasAllOfIs(false)) - .BDDfy(); - } - - [Fact] - public void should_not_match_matching_header_with_mismatching_value() - { - this.Given(_ => GivenPresetHeaderDictionary()) - .And(_ => AndGivenOneMatchingHeaderWithMismatchingValue()) - .When(_ => WhenICreate()) - .Then(_ => ThenHasAnyOfIs(false)) - .And(_ => ThenHasAllOfIs(false)) - .BDDfy(); - } - - [Fact] - public void should_match_any_header_not_all() - { - this.Given(_ => GivenPresetHeaderDictionary()) - .And(_ => AndGivenOneMatchingHeaderWithMatchingValue()) - .When(_ => WhenICreate()) - .Then(_ => ThenHasAnyOfIs(true)) - .And(_ => ThenHasAllOfIs(false)) - .BDDfy(); - } - - [Fact] - public void should_match_any_and_all_headers() - { - this.Given(_ => GivenPresetHeaderDictionary()) - .And(_ => AndGivenTwoMatchingHeadersWithMatchingValues()) - .When(_ => WhenICreate()) - .Then(_ => ThenHasAnyOfIs(true)) - .And(_ => ThenHasAllOfIs(true)) - .BDDfy(); - } - - private void GivenEmptyHeaderDictionary() - { - _headersDictionary = new Dictionary>(); - } + [Fact] + public void should_create_preset_headers() + { + this.Given(_ => GivenPresetHeaderDictionary()) + .When(_ => WhenICreate()) + .Then(_ => ThenAnyIs(true)) + .BDDfy(); + } - private void GivenPresetHeaderDictionary() - { - _headersDictionary = new Dictionary>() - { - { "testheader1", new HashSet() { "testheader1value1", "testheader1value2" } }, - { "testheader2", new HashSet() { "testheader1Value1", "testheader2value2" } }, - }; - } - - private void AndGivenMismatchingRequestHeaders() - { - _requestHeaders = new HeaderDictionary() { - { "someHeader", new StringValues(new []{ "someHeaderValue" })}, - }; - } + [Fact] + public void should_not_match_mismatching_request_headers() + { + this.Given(_ => GivenPresetHeaderDictionary()) + .And(_ => AndGivenMismatchingRequestHeaders()) + .When(_ => WhenICreate()) + .Then(_ => ThenHasAnyOfIs(false)) + .And(_ => ThenHasAllOfIs(false)) + .BDDfy(); + } - private void AndGivenOneMatchingHeaderWithMismatchingValue() - { - _requestHeaders = new HeaderDictionary() { - { "testHeader1", new StringValues(new []{ "mismatchingValue" })}, - }; - } + [Fact] + public void should_not_match_matching_header_with_mismatching_value() + { + this.Given(_ => GivenPresetHeaderDictionary()) + .And(_ => AndGivenOneMatchingHeaderWithMismatchingValue()) + .When(_ => WhenICreate()) + .Then(_ => ThenHasAnyOfIs(false)) + .And(_ => ThenHasAllOfIs(false)) + .BDDfy(); + } - private void AndGivenOneMatchingHeaderWithMatchingValue() - { - _requestHeaders = new HeaderDictionary() { - { "testHeader1", new StringValues(new []{ "testHeader1Value1" })}, - }; - } + [Fact] + public void should_match_any_header_not_all() + { + this.Given(_ => GivenPresetHeaderDictionary()) + .And(_ => AndGivenOneMatchingHeaderWithMatchingValue()) + .When(_ => WhenICreate()) + .Then(_ => ThenHasAnyOfIs(true)) + .And(_ => ThenHasAllOfIs(false)) + .BDDfy(); + } - private void AndGivenTwoMatchingHeadersWithMatchingValues() - { - _requestHeaders = new HeaderDictionary() { - { "testHeader1", new StringValues(new []{ "testHeader1Value1", "bogusValue" })}, - { "testHeader2", new StringValues(new []{ "bogusValue", "testHeader2Value2" })}, - }; - } + [Fact] + public void should_match_any_and_all_headers() + { + this.Given(_ => GivenPresetHeaderDictionary()) + .And(_ => AndGivenTwoMatchingHeadersWithMatchingValues()) + .When(_ => WhenICreate()) + .Then(_ => ThenHasAnyOfIs(true)) + .And(_ => ThenHasAllOfIs(true)) + .BDDfy(); + } - private void WhenICreate() - { - _upstreamRoutingHeaders = new UpstreamRoutingHeaders(_headersDictionary); - } + private void GivenEmptyHeaderDictionary() + { + _headersDictionary = new Dictionary>(); + } - private void ThenAnyIs(bool expected) + private void GivenPresetHeaderDictionary() + { + _headersDictionary = new Dictionary>() { - _upstreamRoutingHeaders.Any().ShouldBe(expected); - } + { "testheader1", new HashSet() { "testheader1value1", "testheader1value2" } }, + { "testheader2", new HashSet() { "testheader1Value1", "testheader2value2" } }, + }; + } - private void ThenHasAnyOfIs(bool expected) - { - _upstreamRoutingHeaders.HasAnyOf(_requestHeaders).ShouldBe(expected); - } + private void AndGivenMismatchingRequestHeaders() + { + _requestHeaders = new HeaderDictionary() { + { "someHeader", new StringValues(new []{ "someHeaderValue" })}, + }; + } - private void ThenHasAllOfIs(bool expected) - { - _upstreamRoutingHeaders.HasAllOf(_requestHeaders).ShouldBe(expected); - } + private void AndGivenOneMatchingHeaderWithMismatchingValue() + { + _requestHeaders = new HeaderDictionary() { + { "testHeader1", new StringValues(new []{ "mismatchingValue" })}, + }; + } + + private void AndGivenOneMatchingHeaderWithMatchingValue() + { + _requestHeaders = new HeaderDictionary() { + { "testHeader1", new StringValues(new []{ "testHeader1Value1" })}, + }; + } + + private void AndGivenTwoMatchingHeadersWithMatchingValues() + { + _requestHeaders = new HeaderDictionary() { + { "testHeader1", new StringValues(new []{ "testHeader1Value1", "bogusValue" })}, + { "testHeader2", new StringValues(new []{ "bogusValue", "testHeader2Value2" })}, + }; + } + + private void WhenICreate() + { + _upstreamRoutingHeaders = new UpstreamRoutingHeaders(_headersDictionary); + } + + private void ThenAnyIs(bool expected) + { + _upstreamRoutingHeaders.Any().ShouldBe(expected); + } + + private void ThenHasAnyOfIs(bool expected) + { + _upstreamRoutingHeaders.HasAnyOf(_requestHeaders).ShouldBe(expected); + } + + private void ThenHasAllOfIs(bool expected) + { + _upstreamRoutingHeaders.HasAllOf(_requestHeaders).ShouldBe(expected); } } diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs index 85c4528a4..f265d9e17 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs +++ b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs @@ -823,9 +823,9 @@ private void GivenTheUpstreamHttpMethodIs(string upstreamHttpMethod) private void GivenUpstreamHeaderRoutingOptions() { - var headers = new Dictionary>() + var headers = new Dictionary>() { - { "header", new HashSet() { "value" }}, + { "header", new[] { "value" }}, }; _upstreamHeaderRoutingOptions = new UpstreamHeaderRoutingOptions(headers, UpstreamHeaderRoutingTriggerMode.All); } From bfbc586c7d867cf1ccf5c91a59cf201bec255438 Mon Sep 17 00:00:00 2001 From: raman-m Date: Thu, 21 Sep 2023 13:58:54 +0300 Subject: [PATCH 05/13] Remove redundant private helper 'GivenEmptyRequestHeaders' --- .../DownstreamRouteFinderTests.cs | 41 ++++--------------- 1 file changed, 7 insertions(+), 34 deletions(-) diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs index f265d9e17..c66abd198 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs +++ b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs @@ -25,13 +25,14 @@ public class DownstreamRouteFinderTests private string _upstreamHost; private string _upstreamQuery; private UpstreamHeaderRoutingOptions _upstreamHeaderRoutingOptions; - private IHeaderDictionary _requestHeaders; + private readonly HeaderDictionary _requestHeaders; public DownstreamRouteFinderTests() { - _mockMatcher = new Mock(); - _finder = new Mock(); - _downstreamRouteFinder = new Ocelot.DownstreamRouteFinder.Finder.DownstreamRouteFinder(_mockMatcher.Object, _finder.Object); + _mockMatcher = new(); + _finder = new(); + _downstreamRouteFinder = new Ocelot.DownstreamRouteFinder.Finder.DownstreamRouteFinder(_mockMatcher.Object, _finder.Object); + _requestHeaders = new(); } [Fact] @@ -65,7 +66,6 @@ public void should_return_highest_priority_when_first() }, string.Empty, serviceProviderConfig)) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder(new List(), new RouteBuilder() @@ -112,7 +112,6 @@ public void should_return_highest_priority_when_lowest() }, string.Empty, serviceProviderConfig)) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder(new List(), new RouteBuilder() @@ -152,7 +151,6 @@ public void should_return_route() )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder( @@ -195,7 +193,6 @@ public void should_not_append_slash_to_upstream_url_path() )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder( @@ -239,7 +236,6 @@ public void should_return_route_if_upstream_path_and_upstream_template_are_the_s )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder(new List(), @@ -290,7 +286,6 @@ public void should_return_correct_route_for_http_verb() )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder(new List(), @@ -328,7 +323,6 @@ public void should_not_return_route() )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(false)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenAnErrorResponseIsReturned()) .And(x => x.ThenTheUrlMatcherIsCalledCorrectly()) @@ -360,7 +354,6 @@ public void should_return_correct_route_for_http_verb_setting_multiple_upstream_ )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder(new List(), @@ -402,7 +395,6 @@ public void should_return_correct_route_for_http_verb_setting_all_upstream_http_ )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder(new List(), @@ -444,7 +436,6 @@ public void should_not_return_route_for_http_verb_not_setting_in_upstream_http_m )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenAnErrorResponseIsReturned()) .And(x => x.ThenTheUrlMatcherIsNotCalled()) @@ -477,7 +468,6 @@ public void should_return_route_when_host_matches() )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder( @@ -521,7 +511,6 @@ public void should_return_route_when_upstreamhost_is_null() )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then( x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder( @@ -574,7 +563,6 @@ public void should_not_return_route_when_host_doesnt_match() )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenAnErrorResponseIsReturned()) .And(x => x.ThenTheUrlMatcherIsNotCalled()) @@ -605,7 +593,6 @@ public void should_not_return_route_when_host_doesnt_match_with_empty_upstream_h )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenAnErrorResponseIsReturned()) .And(x => x.ThenTheUrlMatcherIsNotCalled()) @@ -636,7 +623,6 @@ public void should_return_route_when_host_does_match_with_empty_upstream_http_me )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .And(x => x.ThenTheUrlMatcherIsCalledCorrectly(1, 0)) .BDDfy(); @@ -677,7 +663,6 @@ public void should_return_route_when_host_matches_but_null_host_on_same_path_fir )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder( new List(), @@ -722,7 +707,6 @@ public void should_not_return_route_with_upstream_header_routing_options_enabled )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) - .And(x => x.GivenEmptyRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenAnErrorResponseIsReturned()) .BDDfy(); @@ -830,25 +814,14 @@ private void GivenUpstreamHeaderRoutingOptions() _upstreamHeaderRoutingOptions = new UpstreamHeaderRoutingOptions(headers, UpstreamHeaderRoutingTriggerMode.All); } - private void GivenEmptyRequestHeaders() - { - _requestHeaders = new HeaderDictionary(); - } - private void GivenNonEmptyNonMatchingRequestHeaders() { - _requestHeaders = new HeaderDictionary() - { - { "header", new StringValues(new []{ "mismatch" }) }, - }; + _requestHeaders.Add("header", new StringValues(new[] { "mismatch" })); } private void GivenNonEmptyMatchingRequestHeaders() { - _requestHeaders = new HeaderDictionary() - { - { "header", new StringValues(new []{ "value" }) }, - }; + _requestHeaders.Add("header", new StringValues(new[] { "value" })); } private void ThenAnErrorResponseIsReturned() From 780a7f90b814d697a46b3c993cb935f5409c338d Mon Sep 17 00:00:00 2001 From: raman-m Date: Thu, 21 Sep 2023 14:18:19 +0300 Subject: [PATCH 06/13] IDE1006 Naming rule violation: These words must begin with upper case characters: should_* --- .../DownstreamRouteFinderTests.cs | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs index c66abd198..b8da13eb6 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs +++ b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs @@ -36,7 +36,7 @@ public DownstreamRouteFinderTests() } [Fact] - public void should_return_highest_priority_when_first() + public void Should_return_highest_priority_when_first() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -82,7 +82,7 @@ public void should_return_highest_priority_when_first() } [Fact] - public void should_return_highest_priority_when_lowest() + public void Should_return_highest_priority_when_lowest() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -128,7 +128,7 @@ public void should_return_highest_priority_when_lowest() } [Fact] - public void should_return_route() + public void Should_return_route() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -170,7 +170,7 @@ public void should_return_route() } [Fact] - public void should_not_append_slash_to_upstream_url_path() + public void Should_not_append_slash_to_upstream_url_path() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -212,7 +212,7 @@ public void should_not_append_slash_to_upstream_url_path() } [Fact] - public void should_return_route_if_upstream_path_and_upstream_template_are_the_same() + public void Should_return_route_if_upstream_path_and_upstream_template_are_the_same() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -253,7 +253,7 @@ public void should_return_route_if_upstream_path_and_upstream_template_are_the_s } [Fact] - public void should_return_correct_route_for_http_verb() + public void Should_return_correct_route_for_http_verb() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -303,7 +303,7 @@ public void should_return_correct_route_for_http_verb() } [Fact] - public void should_not_return_route() + public void Should_not_return_route() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -330,7 +330,7 @@ public void should_not_return_route() } [Fact] - public void should_return_correct_route_for_http_verb_setting_multiple_upstream_http_method() + public void Should_return_correct_route_for_http_verb_setting_multiple_upstream_http_method() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -371,7 +371,7 @@ public void should_return_correct_route_for_http_verb_setting_multiple_upstream_ } [Fact] - public void should_return_correct_route_for_http_verb_setting_all_upstream_http_method() + public void Should_return_correct_route_for_http_verb_setting_all_upstream_http_method() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -412,7 +412,7 @@ public void should_return_correct_route_for_http_verb_setting_all_upstream_http_ } [Fact] - public void should_not_return_route_for_http_verb_not_setting_in_upstream_http_method() + public void Should_not_return_route_for_http_verb_not_setting_in_upstream_http_method() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -443,7 +443,7 @@ public void should_not_return_route_for_http_verb_not_setting_in_upstream_http_m } [Fact] - public void should_return_route_when_host_matches() + public void Should_return_route_when_host_matches() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -487,7 +487,7 @@ public void should_return_route_when_host_matches() } [Fact] - public void should_return_route_when_upstreamhost_is_null() + public void Should_return_route_when_upstreamhost_is_null() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -530,7 +530,7 @@ public void should_return_route_when_upstreamhost_is_null() } [Fact] - public void should_not_return_route_when_host_doesnt_match() + public void Should_not_return_route_when_host_doesnt_match() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -570,7 +570,7 @@ public void should_not_return_route_when_host_doesnt_match() } [Fact] - public void should_not_return_route_when_host_doesnt_match_with_empty_upstream_http_method() + public void Should_not_return_route_when_host_doesnt_match_with_empty_upstream_http_method() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -600,7 +600,7 @@ public void should_not_return_route_when_host_doesnt_match_with_empty_upstream_h } [Fact] - public void should_return_route_when_host_does_match_with_empty_upstream_http_method() + public void Should_return_route_when_host_does_match_with_empty_upstream_http_method() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -629,7 +629,7 @@ public void should_return_route_when_host_does_match_with_empty_upstream_http_me } [Fact] - public void should_return_route_when_host_matches_but_null_host_on_same_path_first() + public void Should_return_route_when_host_matches_but_null_host_on_same_path_first() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -682,7 +682,7 @@ public void should_return_route_when_host_matches_but_null_host_on_same_path_fir } [Fact] - public void should_not_return_route_with_upstream_header_routing_options_enabled_and_no_request_headers() + public void Should_not_return_route_with_upstream_header_routing_options_enabled_and_no_request_headers() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -713,7 +713,7 @@ public void should_not_return_route_with_upstream_header_routing_options_enabled } [Fact] - public void should_not_return_route_with_upstream_header_routing_options_enabled_and_non_matching_request_headers() + public void Should_not_return_route_with_upstream_header_routing_options_enabled_and_non_matching_request_headers() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -745,7 +745,7 @@ public void should_not_return_route_with_upstream_header_routing_options_enabled } [Fact] - public void should_return_route_with_upstream_header_routing_options_enabled_and_matching_request_headers() + public void Should_return_route_with_upstream_header_routing_options_enabled_and_matching_request_headers() { var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); From eb00e0114832789dcd763c0f4ff8cae2e180912d Mon Sep 17 00:00:00 2001 From: raman-m Date: Thu, 21 Sep 2023 15:57:49 +0300 Subject: [PATCH 07/13] Add GivenUpstreamHeaderRoutingOptionsEnabled setup helper --- .../DownstreamRouteFinderTests.cs | 73 ++++--------------- 1 file changed, 15 insertions(+), 58 deletions(-) diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs index b8da13eb6..ebc2908a0 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs +++ b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs @@ -31,7 +31,7 @@ public DownstreamRouteFinderTests() { _mockMatcher = new(); _finder = new(); - _downstreamRouteFinder = new Ocelot.DownstreamRouteFinder.Finder.DownstreamRouteFinder(_mockMatcher.Object, _finder.Object); + _downstreamRouteFinder = new Ocelot.DownstreamRouteFinder.Finder.DownstreamRouteFinder(_mockMatcher.Object, _finder.Object); _requestHeaders = new(); } @@ -681,15 +681,9 @@ public void Should_return_route_when_host_matches_but_null_host_on_same_path_fir .BDDfy(); } - [Fact] - public void Should_not_return_route_with_upstream_header_routing_options_enabled_and_no_request_headers() - { - var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); - - this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher/")) - .And(x => x.GivenTheTemplateVariableAndNameFinderReturns( - new OkResponse>( - new List()))) + private IFluentStepBuilder GivenUpstreamHeaderRoutingOptionsEnabled() + => this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher/")) + .And(x => x.GivenTheTemplateVariableAndNameFinderReturns(new OkResponse>(new()))) .And(x => x.GivenUpstreamHeaderRoutingOptions()) .And(x => x.GivenTheConfigurationIs(new List { @@ -703,10 +697,17 @@ public void Should_not_return_route_with_upstream_header_routing_options_enabled .WithUpstreamPathTemplate(new UpstreamPathTemplate("someUpstreamPath", 1, false, "someUpstreamPath")) .WithUpstreamHeaderRoutingOptions(_upstreamHeaderRoutingOptions) .Build(), - }, string.Empty, serviceProviderConfig + }, + string.Empty, + new ServiceProviderConfigurationBuilder().Build() )) .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) - .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .And(x => x.GivenTheUpstreamHttpMethodIs("Get")); + + [Fact] + public void Should_not_return_route_with_upstream_header_routing_options_enabled_and_no_request_headers() + { + GivenUpstreamHeaderRoutingOptionsEnabled() .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenAnErrorResponseIsReturned()) .BDDfy(); @@ -715,29 +716,7 @@ public void Should_not_return_route_with_upstream_header_routing_options_enabled [Fact] public void Should_not_return_route_with_upstream_header_routing_options_enabled_and_non_matching_request_headers() { - var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); - - this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher/")) - .And(x => x.GivenTheTemplateVariableAndNameFinderReturns( - new OkResponse>( - new List()))) - .And(x => x.GivenUpstreamHeaderRoutingOptions()) - .And(x => x.GivenTheConfigurationIs(new List - { - new RouteBuilder() - .WithDownstreamRoute(new DownstreamRouteBuilder() - .WithDownstreamPathTemplate("someDownstreamPath") - .WithUpstreamHttpMethod(new List { "Get" }) - .WithUpstreamPathTemplate(new UpstreamPathTemplate("someUpstreamPath", 1, false, "someUpstreamPath")) - .Build()) - .WithUpstreamHttpMethod(new List { "Get" }) - .WithUpstreamPathTemplate(new UpstreamPathTemplate("someUpstreamPath", 1, false, "someUpstreamPath")) - .WithUpstreamHeaderRoutingOptions(_upstreamHeaderRoutingOptions) - .Build(), - }, string.Empty, serviceProviderConfig - )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) - .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + GivenUpstreamHeaderRoutingOptionsEnabled() .And(x => x.GivenNonEmptyNonMatchingRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenAnErrorResponseIsReturned()) @@ -747,29 +726,7 @@ public void Should_not_return_route_with_upstream_header_routing_options_enabled [Fact] public void Should_return_route_with_upstream_header_routing_options_enabled_and_matching_request_headers() { - var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); - - this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher/")) - .And(x => x.GivenTheTemplateVariableAndNameFinderReturns( - new OkResponse>( - new List()))) - .And(x => x.GivenUpstreamHeaderRoutingOptions()) - .And(x => x.GivenTheConfigurationIs(new List - { - new RouteBuilder() - .WithDownstreamRoute(new DownstreamRouteBuilder() - .WithDownstreamPathTemplate("someDownstreamPath") - .WithUpstreamHttpMethod(new List { "Get" }) - .WithUpstreamPathTemplate(new UpstreamPathTemplate("someUpstreamPath", 1, false, "someUpstreamPath")) - .Build()) - .WithUpstreamHttpMethod(new List { "Get" }) - .WithUpstreamPathTemplate(new UpstreamPathTemplate("someUpstreamPath", 1, false, "someUpstreamPath")) - .WithUpstreamHeaderRoutingOptions(_upstreamHeaderRoutingOptions) - .Build(), - }, string.Empty, serviceProviderConfig - )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) - .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + GivenUpstreamHeaderRoutingOptionsEnabled() .And(x => x.GivenNonEmptyMatchingRequestHeaders()) .When(x => x.WhenICallTheFinder()) .Then(x => x.ThenTheFollowingIsReturned(new DownstreamRouteHolder( From af708f265e0bf0f52b92e3b320f197b060489922 Mon Sep 17 00:00:00 2001 From: raman-m Date: Thu, 21 Sep 2023 16:26:19 +0300 Subject: [PATCH 08/13] Use 'HttpMethod.Equals' method which has case insensitive comparer --- .../DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs index cecca0a17..444ca4c88 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs +++ b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs @@ -59,8 +59,8 @@ private static bool RouteIsApplicableToThisRequest(Route route, string httpMetho } private static bool RouteHasHttpMethod(Route route, string httpMethod) => - route.UpstreamHttpMethod.Select(x => x.Method).Contains(httpMethod, StringComparer.OrdinalIgnoreCase); - + route.UpstreamHttpMethod.Contains(new HttpMethod(httpMethod)); + private static bool RouteHasRequiredUpstreamHeaders(Route route, IHeaderDictionary requestHeaders) => route.UpstreamHeaderRoutingOptions.Mode == UpstreamHeaderRoutingTriggerMode.Any ? route.UpstreamHeaderRoutingOptions.Headers.HasAnyOf(requestHeaders) From 4622bb80f776d57b39234b4869c1005e05da6912 Mon Sep 17 00:00:00 2001 From: raman-m Date: Thu, 21 Sep 2023 19:23:09 +0300 Subject: [PATCH 09/13] Optimize nullable bool expression --- .../DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs index 444ca4c88..5dc179851 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs +++ b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs @@ -52,11 +52,9 @@ public DownstreamRouteFinder(IUrlPathToUrlTemplateMatcher urlMatcher, IPlacehold } private static bool RouteIsApplicableToThisRequest(Route route, string httpMethod, string upstreamHost, IHeaderDictionary requestHeaders) - { - return (route.UpstreamHttpMethod.Count == 0 || RouteHasHttpMethod(route, httpMethod)) && - (string.IsNullOrEmpty(route.UpstreamHost) || route.UpstreamHost == upstreamHost) && - (!(route.UpstreamHeaderRoutingOptions?.Enabled() ?? false) || RouteHasRequiredUpstreamHeaders(route, requestHeaders)); - } + => (route.UpstreamHttpMethod.Count == 0 || RouteHasHttpMethod(route, httpMethod)) && + (string.IsNullOrEmpty(route.UpstreamHost) || route.UpstreamHost == upstreamHost) && + (route.UpstreamHeaderRoutingOptions?.Enabled() != true || RouteHasRequiredUpstreamHeaders(route, requestHeaders)); private static bool RouteHasHttpMethod(Route route, string httpMethod) => route.UpstreamHttpMethod.Contains(new HttpMethod(httpMethod)); From fec3b9fecd53d4be43638035430c31fb852b8a71 Mon Sep 17 00:00:00 2001 From: raman-m Date: Thu, 21 Sep 2023 19:40:16 +0300 Subject: [PATCH 10/13] Shorten method name --- .../Validator/FileConfigurationFluentValidator.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs b/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs index 2d9455aa5..09cab6cdf 100644 --- a/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs +++ b/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs @@ -116,9 +116,7 @@ private static bool IsPlaceholderNotDuplicatedIn(string upstreamPathTemplate) var matchingRoutes = routes .Where(r => r.UpstreamPathTemplate == route.UpstreamPathTemplate && r.UpstreamHost == route.UpstreamHost - && AreDuplicateUpstreamRoutingHeaders( - route.UpstreamHeaderRoutingOptions.Headers, - r.UpstreamHeaderRoutingOptions.Headers)) + && AreDuplicates(route.UpstreamHeaderRoutingOptions.Headers, r.UpstreamHeaderRoutingOptions.Headers)) .ToList(); if (matchingRoutes.Count == 1) @@ -160,15 +158,14 @@ private static bool IsNotDuplicateIn(FileAggregateRoute route, IEnumerable> first, - IDictionary> second) + private static bool AreDuplicates(IDictionary> first, IDictionary> second) { if (!first.Any() && !second.Any()) { return true; } - + + // if either of the two header collections is empty while the other is not, it's obvious that they can never be duplicate if (first.Any() ^ second.Any()) { return false; From 453cc698084c2f67f11e48849995c23480df973d Mon Sep 17 00:00:00 2001 From: raman-m Date: Thu, 21 Sep 2023 19:51:07 +0300 Subject: [PATCH 11/13] Return back whitespace as line ending --- docs/features/routing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/features/routing.rst b/docs/features/routing.rst index d39c5b6eb..6b047c744 100644 --- a/docs/features/routing.rst +++ b/docs/features/routing.rst @@ -178,7 +178,7 @@ e.g. you could have "Priority": 0 } -and +and .. code-block:: json From 1bf051769b3b402c711e8a5a6487bcf0661b8720 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Thu, 21 Sep 2023 20:22:52 +0300 Subject: [PATCH 12/13] Update routing.rst; Extract main property block & Review the phrases --- docs/features/routing.rst | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/docs/features/routing.rst b/docs/features/routing.rst index 6b047c744..d07b43b49 100644 --- a/docs/features/routing.rst +++ b/docs/features/routing.rst @@ -293,7 +293,7 @@ Here are two user scenarios. Finally, the ``userId`` parameter is removed. -Upstream header-based routing +Upstream Header-Based Routing ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This feature was requested in `issue 360 `_ and `issue 624 `_. @@ -307,16 +307,8 @@ A sample configuration might look like the following: { "Routes": [ { - "DownstreamPathTemplate": "/api/posts", - "DownstreamScheme": "http", - "DownstreamHostAndPorts": [ - { - "Host": "server1", - "Port": 80, - } - ], - "UpstreamPathTemplate": "/posts", - "UpstreamHttpMethod": [ "Put", "Delete" ] + // Downstream* props + // Upstream* props "UpstreamHeaderRoutingOptions": { "Headers": { "X-API-Version": [ "1" ], @@ -326,19 +318,11 @@ A sample configuration might look like the following: } }, { - "DownstreamPathTemplate": "/api/posts", - "DownstreamScheme": "http", - "DownstreamHostAndPorts": [ - { - "Host": "server2", - "Port": 80, - } - ], - "UpstreamPathTemplate": "/posts", - "UpstreamHttpMethod": [ "Put", "Delete" ] + // Downstream* props + // Upstream* props "UpstreamHeaderRoutingOptions": { "Headers": { - "X-API-Version": [ "2" ] + "X-API-Version": [ "1", "2" ] }, "TriggerOn": "any" } @@ -346,11 +330,13 @@ A sample configuration might look like the following: ] } -The ``UpstreamHeaderRoutingOptions`` block defines two attributes -- the ``Headers`` block and the ``TriggerOn`` attribute. The ``Headers`` attribute defines required header names as keys and lists of acceptable header values as values. During route matching, both header names and values are matched in *case insensitive* manner. Please note that if a header has more than one acceptable value configured, presence of any of those values in a request is sufficient for a header to be a match. +The ``UpstreamHeaderRoutingOptions`` block defines two attributes: the ``Headers`` block and the ``TriggerOn`` attribute. + +The ``Headers`` attribute defines required header names as keys and lists of acceptable header values as values. During route matching, both header names and values are matched in *case insensitive* manner. Please note that if a header has more than one acceptable value configured, presence of any of those values in a request is sufficient for a header to be a match. The second attribute, ``TriggerOn``, defines how the route finder will determine whether a particular header configuration in a request matches a Route's header configuration. The attribute accepts two values: -* ``"Any"`` causes the route finder to match a Route if any value of any configured header is present in a request +* ``"Any"`` causes the route finder to match a Route if any value of *any* configured header is present in a request * ``"All"`` causes the route finder to match a Route only if any value of *all* configured headers is present in a request .. _routing-security-options: From 5f56f96b964705d972b7b14494c3393ab79be585 Mon Sep 17 00:00:00 2001 From: Zbynek Novotny Date: Fri, 22 Sep 2023 22:53:38 +0200 Subject: [PATCH 13/13] Couple of changes for the second round of PR feedback. --- .../Validator/FileConfigurationFluentValidator.cs | 10 +++------- .../Finder/DownstreamRouteFinder.cs | 10 +++++----- .../UpstreamHeaderRoutingOptionsCreatorTests.cs | 7 +------ 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs b/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs index 09cab6cdf..bc60ac795 100644 --- a/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs +++ b/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs @@ -164,19 +164,15 @@ private static bool AreDuplicates(IDictionary> first { return true; } - + // if either of the two header collections is empty while the other is not, it's obvious that they can never be duplicate if (first.Any() ^ second.Any()) { return false; } - var firstKeySet = first.Keys - .Select(k => k.ToUpperInvariant()) - .ToArray(); - var secondKeySet = second.Keys - .Select(k => k.ToUpperInvariant()) - .ToArray(); + var firstKeySet = first.Keys.Select(k => k.ToUpperInvariant()); + var secondKeySet = second.Keys.Select(k => k.ToUpperInvariant()); if (!firstKeySet.Intersect(secondKeySet).Any()) { return false; diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs index 5dc179851..99b8ce45b 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs +++ b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs @@ -54,15 +54,15 @@ public DownstreamRouteFinder(IUrlPathToUrlTemplateMatcher urlMatcher, IPlacehold private static bool RouteIsApplicableToThisRequest(Route route, string httpMethod, string upstreamHost, IHeaderDictionary requestHeaders) => (route.UpstreamHttpMethod.Count == 0 || RouteHasHttpMethod(route, httpMethod)) && (string.IsNullOrEmpty(route.UpstreamHost) || route.UpstreamHost == upstreamHost) && - (route.UpstreamHeaderRoutingOptions?.Enabled() != true || RouteHasRequiredUpstreamHeaders(route, requestHeaders)); + (route.UpstreamHeaderRoutingOptions?.Enabled() != true || RequiredUpstreamHeadersArePresent(route.UpstreamHeaderRoutingOptions, requestHeaders)); private static bool RouteHasHttpMethod(Route route, string httpMethod) => route.UpstreamHttpMethod.Contains(new HttpMethod(httpMethod)); - private static bool RouteHasRequiredUpstreamHeaders(Route route, IHeaderDictionary requestHeaders) => - route.UpstreamHeaderRoutingOptions.Mode == UpstreamHeaderRoutingTriggerMode.Any - ? route.UpstreamHeaderRoutingOptions.Headers.HasAnyOf(requestHeaders) - : route.UpstreamHeaderRoutingOptions.Headers.HasAllOf(requestHeaders); + private static bool RequiredUpstreamHeadersArePresent(UpstreamHeaderRoutingOptions options, IHeaderDictionary requestHeaders) => + options.Mode == UpstreamHeaderRoutingTriggerMode.Any + ? options.Headers.HasAnyOf(requestHeaders) + : options.Headers.HasAllOf(requestHeaders); private DownstreamRouteHolder GetPlaceholderNamesAndValues(string path, string query, Route route) { diff --git a/test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs index 178a83ac2..d6a8e3b73 100644 --- a/test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/UpstreamHeaderRoutingOptionsCreatorTests.cs @@ -12,14 +12,9 @@ namespace Ocelot.UnitTests.Configuration; public class UpstreamHeaderRoutingOptionsCreatorTests { private FileUpstreamHeaderRoutingOptions _fileUpstreamHeaderRoutingOptions; - private IUpstreamHeaderRoutingOptionsCreator _creator; + private readonly IUpstreamHeaderRoutingOptionsCreator _creator = new UpstreamHeaderRoutingOptionsCreator(); private UpstreamHeaderRoutingOptions _upstreamHeaderRoutingOptions; - public UpstreamHeaderRoutingOptionsCreatorTests() - { - _creator = new UpstreamHeaderRoutingOptionsCreator(); - } - [Fact] public void should_create_upstream_routing_header_options() {