Skip to content
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

Configuration of the proxied HTTP request version. #512

Merged
merged 15 commits into from
Nov 10, 2020
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -353,3 +353,9 @@ MigrationBackup/

# Ionide (cross platform F# VS Code tools) working folder
.ionide/

# Rider
.idea/

# nohup
nohup.out
5 changes: 3 additions & 2 deletions samples/ReverseProxy.Code.Sample/CustomConfigFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,16 @@ public class CustomConfigFilter : IProxyConfigFilter
public Task ConfigureClusterAsync(Cluster cluster, CancellationToken cancel)
{
// How to use custom metadata to configure clusters
if (cluster.Metadata?.TryGetValue("CustomHealth", out var customHealth) ?? false
if (cluster.Metadata != null
&& cluster.Metadata.TryGetValue("CustomHealth", out var customHealth)
&& string.Equals(customHealth, "true", StringComparison.OrdinalIgnoreCase))
{
cluster.HealthCheck ??= new HealthCheckOptions { Active = new ActiveHealthCheckOptions() };
cluster.HealthCheck.Active.Enabled = true;
cluster.HealthCheck.Active.Policy = HealthCheckConstants.ActivePolicy.ConsecutiveFailures;
}

// Or wrap the meatadata in config sugar
// Or wrap the metadata in config sugar
var config = new ConfigurationBuilder().AddInMemoryCollection(cluster.Metadata).Build();
if (config.GetValue<bool>("CustomHealth"))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ public sealed class Cluster : IDeepCloneable<Cluster>
/// </summary>
public ProxyHttpClientOptions HttpClient { get; set; }

/// <summary>
/// Options of an outgoing HTTP request.
/// </summary>
public ProxyHttpRequestOptions HttpRequest { get; set; }

/// <summary>
/// The set of destinations associated with this cluster.
/// </summary>
Expand All @@ -78,6 +83,7 @@ Cluster IDeepCloneable<Cluster>.DeepClone()
SessionAffinity = SessionAffinity?.DeepClone(),
HealthCheck = HealthCheck?.DeepClone(),
HttpClient = HttpClient?.DeepClone(),
HttpRequest = HttpRequest?.DeepClone(),
Destinations = Destinations.DeepClone(StringComparer.OrdinalIgnoreCase),
Metadata = Metadata?.DeepClone(StringComparer.OrdinalIgnoreCase),
};
Expand All @@ -103,6 +109,7 @@ internal static bool Equals(Cluster cluster1, Cluster cluster2)
&& SessionAffinityOptions.Equals(cluster1.SessionAffinity, cluster2.SessionAffinity)
&& HealthCheckOptions.Equals(cluster1.HealthCheck, cluster2.HealthCheck)
&& ProxyHttpClientOptions.Equals(cluster1.HttpClient, cluster2.HttpClient)
&& ProxyHttpRequestOptions.Equals(cluster1.HttpRequest, cluster2.HttpRequest)
&& CaseInsensitiveEqualHelper.Equals(cluster1.Destinations, cluster2.Destinations, Destination.Equals)
&& CaseInsensitiveEqualHelper.Equals(cluster1.Metadata, cluster2.Metadata);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
using System;
using System.Net.Http;

namespace Microsoft.ReverseProxy.Abstractions
{
public sealed class ProxyHttpRequestOptions
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
{
public TimeSpan? RequestTimeout { get; set; }
Tratcher marked this conversation as resolved.
Show resolved Hide resolved

public Version Version { get; set; }

#if NET
public HttpVersionPolicy VersionPolicy { get; set; }
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
#endif

internal ProxyHttpRequestOptions DeepClone()
{
return new ProxyHttpRequestOptions
{
RequestTimeout = RequestTimeout,
Version = Version,
#if NET
VersionPolicy = VersionPolicy,
#endif
};
}

internal static bool Equals(ProxyHttpRequestOptions options1, ProxyHttpRequestOptions options2)
{
if (options1 == null && options2 == null)
{
return true;
}

if (options1 == null || options2 == null)
{
return false;
}

return options1.RequestTimeout == options2.RequestTimeout
&& options1.Version == options2.Version
#if NET
&& options1.VersionPolicy == options2.VersionPolicy
#endif
;
}
}
}
25 changes: 25 additions & 0 deletions src/ReverseProxy/Configuration/ConfigurationConfigProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ private Cluster Convert(string clusterId, ClusterData data)
SessionAffinity = Convert(data.SessionAffinity),
HealthCheck = Convert(data.HealthCheck),
HttpClient = Convert(data.HttpClient),
HttpRequest = Convert(data.HttpRequest),
Metadata = data.Metadata?.DeepClone(StringComparer.OrdinalIgnoreCase)
};
foreach(var destination in data.Destinations)
Expand Down Expand Up @@ -353,6 +354,30 @@ private ProxyHttpClientOptions Convert(ProxyHttpClientData data)
};
}

