Repository: zalando/patroni
Pull request #3015 (2024-02-01 15:42:13)

Patroni doesn't filter out some not allowed options from `pg_basebackup`
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.

References: PAT-235.

coveralls: ## Pull Request Test Coverage Report for [Build 7743829765](https://coveralls.io/builds/65441380)


* **0** of **1**   **(100.0%)**  changed or added relevant line in **1** file are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at **99.86%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/65441380/badge)](https://coveralls.io/builds/65441380) |
| :-- | --: |
| Change from base [Build 7694067555](https://coveralls.io/builds/65349277): |  0.0% |
| Covered Lines: | 13588 |
| Relevant Lines: | 13607 |

---
##### 💛  - [Coveralls](https://coveralls.io)

hughcapet: 👍 
CyberDem0n: :+1:

Patch:
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))


Test patch:
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))