From 2a19fc76fc247c80159742257de5b5eed3bb0678 Mon Sep 17 00:00:00 2001 From: Lars Johansen Date: Tue, 31 Oct 2023 13:53:21 +0100 Subject: [PATCH 01/14] refactor ConfigValidator --- .../ClusterValidators/DestinationValidator.cs | 26 ++ .../ClusterValidators/HealthCheckValidator.cs | 100 +++++ .../ClusterValidators/IClusterValidator.cs | 15 + .../LoadBalancingValidator.cs | 34 ++ .../ProxyHttpClientValidator.cs | 51 +++ .../ProxyHttpRequestValidator.cs | 56 +++ .../SessionAffinityValidator.cs | 65 +++ .../Configuration/ConfigValidator.cs | 418 +----------------- .../RouteValidators/HeadersValidator.cs | 45 ++ .../RouteValidators/HostValidator.cs | 32 ++ .../RouteValidators/IRouteValidator.cs | 15 + .../RouteValidators/MethodsValidator.cs | 40 ++ .../RouteValidators/PathValidator.cs | 30 ++ .../QueryParametersValidator.cs | 44 ++ .../IReverseProxyBuilderExtensions.cs | 13 + 15 files changed, 586 insertions(+), 398 deletions(-) create mode 100644 src/ReverseProxy/Configuration/ClusterValidators/DestinationValidator.cs create mode 100644 src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs create mode 100644 src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs create mode 100644 src/ReverseProxy/Configuration/ClusterValidators/LoadBalancingValidator.cs create mode 100644 src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpClientValidator.cs create mode 100644 src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs create mode 100644 src/ReverseProxy/Configuration/ClusterValidators/SessionAffinityValidator.cs create mode 100644 src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs create mode 100644 src/ReverseProxy/Configuration/RouteValidators/HostValidator.cs create mode 100644 src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs create mode 100644 src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs create mode 100644 src/ReverseProxy/Configuration/RouteValidators/PathValidator.cs create mode 100644 src/ReverseProxy/Configuration/RouteValidators/QueryParametersValidator.cs diff --git a/src/ReverseProxy/Configuration/ClusterValidators/DestinationValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/DestinationValidator.cs new file mode 100644 index 000000000..753f49581 --- /dev/null +++ b/src/ReverseProxy/Configuration/ClusterValidators/DestinationValidator.cs @@ -0,0 +1,26 @@ +using System; +using System.Collections.Generic; + +namespace Yarp.ReverseProxy.Configuration.ClusterValidators; + +internal sealed class DestinationValidator : IClusterValidator +{ + public IList Validate(ClusterConfig cluster) + { + if (cluster.Destinations is null) + { + return Array.Empty(); + } + + var errors = new List(); + foreach (var (name, destination) in cluster.Destinations) + { + if (string.IsNullOrEmpty(destination.Address)) + { + errors.Add(new ArgumentException($"No address found for destination '{name}' on cluster '{cluster.ClusterId}'.")); + } + } + + return errors; + } +} diff --git a/src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs new file mode 100644 index 000000000..031265e53 --- /dev/null +++ b/src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs @@ -0,0 +1,100 @@ +using System; +using System.Collections.Frozen; +using System.Collections.Generic; +using Yarp.ReverseProxy.Health; +using Yarp.ReverseProxy.Utilities; + +namespace Yarp.ReverseProxy.Configuration.ClusterValidators; + +internal sealed class HealthCheckValidator : IClusterValidator +{ + private readonly FrozenDictionary _availableDestinationsPolicies; + private readonly FrozenDictionary _activeHealthCheckPolicies; + private readonly FrozenDictionary _passiveHealthCheckPolicies; + public HealthCheckValidator(IEnumerable availableDestinationsPolicies, + IEnumerable activeHealthCheckPolicies, + IEnumerable passiveHealthCheckPolicies) + { + _availableDestinationsPolicies = availableDestinationsPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(availableDestinationsPolicies)); + _activeHealthCheckPolicies = activeHealthCheckPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(availableDestinationsPolicies)); + _passiveHealthCheckPolicies = passiveHealthCheckPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(availableDestinationsPolicies)); + } + + public IList Validate(ClusterConfig cluster) + { + var availableDestinationsPolicy = cluster.HealthCheck?.AvailableDestinationsPolicy; + if (string.IsNullOrEmpty(availableDestinationsPolicy)) + { + // The default. + availableDestinationsPolicy = HealthCheckConstants.AvailableDestinations.HealthyOrPanic; + } + + var errors = new List(); + if (!_availableDestinationsPolicies.ContainsKey(availableDestinationsPolicy)) + { + errors.Add(new ArgumentException($"No matching {nameof(IAvailableDestinationsPolicy)} found for the available destinations policy '{availableDestinationsPolicy}' set on the cluster.'{cluster.ClusterId}'.")); + } + + ValidateActiveHealthCheck(errors, cluster); + ValidatePassiveHealthCheck(errors, cluster); + + return errors; + } + + private void ValidateActiveHealthCheck(IList errors, ClusterConfig cluster) + { + if (!(cluster.HealthCheck?.Active?.Enabled ?? false)) + { + // Active health check is disabled + return; + } + + var activeOptions = cluster.HealthCheck.Active; + var policy = activeOptions.Policy; + if (string.IsNullOrEmpty(policy)) + { + // default policy + policy = HealthCheckConstants.ActivePolicy.ConsecutiveFailures; + } + if (!_activeHealthCheckPolicies.ContainsKey(policy)) + { + errors.Add(new ArgumentException($"No matching {nameof(IActiveHealthCheckPolicy)} found for the active health check policy name '{policy}' set on the cluster '{cluster.ClusterId}'.")); + } + + if (activeOptions.Interval is not null && activeOptions.Interval <= TimeSpan.Zero) + { + errors.Add(new ArgumentException($"Destination probing interval set on the cluster '{cluster.ClusterId}' must be positive.")); + } + + if (activeOptions.Timeout is not null && activeOptions.Timeout <= TimeSpan.Zero) + { + errors.Add(new ArgumentException($"Destination probing timeout set on the cluster '{cluster.ClusterId}' must be positive.")); + } + } + + private void ValidatePassiveHealthCheck(IList errors, ClusterConfig cluster) + { + if (!(cluster.HealthCheck?.Passive?.Enabled ?? false)) + { + // Passive health check is disabled + return; + } + + var passiveOptions = cluster.HealthCheck.Passive; + var policy = passiveOptions.Policy; + if (string.IsNullOrEmpty(policy)) + { + // default policy + policy = HealthCheckConstants.PassivePolicy.TransportFailureRate; + } + if (!_passiveHealthCheckPolicies.ContainsKey(policy)) + { + errors.Add(new ArgumentException($"No matching {nameof(IPassiveHealthCheckPolicy)} found for the passive health check policy name '{policy}' set on the cluster '{cluster.ClusterId}'.")); + } + + if (passiveOptions.ReactivationPeriod is not null && passiveOptions.ReactivationPeriod <= TimeSpan.Zero) + { + errors.Add(new ArgumentException($"Unhealthy destination reactivation period set on the cluster '{cluster.ClusterId}' must be positive.")); + } + } +} diff --git a/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs new file mode 100644 index 000000000..d12e98103 --- /dev/null +++ b/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs @@ -0,0 +1,15 @@ +using System; +using System.Collections.Generic; + +namespace Yarp.ReverseProxy.Configuration.ClusterValidators; + +internal interface IClusterValidator +{ + protected IList Validate(ClusterConfig cluster); + + public bool IsValid(ClusterConfig cluster, out IList errors) + { + errors = Validate(cluster); + return errors.Count == 0; + } +} diff --git a/src/ReverseProxy/Configuration/ClusterValidators/LoadBalancingValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/LoadBalancingValidator.cs new file mode 100644 index 000000000..280d1f42c --- /dev/null +++ b/src/ReverseProxy/Configuration/ClusterValidators/LoadBalancingValidator.cs @@ -0,0 +1,34 @@ +using System; +using System.Collections.Frozen; +using System.Collections.Generic; +using Yarp.ReverseProxy.LoadBalancing; +using Yarp.ReverseProxy.Utilities; + +namespace Yarp.ReverseProxy.Configuration.ClusterValidators; + +internal sealed class LoadBalancingValidator : IClusterValidator +{ + private readonly FrozenDictionary _loadBalancingPolicies; + public LoadBalancingValidator(IEnumerable loadBalancingPolicies) + { + _loadBalancingPolicies = loadBalancingPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(loadBalancingPolicies)); + } + + public IList Validate(ClusterConfig cluster) + { + var loadBalancingPolicy = cluster.LoadBalancingPolicy; + if (string.IsNullOrEmpty(loadBalancingPolicy)) + { + // The default. + loadBalancingPolicy = LoadBalancingPolicies.PowerOfTwoChoices; + } + + var errors = new List(); + if (!_loadBalancingPolicies.ContainsKey(loadBalancingPolicy)) + { + errors.Add(new ArgumentException($"No matching {nameof(ILoadBalancingPolicy)} found for the load balancing policy '{loadBalancingPolicy}' set on the cluster '{cluster.ClusterId}'.")); + } + + return errors; + } +} diff --git a/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpClientValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpClientValidator.cs new file mode 100644 index 000000000..987a38c99 --- /dev/null +++ b/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpClientValidator.cs @@ -0,0 +1,51 @@ +using System; +using System.Collections.Generic; +using System.Text; + +namespace Yarp.ReverseProxy.Configuration.ClusterValidators; + +internal sealed class ProxyHttpClientValidator : IClusterValidator +{ + public IList Validate(ClusterConfig cluster) + { + if (cluster.HttpClient is null) + { + // Proxy http client options are not set. + return Array.Empty(); + } + + var errors = new List(); + if (cluster.HttpClient.MaxConnectionsPerServer is not null && cluster.HttpClient.MaxConnectionsPerServer <= 0) + { + errors.Add(new ArgumentException($"Max connections per server limit set on the cluster '{cluster.ClusterId}' must be positive.")); + } + + var requestHeaderEncoding = cluster.HttpClient.RequestHeaderEncoding; + if (requestHeaderEncoding is not null) + { + try + { + Encoding.GetEncoding(requestHeaderEncoding); + } + catch (ArgumentException aex) + { + errors.Add(new ArgumentException($"Invalid request header encoding '{requestHeaderEncoding}'.", aex)); + } + } + + var responseHeaderEncoding = cluster.HttpClient.ResponseHeaderEncoding; + if (responseHeaderEncoding is not null) + { + try + { + Encoding.GetEncoding(responseHeaderEncoding); + } + catch (ArgumentException aex) + { + errors.Add(new ArgumentException($"Invalid response header encoding '{responseHeaderEncoding}'.", aex)); + } + } + + return errors; + } +} diff --git a/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs new file mode 100644 index 000000000..d8ef4c9d2 --- /dev/null +++ b/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs @@ -0,0 +1,56 @@ +using System; +using System.Collections.Generic; +using System.Net; +using Microsoft.Extensions.Logging; + +namespace Yarp.ReverseProxy.Configuration.ClusterValidators; + +internal sealed class ProxyHttpRequestValidator : IClusterValidator +{ + private readonly ILogger _logger; + + public ProxyHttpRequestValidator(ILogger logger) + { + _logger = logger; + } + + public IList Validate(ClusterConfig cluster) + { + if (cluster.HttpRequest is null) + { + // Proxy http request options are not set. + return Array.Empty(); + } + + var errors = new List(); + + if (cluster.HttpRequest.Version is not null && + cluster.HttpRequest.Version != HttpVersion.Version10 && + cluster.HttpRequest.Version != HttpVersion.Version11 && + cluster.HttpRequest.Version != HttpVersion.Version20 && + cluster.HttpRequest.Version != HttpVersion.Version30) + { + errors.Add(new ArgumentException($"Outgoing request version '{cluster.HttpRequest.Version}' is not any of supported HTTP versions (1.0, 1.1, 2 and 3).")); + } + + if (cluster.HttpRequest.Version == HttpVersion.Version10) + { + Log.Http10Version(_logger); + } + + return errors; + } + + private static class Log + { + private static readonly Action _http10RequestVersionDetected = LoggerMessage.Define( + LogLevel.Warning, + EventIds.Http10RequestVersionDetected, + "The HttpRequest version is set to 1.0 which can result in poor performance and port exhaustion. Use 1.1, 2, or 3 instead."); + + public static void Http10Version(ILogger logger) + { + _http10RequestVersionDetected(logger, null); + } + } +} diff --git a/src/ReverseProxy/Configuration/ClusterValidators/SessionAffinityValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/SessionAffinityValidator.cs new file mode 100644 index 000000000..fc5c39e85 --- /dev/null +++ b/src/ReverseProxy/Configuration/ClusterValidators/SessionAffinityValidator.cs @@ -0,0 +1,65 @@ +using System; +using System.Collections.Frozen; +using System.Collections.Generic; +using Yarp.ReverseProxy.SessionAffinity; +using Yarp.ReverseProxy.Utilities; + +namespace Yarp.ReverseProxy.Configuration.ClusterValidators; + +internal sealed class SessionAffinityValidator : IClusterValidator +{ + private readonly FrozenDictionary _affinityFailurePolicies; + + public SessionAffinityValidator(IEnumerable affinityFailurePolicies) + { + _affinityFailurePolicies = affinityFailurePolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(affinityFailurePolicies)); + } + + public IList Validate(ClusterConfig cluster) + { + if (!(cluster.SessionAffinity?.Enabled ?? false)) + { + // Session affinity is disabled + return Array.Empty(); + } + + // Note some affinity validation takes place in AffinitizeTransformProvider.ValidateCluster. + + var affinityFailurePolicy = cluster.SessionAffinity.FailurePolicy; + if (string.IsNullOrEmpty(affinityFailurePolicy)) + { + // The default. + affinityFailurePolicy = SessionAffinityConstants.FailurePolicies.Redistribute; + } + + var errors = new List(); + if (!_affinityFailurePolicies.ContainsKey(affinityFailurePolicy)) + { + errors.Add(new ArgumentException($"No matching {nameof(IAffinityFailurePolicy)} found for the affinity failure policy name '{affinityFailurePolicy}' set on the cluster '{cluster.ClusterId}'.")); + } + + if (string.IsNullOrEmpty(cluster.SessionAffinity.AffinityKeyName)) + { + errors.Add(new ArgumentException($"Affinity key name set on the cluster '{cluster.ClusterId}' must not be null.")); + } + + var cookieConfig = cluster.SessionAffinity.Cookie; + + if (cookieConfig is null) + { + return errors; + } + + if (cookieConfig.Expiration is not null && cookieConfig.Expiration <= TimeSpan.Zero) + { + errors.Add(new ArgumentException($"Session affinity cookie expiration must be positive or null.")); + } + + if (cookieConfig.MaxAge is not null && cookieConfig.MaxAge <= TimeSpan.Zero) + { + errors.Add(new ArgumentException($"Session affinity cookie max-age must be positive or null.")); + } + + return errors; + } +} diff --git a/src/ReverseProxy/Configuration/ConfigValidator.cs b/src/ReverseProxy/Configuration/ConfigValidator.cs index 51d9c4da0..bfcd6cbda 100644 --- a/src/ReverseProxy/Configuration/ConfigValidator.cs +++ b/src/ReverseProxy/Configuration/ConfigValidator.cs @@ -2,65 +2,40 @@ // Licensed under the MIT License. using System; -using System.Collections.Frozen; using System.Collections.Generic; using System.Linq; -using System.Net; -using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Cors.Infrastructure; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Routing.Patterns; -using Microsoft.Extensions.Logging; -using Yarp.ReverseProxy.Health; -using Yarp.ReverseProxy.LoadBalancing; -using Yarp.ReverseProxy.SessionAffinity; +using Yarp.ReverseProxy.Configuration.ClusterValidators; +using Yarp.ReverseProxy.Configuration.RouteValidators; using Yarp.ReverseProxy.Transforms.Builder; -using Yarp.ReverseProxy.Utilities; namespace Yarp.ReverseProxy.Configuration; internal sealed class ConfigValidator : IConfigValidator { - private static readonly HashSet _validMethods = new HashSet(StringComparer.OrdinalIgnoreCase) - { - "HEAD", "OPTIONS", "GET", "PUT", "POST", "PATCH", "DELETE", "TRACE", - }; - private readonly ITransformBuilder _transformBuilder; private readonly IAuthorizationPolicyProvider _authorizationPolicyProvider; private readonly IYarpRateLimiterPolicyProvider _rateLimiterPolicyProvider; private readonly ICorsPolicyProvider _corsPolicyProvider; - private readonly FrozenDictionary _loadBalancingPolicies; - private readonly FrozenDictionary _affinityFailurePolicies; - private readonly FrozenDictionary _availableDestinationsPolicies; - private readonly FrozenDictionary _activeHealthCheckPolicies; - private readonly FrozenDictionary _passiveHealthCheckPolicies; - private readonly ILogger _logger; - + private readonly IEnumerable _routeValidators; + private readonly IEnumerable _clusterValidators; public ConfigValidator(ITransformBuilder transformBuilder, IAuthorizationPolicyProvider authorizationPolicyProvider, IYarpRateLimiterPolicyProvider rateLimiterPolicyProvider, ICorsPolicyProvider corsPolicyProvider, - IEnumerable loadBalancingPolicies, - IEnumerable affinityFailurePolicies, - IEnumerable availableDestinationsPolicies, - IEnumerable activeHealthCheckPolicies, - IEnumerable passiveHealthCheckPolicies, - ILogger logger) + IEnumerable routeValidators, + IEnumerable clusterValidators) { _transformBuilder = transformBuilder ?? throw new ArgumentNullException(nameof(transformBuilder)); _authorizationPolicyProvider = authorizationPolicyProvider ?? throw new ArgumentNullException(nameof(authorizationPolicyProvider)); _rateLimiterPolicyProvider = rateLimiterPolicyProvider ?? throw new ArgumentNullException(nameof(rateLimiterPolicyProvider)); _corsPolicyProvider = corsPolicyProvider ?? throw new ArgumentNullException(nameof(corsPolicyProvider)); - _loadBalancingPolicies = loadBalancingPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(loadBalancingPolicies)); - _affinityFailurePolicies = affinityFailurePolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(affinityFailurePolicies)); - _availableDestinationsPolicies = availableDestinationsPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(availableDestinationsPolicies)); - _activeHealthCheckPolicies = activeHealthCheckPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(activeHealthCheckPolicies)); - _passiveHealthCheckPolicies = passiveHealthCheckPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(passiveHealthCheckPolicies)); - _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _routeValidators = routeValidators; + _clusterValidators = clusterValidators; } // Note this performs all validation steps without short circuiting in order to report all possible errors. @@ -87,16 +62,18 @@ public async ValueTask> ValidateRouteAsync(RouteConfig route) return errors; } - if ((route.Match.Hosts is null || !route.Match.Hosts.Any(host => !string.IsNullOrEmpty(host))) && string.IsNullOrEmpty(route.Match.Path)) + if ((route.Match.Hosts is null || route.Match.Hosts.All(string.IsNullOrEmpty)) && string.IsNullOrEmpty(route.Match.Path)) { errors.Add(new ArgumentException($"Route '{route.RouteId}' requires Hosts or Path specified. Set the Path to '/{{**catchall}}' to match all requests.")); } - ValidateHost(errors, route.Match.Hosts, route.RouteId); - ValidatePath(errors, route.Match.Path, route.RouteId); - ValidateMethods(errors, route.Match.Methods, route.RouteId); - ValidateHeaders(errors, route.Match.Headers, route.RouteId); - ValidateQueryParameters(errors, route.Match.QueryParameters, route.RouteId); + foreach (var routeValidator in _routeValidators) + { + if (!routeValidator.IsValid(route.Match, route.RouteId, out var validationErrors)) + { + errors.AddRange(validationErrors); + } + } return errors; } @@ -113,145 +90,16 @@ public ValueTask> ValidateClusterAsync(ClusterConfig cluster) } errors.AddRange(_transformBuilder.ValidateCluster(cluster)); - ValidateDestinations(errors, cluster); - ValidateLoadBalancing(errors, cluster); - ValidateSessionAffinity(errors, cluster); - ValidateProxyHttpClient(errors, cluster); - ValidateProxyHttpRequest(errors, cluster); - ValidateHealthChecks(errors, cluster); - return new ValueTask>(errors); - } - - private static void ValidateHost(IList errors, IReadOnlyList? hosts, string routeId) - { - // Host is optional when Path is specified - if (hosts is null || hosts.Count == 0) + foreach (var clusterValidator in _clusterValidators) { - return; - } - - foreach (var host in hosts) - { - if (string.IsNullOrEmpty(host)) - { - errors.Add(new ArgumentException($"Empty host name has been set for route '{routeId}'.")); - } - else if (host.Contains("xn--", StringComparison.OrdinalIgnoreCase)) + if (!clusterValidator.IsValid(cluster, out var validationErrors)) { - errors.Add(new ArgumentException($"Punycode host name '{host}' has been set for route '{routeId}'. Use the unicode host name instead.")); + errors.AddRange(validationErrors); } } - } - - private static void ValidatePath(IList errors, string? path, string routeId) - { - // Path is optional when Host is specified - if (string.IsNullOrEmpty(path)) - { - return; - } - - try - { - RoutePatternFactory.Parse(path); - } - catch (RoutePatternException ex) - { - errors.Add(new ArgumentException($"Invalid path '{path}' for route '{routeId}'.", ex)); - } - } - - private static void ValidateMethods(IList errors, IReadOnlyList? methods, string routeId) - { - // Methods are optional - if (methods is null) - { - return; - } - - var seenMethods = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (var method in methods) - { - if (!seenMethods.Add(method)) - { - errors.Add(new ArgumentException($"Duplicate HTTP method '{method}' for route '{routeId}'.")); - continue; - } - - if (!_validMethods.Contains(method)) - { - errors.Add(new ArgumentException($"Unsupported HTTP method '{method}' has been set for route '{routeId}'.")); - } - } - } - - private static void ValidateHeaders(List errors, IReadOnlyList? headers, string routeId) - { - // Headers are optional - if (headers is null) - { - return; - } - - foreach (var header in headers) - { - if (header is null) - { - errors.Add(new ArgumentException($"A null route header has been set for route '{routeId}'.")); - continue; - } - if (string.IsNullOrEmpty(header.Name)) - { - errors.Add(new ArgumentException($"A null or empty route header name has been set for route '{routeId}'.")); - } - - if ((header.Mode != HeaderMatchMode.Exists && header.Mode != HeaderMatchMode.NotExists) - && (header.Values is null || header.Values.Count == 0)) - { - errors.Add(new ArgumentException($"No header values were set on route header '{header.Name}' for route '{routeId}'.")); - } - - if ((header.Mode == HeaderMatchMode.Exists || header.Mode == HeaderMatchMode.NotExists) && header.Values?.Count > 0) - { - errors.Add(new ArgumentException($"Header values were set when using mode '{header.Mode}' on route header '{header.Name}' for route '{routeId}'.")); - } - } - } - - private static void ValidateQueryParameters(List errors, IReadOnlyList? queryparams, string routeId) - { - // Query Parameters are optional - if (queryparams is null) - { - return; - } - - foreach (var queryparam in queryparams) - { - if (queryparam is null) - { - errors.Add(new ArgumentException($"A null route query parameter has been set for route '{routeId}'.")); - continue; - } - - if (string.IsNullOrEmpty(queryparam.Name)) - { - errors.Add(new ArgumentException($"A null or empty route query parameter name has been set for route '{routeId}'.")); - } - - if (queryparam.Mode != QueryParameterMatchMode.Exists - && (queryparam.Values is null || queryparam.Values.Count == 0)) - { - errors.Add(new ArgumentException($"No query parameter values were set on route query parameter '{queryparam.Name}' for route '{routeId}'.")); - } - - if (queryparam.Mode == QueryParameterMatchMode.Exists && queryparam.Values?.Count > 0) - { - errors.Add(new ArgumentException($"Query parameter values where set when using mode '{nameof(QueryParameterMatchMode.Exists)}' on route query parameter '{queryparam.Name}' for route '{routeId}'.")); - } - } + return new ValueTask>(errors); } private async ValueTask ValidateAuthorizationPolicyAsync(IList errors, string? authorizationPolicyName, string routeId) @@ -367,230 +215,4 @@ private async ValueTask ValidateCorsPolicyAsync(IList errors, string? errors.Add(new ArgumentException($"Unable to retrieve the CORS policy '{corsPolicyName}' for route '{routeId}'.", ex)); } } - - private void ValidateDestinations(IList errors, ClusterConfig cluster) - { - if (cluster.Destinations is null) - { - return; - } - foreach (var (name, destination) in cluster.Destinations) - { - if (string.IsNullOrEmpty(destination.Address)) - { - errors.Add(new ArgumentException($"No address found for destination '{name}' on cluster '{cluster.ClusterId}'.")); - } - } - } - - private void ValidateLoadBalancing(IList errors, ClusterConfig cluster) - { - var loadBalancingPolicy = cluster.LoadBalancingPolicy; - if (string.IsNullOrEmpty(loadBalancingPolicy)) - { - // The default. - loadBalancingPolicy = LoadBalancingPolicies.PowerOfTwoChoices; - } - - if (!_loadBalancingPolicies.ContainsKey(loadBalancingPolicy)) - { - errors.Add(new ArgumentException($"No matching {nameof(ILoadBalancingPolicy)} found for the load balancing policy '{loadBalancingPolicy}' set on the cluster '{cluster.ClusterId}'.")); - } - } - - private void ValidateSessionAffinity(IList errors, ClusterConfig cluster) - { - if (!(cluster.SessionAffinity?.Enabled ?? false)) - { - // Session affinity is disabled - return; - } - - // Note some affinity validation takes place in AffinitizeTransformProvider.ValidateCluster. - - var affinityFailurePolicy = cluster.SessionAffinity.FailurePolicy; - if (string.IsNullOrEmpty(affinityFailurePolicy)) - { - // The default. - affinityFailurePolicy = SessionAffinityConstants.FailurePolicies.Redistribute; - } - - if (!_affinityFailurePolicies.ContainsKey(affinityFailurePolicy)) - { - errors.Add(new ArgumentException($"No matching {nameof(IAffinityFailurePolicy)} found for the affinity failure policy name '{affinityFailurePolicy}' set on the cluster '{cluster.ClusterId}'.")); - } - - if (string.IsNullOrEmpty(cluster.SessionAffinity.AffinityKeyName)) - { - errors.Add(new ArgumentException($"Affinity key name set on the cluster '{cluster.ClusterId}' must not be null.")); - } - - var cookieConfig = cluster.SessionAffinity.Cookie; - - if (cookieConfig is null) - { - return; - } - - if (cookieConfig.Expiration is not null && cookieConfig.Expiration <= TimeSpan.Zero) - { - errors.Add(new ArgumentException($"Session affinity cookie expiration must be positive or null.")); - } - - if (cookieConfig.MaxAge is not null && cookieConfig.MaxAge <= TimeSpan.Zero) - { - errors.Add(new ArgumentException($"Session affinity cookie max-age must be positive or null.")); - } - } - - private static void ValidateProxyHttpClient(IList errors, ClusterConfig cluster) - { - if (cluster.HttpClient is null) - { - // Proxy http client options are not set. - return; - } - - if (cluster.HttpClient.MaxConnectionsPerServer is not null && cluster.HttpClient.MaxConnectionsPerServer <= 0) - { - errors.Add(new ArgumentException($"Max connections per server limit set on the cluster '{cluster.ClusterId}' must be positive.")); - } - - var requestHeaderEncoding = cluster.HttpClient.RequestHeaderEncoding; - if (requestHeaderEncoding is not null) - { - try - { - Encoding.GetEncoding(requestHeaderEncoding); - } - catch (ArgumentException aex) - { - errors.Add(new ArgumentException($"Invalid request header encoding '{requestHeaderEncoding}'.", aex)); - } - } - - var responseHeaderEncoding = cluster.HttpClient.ResponseHeaderEncoding; - if (responseHeaderEncoding is not null) - { - try - { - Encoding.GetEncoding(responseHeaderEncoding); - } - catch (ArgumentException aex) - { - errors.Add(new ArgumentException($"Invalid response header encoding '{responseHeaderEncoding}'.", aex)); - } - } - } - - private void ValidateProxyHttpRequest(IList errors, ClusterConfig cluster) - { - if (cluster.HttpRequest is null) - { - // Proxy http request options are not set. - return; - } - - if (cluster.HttpRequest.Version is not null && - cluster.HttpRequest.Version != HttpVersion.Version10 && - cluster.HttpRequest.Version != HttpVersion.Version11 && - cluster.HttpRequest.Version != HttpVersion.Version20 && - cluster.HttpRequest.Version != HttpVersion.Version30) - { - errors.Add(new ArgumentException($"Outgoing request version '{cluster.HttpRequest.Version}' is not any of supported HTTP versions (1.0, 1.1, 2 and 3).")); - } - - if (cluster.HttpRequest.Version == HttpVersion.Version10) - { - Log.Http10Version(_logger); - } - } - - private void ValidateHealthChecks(IList errors, ClusterConfig cluster) - { - var availableDestinationsPolicy = cluster.HealthCheck?.AvailableDestinationsPolicy; - if (string.IsNullOrEmpty(availableDestinationsPolicy)) - { - // The default. - availableDestinationsPolicy = HealthCheckConstants.AvailableDestinations.HealthyOrPanic; - } - - if (!_availableDestinationsPolicies.ContainsKey(availableDestinationsPolicy)) - { - errors.Add(new ArgumentException($"No matching {nameof(IAvailableDestinationsPolicy)} found for the available destinations policy '{availableDestinationsPolicy}' set on the cluster.'{cluster.ClusterId}'.")); - } - - ValidateActiveHealthCheck(errors, cluster); - ValidatePassiveHealthCheck(errors, cluster); - } - - private void ValidateActiveHealthCheck(IList errors, ClusterConfig cluster) - { - if (!(cluster.HealthCheck?.Active?.Enabled ?? false)) - { - // Active health check is disabled - return; - } - - var activeOptions = cluster.HealthCheck.Active; - var policy = activeOptions.Policy; - if (string.IsNullOrEmpty(policy)) - { - // default policy - policy = HealthCheckConstants.ActivePolicy.ConsecutiveFailures; - } - if (!_activeHealthCheckPolicies.ContainsKey(policy)) - { - errors.Add(new ArgumentException($"No matching {nameof(IActiveHealthCheckPolicy)} found for the active health check policy name '{policy}' set on the cluster '{cluster.ClusterId}'.")); - } - - if (activeOptions.Interval is not null && activeOptions.Interval <= TimeSpan.Zero) - { - errors.Add(new ArgumentException($"Destination probing interval set on the cluster '{cluster.ClusterId}' must be positive.")); - } - - if (activeOptions.Timeout is not null && activeOptions.Timeout <= TimeSpan.Zero) - { - errors.Add(new ArgumentException($"Destination probing timeout set on the cluster '{cluster.ClusterId}' must be positive.")); - } - } - - private void ValidatePassiveHealthCheck(IList errors, ClusterConfig cluster) - { - if (!(cluster.HealthCheck?.Passive?.Enabled ?? false)) - { - // Passive health check is disabled - return; - } - - var passiveOptions = cluster.HealthCheck.Passive; - var policy = passiveOptions.Policy; - if (string.IsNullOrEmpty(policy)) - { - // default policy - policy = HealthCheckConstants.PassivePolicy.TransportFailureRate; - } - if (!_passiveHealthCheckPolicies.ContainsKey(policy)) - { - errors.Add(new ArgumentException($"No matching {nameof(IPassiveHealthCheckPolicy)} found for the passive health check policy name '{policy}' set on the cluster '{cluster.ClusterId}'.")); - } - - if (passiveOptions.ReactivationPeriod is not null && passiveOptions.ReactivationPeriod <= TimeSpan.Zero) - { - errors.Add(new ArgumentException($"Unhealthy destination reactivation period set on the cluster '{cluster.ClusterId}' must be positive.")); - } - } - - private static class Log - { - private static readonly Action _http10RequestVersionDetected = LoggerMessage.Define( - LogLevel.Warning, - EventIds.Http10RequestVersionDetected, - "The HttpRequest version is set to 1.0 which can result in poor performance and port exhaustion. Use 1.1, 2, or 3 instead."); - - public static void Http10Version(ILogger logger) - { - _http10RequestVersionDetected(logger, null); - } - } } diff --git a/src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs new file mode 100644 index 000000000..021a15e0c --- /dev/null +++ b/src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs @@ -0,0 +1,45 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; + +namespace Yarp.ReverseProxy.Configuration.RouteValidators; + +internal sealed class HeadersValidator : IRouteValidator +{ + public IList Validate(RouteMatch route, string routeId) + { + // Headers are optional + if (route.Headers is null) + { + return Array.Empty(); + } + + var errors = new List(); + foreach (var header in route.Headers) + { + if (header is null) + { + errors.Add(new ArgumentException($"A null route header has been set for route '{routeId}'.")); + continue; + } + + if (string.IsNullOrEmpty(header.Name)) + { + errors.Add(new ArgumentException($"A null or empty route header name has been set for route '{routeId}'.")); + } + + if (header.Mode != HeaderMatchMode.Exists && header.Mode != HeaderMatchMode.NotExists + && (header.Values is null || header.Values.Count == 0)) + { + errors.Add(new ArgumentException($"No header values were set on route header '{header.Name}' for route '{routeId}'.")); + } + + if ((header.Mode == HeaderMatchMode.Exists || header.Mode == HeaderMatchMode.NotExists) && header.Values?.Count > 0) + { + errors.Add(new ArgumentException($"Header values were set when using mode '{header.Mode}' on route header '{header.Name}' for route '{routeId}'.")); + } + } + + return errors; + } +} diff --git a/src/ReverseProxy/Configuration/RouteValidators/HostValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/HostValidator.cs new file mode 100644 index 000000000..6b7fb4647 --- /dev/null +++ b/src/ReverseProxy/Configuration/RouteValidators/HostValidator.cs @@ -0,0 +1,32 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; + +namespace Yarp.ReverseProxy.Configuration.RouteValidators; + +internal sealed class HostValidator : IRouteValidator +{ + public IList Validate(RouteMatch route, string routeId) + { + // Host is optional when Path is specified + if (route.Hosts is null || route.Hosts.Count == 0) + { + return Array.Empty(); + } + + var errors = new List(); + foreach (var host in route.Hosts) + { + if (string.IsNullOrEmpty(host)) + { + errors.Add(new ArgumentException($"Empty host name has been set for route '{routeId}'.")); + } + else if (host.Contains("xn--", StringComparison.OrdinalIgnoreCase)) + { + errors.Add(new ArgumentException($"Punycode host name '{host}' has been set for route '{routeId}'. Use the unicode host name instead.")); + } + } + + return errors; + } +} diff --git a/src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs new file mode 100644 index 000000000..e07583015 --- /dev/null +++ b/src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs @@ -0,0 +1,15 @@ +using System; +using System.Collections.Generic; + +namespace Yarp.ReverseProxy.Configuration.RouteValidators; + +internal interface IRouteValidator +{ + protected IList Validate(RouteMatch route, string routeId); + + public bool IsValid(RouteMatch route, string routeId, out IList errors) + { + errors = Validate(route, routeId); + return errors.Count == 0; + } +} diff --git a/src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs new file mode 100644 index 000000000..2b41b0e54 --- /dev/null +++ b/src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs @@ -0,0 +1,40 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; + +namespace Yarp.ReverseProxy.Configuration.RouteValidators; + +internal sealed class MethodsValidator : IRouteValidator +{ + private static readonly HashSet _validMethods = new(StringComparer.OrdinalIgnoreCase) + { + "HEAD", "OPTIONS", "GET", "PUT", "POST", "PATCH", "DELETE", "TRACE", + }; + + public IList Validate(RouteMatch route, string routeId) + { + // Methods are optional + if (route.Methods is null) + { + return Array.Empty(); + } + + var errors = new List(); + var seenMethods = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var method in route.Methods) + { + if (!seenMethods.Add(method)) + { + errors.Add(new ArgumentException($"Duplicate HTTP method '{method}' for route '{routeId}'.")); + continue; + } + + if (!_validMethods.Contains(method)) + { + errors.Add(new ArgumentException($"Unsupported HTTP method '{method}' has been set for route '{routeId}'.")); + } + } + + return errors; + } +} diff --git a/src/ReverseProxy/Configuration/RouteValidators/PathValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/PathValidator.cs new file mode 100644 index 000000000..cc9a21e08 --- /dev/null +++ b/src/ReverseProxy/Configuration/RouteValidators/PathValidator.cs @@ -0,0 +1,30 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using Microsoft.AspNetCore.Routing.Patterns; + +namespace Yarp.ReverseProxy.Configuration.RouteValidators; + +internal sealed class PathValidator : IRouteValidator +{ + public IList Validate(RouteMatch route, string routeId) + { + // Path is optional when Host is specified + if (string.IsNullOrEmpty(route.Path)) + { + return ImmutableList.Empty; + } + + var errors = new List(); + try + { + RoutePatternFactory.Parse(route.Path); + } + catch (RoutePatternException ex) + { + errors.Add(new ArgumentException($"Invalid path '{route.Path}' for route '{routeId}'.", ex)); + } + + return errors; + } +} diff --git a/src/ReverseProxy/Configuration/RouteValidators/QueryParametersValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/QueryParametersValidator.cs new file mode 100644 index 000000000..6ffacbb5e --- /dev/null +++ b/src/ReverseProxy/Configuration/RouteValidators/QueryParametersValidator.cs @@ -0,0 +1,44 @@ +using System; +using System.Collections.Generic; + +namespace Yarp.ReverseProxy.Configuration.RouteValidators; + +internal sealed class QueryParametersValidator : IRouteValidator +{ + public IList Validate(RouteMatch route, string routeId) + { + // Query Parameters are optional + if (route.QueryParameters is null) + { + return Array.Empty(); + } + + var errors = new List(); + foreach (var queryParameter in route.QueryParameters) + { + if (queryParameter is null) + { + errors.Add(new ArgumentException($"A null route query parameter has been set for route '{routeId}'.")); + continue; + } + + if (string.IsNullOrEmpty(queryParameter.Name)) + { + errors.Add(new ArgumentException($"A null or empty route query parameter name has been set for route '{routeId}'.")); + } + + if (queryParameter.Mode != QueryParameterMatchMode.Exists + && (queryParameter.Values is null || queryParameter.Values.Count == 0)) + { + errors.Add(new ArgumentException($"No query parameter values were set on route query parameter '{queryParameter.Name}' for route '{routeId}'.")); + } + + if (queryParameter.Mode == QueryParameterMatchMode.Exists && queryParameter.Values?.Count > 0) + { + errors.Add(new ArgumentException($"Query parameter values where set when using mode '{nameof(QueryParameterMatchMode.Exists)}' on route query parameter '{queryParameter.Name}' for route '{routeId}'.")); + } + } + + return errors; + } +} diff --git a/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs b/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs index d91dc0062..63ef13df2 100644 --- a/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs +++ b/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs @@ -6,6 +6,8 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Yarp.ReverseProxy.Configuration; +using Yarp.ReverseProxy.Configuration.ClusterValidators; +using Yarp.ReverseProxy.Configuration.RouteValidators; using Yarp.ReverseProxy.Delegation; using Yarp.ReverseProxy.Forwarder; using Yarp.ReverseProxy.Health; @@ -25,6 +27,17 @@ public static IReverseProxyBuilder AddConfigBuilder(this IReverseProxyBuilder bu { builder.Services.TryAddSingleton(); builder.Services.TryAddSingleton(); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); builder.Services.TryAddSingleton(); builder.AddTransformFactory(); builder.AddTransformFactory(); From a910dcf76355fb0bb1e0ed649d0f74088e819587 Mon Sep 17 00:00:00 2001 From: lbjn Date: Thu, 16 Nov 2023 20:40:28 +0100 Subject: [PATCH 02/14] fix review comments --- .../ClusterValidators/DestinationValidator.cs | 7 ++---- .../ClusterValidators/HealthCheckValidator.cs | 13 ++++------ .../ClusterValidators/IClusterValidator.cs | 10 ++------ .../LoadBalancingValidator.cs | 5 +--- .../ProxyHttpClientValidator.cs | 25 +++++++++---------- .../ProxyHttpRequestValidator.cs | 8 ++---- .../SessionAffinityValidator.cs | 9 +++---- .../Configuration/ConfigValidator.cs | 10 ++------ .../RouteValidators/HeadersValidator.cs | 9 +++---- .../RouteValidators/HostValidator.cs | 9 +++---- .../RouteValidators/IRouteValidator.cs | 10 ++------ .../RouteValidators/MethodsValidator.cs | 9 +++---- .../RouteValidators/PathValidator.cs | 9 +++---- .../QueryParametersValidator.cs | 9 +++---- 14 files changed, 46 insertions(+), 96 deletions(-) diff --git a/src/ReverseProxy/Configuration/ClusterValidators/DestinationValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/DestinationValidator.cs index 753f49581..8a0cd693c 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/DestinationValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/DestinationValidator.cs @@ -5,14 +5,13 @@ namespace Yarp.ReverseProxy.Configuration.ClusterValidators; internal sealed class DestinationValidator : IClusterValidator { - public IList Validate(ClusterConfig cluster) + public void AddValidationErrors(ClusterConfig cluster, IList errors) { if (cluster.Destinations is null) { - return Array.Empty(); + return; } - var errors = new List(); foreach (var (name, destination) in cluster.Destinations) { if (string.IsNullOrEmpty(destination.Address)) @@ -20,7 +19,5 @@ public IList Validate(ClusterConfig cluster) errors.Add(new ArgumentException($"No address found for destination '{name}' on cluster '{cluster.ClusterId}'.")); } } - - return errors; } } diff --git a/src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs index 031265e53..ed3b8f61e 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs @@ -20,7 +20,7 @@ public HealthCheckValidator(IEnumerable availableD _passiveHealthCheckPolicies = passiveHealthCheckPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(availableDestinationsPolicies)); } - public IList Validate(ClusterConfig cluster) + public void AddValidationErrors(ClusterConfig cluster, IList errors) { var availableDestinationsPolicy = cluster.HealthCheck?.AvailableDestinationsPolicy; if (string.IsNullOrEmpty(availableDestinationsPolicy)) @@ -29,19 +29,16 @@ public IList Validate(ClusterConfig cluster) availableDestinationsPolicy = HealthCheckConstants.AvailableDestinations.HealthyOrPanic; } - var errors = new List(); if (!_availableDestinationsPolicies.ContainsKey(availableDestinationsPolicy)) { errors.Add(new ArgumentException($"No matching {nameof(IAvailableDestinationsPolicy)} found for the available destinations policy '{availableDestinationsPolicy}' set on the cluster.'{cluster.ClusterId}'.")); } - ValidateActiveHealthCheck(errors, cluster); - ValidatePassiveHealthCheck(errors, cluster); - - return errors; + ValidateActiveHealthCheck(cluster, errors); + ValidatePassiveHealthCheck(cluster, errors); } - private void ValidateActiveHealthCheck(IList errors, ClusterConfig cluster) + private void ValidateActiveHealthCheck(ClusterConfig cluster, IList errors) { if (!(cluster.HealthCheck?.Active?.Enabled ?? false)) { @@ -72,7 +69,7 @@ private void ValidateActiveHealthCheck(IList errors, ClusterConfig cl } } - private void ValidatePassiveHealthCheck(IList errors, ClusterConfig cluster) + private void ValidatePassiveHealthCheck(ClusterConfig cluster, IList errors) { if (!(cluster.HealthCheck?.Passive?.Enabled ?? false)) { diff --git a/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs index d12e98103..a4f0c2de3 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs @@ -3,13 +3,7 @@ namespace Yarp.ReverseProxy.Configuration.ClusterValidators; -internal interface IClusterValidator +public interface IClusterValidator { - protected IList Validate(ClusterConfig cluster); - - public bool IsValid(ClusterConfig cluster, out IList errors) - { - errors = Validate(cluster); - return errors.Count == 0; - } + public void AddValidationErrors(ClusterConfig cluster, IList errors); } diff --git a/src/ReverseProxy/Configuration/ClusterValidators/LoadBalancingValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/LoadBalancingValidator.cs index 280d1f42c..2c0cad7da 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/LoadBalancingValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/LoadBalancingValidator.cs @@ -14,7 +14,7 @@ public LoadBalancingValidator(IEnumerable loadBalancingPol _loadBalancingPolicies = loadBalancingPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(loadBalancingPolicies)); } - public IList Validate(ClusterConfig cluster) + public void AddValidationErrors(ClusterConfig cluster, IList errors) { var loadBalancingPolicy = cluster.LoadBalancingPolicy; if (string.IsNullOrEmpty(loadBalancingPolicy)) @@ -23,12 +23,9 @@ public IList Validate(ClusterConfig cluster) loadBalancingPolicy = LoadBalancingPolicies.PowerOfTwoChoices; } - var errors = new List(); if (!_loadBalancingPolicies.ContainsKey(loadBalancingPolicy)) { errors.Add(new ArgumentException($"No matching {nameof(ILoadBalancingPolicy)} found for the load balancing policy '{loadBalancingPolicy}' set on the cluster '{cluster.ClusterId}'.")); } - - return errors; } } diff --git a/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpClientValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpClientValidator.cs index 987a38c99..86da14333 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpClientValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpClientValidator.cs @@ -6,15 +6,14 @@ namespace Yarp.ReverseProxy.Configuration.ClusterValidators; internal sealed class ProxyHttpClientValidator : IClusterValidator { - public IList Validate(ClusterConfig cluster) + public void AddValidationErrors(ClusterConfig cluster, IList errors) { if (cluster.HttpClient is null) { // Proxy http client options are not set. - return Array.Empty(); + return; } - var errors = new List(); if (cluster.HttpClient.MaxConnectionsPerServer is not null && cluster.HttpClient.MaxConnectionsPerServer <= 0) { errors.Add(new ArgumentException($"Max connections per server limit set on the cluster '{cluster.ClusterId}' must be positive.")); @@ -34,18 +33,18 @@ public IList Validate(ClusterConfig cluster) } var responseHeaderEncoding = cluster.HttpClient.ResponseHeaderEncoding; - if (responseHeaderEncoding is not null) + if (responseHeaderEncoding is null) { - try - { - Encoding.GetEncoding(responseHeaderEncoding); - } - catch (ArgumentException aex) - { - errors.Add(new ArgumentException($"Invalid response header encoding '{responseHeaderEncoding}'.", aex)); - } + return; } - return errors; + try + { + Encoding.GetEncoding(responseHeaderEncoding); + } + catch (ArgumentException aex) + { + errors.Add(new ArgumentException($"Invalid response header encoding '{responseHeaderEncoding}'.", aex)); + } } } diff --git a/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs index d8ef4c9d2..191f11016 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs @@ -14,16 +14,14 @@ public ProxyHttpRequestValidator(ILogger logger) _logger = logger; } - public IList Validate(ClusterConfig cluster) + public void AddValidationErrors(ClusterConfig cluster, IList errors) { if (cluster.HttpRequest is null) { // Proxy http request options are not set. - return Array.Empty(); + return; } - var errors = new List(); - if (cluster.HttpRequest.Version is not null && cluster.HttpRequest.Version != HttpVersion.Version10 && cluster.HttpRequest.Version != HttpVersion.Version11 && @@ -37,8 +35,6 @@ public IList Validate(ClusterConfig cluster) { Log.Http10Version(_logger); } - - return errors; } private static class Log diff --git a/src/ReverseProxy/Configuration/ClusterValidators/SessionAffinityValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/SessionAffinityValidator.cs index fc5c39e85..ac8c04072 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/SessionAffinityValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/SessionAffinityValidator.cs @@ -15,12 +15,12 @@ public SessionAffinityValidator(IEnumerable affinityFail _affinityFailurePolicies = affinityFailurePolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(affinityFailurePolicies)); } - public IList Validate(ClusterConfig cluster) + public void AddValidationErrors(ClusterConfig cluster, IList errors) { if (!(cluster.SessionAffinity?.Enabled ?? false)) { // Session affinity is disabled - return Array.Empty(); + return; } // Note some affinity validation takes place in AffinitizeTransformProvider.ValidateCluster. @@ -32,7 +32,6 @@ public IList Validate(ClusterConfig cluster) affinityFailurePolicy = SessionAffinityConstants.FailurePolicies.Redistribute; } - var errors = new List(); if (!_affinityFailurePolicies.ContainsKey(affinityFailurePolicy)) { errors.Add(new ArgumentException($"No matching {nameof(IAffinityFailurePolicy)} found for the affinity failure policy name '{affinityFailurePolicy}' set on the cluster '{cluster.ClusterId}'.")); @@ -47,7 +46,7 @@ public IList Validate(ClusterConfig cluster) if (cookieConfig is null) { - return errors; + return; } if (cookieConfig.Expiration is not null && cookieConfig.Expiration <= TimeSpan.Zero) @@ -59,7 +58,5 @@ public IList Validate(ClusterConfig cluster) { errors.Add(new ArgumentException($"Session affinity cookie max-age must be positive or null.")); } - - return errors; } } diff --git a/src/ReverseProxy/Configuration/ConfigValidator.cs b/src/ReverseProxy/Configuration/ConfigValidator.cs index bfcd6cbda..10fe6d334 100644 --- a/src/ReverseProxy/Configuration/ConfigValidator.cs +++ b/src/ReverseProxy/Configuration/ConfigValidator.cs @@ -69,10 +69,7 @@ public async ValueTask> ValidateRouteAsync(RouteConfig route) foreach (var routeValidator in _routeValidators) { - if (!routeValidator.IsValid(route.Match, route.RouteId, out var validationErrors)) - { - errors.AddRange(validationErrors); - } + routeValidator.AddValidationErrors(route.Match, route.RouteId, errors); } return errors; @@ -93,10 +90,7 @@ public ValueTask> ValidateClusterAsync(ClusterConfig cluster) foreach (var clusterValidator in _clusterValidators) { - if (!clusterValidator.IsValid(cluster, out var validationErrors)) - { - errors.AddRange(validationErrors); - } + clusterValidator.AddValidationErrors(cluster, errors); } return new ValueTask>(errors); diff --git a/src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs index 021a15e0c..cb6d11df0 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs @@ -6,15 +6,14 @@ namespace Yarp.ReverseProxy.Configuration.RouteValidators; internal sealed class HeadersValidator : IRouteValidator { - public IList Validate(RouteMatch route, string routeId) + public void AddValidationErrors(RouteMatch route, string routeId, IList errors) { - // Headers are optional if (route.Headers is null) { - return Array.Empty(); + // Headers are optional + return; } - var errors = new List(); foreach (var header in route.Headers) { if (header is null) @@ -39,7 +38,5 @@ public IList Validate(RouteMatch route, string routeId) errors.Add(new ArgumentException($"Header values were set when using mode '{header.Mode}' on route header '{header.Name}' for route '{routeId}'.")); } } - - return errors; } } diff --git a/src/ReverseProxy/Configuration/RouteValidators/HostValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/HostValidator.cs index 6b7fb4647..430c816ca 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/HostValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/HostValidator.cs @@ -6,15 +6,14 @@ namespace Yarp.ReverseProxy.Configuration.RouteValidators; internal sealed class HostValidator : IRouteValidator { - public IList Validate(RouteMatch route, string routeId) + public void AddValidationErrors(RouteMatch route, string routeId, IList errors) { - // Host is optional when Path is specified if (route.Hosts is null || route.Hosts.Count == 0) { - return Array.Empty(); + // Host is optional when Path is specified + return; } - var errors = new List(); foreach (var host in route.Hosts) { if (string.IsNullOrEmpty(host)) @@ -26,7 +25,5 @@ public IList Validate(RouteMatch route, string routeId) errors.Add(new ArgumentException($"Punycode host name '{host}' has been set for route '{routeId}'. Use the unicode host name instead.")); } } - - return errors; } } diff --git a/src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs index e07583015..da2b92b2e 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs @@ -3,13 +3,7 @@ namespace Yarp.ReverseProxy.Configuration.RouteValidators; -internal interface IRouteValidator +public interface IRouteValidator { - protected IList Validate(RouteMatch route, string routeId); - - public bool IsValid(RouteMatch route, string routeId, out IList errors) - { - errors = Validate(route, routeId); - return errors.Count == 0; - } + public void AddValidationErrors(RouteMatch route, string routeId, IList errors); } diff --git a/src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs index 2b41b0e54..c2f4468df 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs @@ -11,15 +11,14 @@ internal sealed class MethodsValidator : IRouteValidator "HEAD", "OPTIONS", "GET", "PUT", "POST", "PATCH", "DELETE", "TRACE", }; - public IList Validate(RouteMatch route, string routeId) + public void AddValidationErrors(RouteMatch route, string routeId, IList errors) { - // Methods are optional if (route.Methods is null) { - return Array.Empty(); + // Methods are optional + return; } - var errors = new List(); var seenMethods = new HashSet(StringComparer.OrdinalIgnoreCase); foreach (var method in route.Methods) { @@ -34,7 +33,5 @@ public IList Validate(RouteMatch route, string routeId) errors.Add(new ArgumentException($"Unsupported HTTP method '{method}' has been set for route '{routeId}'.")); } } - - return errors; } } diff --git a/src/ReverseProxy/Configuration/RouteValidators/PathValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/PathValidator.cs index cc9a21e08..dc4dc663a 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/PathValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/PathValidator.cs @@ -7,15 +7,14 @@ namespace Yarp.ReverseProxy.Configuration.RouteValidators; internal sealed class PathValidator : IRouteValidator { - public IList Validate(RouteMatch route, string routeId) + public void AddValidationErrors(RouteMatch route, string routeId, IList errors) { - // Path is optional when Host is specified if (string.IsNullOrEmpty(route.Path)) { - return ImmutableList.Empty; + // Path is optional when Host is specified + return; } - var errors = new List(); try { RoutePatternFactory.Parse(route.Path); @@ -24,7 +23,5 @@ public IList Validate(RouteMatch route, string routeId) { errors.Add(new ArgumentException($"Invalid path '{route.Path}' for route '{routeId}'.", ex)); } - - return errors; } } diff --git a/src/ReverseProxy/Configuration/RouteValidators/QueryParametersValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/QueryParametersValidator.cs index 6ffacbb5e..039ef190c 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/QueryParametersValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/QueryParametersValidator.cs @@ -5,15 +5,14 @@ namespace Yarp.ReverseProxy.Configuration.RouteValidators; internal sealed class QueryParametersValidator : IRouteValidator { - public IList Validate(RouteMatch route, string routeId) + public void AddValidationErrors(RouteMatch route, string routeId, IList errors) { - // Query Parameters are optional if (route.QueryParameters is null) { - return Array.Empty(); + // Query Parameters are optional + return; } - var errors = new List(); foreach (var queryParameter in route.QueryParameters) { if (queryParameter is null) @@ -38,7 +37,5 @@ public IList Validate(RouteMatch route, string routeId) errors.Add(new ArgumentException($"Query parameter values where set when using mode '{nameof(QueryParameterMatchMode.Exists)}' on route query parameter '{queryParameter.Name}' for route '{routeId}'.")); } } - - return errors; } } From b8d1b55050ada1bab4457ee237da6c0e24477209 Mon Sep 17 00:00:00 2001 From: Lars Johansen Date: Sun, 19 Nov 2023 15:07:42 +0100 Subject: [PATCH 03/14] Update src/ReverseProxy/Configuration/ConfigValidator.cs Co-authored-by: Chris Ross --- src/ReverseProxy/Configuration/ConfigValidator.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ReverseProxy/Configuration/ConfigValidator.cs b/src/ReverseProxy/Configuration/ConfigValidator.cs index c42aa606f..4cb9928d8 100644 --- a/src/ReverseProxy/Configuration/ConfigValidator.cs +++ b/src/ReverseProxy/Configuration/ConfigValidator.cs @@ -29,8 +29,9 @@ internal sealed class ConfigValidator : IConfigValidator #if NET8_0_OR_GREATER private readonly IOptionsMonitor _timeoutOptions; #endif - private readonly IEnumerable _routeValidators; - private readonly IEnumerable _clusterValidators; + private readonly List _routeValidators; + private readonly List _clusterValidators; + public ConfigValidator(ITransformBuilder transformBuilder, IAuthorizationPolicyProvider authorizationPolicyProvider, IYarpRateLimiterPolicyProvider rateLimiterPolicyProvider, From 2baec7ffdf4f91f8030dddca538e6fb33c0db72f Mon Sep 17 00:00:00 2001 From: Lars Johansen Date: Sun, 19 Nov 2023 15:08:35 +0100 Subject: [PATCH 04/14] Update src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs Co-authored-by: Chris Ross --- .../Configuration/ClusterValidators/IClusterValidator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs index a4f0c2de3..4b77a1e1c 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs @@ -5,5 +5,5 @@ namespace Yarp.ReverseProxy.Configuration.ClusterValidators; public interface IClusterValidator { - public void AddValidationErrors(ClusterConfig cluster, IList errors); + public ValueTask ValidateAsync(ClusterConfig cluster, IList errors); } From 21d77f9a3fdc7edb9fcb7edbbeb6d1834916c20e Mon Sep 17 00:00:00 2001 From: lbjn Date: Sun, 19 Nov 2023 15:26:32 +0100 Subject: [PATCH 05/14] Convert to valuetask return type --- .../ClusterValidators/DestinationValidator.cs | 7 +++++-- .../ClusterValidators/HealthCheckValidator.cs | 5 ++++- .../ClusterValidators/IClusterValidator.cs | 1 + .../ClusterValidators/LoadBalancingValidator.cs | 5 ++++- .../ClusterValidators/ProxyHttpClientValidator.cs | 9 ++++++--- .../ClusterValidators/ProxyHttpRequestValidator.cs | 7 +++++-- .../ClusterValidators/SessionAffinityValidator.cs | 10 ++++++---- src/ReverseProxy/Configuration/ConfigValidator.cs | 14 +++++++------- .../RouteValidators/HeadersValidator.cs | 7 +++++-- .../Configuration/RouteValidators/HostValidator.cs | 7 +++++-- .../RouteValidators/IRouteValidator.cs | 3 ++- .../RouteValidators/MethodsValidator.cs | 7 +++++-- .../Configuration/RouteValidators/PathValidator.cs | 7 +++++-- .../RouteValidators/QueryParametersValidator.cs | 7 +++++-- 14 files changed, 65 insertions(+), 31 deletions(-) diff --git a/src/ReverseProxy/Configuration/ClusterValidators/DestinationValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/DestinationValidator.cs index 8a0cd693c..5f0846cee 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/DestinationValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/DestinationValidator.cs @@ -1,15 +1,16 @@ using System; using System.Collections.Generic; +using System.Threading.Tasks; namespace Yarp.ReverseProxy.Configuration.ClusterValidators; internal sealed class DestinationValidator : IClusterValidator { - public void AddValidationErrors(ClusterConfig cluster, IList errors) + public ValueTask ValidateAsync(ClusterConfig cluster, IList errors) { if (cluster.Destinations is null) { - return; + return ValueTask.CompletedTask; } foreach (var (name, destination) in cluster.Destinations) @@ -19,5 +20,7 @@ public void AddValidationErrors(ClusterConfig cluster, IList errors) errors.Add(new ArgumentException($"No address found for destination '{name}' on cluster '{cluster.ClusterId}'.")); } } + + return ValueTask.CompletedTask; } } diff --git a/src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs index ed3b8f61e..ec2247ea4 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Frozen; using System.Collections.Generic; +using System.Threading.Tasks; using Yarp.ReverseProxy.Health; using Yarp.ReverseProxy.Utilities; @@ -20,7 +21,7 @@ public HealthCheckValidator(IEnumerable availableD _passiveHealthCheckPolicies = passiveHealthCheckPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(availableDestinationsPolicies)); } - public void AddValidationErrors(ClusterConfig cluster, IList errors) + public ValueTask ValidateAsync(ClusterConfig cluster, IList errors) { var availableDestinationsPolicy = cluster.HealthCheck?.AvailableDestinationsPolicy; if (string.IsNullOrEmpty(availableDestinationsPolicy)) @@ -36,6 +37,8 @@ public void AddValidationErrors(ClusterConfig cluster, IList errors) ValidateActiveHealthCheck(cluster, errors); ValidatePassiveHealthCheck(cluster, errors); + + return ValueTask.CompletedTask; } private void ValidateActiveHealthCheck(ClusterConfig cluster, IList errors) diff --git a/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs index 4b77a1e1c..d9a95c800 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Threading.Tasks; namespace Yarp.ReverseProxy.Configuration.ClusterValidators; diff --git a/src/ReverseProxy/Configuration/ClusterValidators/LoadBalancingValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/LoadBalancingValidator.cs index 2c0cad7da..ab92645a9 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/LoadBalancingValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/LoadBalancingValidator.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Frozen; using System.Collections.Generic; +using System.Threading.Tasks; using Yarp.ReverseProxy.LoadBalancing; using Yarp.ReverseProxy.Utilities; @@ -14,7 +15,7 @@ public LoadBalancingValidator(IEnumerable loadBalancingPol _loadBalancingPolicies = loadBalancingPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(loadBalancingPolicies)); } - public void AddValidationErrors(ClusterConfig cluster, IList errors) + public ValueTask ValidateAsync(ClusterConfig cluster, IList errors) { var loadBalancingPolicy = cluster.LoadBalancingPolicy; if (string.IsNullOrEmpty(loadBalancingPolicy)) @@ -27,5 +28,7 @@ public void AddValidationErrors(ClusterConfig cluster, IList errors) { errors.Add(new ArgumentException($"No matching {nameof(ILoadBalancingPolicy)} found for the load balancing policy '{loadBalancingPolicy}' set on the cluster '{cluster.ClusterId}'.")); } + + return ValueTask.CompletedTask; } } diff --git a/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpClientValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpClientValidator.cs index 86da14333..1c79901e9 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpClientValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpClientValidator.cs @@ -1,17 +1,18 @@ using System; using System.Collections.Generic; using System.Text; +using System.Threading.Tasks; namespace Yarp.ReverseProxy.Configuration.ClusterValidators; internal sealed class ProxyHttpClientValidator : IClusterValidator { - public void AddValidationErrors(ClusterConfig cluster, IList errors) + public ValueTask ValidateAsync(ClusterConfig cluster, IList errors) { if (cluster.HttpClient is null) { // Proxy http client options are not set. - return; + return ValueTask.CompletedTask; } if (cluster.HttpClient.MaxConnectionsPerServer is not null && cluster.HttpClient.MaxConnectionsPerServer <= 0) @@ -35,7 +36,7 @@ public void AddValidationErrors(ClusterConfig cluster, IList errors) var responseHeaderEncoding = cluster.HttpClient.ResponseHeaderEncoding; if (responseHeaderEncoding is null) { - return; + return ValueTask.CompletedTask; } try @@ -46,5 +47,7 @@ public void AddValidationErrors(ClusterConfig cluster, IList errors) { errors.Add(new ArgumentException($"Invalid response header encoding '{responseHeaderEncoding}'.", aex)); } + + return ValueTask.CompletedTask; } } diff --git a/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs index 191f11016..8a7d64804 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Net; +using System.Threading.Tasks; using Microsoft.Extensions.Logging; namespace Yarp.ReverseProxy.Configuration.ClusterValidators; @@ -14,12 +15,12 @@ public ProxyHttpRequestValidator(ILogger logger) _logger = logger; } - public void AddValidationErrors(ClusterConfig cluster, IList errors) + public ValueTask ValidateAsync(ClusterConfig cluster, IList errors) { if (cluster.HttpRequest is null) { // Proxy http request options are not set. - return; + return ValueTask.CompletedTask; } if (cluster.HttpRequest.Version is not null && @@ -35,6 +36,8 @@ public void AddValidationErrors(ClusterConfig cluster, IList errors) { Log.Http10Version(_logger); } + + return ValueTask.CompletedTask; } private static class Log diff --git a/src/ReverseProxy/Configuration/ClusterValidators/SessionAffinityValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/SessionAffinityValidator.cs index ac8c04072..b74b8109a 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/SessionAffinityValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/SessionAffinityValidator.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Frozen; using System.Collections.Generic; +using System.Threading.Tasks; using Yarp.ReverseProxy.SessionAffinity; using Yarp.ReverseProxy.Utilities; @@ -15,16 +16,15 @@ public SessionAffinityValidator(IEnumerable affinityFail _affinityFailurePolicies = affinityFailurePolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(affinityFailurePolicies)); } - public void AddValidationErrors(ClusterConfig cluster, IList errors) + public ValueTask ValidateAsync(ClusterConfig cluster, IList errors) { if (!(cluster.SessionAffinity?.Enabled ?? false)) { // Session affinity is disabled - return; + return ValueTask.CompletedTask; } // Note some affinity validation takes place in AffinitizeTransformProvider.ValidateCluster. - var affinityFailurePolicy = cluster.SessionAffinity.FailurePolicy; if (string.IsNullOrEmpty(affinityFailurePolicy)) { @@ -46,7 +46,7 @@ public void AddValidationErrors(ClusterConfig cluster, IList errors) if (cookieConfig is null) { - return; + return ValueTask.CompletedTask; } if (cookieConfig.Expiration is not null && cookieConfig.Expiration <= TimeSpan.Zero) @@ -58,5 +58,7 @@ public void AddValidationErrors(ClusterConfig cluster, IList errors) { errors.Add(new ArgumentException($"Session affinity cookie max-age must be positive or null.")); } + + return ValueTask.CompletedTask; } } diff --git a/src/ReverseProxy/Configuration/ConfigValidator.cs b/src/ReverseProxy/Configuration/ConfigValidator.cs index 4cb9928d8..5d28552a6 100644 --- a/src/ReverseProxy/Configuration/ConfigValidator.cs +++ b/src/ReverseProxy/Configuration/ConfigValidator.cs @@ -31,7 +31,7 @@ internal sealed class ConfigValidator : IConfigValidator #endif private readonly List _routeValidators; private readonly List _clusterValidators; - + public ConfigValidator(ITransformBuilder transformBuilder, IAuthorizationPolicyProvider authorizationPolicyProvider, IYarpRateLimiterPolicyProvider rateLimiterPolicyProvider, @@ -39,8 +39,8 @@ public ConfigValidator(ITransformBuilder transformBuilder, #if NET8_0_OR_GREATER IOptionsMonitor timeoutOptions, #endif - IEnumerable routeValidators, - IEnumerable clusterValidators) + List routeValidators, + List clusterValidators) { _transformBuilder = transformBuilder ?? throw new ArgumentNullException(nameof(transformBuilder)); _authorizationPolicyProvider = authorizationPolicyProvider ?? throw new ArgumentNullException(nameof(authorizationPolicyProvider)); @@ -87,14 +87,14 @@ public async ValueTask> ValidateRouteAsync(RouteConfig route) foreach (var routeValidator in _routeValidators) { - routeValidator.AddValidationErrors(route.Match, route.RouteId, errors); + await routeValidator.ValidateAsync(route.Match, route.RouteId, errors); } return errors; } // Note this performs all validation steps without short circuiting in order to report all possible errors. - public ValueTask> ValidateClusterAsync(ClusterConfig cluster) + public async ValueTask> ValidateClusterAsync(ClusterConfig cluster) { _ = cluster ?? throw new ArgumentNullException(nameof(cluster)); var errors = new List(); @@ -108,10 +108,10 @@ public ValueTask> ValidateClusterAsync(ClusterConfig cluster) foreach (var clusterValidator in _clusterValidators) { - clusterValidator.AddValidationErrors(cluster, errors); + await clusterValidator.ValidateAsync(cluster, errors); } - return new ValueTask>(errors); + return errors; } private async ValueTask ValidateAuthorizationPolicyAsync(IList errors, string? authorizationPolicyName, string routeId) diff --git a/src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs index cb6d11df0..a45f0bfaf 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs @@ -1,17 +1,18 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Threading.Tasks; namespace Yarp.ReverseProxy.Configuration.RouteValidators; internal sealed class HeadersValidator : IRouteValidator { - public void AddValidationErrors(RouteMatch route, string routeId, IList errors) + public ValueTask ValidateAsync(RouteMatch route, string routeId, IList errors) { if (route.Headers is null) { // Headers are optional - return; + return ValueTask.CompletedTask; } foreach (var header in route.Headers) @@ -38,5 +39,7 @@ public void AddValidationErrors(RouteMatch route, string routeId, IList errors) + public ValueTask ValidateAsync(RouteMatch route, string routeId, IList errors) { if (route.Hosts is null || route.Hosts.Count == 0) { // Host is optional when Path is specified - return; + return ValueTask.CompletedTask; } foreach (var host in route.Hosts) @@ -25,5 +26,7 @@ public void AddValidationErrors(RouteMatch route, string routeId, IList errors); + public ValueTask ValidateAsync(RouteMatch route, string routeId, IList errors); } diff --git a/src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs index c2f4468df..bc3bd898f 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Threading.Tasks; namespace Yarp.ReverseProxy.Configuration.RouteValidators; @@ -11,12 +12,12 @@ internal sealed class MethodsValidator : IRouteValidator "HEAD", "OPTIONS", "GET", "PUT", "POST", "PATCH", "DELETE", "TRACE", }; - public void AddValidationErrors(RouteMatch route, string routeId, IList errors) + public ValueTask ValidateAsync(RouteMatch route, string routeId, IList errors) { if (route.Methods is null) { // Methods are optional - return; + return ValueTask.CompletedTask; } var seenMethods = new HashSet(StringComparer.OrdinalIgnoreCase); @@ -33,5 +34,7 @@ public void AddValidationErrors(RouteMatch route, string routeId, IList errors) + public ValueTask ValidateAsync(RouteMatch route, string routeId, IList errors) { if (string.IsNullOrEmpty(route.Path)) { // Path is optional when Host is specified - return; + return ValueTask.CompletedTask; } try @@ -23,5 +24,7 @@ public void AddValidationErrors(RouteMatch route, string routeId, IList errors) + public ValueTask ValidateAsync(RouteMatch route, string routeId, IList errors) { if (route.QueryParameters is null) { // Query Parameters are optional - return; + return ValueTask.CompletedTask; } foreach (var queryParameter in route.QueryParameters) @@ -37,5 +38,7 @@ public void AddValidationErrors(RouteMatch route, string routeId, IList Date: Sun, 19 Nov 2023 15:28:25 +0100 Subject: [PATCH 06/14] revert --- src/ReverseProxy/Configuration/ConfigValidator.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/ReverseProxy/Configuration/ConfigValidator.cs b/src/ReverseProxy/Configuration/ConfigValidator.cs index 5d28552a6..a2e5379e2 100644 --- a/src/ReverseProxy/Configuration/ConfigValidator.cs +++ b/src/ReverseProxy/Configuration/ConfigValidator.cs @@ -29,9 +29,8 @@ internal sealed class ConfigValidator : IConfigValidator #if NET8_0_OR_GREATER private readonly IOptionsMonitor _timeoutOptions; #endif - private readonly List _routeValidators; - private readonly List _clusterValidators; - + private readonly IEnumerable _routeValidators; + private readonly IEnumerable _clusterValidators; public ConfigValidator(ITransformBuilder transformBuilder, IAuthorizationPolicyProvider authorizationPolicyProvider, IYarpRateLimiterPolicyProvider rateLimiterPolicyProvider, @@ -39,8 +38,8 @@ public ConfigValidator(ITransformBuilder transformBuilder, #if NET8_0_OR_GREATER IOptionsMonitor timeoutOptions, #endif - List routeValidators, - List clusterValidators) + IEnumerable routeValidators, + IEnumerable clusterValidators) { _transformBuilder = transformBuilder ?? throw new ArgumentNullException(nameof(transformBuilder)); _authorizationPolicyProvider = authorizationPolicyProvider ?? throw new ArgumentNullException(nameof(authorizationPolicyProvider)); From b0a6ff5c13530d970ae66b1b9586c51fd02adbf4 Mon Sep 17 00:00:00 2001 From: lbjn Date: Sun, 19 Nov 2023 15:57:43 +0100 Subject: [PATCH 07/14] use routeConfig as parameter --- .../RouteValidators/HeadersValidator.cs | 12 ++++++------ .../Configuration/RouteValidators/HostValidator.cs | 8 ++++---- .../Configuration/RouteValidators/IRouteValidator.cs | 2 +- .../RouteValidators/MethodsValidator.cs | 8 ++++---- .../Configuration/RouteValidators/PathValidator.cs | 6 +++--- .../RouteValidators/QueryParametersValidator.cs | 11 ++++++----- .../Management/IReverseProxyBuilderExtensions.cs | 1 + 7 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs index a45f0bfaf..a955b0e81 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/HeadersValidator.cs @@ -1,14 +1,14 @@ using System; using System.Collections.Generic; -using System.Collections.Immutable; using System.Threading.Tasks; namespace Yarp.ReverseProxy.Configuration.RouteValidators; internal sealed class HeadersValidator : IRouteValidator { - public ValueTask ValidateAsync(RouteMatch route, string routeId, IList errors) + public ValueTask ValidateAsync(RouteConfig routeConfig, IList errors) { + var route = routeConfig.Match; if (route.Headers is null) { // Headers are optional @@ -19,24 +19,24 @@ public ValueTask ValidateAsync(RouteMatch route, string routeId, IList 0) { - errors.Add(new ArgumentException($"Header values were set when using mode '{header.Mode}' on route header '{header.Name}' for route '{routeId}'.")); + errors.Add(new ArgumentException($"Header values were set when using mode '{header.Mode}' on route header '{header.Name}' for route '{routeConfig.RouteId}'.")); } } diff --git a/src/ReverseProxy/Configuration/RouteValidators/HostValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/HostValidator.cs index 5afaa856c..df77551fb 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/HostValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/HostValidator.cs @@ -1,14 +1,14 @@ using System; using System.Collections.Generic; -using System.Collections.Immutable; using System.Threading.Tasks; namespace Yarp.ReverseProxy.Configuration.RouteValidators; internal sealed class HostValidator : IRouteValidator { - public ValueTask ValidateAsync(RouteMatch route, string routeId, IList errors) + public ValueTask ValidateAsync(RouteConfig routeConfig, IList errors) { + var route = routeConfig.Match; if (route.Hosts is null || route.Hosts.Count == 0) { // Host is optional when Path is specified @@ -19,11 +19,11 @@ public ValueTask ValidateAsync(RouteMatch route, string routeId, IList errors); + public ValueTask ValidateAsync(RouteConfig routeConfig, IList errors); } diff --git a/src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs index bc3bd898f..599b0ac10 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/MethodsValidator.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Collections.Immutable; using System.Threading.Tasks; namespace Yarp.ReverseProxy.Configuration.RouteValidators; @@ -12,8 +11,9 @@ internal sealed class MethodsValidator : IRouteValidator "HEAD", "OPTIONS", "GET", "PUT", "POST", "PATCH", "DELETE", "TRACE", }; - public ValueTask ValidateAsync(RouteMatch route, string routeId, IList errors) + public ValueTask ValidateAsync(RouteConfig routeConfig, IList errors) { + var route = routeConfig.Match; if (route.Methods is null) { // Methods are optional @@ -25,13 +25,13 @@ public ValueTask ValidateAsync(RouteMatch route, string routeId, IList errors) + public ValueTask ValidateAsync(RouteConfig routeConfig, IList errors) { + var route = routeConfig.Match; if (string.IsNullOrEmpty(route.Path)) { // Path is optional when Host is specified @@ -22,7 +22,7 @@ public ValueTask ValidateAsync(RouteMatch route, string routeId, IList errors) + public ValueTask ValidateAsync(RouteConfig routeConfig, IList errors) { + var route = routeConfig.Match; if (route.QueryParameters is null) { // Query Parameters are optional @@ -18,24 +19,24 @@ public ValueTask ValidateAsync(RouteMatch route, string routeId, IList 0) { - errors.Add(new ArgumentException($"Query parameter values where set when using mode '{nameof(QueryParameterMatchMode.Exists)}' on route query parameter '{queryParameter.Name}' for route '{routeId}'.")); + errors.Add(new ArgumentException($"Query parameter values where set when using mode '{nameof(QueryParameterMatchMode.Exists)}' on route query parameter '{queryParameter.Name}' for route '{routeConfig.RouteId}'.")); } } diff --git a/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs b/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs index 63ef13df2..f36a4d014 100644 --- a/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs +++ b/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs @@ -27,6 +27,7 @@ public static IReverseProxyBuilder AddConfigBuilder(this IReverseProxyBuilder bu { builder.Services.TryAddSingleton(); builder.Services.TryAddSingleton(); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); From 640ed28f476eb3a4b766e7efc27fe0f9162e259f Mon Sep 17 00:00:00 2001 From: lbjn Date: Sun, 19 Nov 2023 15:58:59 +0100 Subject: [PATCH 08/14] Move AuthorizationPolicyValidation to class --- .../Configuration/ConfigValidator.cs | 50 +--------------- .../AuthorizationPolicyValidator.cs | 58 +++++++++++++++++++ 2 files changed, 61 insertions(+), 47 deletions(-) create mode 100644 src/ReverseProxy/Configuration/RouteValidators/AuthorizationPolicyValidator.cs diff --git a/src/ReverseProxy/Configuration/ConfigValidator.cs b/src/ReverseProxy/Configuration/ConfigValidator.cs index a2e5379e2..5262748d2 100644 --- a/src/ReverseProxy/Configuration/ConfigValidator.cs +++ b/src/ReverseProxy/Configuration/ConfigValidator.cs @@ -23,7 +23,6 @@ namespace Yarp.ReverseProxy.Configuration; internal sealed class ConfigValidator : IConfigValidator { private readonly ITransformBuilder _transformBuilder; - private readonly IAuthorizationPolicyProvider _authorizationPolicyProvider; private readonly IYarpRateLimiterPolicyProvider _rateLimiterPolicyProvider; private readonly ICorsPolicyProvider _corsPolicyProvider; #if NET8_0_OR_GREATER @@ -32,7 +31,6 @@ internal sealed class ConfigValidator : IConfigValidator private readonly IEnumerable _routeValidators; private readonly IEnumerable _clusterValidators; public ConfigValidator(ITransformBuilder transformBuilder, - IAuthorizationPolicyProvider authorizationPolicyProvider, IYarpRateLimiterPolicyProvider rateLimiterPolicyProvider, ICorsPolicyProvider corsPolicyProvider, #if NET8_0_OR_GREATER @@ -42,8 +40,7 @@ public ConfigValidator(ITransformBuilder transformBuilder, IEnumerable clusterValidators) { _transformBuilder = transformBuilder ?? throw new ArgumentNullException(nameof(transformBuilder)); - _authorizationPolicyProvider = authorizationPolicyProvider ?? throw new ArgumentNullException(nameof(authorizationPolicyProvider)); - _rateLimiterPolicyProvider = rateLimiterPolicyProvider ?? throw new ArgumentNullException(nameof(rateLimiterPolicyProvider)); + _rateLimiterPolicyProvider = rateLimiterPolicyProvider ?? throw new ArgumentNullException(nameof(rateLimiterPolicyProvider)); _corsPolicyProvider = corsPolicyProvider ?? throw new ArgumentNullException(nameof(corsPolicyProvider)); #if NET8_0_OR_GREATER _timeoutOptions = timeoutOptions ?? throw new ArgumentNullException(nameof(timeoutOptions)); @@ -64,8 +61,7 @@ public async ValueTask> ValidateRouteAsync(RouteConfig route) } errors.AddRange(_transformBuilder.ValidateRoute(route)); - await ValidateAuthorizationPolicyAsync(errors, route.AuthorizationPolicy, route.RouteId); -#if NET7_0_OR_GREATER + #if NET7_0_OR_GREATER await ValidateRateLimiterPolicyAsync(errors, route.RateLimiterPolicy, route.RouteId); #endif #if NET8_0_OR_GREATER @@ -86,7 +82,7 @@ public async ValueTask> ValidateRouteAsync(RouteConfig route) foreach (var routeValidator in _routeValidators) { - await routeValidator.ValidateAsync(route.Match, route.RouteId, errors); + await routeValidator.ValidateAsync(route, errors); } return errors; @@ -113,46 +109,6 @@ public async ValueTask> ValidateClusterAsync(ClusterConfig clus return errors; } - private async ValueTask ValidateAuthorizationPolicyAsync(IList errors, string? authorizationPolicyName, string routeId) - { - if (string.IsNullOrEmpty(authorizationPolicyName)) - { - return; - } - - if (string.Equals(AuthorizationConstants.Default, authorizationPolicyName, StringComparison.OrdinalIgnoreCase)) - { - var policy = await _authorizationPolicyProvider.GetPolicyAsync(authorizationPolicyName); - if (policy is not null) - { - errors.Add(new ArgumentException($"The application has registered an authorization policy named '{authorizationPolicyName}' that conflicts with the reserved authorization policy name used on this route. The registered policy name needs to be changed for this route to function.")); - } - return; - } - - if (string.Equals(AuthorizationConstants.Anonymous, authorizationPolicyName, StringComparison.OrdinalIgnoreCase)) - { - var policy = await _authorizationPolicyProvider.GetPolicyAsync(authorizationPolicyName); - if (policy is not null) - { - errors.Add(new ArgumentException($"The application has registered an authorization policy named '{authorizationPolicyName}' that conflicts with the reserved authorization policy name used on this route. The registered policy name needs to be changed for this route to function.")); - } - return; - } - - try - { - var policy = await _authorizationPolicyProvider.GetPolicyAsync(authorizationPolicyName); - if (policy is null) - { - errors.Add(new ArgumentException($"Authorization policy '{authorizationPolicyName}' not found for route '{routeId}'.")); - } - } - catch (Exception ex) - { - errors.Add(new ArgumentException($"Unable to retrieve the authorization policy '{authorizationPolicyName}' for route '{routeId}'.", ex)); - } - } #if NET8_0_OR_GREATER private void ValidateTimeoutPolicy(IList errors, string? timeoutPolicyName, TimeSpan? timeout, string routeId) { diff --git a/src/ReverseProxy/Configuration/RouteValidators/AuthorizationPolicyValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/AuthorizationPolicyValidator.cs new file mode 100644 index 000000000..c1f16dd91 --- /dev/null +++ b/src/ReverseProxy/Configuration/RouteValidators/AuthorizationPolicyValidator.cs @@ -0,0 +1,58 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authorization; + +namespace Yarp.ReverseProxy.Configuration.RouteValidators; + +internal sealed class AuthorizationPolicyValidator(IAuthorizationPolicyProvider authorizationPolicyProvider) + : IRouteValidator +{ + public async ValueTask ValidateAsync(RouteConfig routeConfig, IList errors) + { + var authorizationPolicyName = routeConfig.AuthorizationPolicy; + if (string.IsNullOrEmpty(authorizationPolicyName)) + { + return; + } + + if (string.Equals(AuthorizationConstants.Default, authorizationPolicyName, StringComparison.OrdinalIgnoreCase)) + { + var policy = await authorizationPolicyProvider.GetPolicyAsync(authorizationPolicyName); + if (policy is not null) + { + errors.Add(new ArgumentException($"The application has registered an authorization policy named '{authorizationPolicyName}' that conflicts with the reserved authorization policy name used on this route. The registered policy name needs to be changed for this route to function.")); + } + + return; + } + + if (string.Equals(AuthorizationConstants.Anonymous, authorizationPolicyName, + StringComparison.OrdinalIgnoreCase)) + { + var policy = await authorizationPolicyProvider.GetPolicyAsync(authorizationPolicyName); + if (policy is not null) + { + errors.Add(new ArgumentException( + $"The application has registered an authorization policy named '{authorizationPolicyName}' that conflicts with the reserved authorization policy name used on this route. The registered policy name needs to be changed for this route to function.")); + } + + return; + } + + try + { + var policy = await authorizationPolicyProvider.GetPolicyAsync(authorizationPolicyName); + if (policy is null) + { + errors.Add(new ArgumentException( + $"Authorization policy '{authorizationPolicyName}' not found for route '{routeConfig.RouteId}'.")); + } + } + catch (Exception ex) + { + errors.Add(new ArgumentException( + $"Unable to retrieve the authorization policy '{authorizationPolicyName}' for route '{routeConfig.RouteId}'.", ex)); + } + } +} From 5c2f99fd20b30f51b852a8c3803d6d889ad749f7 Mon Sep 17 00:00:00 2001 From: lbjn Date: Sun, 19 Nov 2023 16:28:17 +0100 Subject: [PATCH 09/14] Move TimeoutPolicy to single class --- .../Configuration/ConfigValidator.cs | 96 +------------------ .../AuthorizationPolicyValidator.cs | 4 +- .../RateLimitPolicyValidator.cs | 54 +++++++++++ .../RouteValidators/TimeoutPolicyValidator.cs | 59 ++++++++++++ .../IReverseProxyBuilderExtensions.cs | 2 + 5 files changed, 121 insertions(+), 94 deletions(-) create mode 100644 src/ReverseProxy/Configuration/RouteValidators/RateLimitPolicyValidator.cs create mode 100644 src/ReverseProxy/Configuration/RouteValidators/TimeoutPolicyValidator.cs diff --git a/src/ReverseProxy/Configuration/ConfigValidator.cs b/src/ReverseProxy/Configuration/ConfigValidator.cs index 5262748d2..76c5617f8 100644 --- a/src/ReverseProxy/Configuration/ConfigValidator.cs +++ b/src/ReverseProxy/Configuration/ConfigValidator.cs @@ -5,15 +5,8 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; -using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Cors.Infrastructure; using Microsoft.AspNetCore.Http; -#if NET8_0_OR_GREATER -using Microsoft.AspNetCore.Http.Timeouts; -#endif -#if NET8_0_OR_GREATER -using Microsoft.Extensions.Options; -#endif using Yarp.ReverseProxy.Configuration.ClusterValidators; using Yarp.ReverseProxy.Configuration.RouteValidators; using Yarp.ReverseProxy.Transforms.Builder; @@ -23,28 +16,17 @@ namespace Yarp.ReverseProxy.Configuration; internal sealed class ConfigValidator : IConfigValidator { private readonly ITransformBuilder _transformBuilder; - private readonly IYarpRateLimiterPolicyProvider _rateLimiterPolicyProvider; private readonly ICorsPolicyProvider _corsPolicyProvider; -#if NET8_0_OR_GREATER - private readonly IOptionsMonitor _timeoutOptions; -#endif + private readonly IEnumerable _routeValidators; private readonly IEnumerable _clusterValidators; public ConfigValidator(ITransformBuilder transformBuilder, - IYarpRateLimiterPolicyProvider rateLimiterPolicyProvider, - ICorsPolicyProvider corsPolicyProvider, -#if NET8_0_OR_GREATER - IOptionsMonitor timeoutOptions, -#endif + ICorsPolicyProvider corsPolicyProvider, IEnumerable routeValidators, IEnumerable clusterValidators) { _transformBuilder = transformBuilder ?? throw new ArgumentNullException(nameof(transformBuilder)); - _rateLimiterPolicyProvider = rateLimiterPolicyProvider ?? throw new ArgumentNullException(nameof(rateLimiterPolicyProvider)); - _corsPolicyProvider = corsPolicyProvider ?? throw new ArgumentNullException(nameof(corsPolicyProvider)); -#if NET8_0_OR_GREATER - _timeoutOptions = timeoutOptions ?? throw new ArgumentNullException(nameof(timeoutOptions)); -#endif + _corsPolicyProvider = corsPolicyProvider ?? throw new ArgumentNullException(nameof(corsPolicyProvider)); _routeValidators = routeValidators; _clusterValidators = clusterValidators; } @@ -61,12 +43,7 @@ public async ValueTask> ValidateRouteAsync(RouteConfig route) } errors.AddRange(_transformBuilder.ValidateRoute(route)); - #if NET7_0_OR_GREATER - await ValidateRateLimiterPolicyAsync(errors, route.RateLimiterPolicy, route.RouteId); -#endif -#if NET8_0_OR_GREATER - ValidateTimeoutPolicy(errors, route.TimeoutPolicy, route.Timeout, route.RouteId); -#endif + await ValidateCorsPolicyAsync(errors, route.CorsPolicy, route.RouteId); if (route.Match is null) @@ -109,71 +86,6 @@ public async ValueTask> ValidateClusterAsync(ClusterConfig clus return errors; } -#if NET8_0_OR_GREATER - private void ValidateTimeoutPolicy(IList errors, string? timeoutPolicyName, TimeSpan? timeout, string routeId) - { - if (!string.IsNullOrEmpty(timeoutPolicyName)) - { - var policies = _timeoutOptions.CurrentValue.Policies; - - if (string.Equals(TimeoutPolicyConstants.Disable, timeoutPolicyName, StringComparison.OrdinalIgnoreCase)) - { - if (policies.TryGetValue(timeoutPolicyName, out var _)) - { - errors.Add(new ArgumentException($"The application has registered a timeout policy named '{timeoutPolicyName}' that conflicts with the reserved timeout policy name used on this route. The registered policy name needs to be changed for this route to function.")); - } - } - else if (!policies.TryGetValue(timeoutPolicyName, out var _)) - { - errors.Add(new ArgumentException($"Timeout policy '{timeoutPolicyName}' not found for route '{routeId}'.")); - } - - if (timeout.HasValue) - { - errors.Add(new ArgumentException($"Route '{routeId}' has both a Timeout '{timeout}' and TimeoutPolicy '{timeoutPolicyName}'.")); - } - } - - if (timeout.HasValue && timeout.Value.TotalMilliseconds <= 0) - { - errors.Add(new ArgumentException($"The Timeout value '{timeout.Value}' is invalid for route '{routeId}'. The Timeout must be greater than zero milliseconds.")); - } - } -#endif - private async ValueTask ValidateRateLimiterPolicyAsync(IList errors, string? rateLimiterPolicyName, string routeId) - { - if (string.IsNullOrEmpty(rateLimiterPolicyName)) - { - return; - } - - if (string.Equals(RateLimitingConstants.Default, rateLimiterPolicyName, StringComparison.OrdinalIgnoreCase) - || string.Equals(RateLimitingConstants.Disable, rateLimiterPolicyName, StringComparison.OrdinalIgnoreCase)) - { - var policy = await _rateLimiterPolicyProvider.GetPolicyAsync(rateLimiterPolicyName); - if (policy is not null) - { - // We weren't expecting to find a policy with these names. - errors.Add(new ArgumentException($"The application has registered a RateLimiter policy named '{rateLimiterPolicyName}' that conflicts with the reserved RateLimiter policy name used on this route. The registered policy name needs to be changed for this route to function.")); - } - return; - } - - try - { - var policy = await _rateLimiterPolicyProvider.GetPolicyAsync(rateLimiterPolicyName); - - if (policy is null) - { - errors.Add(new ArgumentException($"RateLimiter policy '{rateLimiterPolicyName}' not found for route '{routeId}'.")); - } - } - catch (Exception ex) - { - errors.Add(new ArgumentException($"Unable to retrieve the RateLimiter policy '{rateLimiterPolicyName}' for route '{routeId}'.", ex)); - } - } - private async ValueTask ValidateCorsPolicyAsync(IList errors, string? corsPolicyName, string routeId) { if (string.IsNullOrEmpty(corsPolicyName)) diff --git a/src/ReverseProxy/Configuration/RouteValidators/AuthorizationPolicyValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/AuthorizationPolicyValidator.cs index c1f16dd91..2e34e4cf5 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/AuthorizationPolicyValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/AuthorizationPolicyValidator.cs @@ -5,8 +5,8 @@ namespace Yarp.ReverseProxy.Configuration.RouteValidators; -internal sealed class AuthorizationPolicyValidator(IAuthorizationPolicyProvider authorizationPolicyProvider) - : IRouteValidator +internal sealed class AuthorizationPolicyValidator + (IAuthorizationPolicyProvider authorizationPolicyProvider) : IRouteValidator { public async ValueTask ValidateAsync(RouteConfig routeConfig, IList errors) { diff --git a/src/ReverseProxy/Configuration/RouteValidators/RateLimitPolicyValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/RateLimitPolicyValidator.cs new file mode 100644 index 000000000..8d1e2f8f5 --- /dev/null +++ b/src/ReverseProxy/Configuration/RouteValidators/RateLimitPolicyValidator.cs @@ -0,0 +1,54 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; + +namespace Yarp.ReverseProxy.Configuration.RouteValidators; + +internal sealed class RateLimitPolicyValidator + (IYarpRateLimiterPolicyProvider rateLimiterPolicyProvider) : IRouteValidator +{ +#if NET7_0_OR_GREATER + public async ValueTask ValidateAsync(RouteConfig routeConfig, IList errors) + { + var rateLimiterPolicyName = routeConfig.RateLimiterPolicy; + + if (string.IsNullOrEmpty(rateLimiterPolicyName)) + { + return; + } + + if (string.Equals(RateLimitingConstants.Default, rateLimiterPolicyName, StringComparison.OrdinalIgnoreCase) + || string.Equals(RateLimitingConstants.Disable, rateLimiterPolicyName, StringComparison.OrdinalIgnoreCase)) + { + var policy = await rateLimiterPolicyProvider.GetPolicyAsync(rateLimiterPolicyName); + if (policy is not null) + { + // We weren't expecting to find a policy with these names. + errors.Add(new ArgumentException( + $"The application has registered a RateLimiter policy named '{rateLimiterPolicyName}' that conflicts with the reserved RateLimiter policy name used on this route. The registered policy name needs to be changed for this route to function.")); + } + + return; + } + + try + { + var policy = await rateLimiterPolicyProvider.GetPolicyAsync(rateLimiterPolicyName); + + if (policy is null) + { + errors.Add(new ArgumentException( + $"RateLimiter policy '{rateLimiterPolicyName}' not found for route '{routeConfig.RouteId}'.")); + } + } + catch (Exception ex) + { + errors.Add(new ArgumentException( + $"Unable to retrieve the RateLimiter policy '{rateLimiterPolicyName}' for route '{routeConfig.RouteId}'.", + ex)); + } + } +#else + public ValueTask ValidateAsync(RouteConfig routeConfig, IList errors) => ValueTask.CompletedTask; +#endif +} diff --git a/src/ReverseProxy/Configuration/RouteValidators/TimeoutPolicyValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/TimeoutPolicyValidator.cs new file mode 100644 index 000000000..a57131f35 --- /dev/null +++ b/src/ReverseProxy/Configuration/RouteValidators/TimeoutPolicyValidator.cs @@ -0,0 +1,59 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +#if NET8_0_OR_GREATER +using Microsoft.AspNetCore.Http.Timeouts; +#endif +using Microsoft.Extensions.Options; + +namespace Yarp.ReverseProxy.Configuration.RouteValidators; + +internal sealed class TimeoutPolicyValidator : IRouteValidator +{ +#if NET8_0_OR_GREATER + private readonly IOptionsMonitor _timeoutOptions; + + public TimeoutPolicyValidator(IOptionsMonitor timeoutOptions) + { + _timeoutOptions = timeoutOptions; + } + + public ValueTask ValidateAsync(RouteConfig routeConfig, IList errors) + { + var timeoutPolicyName = routeConfig.TimeoutPolicy; + var timeout = routeConfig.Timeout; + + if (!string.IsNullOrEmpty(timeoutPolicyName)) + { + var policies = _timeoutOptions.CurrentValue.Policies; + + if (string.Equals(TimeoutPolicyConstants.Disable, timeoutPolicyName, StringComparison.OrdinalIgnoreCase)) + { + if (policies.TryGetValue(timeoutPolicyName, out var _)) + { + errors.Add(new ArgumentException($"The application has registered a timeout policy named '{timeoutPolicyName}' that conflicts with the reserved timeout policy name used on this route. The registered policy name needs to be changed for this route to function.")); + } + } + else if (!policies.TryGetValue(timeoutPolicyName, out var _)) + { + errors.Add(new ArgumentException($"Timeout policy '{timeoutPolicyName}' not found for route '{routeConfig.RouteId}'.")); + } + + if (timeout.HasValue) + { + errors.Add(new ArgumentException($"Route '{routeConfig.RouteId}' has both a Timeout '{timeout}' and TimeoutPolicy '{timeoutPolicyName}'.")); + } + } + + if (timeout.HasValue && timeout.Value.TotalMilliseconds <= 0) + { + errors.Add(new ArgumentException($"The Timeout value '{timeout.Value}' is invalid for route '{routeConfig.RouteId}'. The Timeout must be greater than zero milliseconds.")); + } + + return ValueTask.CompletedTask; + } + +#else + public ValueTask ValidateAsync(RouteConfig routeConfig, IList errors) => ValueTask.CompletedTask; +#endif +} diff --git a/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs b/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs index f36a4d014..cf7da9195 100644 --- a/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs +++ b/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs @@ -28,6 +28,8 @@ public static IReverseProxyBuilder AddConfigBuilder(this IReverseProxyBuilder bu builder.Services.TryAddSingleton(); builder.Services.TryAddSingleton(); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); From 1866e947ea1a3f729aa3dccce44bd980f1c8f52d Mon Sep 17 00:00:00 2001 From: lbjn Date: Sun, 19 Nov 2023 16:35:15 +0100 Subject: [PATCH 10/14] Move CorsPolicy to single class --- .../Configuration/ConfigValidator.cs | 52 -------------- .../RouteValidators/CorsPolicyValidator.cs | 68 +++++++++++++++++++ .../IReverseProxyBuilderExtensions.cs | 1 + 3 files changed, 69 insertions(+), 52 deletions(-) create mode 100644 src/ReverseProxy/Configuration/RouteValidators/CorsPolicyValidator.cs diff --git a/src/ReverseProxy/Configuration/ConfigValidator.cs b/src/ReverseProxy/Configuration/ConfigValidator.cs index 76c5617f8..b483c65df 100644 --- a/src/ReverseProxy/Configuration/ConfigValidator.cs +++ b/src/ReverseProxy/Configuration/ConfigValidator.cs @@ -5,8 +5,6 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; -using Microsoft.AspNetCore.Cors.Infrastructure; -using Microsoft.AspNetCore.Http; using Yarp.ReverseProxy.Configuration.ClusterValidators; using Yarp.ReverseProxy.Configuration.RouteValidators; using Yarp.ReverseProxy.Transforms.Builder; @@ -16,17 +14,13 @@ namespace Yarp.ReverseProxy.Configuration; internal sealed class ConfigValidator : IConfigValidator { private readonly ITransformBuilder _transformBuilder; - private readonly ICorsPolicyProvider _corsPolicyProvider; - private readonly IEnumerable _routeValidators; private readonly IEnumerable _clusterValidators; public ConfigValidator(ITransformBuilder transformBuilder, - ICorsPolicyProvider corsPolicyProvider, IEnumerable routeValidators, IEnumerable clusterValidators) { _transformBuilder = transformBuilder ?? throw new ArgumentNullException(nameof(transformBuilder)); - _corsPolicyProvider = corsPolicyProvider ?? throw new ArgumentNullException(nameof(corsPolicyProvider)); _routeValidators = routeValidators; _clusterValidators = clusterValidators; } @@ -44,8 +38,6 @@ public async ValueTask> ValidateRouteAsync(RouteConfig route) errors.AddRange(_transformBuilder.ValidateRoute(route)); - await ValidateCorsPolicyAsync(errors, route.CorsPolicy, route.RouteId); - if (route.Match is null) { errors.Add(new ArgumentException($"Route '{route.RouteId}' did not set any match criteria, it requires Hosts or Path specified. Set the Path to '/{{**catchall}}' to match all requests.")); @@ -85,48 +77,4 @@ public async ValueTask> ValidateClusterAsync(ClusterConfig clus return errors; } - - private async ValueTask ValidateCorsPolicyAsync(IList errors, string? corsPolicyName, string routeId) - { - if (string.IsNullOrEmpty(corsPolicyName)) - { - return; - } - - if (string.Equals(CorsConstants.Default, corsPolicyName, StringComparison.OrdinalIgnoreCase)) - { - var dummyHttpContext = new DefaultHttpContext(); - var policy = await _corsPolicyProvider.GetPolicyAsync(dummyHttpContext, corsPolicyName); - if (policy is not null) - { - errors.Add(new ArgumentException($"The application has registered a CORS policy named '{corsPolicyName}' that conflicts with the reserved CORS policy name used on this route. The registered policy name needs to be changed for this route to function.")); - } - return; - } - - if (string.Equals(CorsConstants.Disable, corsPolicyName, StringComparison.OrdinalIgnoreCase)) - { - var dummyHttpContext = new DefaultHttpContext(); - var policy = await _corsPolicyProvider.GetPolicyAsync(dummyHttpContext, corsPolicyName); - if (policy is not null) - { - errors.Add(new ArgumentException($"The application has registered a CORS policy named '{corsPolicyName}' that conflicts with the reserved CORS policy name used on this route. The registered policy name needs to be changed for this route to function.")); - } - return; - } - - try - { - var dummyHttpContext = new DefaultHttpContext(); - var policy = await _corsPolicyProvider.GetPolicyAsync(dummyHttpContext, corsPolicyName); - if (policy is null) - { - errors.Add(new ArgumentException($"CORS policy '{corsPolicyName}' not found for route '{routeId}'.")); - } - } - catch (Exception ex) - { - errors.Add(new ArgumentException($"Unable to retrieve the CORS policy '{corsPolicyName}' for route '{routeId}'.", ex)); - } - } } diff --git a/src/ReverseProxy/Configuration/RouteValidators/CorsPolicyValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/CorsPolicyValidator.cs new file mode 100644 index 000000000..4168c5bdb --- /dev/null +++ b/src/ReverseProxy/Configuration/RouteValidators/CorsPolicyValidator.cs @@ -0,0 +1,68 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Cors.Infrastructure; +using Microsoft.AspNetCore.Http; + +namespace Yarp.ReverseProxy.Configuration.RouteValidators; + +internal sealed class CorsPolicyValidator : IRouteValidator +{ + private readonly ICorsPolicyProvider _corsPolicyProvider; + + public CorsPolicyValidator(ICorsPolicyProvider corsPolicyProvider) + { + _corsPolicyProvider = corsPolicyProvider; + } + + public async ValueTask ValidateAsync(RouteConfig routeConfig, IList errors) + { + var corsPolicyName = routeConfig.CorsPolicy; + if (string.IsNullOrEmpty(corsPolicyName)) + { + return; + } + + if (string.Equals(CorsConstants.Default, corsPolicyName, StringComparison.OrdinalIgnoreCase)) + { + var dummyHttpContext = new DefaultHttpContext(); + var policy = await _corsPolicyProvider.GetPolicyAsync(dummyHttpContext, corsPolicyName); + if (policy is not null) + { + errors.Add(new ArgumentException( + $"The application has registered a CORS policy named '{corsPolicyName}' that conflicts with the reserved CORS policy name used on this route. The registered policy name needs to be changed for this route to function.")); + } + + return; + } + + if (string.Equals(CorsConstants.Disable, corsPolicyName, StringComparison.OrdinalIgnoreCase)) + { + var dummyHttpContext = new DefaultHttpContext(); + var policy = await _corsPolicyProvider.GetPolicyAsync(dummyHttpContext, corsPolicyName); + if (policy is not null) + { + errors.Add(new ArgumentException( + $"The application has registered a CORS policy named '{corsPolicyName}' that conflicts with the reserved CORS policy name used on this route. The registered policy name needs to be changed for this route to function.")); + } + + return; + } + + try + { + var dummyHttpContext = new DefaultHttpContext(); + var policy = await _corsPolicyProvider.GetPolicyAsync(dummyHttpContext, corsPolicyName); + if (policy is null) + { + errors.Add(new ArgumentException( + $"CORS policy '{corsPolicyName}' not found for route '{routeConfig.RouteId}'.")); + } + } + catch (Exception ex) + { + errors.Add(new ArgumentException( + $"Unable to retrieve the CORS policy '{corsPolicyName}' for route '{routeConfig.RouteId}'.", ex)); + } + } +} diff --git a/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs b/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs index cf7da9195..22539fcd9 100644 --- a/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs +++ b/src/ReverseProxy/Management/IReverseProxyBuilderExtensions.cs @@ -30,6 +30,7 @@ public static IReverseProxyBuilder AddConfigBuilder(this IReverseProxyBuilder bu builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); + builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton()); From 045222dad9b94fedd8dc40fbbdbd8490c1437bee Mon Sep 17 00:00:00 2001 From: lbjn Date: Sun, 19 Nov 2023 16:39:15 +0100 Subject: [PATCH 11/14] build fix --- .../RouteValidators/RateLimitPolicyValidator.cs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/ReverseProxy/Configuration/RouteValidators/RateLimitPolicyValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/RateLimitPolicyValidator.cs index 8d1e2f8f5..fd8306cfa 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/RateLimitPolicyValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/RateLimitPolicyValidator.cs @@ -4,10 +4,15 @@ namespace Yarp.ReverseProxy.Configuration.RouteValidators; -internal sealed class RateLimitPolicyValidator - (IYarpRateLimiterPolicyProvider rateLimiterPolicyProvider) : IRouteValidator +internal sealed class RateLimitPolicyValidator : IRouteValidator { #if NET7_0_OR_GREATER + private readonly IYarpRateLimiterPolicyProvider _rateLimiterPolicyProvider; + public RateLimitPolicyValidator(IYarpRateLimiterPolicyProvider rateLimiterPolicyProvider) + { + _rateLimiterPolicyProvider = rateLimiterPolicyProvider; + } + public async ValueTask ValidateAsync(RouteConfig routeConfig, IList errors) { var rateLimiterPolicyName = routeConfig.RateLimiterPolicy; @@ -20,7 +25,7 @@ public async ValueTask ValidateAsync(RouteConfig routeConfig, IList e if (string.Equals(RateLimitingConstants.Default, rateLimiterPolicyName, StringComparison.OrdinalIgnoreCase) || string.Equals(RateLimitingConstants.Disable, rateLimiterPolicyName, StringComparison.OrdinalIgnoreCase)) { - var policy = await rateLimiterPolicyProvider.GetPolicyAsync(rateLimiterPolicyName); + var policy = await _rateLimiterPolicyProvider.GetPolicyAsync(rateLimiterPolicyName); if (policy is not null) { // We weren't expecting to find a policy with these names. @@ -33,7 +38,7 @@ public async ValueTask ValidateAsync(RouteConfig routeConfig, IList e try { - var policy = await rateLimiterPolicyProvider.GetPolicyAsync(rateLimiterPolicyName); + var policy = await _rateLimiterPolicyProvider.GetPolicyAsync(rateLimiterPolicyName); if (policy is null) { From 660b5ea5750db16ea61469192b30e5d2e59ee7f0 Mon Sep 17 00:00:00 2001 From: lbjn Date: Mon, 20 Nov 2023 21:05:33 +0100 Subject: [PATCH 12/14] Ensure enumeration --- src/ReverseProxy/Configuration/ConfigValidator.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ReverseProxy/Configuration/ConfigValidator.cs b/src/ReverseProxy/Configuration/ConfigValidator.cs index b483c65df..a6b7b0bc0 100644 --- a/src/ReverseProxy/Configuration/ConfigValidator.cs +++ b/src/ReverseProxy/Configuration/ConfigValidator.cs @@ -14,15 +14,15 @@ namespace Yarp.ReverseProxy.Configuration; internal sealed class ConfigValidator : IConfigValidator { private readonly ITransformBuilder _transformBuilder; - private readonly IEnumerable _routeValidators; - private readonly IEnumerable _clusterValidators; + private readonly IRouteValidator[] _routeValidators; + private readonly IClusterValidator[] _clusterValidators; public ConfigValidator(ITransformBuilder transformBuilder, IEnumerable routeValidators, IEnumerable clusterValidators) { _transformBuilder = transformBuilder ?? throw new ArgumentNullException(nameof(transformBuilder)); - _routeValidators = routeValidators; - _clusterValidators = clusterValidators; + _routeValidators = routeValidators?.ToArray() ?? throw new ArgumentNullException(nameof(routeValidators)); + _clusterValidators = clusterValidators?.ToArray() ?? throw new ArgumentNullException(nameof(clusterValidators)); } // Note this performs all validation steps without short circuiting in order to report all possible errors. From a33b35b25227a6f569e306124d0d941bdca6ff93 Mon Sep 17 00:00:00 2001 From: lbjn Date: Mon, 20 Nov 2023 22:22:55 +0100 Subject: [PATCH 13/14] Review comments --- .../ClusterValidators/HealthCheckValidator.cs | 5 +++-- .../ClusterValidators/IClusterValidator.cs | 3 +++ .../ProxyHttpRequestValidator.cs | 11 ++--------- src/ReverseProxy/Configuration/ConfigValidator.cs | 1 + .../RouteValidators/CorsPolicyValidator.cs | 15 ++++----------- .../RouteValidators/IRouteValidator.cs | 3 +++ 6 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs index ec2247ea4..2ff17945f 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs @@ -12,9 +12,10 @@ internal sealed class HealthCheckValidator : IClusterValidator private readonly FrozenDictionary _availableDestinationsPolicies; private readonly FrozenDictionary _activeHealthCheckPolicies; private readonly FrozenDictionary _passiveHealthCheckPolicies; + public HealthCheckValidator(IEnumerable availableDestinationsPolicies, - IEnumerable activeHealthCheckPolicies, - IEnumerable passiveHealthCheckPolicies) + IEnumerable activeHealthCheckPolicies, + IEnumerable passiveHealthCheckPolicies) { _availableDestinationsPolicies = availableDestinationsPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(availableDestinationsPolicies)); _activeHealthCheckPolicies = activeHealthCheckPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(availableDestinationsPolicies)); diff --git a/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs index d9a95c800..00fd7ef0d 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs @@ -4,6 +4,9 @@ namespace Yarp.ReverseProxy.Configuration.ClusterValidators; +/// +/// Provides method to validate cluster configuration. +/// public interface IClusterValidator { public ValueTask ValidateAsync(ClusterConfig cluster, IList errors); diff --git a/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs index 8a7d64804..3ea737b41 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs @@ -6,15 +6,8 @@ namespace Yarp.ReverseProxy.Configuration.ClusterValidators; -internal sealed class ProxyHttpRequestValidator : IClusterValidator +internal sealed class ProxyHttpRequestValidator(ILogger logger) : IClusterValidator { - private readonly ILogger _logger; - - public ProxyHttpRequestValidator(ILogger logger) - { - _logger = logger; - } - public ValueTask ValidateAsync(ClusterConfig cluster, IList errors) { if (cluster.HttpRequest is null) @@ -34,7 +27,7 @@ public ValueTask ValidateAsync(ClusterConfig cluster, IList errors) if (cluster.HttpRequest.Version == HttpVersion.Version10) { - Log.Http10Version(_logger); + Log.Http10Version(logger); } return ValueTask.CompletedTask; diff --git a/src/ReverseProxy/Configuration/ConfigValidator.cs b/src/ReverseProxy/Configuration/ConfigValidator.cs index a6b7b0bc0..8c35bbac9 100644 --- a/src/ReverseProxy/Configuration/ConfigValidator.cs +++ b/src/ReverseProxy/Configuration/ConfigValidator.cs @@ -16,6 +16,7 @@ internal sealed class ConfigValidator : IConfigValidator private readonly ITransformBuilder _transformBuilder; private readonly IRouteValidator[] _routeValidators; private readonly IClusterValidator[] _clusterValidators; + public ConfigValidator(ITransformBuilder transformBuilder, IEnumerable routeValidators, IEnumerable clusterValidators) diff --git a/src/ReverseProxy/Configuration/RouteValidators/CorsPolicyValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/CorsPolicyValidator.cs index 4168c5bdb..288cff809 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/CorsPolicyValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/CorsPolicyValidator.cs @@ -6,15 +6,8 @@ namespace Yarp.ReverseProxy.Configuration.RouteValidators; -internal sealed class CorsPolicyValidator : IRouteValidator +internal sealed class CorsPolicyValidator(ICorsPolicyProvider corsPolicyProvider) : IRouteValidator { - private readonly ICorsPolicyProvider _corsPolicyProvider; - - public CorsPolicyValidator(ICorsPolicyProvider corsPolicyProvider) - { - _corsPolicyProvider = corsPolicyProvider; - } - public async ValueTask ValidateAsync(RouteConfig routeConfig, IList errors) { var corsPolicyName = routeConfig.CorsPolicy; @@ -26,7 +19,7 @@ public async ValueTask ValidateAsync(RouteConfig routeConfig, IList e if (string.Equals(CorsConstants.Default, corsPolicyName, StringComparison.OrdinalIgnoreCase)) { var dummyHttpContext = new DefaultHttpContext(); - var policy = await _corsPolicyProvider.GetPolicyAsync(dummyHttpContext, corsPolicyName); + var policy = await corsPolicyProvider.GetPolicyAsync(dummyHttpContext, corsPolicyName); if (policy is not null) { errors.Add(new ArgumentException( @@ -39,7 +32,7 @@ public async ValueTask ValidateAsync(RouteConfig routeConfig, IList e if (string.Equals(CorsConstants.Disable, corsPolicyName, StringComparison.OrdinalIgnoreCase)) { var dummyHttpContext = new DefaultHttpContext(); - var policy = await _corsPolicyProvider.GetPolicyAsync(dummyHttpContext, corsPolicyName); + var policy = await corsPolicyProvider.GetPolicyAsync(dummyHttpContext, corsPolicyName); if (policy is not null) { errors.Add(new ArgumentException( @@ -52,7 +45,7 @@ public async ValueTask ValidateAsync(RouteConfig routeConfig, IList e try { var dummyHttpContext = new DefaultHttpContext(); - var policy = await _corsPolicyProvider.GetPolicyAsync(dummyHttpContext, corsPolicyName); + var policy = await corsPolicyProvider.GetPolicyAsync(dummyHttpContext, corsPolicyName); if (policy is null) { errors.Add(new ArgumentException( diff --git a/src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs index f6b3f8015..a70193977 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs @@ -4,6 +4,9 @@ namespace Yarp.ReverseProxy.Configuration.RouteValidators; +/// +/// Provides method to validate route configuration. +/// public interface IRouteValidator { public ValueTask ValidateAsync(RouteConfig routeConfig, IList errors); From aea419ec7e4af24a5a58e98e0148a5a1c025d457 Mon Sep 17 00:00:00 2001 From: lbjn Date: Wed, 22 Nov 2023 20:29:20 +0100 Subject: [PATCH 14/14] Comments for public methods --- .../Configuration/ClusterValidators/IClusterValidator.cs | 7 +++++++ .../Configuration/RouteValidators/IRouteValidator.cs | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs b/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs index 00fd7ef0d..d94f1dda0 100644 --- a/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs +++ b/src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs @@ -9,5 +9,12 @@ namespace Yarp.ReverseProxy.Configuration.ClusterValidators; /// public interface IClusterValidator { + + /// + /// Perform validation on a cluster configuration by adding exceptions to the provided collection. + /// + /// Cluster configuration to validate + /// Collection of all validation exceptions + /// A ValueTask representing the asynchronous validation operation. public ValueTask ValidateAsync(ClusterConfig cluster, IList errors); } diff --git a/src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs b/src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs index a70193977..aac53e356 100644 --- a/src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs +++ b/src/ReverseProxy/Configuration/RouteValidators/IRouteValidator.cs @@ -9,5 +9,11 @@ namespace Yarp.ReverseProxy.Configuration.RouteValidators; /// public interface IRouteValidator { + /// + /// Perform validation on a route by adding exceptions to the provided collection. + /// + /// Route configuration to validate + /// Collection of all validation exceptions + /// A ValueTask representing the asynchronous validation operation. public ValueTask ValidateAsync(RouteConfig routeConfig, IList errors); }