Runtime: EnvironmentVariablesConfigurationProvider should support all types of connectionstrings

Created on 8 May 2019  路  14Comments  路  Source: dotnet/runtime

When setting a connection string on an Azure Webapp using Azure CLI the type must be specified.
The following types are supported:
ApiHub, Custom, DocDb, EventHub, MySql, NotificationHub, PostgreSQL, RedisCache, SQLAzure, SQLServer, ServiceBus
az webapp config connection-string set

It seems that Microsoft.Extensions.Configuration.EnvironmentVariables.EnvironmentVariablesConfigurationProvider only support four of these: MySql, SQLAzure, SQLServer and Custom

This means that if I use Configuration.GetConnectionString("MyServiceBusConnectionString") and I have added a connection string of type ServiceBus on my Azure Website with the same name the code will unexpectedly fail. I will have to define the type of the connectionstring in azure to be Custom for this to work.

It would be much better if EnvironmentVariablesConfigurationProvider and the method AzureEnvToAppEnv supported the same types as the Azure CLI does.

These are the environment variable prefixes that I think should be added:
APIHUBCONNSTR_
DOCDBCONNSTR_
EVENTHUBCONNSTR_
NOTIFICATIONHUBCONNSTR_
POSTGRESQLCONNSTR_
REDISCACHECONNSTR_
SERVICEBUSCONNSTR_

I would be happy to make a PR for this should you agree that this is something that should be supported.

area-Extensions-Configuration feature request

Most helpful comment

Wow, this was a bizarre experience.

I created a stock "Web App with Postgres" in Azure and deployed my application, only to spend hours trying to debug why the app wasn't picking up my connection string (which had been automatically added to the web app using the "PostgreSQL" type).

That lead me to eventually find this issue and switch the connection string type to "Custom" instead, and suddenly the app worked fine. What a mess 馃槥

All 14 comments

Wow, that layering is really bizarre. I'd have expected there to be a library that read from config and dealt with this, not having it embedded in the provider itself.

Do we actually use those provider names for anything? I did a quick search across Extensions, AspNetCore, and EF Core, and outside of configuration tests, the _ProviderName config doesn鈥檛 appear to be used. What are those for?
And why would this configuration provider decide about what database provider I use? For example, it鈥檚 just by chance that MySqlConnector uses the same namespace for their MySQL provider (for migration and compatibility reasons). But they didn鈥檛 have to do it like that.

Adding a note because we got a report via email that a customer hit the fact that we don't read the connection string correctly for PostgreSQL when it is configured in app services. If this issue was fixed, the simple code they used would work as they expect and they would get the connection string from the value configured in Azure App Services:

var cs = Configuration.GetConnectionString("MyDatabase");

Possibly the simplest workaround is to use the "custom" option instead.

To answer to previous comments:

@poke No framework code that I am aware currently uses _ProviderName , but applications can do it if the wish to. For example an application could support using different kinds of databases with the same code and resolve the DbProviderFactory from DbProviderFactories using the invariant name.

@Tratcher I am not sure I like the layering either but the logic needs to go somewhere. My main complaint is that the logic is bound to become stale anytime Azure App Services adds new connection string types. Ideally they would publish a package that encapsulates the logic. But this way we own it and it is easier for us to make it right.

@bradygaster, I was told by @glennc that you will be following up on this. I will add you to the email thread about the customer issue.

@divega can we at least look at properly re-layering this in 5.x? E.g. move the connection strings out of the environment variable provider to their own API that consumes config and is called from the templates. At the very least it would be easier to maintain and ship an out of band leaf node dependency.

@Tratcher I think you can make a proposal, check with @bradygaster, @DamianEdwards that the resulting experience (and possible breaking change) is worth it.

Eeeek, having strings that are hard-coded to an Azure service's set of supported things in this provider is making me feel poorly. We shouldn't definitely look at this in 5.0

@davidfowl @glennc

absolutely agree with @DamianEdwards on this - I don't see those prefixes adding value, and have been confused by their existence for some time. I'd like to re-evaluate this, and hopefully find a way to stop doing it. IMO, a "connection string is a connection string," and shouldn't contain any "triggering behavior" for the consumer or configurator. Screen shot from the AZ CLI below for reference. Adding @btardif here from App Service team as he's on the email thread in which we're discussing this internally. @btardif is this a requirement of the web apps side or something imposed by the CLI side?

Al - is there any reason why we can't agree "a connection string is a connection string" and figure out if unwinding the prefixes is possible?

I believe the original reason for the prefix was because those got projected into the .NET Framework configuration system as provider specific connection strings.

<connectionStrings>
    <add name="ConnStringDb1" connectionString="Data Source=localhost;Initial Catalog=YourDataBaseName;Integrated Security=True;" providerName="System.Data.SqlClient" />
</connectionStrings>

There's no first class concept like that in .NET Core so we mapped it internally (in a hacky way) to ConnectionString:{Name} or ConnectionString:{Name}_{Provider}. So the options here are to:

  • Change AppService to match our conventions or add our conventions to the list natively when connection strings appear.
  • Create an app service specific configuration provider and have users add that (or light it up on app service)
  • Keep hacking and add more prefixes

Just to mention the point discussed offline, having a provider classification on a connection string could allow us to do interesting things in the UI, e.g. a provider-specific UI for building the string, validation, or whatever. There's definitely value in that, especially for people starting out and not familiar with what options are supported in the connection string of which provider, etc.

Wow, this was a bizarre experience.

I created a stock "Web App with Postgres" in Azure and deployed my application, only to spend hours trying to debug why the app wasn't picking up my connection string (which had been automatically added to the web app using the "PostgreSQL" type).

That lead me to eventually find this issue and switch the connection string type to "Custom" instead, and suddenly the app worked fine. What a mess 馃槥

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!

I would like to see this issue addressed if possible, or perhaps an explicit warning for those of us who select an unsupported connection string type such as PostgreSQL if that'd be easier.

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

Just spent half the day discovering this absolute stink. Please fix, or at least document.

I understand the discussion about the hack above, but right now you have half a hack. Might as well add the missing values quickly to at least have a complete hack. The refactor can happen separately

Was this page helpful?
0 / 5 - 0 ratings

Related issues

GitAntoinee picture GitAntoinee  路  3Comments

EgorBo picture EgorBo  路  3Comments

v0l picture v0l  路  3Comments

jamesqo picture jamesqo  路  3Comments

Timovzl picture Timovzl  路  3Comments