Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuration provider that loads configs from shell settings (OSOE-637) #65

Closed
Piedone opened this issue May 29, 2023 · 6 comments
Closed
Labels
enhancement New feature or request

Comments

@Piedone
Copy link
Member

Piedone commented May 29, 2023

Let's add an ASP.NET configuration provider (see IConfiguration) that loads configs from the current tenant's shell settings, playing along with Orchard's built-in tenant postconfiguration.

This would allow us to configure tenant-specific settings in a way that's separated by tenant, not all together in the same appsettings.json file or app configuration of an Azure App Service. This includes settings like media storage quota settings, idle shutdown timeouts, or even password requirements.

To make this easier to utilize, let's do #66 too.

Jira issue

@github-actions github-actions bot changed the title Configuration provider that loads configs from shell settings Configuration provider that loads configs from shell settings (OSOE-637) May 29, 2023
@Piedone Piedone added the enhancement New feature or request label May 29, 2023
@jtkech
Copy link
Member

jtkech commented Jun 3, 2023

Here some thoughts as I remember.

There are 2 config flows, required settings and shell config that at the end are respectively persisted in tenants.json for all tenants and in the tenant specific ../Sites/{tenant}/appsettings.json, by default (can be the database) each file is the last (higher precedence) config source of its flow. They are already editable and mutable, we mutate them when a tenant is setup/edited.

Let's only talk about the shell config flow.

So ShellSettings exposes the result of the config stack and normally can't be a config provider itself. But yes, shell settings has an inner updatable in memory config provider, so you can update it with any value that will be persisted through the last mutable config source (by default the tenant specific appsettings).

Doing this way, for now we can only update simple strings, maybe something to extend here.

Another way is to override IShellConfigurationSources (as done when coming from the database), yes for now we can't register multiple ones, need to be overridden to define inside the sources to be used.

@Piedone
Copy link
Member Author

Piedone commented Jun 4, 2023

Yep, I was thinking about storing strings (or rather, only strings would be stored in ShellSettings, much like the usual config providers store strings only, but these can then be parsed as bool, int, or something else).

However, are you saying that what's by default in tenant-specific appsettings file can also be stored in the DB, i.e. what I envision here is actually already possible?

@jtkech
Copy link
Member

jtkech commented Jun 4, 2023

Yes, AddDatabaseShellsConfiguration() registers a custom IShellsSettingsSources but also a custom IShellConfigurationSources but for now there is no UI editor oob.

Hmm, yes if you can edit and save the related Document and then release / reload the shell it may work, but normally we update first the ShellSettings and then call UpdateShellSettings() that will update e.g. the ShellSettings.VersionId and then call the SaveAsync of the current IShellsSettingsSources and IShellConfigurationSources.

Would need to think about it, maybe not a problem to edit directly the Document if we don't update shell settings but only shell configuration.

@Piedone
Copy link
Member Author

Piedone commented Jun 4, 2023

Oh, I see what you mean now. Yes, I was aware of these but it's not the same as what I'm looking for here.

So while yes, there are tenant-specific appsettings files, with the content like below:

{
  "DatabaseProvider": "Sqlite",
  "FeatureProfile": "standard-tenant",
   ...
}

...and these will be loaded into ShellSettings, but I'd like to load data into IConfiguration. E.g., currently if we want to increase the Media storage quota of a tenant, we add this to either the root web app's appsettings.jon, or to something like the configuration of an Azure App Service:

OrchardCore__TenantName__Lombiq_Hosting_Tenants_MediaStorageManagement__Media_Storage_Management_Options__MaximumSpace: 12345

Instead, I'd like to load this from the shell settings, i.e. be able to have a value in the tenant-specific appsettings file or in the shell settings document in the DB, that'll be accessible as exactly this IConfiguration value (in a generic way, this quota management config is just an example).

@jtkech
Copy link
Member

jtkech commented Jun 4, 2023

Okay then, if you only have strings to config I would recommend to use the ShellSettings to retrieve the possible current values before using a custom editor, and on submitting update the ShellSettings.

Note: ShellSettings has 2 inner ShellConfigurations named _settings and _configuration, when you use the indexer shellSettings[key] you are not using _settings[key] but _configuration[key].

So you can update the shell settings by using the indexer and then call UpdateShellSettings(), the related appsettings file (or db document) will be updated and the shell reloaded. Then by using IShellConfiguration which is an IConfiguration you will be able to retrieve the config values (that has been persited in the config source of the higher precedence).

If you have more complex data to config, maybe edit directly the appsettings (or db document) but you will have to merge data (not override existing values) and then still reload the shell. In case of the file not obvious to be thread safe, for example if a tenant is edited and saved at the same time, this is in the settings manager that we use an internal semaphore (but that you can't use externally).

@Piedone
Copy link
Member Author

Piedone commented Jun 5, 2023

Ah I see, thank you! I thought about merging these configs into IConfiguration, but we can actually live with IShellConfiguration too. Then, nothing to do here, only #66.

@Piedone Piedone closed this as completed Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants