[FIX] 20-postgres-wait: avoid assuming you have permission to list database

[vaab]

When having to connect to external database access, you often only have access to a specific user/password on a single database, with no rights to access 'postgres' database, and thus no rights to list databases. (Using: REVOKE ALL ON DATABASE postgres FROM PUBLIC;, which is a common security practice). Notice that OVH for instance seems to do that on their mutualized postgres offers

If odoo itself has known issues into supporting this scenario, it is easily patchable (cf: odoo/odoo#12850 (comment) ).

If you can't list databases, you probably have the $PGDATABASE set, so I triggered the change on the existence of this variable. Although, I'm not sure how other people use this image, so I'd be happy to have your feedback on the pertinence of this patch in your perspective.

Also, it should probably be a good idea to set $DB_FILTER to be empty (to override the default of the current image which is .*, which by defaults, prevents odoo to actually use db_name option, cf: https://github.com/odoo/odoo/blob/master/odoo/service/db.py#L413-L418 ).

With my patch, the tecnativa image won't stop forever asking for something it can't have in the aforementioned scenario.

Many thanks for your work on this image.

[ap-wtioit]

We never had this issue (using different users for different databases and list was always possible) however we have a patched version that detects when the password for the postgres user doesn't match (and the current loop would loop forever) and instead exits the container so we are able to detect it as an erroneous odoo container. (and we also call psql --list with timeout 5s psql --list to prevent issues were the connection isn't quite possible yet.)

Edit: the linked issue describes running Odoo without Create DB privileges, i'm not sure that is supported as we have run into issues when Odoo doesn't create the database itself (with the correct settings). How are you creating your databases?

[ap-wtioit]

this breaks waiting for postgres container when $PGDATABASE is defined and the database would be created by Odoo but the database does not exist yet when entrypoint.d/20-postgres-wait is running.

[ap-wtioit]

error from postgres during tests:

psql: error: connection to server at "db" (172.16.2.194), port 5432 failed: FATAL:  database "prod" does not exist

[vaab]

Good point, what about a simple or between the two ? Would that fix this issue for you ?
For the case I described, this would allow to pass this script safely, and for your case also, while making sure we checked the access (HOST/PORT/USER/PASSWORD) to postgres...

[ap-wtioit]

yes an or should work, as the (error) output is redirected to /dev/null no errors are shown by default anyways so having a try a the correct database first seems ok.

Edit: clarified that i was worried about errors showing up during startup.

[vaab]

Edit: the linked issue describes running Odoo without Create DB privileges, i'm not sure that is supported as we have run into issues when Odoo doesn't create the database itself (with the correct settings). How are you creating your databases?

Hmm, there might be some misunderstanding on various levels:

• The linked issue (the odoo bug) is about not having CONNECT right to 'postgres' originally, which has various consequences, but the core of the issue is that odoo is expecting way too much (createdb, listing db) and this isn't realistical in standard external mutualized offers. Odoo should probably elegantly probe what rights it has and avoid failing in these highly professional secured setups : in these cases, we give you the standard HOST/PORT/USER/PASSWORD and DATABASE...
• There are no "magic" on the part of odoo when it creates databases. It's a CREATE DATABASE with some CREATE EXTENSION (cf), there are a few details in this function that can be what you refer as "correct settings" that are actually defaults settings in most installation nowadays. In fact, in our experience we nearly always create it by hand with these commands prior to running odoo for internal reasons.
• odoo works perfectly well once we divert it's attempts to connect to the database 'postgres'. Not having listdb or createdb is indeed not supported as-is by current odoo code, but only requires a very simple patch as already mentionned. Of course, all the "database manager" is useless (except the ability to dump/restore the given database).

To put it simply, the image you provide introduce an additional unnecessary burden to integrators wishing to connect their odoo to an external postgres provider or any standard production-ready restricted access. It is done by assuming wrongly that odoo will always have createdb or listdb access.

And that assumption is unnecessary if you just want to test if your database params are okay, and the postgres connection is up and available before running odoo.

[vaab]

I updated the commit. Is that better ? Many thanks for your attention. I'll be happy to make more change if anything is amiss.

[josep-tecnativa]

Can you rebase, please?

[vaab]

Done ;-)

[josep-tecnativa]

What do you think @ap-wtioit ? Is it OK for you?

[ap-wtioit]

@josep-tecnativa yes it looks like a good improvement to me. my initial feedback was based on the tests failing for the change and the code looks good now.