private ProxyHttpRequestOptions Convert(ProxyHttpRequestData data)
{
if (data == null)
{
return null;
}

// Parse version only if it contains any characters; otherwise, leave it null.
var version = default(Version);
if (!string.IsNullOrEmpty(data.Version))
{
version = Version.Parse(data.Version + (data.Version.Contains('.') ? "" : ".0"));
}

return new ProxyHttpRequestOptions
{
RequestTimeout = data.RequestTimeout,
Version = version,
#if NET
VersionPolicy = data.VersionPolicy,
#endif
};
}

private static Destination Convert(DestinationData data)
{
if (data == null)
Expand Down
5 changes: 5 additions & 0 deletions src/ReverseProxy/Configuration/Contract/ClusterData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ public sealed class ClusterData
/// </summary>
public ProxyHttpClientData HttpClient { get; set; }

/// <summary>
/// Options of an outgoing HTTP request.
/// </summary>
public ProxyHttpRequestData HttpRequest { get; set; }

/// <summary>
/// The set of destinations associated with this cluster.
/// </summary>
Expand Down
32 changes: 32 additions & 0 deletions src/ReverseProxy/Configuration/Contract/ProxyHttpRequestData.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
using System;
using System.Net.Http;

namespace Microsoft.ReverseProxy.Configuration.Contract
{
/// <summary>
/// Outgoing request configuration.
/// </summary>
public class ProxyHttpRequestData
{
/// <summary>
/// Timeout for the outgoing request.
/// Default is 100 seconds.
/// </summary>
public TimeSpan? RequestTimeout { get; set; }

/// <summary>
/// HTTP version for the outgoing request.
/// Default is HTTP/2.
/// </summary>
public string Version { get; set; }

#if NET
/// <summary>
/// Version policy for the outgoing request.
/// Defines whether to upgrade or downgrade HTTP version if possible.
/// Default is <c>RequestVersionOrLower</c>.
/// </summary>
public HttpVersionPolicy VersionPolicy { get; set; }
#endif
}
}
14 changes: 14 additions & 0 deletions src/ReverseProxy/Middleware/ProxyInvokerMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;
using Microsoft.ReverseProxy.Abstractions.Telemetry;
using Microsoft.ReverseProxy.RuntimeModel;
using Microsoft.ReverseProxy.Service.Proxy;
using Microsoft.ReverseProxy.Utilities;

Expand Down Expand Up @@ -80,6 +81,19 @@ public async Task Invoke(HttpContext context)
Transforms = routeConfig.Transforms,
};

var requestOptions = reverseProxyFeature.ClusterConfig.HttpRequestOptions;
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
if (requestOptions.RequestTimeout != default)
{
proxyOptions.RequestTimeout = requestOptions.RequestTimeout;
}
if (requestOptions.Version != default)
{
proxyOptions.Version = requestOptions.Version;
alnikola marked this conversation as resolved.
Show resolved Hide resolved
}
#if NET
proxyOptions.VersionPolicy = requestOptions.VersionPolicy;
#endif

try
{
cluster.ConcurrencyCounter.Increment();
Expand Down
19 changes: 19 additions & 0 deletions src/ReverseProxy/Service/Config/ConfigValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Authorization;
Expand Down Expand Up @@ -118,6 +119,7 @@ public ValueTask<IList<Exception>> ValidateClusterAsync(Cluster cluster)

ValidateSessionAffinity(errors, cluster);
ValidateProxyHttpClient(errors, cluster);
ValidateProxyHttpRequest(errors, cluster);
ValidateActiveHealthCheck(errors, cluster);
ValidatePassiveHealthCheck(errors, cluster);

Expand Down Expand Up @@ -322,6 +324,23 @@ private void ValidateProxyHttpClient(IList<Exception> errors, Cluster cluster)
}
}

private void ValidateProxyHttpRequest(IList<Exception> errors, Cluster cluster)
alnikola marked this conversation as resolved.
Show resolved Hide resolved
{
if (cluster.HttpRequest == null)
{
// Proxy http request options are not set.
return;
}

if (cluster.HttpRequest.Version != null &&
cluster.HttpRequest.Version != HttpVersion.Version10 &&
cluster.HttpRequest.Version != HttpVersion.Version11 &&
cluster.HttpRequest.Version != HttpVersion.Version20)
{
errors.Add(new ArgumentException($"Outgoing request version '{cluster.HttpRequest.Version}' is not any of supported HTTP versions (1.0, 1.1 and 2)."));
}
}

