Add PostgreSQL primary support to the kosmos-ejabberd cookbook #181

greg · 2020-06-10T16:45:44Z

greg commented

2020-06-10 16:45:44 +00:00

Move the PostgreSQL user and database creation to a pg_db recipe
Generate access rights for the ejabberd servers in the pg_db recipe
Connect to the PostgreSQL primary instead of localhost

* Move the PostgreSQL user and database creation to a `pg_db` recipe * Generate access rights for the ejabberd servers in the `pg_db` recipe * Connect to the PostgreSQL primary instead of localhost Refs #180

greg self-assigned this 2020-06-10 16:45:44 +00:00

raucao commented

2020-06-11 09:52:55 +00:00

I find it rather complicated that we should specifically allow PostgreSQL access per app and database, instead of allowing our own servers to connect to the primary by default. Why wouldn't a replica be allowed to connect to the primary just because it's a replica? It does that for replication anyway, and thus should needs access to all databases in the first place.

The fact that configuring simple database access requires an entire recipe should ring some alarm bells in my opinion.

I find it rather complicated that we should specifically allow PostgreSQL access per app and database, instead of allowing our own servers to connect to the primary by default. Why wouldn't a replica be allowed to connect to the primary just because it's a replica? It does that for replication anyway, and thus should needs access to all databases in the first place. The fact that configuring simple database access requires an entire recipe should ring some alarm bells in my opinion.

greg commented

2020-06-11 14:27:11 +00:00

Replication is a special right, this is defined here for replicas:

		site-cookbooks/kosmos-postgresql/recipes/default.rb
		Lines 48 to 55 in 81403b7cb9
	
				  postgresql_access "#{replica[:hostname]} replication" do

				    access_type "host"

				    access_db "replication"

				    access_user "replication"

				    access_addr "#{replica[:ipaddress]}/32"

				    access_method "md5"

				    notifies :reload, "service[#{postgresql_service}]", :immediately

				  end

.

A server connecting to the PostgreSQL primary as a client is not necessarily a replica, but as you said it could be done for all databases at once. I had the idea of definining each database as we need a recipe anyway to create the postgreSQL user and database for each service. Allowing access to all databases for a server could be done as part of the primary recipe, however it means the list of all services that use PostgreSQL would live there instead of the recipe for the service. I'm not opposed to doing that, I just thought doing it per database was clearer.

Replication is a special right, this is defined here for replicas: https://gitea.kosmos.org/kosmos/chef/src/commit/81403b7cb9858c48cc9262d4b79f1a63f5766016/site-cookbooks/kosmos-postgresql/recipes/default.rb#L48-L55. A server connecting to the PostgreSQL primary as a client is not necessarily a replica, but as you said it could be done for all databases at once. I had the idea of definining each database as we need a recipe anyway to create the postgreSQL user and database for each service. Allowing access to all databases for a server could be done as part of the primary recipe, however it means the list of all services that use PostgreSQL would live there instead of the recipe for the service. I'm not opposed to doing that, I just thought doing it per database was clearer.

raucao commented

2020-06-11 15:01:54 +00:00

I didn't mean replication per se, but all Hetzner machines would definitely be replicas for now anyway.

I mean that any of our Hetzner machines should be able to connect to the Posgtgres server in the first place. The actual authentication is then the username/password for the database, and doesn't require whitelisting the host per database.

I didn't mean replication per se, but all Hetzner machines would definitely be replicas for now anyway. I mean that any of our Hetzner machines should be able to connect to the Posgtgres server in the first place. The actual authentication is then the username/password for the database, and doesn't require whitelisting the host per database.

greg commented

2020-06-11 15:30:04 +00:00

I'm going to look into it, there has to be a way that looks good in Chef code. Not having to run Chef again on the primary every time we add a new service will be useful

raucao commented

2020-06-11 15:42:13 +00:00

It's not about looking good, it's about adding unnecessary complexities and code.

greg commented

2020-06-11 16:22:21 +00:00

Pushed 6f696d7, what do you think?

Pushed 6f696d7, what do you think?

raucao commented

2020-06-11 17:02:11 +00:00

At least it's half the LOC. Looks much better to me.

However, I don't quite understand why it still has to be searching for every single app. Isn't there an easier way to know that a machine has roles/attributes that require postgres access? If it's not bound to the application, then it can never be out of sync.

At least it's half the LOC. Looks much better to me. However, I don't quite understand why it still has to be searching for every single app. Isn't there an easier way to know that a machine has roles/attributes that require postgres access? If it's not bound to the application, then it can never be out of sync.

greg commented

2020-06-12 14:58:11 +00:00

However, I don't quite understand why it still has to be searching for every single app. Isn't there an easier way to know that a machine has roles/attributes that require postgres access? If it's not bound to the application, then it can never be out of sync.

I have found a better way. See ee9c241a4d, I have added a postgresql_client role

> However, I don't quite understand why it still has to be searching for every single app. Isn't there an easier way to know that a machine has roles/attributes that require postgres access? If it's not bound to the application, then it can never be out of sync. I have found a better way. See ee9c241a4d, I have added a `postgresql_client` role

👍 1

raucao approved these changes 2020-06-12 15:27:12 +00:00

raucao left a comment

LGTM now. 👍

LGTM now. :+1:

greg referenced this issue from a commit

2020-06-19 14:46:52 +00:00

Merge pull request 'Add PostgreSQL primary support to the kosmos-ejabberd cookbook' (#181) from feature/180-ejabberd_pg_primary into master

greg closed this pull request

2020-06-19 14:46:52 +00:00

greg deleted branch feature/180-ejabberd_pg_primary

2020-06-19 14:46:59 +00:00