azure-webjobs-sdk icon indicating copy to clipboard operation
azure-webjobs-sdk copied to clipboard

WebJobsHostBuilderExtensions.ConfigureWebJobs should not add configuration sources

Open Skovvart opened this issue 6 years ago • 45 comments

When configuring the HostBuilder the order of ConfigureWebJobs and ConfigureAppConfiguration is significant, as ConfigureWebJobs adds "appsettings.json" and environment variables as configuration sources. This can lead to invalid configuration values and unexpected behavior, as the order of configuration sources defined in ConfigureAppConfiguration is not the definitive list of configuration sources.

Repro steps

  1. Create a new HostBuilder, call ConfigureAppConfiguration with one or more configuration sources that has values that should take precedence over appsettings.json and/or environment variables

  2. Call ConfigureWebJobs on the host builder

  3. Access a configuration value (example: in hostBuilder.ConfigureServices((context, services) => { var configValue = context.Configuration["ExampleValue"]; })

  4. Build and run the Host.

Expected behavior

ConfigureWebJobs should not manipulate configuration sources at all, or at the very least have an option to disable the behavior. Configuration values should appear as defined in ConfigureAppConfiguration.

Actual behavior

The rogue configuration sources are added to the configuration sources and may provide invalid configuration values.

Known workarounds

Call ConfigureWebJobs before ConfigureAppConfiguration. The rogue application sources are still added, but the desired sources takes precedence.

Related information

  • Package version: 3.0.0
  • Links to source: https://github.com/Azure/azure-webjobs-sdk/blob/85bff2b622f172f5792685cc9eca5c14cf284554/src/Microsoft.Azure.WebJobs.Host/Hosting/WebJobsHostBuilderExtensions.cs#L28-L32

Skovvart avatar Sep 26 '18 16:09 Skovvart

To tack on here related but slightly different (given minimal docs) how do you determine the environment such that you can load an environment specific appsettings.json ?

acds avatar Oct 11 '18 00:10 acds

I'd say the general-purpose answer would be var environment = System.Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") ?? "Development" (or another appropriate settings and default), but I wouldn't say it's really related.

Skovvart avatar Oct 11 '18 21:10 Skovvart

What's the official stance on this? This behavior is clearly incorrect, as it silently overrides the user defined configuration

thomaslevesque avatar Feb 13 '19 23:02 thomaslevesque

@fabiocav should weigh in on this as well. If I recall, the intention was for this to be similar to WebHost.CreateDefaultBuilder() (https://github.com/aspnet/AspNetCore/blob/master/src/DefaultBuilder/src/WebHost.cs#L162-L184), where a bunch of "standard" wire-up is done for you and then you can overwrite those services/sources in subsequent calls. The idea was that you call this first, then go from there.

I agree that's not entirely clear, though. Perhaps there should be a CreateDefaultWebJobsBuilder() that makes it clear we're doing some initial wire-up -- and ConfigureWebJobs() should just do this?: https://github.com/Azure/azure-webjobs-sdk/blob/85bff2b622f172f5792685cc9eca5c14cf284554/src/Microsoft.Azure.WebJobs.Host/Hosting/WebJobsHostBuilderExtensions.cs#L34-L40

brettsam avatar Feb 22 '19 15:02 brettsam

Perhaps there should be a CreateDefaultWebJobsBuilder() that makes it clear we're doing some initial wire-up -- and ConfigureWebJobs() should just do this?:

Sounds good to me!

thomaslevesque avatar Feb 24 '19 11:02 thomaslevesque

I just spent a day tracking down why my configuration does not work. I was adding Key Vault configuration and environment specific settings which seemed to be quite easy task but after deployment to Azure it didn't work. I really didn't expect that configuring webjobs add appsettings.json on top of my configuration effectively reverting all the specific changes

This is quite old issue, still not fixed, causing unnecessary waste of time and workarounds.. 👎

fhurta avatar May 03 '19 07:05 fhurta

I spent an evening looking at this in our service and filed what I guess is a duplicate bug here #2247

Is there a plan to fix this?

nathansoz avatar Jun 27 '19 05:06 nathansoz

I agree that the current behavior is not ideal and unexpected. We need to be careful though, as changing this behavior would be a breaking change (customers may be relying on the current behavior), so we need to introduce an API that is clear while preserving the current functionality.

@brettsam

If I recall, the intention was for this to be similar to WebHost.CreateDefaultBuilder()

Yes, that was the original design.

fabiocav avatar Jul 11 '19 21:07 fabiocav

I agree that it would be a breaking change but I can't really think of a scenario which you want this behavior so I would go for changing the current behavior, but that's just me ;-)

Zenuka avatar Jul 12 '19 06:07 Zenuka

@fabiocav You could introduce a new set of ConfigureWebJobs with a flag parameter, or add such parameter to the existing methods with default value (though this would be a breaking change). However, there is high risk that the flag will get unnoticed and everybody will face the issue at least initially. I had to re-implement this set of extension methods under ConfigureWebJobsWithoutConfig name as a workaround. The name is not ideal, and I would prefer the original ConfigureWebJobs had more cryptic name, and this one to be just ConfigureWebJobs.

andriysavin avatar Nov 20 '19 11:11 andriysavin

Just ran into this, after spending couple of hours troubleshooting the issues with configuration of my webjobs. Fixed just by placing ConfigureWebJobs method closer to the starting of my host configuration. The fact that any app configuration is taking place within ConfigureWebJobs method is really not intuitive. Any progress on this one? I definately support the idea of creating new CreateDefaultWebJobsBuilder() method, likewise the asp.net core. This is familiar in general, and also quite intuitive. And it guarantees that it is called first and so we have default app configuration go first, which can be overriden if needed. And on the other hand, since ConfigureWebJobs is extension method, so there's high probability that it is called after ConfigureAppConfiguration method, and this is breaking things.

aorlenko83 avatar Nov 22 '19 17:11 aorlenko83

I just ran into this issue as well. The offending lines in ConfigreWebJobs should just be removed. Best practices should be to call Host.CreateDefaultBuilder() before calling ConfigureWebJobs(). I'm using the WebJobs in a unique way for SimpleMessageBus, and because of this, I have to manually remove the registered ConfigurationProviders from the collection after the fact with code like this:

var configProviders = ((hostContext.Configuration as ConfigurationRoot).Providers as List<IConfigurationProvider>);

if (configProviders.Count(c => c.GetType() == typeof(JsonConfigurationProvider) && (c as JsonConfigurationProvider).Source.Path == "appsettings.json") > 1)
{
    configProviders.Remove(configProviders.Last(c => c.GetType() == typeof(JsonConfigurationProvider) && (c as JsonConfigurationProvider).Source.Path == "appsettings.json"));
}

if (configProviders.Count(c => c.GetType() == typeof(EnvironmentVariablesConfigurationProvider)) > 1)
{
    configProviders.Remove(configProviders.Last(c => c.GetType() == typeof(EnvironmentVariablesConfigurationProvider)));
}

Couldn't figure out why my code was breaking... it's entirely not obvious that this behavior is happening.

If you're concerned about a breaking change @fabiocav, then just do the opposite of my code above, and check to see if those providers are registered before adding them yourself. That way no one is broken, and the correct behavior just works.

robertmclaws avatar Nov 30 '19 22:11 robertmclaws

Here is how I remove the configuration sources added silently by ConfigureWebJobs() and add the configuration sources explicitly.

var hostBuilder = new HostBuilder();
            
hostBuilder.ConfigureWebJobs(b =>
    {
        b.AddAzureStorageCoreServices();
        b.AddAzureStorage();
    });

hostBuilder.ConfigureAppConfiguration(configurationBuilder =>
{
    // Replace the configuration sources added by ConfigureWebJobs()
    configurationBuilder.Sources.Clear();

    configurationBuilder.AddJsonFile("appsettings.json", false, true);
    configurationBuilder.AddJsonFile($"appsettings.{environment}.json", true, true);
    if (environment == ProgramEnvironment.Development)
        configurationBuilder.AddUserSecrets<Program>();
});

bassem-mf avatar Dec 06 '19 18:12 bassem-mf

@robertmclaws @bassem-mf , this hasn't been assigned, but I wanted to comment to make make you aware that your feedback is not being ignored :)

We have a plan to update this with the proper behavior (again, as mentioned above, the current behavior was not really intentional), but haven't picked this item up due to competing priorities. We'll have this assigned soon.

fabiocav avatar Dec 06 '19 18:12 fabiocav

Overwriting appsettings only with AddJsonFile("appsettings.json) is a bad idea indeed. It should not override anything or use both AddJsonFile("appsettings.json) and .AddJsonFile($"appsettings.{env.EnvironmentName}.json", like ConfigureWebJobs. Actually, I think having a CreateDefaultWebjobBuilder, analogous to CreateDefaultBuilder (with AddEnvironmentVariables, all the logging functionality, etc), it would be great.

taixes avatar Dec 11 '19 08:12 taixes

I am REALLY struggling with this same issue in Azure Functions for dot net core 3.x! When I run locally my local.settings.json is loaded and all is well. After I publish the Azure Function to Azure all of the sudden my custom settings are null? Here is the code I am currently using to load my configuration:

internal static IHost Configure(Action<IServiceCollection, IConfiguration> addtionalServices = null)
{
	return Host.CreateDefaultBuilder()
		.ConfigureAppConfiguration((context, builder) =>
		{
			builder.AddJsonFile("appsettings.json", optional: false);
			builder.AddJsonFile($"appsettings-{context.HostingEnvironment.EnvironmentName}.json", optional: true);
			builder.AddEnvironmentVariables();
		})
		.ConfigureServices((context, services) =>
		{
			//  Configure additional services and configurations
			addtionalServices?.Invoke(services, context.Configuration);
		})
		.Build();
}

I am using VS 2019 and dot net core 3.1 to author Azure Functions in C#. I have really struggled for most of 2 days with this issue! I ran extensive testing locally and then it just started losing my configuration settings after it was published. Thanks!

ScottyMac52 avatar Feb 11 '20 01:02 ScottyMac52

I FINALLY found a way to make this work. Many many many thanks to @bassem-mf for his solution created a light at the end of the tunnel!

internal static IHost Configure2(Action<IServiceCollection, IConfiguration> addtionalServices = null, ILogger logger = null, ExecutionContext context = null)
{
	var hostBuilder = new HostBuilder();
		hostBuilder.ConfigureWebJobs(b =>
	{
		b.AddAzureStorageCoreServices();
		b.AddAzureStorage((configureQueues) =>
		{
			configureQueues.MaxDequeueCount = 1;
		});
	});
	hostBuilder.ConfigureAppConfiguration(configurationBuilder =>
	{
		// Replace the configuration sources added by ConfigureWebJobs()
		configurationBuilder.Sources.Clear();
		var fileLocation = Path.Combine(context?.FunctionAppDirectory ?? Environment.CurrentDirectory, "appsettings.json");
		logger?.LogInformation($"Should load config from {fileLocation}");
		configurationBuilder.AddJsonFile(fileLocation, false, true);
	});
	hostBuilder.ConfigureServices((context, services) =>
	{
		//  Configure additional services and configurations
		addtionalServices?.Invoke(services, context.Configuration);
	});
	return hostBuilder.Build();
}

Now I KNOW that the appsettings.json file is loaded for my function app!

ScottyMac52 avatar Feb 11 '20 02:02 ScottyMac52

I use Host.CreateDefaultBuilder() to create my host, but a setting in my appSettings.json was overriding a setting in my appSettings.Production.json. Eventually I figured out that ConfigureWebJobs() also adds appSettings.json. This is pretty dumb -- ConfigureWebJobs() has no business touching my app configuration this way and the breaking change just needs to happen to fix this.

As a work-around, what I did was create a wrapping class around IHostBuilder which just forwards all work to the default implementation, except calls to ConfigureAppConfiguration() are just dropped. This allows me to allow ConfigureWebJobs() to do all it's work but filter out app configuration changes.

public static class WebJobsHostBuilderExtensions
{
    public static IHostBuilder ConfigureWebJobsWithoutAppConfiguration(
        this IHostBuilder hostBuilder,
        Action<IWebJobsBuilder> configure,
        Action<JobHostOptions> configureOptions = null)
    {
        if (hostBuilder is null)
        {
            throw new ArgumentNullException(nameof(hostBuilder));
        }

        IHostBuilder hostBuilderWithoutAppConfiguration = new HostBuilderWithoutAppConfiguration(hostBuilder);

        hostBuilderWithoutAppConfiguration.ConfigureWebJobs(
            configure ?? (webJobsBuilder => { }),
            configureOptions ?? (configureOptions => { }));

        return hostBuilder;
    }

    public static IHostBuilder ConfigureWebJobsWithoutAppConfiguration(
        this IHostBuilder hostBuilder,
        Action<HostBuilderContext, IWebJobsBuilder> configure = null,
        Action<JobHostOptions> configureOptions = null)
    {
        if (hostBuilder is null)
        {
            throw new ArgumentNullException(nameof(hostBuilder));
        }

        IHostBuilder hostBuilderWithoutAppConfiguration = new HostBuilderWithoutAppConfiguration(hostBuilder);

        hostBuilderWithoutAppConfiguration.ConfigureWebJobs(
            configure ?? ((hostBuilderContext, webJobsBuilder) => { }),
            configureOptions ?? (configureOptions => { }));

        return hostBuilder;
    }

    private class HostBuilderWithoutAppConfiguration : IHostBuilder
    {
        private readonly IHostBuilder _hostBuilder;

        public HostBuilderWithoutAppConfiguration(IHostBuilder hostBuilder)
        {
            _hostBuilder = hostBuilder ?? throw new ArgumentNullException(nameof(hostBuilder));
        }

        public IDictionary<object, object> Properties => _hostBuilder.Properties;

        public IHost Build() => _hostBuilder.Build();

        public IHostBuilder ConfigureAppConfiguration(Action<HostBuilderContext, IConfigurationBuilder> configureDelegate)
        {
            return this;
        }

        public IHostBuilder ConfigureContainer<TContainerBuilder>(Action<HostBuilderContext, TContainerBuilder> configureDelegate)
        {
            _hostBuilder.ConfigureContainer(configureDelegate);
            return this;
        }

        public IHostBuilder ConfigureHostConfiguration(Action<IConfigurationBuilder> configureDelegate)
        {
            _hostBuilder.ConfigureHostConfiguration(configureDelegate);
            return this;
        }

        public IHostBuilder ConfigureServices(Action<HostBuilderContext, IServiceCollection> configureDelegate)
        {
            _hostBuilder.ConfigureServices(configureDelegate);
            return this;
        }

        public IHostBuilder UseServiceProviderFactory<TContainerBuilder>(IServiceProviderFactory<TContainerBuilder> factory)
        {
            _hostBuilder.UseServiceProviderFactory(factory);
            return this;
        }

        public IHostBuilder UseServiceProviderFactory<TContainerBuilder>(Func<HostBuilderContext, IServiceProviderFactory<TContainerBuilder>> factory)
        {
            _hostBuilder.UseServiceProviderFactory(factory);
            return this;
        }
    }
}

Agendum avatar Apr 19 '20 02:04 Agendum

The way I got around this (VERY ANNOYING BUG ) was to remove the EnvironmentVariables. So that my appsettings.{environment}.json wouldn't override them. Once it is added. I can then call configApp.AddEnvironmentVariables(); to get them in the right order.

var builder = new HostBuilder()
            .ConfigureWebJobs(webJobConfiguration =>
            {
                webJobConfiguration.AddTimers();
                webJobConfiguration.AddAzureStorageCoreServices();
            }).ConfigureAppConfiguration((hostContext, configApp) =>
            {
                // Remove the environment configuration source added by ConfigureWebJobs()
                if (configApp.Sources != null && configApp.Sources.Count > 0) {
                    List<IConfigurationSource> RemoveSources = configApp.Sources.Where(cs => cs is Microsoft.Extensions.Configuration.EnvironmentVariables.EnvironmentVariablesConfigurationSource).ToList();
                    if (RemoveSources.Any())
                    {
                        RemoveSources.ForEach(r => configApp.Sources.Remove(r));
                    }
                 }
                configApp.SetBasePath(Directory.GetCurrentDirectory());
                        configApp.AddJsonFile($"appsettings.{Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT")}.json", optional: true, reloadOnChange: true);
                        configApp.AddEnvironmentVariables();
             })
            .ConfigureServices((context, services) =>
            {
                services.AddSingleton<IConfiguration>(context.Configuration);
                services.AddSingleton<TimmerJob>();
                services.AddTransient<IUpdateService, UpdateService>();
            })
            .Build();
            builder.Run();

david-campbell avatar Apr 30 '20 00:04 david-campbell

Out of curiosity to those that came up with other solutions, did my 7 LoC solution not work?

robertmclaws avatar Apr 30 '20 00:04 robertmclaws

Out of curiosity to those that came up with other solutions, did my 7 LoC solution not work?

I already implemented my fix before I found this thread. I just didn't quite understand why it was happening until I found this thread.

david-campbell avatar Apr 30 '20 01:04 david-campbell

Out of curiosity to those that came up with other solutions, did my 7 LoC solution not work?

I really appreciate your contribution, and I am sure it works great in your application. The reason I chose to go my route was because I don't want to make any assumptions of how the underlying configuration providers are configured, their order, or what they are pointing to. Your solution assumes no other code adds configuration settings, especially environment ones (i.e. a new prefix). I don't like those hidden side-effects. In fact all solutions in this thread are just mucking with the configuration providers, but all of them could have side-effects and will probably cause future bugs if WebJobs does decide to truly fix this with a breaking change.

Instead I wanted to fix the problem at its root and just disable WebJobs ability to cause the problem in the first place, without affecting any other component. So not only does my solution have zero potential side-effects on any other component, it likely is future-proof on WebJobs too since it just disables its ability to run ConfigureAppConfiguration.

Agendum avatar Apr 30 '20 05:04 Agendum

I'm not a fan of manually removing the config sources... I just use a modified version of ConfigureWebJobs, which does what ConfigureWebJobs should be doing.

thomaslevesque avatar Apr 30 '20 07:04 thomaslevesque

So, the way the Configuration API works is, the registration order matters, because the data is merged all the way down. If the same Provider with the same FileName is registered more than one later down the chain, that file will override any previous merges.

So, regardless of what the WebJobs SDK is doing, You should never add a configuration provider more than once with the same file path, or you will negate any previous merges.

The code I posted was heavily tested and sould not break in any situation because it checks to see if the provider has been registered more than once for the same path, and removes any subsequent registrations.

So in my application, I put those calls in an extension method (like .FixWebJobsHost()) that gets called immediately after .AddWebJobs(), which means if you add your dependencies after that, the way you're supposed to, your DI container should be configured properly, whether the WebJobs SDK fixes the problem at some point in the future, or not.

OTOH, the other solutions introduce the potential for breakage if the WebJobs SDK changes their API surface at any point in the future... at least, if I am reading them correctly.

Just food for thought. HTH!

robertmclaws avatar Apr 30 '20 21:04 robertmclaws

I don't love my solution.. I just posted it as an alternative.. it doesn't remove all as suggested. it removes all with a type of EnvironmentVariablesConfigurationSource. My issue was my environment fille was being registered after the environment variables. so..wrong order. it fixed my issue.. but I actually went back and switched this use an extension implementation. and I'll likely review your implementation again. as usual..my code will be reviewed and improved many times ... ;)

david-campbell avatar Apr 30 '20 22:04 david-campbell

I've noticed this too and believe it plays into my current issue with ApplicationInsights. No matter what i do in my config settings, the built in webjobs logger nuget Microsoft.Azure.WebJobs.Logging.ApplicationInsights 3.0.18 wants to default to LogLevel == Information which is a lot of chatter/noise. In code I can override this by setting the log level in the configuration builder.

builder.ConfigureLogging((context, b) =>
            {
                b.AddConsole();

                string instrumentationKey = context.Configuration["APPINSIGHTS_INSTRUMENTATIONKEY"];
                if (!string.IsNullOrEmpty(instrumentationKey))
                {
                    b.AddApplicationInsightsWebJobs(o => o.InstrumentationKey = instrumentationKey);
                    b.SetMinimumLevel(LogLevel.Warning);
                }
            });

`
Annoyingly, doing this workaround then kills the DI telemetryclient from logging TrackEvents.

public Sync(TelemetryClient telemetryclient)
        {
            _telemetryclient = telemetryclient;
           _telemetryclient.TrackEvent("22a");
        }

image

-BC

BC89 avatar Aug 18 '20 13:08 BC89

@BC89 -- that seems like a different issue -- can you spin this off into a new issue and tag me in it?

brettsam avatar Aug 18 '20 14:08 brettsam

It's been over 3 years since this issue was opened -- is there any chance this might get a fix? This still appears to be broken in Microsoft.Azure.WebJobs 3.0.32 (target net60)

bryancrosby avatar May 06 '22 01:05 bryancrosby

It's been over 3 years since this issue was opened -- is there any chance this might get a fix?

v3.0.31 here, this please

fartwhif avatar Jun 24 '22 14:06 fartwhif

This is causing so much confusion because the ConfigureWebJobs does half of the job that Host.CreateDefaultBuilder does (see here) for example it's not configuring logging filters from appSettings and so on. Using Host.CreateDefaultBuilder results in issues reading configurations because the appSettings.Json is added after the eventual call to ConfigureAppConfiguration.

It has always been tricky to get azure functions and web job reading configuration properly, not sure why not stick to the widely used and understood asp.net core model.

We have a plan to update this with the proper behavior (again, as mentioned above, the current behavior was not really intentional), but haven't picked this item up due to competing priorities. We'll have this assigned soon.

@fabiocav is it possible to get this done? We've been waiting for quite a while now...

ilmax avatar Aug 06 '22 12:08 ilmax