Make kestrel limits configurable

[eduherminio]

• Allow Kestrel limits options to be overridden with values of config files and environment variables (see related Microsoft documentation).

[StefH]

Hello @eduherminio, I understand what you want to do, however adding this code currently breaks the CI because the appsettings.json is required, but is not found.

And currently, setting the Kestrel options for netstandard 2.x and netstandard1.3 is done in (https://github.com/WireMock-Net/WireMock.Net/blob/master/src/WireMock.Net/Owin/AspNetCoreSelfHost.NETStandard.cs) and (https://github.com/WireMock-Net/WireMock.Net/blob/master/src/WireMock.Net/Owin/AspNetCoreSelfHost.NETStandard13.cs).

So in case you want to override the KestrelServerLimits, this is the place to do it.

And I think three more settings which should be set to a higher value:

• options.Limits.MaxRequestHeadersTotalSize = ???; // default = 32768
• options.Limits.MaxRequestLineSize = ???; // default = 8192
• options.Limits.MaxRequestHeaderCount = ???; // default = 100

However the question is if all these settings should be made configurable via WireMockSettings or just set to a large enough value so that users from WireMock.Net are not limited by the implementation.
So in this case, setting the MaxRequestHeadersTotalSize to a higher value (64KB?) should solve your current problem.

[codecov]

Codecov Report

Merging #520 into master will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #520      +/-   ##
==========================================
+ Coverage   76.50%   76.57%   +0.07%     
==========================================
  Files         135      135              
  Lines        5248     5264      +16     
  Branches      517      518       +1     
==========================================
+ Hits         4015     4031      +16     
  Misses       1079     1079              
  Partials      154      154              

Impacted Files	Coverage Δ
...ireMock.Net/Owin/AspNetCoreSelfHost.NETStandard.cs	78.78% <100.00%> (+12.12%)	⬆️
src/WireMock.Net/Owin/AspNetCoreSelfHost.cs	85.88% <100.00%> (+0.51%)	⬆️
src/WireMock.Net/Serialization/MatcherMapper.cs	83.11% <0.00%> (+0.22%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 477f3b5...e527868. Read the comment docs.

[eduherminio]

Hi @StefH!

My idea was to try to provide a mechanism so that users of the container images can modify those limits without having to maintain both a fork of the project and the relevant image, by overriding the values in AspNetCoreSelfHost.NETStandard13.cs/AspNetCoreSelfHost.NETStandard.cs using env variables (note that the added ConfigureServices block comes after the .UseKestrel one).
For this use case, config files aren't relevant, so the config-related code could be changed to just use environment variables.
This is my company's case right now.

The same idea would apply to users of the library who need to modify those limits and don't want to maintain a fork of the project.
For that use case, both files can be made optional, which also satisfies previous use case.

Having said that, I respect that you don't like that approach and would be happy to have MaxRequestHeadersTotalSize increased.
We'd need it to be at least 144KB, but the issue with setting a number here is that tomorrow someone else (or ourselves) may come and ask you to rise it again.

I also agree that MaxRequestLineSize, MaxRequestHeaderCount and some fields inside of options.Limits.Http2 could be increased as well, but for now we haven't faced any related issue.

[StefH]

OK I see your point.

To make the main code (AspNetCoreSelfHost.cs) cleaner and use less #if / #endif, propose the following:

1. Make an extension method in the two separate files (AspNetCoreSelfHost.NETStandard.cs & AspNetCoreSelfHost.NETStandard13.cs) with name ConfigureAppConfigurationUsingEnvironmentVariables which for netstandard behaves like you created, except that only environment variables are added, so no json files.

For the .netstandard1.3, there is no implementation, so just an empty extension method.

2. Also create a extension method ConfigureKestrelServerOptions which should do the same as you created (services.Configure<Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions>( context.Configuration.GetSection("Kestrel")); for netstandard. For netstandard13, this method is empty again.

3. Should the UseKestrel be moved up? Because the values here will overwrite the settings you have configured using environment?

4. A new Wiki page should be added which explains all KestrelServerLimits properties which can be overwritten with an environment setting and it should also be explained how this environment setting should look like.

[eduherminio]

I've made the requested changes.
You can find a wiki page first draft here.

Note that:

• .UseKestrel() needs to be invoked before .ConfigureKestrelServerOptions(), but it doesn't modify IConfiguration so the current invocation order works fine and makes sense IMO.
• I believe .ConfigureKestrelServerOptions() could theoretically be implemented for .NET Standard 1.3 (if needed) by creating an IConfiguration instance (using ConfigurationBuilder) on the fly, inside of the method. Something like:

        internal static IWebHostBuilder ConfigureKestrelServerOptions(this IWebHostBuilder builder)
        {
            var configuration = new ConfigurationBuilder()
                .AddEnvironmentVariables()
                .Build();

            return builder.ConfigureServices(services =>
            {
                services.Configure<KestrelServerOptions>(configuration.GetSection("Kestrel"));
            });
        }

[StefH]

Looks good I think.

However if you think that your comment is correct:

I believe .ConfigureKestrelServerOptions() could theoretically be implemented for .NET Standard 1.3 (if needed) by creating an IConfiguration instance (using ConfigurationBuilder) on the fly, inside of the method. Something like:

    internal static IWebHostBuilder ConfigureKestrelServerOptions(this IWebHostBuilder builder)
    {
        var configuration = new ConfigurationBuilder()
            .AddEnvironmentVariables()
            .Build();

        return builder.ConfigureServices(services =>
        {
            services.Configure<KestrelServerOptions>(configuration.GetSection("Kestrel"));
        });
    }

Then you can apply this code also for netstandard 1.3

[StefH]

Funny ;-)

Implementations in an interface , first time I see this in a real project ;-)

[StefH]

@eduherminio
Code looks good, I can merge it.

However, can you test it using a MyGet preview version? Or a real NuGet and Docker?

[eduherminio]

I'm happy to test a MyGet preview version later on the day using Docker.
Can you run your CI pipeline manually over this branch, so that a package is pushed to MyGet?

[StefH]

MyGet version is WireMock.Net.1.3.2-ci-13842.nupkg

I don't have Docker previews, sorry. Maybe later today I can try to make a 1.3.3-preview-01 NuGet + Docker.

[eduherminio]

Successfully tested by:

• Building sheyenrath/wiremock.net locally (both .NET Core 2.2 and .NET Core 3.1 versions) using the MyGet version.

diff --git a/StandAlone.NETCoreApp/Dockerfile b/StandAlone.NETCoreApp/Dockerfile
index b41e637..a60e0f0 100644
--- a/StandAlone.NETCoreApp/Dockerfile
+++ b/StandAlone.NETCoreApp/Dockerfile
@@ -6,6 +6,7 @@ WORKDIR /app

 # copy csproj and restore as distinct layers
 COPY StandAlone.NETCoreApp.csproj ./
+COPY nuget.config ./^M
 RUN dotnet restore

 # copy everything else and build
diff --git a/StandAlone.NETCoreApp/StandAlone.NETCoreApp.csproj b/StandAlone.NETCoreApp/StandAlone.NETCoreApp.csproj
index 8def001..00f6587 100644
--- a/StandAlone.NETCoreApp/StandAlone.NETCoreApp.csproj
+++ b/StandAlone.NETCoreApp/StandAlone.NETCoreApp.csproj
@@ -7,7 +7,7 @@
   </PropertyGroup>

   <ItemGroup>
-    <PackageReference Include="WireMock.Net" Version="1.3.1" />
+    <PackageReference Include="WireMock.Net" Version="1.3.2-ci-13842" />
   </ItemGroup>

 </Project>
\ No newline at end of file
diff --git a/StandAlone.NETCoreApp/nuget.config b/StandAlone.NETCoreApp/nuget.config
new file mode 100644
index 0000000..55e3e1d
--- /dev/null
+++ b/StandAlone.NETCoreApp/nuget.config
@@ -0,0 +1,6 @@
+<EF><BB><BF><?xml version="1.0" encoding="utf-8"?>
+<configuration>
+  <packageSources>
+    <add key="MyGet" value="https://www.myget.org/F/wiremock-net/api/v3/index.json" />
+  </packageSources>
+</configuration>
\ No newline at end of file

• Verifying that using --env Kestrel__Limits__MaxRequestHeadersTotalSize=153600 in docker run commands modifies the Kestrel limits as expected.

From my point of view it's good to go 😃 😃
Feel free to use the Wiki page draft and modify it as needed.

[StefH]

Thank you. I'll merge this one and create a new NuGet + docker.

And sure: I'll use your wiki! Thanks.