Bootstrapped databases with best practice role setup

[FxKu]

At the moment, users are left alone with setting up read and write privileges in PostgreSQL. A best practice setup could look like this:

• three NOLOGIN roles: a DB owner, a DB reader and DB writer
• default access privileges for these three roles
• three (optional) LOGIN roles which inherit the privileges from the NOLOGIN roles and can be granted to application users

This PR adds a new section preparedDatabases to the manifest as the old databases field can not be changed without breaking changes. The whole role setup is hidden from the user. Application roles would still be specified under the users field. A few more features are currently included, but can be removed as it adds quite some complexity:

• If the preparedDatabases section is empty, create a database which is named like the Postgres cluster manifest
• Allow users to define database schemas for each bootstrapped database (default schema would be called data)
• Optional NOLOGIN and LOGIN roles per schema
• For each preparedDatabase allow a string map to specify extensions and which schema they should run (addresses Allow db extensions to be configured declaratively #490)

As a core principle, I tried to only use go maps of complex objects to allow for non-breaking future additions. But in general, we do not aim for adding more complexity. The operator should not become a database migration tool where you specify any DDL stuff. E.g. schemas and extension might already be too much.

[sdudoladov]

Application roles would still be specified under the users field

With this change, what would be the preferred way to create application roles ? With the old users section or with the new login roles created in this PR ?

[sdudoladov]

1. Looks like the default DB with the same name as the cluster and its reader/writer/owner roles is created even if the manifest lacks the preparedDatabases section
2. This scenario does not work:

• Create acid-minimal-cluster that already has the foo database
• Update its manifest with

   preparedDatabases:
     foo:
        defaultUsers: true

That prints a bunch of confusing log messages and report successful update.

time="2020-04-27T05:10:30Z" level=info msg="syncing prepared databases" cluster-name=default/acid-minimal-cluster pkg=cluster worker=0
time="2020-04-27T05:10:30Z" level=info msg="creating database schema \"data\" owner \"foo_data_owner\"" cluster-name=default/acid-minimal-cluster pkg=cluster worker=0
time="2020-04-27T05:10:30Z" level=debug msg="closing database connection" cluster-name=default/acid-minimal-cluster pkg=cluster worker=0
time="2020-04-27T05:10:30Z" level=error msg="could not sync prepared databases: could not execute create database schema: pq: role \"foo_owner\" does not exist" cluster-name=default/acid-minimal-cluster pkg=cluster worker=0
time="2020-04-27T05:10:30Z" level=info msg="cluster has been updated" cluster-name=default/acid-minimal-cluster pkg=controller worker=0

No schema is created / no role change happens.

The next Sync fails with

time="2020-04-27T05:22:04Z" level=debug msg="syncing database schemas" cluster-name=default/acid-minimal-cluster pkg=cluster
time="2020-04-27T05:22:04Z" level=info msg="creating database schema \"data\" owner \"foo_data_owner\"" cluster-name=default/acid-minimal-cluster pkg=cluster
time="2020-04-27T05:22:04Z" level=debug msg="closing database connection" cluster-name=default/acid-minimal-cluster pkg=cluster
time="2020-04-27T05:22:04Z" level=warning msg="error while syncing cluster state: could not sync database schemas: could not execute create database schema: pq: permission denied for database foo" cluster-name=default/acid-minimal-cluster pkg=cluster
time="2020-04-27T05:22:04Z" level=error msg="could not sync cluster: could not sync database schemas: could not execute create database schema: pq: permission denied for database foo" cluster-name=default/acid-minimal-cluster pkg=controller worker=0
time="2020-04-27T05:22:04Z" level=debug msg="cluster already exists" cluster-name=default/acid-minimal-cluster pkg=controller worker=0

Note we can faithfully expect this scenario to happen because there surely will be people who would like to switch from their setup to the recommended one by editing the manifest. In that situation operator should err on the side of caution and report changes in role setup for exisitng DBs should be done manually

3. Bootstrapped is not the best term to describe such DBs because it is easy to confuse with the cluster bootstrap process managd by Patroni.

[sdudoladov]

I am not 100% sure we want to encourage owner roles with login privileges. Yes, there are cases for it like flyway but it is still a suboptimal practice to use it.

[Jan-M]

Yeah that is a bit difficult. Ideally not the case. But then often by mistake objects are ultimately created and owned by the login role. @CyberDem0n any thoughts?

[FxKu]

1. fixed
2. logs should be clearer now. There was indeed a bug, that roles were not created on UPDATE. Putting an existing database under the prepared section will fail if the owner is different. But, the owner can be changed under databases to fix this. Might be worth another paragraph in the docs.
3. I removed the word bootstrapped from the docs.

[sdudoladov]

But, the owner can be changed under databases to fix this. Might be worth another paragraph in the docs.

As a further note, everything Sync touches after prepared DBs (currently connection pooler), will not be Synced until permission issues are resolved

[sdudoladov]

👍

[FxKu]

👍