From c4a4457a750b132b9a253a2a1a3191441ee6cb09 Mon Sep 17 00:00:00 2001 From: David Almgren Date: Tue, 26 May 2026 09:46:46 +0200 Subject: [PATCH] feat: pass additionalCommandArgs to barman-cloud-backup-show / -list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An ObjectStore lets users tack extra command-line flags onto four of the six barman-cloud-* invocations the plugin shells out to: - barman-cloud-backup via Data.AdditionalCommandArgs - barman-cloud-wal-archive via Wal.ArchiveAdditionalCommandArgs - barman-cloud-wal-restore via Wal.RestoreAdditionalCommandArgs - barman-cloud-restore via Data.RestoreAdditionalCommandArgs (PR #914 in the plugin) The remaining two — barman-cloud-backup-show (the post-write verification) and barman-cloud-backup-list (retention pruning) — had no equivalent, which is the gap reported in plugin-barman-cloud #712: on strictly-vhost S3-compatible endpoints, users need --addressing-style=virtual on every cloud command, and currently those two reject the user-provided args, marking otherwise-successful backups as failed and silently breaking retention pruning. This adds two new fields on DataBackupConfiguration - ShowAdditionalCommandArgs and ListAdditionalCommandArgs - plus matching AppendShowAdditionalCommandArgs and AppendListAdditionalCommandArgs helpers, mirroring the existing pattern. executeQueryCommand in pkg/command/backuplist.go grows a separate additionalFlagArgs parameter so the user-provided flags land alongside the standard options (--format json, --endpoint-url, cloud provider opts) rather than after the positional DESTINATION/SERVER/BACKUP_ID arguments; GetBackupList and GetBackupByName thread the new helpers through. Tests mirror the existing AppendAdditionalCommandArgs ones. Refs: cloudnative-pg/plugin-barman-cloud#712 Signed-off-by: David Almgren --- pkg/api/config.go | 45 ++++++++++++++++++++++++++++++++ pkg/api/config_test.go | 44 +++++++++++++++++++++++++++++++ pkg/api/zz_generated.deepcopy.go | 10 +++++++ pkg/command/backuplist.go | 13 +++++++-- 4 files changed, 110 insertions(+), 2 deletions(-) diff --git a/pkg/api/config.go b/pkg/api/config.go index e39505c3..30d3232b 100644 --- a/pkg/api/config.go +++ b/pkg/api/config.go @@ -325,6 +325,33 @@ type DataBackupConfiguration struct { // behavior during execution. // +optional AdditionalCommandArgs []string `json:"additionalCommandArgs,omitempty"` + + // ShowAdditionalCommandArgs represents additional arguments that can be appended + // to the 'barman-cloud-backup-show' command-line invocation. This command is + // used after a successful upload to verify and record metadata about the + // freshly-written backup, so flags relevant to the underlying S3 client + // (e.g. `--addressing-style=virtual` for strictly virtual-hosted S3-compatible + // endpoints) generally belong here as well. + // + // Note: + // It's essential to ensure that the provided arguments are valid and supported + // by the 'barman-cloud-backup-show' command, to avoid potential errors or + // unintended behavior during execution. + // +optional + ShowAdditionalCommandArgs []string `json:"showAdditionalCommandArgs,omitempty"` + + // ListAdditionalCommandArgs represents additional arguments that can be appended + // to the 'barman-cloud-backup-list' command-line invocation. This command is + // used internally for retention-policy enforcement; flags relevant to the + // underlying S3 client (e.g. `--addressing-style=virtual` for strictly + // virtual-hosted S3-compatible endpoints) generally belong here as well. + // + // Note: + // It's essential to ensure that the provided arguments are valid and supported + // by the 'barman-cloud-backup-list' command, to avoid potential errors or + // unintended behavior during execution. + // +optional + ListAdditionalCommandArgs []string `json:"listAdditionalCommandArgs,omitempty"` } // ArePopulated checks if the passed set of credentials contains @@ -466,6 +493,24 @@ func (cfg *DataBackupConfiguration) AppendAdditionalCommandArgs(options []string return appendAdditionalCommandArgs(cfg.AdditionalCommandArgs, options) } +// AppendShowAdditionalCommandArgs adds custom arguments as barman-cloud-backup-show +// command-line options +func (cfg *DataBackupConfiguration) AppendShowAdditionalCommandArgs(options []string) []string { + if cfg == nil || len(cfg.ShowAdditionalCommandArgs) == 0 { + return options + } + return appendAdditionalCommandArgs(cfg.ShowAdditionalCommandArgs, options) +} + +// AppendListAdditionalCommandArgs adds custom arguments as barman-cloud-backup-list +// command-line options +func (cfg *DataBackupConfiguration) AppendListAdditionalCommandArgs(options []string) []string { + if cfg == nil || len(cfg.ListAdditionalCommandArgs) == 0 { + return options + } + return appendAdditionalCommandArgs(cfg.ListAdditionalCommandArgs, options) +} + // AppendArchiveAdditionalCommandArgs adds custom arguments as barman-cloud-wal-archive command-line options func (cfg *WalBackupConfiguration) AppendArchiveAdditionalCommandArgs(options []string) []string { if cfg == nil || len(cfg.ArchiveAdditionalCommandArgs) == 0 { diff --git a/pkg/api/config_test.go b/pkg/api/config_test.go index 53d84d89..39acfab7 100644 --- a/pkg/api/config_test.go +++ b/pkg/api/config_test.go @@ -49,6 +49,50 @@ var _ = Describe("DataBackupConfiguration.AppendAdditionalCommandArgs", func() { }) }) +var _ = Describe("DataBackupConfiguration.AppendShowAdditionalCommandArgs", func() { + var options []string + var config DataBackupConfiguration + BeforeEach(func() { + options = []string{"--option1", "--option2"} + config = DataBackupConfiguration{ + ShowAdditionalCommandArgs: []string{"--option3", "--option4"}, + } + }) + + It("should append additional command args to the options", func() { + updatedOptions := config.AppendShowAdditionalCommandArgs(options) + Expect(updatedOptions).To(Equal([]string{"--option1", "--option2", "--option3", "--option4"})) + }) + + It("should return the original options if there are no additional command args", func() { + config.ShowAdditionalCommandArgs = nil + updatedOptions := config.AppendShowAdditionalCommandArgs(options) + Expect(updatedOptions).To(Equal(options)) + }) +}) + +var _ = Describe("DataBackupConfiguration.AppendListAdditionalCommandArgs", func() { + var options []string + var config DataBackupConfiguration + BeforeEach(func() { + options = []string{"--option1", "--option2"} + config = DataBackupConfiguration{ + ListAdditionalCommandArgs: []string{"--option3", "--option4"}, + } + }) + + It("should append additional command args to the options", func() { + updatedOptions := config.AppendListAdditionalCommandArgs(options) + Expect(updatedOptions).To(Equal([]string{"--option1", "--option2", "--option3", "--option4"})) + }) + + It("should return the original options if there are no additional command args", func() { + config.ListAdditionalCommandArgs = nil + updatedOptions := config.AppendListAdditionalCommandArgs(options) + Expect(updatedOptions).To(Equal(options)) + }) +}) + var _ = Describe("WalBackupConfiguration.AppendArchiveAdditionalCommandArgs", func() { var options []string var config WalBackupConfiguration diff --git a/pkg/api/zz_generated.deepcopy.go b/pkg/api/zz_generated.deepcopy.go index 88eb628f..5c9921f8 100644 --- a/pkg/api/zz_generated.deepcopy.go +++ b/pkg/api/zz_generated.deepcopy.go @@ -150,6 +150,16 @@ func (in *DataBackupConfiguration) DeepCopyInto(out *DataBackupConfiguration) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.ShowAdditionalCommandArgs != nil { + in, out := &in.ShowAdditionalCommandArgs, &out.ShowAdditionalCommandArgs + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.ListAdditionalCommandArgs != nil { + in, out := &in.ListAdditionalCommandArgs, &out.ListAdditionalCommandArgs + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DataBackupConfiguration. diff --git a/pkg/command/backuplist.go b/pkg/command/backuplist.go index 4dc1e19d..0afef0da 100644 --- a/pkg/command/backuplist.go +++ b/pkg/command/backuplist.go @@ -61,7 +61,8 @@ func executeQueryCommand( barmanCommand string, barmanConfiguration *barmanApi.BarmanObjectStoreConfiguration, serverName string, - additionalOptions []string, + additionalFlagArgs []string, + trailingArgs []string, env []string, ) (string, error) { contextLogger := log.FromContext(ctx).WithName("barman") @@ -77,8 +78,14 @@ func executeQueryCommand( return "", err } + // User-provided per-subcommand flags are appended after the standard + // options (cloud provider, endpoint, etc.) but before the positional + // arguments, matching the shape of the other Append*AdditionalCommandArgs + // helpers in the rest of this library. + options = append(options, additionalFlagArgs...) + options = append(options, barmanConfiguration.DestinationPath, serverName) - options = append(options, additionalOptions...) + options = append(options, trailingArgs...) var stdoutBuffer bytes.Buffer var stderrBuffer bytes.Buffer @@ -114,6 +121,7 @@ func GetBackupList( utils.BarmanCloudBackupList, barmanConfiguration, serverName, + barmanConfiguration.Data.AppendListAdditionalCommandArgs(nil), []string{}, env, ) @@ -146,6 +154,7 @@ func GetBackupByName( utils.BarmanCloudBackupShow, barmanConfiguration, serverName, + barmanConfiguration.Data.AppendShowAdditionalCommandArgs(nil), []string{backupName}, env, )