-
Notifications
You must be signed in to change notification settings - Fork 881
Refactor Configvalidator #2293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Refactor Configvalidator #2293
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
2a19fc7
refactor ConfigValidator
larsbj1988 a910dcf
fix review comments
larsbj1988 6757a3f
merge
larsbj1988 f5c5ce9
Merge branch 'main' into cleanup
larsbj1988 b8d1b55
Update src/ReverseProxy/Configuration/ConfigValidator.cs
larsbj1988 2baec7f
Update src/ReverseProxy/Configuration/ClusterValidators/IClusterValid…
larsbj1988 85a8f3e
Merge remote-tracking branch 'origin/cleanup' into cleanup
larsbj1988 21d77f9
Convert to valuetask return type
larsbj1988 6baba60
revert
larsbj1988 b0a6ff5
use routeConfig as parameter
larsbj1988 640ed28
Move AuthorizationPolicyValidation to class
larsbj1988 5c2f99f
Move TimeoutPolicy to single class
larsbj1988 1866e94
Move CorsPolicy to single class
larsbj1988 045222d
build fix
larsbj1988 660b5ea
Ensure enumeration
larsbj1988 5aebc9f
Merge branch 'microsoft:main' into cleanup
larsbj1988 a33b35b
Review comments
larsbj1988 aea419e
Comments for public methods
larsbj1988 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
26 changes: 26 additions & 0 deletions
26
src/ReverseProxy/Configuration/ClusterValidators/DestinationValidator.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Threading.Tasks; | ||
|
||
namespace Yarp.ReverseProxy.Configuration.ClusterValidators; | ||
|
||
internal sealed class DestinationValidator : IClusterValidator | ||
{ | ||
public ValueTask ValidateAsync(ClusterConfig cluster, IList<Exception> errors) | ||
{ | ||
if (cluster.Destinations is null) | ||
{ | ||
return ValueTask.CompletedTask; | ||
} | ||
|
||
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 ValueTask.CompletedTask; | ||
} | ||
} |
101 changes: 101 additions & 0 deletions
101
src/ReverseProxy/Configuration/ClusterValidators/HealthCheckValidator.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
using System; | ||
using System.Collections.Frozen; | ||
using System.Collections.Generic; | ||
using System.Threading.Tasks; | ||
using Yarp.ReverseProxy.Health; | ||
using Yarp.ReverseProxy.Utilities; | ||
|
||
namespace Yarp.ReverseProxy.Configuration.ClusterValidators; | ||
|
||
internal sealed class HealthCheckValidator : IClusterValidator | ||
{ | ||
private readonly FrozenDictionary<string, IAvailableDestinationsPolicy> _availableDestinationsPolicies; | ||
private readonly FrozenDictionary<string, IActiveHealthCheckPolicy> _activeHealthCheckPolicies; | ||
private readonly FrozenDictionary<string, IPassiveHealthCheckPolicy> _passiveHealthCheckPolicies; | ||
|
||
public HealthCheckValidator(IEnumerable<IAvailableDestinationsPolicy> availableDestinationsPolicies, | ||
IEnumerable<IActiveHealthCheckPolicy> activeHealthCheckPolicies, | ||
IEnumerable<IPassiveHealthCheckPolicy> 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 ValueTask ValidateAsync(ClusterConfig cluster, IList<Exception> errors) | ||
{ | ||
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(cluster, errors); | ||
ValidatePassiveHealthCheck(cluster, errors); | ||
|
||
return ValueTask.CompletedTask; | ||
} | ||
|
||
private void ValidateActiveHealthCheck(ClusterConfig cluster, IList<Exception> errors) | ||
{ | ||
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(ClusterConfig cluster, IList<Exception> errors) | ||
{ | ||
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.")); | ||
} | ||
} | ||
} |
20 changes: 20 additions & 0 deletions
20
src/ReverseProxy/Configuration/ClusterValidators/IClusterValidator.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Threading.Tasks; | ||
|
||
namespace Yarp.ReverseProxy.Configuration.ClusterValidators; | ||
|
||
/// <summary> | ||
/// Provides method to validate cluster configuration. | ||
/// </summary> | ||
public interface IClusterValidator | ||
{ | ||
|
||
/// <summary> | ||
/// Perform validation on a cluster configuration by adding exceptions to the provided collection. | ||
/// </summary> | ||
/// <param name="cluster">Cluster configuration to validate</param> | ||
/// <param name="errors">Collection of all validation exceptions</param> | ||
/// <returns>A ValueTask representing the asynchronous validation operation.</returns> | ||
public ValueTask ValidateAsync(ClusterConfig cluster, IList<Exception> errors); | ||
larsbj1988 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
34 changes: 34 additions & 0 deletions
34
src/ReverseProxy/Configuration/ClusterValidators/LoadBalancingValidator.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
using System; | ||
using System.Collections.Frozen; | ||
using System.Collections.Generic; | ||
using System.Threading.Tasks; | ||
using Yarp.ReverseProxy.LoadBalancing; | ||
using Yarp.ReverseProxy.Utilities; | ||
|
||
namespace Yarp.ReverseProxy.Configuration.ClusterValidators; | ||
|
||
internal sealed class LoadBalancingValidator : IClusterValidator | ||
{ | ||
private readonly FrozenDictionary<string, ILoadBalancingPolicy> _loadBalancingPolicies; | ||
public LoadBalancingValidator(IEnumerable<ILoadBalancingPolicy> loadBalancingPolicies) | ||
{ | ||
_loadBalancingPolicies = loadBalancingPolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(loadBalancingPolicies)); | ||
} | ||
|
||
public ValueTask ValidateAsync(ClusterConfig cluster, IList<Exception> errors) | ||
{ | ||
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}'.")); | ||
} | ||
|
||
return ValueTask.CompletedTask; | ||
} | ||
} |
53 changes: 53 additions & 0 deletions
53
src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpClientValidator.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Text; | ||
using System.Threading.Tasks; | ||
|
||
namespace Yarp.ReverseProxy.Configuration.ClusterValidators; | ||
|
||
internal sealed class ProxyHttpClientValidator : IClusterValidator | ||
{ | ||
public ValueTask ValidateAsync(ClusterConfig cluster, IList<Exception> errors) | ||
{ | ||
if (cluster.HttpClient is null) | ||
{ | ||
// Proxy http client options are not set. | ||
return ValueTask.CompletedTask; | ||
} | ||
|
||
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 null) | ||
{ | ||
return ValueTask.CompletedTask; | ||
} | ||
|
||
try | ||
{ | ||
Encoding.GetEncoding(responseHeaderEncoding); | ||
} | ||
catch (ArgumentException aex) | ||
{ | ||
errors.Add(new ArgumentException($"Invalid response header encoding '{responseHeaderEncoding}'.", aex)); | ||
} | ||
|
||
return ValueTask.CompletedTask; | ||
} | ||
} |
48 changes: 48 additions & 0 deletions
48
src/ReverseProxy/Configuration/ClusterValidators/ProxyHttpRequestValidator.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Net; | ||
using System.Threading.Tasks; | ||
using Microsoft.Extensions.Logging; | ||
|
||
namespace Yarp.ReverseProxy.Configuration.ClusterValidators; | ||
|
||
internal sealed class ProxyHttpRequestValidator(ILogger<ConfigValidator> logger) : IClusterValidator | ||
{ | ||
public ValueTask ValidateAsync(ClusterConfig cluster, IList<Exception> errors) | ||
{ | ||
if (cluster.HttpRequest is null) | ||
{ | ||
// Proxy http request options are not set. | ||
return ValueTask.CompletedTask; | ||
} | ||
|
||
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 ValueTask.CompletedTask; | ||
} | ||
|
||
private static class Log | ||
{ | ||
private static readonly Action<ILogger, Exception?> _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); | ||
} | ||
} | ||
} |
64 changes: 64 additions & 0 deletions
64
src/ReverseProxy/Configuration/ClusterValidators/SessionAffinityValidator.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
using System; | ||
using System.Collections.Frozen; | ||
using System.Collections.Generic; | ||
using System.Threading.Tasks; | ||
using Yarp.ReverseProxy.SessionAffinity; | ||
using Yarp.ReverseProxy.Utilities; | ||
|
||
namespace Yarp.ReverseProxy.Configuration.ClusterValidators; | ||
|
||
internal sealed class SessionAffinityValidator : IClusterValidator | ||
{ | ||
private readonly FrozenDictionary<string, IAffinityFailurePolicy> _affinityFailurePolicies; | ||
|
||
public SessionAffinityValidator(IEnumerable<IAffinityFailurePolicy> affinityFailurePolicies) | ||
{ | ||
_affinityFailurePolicies = affinityFailurePolicies?.ToDictionaryByUniqueId(p => p.Name) ?? throw new ArgumentNullException(nameof(affinityFailurePolicies)); | ||
} | ||
|
||
public ValueTask ValidateAsync(ClusterConfig cluster, IList<Exception> errors) | ||
{ | ||
if (!(cluster.SessionAffinity?.Enabled ?? false)) | ||
{ | ||
// Session affinity is disabled | ||
return ValueTask.CompletedTask; | ||
} | ||
|
||
// 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 ValueTask.CompletedTask; | ||
} | ||
|
||
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 ValueTask.CompletedTask; | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.