Repository: zalando/patroni (python)

Issue:
load available_parameters/0_postgres.yml failed silently when package using zipapp
### What happened?

In some cases I use `zipapp` to distribute patroni package. After upgrading to the new version, I found that pg10/11 cannot properly init standby servers. 
After some research, I found validator rule files under `postgresql/available_parameters` directory failed to load, causing the recovery parameters in recovery.conf to be removed abnormally.

read data files using file system is fragile, I think using Python's import system to locate data files is more robust and reliable. I have implemented a patch using `importlib_resources` (for better compatibility than std `importlib`). 

### How can we reproduce it (as minimally and precisely as possible)?

use zip to distribute patroni (zipapp or zipfile).

### What did you expect to happen?

-

### Patroni/PostgreSQL/DCS version

-   Patroni version: 3.2.2
-   PostgreSQL version: 10
-   DCS (and its version): 


### Patroni configuration file

```yaml
-
```


### patronictl show-config

```yaml
-
```


### Patroni log files

```shell
2024-02-02 10:00:12,123 INFO: Lock owner: postgres-3dfb502f-0-0; I am postgres-3dfb502f-1-0
2024-02-02 10:00:12,132 INFO: Reaped pid=3958, exit status=0
2024-02-02 10:00:12,135 INFO: Reaped pid=3959, exit status=0
2024-02-02 10:00:12,135 INFO: Local timeline=1 lsn=0/4000028
2024-02-02 10:00:12,157 INFO: primary_timeline=1
2024-02-02 10:00:12,158 INFO: Lock owner: postgres-3dfb502f-0-0; I am postgres-3dfb502f-1-0
2024-02-02 10:00:12,158 INFO: starting as a secondary
2024-02-02 10:00:12,160 WARNING: Removing unexpected parameter=primary_slot_name value=postgres_3dfb502f_1_0 from the config
2024-02-02 10:00:12,160 WARNING: Removing unexpected parameter=recovery_target_timeline value=latest from the config
2024-02-02 10:00:12,160 WARNING: Removing unexpected parameter=restore_command value=/home/postgres/bin/pre_failover_restore.sh && pgbackrest archive-get --archive-async=y --process-max=2 --archive-get-queue-max=1GiB %f "%p" from the config
2024-02-02 10:00:12,160 WARNING: Removing unexpected parameter=standby_mode value=on from the config
2024-02-02 10:00:12,165 WARNING: Removing unexpected parameter=jit value=False from the config
2024-02-02 10:00:12.781 CST [3967] LOG:  listening on IPv4 address "0.0.0.0", port 5432
2024-02-02 10:00:12.782 CST [3967] LOG:  listening on Unix socket "/tmp/.s.PGSQL.5432"
2024-02-02 10:00:12,791 INFO: postmaster pid=3967
2024-02-02 10:00:12.815 CST [3967] LOG:  redirecting log output to logging collector process
2024-02-02 10:00:12.815 CST [3967] HINT:  Future log output will appear in directory "/pglog".
/tmp:5432 - rejecting connections
/tmp:5432 - no response
2024-02-02 10:00:22,110 INFO: Lock owner: postgres-3dfb502f-0-0; I am postgres-3dfb502f-1-0
2024-02-02 10:00:22,111 INFO: failed to start postgres
2024-02-02 10:00:32,106 WARNING: Postgresql is not running.
```


### PostgreSQL log files

```shell
-
```


### Have you tried to use GitHub issue search?

- [X] Yes

### Anything else we need to know?

_No response_

Pull request:
Use importlib_resources to read validators file
resolve #3017 

