Multi-database support and other fixes

[antespi]

• Support for multi-database
• Better checking of private IP addresses
• Get current data_dir, not default
• Use of official ACME Tiny library from pip, as a dependency

[hbrunn]

I don't think this is true. We run quite some instances with multiple databases, and there the dbfilter (which comes from a header in our case, but that should be the same mechanism) works fine.

[antespi]

This issue is not related with dbfilter. If there are two or more DBs after applying dbfilter Odoo can't preload one of them, and HTTP request for ACME validation come with no DB selection, so only web addon is available for routing map.

I've check in one production environment with this use case, and there are two issues:

1. letsencrypt controller not found. Then we need --load=web,letsencrypt
2. We have no request.env defined, so we can't call to models. Then we can only call to static methods.

[hbrunn]

ah, this way. Well, the db filter must resolve to exactly one database anyways, because otherwise it's not defined in which data dir you end up. And we want a database specific data directory because that's the only place where it's safe to assume that we can write, and because we don't want different databases to overwrite each other's certificates/keys. Then I also understand you followup changes to remove env here, but I don't agree with them.

[pedrobaeza]

But we usually have several DBs under the same dbfilter (real DB and test DB for example), and with this, it works also for this case. And you don't need here the env for anything more than to call some methods, so please accept the change.

[antespi]

data_dir is defined in odoo config file, not in database, AFAIK. It's safe to assume that we can write in that directory because there Odoo writes filestore and sessions.

we don't want different databases to overwrite each other's certificates/keys

I'm looking for a way to change well-known URI to add database parameter in ACME protocol. This way we can load registry for that DB in controller, and safe certificates/keys in a subdirectory by database name.

[hbrunn]

unfortunately, it doesn't. With storage per database (in the original version, you have one key/certificate per database, because the letsencrypt-folder is a sibling of the filestore folder), we can separate requesting certificates for different scenarios on the same instance. This won't work with this implementation proposal. For any SaaS-scenario, it's also completely unviable.

What you could do before is

server {
    server_name your_domain;
    location /.well-known {
        proxy_set_header X-Odoo-dbfilter ^your_database_with_letsencrypt_installed$;
    }

in nginx (adapt for other proxies) to fix your problem, provided dbfilter_from_header is installed, and I'd rather use that for supporting multiple databases on the same domain. As soon as you get serious with the website module, you'll need one domain per database anyways, so yes, I consider everything else than a dbfilter to point to the right database to use a hack.

[antespi]

in the original version, you have one key/certificate per database, because the letsencrypt-folder is a sibling of the filestore folder

In the original version, default data_dir was used (.local/share/Odoo) instead of configured data_dir. So, there was only one letsencrypt folder for each Odoo instance, even with dbfilter actived and filtering databases (SaaS use case).

So, why this approach is not working in SaaS use case?

As soon as you get serious with the website module, you'll need one domain per database anyways

You're right, but this addon is not only for website scenarios, but for traditional ERP backend scenarios as well. With this implementation you have both scenarios covered.

[hbrunn]

oh dear, so I did it wrong in the first place. I wanted to avoid having to migrate existing installations, but this won't be necessary indeed, and the case I've in mind doesn't work with either. Sorry for the noise then.

Just make clear then that loading the module server wide is only necessary if the dbfilter doesn't restrict to exactly one database.

[antespi]

Yes, that's why I point to check /web/database/selector and if there are two or more databases in combo list, then --load is needed.

[hbrunn]

perfect, thanks

[hbrunn]

thanks, I never found the time to react on comments there. Just a few comments, then I'll merge. Don't you want to add yourself as contributor too?

[antespi]

You're right. I've forgotten to add myself as contributor. Thanks

[pedrobaeza]

@antespi, please add Tecnativa in the module manifest and squash the commits.