dotnet/runtime

Allow access to child providers in ChainedConfigurationProvider

avanigupta opened this issue · 10 comments

API Proposal

This API proposal provides the ability to enumerate a ChainedConfigurationProvider's child providers:

namespace Microsoft.Extensions.Configuration
{
    public class ChainedConfigurationProvider : IConfigurationProvider, IDisposable
    {
+        /// <summary>
+        /// The chained <see cref="IConfigurationProvider"/> instances.
+        /// </summary>
+        IEnumerable<IConfigurationProvider>? Providers { get; }

Motivation

Developers tend to add their configuration source via a ChainedConfigurationSource/Provider and they would want to access it from the outer ConfigurationRoot.

ConfigurationRoot exposed Providers. With this new API now, configuration providers that are nested under a ChainedConfigurationProvider if the user calls IConfigurationBulder.AddConfiguration(...) would now also be able to be accessed.

Usage

Sample usage can be found in AzureAppConfigurationRefresherProvider under the Azure/AppConfiguration-DotnetProvider repo.

Original Description

We have an issue in Azure App Configuration where AzureAppConfigurationProvider could be nested under a ChainedConfigurationProvider if the user calls IConfigurationBuilder.AddConfiguration(...) to add configuration built from the App Configuration provider.

Sample code snippet of this use case:

public static IWebHost BuildWebHost(string[] args)
{
    var configBuilder = new ConfigurationBuilder();
    IConfiguration configuration = configBuilder.AddAzureAppConfiguration(options =>
    {
        options.Connect(connectionString)
        .ConfigureRefresh(refreshOptions =>
        {
            refreshOptions.Register("Settings:BackgroundColor", refreshAll: true)
                          .SetCacheExpiration(TimeSpan.FromSeconds(10));
        });
    })
    .Build();

    return WebHost.CreateDefaultBuilder(args)
                  .ConfigureAppConfiguration((hostingContext, config) =>
                  {
                      config.AddJsonFile("appsettings.json").Build();
                      config.AddConfiguration(configuration);
                  })
                  .UseStartup<Startup>()
                  .Build();
}

Unfortunately, ChainedConfigurationProvider does not provide a way for others to access its child providers. This caused the issue that the App Configuration middleware could not find the instance of AzureAppConfigurationProvider (because in this case its under ChainedConfigurationProvider) and thus cannot obtain the instance of IConfigurationRefresher, which is responsible for refreshing the application configuration (our relevant middleware code here).

Would it be possible to expose the underlying providers in ChainedConfigurationProvider so that we can extract AzureAppConfigurationProvider and enable users to leverage our dynamic refresh functionality?

Tagging subscribers to this area: @maryamariyan
See info in area-owners.md if you want to be subscribed.


Issue meta data
Issue content: We have an [issue](https://github.com/Azure/AppConfiguration-DotnetProvider/issues/168) in Azure App Configuration where `AzureAppConfigurationProvider` could be nested under a `ChainedConfigurationProvider` if the user calls `IConfigurationBuilder.AddConfiguration(...)` to add configuration built from the App Configuration provider.

Sample code snippet of this use case:

public static IWebHost BuildWebHost(string[] args)
{
    var configBuilder = new ConfigurationBuilder();
    IConfiguration configuration = configBuilder.AddAzureAppConfiguration(options =>
    {
        options.Connect(connectionString)
        .ConfigureRefresh(refreshOptions =>
        {
            refreshOptions.Register("Settings:BackgroundColor", refreshAll: true)
                          .SetCacheExpiration(TimeSpan.FromSeconds(10));
        });
    })
    .Build();

    return WebHost.CreateDefaultBuilder(args)
                  .ConfigureAppConfiguration((hostingContext, config) =>
                  {
                      config.AddJsonFile("appsettings.json").Build();
                      config.AddConfiguration(configuration);
                  })
                  .UseStartup<Startup>()
                  .Build();
}

Unfortunately, ChainedConfigurationProvider does not provide a way for others to access its child providers. This caused the issue that the App Configuration middleware could not find the instance of AzureAppConfigurationProvider (because in this case its under ChainedConfigurationProvider) and thus cannot obtain the instance of IConfigurationRefresher, which is responsible for refreshing the application configuration (our relevant middleware code here).

Would it be possible to expose the underlying providers in ChainedConfigurationProvider so that we can extract AzureAppConfigurationProvider and enable users to leverage our dynamic refresh functionality?

Issue author: avanigupta
Assignees: -
Milestone: -

With the upcoming .NET 6 release, when WebApplicationBuilder.Build() is called, all configuration providers will be consolidated under ChainedConfigurationProvider by default.
As a result, .NET 6 applications will not be able to use AzureAppConfigurationProvider for dynamically refreshing their config. We need to be able to access child providers under ChainedConfigurationProvider to address this issue. Is there any update on when this will be possible?

@avanigupta are you still blocked by this or you already have the workaround?

/cc @halter73

@maryamariyan , we are still blocked since there's no clean workaround for this.

With the upcoming .NET 6 release, when WebApplicationBuilder.Build() is called, all configuration providers will be consolidated under ChainedConfigurationProvider by default.

I investigated this with @avanigupta before the 6.0 release and we confirmed that this is no longer the case in the final release. However, I realize the original issue wasn't about a regression in 6.0, but instead was asking for the ability to enumerate a ChainedConfigurationProvider's child providers which likely would have avoided the regression we saw in preview releases.

I think the proposal is basically:

namespace Microsoft.Extensions.Configuration
{
    public class ChainedConfigurationProvider : IConfigurationProvider, IDisposable
    {
+        /// <summary>
+        /// The chained <see cref="IConfigurationProvider"/> instances.
+        /// </summary>
+        IEnumerable<IConfigurationProvider> Providers { get; }

And then the usage in AzureAppConfigurationRefresherProvider would be updated like this:

// This code is from the ctor of a singleton service that injects IConfiguration configuration.

var configurationRoot = configuration as IConfigurationRoot;
var refreshers = new List<IConfigurationRefresher>();

if (configurationRoot != null)
{
+   void AddRefreshers(IEnumerable<IConfigurationProvider> providers)
+   {
-   foreach (IConfigurationProvider provider in configurationRoot.Providers)
+   foreach (IConfigurationProvider provider in providers)
    {
        if (provider is IConfigurationRefresher refresher)
        {
            // Use _loggerFactory only if LoggerFactory hasn't been set in AzureAppConfigurationOptions
            if (refresher.LoggerFactory == null)
            {
                refresher.LoggerFactory = _loggerFactory;
            }

            refreshers.Add(refresher);
        }
+       else if (provider is ChainedConfigurationProvider chainedProvider)
+       {
+           AddRefreshers(chainedProvider.Providers);
+       }
    }
+   }
+
+   AddRefreshers(configurationRoot.Providers);
}

I didn't bother reindenting the code now in the local void AddRefreshers(IEnumerable<IConfigurationProvider> providers) function to make the diff more readable.

This is being done so a custom service can reference the configuration providers added by the call to AddAzureAppConfiguration. At the point configuration is added, no services exist yet to register the providers with and the IConfigurationBuilder does not give any sort of access to the IServiceCollection.

In an ideal world, there would be a single API to both add the AzureAppConfigurationSource to the IConfigurationBuilder and add service descriptors to the IServiceCollection that reference the added AzureAppConfigurationSource and any AzureAppConfigurationProviders it creates. This would work even if someone created their own version of ChainedConfigurationProvider unlike the API proposed above.

@avanigupta What happens if someone tries to call AddAzureAppConfiguration on a ConfigurationBuilder they've newed up themselves that never chain it to the IConfigurationBuilder provided by Host or WebHost? Would config never get refreshed because there are no services?

Google took me here, I have a similar issue.

in Program.cs I build my IConfiguration, then I pass it into WebHostBuilder()UseConfiguration(config) that uses "UseStartup().
I inject the IConfiguration into the startup constructor, and it too loses all the providers, and is replaced by a single ChainedConfigurationProvider.

I am on Core 3.1, and It fails to register the Azure Configurations too, at IApplicationBuilder.UseAzureAppConfiguration()

Is there a workaround for my scenario?

The easiest workaround probably would be to build your configuration using the IConfigurationBuilder in IWebHostBuilder.ConfigureAppConfiguration(Action<WebHostBuilderContext, IConfigurationBuilder> configureDelegate).

Or, if you can upgrade to .NET 6, you could use WebApplicationBuilder.Configuration ConfigurationManager.

Thanks, @halter73 for that. Both are good solutions for us (not perfect), as a short term solution I applied your first suggestion and refactored my own code to this:

          await new WebHostBuilder()
                 .ConfigureAppConfiguration((context, configBuilder) => { configBuilder.AddConfigs(); })

.AddConfigs() is the custom method into which I moved all of our IConfigurationBuilder additions and ended up calling this AddConfigs() twice.
Hopefully, the Azure Configurations integration (IConfigurationBuilder.AddAzureAppConfiguration) doesn't pull configs twice - yet to be tested.

I was able to resolve it without duplicating the builder code, by configuring the DI using ConfigureServices and passing the instance in:

new WebHostBuilder()
     .ConfigureServices(services => { services.AddSingleton<IConfiguration>(config); })
     .UseStartup<Startup>()

That resulted in getting the ConfigurationRoot with its providers correctly.

Video

During discussion it was believed that exposing the IConfiguration directly instead of inventing the IEnumerable for it was a better approach.

namespace Microsoft.Extensions.Configuration
{
    public partial class ChainedConfigurationProvider : IConfigurationProvider, IDisposable
    {
        public IConfiguration Configuration { get; }
    }
}