feat: add ShowAdditionalCommandArgs and ListAdditionalCommandArgs#249
Open
davidalmgren95 wants to merge 1 commit into
Open
feat: add ShowAdditionalCommandArgs and ListAdditionalCommandArgs#249davidalmgren95 wants to merge 1 commit into
davidalmgren95 wants to merge 1 commit into
Conversation
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 <dalmgren@coreweave.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
An
ObjectStorealready lets users tack extra command-line flags onto four of the sixbarman-cloud-*invocations the CNPG plugin shells out to:barman-cloud-backupData.AdditionalCommandArgsbarman-cloud-wal-archiveWal.ArchiveAdditionalCommandArgsbarman-cloud-wal-restoreWal.RestoreAdditionalCommandArgsbarman-cloud-restoreData.RestoreAdditionalCommandArgs(plugin PR #914)The remaining two —
barman-cloud-backup-show(the post-write verification step) andbarman-cloud-backup-list(used internally for retention pruning) — had no equivalent, which is the gap reported in cloudnative-pg/plugin-barman-cloud#712.This is observable in practice on S3-compatible endpoints that strictly require virtual-hosted-style addressing for LIST. On such backends:
barman-cloud-backupaccepts user-provided--addressing-style=virtualvia the existingData.AdditionalCommandArgs✅ — base backup completes and lands in the bucketbarman-cloud-backup-showruns immediately afterwards with no way to thread the flag through, sends path-style by default, getsPathStyleRequestNotAllowed, exits with status 4phase: failedbarman-cloud-backup-list) is similarly broken — old backups silently never get expiredSo user-visible Backup CR status is wrong and retention is silently disabled, even though the actual base-backup data is intact in the bucket.
Changes
DataBackupConfiguration.ShowAdditionalCommandArgs(forbarman-cloud-backup-show)DataBackupConfiguration.ListAdditionalCommandArgs(forbarman-cloud-backup-list)AppendShowAdditionalCommandArgsandAppendListAdditionalCommandArgshelpers, mirroring the existing pattern in this fileGetBackupListandGetBackupByNamein `pkg/command/backuplist.go`Sister PR
The CRD fields on the consuming side are added in cloudnative-pg/plugin-barman-cloud#924 (filed in parallel; will be linked once open). This library change is the prerequisite for the plugin's CRD generation to pick up the new fields.
Refs: cloudnative-pg/plugin-barman-cloud#712