private void ValidateActiveHealthCheck(IList<Exception> errors, Cluster cluster)
{
if (cluster.HealthCheck == null || cluster.HealthCheck.Active == null || !cluster.HealthCheck.Active.Enabled)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Net;
using System.Net.Http;
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.ReverseProxy.RuntimeModel;
Expand All @@ -15,7 +16,18 @@ public HttpRequestMessage CreateRequest(ClusterConfig clusterConfig, Destination
var probePath = clusterConfig.HealthCheckOptions.Active.Path;
UriHelper.FromAbsolute(probeAddress, out var destinationScheme, out var destinationHost, out var destinationPathBase, out _, out _);
var probeUri = UriHelper.BuildAbsolute(destinationScheme, destinationHost, destinationPathBase, probePath, default);
return new HttpRequestMessage(HttpMethod.Get, probeUri) { Version = ProtocolHelper.Http2Version };
var version = HttpVersion.Version20;
if (clusterConfig.HttpRequestOptions.Version != default)
{
version = clusterConfig.HttpRequestOptions.Version;
}
return new HttpRequestMessage(HttpMethod.Get, probeUri)
{
Version = version,
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
#if NET
VersionPolicy = clusterConfig.HttpRequestOptions.VersionPolicy
#endif
};
}
}
}
9 changes: 9 additions & 0 deletions src/ReverseProxy/Service/Management/ProxyConfigManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
Expand Down Expand Up @@ -311,6 +313,13 @@ private void UpdateRuntimeClusters(IList<Cluster> newClusters)
settings: newCluster.SessionAffinity?.Settings as IReadOnlyDictionary<string, string>),
httpClient,
newClusterHttpClientOptions,
new ClusterProxyHttpRequestOptions(
requestTimeout: newCluster.HttpRequest?.RequestTimeout ?? TimeSpan.FromSeconds(100),
version: newCluster.HttpRequest?.Version ?? HttpVersion.Version20
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
#if NET
, versionPolicy: newCluster.HttpRequest?.VersionPolicy ?? HttpVersionPolicy.RequestVersionOrLower
#endif
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
),
(IReadOnlyDictionary<string, string>)newCluster.Metadata);

if (currentClusterConfig == null ||
Expand Down
6 changes: 5 additions & 1 deletion src/ReverseProxy/Service/RuntimeModel/ClusterConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace Microsoft.ReverseProxy.RuntimeModel
/// <remarks>
/// All members must remain immutable to avoid thread safety issues.
/// Instead, instances of <see cref="ClusterConfig"/> are replaced
/// in ther entirety when values need to change.
/// in their entirety when values need to change.
/// </remarks>
public sealed class ClusterConfig
{
Expand All @@ -29,6 +29,7 @@ public ClusterConfig(
ClusterSessionAffinityOptions sessionAffinityOptions,
HttpMessageInvoker httpClient,
ClusterProxyHttpClientOptions httpClientOptions,
ClusterProxyHttpRequestOptions httpRequestOptions,
IReadOnlyDictionary<string, string> metadata)
{
_cluster = cluster;
Expand All @@ -37,6 +38,7 @@ public ClusterConfig(
SessionAffinityOptions = sessionAffinityOptions;
HttpClient = httpClient;
HttpClientOptions = httpClientOptions;
HttpRequestOptions = httpRequestOptions;
Metadata = metadata;
}

Expand All @@ -48,6 +50,8 @@ public ClusterConfig(

public ClusterProxyHttpClientOptions HttpClientOptions { get; }

public ClusterProxyHttpRequestOptions HttpRequestOptions { get; }

/// <summary>
/// An <see cref="HttpMessageInvoker"/> that used for proxying requests to an upstream server.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
using System;
using System.Net.Http;

namespace Microsoft.ReverseProxy.RuntimeModel
{
public readonly struct ClusterProxyHttpRequestOptions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline: we'll want to see how we can unify this with RequestProxyOptions so we don't have to allocate that per request. The big differences are:

  • RequestProxyOptions has Transforms on it which come from routeConfig.
  • RequestProxyOptions has the Timeout which should be easy to move/mirror.
  • struct vs class, readonly vs mutable
  • Naming

Some possibilities:

  • We come up with a type that works in both scenarios and use that on ClusterConfig.
  • RequestProxyOptions becomes a readonly struct so at least we don't have to allocate it. We'd still end up copying the paramters across.
  • Transforms get passed into IHttpProxy.ProxyAsync as a separate parameter, making the options types more compatible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
public ClusterProxyHttpRequestOptions(
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
TimeSpan requestTimeout,
Version version
#if NET
, HttpVersionPolicy versionPolicy
#endif
)
{
RequestTimeout = requestTimeout;
Version = version;
#if NET
VersionPolicy = versionPolicy;
#endif
}

public TimeSpan RequestTimeout { get; }

public Version Version { get; }

#if NET
public HttpVersionPolicy VersionPolicy { get; }
#endif
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public void DeepClone_Works()
LoadBalancing = new LoadBalancingOptions(),
HealthCheck = new HealthCheckOptions(),
HttpClient = new ProxyHttpClientOptions(),
HttpRequest = new ProxyHttpRequestOptions(),
Metadata = new Dictionary<string, string>
{
{ "key", "value" },
Expand Down
Loading