Patch:
diff --git a/features/environment.py b/features/environment.py
index db10c93d..4ed28c57 100644
--- a/features/environment.py
+++ b/features/environment.py
@@ -256,10 +256,18 @@ class PatroniController(AbstractController):
                         'parameters': {
                             'wal_keep_segments': 100,
                             'archive_mode': 'on',
-                            'archive_command': (PatroniPoolController.ARCHIVE_RESTORE_SCRIPT
-                                                + ' --mode archive '
-                                                + '--dirname {} --filename %f --pathname %p').format(
-                                                    os.path.join(self._work_directory, 'data', 'wal_archive'))
+                            'archive_command':
+                                (PatroniPoolController.ARCHIVE_RESTORE_SCRIPT
+                                 + ' --mode archive '
+                                 + '--dirname {} --filename %f --pathname %p').format(
+                                     os.path.join(self._work_directory, 'data',
+                                                  f'wal_archive{str(self._citus_group or "")}')).replace('\\', '/'),
+                            'restore_command':
+                                (PatroniPoolController.ARCHIVE_RESTORE_SCRIPT
+                                 + ' --mode restore '
+                                 + '--dirname {} --filename %f --pathname %p').format(
+                                     os.path.join(self._work_directory, 'data',
+                                                  f'wal_archive{str(self._citus_group or "")}')).replace('\\', '/')
                         }
                     }
                 }
