OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.35k stars 2.37k forks source link

Support for additional configuration sections other than OrchardCore #3452

Open BenedekFarkas opened 5 years ago

BenedekFarkas commented 5 years ago

Aim

The ability to define further configuration sections for the application and the tenants other the ones read from the OrchardCore configuration section. Why it matters, is because in some cases you don't have complete control over the configuration (and their names), but the application/tenants need to be able to access it.

Backstory - Orchard 1.x

The way you would go about configuring Azure Media for local development vs. hosting for the least amount of interaction is to define the connection to the development storage by adding a connection string in your web.config (in the <connectionStrings>, of course): <add name="Orchard.Azure.Media.StorageConnectionString" connectionString="UseDevelopmentStorage=true" />

Then deploy the application to Azure, set up a blob storage account. To override Azure Media connection string, create a connection string for the App Service in the configuration UI with and set its value to the blob storage. This config will override the one written in the web.config, so when the application is running in Azure, it will connect to the Azure blob storage instead.

ASP.NET core is a different story

First of all, web.config is a lot less meaningful, since you don't have to use IIS. If we stick to Azure, App Settings and Connection Strings defined in the App Service configuration are exposed towards the application as environment variables (that are prefixed with either "AppSettings:" or "ConnectionStrings:", so we need to do two things: Make them available to the application, then add them to the shell configuration so the Orchard extensions can access them.

App configuration sources

This is fortunately quite simple using the IWebHostBuilder.ConfigureAppConfiguration method, for example: .ConfigureAppConfiguration((hostingContext, config) => config.AddEnvironmentVariables()) The method accepts a string parameter restrict the environment variables processed to a specific section, e.g. "AppSettings".

Tenant configuration sources

So the App Settings and Connection String are available to the application now, but we need to expose them towards the tenants as well. ShellSettingsManager, by default, only accepts configuration sources from the OrchardCore root configuration section, so nothing else will be passed on to the tenants. I don't think that passing on every other configuration source to the tenants would be beneficial (considering collisions), but configuration sections named OrchardCore under any other root configuration section could be accepted. If processed correctly, these could work the same way as the root OrchardCore configuration section: Provide a global configuration, but tenants can have specific overrides (using sub-sections that match the name of the tenant). Consider the following code snippet:

_configuration = configurationBuilder.Build();

// This is default OrchardCore configuration section in the root.
var orchardCoreRootSection = _configuration.GetSection("OrchardCore");
var orchardCoreSections = new List<IConfigurationSection> { orchardCoreRootSection };

// Go through each root configuration section and try to find their children named OrchardCore.
foreach (var rootSection in _configuration.GetChildren())
{
    // The OrchardCore root section is already processed.
    if (rootSection.Key == "OrchardCore") continue;

    var orchardCoreSection = rootSection.GetChildren().FirstOrDefault(subSection => subSection.Key == "OrchardCore");

    if (orchardCoreSection is object) orchardCoreSections.Add(orchardCoreSection);
}

// Tenants are fetched from the root OrchardCore section.
_configuredTenants = orchardCoreRootSection.GetChildren()
    .Where(section => section["State"] != null)
    .Select(section => section.Key)
    .Distinct()
    .ToArray();

_configBuilderFactory = (tenant) =>
{
    var builder = new ConfigurationBuilder();

    // Add global configuration from each OrchardCore configuration section.
    foreach (var section in orchardCoreSections)
        builder.AddConfiguration(section);

    // Add tenant-specific configuration from each OrchardCore configuration section.
    foreach (var section in orchardCoreSections)
        if (_configuredTenants.Contains(tenant))
            builder.AddConfiguration(section.GetSection(tenant));

    return builder.AddSources(tenant, _tenantConfigSources);
};
BenedekFarkas commented 5 years ago

@sebastienros based on our previous discussion, I tried naming the connection strings in the Azure Portal for the App Service Configuration as detailed in the documentation.

The issue still remains: When you define App Settings or Connection Strings in the App Service Configuration, in the case of an ASP.NET Core application those values will be added to the system as environment variables, but they will also receive a prefix, for example:

Since Orchard is configured to look at configuration / environment variables that are inside the OrchardCore root section, these will not be picked up by the application.

deanmarcussen commented 5 years ago

Hi @BenedekFarkas

So the trick that @jtkech explained to me, and I wrote up in the docs is to configure on a per tenant basis.

So this works in Azure appsettings for the Default tenant

Screenshot 2019-09-04 at 11 12 16

Note the State appsetting. Which can have any value, just needs to exist.

In Kudu I'm seeing two environment variables defined

BenedekFarkas commented 5 years ago

Hey @deanmarcussen, thanks for your reply!

My issue is not related to tenants, actually - I'd like an overall, application-level connection string for e.g. Azure Blob Storage for Media Library.

The issue is that the connection strings defined in the Azure Portal are added to the environment variables with a prefix (Custom type connection strings are prefixed with CUSTOMCONNSTR_). So that way those bits of configuration will not be picked up by Orchard.

But: Your example actually made me wonder about something: According to the Azure documentation, App Settings are also prefixed when added to the environment variables, but (contrary to connection strings) they are also added without a prefix. So this means that App Settings are actually picked up by Orchard (the ones that are not prefixed), but Connection Strings are not.

This is really strange, but adding the necessary configuration to App Settings (instead of Connection Strings) actually solves this issue.

deanmarcussen commented 5 years ago

Cool, glad it helped even if it wasn't quite what you wanted.

I think Azure Connection Strings settings are more related to aspnet because web.config had a specific section for connection strings.

Whereas dotnetcore appsettings comes from environment variables with no prefix.

So I guess that's why they changed Azure to do environment variables in a way core and aspnet understand.

sebastienros commented 5 years ago

Note the State appsetting. Which can have any value, just needs to exist.

Is that documented too? Can someone explain to me?

deanmarcussen commented 5 years ago

Yes, that's documented too (maybe could be clearer!)

It's in the ShellSettingsManager. If it finds the tenants name in an IShellConfiguration source it includes the values from said source in the general configuration. It's cool what it does.

            _configuredTenants = _configuration.GetChildren()
                .Where(section => section["State"] != null)
                .Select(section => section.Key)
                .Distinct()
                .ToArray();

            _configBuilderFactory = (tenant) =>
            {
                var builder = new ConfigurationBuilder().AddConfiguration(_configuration);

                if (_configuredTenants.Contains(tenant))
                {
                    builder.AddConfiguration(_configuration.GetSection(tenant));
                }

                return builder.AddSources(tenant, _tenantConfigSources);
            };
Piedone commented 4 years ago

Hmm, anything else to do here? @BenedekFarkas?

BenedekFarkas commented 4 years ago

I'll put it on my list to check back on this - there's probably a bunch of other changes around this feature since then, so I'll dig in.