From b576e693622c3cf745fd9c444c50550d460546bc Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 23 Jun 2017 12:38:25 +0200 Subject: [PATCH] Manage pg_hba.conf via patroni config or dynamic_configuration (#458) So far Patroni was populating pg_hba.conf only when running bootstrap code and after that it was not very handy to manage it's content, because it was necessary to login to every node, change pg_hba.conf manually and run pg_ctl reload. This commit intends to fix it and give Patroni control over pg_hba.conf. It is possible to define pg_hba.conf content via `postgresql.pg_hba` in the patroni configuration file or in the `DCS/config` (dynamic configuration). If the `hba_file` is defined in the `postgresql.parameters`, Patroni will ignore `postgresql.pg_hba`. --- docs/SETTINGS.rst | 5 ++++- patroni/postgresql.py | 43 ++++++++++++++++++++++++++++++++-------- tests/test_postgresql.py | 24 ++++++++++++++-------- 3 files changed, 55 insertions(+), 17 deletions(-) diff --git a/docs/SETTINGS.rst b/docs/SETTINGS.rst index 7a035b86..02c66a46 100644 --- a/docs/SETTINGS.rst +++ b/docs/SETTINGS.rst @@ -87,8 +87,11 @@ PostgreSQL - **use\_unix\_socket**: specifies that Patroni should prefer to use unix sockets to connect to the cluster. Default value is ``false``. If ``unix_socket_directories`` is definded, Patroni will use first suitable value from it to connect to the cluster and fallback to tcp if nothing is suitable. If ``unix_socket_directories`` is not specified in ``postgresql.parameters``, Patroni will assume that default value should be used and omit ``host`` from connection parameters. - **pgpass**: path to the `.pgpass `__ password file. Patroni creates this file before executing pg\_basebackup, the post_init script and under some other circumstances. The location must be writable by Patroni. - **recovery\_conf**: additional configuration settings written to recovery.conf when configuring follower. -- **custom_conf** : path to an optional custom ``postgresql.conf`` file, that will be used in place of ``postgresql.base.conf``. The file must exist on all cluster nodes, be readable by PostgreSQL and will be included from its location on the real ``postgresql.conf``. Note that Patroni will not monitor this file for changes, nor backup it. However, its settings can still be overriden by Patroni's own configuration facilities - see `dynamic configuration `__ for details. +- **custom\_conf** : path to an optional custom ``postgresql.conf`` file, that will be used in place of ``postgresql.base.conf``. The file must exist on all cluster nodes, be readable by PostgreSQL and will be included from its location on the real ``postgresql.conf``. Note that Patroni will not monitor this file for changes, nor backup it. However, its settings can still be overriden by Patroni's own configuration facilities - see `dynamic configuration `__ for details. - **parameters**: list of configuration settings for Postgres. Many of these are required for replication to work. +- **pg\_hba**: list of lines that Patroni will use to generate ``pg_hba.conf``. This parameter has higher priority than ``bootstrap.pg_hba``. Together with `dynamic configuration `__ it simplifies management of ``pg_hba.conf``. + - **- host all all 0.0.0.0/0 md5**. + - **- host replication replicator 127.0.0.1/32 md5**: A line like this is required for replication. - **pg\_ctl\_timeout**: How long should pg_ctl wait when doing ``start``, ``stop`` or ``restart``. Default value is 60 seconds. - **use\_pg\_rewind**: try to use pg\_rewind on the former leader when it joins cluster as a replica. - **remove\_data\_directory\_on\_rewind\_failure**: If this option is enabled, Patroni will remove postgres data directory and recreate replica. Otherwise it will try to follow the new leader. Default value is **false**. diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 6a8d1d4b..e8dbb90c 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -157,6 +157,8 @@ class Postgresql(object): self.set_state('running') self.set_role('master' if self.is_leader() else 'replica') self._write_postgresql_conf() # we are "joining" already running postgres + if self._replace_pg_hba(): + self.reload() @property def _configuration_to_save(self): @@ -280,7 +282,7 @@ class Postgresql(object): self._superuser = config['authentication'].get('superuser', {}) server_parameters = self.get_server_parameters(config) - local_connection_address_changed = pending_reload = pending_restart = False + conf_changed = hba_changed = local_connection_address_changed = pending_restart = False if self.state == 'running': changes = {p: v for p, v in server_parameters.items() if '.' not in p} changes.update({p: None for p, v in self._server_parameters.items() if not ('.' in p or p in changes)}) @@ -308,24 +310,27 @@ class Postgresql(object): or r[0] in ('listen_addresses', 'port'): local_connection_address_changed = True else: - pending_reload = True + conf_changed = True for param in changes: if param in server_parameters: logger.warning('Removing invalid parameter `%s` from postgresql.parameters', param) server_parameters.pop(param) # Check that user-defined-paramters have changed (parameters with period in name) - if not pending_reload: + if not conf_changed: for p, v in server_parameters.items(): if '.' in p and (p not in self._server_parameters or str(v) != str(self._server_parameters[p])): - pending_reload = True + conf_changed = True break - if not pending_reload: + if not conf_changed: for p, v in self._server_parameters.items(): if '.' in p and (p not in server_parameters or str(v) != str(server_parameters[p])): - pending_reload = True + conf_changed = True break + if not config['parameters'].get('hba_file') and config.get('pg_hba'): + hba_changed = self.config.get('pg_hba', []) != config['pg_hba'] + self.config = config self._pending_restart = pending_restart self._server_parameters = server_parameters @@ -334,9 +339,15 @@ class Postgresql(object): if not local_connection_address_changed: self.resolve_connection_addresses() - if pending_reload: + if conf_changed: self._write_postgresql_conf() + + if hba_changed: + self._replace_pg_hba() + + if conf_changed or hba_changed: self.reload() + self._is_leader_retry.deadline = self.retry.deadline = config['retry_timeout']/2.0 @property @@ -501,7 +512,8 @@ class Postgresql(object): if pwfile: os.remove(pwfile) if ret: - self.write_pg_hba(config.get('pg_hba', [])) + if not self.config['parameters'].get('hba_file') and not self.config.get('pg_hba'): + self.write_pg_hba(config.get('pg_hba', [])) self._major_version = self.get_major_version() self._server_parameters = self.get_server_parameters(self.config) else: @@ -789,6 +801,7 @@ class Postgresql(object): self._pending_restart = False self._write_postgresql_conf() + self._replace_pg_hba() self.resolve_connection_addresses() opts = {p: self._server_parameters[p] for p in self.CMDLINE_OPTIONS if p in self._server_parameters} @@ -1060,6 +1073,20 @@ class Postgresql(object): with open(os.path.join(self._data_dir, 'pg_hba.conf'), 'a') as f: f.write('\n{}\n'.format('\n'.join(config))) + def _replace_pg_hba(self): + """ + Replace pg_hba.conf content in the PGDATA if hba_file is not defined in the + `postgresql.parameters` and pg_hba is defined in `postgresql` configuration section. + + :returns: True if pg_hba.conf was rewritten. + """ + if not self.config['parameters'].get('hba_file') and self.config.get('pg_hba'): + with open(os.path.join(self._data_dir, 'pg_hba.conf'), 'w') as f: + f.write('# Do not edit this file manually!\n# It will be overwritten by Patroni!\n') + for line in self.config['pg_hba']: + f.write('{0}\n'.format(line)) + return True + def primary_conninfo(self, member): if not (member and member.conn_url) or member.name == self.name: return None diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index a7a477bd..b2376d7b 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -178,6 +178,7 @@ class TestPostgresql(unittest.TestCase): 'use_pg_rewind': True, 'pg_ctl_timeout': 'bla', 'parameters': self._PARAMETERS, 'recovery_conf': {'foo': 'bar'}, + 'pg_hba': ['host all all 0.0.0.0/0 md5'], 'callbacks': {'on_start': 'true', 'on_stop': 'true', 'on_restart': 'true', 'on_role_change': 'true', 'on_reload': 'true' @@ -498,15 +499,22 @@ class TestPostgresql(unittest.TestCase): with patch.object(Postgresql, 'run_bootstrap_post_init', Mock(return_value=False)): self.assertRaises(PostgresException, self.p.bootstrap, {}) - self.p.bootstrap({'users': {'replicator': {'password': 'rep-pass', 'options': ['replication']}}, - 'pg_hba': ['host replication replicator 127.0.0.1/32 md5', - 'hostssl all all 0.0.0.0/0 md5', - 'host all all 0.0.0.0/0 md5'], - 'post_init': '/bin/false'}) + config = {'users': {'replicator': {'password': 'rep-pass', 'options': ['replication']}}} + + self.p.bootstrap(config) with open(os.path.join(self.data_dir, 'pg_hba.conf')) as f: lines = f.readlines() - assert 'host replication replicator 127.0.0.1/32 md5\n' in lines - assert 'host all all 0.0.0.0/0 md5\n' in lines + self.assertTrue('host all all 0.0.0.0/0 md5\n' in lines) + + self.p.config.pop('pg_hba') + config.update({'post_init': '/bin/false', + 'pg_hba': ['host replication replicator 127.0.0.1/32 md5', + 'hostssl all all 0.0.0.0/0 md5', + 'host all all 0.0.0.0/0 md5']}) + self.p.bootstrap(config) + with open(os.path.join(self.data_dir, 'pg_hba.conf')) as f: + lines = f.readlines() + self.assertTrue('host replication replicator 127.0.0.1/32 md5\n' in lines) def test_run_bootstrap_post_init(self): with patch('subprocess.call', Mock(return_value=1)): @@ -593,7 +601,7 @@ class TestPostgresql(unittest.TestCase): def test_reload_config(self): parameters = self._PARAMETERS.copy() parameters.pop('f.oo') - config = {'use_unix_socket': True, 'authentication': {}, + config = {'pg_hba': [''], 'use_unix_socket': True, 'authentication': {}, 'retry_timeout': 10, 'listen': '*', 'parameters': parameters} self.p.reload_config(config) parameters['b.ar'] = 'bar'