From 3ca547f186dd0b105aa0947238a281406f6e3430 Mon Sep 17 00:00:00 2001 From: Ajay Sharma Date: Tue, 4 Feb 2025 13:25:49 +0000 Subject: [PATCH 1/3] validate `--from-schedule` flag Signed-off-by: Ajay Sharma --- pkg/cmd/cli/backup/create.go | 22 ++++++++++++++++++ pkg/cmd/cli/backup/create_test.go | 37 +++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index 6daacef7d..17ce073f3 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -179,6 +179,11 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto return fmt.Errorf("either a 'selector' or an 'or-selector' can be specified, but not both") } + // Ensure if FromSchedule is set, it has a non-empty value + if err := o.validateFromScheduleFlag(c); err != nil { + return err + } + // Ensure that unless FromSchedule is set, args contains a backup name if o.FromSchedule == "" && len(args) != 1 { return fmt.Errorf("a backup name is required, unless you are creating based on a schedule") @@ -215,6 +220,23 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto return nil } +func (o *CreateOptions) validateFromScheduleFlag(c *cobra.Command) error { + fromSchedule, err := c.Flags().GetString("from-schedule") + if err != nil { + return err + } + + trimmed := strings.TrimSpace(fromSchedule) + if c.Flags().Changed("from-schedule") && trimmed == "" { + return fmt.Errorf("flag must have a non-empty value: --from-schedule") + } + + // Assign the trimmed value back + o.FromSchedule = trimmed + + return nil +} + func (o *CreateOptions) Complete(args []string, f client.Factory) error { // If an explicit name is specified, use that name if len(args) > 0 { diff --git a/pkg/cmd/cli/backup/create_test.go b/pkg/cmd/cli/backup/create_test.go index b9be38e1e..66f0c4b77 100644 --- a/pkg/cmd/cli/backup/create_test.go +++ b/pkg/cmd/cli/backup/create_test.go @@ -24,6 +24,7 @@ import ( "testing" "time" + "github.com/spf13/cobra" flag "github.com/spf13/pflag" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -86,6 +87,42 @@ func TestCreateOptions_BuildBackup(t *testing.T) { }, backup.Spec.OrderedResources) } +func TestCreateOptions_ValidateFromScheduleFlag(t *testing.T) { + o := &CreateOptions{} + cmd := &cobra.Command{} + + cmd.Flags().String("from-schedule", "", "Test from-schedule flag") + + t.Run("from-schedule with empty or no value", func(t *testing.T) { + cmd.Flags().Set("from-schedule", "") + err := o.validateFromScheduleFlag(cmd) + require.True(t, cmd.Flags().Changed("from-schedule")) + require.Error(t, err) + require.Equal(t, "flag must have a non-empty value: --from-schedule", err.Error()) + }) + + t.Run("from-schedule with spaces only", func(t *testing.T) { + cmd.Flags().Set("from-schedule", " ") + err := o.validateFromScheduleFlag(cmd) + require.Error(t, err) + require.Equal(t, "flag must have a non-empty value: --from-schedule", err.Error()) + }) + + t.Run("from-schedule with valid value", func(t *testing.T) { + cmd.Flags().Set("from-schedule", "daily") + err := o.validateFromScheduleFlag(cmd) + require.NoError(t, err) + require.Equal(t, "daily", o.FromSchedule) + }) + + t.Run("from-schedule with leading and trailing spaces", func(t *testing.T) { + cmd.Flags().Set("from-schedule", " daily ") + err := o.validateFromScheduleFlag(cmd) + require.NoError(t, err) + require.Equal(t, "daily", o.FromSchedule) + }) +} + func TestCreateOptions_BuildBackupFromSchedule(t *testing.T) { o := NewCreateOptions() o.FromSchedule = "test" From e9bd9f3c8d91655ab2a1f9009144a433e7401590 Mon Sep 17 00:00:00 2001 From: Ajay Sharma Date: Wed, 5 Feb 2025 14:21:36 +0000 Subject: [PATCH 2/3] add changelog Signed-off-by: Ajay Sharma --- changelogs/unreleased/8665-aj-2000 | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelogs/unreleased/8665-aj-2000 diff --git a/changelogs/unreleased/8665-aj-2000 b/changelogs/unreleased/8665-aj-2000 new file mode 100644 index 000000000..2c37bc7fe --- /dev/null +++ b/changelogs/unreleased/8665-aj-2000 @@ -0,0 +1 @@ +Fixes issue #8214, validate `--from-schedule` flag in create backup command to prevent empty or whitespace-only values. From 06fc9da9250b578aff52ad53b893a156ce44b184 Mon Sep 17 00:00:00 2001 From: Ajay Sharma Date: Fri, 7 Feb 2025 14:55:43 +0000 Subject: [PATCH 3/3] refactor code Signed-off-by: Ajay Sharma --- pkg/cmd/cli/backup/create.go | 8 +------- pkg/cmd/cli/backup/create_test.go | 6 +++--- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index 17ce073f3..da2ddc770 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -221,19 +221,13 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto } func (o *CreateOptions) validateFromScheduleFlag(c *cobra.Command) error { - fromSchedule, err := c.Flags().GetString("from-schedule") - if err != nil { - return err - } - - trimmed := strings.TrimSpace(fromSchedule) + trimmed := strings.TrimSpace(o.FromSchedule) if c.Flags().Changed("from-schedule") && trimmed == "" { return fmt.Errorf("flag must have a non-empty value: --from-schedule") } // Assign the trimmed value back o.FromSchedule = trimmed - return nil } diff --git a/pkg/cmd/cli/backup/create_test.go b/pkg/cmd/cli/backup/create_test.go index 66f0c4b77..1615c6eef 100644 --- a/pkg/cmd/cli/backup/create_test.go +++ b/pkg/cmd/cli/backup/create_test.go @@ -88,10 +88,9 @@ func TestCreateOptions_BuildBackup(t *testing.T) { } func TestCreateOptions_ValidateFromScheduleFlag(t *testing.T) { - o := &CreateOptions{} cmd := &cobra.Command{} - - cmd.Flags().String("from-schedule", "", "Test from-schedule flag") + o := NewCreateOptions() + o.BindFromSchedule(cmd.Flags()) t.Run("from-schedule with empty or no value", func(t *testing.T) { cmd.Flags().Set("from-schedule", "") @@ -104,6 +103,7 @@ func TestCreateOptions_ValidateFromScheduleFlag(t *testing.T) { t.Run("from-schedule with spaces only", func(t *testing.T) { cmd.Flags().Set("from-schedule", " ") err := o.validateFromScheduleFlag(cmd) + require.True(t, cmd.Flags().Changed("from-schedule")) require.Error(t, err) require.Equal(t, "flag must have a non-empty value: --from-schedule", err.Error()) })