diff --git a/patroni/config.py b/patroni/config.py index 66ffd891..facf16fd 100644 --- a/patroni/config.py +++ b/patroni/config.py @@ -329,9 +329,19 @@ class Config(object): @staticmethod def _process_postgresql_parameters(parameters: Dict[str, Any], is_local: bool = False) -> Dict[str, Any]: - return {name: value for name, value in (parameters or {}).items() - if name not in ConfigHandler.CMDLINE_OPTIONS - or not is_local and ConfigHandler.CMDLINE_OPTIONS[name][1](value)} + pg_params: Dict[str, Any] = {} + + for name, value in (parameters or {}).items(): + if name not in ConfigHandler.CMDLINE_OPTIONS: + pg_params[name] = value + elif not is_local: + if ConfigHandler.CMDLINE_OPTIONS[name][1](value): + pg_params[name] = value + else: + logging.warning("postgresql parameter %s=%s failed validation, defaulting to %s", + name, value, ConfigHandler.CMDLINE_OPTIONS[name][0]) + + return pg_params def _safe_copy_dynamic_configuration(self, dynamic_configuration: Dict[str, Any]) -> Dict[str, Any]: config = deepcopy(self.__DEFAULT_CONFIG) diff --git a/patroni/postgresql/config.py b/patroni/postgresql/config.py index 9461714f..3a61a31d 100644 --- a/patroni/postgresql/config.py +++ b/patroni/postgresql/config.py @@ -15,7 +15,7 @@ from ..collections import CaseInsensitiveDict, CaseInsensitiveSet from ..dcs import Leader, Member, RemoteMember, slot_name_from_member_name from ..exceptions import PatroniFatalException from ..utils import compare_values, parse_bool, parse_int, split_host_port, uri, validate_directory, is_subpath -from ..validator import IntValidator +from ..validator import IntValidator, EnumValidator if TYPE_CHECKING: # pragma: no cover from . import Postgresql @@ -258,14 +258,14 @@ def _false_validator(value: Any) -> bool: return False -def _wal_level_validator(value: Any) -> bool: - return str(value).lower() in ('hot_standby', 'replica', 'logical') - - def _bool_validator(value: Any) -> bool: return parse_bool(value) is not None +def _bool_is_true_validator(value: Any) -> bool: + return parse_bool(value) is True + + class ConfigHandler(object): # List of parameters which must be always passed to postmaster as command line options @@ -286,8 +286,8 @@ class ConfigHandler(object): 'listen_addresses': (None, _false_validator, 90100), 'port': (None, _false_validator, 90100), 'cluster_name': (None, _false_validator, 90500), - 'wal_level': ('hot_standby', _wal_level_validator, 90100), - 'hot_standby': ('on', _false_validator, 90100), + 'wal_level': ('hot_standby', EnumValidator(('hot_standby', 'replica', 'logical')), 90100), + 'hot_standby': ('on', _bool_is_true_validator, 90100), 'max_connections': (100, IntValidator(min=25), 90100), 'max_wal_senders': (10, IntValidator(min=3), 90100), 'wal_keep_segments': (8, IntValidator(min=1), 90100), @@ -297,7 +297,7 @@ class ConfigHandler(object): 'track_commit_timestamp': ('off', _bool_validator, 90500), 'max_replication_slots': (10, IntValidator(min=4), 90400), 'max_worker_processes': (8, IntValidator(min=2), 90400), - 'wal_log_hints': ('on', _false_validator, 90400) + 'wal_log_hints': ('on', _bool_is_true_validator, 90400) }) _RECOVERY_PARAMETERS = CaseInsensitiveSet(recovery_parameters.keys()) diff --git a/patroni/validator.py b/patroni/validator.py index b68f9654..a8ada1f5 100644 --- a/patroni/validator.py +++ b/patroni/validator.py @@ -785,7 +785,7 @@ class IntValidator(object): self.base_unit = base_unit self.raise_assert = raise_assert - def __call__(self, value: Union[int, str]) -> bool: + def __call__(self, value: Any) -> bool: """Check if *value* is a valid integer and within the expected range. .. note:: @@ -821,7 +821,7 @@ class EnumValidator(object): self.allowed_values = set(allowed_values) if case_sensitive else CaseInsensitiveSet(allowed_values) self.raise_assert = raise_assert - def __call__(self, value: str) -> bool: + def __call__(self, value: Any) -> bool: """Check if provided *value* could be found within *allowed_values*. .. note:: @@ -829,7 +829,7 @@ class EnumValidator(object): :param value: value to be checked. :returns: ``True`` if *value* could be found within *allowed_values*. """ - ret = value in self.allowed_values + ret = isinstance(value, str) and value in self.allowed_values if self.raise_assert: assert_(ret) diff --git a/tests/test_config.py b/tests/test_config.py index 57a717f5..3f7e4049 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -21,7 +21,8 @@ class TestConfig(unittest.TestCase): with patch.object(Config, '_build_effective_configuration', Mock(side_effect=Exception)): self.assertFalse(self.config.set_dynamic_configuration({'foo': 'bar'})) self.assertTrue(self.config.set_dynamic_configuration({'standby_cluster': {}, 'postgresql': { - 'parameters': {'cluster_name': 1, 'wal_keep_size': 1, 'track_commit_timestamp': 1, 'wal_level': 1}}})) + 'parameters': {'cluster_name': 1, 'hot_standby': 1, 'wal_keep_size': 1, + 'track_commit_timestamp': 1, 'wal_level': 1}}})) def test_reload_local_configuration(self): os.environ.update({