From 5e5d8d6b153ad881d9c3b9b9123a13aee37b56f0 Mon Sep 17 00:00:00 2001 From: Jason Lynch Date: Fri, 19 Dec 2025 08:55:24 -0500 Subject: [PATCH] fix: move quoting logic for pgbackrest Replaces custom quoting logic with the shellescape library and moves it to only affect the Patroni bootstrap command. Explanation: Docker already escapes and quotes arguments, so the backup workflow breaks when we pre-quote those arguments. However, we still need the quoting when we build the bootstrap command in a Patroni config. Using a library for this handles more cases than just whitespace characters. You can test this by adding `restore_options` to the `local-backup-and-restore` Bruno scenario, e.g.: ```json { "restore_config": { "source_database_id": "storefront", "source_node_name": "n1", "source_database_name": "storefront", "repository": { "id": "{{repository_id}}", "type": "posix", "base_path": "/backups" }, "restore_options": { "type": "time", "target": "2025-12-19 13:27:12+00" } } } ``` Note that when testing the restore functionality, you should use a timestamp after the end of the most recent full backup. When testing the create from backup functionality, you should just the beginning timestamp for your backup set. You can inspect your backup sets by running `pgbackrest info` inside the container, e.g.: ```sh # use the dev-env.zsh helper to exec into the n1 container cp-docker-exec # then from inside the container pgbackrest --stanza=db --config=/opt/pgedge/configs/pgbackrest.backup.conf ``` PLAT-365 --- NOTICE.txt | 31 +++++++++++++++++++ changes/unreleased/Fixed-20251219-094938.yaml | 3 ++ go.mod | 1 + go.sum | 2 ++ .../orchestrator/swarm/patroni_config.go | 4 +++ server/internal/utils/utils.go | 4 --- server/internal/utils/utils_test.go | 6 ++-- 7 files changed, 44 insertions(+), 7 deletions(-) create mode 100644 changes/unreleased/Fixed-20251219-094938.yaml diff --git a/NOTICE.txt b/NOTICE.txt index 8707f879..fadcfa75 100644 --- a/NOTICE.txt +++ b/NOTICE.txt @@ -67,6 +67,37 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. ``` +## github.com/alessio/shellescape + +* Name: github.com/alessio/shellescape +* Version: v1.4.2 +* License: [MIT](https://github.com/alessio/shellescape/blob/v1.4.2/LICENSE) + +``` +The MIT License (MIT) + +Copyright (c) 2016 Alessio Treglia + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. + +``` + ## github.com/benbjohnson/clock * Name: github.com/benbjohnson/clock diff --git a/changes/unreleased/Fixed-20251219-094938.yaml b/changes/unreleased/Fixed-20251219-094938.yaml new file mode 100644 index 00000000..bd889f41 --- /dev/null +++ b/changes/unreleased/Fixed-20251219-094938.yaml @@ -0,0 +1,3 @@ +kind: Fixed +body: Fixed incorrect pgbackrest command format for database restore endpoint. +time: 2025-12-19T09:49:38.606386-05:00 diff --git a/go.mod b/go.mod index 82811343..0db99378 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/pgEdge/control-plane go 1.24.3 require ( + github.com/alessio/shellescape v1.4.2 github.com/cilium/ipam v0.0.0-20230509084518-fd66eae7909b github.com/cschleiden/go-workflows v0.19.0 github.com/docker/docker v27.1.1+incompatible diff --git a/go.sum b/go.sum index 2cd988d2..5bc4ef66 100644 --- a/go.sum +++ b/go.sum @@ -46,6 +46,8 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= +github.com/alessio/shellescape v1.4.2 h1:MHPfaU+ddJ0/bYWpgIeUnQUqKrlJ1S7BfEYPM4uEoM0= +github.com/alessio/shellescape v1.4.2/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPpyFjDCtHLS30= github.com/benbjohnson/clock v1.3.5 h1:VvXlSJBzZpA/zum6Sj74hxwYI2DIxRWuNIoXAzHZz5o= github.com/benbjohnson/clock v1.3.5/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= diff --git a/server/internal/orchestrator/swarm/patroni_config.go b/server/internal/orchestrator/swarm/patroni_config.go index 82b60387..a2ba445e 100644 --- a/server/internal/orchestrator/swarm/patroni_config.go +++ b/server/internal/orchestrator/swarm/patroni_config.go @@ -9,6 +9,7 @@ import ( "net/url" "path/filepath" + "github.com/alessio/shellescape" "github.com/samber/do" "github.com/spf13/afero" clientv3 "go.etcd.io/etcd/client/v3" @@ -475,6 +476,9 @@ func generatePatroniConfig( if spec.RestoreConfig != nil { restoreOptions := utils.BuildOptionArgs(spec.RestoreConfig.RestoreOptions) + for i, o := range restoreOptions { + restoreOptions[i] = shellescape.Quote(o) + } cfg.Bootstrap.Method = utils.PointerTo(patroni.BootstrapMethodNameRestore) cfg.Bootstrap.Restore = &patroni.BootstrapMethodConf{ Command: utils.PointerTo(PgBackRestRestoreCmd("restore", restoreOptions...).String()), diff --git a/server/internal/utils/utils.go b/server/internal/utils/utils.go index bd54f177..80eacf74 100644 --- a/server/internal/utils/utils.go +++ b/server/internal/utils/utils.go @@ -120,10 +120,6 @@ func BuildOptionArgs(options map[string]string) []string { res = append(res, prefix+k) continue } - // Quote value if it contains whitespace - if strings.ContainsAny(v, " \t\r\n") { - v = `"` + v + `"` - } res = append(res, prefix+k+"="+v) } diff --git a/server/internal/utils/utils_test.go b/server/internal/utils/utils_test.go index f52dc778..68381e1b 100644 --- a/server/internal/utils/utils_test.go +++ b/server/internal/utils/utils_test.go @@ -43,7 +43,7 @@ func TestBuildOptionArgs(t *testing.T) { }, expected: []string{ "--type=time", - `--target="2025-06-11 09:00:00"`, + `--target=2025-06-11 09:00:00`, }, }, { @@ -53,8 +53,8 @@ func TestBuildOptionArgs(t *testing.T) { "tab": "one\t two", }, expected: []string{ - "--note=\"line1\nline2\"", - "--tab=\"one\t two\"", + "--note=line1\nline2", + "--tab=one\t two", }, }, }