diff --git a/cmd/non-admin/backup/backup_test.go b/cmd/non-admin/backup/backup_test.go index 91eb1fd..fd2077d 100644 --- a/cmd/non-admin/backup/backup_test.go +++ b/cmd/non-admin/backup/backup_test.go @@ -69,6 +69,7 @@ func TestNonAdminBackupCommands(t *testing.T) { expectContains: []string{ "Delete one or more non-admin backups", "--confirm", + "--all", }, }, { diff --git a/cmd/non-admin/backup/delete.go b/cmd/non-admin/backup/delete.go index 6438514..c86e1e0 100644 --- a/cmd/non-admin/backup/delete.go +++ b/cmd/non-admin/backup/delete.go @@ -41,13 +41,19 @@ func NewDeleteCommand(f client.Factory, use string) *cobra.Command { o := NewDeleteOptions() c := &cobra.Command{ - Use: use + " NAME [NAME...]", + Use: use + " [NAME...] | --all", Short: "Delete one or more non-admin backups", Long: "Delete one or more non-admin backups by setting the deletebackup field to true", - Args: cobra.MinimumNArgs(1), + Args: func(cmd *cobra.Command, args []string) error { + allFlag, _ := cmd.Flags().GetBool("all") + if len(args) == 0 && !allFlag { + return fmt.Errorf("at least one backup name or the --all flag must be specified") + } + return nil + }, Run: func(c *cobra.Command, args []string) { cmd.CheckError(o.Complete(args, f)) - cmd.CheckError(o.Validate()) + cmd.CheckError(o.Validate(args)) cmd.CheckError(o.Run()) }, } @@ -64,6 +70,7 @@ type DeleteOptions struct { Names []string Namespace string // Internal field - automatically determined from kubectl context Confirm bool // Skip confirmation prompt + All bool // Delete all backups in the namespace client kbclient.Client } @@ -75,11 +82,15 @@ func NewDeleteOptions() *DeleteOptions { // BindFlags binds the command line flags to the options func (o *DeleteOptions) BindFlags(flags *pflag.FlagSet) { flags.BoolVar(&o.Confirm, "confirm", false, "Skip confirmation prompt and delete immediately") + flags.BoolVar(&o.All, "all", false, "Delete all backups in the current namespace") } // Complete completes the options by setting up the client and determining the namespace func (o *DeleteOptions) Complete(args []string, f client.Factory) error { - o.Names = args + // If --all flag is not used, use the provided args + if !o.All { + o.Names = args + } // Create client with NonAdmin scheme kbClient, err := shared.NewClientWithScheme(f, shared.ClientOptions{ @@ -98,17 +109,46 @@ func (o *DeleteOptions) Complete(args []string, f client.Factory) error { } o.Namespace = currentNS + // If --all flag is used, get all backup names in the namespace + if o.All { + var nabList nacv1alpha1.NonAdminBackupList + err := o.client.List(context.TODO(), &nabList, kbclient.InNamespace(o.Namespace)) + if err != nil { + return fmt.Errorf("failed to list backups: %w", err) + } + + // Extract backup names + var allNames []string + for _, nab := range nabList.Items { + allNames = append(allNames, nab.Name) + } + o.Names = allNames + } + return nil } // Validate validates the options -func (o *DeleteOptions) Validate() error { - if len(o.Names) == 0 { - return fmt.Errorf("at least one backup name is required") - } +func (o *DeleteOptions) Validate(args []string) error { if o.Namespace == "" { return fmt.Errorf("namespace is required") } + + // Check for conflicting options: both args and --all flag + if o.All && len(args) > 0 { + return fmt.Errorf("cannot specify both backup names and --all flag") + } + + // Check if neither args nor --all flag provided + if !o.All && len(args) == 0 { + return fmt.Errorf("at least one backup name is required, or use --all to delete all backups") + } + + // Special case: if --all is used but no backups found (after Complete) + if o.All && len(o.Names) == 0 { + return fmt.Errorf("no backups found in namespace '%s'", o.Namespace) + } + return nil } diff --git a/cmd/non-admin/backup/delete_test.go b/cmd/non-admin/backup/delete_test.go new file mode 100644 index 0000000..1468393 --- /dev/null +++ b/cmd/non-admin/backup/delete_test.go @@ -0,0 +1,142 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package backup + +import ( + "testing" +) + +func TestDeleteOptionsValidate(t *testing.T) { + tests := []struct { + name string + options *DeleteOptions + args []string + expectError bool + errorMsg string + }{ + { + name: "valid with backup names", + options: &DeleteOptions{ + Names: []string{"backup1", "backup2"}, + Namespace: "test-namespace", + All: false, + }, + args: []string{"backup1", "backup2"}, + expectError: false, + }, + { + name: "valid with --all flag", + options: &DeleteOptions{ + Names: []string{"backup1"}, // Simulating found backups after listing + Namespace: "test-namespace", + All: true, + }, + args: []string{}, // No args when using --all + expectError: false, + }, + { + name: "invalid - both names and --all", + options: &DeleteOptions{ + Names: []string{}, + Namespace: "test-namespace", + All: true, + }, + args: []string{"backup1"}, // Args provided with --all + expectError: true, + errorMsg: "cannot specify both backup names and --all flag", + }, + { + name: "invalid - neither names nor --all", + options: &DeleteOptions{ + Names: []string{}, + Namespace: "test-namespace", + All: false, + }, + args: []string{}, // No args and no --all + expectError: true, + errorMsg: "at least one backup name is required, or use --all to delete all backups", + }, + { + name: "invalid - missing namespace", + options: &DeleteOptions{ + Names: []string{"backup1"}, + Namespace: "", + All: false, + }, + args: []string{"backup1"}, + expectError: true, + errorMsg: "namespace is required", + }, + { + name: "invalid - --all flag but no backups found", + options: &DeleteOptions{ + Names: []string{}, // No backups found + Namespace: "test-namespace", + All: true, + }, + args: []string{}, // No args + expectError: true, + errorMsg: "no backups found in namespace 'test-namespace'", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.options.Validate(tt.args) + + if tt.expectError { + if err == nil { + t.Errorf("expected error but got none") + return + } + if err.Error() != tt.errorMsg { + t.Errorf("expected error message %q, got %q", tt.errorMsg, err.Error()) + } + } else { + if err != nil { + t.Errorf("expected no error but got: %v", err) + } + } + }) + } +} + +func TestDeleteOptionsValidateLogic(t *testing.T) { + // Test the special validation logic for --all with args provided + t.Run("all flag should reject explicit backup names as args", func(t *testing.T) { + // This simulates what would happen if someone runs: + // oadp nonadmin backup delete backup1 backup2 --all + // The args would be captured as Names, but All=true should cause validation error + options := &DeleteOptions{ + Names: []string{}, // Names not set yet (before Complete) + Namespace: "test-namespace", + All: true, // But --all flag is also specified + } + + args := []string{"backup1", "backup2"} // From command line args + + err := options.Validate(args) + if err == nil { + t.Errorf("expected error when both backup names and --all flag are provided") + } + + expectedMsg := "cannot specify both backup names and --all flag" + if err.Error() != expectedMsg { + t.Errorf("expected error message %q, got %q", expectedMsg, err.Error()) + } + }) +}