diff --git a/changelogs/unreleased/10167-samay43 b/changelogs/unreleased/10167-samay43 new file mode 100644 index 000000000..0955fcc99 --- /dev/null +++ b/changelogs/unreleased/10167-samay43 @@ -0,0 +1 @@ +Validate backup name format before contacting the API server diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index 8cdec3d99..2150061ab 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -25,6 +25,7 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" kubeerrs "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/client-go/tools/cache" kbclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -45,7 +46,21 @@ func NewCreateCommand(f client.Factory, use string) *cobra.Command { c := &cobra.Command{ Use: use + " NAME", Short: "Create a backup", - Args: cobra.MaximumNArgs(1), + Args: func(c *cobra.Command, args []string) error { + if err := cobra.MaximumNArgs(1)(c, args); err != nil { + return err + } + fromSchedule, _ := c.Flags().GetString("from-schedule") + if fromSchedule == "" && len(args) == 0 { + return fmt.Errorf("a backup name is required, unless you are creating based on a schedule") + } + if len(args) == 1 { + if errs := validation.IsDNS1123Subdomain(args[0]); len(errs) > 0 { + return fmt.Errorf("invalid backup name %q: %s", args[0], strings.Join(errs, "; ")) + } + } + return nil + }, Run: func(c *cobra.Command, args []string) { cmd.CheckError(o.Complete(args, f)) cmd.CheckError(o.Validate(c, args, f)) @@ -191,11 +206,27 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto return err } - // Ensure that unless FromSchedule is set, args contains a backup name - if o.FromSchedule == "" && len(args) != 1 { + // Ensure that unless FromSchedule is set, a backup name is required + if o.FromSchedule == "" && o.Name == "" { return fmt.Errorf("a backup name is required, unless you are creating based on a schedule") } - + // Validate the backup name format whenever a name is provided + if o.Name != "" { + if errs := validation.IsDNS1123Subdomain(o.Name); len(errs) > 0 { + return fmt.Errorf("invalid backup name %q: %s", o.Name, strings.Join(errs, "; ")) + } + } + // When a backup name will be generated from the schedule (i.e. FromSchedule + // is set and no explicit name was given), ensure the schedule name leaves + // enough room for the generated timestamp suffix ("-" + 14-digit timestamp, + // 15 characters total) within the DNS1123 subdomain length limit. + if o.FromSchedule != "" && o.Name == "" { + const timestampSuffixLen = 15 // "-" + "20060102150405" + maxScheduleNameLen := validation.DNS1123SubdomainMaxLength - timestampSuffixLen + if len(o.FromSchedule) > maxScheduleNameLen { + return fmt.Errorf("schedule name %q is too long: must be %d characters or fewer to leave room for the generated timestamp suffix", o.FromSchedule, maxScheduleNameLen) + } + } errs := collections.ValidateNamespaceIncludesExcludes(o.IncludeNamespaces, o.ExcludeNamespaces) if len(errs) > 0 { return kubeerrs.NewAggregate(errs) diff --git a/pkg/cmd/cli/backup/create_test.go b/pkg/cmd/cli/backup/create_test.go index 07d5bb493..90e9e8c8a 100644 --- a/pkg/cmd/cli/backup/create_test.go +++ b/pkg/cmd/cli/backup/create_test.go @@ -245,7 +245,7 @@ func TestCreateOptions_OrderedResources(t *testing.T) { } func TestCreateCommand(t *testing.T) { - name := "nameToBeCreated" + name := "name-to-be-created" args := []string{name} t.Run("create a backup create command with full options except fromSchedule and wait, then run by create option", func(t *testing.T) { @@ -457,3 +457,127 @@ func TestCreateCommand(t *testing.T) { assert.NoError(t, e) }) } +func TestCreateCommand_Args(t *testing.T) { + testCases := []struct { + name string + args []string + fromSchedule string + expectError bool + }{ + { + name: "should error when no name and no from-schedule", + args: []string{}, + expectError: true, + }, + { + name: "should pass when a valid name is provided", + args: []string{"my-backup"}, + expectError: false, + }, + { + name: "should error when the name is not a valid DNS1123 subdomain", + args: []string{"Invalid_Name!"}, + expectError: true, + }, + { + name: "should pass with no name when from-schedule is set", + args: []string{}, + fromSchedule: "daily-backup", + expectError: false, + }, + { + name: "should error when more than one arg is given", + args: []string{"name1", "name2"}, + expectError: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + f := &factorymocks.Factory{} + cmd := NewCreateCommand(f, "") + if tc.fromSchedule != "" { + err := cmd.Flags().Set("from-schedule", tc.fromSchedule) + require.NoError(t, err) + } + + err := cmd.Args(cmd, tc.args) + + if tc.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestCreateOptions_Validate(t *testing.T) { + testCases := []struct { + name string + optName string + fromSchedule string + args []string + expectError bool + }{ + { + name: "should error when no name and no from-schedule", + optName: "", + args: []string{}, + expectError: true, + }, + { + name: "should pass with a valid name and no from-schedule", + optName: "my-backup", + args: []string{"my-backup"}, + expectError: false, + }, + { + name: "should error when name is invalid, regardless of from-schedule", + optName: "Invalid_Name!", + fromSchedule: "daily-backup", + args: []string{"Invalid_Name!"}, + expectError: true, + }, + { + name: "should pass when from-schedule is set and no name given", + optName: "", + fromSchedule: "daily-backup", + args: []string{}, + expectError: false, + }, + { + name: "should pass when schedule name leaves room for timestamp suffix", + optName: "", + fromSchedule: strings.Repeat("a", 238), // exactly at the 238-char limit + args: []string{}, + expectError: false, + }, + { + name: "should error when schedule name is too long to leave room for timestamp suffix", + optName: "", + fromSchedule: strings.Repeat("a", 239), // one over the 238-char limit + args: []string{}, + expectError: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + f := &factorymocks.Factory{} + cmd := NewCreateCommand(f, "") + + o := NewCreateOptions() + o.Name = tc.optName + o.FromSchedule = tc.fromSchedule + + err := o.Validate(cmd, tc.args, f) + + if tc.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +}