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.36k stars 2.37k forks source link

Abiltiy to add custom database presets #6592

Closed JoeBerkley closed 3 years ago

JoeBerkley commented 4 years ago

Is it currently possible to add custom database presets ?

The use case we're trying to solve is, we often have several tenants off of a single host, but all will use the same DB with different prefixes. Ideally when setting up the tenants would just use the host's connection string and either let the user enter a prefix or auto-generate a prefix based on the tenants name.

The solution to this might not be adding a custom presets but just add a database type that addresses this use case?

jtkech commented 4 years ago

@JoeBerkley

You can define config values in different config sources, e.g the appsettings.json at the root of the app, this under a main section whose name is OrchardCore. Then you can use a child section whose name is a tenant name if you want to define config values specific to a given tenant. Otherwise, config values that are just under the OrchardCore section will be shared by all tenants.

So in config sources e.g appsettings.json you can pre-define the DatabaseProvider, ConnectionString and so on, so that they will be auto filled and not requested when you setup a new tenant. If you want the TablePrefix to be auto generated you will have to provide some custom code to be able to provide a templatable value (e.g. a liquid expression) that would be evaluated in a custom IConfigureOptions<>, the OC.Media.Azure module is doing this to auto-generate e.g. the ContainerName e.g. based on the tenant name, see the MediaBlobStorageOptionsConfiguration

willnationsdev commented 3 years ago

@jtkech I just encountered this scenario. We don't want those creating tenants to need to re-enter (or even see) the connection string to our database every time they do it. If I follow your approach to adding an appsettings.json at the root:

{
    "OrchardCore": {
        "ConnectionString": "[connstr]"
    }
}

...then the [connstr] bit will show up pre-filled in the ConnectionString area when picking a relevant database. However, we would prefer to prevent the connection string input from rendering in its entirety. Is there an existing way to do this in core? If not, should we make changes to support it, or should this be handled by those using the CMS assembly by creating our own custom template override for this, using the source code as a starting point?

Skrypt commented 3 years ago

You could maybe take a look at what @agriffard did with https://github.com/OrchardCMS/TryOrchardCore https://try.orchardcore.net/

So the idea is that tenants can be created from a public form which takes care of the tenants creation and technical details. This same form could easily be private to some administrators on your website.

willnationsdev commented 3 years ago

@Skrypt That approach seems like it would work well, and simple enough to implement. Kinda awkward since it's separate from the Tenants admin page, but shouldn't be that big of a problem. Thanks!

Skrypt commented 3 years ago

Well the tenants admin page is for admins! So the context of having everything configurable makes sense for that matter. Also, instead of changing the current source code you can simply have a module that does that for you and customize it for your needs which makes also sense 😉

willnationsdev commented 3 years ago

@Skrypt

Also, instead of changing the current source code you can simply have a module that does that for you and customize it for your needs which makes also sense

This is what we were planning to do in the short-term if it wasn't already supported.

Well the tenants admin page is for admins! So the context of having everything configurable makes sense for that matter.

Yes, but for the long-term, it would be a nice QOL improvement to allow people to define key-value pairs of connection information which can then populate those areas without having to use a separate section of the Admin menu or implement their own controller/frontend for a new form.

Our use case is that we will be making many different websites all to the same database, but with different table prefixes. We don't want the connection string rendered on the web page, but we would prefer people to be able to use the default tenants admin web page.

I'm imagining you could just do this somewhere:

{
    "OrchardCore": {
        "Admin": {
            "Tenants": {
                "ConnectionStrings": {
                    "OrganizationSitePreset": {
                        "Provider": "SqlServer",
                        "ConnectionString": "..."
                    }
                }
            }
        }
    }
}

And then the UI would assume the provider, not bother sending the connection string over the network to display/post, and instead only display the remaining inputs on the page.

Of course, it isn't necessary to do any of that. But it would make OC feel a bit more "batteries-included" with that being a supported option. A "would be cool to have" enhancement I guess.

Just musing to myself / wondering if such a thing would be merged if it were implemented.

Skrypt commented 3 years ago

There is actually one other PR which can populate one default tenant based on configuration which I'm currently reviewing : https://github.com/OrchardCMS/OrchardCore/pull/4567

And I'm trying to implement something with Docker which will use this Autosetup feature and populate tenants for each RDB that we want to test code/migration against. https://github.com/OrchardCMS/OrchardCore/pull/7085

So, yes the first PR mentioned here will allow to create one default tenant from a configuration file. Now, I'm trying to make it to create multiple tenants.

So basically I will add some tenants configurations in my appsettings.json file then run docker-compose up and it will create every container required to run OC with each RDB's automagically 😄

Though, a tenants preset right now doesn't exist it would need to be implemented like @jtkech explained.

Skrypt commented 3 years ago

I will think about that one because I'm not sure where it stands in the workflow I have in mind. Maybe we need to define different use cases and figure out how to fix blocking issues.

Here, if you want to use the tenant admin web page you could surely implement something that would take in consideration a configuration key/value coming from the appsettings.json file which would simply include the common database connection string and then hide it in the admin tenants creation UI. If you want to do a PR you're welcome. Please though don't forget to add documentation on that in the tenants module if you do so 😉

Though, normally, you would want to deploy your websites with Docker and have the ability to recreate tenants in your containers automatically for dev purposes. Here your request is more about having a way to let some users create tenants without sharing the connectionstring with them.

Ok, I like that idea please implement it 😄

jtkech commented 3 years ago

Just for infos, just retried the following in appsettings.json (could be any other config source)

"OrchardCore": {
  "DatabaseProvider": "SqlConnection",
  "ConnectionString": "Data Source=.\\SQLEXPRESS;Initial Catalog=Orchard 2.0 Dev;Integrated Security=True"

Directly under the OrchardCore section so that it acts on all tenants, then these pre-configured values are not shown when doing a setup of any tenant. So, as i understand, most of what we need is already there.

The only thing is that when creating / editing a tenant through the admin UI they are shown so that we can change them, but easy to fix or i think having an option to show or not these pre-configured config values

Skrypt commented 3 years ago

Please document it.

willnationsdev commented 3 years ago

@jtkech @Skrypt

Just for infos, just retried the following in appsettings.json (could be any other config source)

It would appear that this is already documented, if I'm not misunderstanding?

https://docs.orchardcore.net/en/dev/docs/reference/core/Shells/#enable-database-shells-configuration

Skrypt commented 3 years ago

Yes my mistake. I was not remembering that one. I've been off dev this summer and memory fails 😉

willnationsdev commented 3 years ago

As such, I don't think the "documentation" label is accurate for this ticket. It's really more requesting feature proposal to add the ability to create preset databases via configuration. At least, as I see it.

deanmarcussen commented 3 years ago

I believe this may have been fixed in https://github.com/OrchardCMS/OrchardCore/pull/7747

Please could someone confirm it achieves the requested behaviour