Skip to content

Commit

Permalink
removing AllowSynchronousIO; hiding behind v2 compat mode
Browse files Browse the repository at this point in the history
  • Loading branch information
brettsam committed Nov 26, 2019
1 parent 975b436 commit c6942f2
Show file tree
Hide file tree
Showing 31 changed files with 399 additions and 168 deletions.
24 changes: 24 additions & 0 deletions src/WebJobs.Script.WebHost/Configuration/HttpBodyControlOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

using Microsoft.Azure.WebJobs.Hosting;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;

namespace Microsoft.Azure.WebJobs.Script.WebHost.Configuration
{
internal class HttpBodyControlOptions : IOptionsFormatter
{
public bool AllowSynchronousIO { get; set; }

public string Format()
{
var options = new JObject
{
{ nameof(AllowSynchronousIO), AllowSynchronousIO }
};

return options.ToString(Formatting.Indented);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

using Microsoft.Azure.WebJobs.Script.Config;
using Microsoft.Extensions.Options;

namespace Microsoft.Azure.WebJobs.Script.WebHost.Configuration
{
internal class HttpBodyControlOptionsSetup : IConfigureOptions<HttpBodyControlOptions>
{
private readonly IEnvironment _environment;

public HttpBodyControlOptionsSetup(IEnvironment environment)
{
_environment = environment;
}

public void Configure(HttpBodyControlOptions options)
{
options.AllowSynchronousIO = _environment.IsV2CompatibilityMode()
|| FeatureFlags.IsEnabled(ScriptConstants.FeatureFlagAllowSynchronousIO, _environment);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;

namespace Microsoft.Azure.WebJobs.Script.WebHost.Middleware
{
public class AllowSynchronousIOMiddleware
{
private readonly RequestDelegate _next;

public AllowSynchronousIOMiddleware(RequestDelegate next)
{
_next = next;
}

public async Task Invoke(HttpContext context)
{
var bodyControlFeature = context.Features.Get<IHttpBodyControlFeature>();
if (bodyControlFeature != null)
{
bodyControlFeature.AllowSynchronousIO = true;
}

await _next.Invoke(context);
}
}
}
3 changes: 0 additions & 3 deletions src/WebJobs.Script.WebHost/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ public static IWebHostBuilder CreateWebHostBuilder(string[] args = null)
.ConfigureKestrel(o =>
{
o.Limits.MaxRequestBodySize = 104857600;

// TODO: https://github.com/Azure/azure-functions-host/issues/4876
o.AllowSynchronousIO = true;
})
.UseSetting(WebHostDefaults.EnvironmentKey, Environment.GetEnvironmentVariable(EnvironmentSettingNames.EnvironmentNameKey))
.ConfigureServices(services =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public static void AddWebJobsScriptHost(this IServiceCollection services, IConfi
services.ConfigureOptions<StandbyOptionsSetup>();
services.ConfigureOptions<LanguageWorkerOptionsSetup>();
services.ConfigureOptionsWithChangeTokenSource<AppServiceOptions, AppServiceOptionsSetup, SpecializationChangeTokenSource<AppServiceOptions>>();
services.AddSingleton<IOptionsChangeTokenSource<CompatibilityModeOptions>, SpecializationChangeTokenSource<CompatibilityModeOptions>>();
services.ConfigureOptionsWithChangeTokenSource<HttpBodyControlOptions, HttpBodyControlOptionsSetup, SpecializationChangeTokenSource<HttpBodyControlOptions>>();

services.TryAddSingleton<IDependencyValidator, DependencyValidator>();
services.TryAddSingleton<IJobHostMiddlewarePipeline>(s => DefaultMiddlewarePipeline.Empty);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Microsoft.AspNetCore.Hosting;
using Microsoft.Azure.WebJobs.Extensions.Http;
using Microsoft.Azure.WebJobs.Script.Extensions;
using Microsoft.Azure.WebJobs.Script.WebHost.Configuration;
using Microsoft.Azure.WebJobs.Script.WebHost.Middleware;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;
Expand All @@ -23,6 +24,7 @@ public static IApplicationBuilder UseWebJobsScriptHost(this IApplicationBuilder
{
IEnvironment environment = builder.ApplicationServices.GetService<IEnvironment>() ?? SystemEnvironment.Instance;
IOptionsMonitor<StandbyOptions> standbyOptions = builder.ApplicationServices.GetService<IOptionsMonitor<StandbyOptions>>();
IOptionsMonitor<HttpBodyControlOptions> httpBodyControlOptions = builder.ApplicationServices.GetService<IOptionsMonitor<HttpBodyControlOptions>>();

builder.UseMiddleware<SystemTraceMiddleware>();
builder.UseMiddleware<HostnameFixupMiddleware>();
Expand All @@ -33,6 +35,13 @@ public static IApplicationBuilder UseWebJobsScriptHost(this IApplicationBuilder
builder.UseMiddleware<PlaceholderSpecializationMiddleware>();
}

// Specialization can change the CompatMode setting, so this must run later than
// the PlaceholderSpecializationMiddleware
builder.UseWhen(context => httpBodyControlOptions.CurrentValue.AllowSynchronousIO, config =>
{
config.UseMiddleware<AllowSynchronousIOMiddleware>();
});

// This middleware must be registered before we establish the request service provider.
builder.UseWhen(context => !context.Request.IsAdminRequest(), config =>
{
Expand Down
10 changes: 0 additions & 10 deletions src/WebJobs.Script/Config/CompatibilityModeOptions.cs

This file was deleted.

22 changes: 0 additions & 22 deletions src/WebJobs.Script/Config/CompatibilityModeOptionsSetup.cs

This file was deleted.

6 changes: 4 additions & 2 deletions src/WebJobs.Script/Config/FeatureFlags.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ namespace Microsoft.Azure.WebJobs.Script.Config
/// </summary>
public static class FeatureFlags
{
public static bool IsEnabled(string name)
public static bool IsEnabled(string name) => IsEnabled(name, SystemEnvironment.Instance);

public static bool IsEnabled(string name, IEnvironment environment)
{
string featureFlags = Environment.GetEnvironmentVariable("AzureWebJobsFeatureFlags");
string featureFlags = environment.GetEnvironmentVariable(EnvironmentSettingNames.AzureWebJobsFeatureFlags);
if (!string.IsNullOrEmpty(featureFlags))
{
string[] flags = featureFlags.Split(',');
Expand Down
1 change: 1 addition & 0 deletions src/WebJobs.Script/Environment/EnvironmentSettingNames.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public static class EnvironmentSettingNames
public const string AzureFilesContentShare = "WEBSITE_CONTENTSHARE";
public const string AzureWebsiteRuntimeSiteName = "WEBSITE_DEPLOYMENT_ID";
public const string FunctionsRuntimeScaleMonitoringEnabled = "FUNCTIONS_RUNTIME_SCALE_MONITORING_ENABLED";
public const string AzureWebJobsFeatureFlags = "AzureWebJobsFeatureFlags";

/// <summary>
/// Environment variable dynamically set by the platform when it is safe to
Expand Down
1 change: 1 addition & 0 deletions src/WebJobs.Script/ScriptConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ public static class ScriptConstants
public const string FeatureFlagDisableShadowCopy = "DisableShadowCopy";
public const string FeatureFlagsEnableDynamicExtensionLoading = "EnableDynamicExtensionLoading";
public const string FeatureFlagEnableActionResultHandling = "EnableActionResultHandling";
public const string FeatureFlagAllowSynchronousIO = "AllowSynchronousIO";

public const string AdminJwtValidAudienceFormat = "https://{0}.azurewebsites.net/azurefunctions";
public const string AdminJwtValidIssuerFormat = "https://{0}.scm.azurewebsites.net";
Expand Down
2 changes: 0 additions & 2 deletions src/WebJobs.Script/ScriptHostBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,6 @@ public static IHostBuilder AddScriptHostCore(this IHostBuilder builder, ScriptAp

public static void AddCommonServices(IServiceCollection services)
{
services.ConfigureOptions<CompatibilityModeOptionsSetup>();

// The scope for these services is beyond a single host instance.
// They are not recreated for each new host instance, so you have
// to be careful with caching, etc. E.g. these services will get
Expand Down
12 changes: 7 additions & 5 deletions test/TestFunctions/Function1.cs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
using System.IO;
using System;
using System.IO;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Azure.WebJobs;
using Microsoft.Azure.WebJobs.Extensions.Http;
using Microsoft.AspNetCore.Http;
using Microsoft.Azure.WebJobs.Host;
using Newtonsoft.Json;
using System;
using Microsoft.Extensions.Logging;
using Newtonsoft.Json;

namespace TestFunctions
{
Expand All @@ -24,7 +23,10 @@ public static IActionResult Run([HttpTrigger(AuthorizationLevel.Function, "get",
}

string name = req.Query["name"];

// Explicitly read the body synchronously; this is used in the AllowSynchronousIOMiddleware tests
string requestBody = new StreamReader(req.Body).ReadToEnd();

dynamic data = JsonConvert.DeserializeObject(requestBody);
name = name ?? data?.name;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public virtual async Task InitializeAsync()
ConfigureWebHostBuilder(webHostBuilder);

// TODO: https://github.com/Azure/azure-functions-host/issues/4876
HttpServer = new TestServer(webHostBuilder) { AllowSynchronousIO = true };
HttpServer = new TestServer(webHostBuilder);

HttpClient = HttpServer.CreateClient();
HttpClient.BaseAddress = new Uri("https://localhost/");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public async Task IsPlaceholderMode_ThroughoutInitialization_EvaluatesCorrectly(
});

// TODO: https://github.com/Azure/azure-functions-host/issues/4876
var server = new TestServer(builder) { AllowSynchronousIO = true };
var server = new TestServer(builder);
var client = server.CreateClient();

// Force the specialization middleware to run
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ protected async Task InitializeTestHostAsync(string testDirName, IEnvironment en
});

// TODO: https://github.com/Azure/azure-functions-host/issues/4876
_httpServer = new TestServer(webHostBuilder) { AllowSynchronousIO = true };
_httpServer = new TestServer(webHostBuilder);
_httpClient = _httpServer.CreateClient();
_httpClient.BaseAddress = new Uri("https://localhost/");

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Text;
using System.Threading.Tasks;
using Microsoft.Azure.WebJobs.Script.WebHost;
using Microsoft.Extensions.DependencyInjection;
using Newtonsoft.Json.Linq;
using Xunit;

namespace Microsoft.Azure.WebJobs.Script.Tests.Integration.Middleware
{
public class AllowSynchronousIOMiddlewareTests
{
[Fact]
public async Task SyncRead_Fails_ByDefault()
{
using (var host = GetHost())
{
HostSecretsInfo secrets = await host.SecretManager.GetHostSecretsAsync();
var response = await MakeRequest(host.HttpClient, secrets.MasterKey);

Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);

var a = host.GetScriptHostLogMessages().Select(p => p.Exception?.ToString());

Assert.Contains(host.GetScriptHostLogMessages(), p => p.Exception != null && p.Exception.ToString().Contains("Synchronous operations are disallowed."));
}
}

[Fact]
public async Task SyncRead_Succeeds_WithFlag()
{
using (var host = GetHost(d => d.Add(EnvironmentSettingNames.AzureWebJobsFeatureFlags, ScriptConstants.FeatureFlagAllowSynchronousIO)))
{
HostSecretsInfo secrets = await host.SecretManager.GetHostSecretsAsync();
var response = await MakeRequest(host.HttpClient, secrets.MasterKey);

response.EnsureSuccessStatusCode();
}
}

[Fact]
public async Task SyncRead_Succeeds_InV2CompatMode()
{
using (var host = GetHost(d => d.Add(EnvironmentSettingNames.FunctionsV2CompatibilityModeKey, "true")))
{
HostSecretsInfo secrets = await host.SecretManager.GetHostSecretsAsync();
var response = await MakeRequest(host.HttpClient, secrets.MasterKey);

response.EnsureSuccessStatusCode();

var content = await response.Content.ReadAsStringAsync();
}
}

private static TestFunctionHost GetHost(Action<IDictionary<string, string>> addEnvironmentVariables = null)
{
string scriptPath = @"TestScripts\DirectLoad\";
string logPath = Path.Combine(Path.GetTempPath(), @"Functions");

var host = new TestFunctionHost(scriptPath, logPath,
configureWebHostServices: s =>
{
IDictionary<string, string> dict = new Dictionary<string, string>();
addEnvironmentVariables?.Invoke(dict);
s.AddSingleton<IEnvironment>(_ => new TestEnvironment(dict));
});

return host;
}

private static Task<HttpResponseMessage> MakeRequest(HttpClient client, string masterKey)
{
var request = new HttpRequestMessage
{
RequestUri = new Uri(string.Format($"http://localhost/api/function1?code={masterKey}&name=brett")),
Method = HttpMethod.Post
};

var input = new JObject
{
{ "scenario", "syncRead" }
};

request.Content = new StringContent(input.ToString(), Encoding.UTF8, "application/json");

return client.SendAsync(request);
}
}
}
Loading

0 comments on commit c6942f2

Please sign in to comment.