azure-webjobs-sdk: WebJobsHostBuilderExtensions.ConfigureWebJobs should not add configuration sources

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… 👎

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

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)

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 😉

First off, I’m glad the team is finally giving this the attention it deserves. Having digested this a bit, here is my feedback: I would humbly suggest that this plan take the opposite approach.

Step outside of the fact that you folks built this framework years ago, and ask yourselves how you would build it NOW if you started over? The whole point of the Host.CreateDefaultBuilder() mechanism is that the developer should be deciding how the host system is configured, not you.

Why should the SDK even be presuming to know anything about how the Configuration system is set up? That’s not it’s job! It’s job is to register services in DI, simply USE the configuration that is available, and start watching for events to be processed. That’s it. If configuration is not registered, then the Host should fail to finish loading. NOT override and break people who were already doing the right thing.

So the priority should be to bring the API up to date with modern IHost development practices, so that using it on .NET 6 or 7 doesn’t feel as out of place as it does today. I would recommend the following pattern:

• AddWebJobs(this IServiceCollection services) that would register the services with the container (similar to what we did here, but without the configuration hacks we had to use to work around the current limitations),
• UseWebJobs(this IHostBuilder builder, Action config) that would configure the WebJobs system, and register the proper lifetime depending on how it is being hosted (similar to what I did here).

This would be the pattern of record moving forward. I would do this first, get alignment on how the pattern works, write unit tests to back up the separation of concerns, and heavily XML Documentation comment both public AND private methods.

Then in the same release, to fix the people that are currently broken, I would:

• First, add an [Obsolete("This method can cause issues on .NET Core 3.1 and later. Please use 'AddWebJobs()' and UseWebJobs() instead.", false)] attribute to the existing configuration methods. That way, people already using them know they’re on the way out, and new people stop using them entirely.
• Next, I would do your first option above, but using the code here instead. The problem with the current code (among other things) is that the team decided to register appsettings.json WITHOUT registering environment-specific overrides. Part of the reason you should not be doing this at all is because the .NET team will likely decide in the future to change how Host.CreateDefaultBuilder() works, and this project has already demonstrated that you don’t have the bandwidth to keep up. Nor should you be trying to, you have other fish to fry.

This way, you maintain backwards compatibility, direct people to get off the old system, and have a new way for developers to register WebJobs that maintains Separation of Responsibility and doesn’t try to outsmart anyone involved.

As someone who spent months hacking the WebJobs platform to be an out-of-band queued event processor that can work on-prem or in the cloud with zero code changes, this is now I would introduce these changes inside my own codebase for my own customers. I hope this helps, and am happy to participate in the code changes to make this happen.

As discussed, we wanted to provide an update to share the plans on how this issue will be addressed:

1. The behavior of the ConfigureWebJobs API will change to conditionally add the configuration providers

The ConfigureWebJobs API will continue to register the following configuration providers:

• JsonConfigurationProvider with a path set to appsettings.json and set as optional
• EnvironmentVariablesConfigurationProvider

Moving forward, similar to one of the workarounds provided above, the API will perform a check against the existing/registered providers and skip registration if a provider with the same settings is already present.

2. API documentation updates

The XML documentation on the ConfigureWebJobs API will be updated to clearly describe the behavior of the API and what is registered with that call.

Additional steps (optional)

These are additional options we’ve discussed and wanted to get feedback on (not in scope for the initial set of changes). This would be done in addition to the above:

1. A new API for WebJobs service registration will be added

The ConfigureWebJobs API will be refactored, with the logic responsible for configuration and service registrations split. The service registration will be made available as an IServiceCollection extension method, enabling users to register the core WebJobs services only.

2. A new API to create a default builder will be introduced

An new API to create the builder will be introduced. This uses a pattern familiar to many developers (similar to WebApplication.CreateBuilder, which will have the same behavior as the updated ConfigureWebJobs API and guarantee the registration order.

We expect PRs for those items to be landing soon, but welcome feedback on the approach.

Thank you again for the help and patience!

@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

We’ve discussed this issue in the review/triage (as it was flagged by @liliankasem) and before the end of the current sprint (Wednesday 8/31), will be sharing details on how we plan to address this and gathering feedback.

Thank you all for the feedback and patience.

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.

I’ve got a PR out https://github.com/Azure/azure-webjobs-sdk/pull/2911 for a couple of the initial improvements we discussed above. For anyone interested, please provide feedback on that PR, thanks 😃

@liliankasem Thanks for responding! Really appreciate you.

No disrespect to @fabiocav and @brettsam, but if they believe there is a problem, they should be having that discussion on here so that the community can triage it together. It has been an issue for 3 years now, and the fix is (and has been) relatively simple. It can be done without breaking changes (something that can easily be verified with the right unit tests). I’m happy to commit a fix, but I need to be reasonably certain that my PR is not going to sit open waiting to be reviewed for years.

For reference, I’m a member of the .NET Foundation, I manage the OData Restier project for Microsoft, and I have a product called SimpleMessageBus that turns the WebJobs SDK into an out-of-band message processor that powers major systems for the State of Florida, a nationwide restaurant chain, a startup called BurnRate, and public crypto projects.

My team heavily depends on this project, and I’d like to be able to have my company work with your team to contribute fixes. Thanks for your time! 😃

Note: this comment has been edited to remove content that violated this project’s code of conduct

@jeroenbongers Here’s a method that can help:

        /// <summary>
        /// The WebJobs SDK (for some reason) calls the Host.AddAppConfiguration for you. The holders for those methods are internal, and the last entry in the pipe overrides previous merging.
        /// So we have to delete the unnecessary ConfigurationProviders until the WebJobs team fixes their mistake.
        /// </summary>
        /// <param name="hostContext"></param>
        private static void FixWebJobsRegistration(HostBuilderContext hostContext)
        {
            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)));
            }
        }

Call this as the very first line of code inside ConfigureServies (ie HostBuilder.ConfigureServices((context, config) => { FixWebJobsRegistration(context); }); and you should be good to go.

I’m trying to get Microsoft to let me more actively contribute to this project so I can fix this bug.

@fabiocav Yep, I think the work is pretty closely aligned, and I hope I conveyed that properly. My concern was more in line with stated priorities + execution order, and directing people off the current codepath long-term. It feels to me like setting up for the right future can’t be less of a priority than fixing a past that people are already having to work around. If we get this done in one atomic unit, then people can make the decision to maintain binary compatibility or set themselves up for a solid future in one motion.

@brettsam @fabiocav I just spent 2 hours figuring out why on earth my appsetting.{environment}.json was not overwriting my appsetting.json. It turns out it was because the ConfigureWebJobs was adding an additional appsettings.json which kept getting priority.

Can something please be done about this, it’s extremely confusing when developing.

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!