From 7afdb2443ed30d237351ce017de36eae4640006a Mon Sep 17 00:00:00 2001 From: Ivan Kirilin Date: Fri, 4 Sep 2026 10:58:03 +0200 Subject: [PATCH 1/3] fix(restore): fail fast on an empty --plugin-config instead of silently restoring from local disk gprestore accepted `--plugin-config ""` and fell back to a local-filesystem restore of a backup set that only exists on the plugin's storage. utils.ValidateFullPath short-circuits on len(path) > 0, so the empty value passed validation, and DoSetup treats an empty plugin config as "no plugin at all" (restore/restore.go:95). The run then failed on the first backup file that is not on local disk. Reported against DD Boost as open .../gpbackup__config.yaml: no such file or directory Backup directories missing or inaccessible on 1 segment both of which point at the local filesystem rather than at the dropped plugin, and neither of which mentions that no plugin was in effect. - Reject an explicitly-set-but-empty --plugin-config in gprestore (ValidatePluginConfigFlag, called from DoValidation) and in gpbackup (validateFlagValues), so the same mistake at backup time cannot silently produce a backup on local disk instead of on the plugin's storage. - Run the plugin-combination check before any check that assumes the backup files are on local disk: ValidateBackupFlagPluginCombinations (renamed from the unexported validateBackupFlagPluginCombinations) now runs at the top of BackupConfigurationValidation, ahead of VerifyBackupDirectoriesExistOnAllHosts. A plugin backup restored without --plugin-config now reports "Backup was taken with plugin ... The --plugin-config flag must be used to restore." instead of a missing local directory. BackupConfigurationValidation was the only production caller, so this is a reordering, not a behavior change. - Log at INFO whether a plugin is in effect and which config path is used. --- backup/validate.go | 7 +++++ backup/validate_test.go | 2 ++ restore/restore.go | 3 ++ restore/validate.go | 23 ++++++++++++-- restore/validate_test.go | 65 ++++++++++++++++++++++++++++++++++++++-- restore/wrappers.go | 4 +++ 6 files changed, 100 insertions(+), 4 deletions(-) diff --git a/backup/validate.go b/backup/validate.go index d87f1466..5317cd0b 100644 --- a/backup/validate.go +++ b/backup/validate.go @@ -165,6 +165,13 @@ func validateFlagCombinations(flags *pflag.FlagSet) { } func validateFlagValues() { + // ValidateFullPath accepts the empty string and DoSetup treats an empty + // plugin config as "no plugin", so an unset or mistyped shell variable would + // otherwise silently produce a local-disk backup instead of one on the + // plugin's storage. + if FlagChanged(options.PLUGIN_CONFIG) && MustGetFlagString(options.PLUGIN_CONFIG) == "" { + gplog.Fatal(errors.Errorf("The --plugin-config flag was specified with an empty value. Specify the absolute path to the plugin configuration file, or omit the flag to back up to local disk."), "") + } err := utils.ValidateFullPath(MustGetFlagString(options.BACKUP_DIR)) gplog.FatalOnError(err) err = utils.ValidateFullPath(MustGetFlagString(options.PLUGIN_CONFIG)) diff --git a/backup/validate_test.go b/backup/validate_test.go index 248fffc0..acde8325 100644 --- a/backup/validate_test.go +++ b/backup/validate_test.go @@ -215,6 +215,8 @@ var _ = Describe("backup/validate tests", func() { } }, Entry("--backup-dir combo", "--backup-dir /tmp --plugin-config /tmp/config", false), + Entry("--plugin-config with an empty value", "--plugin-config ", false), + Entry("--plugin-config with a path", "--plugin-config /tmp/config", true), /* * Below are all the different filter combinations diff --git a/restore/restore.go b/restore/restore.go index 2f7bcfcb..15eaf976 100644 --- a/restore/restore.go +++ b/restore/restore.go @@ -39,6 +39,7 @@ func DoInit(cmd *cobra.Command) { */ func DoValidation(cmd *cobra.Command) { ValidateFlagCombinations(cmd) + ValidatePluginConfigFlag() err := utils.ValidateFullPath(MustGetFlagString(options.BACKUP_DIR)) gplog.FatalOnError(err) err = utils.ValidateFullPath(MustGetFlagString(options.PLUGIN_CONFIG)) @@ -93,8 +94,10 @@ func DoSetup() { // Get restore metadata from plugin if MustGetFlagString(options.PLUGIN_CONFIG) != "" { + gplog.Info("Restoring metadata files using plugin config %s", MustGetFlagString(options.PLUGIN_CONFIG)) RecoverMetadataFilesUsingPlugin() } else { + gplog.Info("No plugin configured; expecting all backup files on local disk under %s", globalFPInfo.GetDirForContent(-1)) InitializeBackupConfig() } diff --git a/restore/validate.go b/restore/validate.go index 4c18e839..b4451059 100644 --- a/restore/validate.go +++ b/restore/validate.go @@ -282,10 +282,29 @@ func ValidateBackupFlagCombinations() { if !backupConfig.SingleDataFile && FlagChanged(options.COPY_QUEUE_SIZE) { gplog.Fatal(errors.Errorf("The --copy-queue-size flag can only be used if the backup was taken with --single-data-file"), "") } - validateBackupFlagPluginCombinations() } -func validateBackupFlagPluginCombinations() { +/* + * ValidatePluginConfigFlag rejects --plugin-config given with an empty value. + * ValidateFullPath accepts the empty string, and DoSetup treats an empty plugin + * config as "no plugin at all", so without this check an unset or mistyped shell + * variable silently downgrades a plugin restore to a local-filesystem restore + * and then fails on the first backup file that is not on local disk. + */ +func ValidatePluginConfigFlag() { + if FlagChanged(options.PLUGIN_CONFIG) && MustGetFlagString(options.PLUGIN_CONFIG) == "" { + gplog.Fatal(errors.Errorf("The --plugin-config flag was specified with an empty value. Specify the absolute path to the plugin configuration file, or omit the flag to restore from local disk."), "") + } +} + +/* + * ValidateBackupFlagPluginCombinations must be called before any check that + * assumes the backup files are on local disk. A backup taken with a plugin + * stores its files on the plugin's storage, so if the plugin is missing here the + * useful error is "the --plugin-config flag must be used", not a report of a + * missing local directory or file. + */ +func ValidateBackupFlagPluginCombinations() { if MustGetFlagBool(options.IGNORE_PLUGIN_CONFIG) { // No-op against a backup taken without a plugin; the flag just means // "do not invoke any plugin during restore", which is already the diff --git a/restore/validate_test.go b/restore/validate_test.go index 31b2b124..79105e49 100644 --- a/restore/validate_test.go +++ b/restore/validate_test.go @@ -421,7 +421,7 @@ var _ = Describe("restore/validate tests", func() { Use: "flag validation", Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { - restore.ValidateBackupFlagCombinations() + restore.ValidateBackupFlagPluginCombinations() }} testCmd.SetArgs([]string{"--ignore-plugin-config"}) restore.SetCmdFlags(testCmd.Flags()) @@ -435,11 +435,72 @@ var _ = Describe("restore/validate tests", func() { Use: "flag validation", Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { - restore.ValidateBackupFlagCombinations() + restore.ValidateBackupFlagPluginCombinations() }} testCmd.SetArgs([]string{"--ignore-plugin-config"}) restore.SetCmdFlags(testCmd.Flags()) + err := testCmd.Execute() + Expect(err).ToNot(HaveOccurred()) + }) + It("should fatal when the backup was taken with a plugin but no plugin config is given", func() { + restore.SetBackupConfig(&history.BackupConfig{Plugin: "/tmp/gpbackup_fake_plugin"}) + testCmd := &cobra.Command{ + Use: "flag validation", + Args: cobra.NoArgs, + Run: func(cmd *cobra.Command, args []string) { + restore.ValidateBackupFlagPluginCombinations() + }} + testCmd.SetArgs([]string{}) + restore.SetCmdFlags(testCmd.Flags()) + + defer testhelper.ShouldPanicWithMessage("The --plugin-config flag must be used to restore") + err := testCmd.Execute() + if err == nil { + Fail("restore of a plugin backup without --plugin-config passed validation check") + } + }) + }) + Describe("ValidatePluginConfigFlag", func() { + It("should fatal when --plugin-config is specified with an empty value", func() { + testCmd := &cobra.Command{ + Use: "flag validation", + Args: cobra.NoArgs, + Run: func(cmd *cobra.Command, args []string) { + restore.ValidatePluginConfigFlag() + }} + testCmd.SetArgs([]string{"--plugin-config", ""}) + restore.SetCmdFlags(testCmd.Flags()) + + defer testhelper.ShouldPanicWithMessage("The --plugin-config flag was specified with an empty value") + err := testCmd.Execute() + if err == nil { + Fail("empty --plugin-config value passed validation check") + } + }) + It("should pass when --plugin-config is specified with a path", func() { + testCmd := &cobra.Command{ + Use: "flag validation", + Args: cobra.NoArgs, + Run: func(cmd *cobra.Command, args []string) { + restore.ValidatePluginConfigFlag() + }} + testCmd.SetArgs([]string{"--plugin-config", "/tmp/plugin_config.yaml"}) + restore.SetCmdFlags(testCmd.Flags()) + + err := testCmd.Execute() + Expect(err).ToNot(HaveOccurred()) + }) + It("should pass when --plugin-config is not specified at all", func() { + testCmd := &cobra.Command{ + Use: "flag validation", + Args: cobra.NoArgs, + Run: func(cmd *cobra.Command, args []string) { + restore.ValidatePluginConfigFlag() + }} + testCmd.SetArgs([]string{}) + restore.SetCmdFlags(testCmd.Flags()) + err := testCmd.Execute() Expect(err).ToNot(HaveOccurred()) }) diff --git a/restore/wrappers.go b/restore/wrappers.go index ebd50904..cef752bf 100644 --- a/restore/wrappers.go +++ b/restore/wrappers.go @@ -163,6 +163,10 @@ func InitializeBackupConfig() { } func BackupConfigurationValidation() { + // This must run first: every check below reads the backup files from local + // disk, which is only where they live when no plugin was used. + ValidateBackupFlagPluginCombinations() + if !backupConfig.MetadataOnly { gplog.Verbose("Gathering information on backup directories") VerifyBackupDirectoriesExistOnAllHosts() From 017aec73883b688249ef3d9b47e5fdabdbff3482 Mon Sep 17 00:00:00 2001 From: Ivan Kirilin Date: Fri, 4 Sep 2026 11:19:15 +0200 Subject: [PATCH 2/3] fix(restore): create the local backup directories instead of requiring them when a plugin is in use A restore through a plugin failed if the local backup directory was missing on the coordinator or on any segment, even though the backup itself lives on the plugin's storage and those directories only ever hold files gprestore downloads into them. A host rebuilt or replaced since the backup was taken therefore could not be restored onto without first recreating the old directory tree by hand (the reported workaround was to NFS-mount the Data Domain storage unit and copy the files back). VerifyBackupDirectoriesExistOnAllHosts is renamed to EnsureBackupDirectoriesExistOnAllHosts and now branches on whether a plugin is in effect: - with a plugin, "mkdir -p" the coordinator and segment directories; - without one, keep asserting them with "test -d", because there a missing directory means the backup data really is gone. The segment directories are now handled for every plugin restore rather than only for single-data-file ones. That combination previously skipped the segments altogether, which is harmless for a plain restore (restoreDataFromTimestamp only starts gpbackup_helper when SingleDataFile || resizeCluster, and otherwise the segments read straight from the plugin via COPY FROM PROGRAM) but leaves the staging directory missing for a resize restore, whose helper writes its pipes, oid files and segment TOCs there. Adds unit coverage for all three cases via the cluster test executor. --- restore/remote.go | 65 ++++++++++++++++++++++++++++++------------ restore/remote_test.go | 45 +++++++++++++++++++++++++++++ restore/wrappers.go | 7 +++-- 3 files changed, 96 insertions(+), 21 deletions(-) diff --git a/restore/remote.go b/restore/remote.go index bb6ff279..c31c9b34 100644 --- a/restore/remote.go +++ b/restore/remote.go @@ -17,25 +17,54 @@ import ( * Functions to run commands on entire cluster during restore */ -func VerifyBackupDirectoriesExistOnAllHosts() { - _, err := globalCluster.ExecuteLocalCommand(fmt.Sprintf("test -d %s", globalFPInfo.GetDirForContent(-1))) - gplog.FatalOnError(err, "Backup directory %s missing or inaccessible", globalFPInfo.GetDirForContent(-1)) - if MustGetFlagString(options.PLUGIN_CONFIG) == "" || backupConfig.SingleDataFile { - origSize, destSize, isResizeRestore, _ := GetResizeClusterInfo() - - remoteOutput := globalCluster.GenerateAndExecuteCommand("Verifying backup directories exist", cluster.ON_SEGMENTS, func(contentID int) string { - if isResizeRestore { // Map origin content to destination content to find where the original files have been placed - if contentID >= origSize { // Don't check for directories for contents that aren't part of the backup set - return "" - } - contentID = contentID % destSize - } - return fmt.Sprintf("test -d %s", globalFPInfo.GetDirForContent(contentID)) - }) - globalCluster.CheckClusterError(remoteOutput, "Backup directories missing or inaccessible", func(contentID int) string { - return fmt.Sprintf("Backup directory %s missing or inaccessible", globalFPInfo.GetDirForContent(contentID)) - }) +/* + * EnsureBackupDirectoriesExistOnAllHosts checks, or creates, the backup + * directories the restore is about to use. + * + * Without a plugin those directories hold the backup itself, so a missing one + * means the data is gone and the restore must stop. With a plugin the backup + * lives on the plugin's storage and the local directories are only staging + * areas for files gprestore downloads into them, so they are created here + * instead of being required. That is what lets a backup be restored onto a + * host that was rebuilt or replaced after the backup was taken, without having + * to reconstruct the old directory tree by hand first. + */ +func EnsureBackupDirectoriesExistOnAllHosts() { + usingPlugin := MustGetFlagString(options.PLUGIN_CONFIG) != "" + + coordinatorDir := globalFPInfo.GetDirForContent(-1) + if usingPlugin { + _, err := globalCluster.ExecuteLocalCommand(fmt.Sprintf("mkdir -p %s", coordinatorDir)) + gplog.FatalOnError(err, "Unable to create backup directory %s", coordinatorDir) + } else { + _, err := globalCluster.ExecuteLocalCommand(fmt.Sprintf("test -d %s", coordinatorDir)) + gplog.FatalOnError(err, "Backup directory %s missing or inaccessible", coordinatorDir) } + + // The segment directories are needed for every plugin restore, not just the + // single-data-file ones: a resize restore stages the helper's pipes, oid + // files and segment TOCs there as well. + dirCommand, verboseMsg, errMsg := "test -d", "Verifying backup directories exist", "Backup directories missing or inaccessible" + if usingPlugin { + dirCommand, verboseMsg, errMsg = "mkdir -p", "Creating backup directories", "Unable to create backup directories" + } + origSize, destSize, isResizeRestore, _ := GetResizeClusterInfo() + + remoteOutput := globalCluster.GenerateAndExecuteCommand(verboseMsg, cluster.ON_SEGMENTS, func(contentID int) string { + if isResizeRestore { // Map origin content to destination content to find where the original files have been placed + if contentID >= origSize { // Don't check for directories for contents that aren't part of the backup set + return "" + } + contentID = contentID % destSize + } + return fmt.Sprintf("%s %s", dirCommand, globalFPInfo.GetDirForContent(contentID)) + }) + globalCluster.CheckClusterError(remoteOutput, errMsg, func(contentID int) string { + if usingPlugin { + return fmt.Sprintf("Unable to create backup directory %s", globalFPInfo.GetDirForContent(contentID)) + } + return fmt.Sprintf("Backup directory %s missing or inaccessible", globalFPInfo.GetDirForContent(contentID)) + }) } func VerifyBackupFileCountOnSegments() { diff --git a/restore/remote_test.go b/restore/remote_test.go index d05a188d..e5f2f453 100644 --- a/restore/remote_test.go +++ b/restore/remote_test.go @@ -117,4 +117,49 @@ var _ = Describe("restore/remote tests", func() { restore.VerifyBackupFileCountOnSegments() }) }) + Describe("EnsureBackupDirectoriesExistOnAllHosts", func() { + BeforeEach(func() { + testExecutor.ClusterOutput = &cluster.RemoteOutput{NumErrors: 0} + testCluster.Executor = testExecutor + restore.SetCluster(testCluster) + restore.SetBackupConfig(&history.BackupConfig{SegmentCount: 2}) + }) + It("requires the directories to exist when no plugin is in use", func() { + restore.EnsureBackupDirectoriesExistOnAllHosts() + + Expect(testExecutor.LocalCommands[0]).To(Equal("test -d /data/gpseg-1/backups/20170101/20170101010101")) + cc := testExecutor.ClusterCommands[0] + Expect(cc).To(HaveLen(2)) + Expect(cc[0].CommandString).To(ContainSubstring("test -d /data/gpseg0/backups/20170101/20170101010101")) + Expect(cc[1].CommandString).To(ContainSubstring("test -d /data/gpseg1/backups/20170101/20170101010101")) + }) + It("creates the directories when a plugin is in use", func() { + // With a plugin these directories only stage files downloaded from the + // plugin's storage, so a host rebuilt since the backup must not fail here. + cmdFlags.Set(options.PLUGIN_CONFIG, "/tmp/plugin_config.yaml") + restore.SetBackupConfig(&history.BackupConfig{SegmentCount: 2, SingleDataFile: true}) + + restore.EnsureBackupDirectoriesExistOnAllHosts() + + Expect(testExecutor.LocalCommands[0]).To(Equal("mkdir -p /data/gpseg-1/backups/20170101/20170101010101")) + cc := testExecutor.ClusterCommands[0] + Expect(cc).To(HaveLen(2)) + Expect(cc[0].CommandString).To(ContainSubstring("mkdir -p /data/gpseg0/backups/20170101/20170101010101")) + Expect(cc[1].CommandString).To(ContainSubstring("mkdir -p /data/gpseg1/backups/20170101/20170101010101")) + }) + It("creates the segment directories for a plugin backup taken without --single-data-file", func() { + // This combination used to skip the segments entirely, leaving the staging + // directory missing for the helper files a resize restore puts there. + cmdFlags.Set(options.PLUGIN_CONFIG, "/tmp/plugin_config.yaml") + restore.SetBackupConfig(&history.BackupConfig{SegmentCount: 2, SingleDataFile: false}) + + restore.EnsureBackupDirectoriesExistOnAllHosts() + + Expect(testExecutor.NumClusterExecutions).To(Equal(1)) + cc := testExecutor.ClusterCommands[0] + Expect(cc).To(HaveLen(2)) + Expect(cc[0].CommandString).To(ContainSubstring("mkdir -p /data/gpseg0/backups/20170101/20170101010101")) + Expect(cc[1].CommandString).To(ContainSubstring("mkdir -p /data/gpseg1/backups/20170101/20170101010101")) + }) + }) }) diff --git a/restore/wrappers.go b/restore/wrappers.go index cef752bf..f7716a07 100644 --- a/restore/wrappers.go +++ b/restore/wrappers.go @@ -163,13 +163,14 @@ func InitializeBackupConfig() { } func BackupConfigurationValidation() { - // This must run first: every check below reads the backup files from local - // disk, which is only where they live when no plugin was used. + // This must run first: with no plugin in effect the checks below assume the + // backup files are on local disk, and would report a missing directory or + // file instead of the missing --plugin-config flag. ValidateBackupFlagPluginCombinations() if !backupConfig.MetadataOnly { gplog.Verbose("Gathering information on backup directories") - VerifyBackupDirectoriesExistOnAllHosts() + EnsureBackupDirectoriesExistOnAllHosts() } VerifyMetadataFilePaths(MustGetFlagBool(options.WITH_STATS)) From 1f20023d6744a6d8688fdabfa4a3dd09a0f14a37 Mon Sep 17 00:00:00 2001 From: Ivan Kirilin Date: Fri, 4 Sep 2026 12:13:08 +0200 Subject: [PATCH 3/3] fix(backup): reject an empty --plugin-config on the delete subcommands too --plugin-config is registered on four flag sets: gpbackup, gprestore, delete-backup and delete-backups-before. gpbackup and gprestore now reject an explicitly-empty value, but the two delete commands never call DoFlagValidation, so an empty value still reached setupDeletionTargets as "" and meant "no plugin". Lifts the guard out of validateFlagValues into an exported ValidatePluginConfigFlag and calls it from DoDeleteBackup and DoDeleteBackupsBefore, after UseCmdFlags has pointed cmdFlags at the subcommand's own flag set. Reported on the ticket: a silent fallback to local storage is dangerous in both directions -- a "backup" that is not on the appliance can fill the segment hosts, and a delete that ignores the plugin removes only the local half of a backup set. --- backup/delete_backup.go | 1 + backup/delete_backup_test.go | 33 +++++++++++++++++++++++++++++++++ backup/delete_backups_before.go | 1 + backup/validate.go | 24 ++++++++++++++++++------ 4 files changed, 53 insertions(+), 6 deletions(-) diff --git a/backup/delete_backup.go b/backup/delete_backup.go index 113f780e..abcffba2 100644 --- a/backup/delete_backup.go +++ b/backup/delete_backup.go @@ -117,6 +117,7 @@ func DoDeleteBackupInit(cmd *cobra.Command) { func DoDeleteBackup(timestamp string) { SetLoggerVerbosity() + ValidatePluginConfigFlag() if !filepath.IsValidTimestamp(timestamp) { gplog.Fatal(errors.Errorf("Invalid timestamp: %s", timestamp), "") diff --git a/backup/delete_backup_test.go b/backup/delete_backup_test.go index 6ed11541..12d429e6 100644 --- a/backup/delete_backup_test.go +++ b/backup/delete_backup_test.go @@ -11,6 +11,7 @@ import ( "github.com/greenplum-db/gpbackup/history" "github.com/greenplum-db/gpbackup/utils" + "github.com/spf13/cobra" "github.com/warehouse-pg/common-go-libs/cluster" "github.com/warehouse-pg/common-go-libs/testhelper" @@ -442,6 +443,38 @@ var _ = Describe("delete-backup internal tests", func() { }) }) + Describe("ValidatePluginConfigFlag", func() { + // delete-backup and delete-backups-before never run DoFlagValidation, so + // they call this guard themselves; exercise it through their own flag set. + runWithArgs := func(args []string) error { + testCmd := &cobra.Command{ + Use: "delete-backup", + Args: cobra.NoArgs, + Run: func(cmd *cobra.Command, _ []string) { + UseCmdFlags(cmd.Flags()) + ValidatePluginConfigFlag() + }} + RegisterDeleteBackupFlags(testCmd.Flags()) + testCmd.SetArgs(args) + return testCmd.Execute() + } + + It("fails when --plugin-config is specified with an empty value", func() { + defer testhelper.ShouldPanicWithMessage("The --plugin-config flag was specified with an empty value") + if err := runWithArgs([]string{"--plugin-config", ""}); err == nil { + Fail("empty --plugin-config value passed validation check") + } + }) + + It("passes when --plugin-config is specified with a path", func() { + Expect(runWithArgs([]string{"--plugin-config", "/etc/ddboost_config.yaml"})).To(Succeed()) + }) + + It("passes when --plugin-config is not specified", func() { + Expect(runWithArgs([]string{})).To(Succeed()) + }) + }) + Describe("newlyAddedDependents", func() { It("returns nothing when recheck matches the original set", func() { original := []*history.BackupConfig{{Timestamp: "20260101000000"}, {Timestamp: "20260101010000"}} diff --git a/backup/delete_backups_before.go b/backup/delete_backups_before.go index a6c99748..2149049a 100644 --- a/backup/delete_backups_before.go +++ b/backup/delete_backups_before.go @@ -29,6 +29,7 @@ func DoDeleteBackupsBeforeInit(cmd *cobra.Command) { // delete-backup --cascade. func DoDeleteBackupsBefore(cutoff string) { SetLoggerVerbosity() + ValidatePluginConfigFlag() if !filepath.IsValidTimestamp(cutoff) { gplog.Fatal(errors.Errorf("Invalid timestamp: %s", cutoff), "") diff --git a/backup/validate.go b/backup/validate.go index 5317cd0b..b368576f 100644 --- a/backup/validate.go +++ b/backup/validate.go @@ -164,14 +164,26 @@ func validateFlagCombinations(flags *pflag.FlagSet) { } } -func validateFlagValues() { - // ValidateFullPath accepts the empty string and DoSetup treats an empty - // plugin config as "no plugin", so an unset or mistyped shell variable would - // otherwise silently produce a local-disk backup instead of one on the - // plugin's storage. +/* + * ValidatePluginConfigFlag rejects --plugin-config given with an empty value. + * ValidateFullPath accepts the empty string and every command that takes this + * flag treats an empty plugin config as "no plugin at all", so an unset or + * mistyped shell variable would otherwise silently write the backup to local + * disk instead of to the plugin's storage -- which can fill up the segment + * hosts -- or delete only the local half of a backup set. + * + * Call this from any command that registers --plugin-config: gpbackup itself + * (via validateFlagValues), delete-backup and delete-backups-before. It must + * run after UseCmdFlags has pointed cmdFlags at the subcommand's flag set. + */ +func ValidatePluginConfigFlag() { if FlagChanged(options.PLUGIN_CONFIG) && MustGetFlagString(options.PLUGIN_CONFIG) == "" { - gplog.Fatal(errors.Errorf("The --plugin-config flag was specified with an empty value. Specify the absolute path to the plugin configuration file, or omit the flag to back up to local disk."), "") + gplog.Fatal(errors.Errorf("The --plugin-config flag was specified with an empty value. Specify the absolute path to the plugin configuration file, or omit the flag to use local storage."), "") } +} + +func validateFlagValues() { + ValidatePluginConfigFlag() err := utils.ValidateFullPath(MustGetFlagString(options.BACKUP_DIR)) gplog.FatalOnError(err) err = utils.ValidateFullPath(MustGetFlagString(options.PLUGIN_CONFIG))