Runtime: Is dynamic IHttpClientFactory registration a bug or a feature?

Created on 27 Aug 2019  路  11Comments  路  Source: dotnet/runtime

Is your feature request related to a problem? Please describe.

My application needs runtime-defined named HttpClient configuration, and I'm using IHttpClientFactory. My application also needs to configure different DelegatingHandler chains unique to each named configuration (for injecting configuration-specific Bearer tokens and access_token refresh logic).

My application is a headless Windows service that stores 1...N-many named HttpClient configurations on-disk and it uses an injected-service to read the stored configurations, so the IServiceProvider isn't available to read the configuration while it's still calling IServiceCollection.AddHttpClient<MyClient>( ... ) (and the idea of maintaining multiple root IServiceProvider instances isn't appealing either).

Ostensibly, the IHttpClientFactory requires all named HttpClients to be set-up during service-configuration - which would make it unsuitable for my application - however I noticed that DefaultHttpClientFactory's pool of HttpMessageHandler instances can be added-to even after configuration has completed - this does not seem to be documented either way - so I'm not sure if it's a bug (i.e. it should be immutable) or a feature (for runtime-configured HttpClient and HttpMessageHandlerBuilder instances).

Describe the solution you'd like

  • A statement from the PM responsible declaring the immutability of IHttpClient's configuration.
  • If it is immutable, then fix the bug.
  • If it's mutable, then make it easier to add dynamic configuration of HttpClient without jumping through hoops with IConfigureNamedOptions.

Describe alternatives you've considered

Right now my application depends on the current behaviour for dynamic HttpClient configuration, like so:

    internal class DynamicHttpClientFactoryConfiguration : IConfigureNamedOptions<HttpClientFactoryOptions>
    {
        public void Configure( String httpClientName, HttpClientFactoryOptions options )
        {
            IAccessTokenRenewerFactoryStore store = this.sp.GetRequiredService<IAccessTokenRenewerFactoryStore>();

            if( store.TryGetConfiguration( httpClientName: httpClientName, out MyHttpClientConfiguration cfg, out AccessTokenRenewer atr ) )
            {
                options.HttpClientActions.Add( httpClient =>
                {
                    httpClient.BaseAddress = cfg.Authority;
                } );

                options.HttpMessageHandlerBuilderActions.Add( httpMessageHandlerBuilder =>
                {
                    AccessTokenRenewDelegatingHandler delegatingHandler = new AccessTokenRenewDelegatingHandler( atr );
                    httpMessageHandlerBuilder.AdditionalHandlers.Add( delegatingHandler );
                } );
            }
        }
    }

And it "just works" whenever code anywhere in my project calls IHttpClientFactory.CreateClient( configurationName ) (provided that configurationName exists in my IAccessTokenRenewerFactoryStore).

area-Extensions-HttpClientFactory

Most helpful comment

@rynowak

HttpClient factory's configuration is immutable. This is not a bug, it's a feature request.

That's not what my post is about. Dynamic configuration already exists, it's just difficult to use.

If the configuration is immutable (as you posit it is), then why is it that I _can_ currently have dynamic configuration using only stock .NET Standard types? (i.e. without using any tricks like reflection, so using only IConfigureOptions<T> and IConfigureNamedOptions).

What's behind your need for N HttpClient configurations?

In my case, I have a single headless Windows Service process that supports multi-tenancy (in the client-side, not just the server-side) so it supports having N-many configuration files which are loaded _after_ ConfigureServices has completed - so each configuration file has its own web-service base URI, client certificate and/or access-token (or OIDC client credentials) and when the process is initialized it means each configuration instance in-memory has its own private HttpClient instance that has the BaseAddress, DefaultRequuestHeaders, and Cookies (via HttpClientHandler) properties set. I couldn't find a good way to get this to work using the stock IHttpClientFactory.

Sidenotes:

  • I did consider storing the multi-tenant configuration dictionary inside app.config (or a separate file) so it would be exposed via IConfiguration, however a project requirement is that the configuration file be editable and reloadable without restarting the entire process.
  • I also have prior existing library code that uses injected HttpClient instances that I wanted to reuse.

    • And this old library code I mentioned must run on embedded systems running rather obscure builds of Windows that aren't supported by .NET Standard 2.0 (downgrading to .NET Standard 1.1-compatible versions of Microsoft.Extensions.* is not feasible).

  • I also wanted to keep "system configuration" separate from "tenant configuration". The serviceExe.config.json file is kept in C:\Program Files\MyService\ServiceExe.config.json and can only be edited by administrators, whereas the other configuration files are kept in C:\ProgramData\MyService\UserEditableConfig.json and my service's installation code sets the NTFS ACLs on the C:\ProgramData\MyService directory to allow any local user to edit it (this is acceptable given our threat-model).

Here's a feel for what my application's UserEditableConfig.json configuration file looks like:

"serviceClientConfigurations": [
    "customerFoobar": {
        "oidcClientId": "foobar123",
        "oidcClientPassword": "base64value-encrypted-with-DPAPI-machine-key",
        "someOtherSetting": "foobar",
        "pollInterval": 180,
        "webSockets": true
    },
    "customerBaz": {
        "oidcClientId": "baz456",
        "oidcClientPassword": "another-base64value-encrypted-with-DPAPI-machine-key",
        "someOtherSetting": "foobar",
        "pollInterval": 360,
        "webSockets": false
    },
    "customerQux": {
        "oidcClientId": null,
        "oidcClientPassword": null,
        "someOtherSetting": "foobar",
        "pollInterval": 360,
        "webSockets": false
    }
]

All 11 comments

Holy sh!t, u a f!cking genious 馃敟馃敟馃敟
Please, choose option three!!!

Hey @Jehoel -- how exactly do I use DynamicHttpClientFactoryConfiguration ? Where and how do I register it (if needed?)

EDIT: ah, I think I get it: services.ConfigureOptions<DynamicHttpClientFactoryConfiguration>() is needed.

@oising As it's an IConfigureNamedOptions<HttpClientFactoryOptions>, I was assuming one had to do:

services.AddSingleton<IConfigureOptions<HttpClientFactoryOptions>, DynamicHttpClientFactoryConfiguration>();

(but interested if the shorter version works).

It should also be possible to achieve a similar effect with an IHttpMessageHandlerBuilderFilter.

A statement from the PM responsible declaring the immutability of IHttpClient's configuration.
If it is immutable, then fix the bug.

HttpClient factory's configuration is immutable. This is not a bug, it's a feature request.

What's behind your need for N HttpClient configurations? Usually when someone is asking for this kind of feature they asking because they need flexibility related to the use of client certs, proxies, other network settings, etc. Which is it in your case?

@rynowak In my case, I've got an event hub and a proxy/relay function that uses information in the message to dynamically determine the REST endpoint to forward the message body. This is represented by a named/typed HttpClient, of which several are cached in a singleton class injected into the function ctor. The problem is that these named clients have to be configured in startup with retry policies, lifetimes etc, when I actually want to configure them dynamically at runtime.

when I actually want to configure them dynamically at runtime.

When you say you want to configure them dynamically... based on what information? Suppose client factory adds a feature to run code dynamically when a client is requested - what data do you need as input?

@rynowak

HttpClient factory's configuration is immutable. This is not a bug, it's a feature request.

That's not what my post is about. Dynamic configuration already exists, it's just difficult to use.

If the configuration is immutable (as you posit it is), then why is it that I _can_ currently have dynamic configuration using only stock .NET Standard types? (i.e. without using any tricks like reflection, so using only IConfigureOptions<T> and IConfigureNamedOptions).

What's behind your need for N HttpClient configurations?

In my case, I have a single headless Windows Service process that supports multi-tenancy (in the client-side, not just the server-side) so it supports having N-many configuration files which are loaded _after_ ConfigureServices has completed - so each configuration file has its own web-service base URI, client certificate and/or access-token (or OIDC client credentials) and when the process is initialized it means each configuration instance in-memory has its own private HttpClient instance that has the BaseAddress, DefaultRequuestHeaders, and Cookies (via HttpClientHandler) properties set. I couldn't find a good way to get this to work using the stock IHttpClientFactory.

Sidenotes:

  • I did consider storing the multi-tenant configuration dictionary inside app.config (or a separate file) so it would be exposed via IConfiguration, however a project requirement is that the configuration file be editable and reloadable without restarting the entire process.
  • I also have prior existing library code that uses injected HttpClient instances that I wanted to reuse.

    • And this old library code I mentioned must run on embedded systems running rather obscure builds of Windows that aren't supported by .NET Standard 2.0 (downgrading to .NET Standard 1.1-compatible versions of Microsoft.Extensions.* is not feasible).

  • I also wanted to keep "system configuration" separate from "tenant configuration". The serviceExe.config.json file is kept in C:\Program Files\MyService\ServiceExe.config.json and can only be edited by administrators, whereas the other configuration files are kept in C:\ProgramData\MyService\UserEditableConfig.json and my service's installation code sets the NTFS ACLs on the C:\ProgramData\MyService directory to allow any local user to edit it (this is acceptable given our threat-model).

Here's a feel for what my application's UserEditableConfig.json configuration file looks like:

"serviceClientConfigurations": [
    "customerFoobar": {
        "oidcClientId": "foobar123",
        "oidcClientPassword": "base64value-encrypted-with-DPAPI-machine-key",
        "someOtherSetting": "foobar",
        "pollInterval": 180,
        "webSockets": true
    },
    "customerBaz": {
        "oidcClientId": "baz456",
        "oidcClientPassword": "another-base64value-encrypted-with-DPAPI-machine-key",
        "someOtherSetting": "foobar",
        "pollInterval": 360,
        "webSockets": false
    },
    "customerQux": {
        "oidcClientId": null,
        "oidcClientPassword": null,
        "someOtherSetting": "foobar",
        "pollInterval": 360,
        "webSockets": false
    }
]

@rynowak My scenario is roughly the same as @Jehoel above. So, that's a plus one.

As part of the migration of components from dotnet/extensions to dotnet/runtime (https://github.com/aspnet/Announcements/issues/411) we will be bulk closing some of the older issues. If you are still interested in having this issue addressed, just comment and the issue will be automatically reactivated (even if you aren't the author). When you do that, I'll page the team to come take a look. If you've moved on or workaround the issue and no longer need this change, just ignore this and the issue will be closed in 7 days.

If you know that the issue affects a package that has moved to a different repo, please consider re-opening the issue in that repo. If you're unsure, that's OK, someone from the team can help!

@msftbot Please keep this issue open.

Personal rant: I strongly disagree with the practice of _closing_ bugs simply because they're stale or seemingly abandoned. Of course bugs/issues should be tagged as stale over time (which allows them to be filtered ) but in my book, bugs should only be Closed after they're Definitely Resolved - and closing bugs as stale after only 7 days after moving them is a bit mean. There are plenty of issues I've created on GitHub and elsewhere that I can't make progress on because they might be in a completely different project or team that I'm currently working-in and I can't always afford to make the mental context-switch just for a minor update to an issue at very short-notice.

What would be great, especially for API issues like these, if if issues were required to have an attached test-case that demonstrates the issue (if possible). Though requiring people to fork the test project is overkill, perhaps if there was something like an online C# equivalent of LinqPad that could be used to quickly submit quick-and-dirty or proof-of-concept test-cases?

Paging @dotnet/extensions-migration ! This issue has been revived from staleness. Please take a look and route to the appropriate repository.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

v0l picture v0l  路  3Comments

jzabroski picture jzabroski  路  3Comments

bencz picture bencz  路  3Comments

nalywa picture nalywa  路  3Comments

noahfalk picture noahfalk  路  3Comments