From 48e3f7eaaaed20513d316bc44faa8bfa1209ff28 Mon Sep 17 00:00:00 2001 From: Chris Ross Date: Tue, 14 Nov 2023 11:17:36 -0800 Subject: [PATCH] Timeout integration (#2307) --- docs/docfx/articles/timeouts.md | 75 ++++++++++ docs/docfx/articles/toc.yml | 2 + docs/docfx/articles/websockets.md | 4 + .../ReverseProxy.Minimal.Sample/Program.cs | 11 +- .../Converters/YarpIngressOptions.cs | 3 + .../Converters/YarpParser.cs | 4 + .../ConfigurationConfigProvider.cs | 4 + .../Configuration/ConfigValidator.cs | 49 ++++++- src/ReverseProxy/Configuration/RouteConfig.cs | 25 ++++ .../Configuration/TimeoutPolicyConstants.cs | 9 ++ src/ReverseProxy/Forwarder/HttpForwarder.cs | 7 + .../ProxyPipelineInitializerMiddleware.cs | 30 +++- .../Routing/ProxyEndpointFactory.cs | 22 +++ src/ReverseProxy/Utilities/EventIds.cs | 1 + .../ConfigurationConfigProviderTests.cs | 6 + .../Configuration/ConfigValidatorTests.cs | 138 ++++++++++++++++++ .../Configuration/RouteConfigTests.cs | 22 +++ ...ProxyPipelineInitializerMiddlewareTests.cs | 35 ++++- .../Routing/ProxyEndpointFactoryTests.cs | 101 +++++++++++++ .../ReverseProxy.Code.csproj | 2 +- testassets/ReverseProxy.Code/Startup.cs | 21 ++- .../TestServer/Controllers/HttpController.cs | 12 ++ 22 files changed, 575 insertions(+), 8 deletions(-) create mode 100644 docs/docfx/articles/timeouts.md create mode 100644 src/ReverseProxy/Configuration/TimeoutPolicyConstants.cs diff --git a/docs/docfx/articles/timeouts.md b/docs/docfx/articles/timeouts.md new file mode 100644 index 000000000..df7b504c8 --- /dev/null +++ b/docs/docfx/articles/timeouts.md @@ -0,0 +1,75 @@ +# Request Timeouts + +## Introduction + +.NET 8 introduced the [Request Timeouts Middleware](https://learn.microsoft.com/aspnet/core/performance/timeouts) to enable configuring request timeouts globally as well as per endpoint. This functionality is also available in YARP 2.1 when running on .NET 8. + +## Defaults +Requests do not have any timeouts by default, other than the [Activity Timeout](http-client-config.md#HttpRequest) used to clean up idle requests. A default policy specified in [RequestTimeoutOptions](https://learn.microsoft.com/dotnet/api/microsoft.aspnetcore.http.timeouts.requesttimeoutoptions) will apply to proxied requests as well. + +## Configuration +Timeouts and Timeout Policies can be specified per route via [RouteConfig](xref:Yarp.ReverseProxy.Configuration.RouteConfig) and can be bound from the `Routes` sections of the config file. As with other route properties, this can be modified and reloaded without restarting the proxy. Policy names are case insensitive. + +Timeouts are specified in a TimeSpan HH:MM:SS format. Specifying both Timeout and TimeoutPolicy on the same route is invalid and will cause the configuration to be rejected. + +Example: +```JSON +{ + "ReverseProxy": { + "Routes": { + "route1" : { + "ClusterId": "cluster1", + "TimeoutPolicy": "customPolicy", + "Match": { + "Hosts": [ "localhost" ] + }, + } + "route2" : { + "ClusterId": "cluster1", + "Timeout": "00:01:00", + "Match": { + "Hosts": [ "localhost2" ] + }, + } + }, + "Clusters": { + "cluster1": { + "Destinations": { + "cluster1/destination1": { + "Address": "https://localhost:10001/" + } + } + } + } + } +} +``` + +Timeout policies and the default policy can be configured in the service collection and the middleware can be added as follows: +```csharp +var builder = WebApplication.CreateBuilder(args); + +builder.Services.AddReverseProxy() + .LoadFromConfig(builder.Configuration.GetSection("ReverseProxy")); + +builder.Services.AddRequestTimeouts(options => +{ + options.AddPolicy("customPolicy", TimeSpan.FromSeconds(20)); +}); + +var app = builder.Build(); + +app.UseRequestTimeouts(); + +app.MapReverseProxy(); + +app.Run(); +``` + +### Disable timeouts + +Specifying the value `disable` in a route's `TimeoutPolicy` parameter means the request timeout middleware will not apply timeouts to this route. + +### WebSockets + +Request timeouts are disabled after the initial WebSocket handshake. diff --git a/docs/docfx/articles/toc.yml b/docs/docfx/articles/toc.yml index e340d8442..b9d69d5c9 100644 --- a/docs/docfx/articles/toc.yml +++ b/docs/docfx/articles/toc.yml @@ -42,6 +42,8 @@ href: grpc.md - name: WebSockets and SPDY href: websockets.md +- name: Timeouts + href: timeouts.md - name: Service Fabric Integration href: service-fabric-int.md - name: Http.sys Delegation diff --git a/docs/docfx/articles/websockets.md b/docs/docfx/articles/websockets.md index c1fe082b2..fb893c424 100644 --- a/docs/docfx/articles/websockets.md +++ b/docs/docfx/articles/websockets.md @@ -21,3 +21,7 @@ The incoming and outgoing protocol versions do not need to match. The incoming W WebSockets require different HTTP headers for HTTP/2 so YARP will add and remove these headers as needed when adapting between the different versions. After the initial handshake WebSockets function the same way over both HTTP versions. + +## Timeout + +[Http Request Timeouts](https://learn.microsoft.com/aspnet/core/performance/timeouts) (.NET 8+) can apply timeouts to all requests by default or by policy. These timeouts will be disabled after a WebSocket handshake. They will still apply to gRPC requests. For additional configuration see [Timeouts](timeouts.md). diff --git a/samples/ReverseProxy.Minimal.Sample/Program.cs b/samples/ReverseProxy.Minimal.Sample/Program.cs index 0e6beaca8..62a48e722 100644 --- a/samples/ReverseProxy.Minimal.Sample/Program.cs +++ b/samples/ReverseProxy.Minimal.Sample/Program.cs @@ -2,9 +2,16 @@ builder.Services.AddReverseProxy() .LoadFromConfig(builder.Configuration.GetSection("ReverseProxy")); - +#if NET8_0_OR_GREATER +builder.Services.AddRequestTimeouts(options => +{ + options.AddPolicy("customPolicy", TimeSpan.FromSeconds(20)); +}); +#endif var app = builder.Build(); - +#if NET8_0_OR_GREATER +app.UseRequestTimeouts(); +#endif app.MapReverseProxy(); app.Run(); diff --git a/src/Kubernetes.Controller/Converters/YarpIngressOptions.cs b/src/Kubernetes.Controller/Converters/YarpIngressOptions.cs index 6d2957698..6f7069f71 100644 --- a/src/Kubernetes.Controller/Converters/YarpIngressOptions.cs +++ b/src/Kubernetes.Controller/Converters/YarpIngressOptions.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System; using System.Collections.Generic; using Yarp.ReverseProxy.Configuration; @@ -18,6 +19,8 @@ internal sealed class YarpIngressOptions public HttpClientConfig HttpClientConfig { get; set; } public string LoadBalancingPolicy { get; set; } public string CorsPolicy { get; set; } + public string TimeoutPolicy { get; set; } + public TimeSpan? Timeout { get; set; } public HealthCheckConfig HealthCheck { get; set; } public Dictionary RouteMetadata { get; set; } public List RouteHeaders { get; set; } diff --git a/src/Kubernetes.Controller/Converters/YarpParser.cs b/src/Kubernetes.Controller/Converters/YarpParser.cs index bf940c310..3da4617ec 100644 --- a/src/Kubernetes.Controller/Converters/YarpParser.cs +++ b/src/Kubernetes.Controller/Converters/YarpParser.cs @@ -142,6 +142,10 @@ private static RouteConfig CreateRoute(YarpIngressContext ingressContext, V1HTTP AuthorizationPolicy = ingressContext.Options.AuthorizationPolicy, #if NET7_0_OR_GREATER RateLimiterPolicy = ingressContext.Options.RateLimiterPolicy, +#endif +#if NET8_0_OR_GREATER + Timeout = ingressContext.Options.Timeout, + TimeoutPolicy = ingressContext.Options.TimeoutPolicy, #endif CorsPolicy = ingressContext.Options.CorsPolicy, Metadata = ingressContext.Options.RouteMetadata, diff --git a/src/ReverseProxy/Configuration/ConfigProvider/ConfigurationConfigProvider.cs b/src/ReverseProxy/Configuration/ConfigProvider/ConfigurationConfigProvider.cs index be174be44..4e29effd5 100644 --- a/src/ReverseProxy/Configuration/ConfigProvider/ConfigurationConfigProvider.cs +++ b/src/ReverseProxy/Configuration/ConfigProvider/ConfigurationConfigProvider.cs @@ -149,6 +149,10 @@ private static RouteConfig CreateRoute(IConfigurationSection section) AuthorizationPolicy = section[nameof(RouteConfig.AuthorizationPolicy)], #if NET7_0_OR_GREATER RateLimiterPolicy = section[nameof(RouteConfig.RateLimiterPolicy)], +#endif +#if NET8_0_OR_GREATER + TimeoutPolicy = section[nameof(RouteConfig.TimeoutPolicy)], + Timeout = section.ReadTimeSpan(nameof(RouteConfig.Timeout)), #endif CorsPolicy = section[nameof(RouteConfig.CorsPolicy)], Metadata = section.GetSection(nameof(RouteConfig.Metadata)).ReadStringDictionary(), diff --git a/src/ReverseProxy/Configuration/ConfigValidator.cs b/src/ReverseProxy/Configuration/ConfigValidator.cs index d6cb70ed9..06ac07976 100644 --- a/src/ReverseProxy/Configuration/ConfigValidator.cs +++ b/src/ReverseProxy/Configuration/ConfigValidator.cs @@ -11,8 +11,14 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Cors.Infrastructure; using Microsoft.AspNetCore.Http; +#if NET8_0_OR_GREATER +using Microsoft.AspNetCore.Http.Timeouts; +#endif using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.Extensions.Logging; +#if NET8_0_OR_GREATER +using Microsoft.Extensions.Options; +#endif using Yarp.ReverseProxy.Health; using Yarp.ReverseProxy.LoadBalancing; using Yarp.ReverseProxy.SessionAffinity; @@ -32,6 +38,9 @@ internal sealed class ConfigValidator : IConfigValidator private readonly IAuthorizationPolicyProvider _authorizationPolicyProvider; private readonly IYarpRateLimiterPolicyProvider _rateLimiterPolicyProvider; private readonly ICorsPolicyProvider _corsPolicyProvider; +#if NET8_0_OR_GREATER + private readonly IOptionsMonitor _timeoutOptions; +#endif private readonly FrozenDictionary _loadBalancingPolicies; private readonly FrozenDictionary _affinityFailurePolicies; private readonly FrozenDictionary _availableDestinationsPolicies; @@ -39,11 +48,13 @@ internal sealed class ConfigValidator : IConfigValidator private readonly FrozenDictionary _passiveHealthCheckPolicies; private readonly ILogger _logger; - public ConfigValidator(ITransformBuilder transformBuilder, IAuthorizationPolicyProvider authorizationPolicyProvider, IYarpRateLimiterPolicyProvider rateLimiterPolicyProvider, ICorsPolicyProvider corsPolicyProvider, +#if NET8_0_OR_GREATER + IOptionsMonitor timeoutOptions, +#endif IEnumerable loadBalancingPolicies, IEnumerable affinityFailurePolicies, IEnumerable availableDestinationsPolicies, @@ -55,6 +66,9 @@ public ConfigValidator(ITransformBuilder transformBuilder, _authorizationPolicyProvider = authorizationPolicyProvider ?? throw new ArgumentNullException(nameof(authorizationPolicyProvider)); _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 _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)); @@ -78,6 +92,9 @@ public async ValueTask> ValidateRouteAsync(RouteConfig route) await ValidateAuthorizationPolicyAsync(errors, route.AuthorizationPolicy, route.RouteId); #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); @@ -294,7 +311,37 @@ private async ValueTask ValidateAuthorizationPolicyAsync(IList errors 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) + { + 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)) diff --git a/src/ReverseProxy/Configuration/RouteConfig.cs b/src/ReverseProxy/Configuration/RouteConfig.cs index 0ca059266..817203b86 100644 --- a/src/ReverseProxy/Configuration/RouteConfig.cs +++ b/src/ReverseProxy/Configuration/RouteConfig.cs @@ -51,6 +51,23 @@ public sealed record RouteConfig /// Set to "Default" or leave empty to use the global rate limits, if any. /// public string? RateLimiterPolicy { get; init; } +#endif +#if NET8_0_OR_GREATER + /// + /// The name of the TimeoutPolicy to apply to this route. + /// Setting both Timeout and TimeoutPolicy is an error. + /// If not set then only the system default will apply. + /// Set to "Disable" to disable timeouts for this route. + /// Set to "Default" or leave empty to use the system defaults, if any. + /// + public string? TimeoutPolicy { get; init; } + + /// + /// The Timeout to apply to this route. This overrides any system defaults. + /// Setting both Timeout and TimeoutPolicy is an error. + /// Timeout granularity is limited to milliseconds. + /// + public TimeSpan? Timeout { get; init; } #endif /// /// The name of the CorsPolicy to apply to this route. @@ -89,6 +106,10 @@ public bool Equals(RouteConfig? other) && string.Equals(AuthorizationPolicy, other.AuthorizationPolicy, StringComparison.OrdinalIgnoreCase) #if NET7_0_OR_GREATER && string.Equals(RateLimiterPolicy, other.RateLimiterPolicy, StringComparison.OrdinalIgnoreCase) +#endif +#if NET8_0_OR_GREATER + && string.Equals(TimeoutPolicy, other.TimeoutPolicy, StringComparison.OrdinalIgnoreCase) + && Timeout == other.Timeout #endif && string.Equals(CorsPolicy, other.CorsPolicy, StringComparison.OrdinalIgnoreCase) && Match == other.Match @@ -106,6 +127,10 @@ public override int GetHashCode() hash.Add(AuthorizationPolicy?.GetHashCode(StringComparison.OrdinalIgnoreCase)); #if NET7_0_OR_GREATER hash.Add(RateLimiterPolicy?.GetHashCode(StringComparison.OrdinalIgnoreCase)); +#endif +#if NET8_0_OR_GREATER + hash.Add(Timeout?.GetHashCode()); + hash.Add(TimeoutPolicy?.GetHashCode(StringComparison.OrdinalIgnoreCase)); #endif hash.Add(CorsPolicy?.GetHashCode(StringComparison.OrdinalIgnoreCase)); hash.Add(Match); diff --git a/src/ReverseProxy/Configuration/TimeoutPolicyConstants.cs b/src/ReverseProxy/Configuration/TimeoutPolicyConstants.cs new file mode 100644 index 000000000..71f0fd38d --- /dev/null +++ b/src/ReverseProxy/Configuration/TimeoutPolicyConstants.cs @@ -0,0 +1,9 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Yarp.ReverseProxy.Configuration; + +internal static class TimeoutPolicyConstants +{ + internal const string Disable = "Disable"; +} diff --git a/src/ReverseProxy/Forwarder/HttpForwarder.cs b/src/ReverseProxy/Forwarder/HttpForwarder.cs index f905e2a4e..81c3ef626 100644 --- a/src/ReverseProxy/Forwarder/HttpForwarder.cs +++ b/src/ReverseProxy/Forwarder/HttpForwarder.cs @@ -12,6 +12,9 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; +#if NET8_0_OR_GREATER +using Microsoft.AspNetCore.Http.Timeouts; +#endif using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging; @@ -719,6 +722,10 @@ private async ValueTask HandleUpgradedResponse(HttpContext conte Debug.Assert(upgradeFeature != null); upgradeResult = await upgradeFeature.UpgradeAsync(); } +#if NET8_0_OR_GREATER + // Disable request timeout, if there is one, after the upgrade has been accepted + context.Features.Get()?.DisableTimeout(); +#endif } catch (Exception ex) { diff --git a/src/ReverseProxy/Model/ProxyPipelineInitializerMiddleware.cs b/src/ReverseProxy/Model/ProxyPipelineInitializerMiddleware.cs index 8006bce0f..c11050b6f 100644 --- a/src/ReverseProxy/Model/ProxyPipelineInitializerMiddleware.cs +++ b/src/ReverseProxy/Model/ProxyPipelineInitializerMiddleware.cs @@ -5,7 +5,13 @@ using System.Diagnostics; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; +#if NET8_0_OR_GREATER +using Microsoft.AspNetCore.Http.Timeouts; +#endif using Microsoft.Extensions.Logging; +#if NET8_0_OR_GREATER +using Yarp.ReverseProxy.Configuration; +#endif using Yarp.ReverseProxy.Utilities; namespace Yarp.ReverseProxy.Model; @@ -41,7 +47,19 @@ public Task Invoke(HttpContext context) context.Response.StatusCode = StatusCodes.Status503ServiceUnavailable; return Task.CompletedTask; } - +#if NET8_0_OR_GREATER + // There's no way to detect the presence of the timeout middleware before this, only the options. + if (endpoint.Metadata.GetMetadata() != null + && context.Features.Get() == null + // The feature is skipped if the request is already canceled. We'll handle canceled requests later for consistency. + && !context.RequestAborted.IsCancellationRequested) + { + Log.TimeoutNotApplied(_logger, route.Config.RouteId); + // Out of an abundance of caution, refuse the request rather than allowing it to proceed without the configured timeout. + throw new InvalidOperationException($"The timeout was not applied for route '{route.Config.RouteId}', ensure `IApplicationBuilder.UseRequestTimeouts()`" + + " is called between `IApplicationBuilder.UseRouting()` and `IApplicationBuilder.UseEndpoints()`."); + } +#endif var destinationsState = cluster.DestinationsState; context.Features.Set(new ReverseProxyFeature { @@ -80,9 +98,19 @@ private static class Log EventIds.NoClusterFound, "Route '{routeId}' has no cluster information."); + private static readonly Action _timeoutNotApplied = LoggerMessage.Define( + LogLevel.Error, + EventIds.TimeoutNotApplied, + "The timeout was not applied for route '{routeId}', ensure `IApplicationBuilder.UseRequestTimeouts()` is called between `IApplicationBuilder.UseRouting()` and `IApplicationBuilder.UseEndpoints()`."); + public static void NoClusterFound(ILogger logger, string routeId) { _noClusterFound(logger, routeId, null); } + + public static void TimeoutNotApplied(ILogger logger, string routeId) + { + _timeoutNotApplied(logger, routeId, null); + } } } diff --git a/src/ReverseProxy/Routing/ProxyEndpointFactory.cs b/src/ReverseProxy/Routing/ProxyEndpointFactory.cs index 2ab8aeae8..1be5aae6e 100644 --- a/src/ReverseProxy/Routing/ProxyEndpointFactory.cs +++ b/src/ReverseProxy/Routing/ProxyEndpointFactory.cs @@ -9,6 +9,9 @@ using Microsoft.AspNetCore.Cors; using Microsoft.AspNetCore.Cors.Infrastructure; using Microsoft.AspNetCore.Http; +#if NET8_0_OR_GREATER +using Microsoft.AspNetCore.Http.Timeouts; +#endif #if NET7_0_OR_GREATER using Microsoft.AspNetCore.RateLimiting; #endif @@ -18,6 +21,7 @@ using CorsConstants = Yarp.ReverseProxy.Configuration.CorsConstants; using AuthorizationConstants = Yarp.ReverseProxy.Configuration.AuthorizationConstants; using RateLimitingConstants = Yarp.ReverseProxy.Configuration.RateLimitingConstants; +using TimeoutPolicyConstants = Yarp.ReverseProxy.Configuration.TimeoutPolicyConstants; namespace Yarp.ReverseProxy.Routing; @@ -26,6 +30,9 @@ internal sealed class ProxyEndpointFactory private static readonly IAuthorizeData _defaultAuthorization = new AuthorizeAttribute(); #if NET7_0_OR_GREATER private static readonly DisableRateLimitingAttribute _disableRateLimit = new(); +#endif +#if NET8_0_OR_GREATER + private static readonly DisableRequestTimeoutAttribute _disableRequestTimeout = new(); #endif private static readonly IEnableCorsAttribute _defaultCors = new EnableCorsAttribute(); private static readonly IDisableCorsAttribute _disableCors = new DisableCorsAttribute(); @@ -131,6 +138,21 @@ public Endpoint CreateEndpoint(RouteModel route, IReadOnlyList err.Message.Equals($"The application has registered an authorization policy named '{authorizationPolicy}' 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.")); } +#if NET8_0_OR_GREATER + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("disAble")] + public async Task Accepts_ReservedTimeoutPolicy(string policy) + { + var route = new RouteConfig + { + RouteId = "route1", + TimeoutPolicy = policy, + Match = new RouteMatch + { + Hosts = new[] { "localhost" }, + }, + ClusterId = "cluster1", + }; + + var services = CreateServices(); + var validator = services.GetRequiredService(); + + var result = await validator.ValidateRouteAsync(route); + Assert.Empty(result); + } + + [Fact] + public async Task Accepts_CustomTimeoutPolicy() + { + var route = new RouteConfig + { + RouteId = "route1", + TimeoutPolicy = "custom", + Match = new RouteMatch + { + Hosts = new[] { "localhost" }, + }, + ClusterId = "cluster1", + }; + + var services = CreateServices(services => + { + services.AddRequestTimeouts(options => + { + options.AddPolicy("custom", TimeSpan.FromSeconds(1)); + }); + }); + var validator = services.GetRequiredService(); + + var result = await validator.ValidateRouteAsync(route); + + Assert.Empty(result); + } + + [Fact] + public async Task Accepts_CustomTimeout() + { + var route = new RouteConfig + { + RouteId = "route1", + Timeout = TimeSpan.FromSeconds(1), + Match = new RouteMatch + { + Hosts = new[] { "localhost" }, + }, + ClusterId = "cluster1", + }; + + var services = CreateServices(); + var validator = services.GetRequiredService(); + + var result = await validator.ValidateRouteAsync(route); + + Assert.Empty(result); + } + + [Fact] + public async Task Rejects_UnknownTimeoutPolicy() + { + var route = new RouteConfig + { + RouteId = "route1", + TimeoutPolicy = "unknown", + ClusterId = "cluster1", + Match = new RouteMatch(), + }; + + var services = CreateServices(); + var validator = services.GetRequiredService(); + + var result = await validator.ValidateRouteAsync(route); + + Assert.NotEmpty(result); + Assert.Contains(result, err => err.Message.Equals("Timeout policy 'unknown' not found for route 'route1'.")); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + public async Task Rejects_InvalidTimeouts(int timeout) + { + var route = new RouteConfig + { + RouteId = "route1", + Timeout = TimeSpan.FromMilliseconds(timeout), + ClusterId = "cluster1", + Match = new RouteMatch(), + }; + + var services = CreateServices(); + var validator = services.GetRequiredService(); + + var result = await validator.ValidateRouteAsync(route); + + Assert.NotEmpty(result); + Assert.Contains(result, err => err.Message.Equals($"The Timeout value '{TimeSpan.FromMilliseconds(timeout)}' is invalid for route 'route1'. The Timeout must be greater than zero milliseconds.")); + } + + [Fact] + public async Task Rejects_TimeoutWithTimeoutPolicy() + { + var route = new RouteConfig + { + RouteId = "route1", + TimeoutPolicy = "unknown", + Timeout = TimeSpan.FromSeconds(1), + ClusterId = "cluster1", + Match = new RouteMatch(), + }; + + var services = CreateServices(); + var validator = services.GetRequiredService(); + + var result = await validator.ValidateRouteAsync(route); + + Assert.NotEmpty(result); + Assert.Contains(result, err => err.Message.Equals("Timeout policy 'unknown' not found for route 'route1'.")); + } +#endif [Theory] [InlineData(null)] [InlineData("")] diff --git a/test/ReverseProxy.Tests/Configuration/RouteConfigTests.cs b/test/ReverseProxy.Tests/Configuration/RouteConfigTests.cs index 073e2b843..9b071b017 100644 --- a/test/ReverseProxy.Tests/Configuration/RouteConfigTests.cs +++ b/test/ReverseProxy.Tests/Configuration/RouteConfigTests.cs @@ -1,8 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System; using System.Collections.Generic; using System.Text.Json; +using System.Threading; using Xunit; namespace Yarp.ReverseProxy.Configuration.Tests; @@ -17,6 +19,10 @@ public void Equals_Positive() AuthorizationPolicy = "a", #if NET7_0_OR_GREATER RateLimiterPolicy = "rl", +#endif +#if NET8_0_OR_GREATER + TimeoutPolicy = "t", + Timeout = TimeSpan.FromSeconds(1), #endif ClusterId = "c", CorsPolicy = "co", @@ -48,6 +54,10 @@ public void Equals_Positive() AuthorizationPolicy = "A", #if NET7_0_OR_GREATER RateLimiterPolicy = "RL", +#endif +#if NET8_0_OR_GREATER + TimeoutPolicy = "T", + Timeout = TimeSpan.FromSeconds(1), #endif ClusterId = "C", CorsPolicy = "Co", @@ -90,6 +100,10 @@ public void Equals_Negative() AuthorizationPolicy = "a", #if NET7_0_OR_GREATER RateLimiterPolicy = "rl", +#endif +#if NET8_0_OR_GREATER + TimeoutPolicy = "t", + Timeout = TimeSpan.FromSeconds(1), #endif ClusterId = "c", CorsPolicy = "co", @@ -126,6 +140,10 @@ public void Equals_Negative() #if NET7_0_OR_GREATER var i = a with { RateLimiterPolicy = "i" }; #endif +#if NET8_0_OR_GREATER + var j = a with { TimeoutPolicy = "j" }; + var k = a with { Timeout = TimeSpan.FromSeconds(107) }; +#endif Assert.False(a.Equals(b)); Assert.False(a.Equals(c)); @@ -136,6 +154,10 @@ public void Equals_Negative() Assert.False(a.Equals(h)); #if NET7_0_OR_GREATER Assert.False(a.Equals(i)); +#endif +#if NET8_0_OR_GREATER + Assert.False(a.Equals(j)); + Assert.False(a.Equals(k)); #endif } diff --git a/test/ReverseProxy.Tests/Model/ProxyPipelineInitializerMiddlewareTests.cs b/test/ReverseProxy.Tests/Model/ProxyPipelineInitializerMiddlewareTests.cs index d1b9d02e4..fdf917ab0 100644 --- a/test/ReverseProxy.Tests/Model/ProxyPipelineInitializerMiddlewareTests.cs +++ b/test/ReverseProxy.Tests/Model/ProxyPipelineInitializerMiddlewareTests.cs @@ -7,6 +7,9 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; +#if NET8_0_OR_GREATER +using Microsoft.AspNetCore.Http.Timeouts; +#endif using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Patterns; using Moq; @@ -119,14 +122,44 @@ public async Task Invoke_NoHealthyEndpoints_CallsNext() Assert.Equal(StatusCodes.Status418ImATeapot, httpContext.Response.StatusCode); } +#if NET8_0_OR_GREATER + [Fact] + public async Task Invoke_MissingTimeoutMiddleware_RefuseRequest() + { + var httpClient = new HttpMessageInvoker(new Mock().Object); + var cluster1 = new ClusterState(clusterId: "cluster1") + { + Model = new ClusterModel(new ClusterConfig(), httpClient) + }; + + var aspNetCoreEndpoints = new List(); + var routeConfig = new RouteModel( + config: new RouteConfig(), + cluster: cluster1, + transformer: HttpTransformer.Default); + var aspNetCoreEndpoint = CreateAspNetCoreEndpoint(routeConfig, + builder => + { + builder.Metadata.Add(new RequestTimeoutAttribute(1)); + }); + aspNetCoreEndpoints.Add(aspNetCoreEndpoint); + var httpContext = new DefaultHttpContext(); + httpContext.SetEndpoint(aspNetCoreEndpoint); + + var sut = Create(); + + await Assert.ThrowsAsync(() => sut.Invoke(httpContext)); + } +#endif - private static Endpoint CreateAspNetCoreEndpoint(RouteModel routeConfig) + private static Endpoint CreateAspNetCoreEndpoint(RouteModel routeConfig, Action configure = null) { var endpointBuilder = new RouteEndpointBuilder( requestDelegate: httpContext => Task.CompletedTask, routePattern: RoutePatternFactory.Parse("/"), order: 0); endpointBuilder.Metadata.Add(routeConfig); + configure?.Invoke(endpointBuilder); return endpointBuilder.Build(); } } diff --git a/test/ReverseProxy.Tests/Routing/ProxyEndpointFactoryTests.cs b/test/ReverseProxy.Tests/Routing/ProxyEndpointFactoryTests.cs index c1c3a88ab..5e96edc7c 100644 --- a/test/ReverseProxy.Tests/Routing/ProxyEndpointFactoryTests.cs +++ b/test/ReverseProxy.Tests/Routing/ProxyEndpointFactoryTests.cs @@ -5,6 +5,9 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Cors; using Microsoft.AspNetCore.Cors.Infrastructure; +#if NET8_0_OR_GREATER +using Microsoft.AspNetCore.Http.Timeouts; +#endif #if NET7_0_OR_GREATER using Microsoft.AspNetCore.RateLimiting; #endif @@ -418,6 +421,104 @@ public void AddEndpoint_NoRateLimiter_Works() Assert.Null(routeEndpoint.Metadata.GetMetadata()); } #endif +#if NET8_0_OR_GREATER + [Fact] + public void AddEndpoint_CustomTimeoutPolicy_Works() + { + var services = CreateServices(); + var factory = services.GetRequiredService(); + factory.SetProxyPipeline(context => Task.CompletedTask); + + var route = new RouteConfig + { + RouteId = "route1", + TimeoutPolicy = "custom", + Order = 12, + Match = new RouteMatch(), + }; + var cluster = new ClusterState("cluster1"); + var routeState = new RouteState("route1"); + + var (routeEndpoint, _) = CreateEndpoint(factory, routeState, route, cluster); + + var attribute = routeEndpoint.Metadata.GetMetadata(); + Assert.NotNull(attribute); + Assert.Equal("custom", attribute.PolicyName); + Assert.Null(attribute.Timeout); + Assert.Null(routeEndpoint.Metadata.GetMetadata()); + } + + [Fact] + public void AddEndpoint_CustomTimeout_Works() + { + var services = CreateServices(); + var factory = services.GetRequiredService(); + factory.SetProxyPipeline(context => Task.CompletedTask); + + var route = new RouteConfig + { + RouteId = "route1", + Timeout = TimeSpan.FromSeconds(5), + Order = 12, + Match = new RouteMatch(), + }; + var cluster = new ClusterState("cluster1"); + var routeState = new RouteState("route1"); + + var (routeEndpoint, _) = CreateEndpoint(factory, routeState, route, cluster); + + var attribute = routeEndpoint.Metadata.GetMetadata(); + Assert.NotNull(attribute); + Assert.Null(attribute.PolicyName); + Assert.Equal(TimeSpan.FromSeconds(5), attribute.Timeout); + Assert.Null(routeEndpoint.Metadata.GetMetadata()); + } + + [Fact] + public void AddEndpoint_DisableTimeoutPolicy_Works() + { + var services = CreateServices(); + var factory = services.GetRequiredService(); + factory.SetProxyPipeline(context => Task.CompletedTask); + + var route = new RouteConfig + { + RouteId = "route1", + TimeoutPolicy = "disAble", + Order = 12, + Match = new RouteMatch(), + }; + var cluster = new ClusterState("cluster1"); + var routeState = new RouteState("route1"); + + var (routeEndpoint, _) = CreateEndpoint(factory, routeState, route, cluster); + + Assert.NotNull(routeEndpoint.Metadata.GetMetadata()); + Assert.Null(routeEndpoint.Metadata.GetMetadata()); + } + + [Fact] + public void AddEndpoint_NoTimeoutPolicy_Works() + { + var services = CreateServices(); + var factory = services.GetRequiredService(); + factory.SetProxyPipeline(context => Task.CompletedTask); + + var route = new RouteConfig + { + RouteId = "route1", + Order = 12, + Match = new RouteMatch(), + }; + var cluster = new ClusterState("cluster1"); + var routeState = new RouteState("route1"); + + var (routeEndpoint, _) = CreateEndpoint(factory, routeState, route, cluster); + + Assert.Null(routeEndpoint.Metadata.GetMetadata()); + Assert.Null(routeEndpoint.Metadata.GetMetadata()); + } +#endif [Fact] public void AddEndpoint_DefaultCors_Works() diff --git a/testassets/ReverseProxy.Code/ReverseProxy.Code.csproj b/testassets/ReverseProxy.Code/ReverseProxy.Code.csproj index 5c56bc355..f8482f5fa 100644 --- a/testassets/ReverseProxy.Code/ReverseProxy.Code.csproj +++ b/testassets/ReverseProxy.Code/ReverseProxy.Code.csproj @@ -1,7 +1,7 @@ - net6.0;net7.0 + net6.0;net7.0;net8.0 Exe Yarp.ReverseProxy.Sample diff --git a/testassets/ReverseProxy.Code/Startup.cs b/testassets/ReverseProxy.Code/Startup.cs index ccd8a5f52..e5b9eb1df 100644 --- a/testassets/ReverseProxy.Code/Startup.cs +++ b/testassets/ReverseProxy.Code/Startup.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Net; using System.Net.Http.Headers; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Builder; @@ -35,7 +36,10 @@ public void ConfigureServices(IServiceCollection services) Match = new RouteMatch { Path = "{**catch-all}" - } + }, +#if NET8_0_OR_GREATER + Timeout = TimeSpan.FromSeconds(5), +#endif } }; var clusters = new[] @@ -72,7 +76,7 @@ public void ConfigureServices(IServiceCollection services) // For each route+cluster pair decide if we want to add transforms, and if so, which? // This logic is re-run each time a route is rebuilt. - transformBuilderContext.AddPathPrefix("/prefix"); + // transformBuilderContext.AddPathPrefix("/prefix"); // Only do this for routes that require auth. if (string.Equals("token", transformBuilderContext.Route.AuthorizationPolicy)) @@ -104,6 +108,16 @@ public void ConfigureServices(IServiceCollection services) services.AddSingleton, ForwarderMetricsConsumer>(); services.AddTelemetryConsumer(); services.AddTelemetryListeners(); +#if NET8_0_OR_GREATER + services.AddRequestTimeouts(o => + { + o.DefaultPolicy = new Microsoft.AspNetCore.Http.Timeouts.RequestTimeoutPolicy() + { + Timeout = TimeSpan.FromSeconds(1), + TimeoutStatusCode = StatusCodes.Status418ImATeapot, + }; + }); +#endif } /// @@ -113,6 +127,9 @@ public void Configure(IApplicationBuilder app, IProxyStateLookup lookup) { app.UseRouting(); app.UseAuthorization(); +#if NET8_0_OR_GREATER + app.UseRequestTimeouts(); +#endif app.UseEndpoints(endpoints => { endpoints.MapControllers(); diff --git a/testassets/TestServer/Controllers/HttpController.cs b/testassets/TestServer/Controllers/HttpController.cs index fa9f9f30b..e265f3649 100644 --- a/testassets/TestServer/Controllers/HttpController.cs +++ b/testassets/TestServer/Controllers/HttpController.cs @@ -37,6 +37,18 @@ public IActionResult SkipBody() return StatusCode(StatusCodes.Status409Conflict); } + /// + /// Returns a 409 response without consuming the request body. + /// This is used to exercise Expect:100-continue behavior. + /// + [HttpGet] + [Route("/api/slow")] + public async Task Slow() + { + await Task.Delay(TimeSpan.FromSeconds(3)); + return StatusCode(StatusCodes.Status200OK); + } + /// /// Returns a 200 response dumping all info from the incoming request. ///