From d284f15d9ba1b8b4945ed74b1918909c3b0e5fb1 Mon Sep 17 00:00:00 2001 From: David Fowler <davidfowl@gmail.com> Date: Tue, 7 Sep 2021 11:13:52 -0700 Subject: [PATCH] Fix how host configuration is handled in WebApplicationBuilder (#36186) (#36233) - The new WebApplicationBuilder merged host configuration and application configuration together into the same configuration source, then applied those changes to the final all as host configuration. This had some bad side effects: - All configuration changes are wrapped in a chained configuration source which prevented updates (that's a runtime bug) - Host configuration could be mutated by changing any value directly in configuration. This meant that different parts of the code base could see a different environment, application name and content root. This change fixes things by snapshotting the immutable host configuration in the constructor and applying it later. Then only applying configuration sources to application configuration, not host configuration. - Added tests --- .../src/BootstrapHostBuilder.cs | 4 +- .../src/WebApplicationBuilder.cs | 20 ++- .../WebApplicationTests.cs | 149 ++++++++++++++---- 3 files changed, 139 insertions(+), 34 deletions(-) diff --git a/src/DefaultBuilder/src/BootstrapHostBuilder.cs b/src/DefaultBuilder/src/BootstrapHostBuilder.cs index 6d863d7db5c..f43a61e5aa1 100644 --- a/src/DefaultBuilder/src/BootstrapHostBuilder.cs +++ b/src/DefaultBuilder/src/BootstrapHostBuilder.cs @@ -93,7 +93,7 @@ namespace Microsoft.AspNetCore.Hosting return this; } - public HostBuilderContext RunDefaultCallbacks(ConfigurationManager configuration, HostBuilder innerBuilder) + public (HostBuilderContext, ConfigurationManager) RunDefaultCallbacks(ConfigurationManager configuration, HostBuilder innerBuilder) { var hostConfiguration = new ConfigurationManager(); @@ -146,7 +146,7 @@ namespace Microsoft.AspNetCore.Hosting callback(innerBuilder); } - return hostContext; + return (hostContext, hostConfiguration); } private class HostingEnvironment : IHostEnvironment diff --git a/src/DefaultBuilder/src/WebApplicationBuilder.cs b/src/DefaultBuilder/src/WebApplicationBuilder.cs index e9a0f1a1417..b60b958fd53 100644 --- a/src/DefaultBuilder/src/WebApplicationBuilder.cs +++ b/src/DefaultBuilder/src/WebApplicationBuilder.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; -using System.Reflection; using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; @@ -19,6 +18,7 @@ namespace Microsoft.AspNetCore.Builder private readonly HostBuilder _hostBuilder = new(); private readonly BootstrapHostBuilder _bootstrapHostBuilder; private readonly WebApplicationServiceCollection _services = new(); + private readonly List<KeyValuePair<string, string>> _hostConfigurationValues; private const string EndpointRouteBuilderKey = "__EndpointRouteBuilder"; private WebApplication? _builtApplication; @@ -81,11 +81,16 @@ namespace Microsoft.AspNetCore.Builder _services.TrackHostedServices = true; // This is the application configuration - var hostContext = _bootstrapHostBuilder.RunDefaultCallbacks(Configuration, _hostBuilder); + var (hostContext, hostConfiguration) = _bootstrapHostBuilder.RunDefaultCallbacks(Configuration, _hostBuilder); // Stop tracking here _services.TrackHostedServices = false; + // Capture the host configuration values here. We capture the values so that + // changes to the host configuration have no effect on the final application. The + // host configuration is immutable at this point. + _hostConfigurationValues = new(hostConfiguration.AsEnumerable()); + // Grab the WebHostBuilderContext from the property bag to use in the ConfigureWebHostBuilder var webHostContext = (WebHostBuilderContext)hostContext.Properties[typeof(WebHostBuilderContext)]; @@ -134,8 +139,17 @@ namespace Microsoft.AspNetCore.Builder /// <returns>A configured <see cref="WebApplication"/>.</returns> public WebApplication Build() { - // Copy the configuration sources into the final IConfigurationBuilder + // Wire up the host configuration here. We don't try to preserve the configuration + // source itself here since we don't support mutating the host values after creating the builder. _hostBuilder.ConfigureHostConfiguration(builder => + { + builder.AddInMemoryCollection(_hostConfigurationValues); + }); + + // Wire up the application configuration by copying the sources over to final configuration builder. + // this will also contain host configuration since it's chained (unless the sources were cleared) + // but it can't affect the hosting configuration at this point so it's harmless. + _hostBuilder.ConfigureAppConfiguration(builder => { foreach (var source in ((IConfigurationBuilder)Configuration).Sources) { diff --git a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs index 05c848b67f5..ba6a2c2863f 100644 --- a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs +++ b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs @@ -1323,35 +1323,6 @@ namespace Microsoft.AspNetCore.Tests Assert.Equal("value", app.Configuration["testhostingstartup:config"]); } - public class TestHostingStartup : IHostingStartup - { - public void Configure(IWebHostBuilder builder) - { - builder - .ConfigureAppConfiguration((context, configurationBuilder) => configurationBuilder.AddInMemoryCollection( - new[] - { - new KeyValuePair<string,string>("testhostingstartup:config", "value") - })); - } - } - - class ThrowingStartupFilter : IStartupFilter - { - public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next) - { - return app => - { - app.Use((HttpContext context, RequestDelegate next) => - { - throw new InvalidOperationException("BOOM Filter"); - }); - - next(app); - }; - } - } - [Fact] public async Task DeveloperExceptionPageWritesBadRequestDetailsToResponseByDefaltInDevelopment() { @@ -1399,6 +1370,126 @@ namespace Microsoft.AspNetCore.Tests Assert.Equal(string.Empty, responseBody); } + [Fact] + public void HostConfigurationNotAffectedByConfiguration() + { + var builder = WebApplication.CreateBuilder(); + + var contentRoot = Path.GetTempPath(); + var webRoot = Path.GetTempPath(); + var envName = $"{nameof(WebApplicationTests)}_ENV"; + + builder.Configuration[WebHostDefaults.ApplicationKey] = nameof(WebApplicationTests); + builder.Configuration[WebHostDefaults.EnvironmentKey] = envName; + builder.Configuration[WebHostDefaults.ContentRootKey] = contentRoot; + + var app = builder.Build(); + var hostEnv = app.Services.GetRequiredService<IHostEnvironment>(); + var webHostEnv = app.Services.GetRequiredService<IWebHostEnvironment>(); + + Assert.Equal(builder.Environment.ApplicationName, hostEnv.ApplicationName); + Assert.Equal(builder.Environment.EnvironmentName, hostEnv.EnvironmentName); + Assert.Equal(builder.Environment.ContentRootPath, hostEnv.ContentRootPath); + + Assert.Equal(webHostEnv.ApplicationName, hostEnv.ApplicationName); + Assert.Equal(webHostEnv.EnvironmentName, hostEnv.EnvironmentName); + Assert.Equal(webHostEnv.ContentRootPath, hostEnv.ContentRootPath); + + Assert.NotEqual(nameof(WebApplicationTests), hostEnv.ApplicationName); + Assert.NotEqual(envName, hostEnv.EnvironmentName); + Assert.NotEqual(contentRoot, hostEnv.ContentRootPath); + } + + [Fact] + public void ClearingConfigurationDoesNotAffectHostConfiguration() + { + var builder = WebApplication.CreateBuilder(new WebApplicationOptions + { + ApplicationName = typeof(WebApplicationOptions).Assembly.FullName, + EnvironmentName = Environments.Staging, + ContentRootPath = Path.GetTempPath() + }); + + ((IConfigurationBuilder)builder.Configuration).Sources.Clear(); + + var app = builder.Build(); + var hostEnv = app.Services.GetRequiredService<IHostEnvironment>(); + var webHostEnv = app.Services.GetRequiredService<IWebHostEnvironment>(); + + Assert.Equal(builder.Environment.ApplicationName, hostEnv.ApplicationName); + Assert.Equal(builder.Environment.EnvironmentName, hostEnv.EnvironmentName); + Assert.Equal(builder.Environment.ContentRootPath, hostEnv.ContentRootPath); + + Assert.Equal(webHostEnv.ApplicationName, hostEnv.ApplicationName); + Assert.Equal(webHostEnv.EnvironmentName, hostEnv.EnvironmentName); + Assert.Equal(webHostEnv.ContentRootPath, hostEnv.ContentRootPath); + + Assert.Equal(typeof(WebApplicationOptions).Assembly.FullName, hostEnv.ApplicationName); + Assert.Equal(Environments.Staging, hostEnv.EnvironmentName); + Assert.Equal(Path.GetTempPath(), hostEnv.ContentRootPath); + } + + [Fact] + public void ConfigurationCanBeReloaded() + { + var builder = WebApplication.CreateBuilder(); + + ((IConfigurationBuilder)builder.Configuration).Sources.Add(new RandomConfigurationSource()); + + var app = builder.Build(); + + var value0 = app.Configuration["Random"]; + ((IConfigurationRoot)app.Configuration).Reload(); + var value1 = app.Configuration["Random"]; + + Assert.NotEqual(value0, value1); + } + + public class RandomConfigurationSource : IConfigurationSource + { + public IConfigurationProvider Build(IConfigurationBuilder builder) + { + return new RandomConfigurationProvider(); + } + } + + public class RandomConfigurationProvider : ConfigurationProvider + { + public override void Load() + { + Data["Random"] = Guid.NewGuid().ToString(); + } + } + + public class TestHostingStartup : IHostingStartup + { + public void Configure(IWebHostBuilder builder) + { + builder + .ConfigureAppConfiguration((context, configurationBuilder) => configurationBuilder.AddInMemoryCollection( + new[] + { + new KeyValuePair<string,string>("testhostingstartup:config", "value") + })); + } + } + + class ThrowingStartupFilter : IStartupFilter + { + public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next) + { + return app => + { + app.Use((HttpContext context, RequestDelegate next) => + { + throw new InvalidOperationException("BOOM Filter"); + }); + + next(app); + }; + } + } + class PropertyFilter : IStartupFilter { public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next) -- GitLab