From f0b784fe7f1a5e88e455a21be9507f5375f9b085 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Tue, 23 Apr 2019 16:16:53 +0200 Subject: [PATCH] Manage pg_ident.conf with Patroni (#1037) This functionality works similarly to the `pg_hba`: If the `postgresql.pg_ident` is defined in the config file or DCS, Patroni will write its value to pg_ident.conf, however, if `postgresql.parameters.ident_file` is defined, Patroni will assume that pg_ident is managed from outside and not update the file. --- docs/SETTINGS.rst | 3 +++ patroni/postgresql.py | 37 ++++++++++++++++++++++++++++++------- tests/test_ha.py | 2 +- tests/test_patroni.py | 2 +- tests/test_postgresql.py | 3 ++- 5 files changed, 37 insertions(+), 10 deletions(-) diff --git a/docs/SETTINGS.rst b/docs/SETTINGS.rst index 9be6edc1..a1280e28 100644 --- a/docs/SETTINGS.rst +++ b/docs/SETTINGS.rst @@ -159,6 +159,9 @@ PostgreSQL - **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 :ref:`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\_ident**: list of lines that Patroni will use to generate ``pg_ident.conf``. Together with :ref:`dynamic configuration ` it simplifies management of ``pg_ident.conf``. + - **- mapname1 systemname1 pguser1**. + - **- mapname1 systemname2 pguser2**. - **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 1cc8c5a7..7beda6ef 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -130,6 +130,7 @@ class Postgresql(object): self._postgresql_base_conf_name = config_base_name + '.base.conf' self._postgresql_base_conf = os.path.join(self._config_dir, self._postgresql_base_conf_name) self._pg_hba_conf = os.path.join(self._config_dir, 'pg_hba.conf') + self._pg_ident_conf = os.path.join(self._config_dir, 'pg_ident.conf') self._recovery_conf = os.path.join(self._data_dir, 'recovery.conf') self._trigger_file = config.get('recovery_conf', {}).get('trigger_file') or 'promote' self._trigger_file = os.path.abspath(os.path.join(self._data_dir, self._trigger_file)) @@ -167,7 +168,7 @@ 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(): + if self._replace_pg_hba() or self._replace_pg_ident(): self.reload() elif self.role == 'master': self.set_role('demoted') @@ -303,7 +304,7 @@ class Postgresql(object): self._superuser = config['authentication'].get('superuser', {}) server_parameters = self.get_server_parameters(config) - conf_changed = hba_changed = local_connection_address_changed = pending_restart = False + conf_changed = hba_changed = ident_changed = local_connection_address_changed = pending_restart = False if self.state == 'running': changes = CaseInsensitiveDict({p: v for p, v in server_parameters.items() if '.' not in p}) changes.update({p: None for p in self._server_parameters.keys() if not ('.' in p or p in changes)}) @@ -356,6 +357,9 @@ class Postgresql(object): if not server_parameters.get('hba_file') and config.get('pg_hba'): hba_changed = self.config.get('pg_hba', []) != config['pg_hba'] + if not server_parameters.get('ident_file') and config.get('pg_ident'): + ident_changed = self.config.get('pg_ident', []) != config['pg_ident'] + self.config = config self._pending_restart = pending_restart self._server_parameters = server_parameters @@ -370,7 +374,10 @@ class Postgresql(object): if hba_changed: self._replace_pg_hba() - if conf_changed or hba_changed: + if ident_changed: + self._replace_pg_ident() + + if conf_changed or hba_changed or ident_changed: logger.info('PostgreSQL configuration items changed, reloading configuration.') self.reload() elif not pending_restart: @@ -902,6 +909,7 @@ class Postgresql(object): self._write_postgresql_conf(configuration) self.resolve_connection_addresses() self._replace_pg_hba() + self._replace_pg_ident() options = ['--{0}={1}'.format(p, configuration[p]) for p in self.CMDLINE_OPTIONS if p in configuration and p != 'wal_keep_segments'] @@ -1128,8 +1136,7 @@ class Postgresql(object): if self._running_custom_bootstrap or 'hba_file' not in self._server_parameters: f.write("hba_file = '{0}'\n".format(self._pg_hba_conf.replace('\\', '\\\\'))) if 'ident_file' not in self._server_parameters: - s = "ident_file = '{0}'\n".format(os.path.join(self._config_dir, 'pg_ident.conf').replace('\\', '\\\\')) - f.write(s) + f.write("ident_file = '{0}'\n".format(self._pg_ident_conf.replace('\\', '\\\\'))) def is_healthy(self): if not self.is_running(): @@ -1137,7 +1144,7 @@ class Postgresql(object): return False return True - def write_pg_hba(self, config): + def append_pg_hba(self, config): if not self._server_parameters.get('hba_file') and not self.config.get('pg_hba'): with open(self._pg_hba_conf, 'a') as f: f.write('\n{}\n'.format('\n'.join(config))) @@ -1174,6 +1181,21 @@ class Postgresql(object): f.write('{0}\n'.format(line)) return True + def _replace_pg_ident(self): + """ + Replace pg_ident.conf content in the PGDATA if ident_file is not defined in the + `postgresql.parameters` and pg_ident is defined in the `postgresql` section. + + :returns: True if pg_ident.conf was rewritten. + """ + + if not self._server_parameters.get('ident_file') and self.config.get('pg_ident'): + with open(self._pg_ident_conf, 'w') as f: + f.write(self._CONFIG_WARNING_HEADER) + for line in self.config['pg_ident']: + 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 @@ -1684,7 +1706,7 @@ $$""".format(name, ' '.join(options)), name, password, password) config = config[method] else: do_initialize = self._initdb - return do_initialize(config) and self.write_pg_hba(pg_hba) and self.save_configuration_files() \ + return do_initialize(config) and self.append_pg_hba(pg_hba) and self.save_configuration_files() \ and self._configure_server_parameters() and self.start() def post_bootstrap(self, config, task): @@ -1709,6 +1731,7 @@ $$""".format(name, ' '.join(options)), name, password, password) os.unlink(self._pg_hba_conf) self.restore_configuration_files() self._write_postgresql_conf() + self._replace_pg_ident() # at this point there should be no recovery.conf if os.path.isfile(self._recovery_conf) or os.path.islink(self._recovery_conf): os.unlink(self._recovery_conf) diff --git a/tests/test_ha.py b/tests/test_ha.py index 282768c8..ca28b926 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -145,7 +145,7 @@ def run_async(self, func, args=()): @patch.object(Postgresql, 'data_directory_empty', Mock(return_value=False)) @patch.object(Postgresql, 'controldata', Mock(return_value={'Database system identifier': SYSID})) @patch.object(Postgresql, 'sync_replication_slots', Mock()) -@patch.object(Postgresql, 'write_pg_hba', Mock()) +@patch.object(Postgresql, 'append_pg_hba', Mock()) @patch.object(Postgresql, 'write_pgpass', Mock(return_value={})) @patch.object(Postgresql, 'write_recovery_conf', Mock()) @patch.object(Postgresql, 'query', Mock()) diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 9a4613e6..324f5a9e 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -24,7 +24,7 @@ class MockFrozenImporter(object): @patch('time.sleep', Mock()) @patch('subprocess.call', Mock(return_value=0)) @patch('psycopg2.connect', psycopg2_connect) -@patch.object(Postgresql, 'write_pg_hba', Mock()) +@patch.object(Postgresql, 'append_pg_hba', Mock()) @patch.object(Postgresql, '_write_postgresql_conf', Mock()) @patch.object(Postgresql, 'write_recovery_conf', Mock()) @patch.object(Postgresql, 'is_running', Mock(return_value=MockPostmaster())) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index e0288c04..d72072cd 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -200,6 +200,7 @@ class TestPostgresql(unittest.TestCase): 'parameters': self._PARAMETERS, 'recovery_conf': {'foo': 'bar'}, 'pg_hba': ['host all all 0.0.0.0/0 md5'], + 'pg_ident': ['krb realm postgres'], 'callbacks': {'on_start': 'true', 'on_stop': 'true', 'on_reload': 'true', 'on_restart': 'true', 'on_role_change': 'true'}}) self.p._callback_executor = Mock() @@ -785,7 +786,7 @@ class TestPostgresql(unittest.TestCase): def test_reload_config(self): parameters = self._PARAMETERS.copy() parameters.pop('f.oo') - config = {'pg_hba': [''], 'use_unix_socket': True, 'authentication': {}, + config = {'pg_hba': [''], 'pg_ident': [''], 'use_unix_socket': True, 'authentication': {}, 'retry_timeout': 10, 'listen': '*', 'parameters': parameters} self.p.reload_config(config) parameters['b.ar'] = 'bar'