From 7adfc0dbe716c2ab804bdcc1629b12481b6d362f Mon Sep 17 00:00:00 2001 From: Israel Date: Tue, 6 Feb 2024 04:36:11 -0300 Subject: [PATCH] Patroni doesn't filter out some not allowed options from `pg_basebackup` (#3015) When running `pg_basebackup` to bootstrap a replica, Patroni sanitizes the custom user options that come from `postgresql.basebackup` configuration section using the `process_user_options` method. However, there is a bug in that method: it filters out not allowed options that are in the format `- setting`, but not the ones in the format `- setting: value` from `postgresql.basebackup`. An example of that issue is the `dbname` setting. If you specify something like this in the configuration file: ```yaml postgresql: basebackup: - dbname: "host=RANDOM" ``` You end up with `--dbname` being specified twice for `pg_basebackup`, with `--dbname='host=RANDOM'` taking precedence as it comes up later in the command. This commit fixes that issue by adding a `continue` statement when the setting in format `- setting: value` is not allowed, thus skipping it. --------- Signed-off-by: Israel Barth Rubio --- patroni/postgresql/bootstrap.py | 5 +++-- tests/test_bootstrap.py | 10 ++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/patroni/postgresql/bootstrap.py b/patroni/postgresql/bootstrap.py index a544bd73..e6904878 100644 --- a/patroni/postgresql/bootstrap.py +++ b/patroni/postgresql/bootstrap.py @@ -100,10 +100,11 @@ class Bootstrap(object): user_options.append('--{0}'.format(opt)) elif isinstance(opt, dict): keys = list(opt.keys()) - if len(keys) != 1 or not isinstance(opt[keys[0]], str) or not option_is_allowed(keys[0]): + if len(keys) == 1 and isinstance(opt[keys[0]], str) and option_is_allowed(keys[0]): + user_options.append('--{0}={1}'.format(keys[0], unquote(opt[keys[0]]))) + else: error_handler('Error when parsing {0} key-value option {1}: only one key-value is allowed' ' and value should be a string'.format(tool, opt[keys[0]])) - user_options.append('--{0}={1}'.format(keys[0], unquote(opt[keys[0]]))) else: error_handler('Error when parsing {0} option {1}: value should be string value' ' or a single key-value pair'.format(tool, opt)) diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py index 8724b03c..9eaeb1af 100644 --- a/tests/test_bootstrap.py +++ b/tests/test_bootstrap.py @@ -142,6 +142,16 @@ class TestBootstrap(BaseTestPostgresql): (), error_handler ), ["--key=value with spaces"]) + # not allowed options in list of dicts/strs are filtered out + self.assertEqual( + self.b.process_user_options( + 'pg_basebackup', + [{'checkpoint': 'fast'}, {'dbname': 'dbname=postgres'}, 'gzip', {'label': 'standby'}, 'verbose'], + ('dbname', 'verbose'), + print + ), + ['--checkpoint=fast', '--gzip', '--label=standby'], + ) @patch.object(CancellableSubprocess, 'call', Mock()) @patch.object(Postgresql, 'is_running', Mock(return_value=True))