@@ -928,11 +936,6 @@ class PatroniPoolController(object):
         custom_config = {
             'scope': cluster_name,
             'postgresql': {
-                'recovery_conf': {
-                    'restore_command': (self.ARCHIVE_RESTORE_SCRIPT + ' --mode restore '
-                                        + '--dirname {} --filename %f --pathname %p')
-                    .format(os.path.join(self.patroni_path, 'data', 'wal_archive').replace('\\', '/'))
-                },
                 'create_replica_methods': ['no_leader_bootstrap'],
                 'no_leader_bootstrap': self.backup_restore_config({'no_leader': '1'})
             }
diff --git a/features/nostream_node.feature b/features/nostream_node.feature
new file mode 100644
index 00000000..e049a706
--- /dev/null
+++ b/features/nostream_node.feature
@@ -0,0 +1,18 @@
+Feature: nostream node
+
+Scenario: check nostream node is recovering from archive
+    When I start postgres0
+    And I configure and start postgres1 with a tag nostream true
+    Then "members/postgres1" key in DCS has replication_state=in archive recovery after 10 seconds
+    And replication works from postgres0 to postgres1 after 30 seconds
+
+@slot-advance
+Scenario: check permanent logical replication slots are not copied
+    When I issue a PATCH request to http://127.0.0.1:8008/config with {"postgresql": {"parameters": {"wal_level": "logical"}}, "slots":{"test_logical":{"type":"logical","database":"postgres","plugin":"test_decoding"}}}
+    Then I receive a response code 200
+    When I run patronictl.py restart batman postgres0 --force
+    Then postgres0 has a logical replication slot named test_logical with the test_decoding plugin after 10 seconds
+    When I configure and start postgres2 with a tag replicatefrom postgres1
+    Then "members/postgres2" key in DCS has replication_state=streaming after 10 seconds 
+    And postgres1 does not have a replication slot named test_logical
+    And postgres2 does not have a replication slot named test_logical
diff --git a/features/standby_cluster.feature b/features/standby_cluster.feature
index 97c27203..53f8bbac 100644
--- a/features/standby_cluster.feature
+++ b/features/standby_cluster.feature
@@ -26,7 +26,7 @@ Feature: standby cluster
   Scenario: Detach exiting node from the cluster
     When I shut down postgres1
     Then postgres0 is a leader after 10 seconds
-    And "members/postgres0" key in DCS has role=master after 3 seconds
+    And "members/postgres0" key in DCS has role=master after 5 seconds
     When I issue a GET request to http://127.0.0.1:8008/
     Then I receive a response code 200
 
@@ -47,6 +47,7 @@ Feature: standby cluster
     And there is a postgres1_cb.log with "on_role_change standby_leader batman1" in postgres1 data directory
     When I start postgres2 in a cluster batman1
     Then postgres2 role is the replica after 24 seconds
+    And postgres2 is replicating from postgres1 after 10 seconds
     And table foo is present on postgres2 after 20 seconds
     When I issue a GET request to http://127.0.0.1:8010/patroni
     Then I receive a response code 200
diff --git a/features/steps/basic_replication.py b/features/steps/basic_replication.py
index e0d6e5b8..6cb0ac42 100644
--- a/features/steps/basic_replication.py
+++ b/features/steps/basic_replication.py
@@ -46,11 +46,17 @@ def kill_postgres(context, name):
     return context.pctl.stop(name, kill=True, postgres=True)
 
 
+def get_wal_name(context, pg_name):
+    version = context.pctl.query(pg_name, "SHOW server_version_num").fetchone()[0]
+    return 'xlog' if int(version) / 10000 < 10 else 'wal'
+
+
 @step('I add the table {table_name:w} to {pg_name:w}')
 def add_table(context, table_name, pg_name):
     # parse the configuration file and get the port
     try:
         context.pctl.query(pg_name, "CREATE TABLE public.{0}()".format(table_name))
+        context.pctl.query(pg_name, "SELECT pg_switch_{0}()".format(get_wal_name(context, pg_name)))
     except pg.Error as e:
         assert False, "Error creating table {0} on {1}: {2}".format(table_name, pg_name, e)
 
@@ -59,9 +65,7 @@ def add_table(context, table_name, pg_name):
 def toggle_wal_replay(context, action, pg_name):
     # pause or resume the wal replay process
     try:
-        version = context.pctl.query(pg_name, "SHOW server_version_num").fetchone()[0]
-        wal_name = 'xlog' if int(version) / 10000 < 10 else 'wal'
-        context.pctl.query(pg_name, "SELECT pg_{0}_replay_{1}()".format(wal_name, action))
+        context.pctl.query(pg_name, "SELECT pg_{0}_replay_{1}()".format(get_wal_name(context, pg_name), action))
     except pg.Error as e:
         assert False, "Error during {0} wal recovery on {1}: {2}".format(action, pg_name, e)
 
diff --git a/patroni/global_config.py b/patroni/global_config.py
index 7731cf59..ded66f6f 100644
--- a/patroni/global_config.py
+++ b/patroni/global_config.py
@@ -44,19 +44,20 @@ class GlobalConfig(types.ModuleType):
         """
         return bool(cluster and cluster.config and cluster.config.modify_version)
 
-    def update(self, cluster: Optional['Cluster']) -> None:
+    def update(self, cluster: Optional['Cluster'], default: Optional[Dict[str, Any]] = None) -> None:
         """Update with the new global configuration from the :class:`Cluster` object view.
 
         .. note::
-            Global configuration is updated only when configuration in the *cluster* view is valid.
-
             Update happens in-place and is executed only from the main heartbeat thread.
 
         :param cluster: the currently known cluster state from DCS.
+        :param default: default configuration, which will be used if there is no valid *cluster.config*.
         """
         # Try to protect from the case when DCS was wiped out
         if self._cluster_has_valid_config(cluster):
             self.__config = cluster.config.data  # pyright: ignore [reportOptionalMemberAccess]
+        elif default:
+            self.__config = default
 
     def from_cluster(self, cluster: Optional['Cluster']) -> 'GlobalConfig':
         """Return :class:`GlobalConfig` instance from the provided :class:`Cluster` object view.
diff --git a/patroni/ha.py b/patroni/ha.py
index 16c050f3..59b5b6dd 100644
--- a/patroni/ha.py
+++ b/patroni/ha.py
@@ -185,6 +185,9 @@ class Ha(object):
         # used only in backoff after failing a pre_promote script
         self._released_leader_key_timestamp = 0
 
+        # Initialize global config
+        global_config.update(None, self.patroni.config.dynamic_configuration)
+
     def primary_stop_timeout(self) -> Union[int, None]:
         """:returns: "primary_stop_timeout" from the global configuration or `None` when not in synchronous mode."""
         ret = global_config.primary_stop_timeout
diff --git a/patroni/postgresql/available_parameters/__init__.py b/patroni/postgresql/available_parameters/__init__.py
new file mode 100644
index 00000000..8af63cd9
--- /dev/null
+++ b/patroni/postgresql/available_parameters/__init__.py
@@ -0,0 +1,58 @@
+import logging
+import sys
+
+from pathlib import Path
+from typing import Iterator
+
+logger = logging.getLogger(__name__)
+
+if sys.version_info < (3, 9):
+    PathLikeObj = Path
+    conf_dir = Path(__file__).parent
+else:
+    from importlib.resources import files
+
+    if sys.version_info < (3, 11):  # pragma: no cover
+        from importlib.abc import Traversable
+    else:  # pragma: no cover
+        from importlib.resources.abc import Traversable
+
+    PathLikeObj = Traversable
+    conf_dir = files(__name__)
+
+
+def get_validator_files() -> Iterator[PathLikeObj]:
+    """Recursively find YAML files from the current package directory.
+
+    :returns: an iterator of :class:`PathLikeObj` objects representing validator files.
+    """
+    return _traversable_walk(conf_dir.iterdir())
+
+
+def _traversable_walk(tvbs: Iterator[PathLikeObj]) -> Iterator[PathLikeObj]:
+    """Recursively walk through Path/Traversable objects, yielding all YAML files in deterministic order.
+
+    :param tvbs: An iterator over :class:`PathLikeObj` objects, where each object is a file or directory
+                 that potentially contains YAML files.
+
+    :yields: :class:`PathLikeObj` objects representing YAML files found during the traversal.
+    """
+    for tvb in _filter_and_sort_files(tvbs):
+        if tvb.is_file():
+            yield tvb
+        elif tvb.is_dir():
+            yield from _traversable_walk(tvb.iterdir())
+
+
+def _filter_and_sort_files(files: Iterator[PathLikeObj]) -> Iterator[PathLikeObj]:
+    """Sort files by name, and filter out non-YAML files and Python files.
+
+    :param files: A list of files and/or directories to be filtered and sorted.
+
+    :yields: filtered and sorted objects.
+    """
+    for file in sorted(files, key=lambda x: x.name):
+        if file.name.lower().endswith((".yml", ".yaml")) or file.is_dir():
+            yield file
+        elif not file.name.lower().endswith((".py", ".pyc")):
+            logger.info("Ignored a non-YAML file found under `%s` directory: `%s`.", __name__.split('.')[-1], file)
diff --git a/patroni/postgresql/validator.py b/patroni/postgresql/validator.py
index 384db08d..f118c973 100644
--- a/patroni/postgresql/validator.py
+++ b/patroni/postgresql/validator.py
@@ -1,11 +1,11 @@
 import abc
 from copy import deepcopy
 import logging
-import os
 import yaml
 
 from typing import Any, Dict, Iterator, List, MutableMapping, Optional, Tuple, Type, Union
 
+from .available_parameters import get_validator_files, PathLikeObj
 from ..collections import CaseInsensitiveDict, CaseInsensitiveSet
 from ..exceptions import PatroniException
 from ..utils import parse_bool, parse_int, parse_real
@@ -258,10 +258,10 @@ class InvalidGucValidatorsFile(PatroniException):
     """Raised when reading or parsing of a YAML file faces an issue."""
 
 
-def _read_postgres_gucs_validators_file(file: str) -> Dict[str, Any]:
+def _read_postgres_gucs_validators_file(file: PathLikeObj) -> Dict[str, Any]:
     """Read an YAML file and return the corresponding Python object.
 
-    :param file: path to the file to be read. It is expected to be encoded with ``UTF-8``, and to be a YAML document.
+    :param file: path-like object to read from. It is expected to be encoded with ``UTF-8``, and to be a YAML document.
 
     :returns: the YAML content parsed into a Python object. If any issue is faced while reading/parsing the file, then
         return ``None``.
@@ -270,7 +270,7 @@ def _read_postgres_gucs_validators_file(file: str) -> Dict[str, Any]:
         :class:`InvalidGucValidatorsFile`: if faces an issue while reading or parsing *file*.
     """
     try:
-        with open(file, encoding='UTF-8') as stream:
+        with file.open(encoding='UTF-8') as stream:
             return yaml.safe_load(stream)
     except Exception as exc:
         raise InvalidGucValidatorsFile(
@@ -385,21 +385,7 @@ def _load_postgres_gucs_validators() -> None:
                     version_till: null
 
     """
-    conf_dir = os.path.join(
-        os.path.dirname(os.path.abspath(__file__)),
-        'available_parameters',
-    )
-    yaml_files: List[str] = []
-
-    for root, _, files in os.walk(conf_dir):
-        for file in sorted(files):
-            full_path = os.path.join(root, file)
-            if file.lower().endswith(('.yml', '.yaml')):
-                yaml_files.append(full_path)
-            else:
-                logger.info('Ignored a non-YAML file found under `available_parameters` directory: `%s`.', full_path)
-
-    for file in yaml_files:
+    for file in get_validator_files():
         try:
             config: Dict[str, Any] = _read_postgres_gucs_validators_file(file)
         except InvalidGucValidatorsFile as exc:


Test patch:
diff --git a/tests/test_ha.py b/tests/test_ha.py
index 43844c5d..abfb4691 100644
--- a/tests/test_ha.py
+++ b/tests/test_ha.py
@@ -256,8 +256,6 @@ class TestHa(PostgresInit):
         self.p.data_directory_empty = true
         self.ha.cluster = get_cluster_not_initialized_without_leader(
             cluster_config=ClusterConfig(1, {"standby_cluster": {"port": 5432}}, 1))
-        global_config.update(self.ha.cluster)
-        self.ha.cluster = get_cluster_not_initialized_without_leader(cluster_config=ClusterConfig(0, {}, 0))
         self.assertEqual(self.ha.run_cycle(), 'trying to bootstrap a new standby leader')
 
     def test_bootstrap_waiting_for_standby_leader(self):
@@ -323,7 +321,6 @@ class TestHa(PostgresInit):
             self.ha.state_handler.cancellable._process = Mock()
             self.ha._crash_recovery_started -= 600
             self.ha.cluster.config.data.update({'maximum_lag_on_failover': 10})
-            global_config.update(self.ha.cluster)
             self.assertEqual(self.ha.run_cycle(), 'terminated crash recovery because of startup timeout')
 
     @patch.object(Rewind, 'ensure_clean_shutdown', Mock())
@@ -776,7 +773,6 @@ class TestHa(PostgresInit):
         with patch('patroni.ha.logger.info') as mock_info:
             self.ha.fetch_node_status = get_node_status(wal_position=1)
             self.ha.cluster.config.data.update({'maximum_lag_on_failover': 5})
-            global_config.update(self.ha.cluster)
             self.assertEqual(self.ha.run_cycle(), 'no action. I am (postgresql0), the leader with the lock')
             self.assertEqual(mock_info.call_args_list[0][0], ('Member %s exceeds maximum replication lag', 'leader'))
 
@@ -1282,7 +1278,6 @@ class TestHa(PostgresInit):
         self.p.is_running = false
         self.ha.cluster = get_cluster_initialized_with_leader(sync=(self.p.name, 'other'))
         self.ha.cluster.config.data.update({'synchronous_mode': True, 'primary_start_timeout': 0})
-        global_config.update(self.ha.cluster)
         self.ha.has_lock = true
         self.ha.update_lock = true
         self.ha.fetch_node_status = get_node_status()  # accessible, in_recovery
@@ -1391,7 +1386,6 @@ class TestHa(PostgresInit):
         mock_set_sync.reset_mock()
         self.p.sync_handler.current_state = Mock(return_value=(CaseInsensitiveSet(), CaseInsensitiveSet()))
         self.ha.cluster.config.data['synchronous_mode_strict'] = True
-        global_config.update(self.ha.cluster)
         self.ha.run_cycle()
         mock_set_sync.assert_called_once_with(CaseInsensitiveSet('*'))
 
diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py
index ef60b9b0..b6ebc3de 100644
--- a/tests/test_postgresql.py
+++ b/tests/test_postgresql.py
@@ -7,6 +7,7 @@ import time
 
 from copy import deepcopy
 from mock import Mock, MagicMock, PropertyMock, patch, mock_open
+from pathlib import Path
 
 import patroni.psycopg as psycopg
 
@@ -1064,7 +1065,7 @@ class TestPostgresql(BaseTestPostgresql):
     def test__read_postgres_gucs_validators_file(self):
         # raise exception
         with self.assertRaises(InvalidGucValidatorsFile) as exc:
-            _read_postgres_gucs_validators_file('random_file.yaml')
+            _read_postgres_gucs_validators_file(Path('random_file.yaml'))
         self.assertEqual(
             str(exc.exception),
             "Unexpected issue while reading parameters file `random_file.yaml`: `[Errno 2] No such file or directory: "
@@ -1073,17 +1074,32 @@ class TestPostgresql(BaseTestPostgresql):
 
     def test__load_postgres_gucs_validators(self):
         # log messages
-        with patch('os.walk', Mock(return_value=iter([('.', [], ['file.txt', 'random.yaml'])]))), \
-             patch('patroni.postgresql.validator.logger.info') as mock_info, \
+        file1_attrs = {'is_file.return_value': True, 'is_dir.return_value': False}
+        file1_mock = MagicMock(**file1_attrs)
+        file1_mock.name = '__init__.py'
+        file2_attrs = {'is_file.return_value': False, 'is_dir.return_value': True, 'iterdir.return_value': []}
+        file2_mock = MagicMock(**file2_attrs)
+        file2_mock.name = '__pycache__'
+        file3_attrs = {'is_file.return_value': True, 'is_dir.return_value': False}
+        file3_mock = MagicMock(**file3_attrs)
+        file3_mock.name = file3_mock.__str__.return_value = 'random.yaml'
+        file3_mock.open.side_effect = FileNotFoundError('[Errno 2] No such file or directory: random.yaml')
+        file4_attrs = {'is_file.return_value': True, 'is_dir.return_value': False}
+        file4_mock = MagicMock(**file4_attrs)
+        file4_mock.name = 'file.txt'
+        dir_attrs = {'name': 'available_parameters', 'is_file.return_value': False, 'is_dir.return_value': True}
+        dir_mock = MagicMock(**dir_attrs)
+        dir_mock.iterdir.return_value = [file1_mock, file2_mock, file3_mock, file4_mock]
+        with patch('patroni.postgresql.available_parameters.conf_dir', dir_mock), \
+             patch('patroni.postgresql.available_parameters.logger.info') as mock_info, \
              patch('patroni.postgresql.validator.logger.warning') as mock_warning:
             _load_postgres_gucs_validators()
-            mock_info.assert_called_once_with('Ignored a non-YAML file found under `available_parameters` directory: '
-                                              '`%s`.', os.path.join('.', 'file.txt'))
+            mock_info.assert_called_once_with('Ignored a non-YAML file found under `%s` '
+                                              'directory: `%s`.', 'available_parameters', file4_mock)
             mock_warning.assert_called_once()
             self.assertIn(
-                "Unexpected issue while reading parameters file `{0}`: `[Errno 2] No such file or "
-                "directory:".format(os.path.join('.', 'random.yaml')),
-                mock_warning.call_args[0][0]
+                "Unexpected issue while reading parameters file `random.yaml`: `[Errno 2] No such file or "
+                "directory:", mock_warning.call_args[0][0]
             )