From 33192acb9a5a3227cee2382dbd17d1603f9ebe6a Mon Sep 17 00:00:00 2001 From: Nicholas Yancey Date: Thu, 4 Jun 2026 15:01:34 -0400 Subject: [PATCH 1/7] ... --- cmd/non-admin/backup/create.go | 121 +++++++++++++++++++++++++-------- 1 file changed, 92 insertions(+), 29 deletions(-) diff --git a/cmd/non-admin/backup/create.go b/cmd/non-admin/backup/create.go index 9c8ac382..4c1df9d7 100644 --- a/cmd/non-admin/backup/create.go +++ b/cmd/non-admin/backup/create.go @@ -17,8 +17,13 @@ limitations under the License. */ import ( + "bufio" "context" "fmt" + "io" + "os" + "strconv" + "strings" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -32,6 +37,7 @@ import ( "github.com/vmware-tanzu/velero/pkg/cmd" velerobackup "github.com/vmware-tanzu/velero/pkg/cmd/cli/backup" "github.com/vmware-tanzu/velero/pkg/cmd/util/output" + "golang.org/x/term" ) func NewCreateCommand(f client.Factory, use string) *cobra.Command { @@ -84,6 +90,7 @@ type CreateOptions struct { currentNamespace string storageLocationFromConfig bool // Track if storage location came from config storageLocationAutoSelected bool // Track if storage location was auto-selected + storageLocationPrompted bool // Track if storage location was chosen interactively } func NewCreateOptions() *CreateOptions { @@ -165,34 +172,8 @@ func (o *CreateOptions) Complete(args []string, f client.Factory) error { o.client = client o.currentNamespace = currentNS - // Load default NABSL from config if not provided via flag, or auto-select if exactly one exists - if o.StorageLocation == "" { - defaultNABSL := getNABSLFromConfig() - if defaultNABSL != "" { - o.StorageLocation = defaultNABSL - o.storageLocationFromConfig = true - } else { - // Auto-select NABSL if exactly one approved/created exists in the namespace - nabslList := &nacv1alpha1.NonAdminBackupStorageLocationList{} - if err := o.client.List(context.TODO(), nabslList, &kbclient.ListOptions{ - Namespace: currentNS, - }); err != nil { - return fmt.Errorf("failed to list NonAdminBackupStorageLocations: %w", err) - } - - // Filter to only approved/created NABSLs (exclude pending/rejected) - var usableNABSLs []nacv1alpha1.NonAdminBackupStorageLocation - for _, nabsl := range nabslList.Items { - if nabsl.Status.Phase == nacv1alpha1.NonAdminPhaseCreated { - usableNABSLs = append(usableNABSLs, nabsl) - } - } - - if len(usableNABSLs) == 1 { - o.StorageLocation = usableNABSLs[0].Name - o.storageLocationAutoSelected = true - } - } + if err := o.resolveStorageLocation(currentNS); err != nil { + return err } return nil @@ -217,9 +198,12 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { fmt.Printf("Using default nonadmin backup storage location from config: %s\n", o.StorageLocation) } if o.storageLocationAutoSelected { - fmt.Printf("Auto-selected storage location: %s (only NABSL in namespace)\n", o.StorageLocation) + fmt.Printf("Auto-selected storage location: %s (only usable NABSL in namespace)\n", o.StorageLocation) fmt.Printf("Warning: If you create another NABSL in this namespace, future backups may not use the same location.\n") } + if o.storageLocationPrompted { + fmt.Printf("Selected storage location: %s\n", o.StorageLocation) + } fmt.Printf("NonAdminBackup request %q submitted successfully.\n", nonAdminBackup.Name) fmt.Printf("Run `oc oadp nonadmin backup describe %s` or `oc oadp nonadmin backup logs %s` for more details.\n", nonAdminBackup.Name, nonAdminBackup.Name) @@ -296,3 +280,82 @@ func getNABSLFromConfig() string { } return "" } + +func (o *CreateOptions) resolveStorageLocation(namespace string) error { + if o.StorageLocation != "" { + return nil + } + + if defaultNABSL := getNABSLFromConfig(); defaultNABSL != "" { + o.StorageLocation = defaultNABSL + o.storageLocationFromConfig = true + return nil + } + + nabslList := &nacv1alpha1.NonAdminBackupStorageLocationList{} + if err := o.client.List(context.TODO(), nabslList, &kbclient.ListOptions{ + Namespace: namespace, + }); err != nil { + return fmt.Errorf("failed to list NonAdminBackupStorageLocations: %w", err) + } + + return o.resolveStorageLocationFromList(nabslList.Items) +} + +func (o *CreateOptions) resolveStorageLocationFromList(items []nacv1alpha1.NonAdminBackupStorageLocation) error { + switch len(items) { + case 0: + return nil + case 1: + if items[0].Status.Phase == nacv1alpha1.NonAdminPhaseCreated { + o.StorageLocation = items[0].Name + o.storageLocationAutoSelected = true + } + return nil + default: + selected, err := promptForNABSLSelection(items, os.Stdin, os.Stdout) + if err != nil { + return err + } + o.StorageLocation = selected + o.storageLocationPrompted = true + return nil + } +} + +func promptForNABSLSelection(items []nacv1alpha1.NonAdminBackupStorageLocation, in io.Reader, out io.Writer) (string, error) { + stdin, ok := in.(*os.File) + if !ok || !term.IsTerminal(int(stdin.Fd())) { + return "", fmt.Errorf("multiple NonAdminBackupStorageLocations found; specify one with --storage-location\n" + + "To list available locations, run: oc oadp nonadmin bsl get") + } + + fmt.Fprintln(out, "Multiple non-admin backup storage locations found. Select one:") + for i, nabsl := range items { + fmt.Fprintf(out, " %d) %s (%s)\n", i+1, nabsl.Name, formatNABSLPhase(&nabsl)) + } + + reader := bufio.NewReader(in) + for { + fmt.Fprintf(out, "Enter number (1-%d): ", len(items)) + response, err := reader.ReadString('\n') + if err != nil { + return "", fmt.Errorf("failed to read user input: %w", err) + } + + choice, err := strconv.Atoi(strings.TrimSpace(response)) + if err != nil || choice < 1 || choice > len(items) { + fmt.Fprintln(out, "Invalid selection. Please enter a number from the list.") + continue + } + + return items[choice-1].Name, nil + } +} + +func formatNABSLPhase(nabsl *nacv1alpha1.NonAdminBackupStorageLocation) string { + if nabsl.Status.Phase != "" { + return string(nabsl.Status.Phase) + } + return "Unknown" +} From 10a3718820d1e318ed83326279fee977e32985fc Mon Sep 17 00:00:00 2001 From: Nicholas Yancey Date: Thu, 4 Jun 2026 15:57:01 -0400 Subject: [PATCH 2/7] go mod tidy --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index a756d365..5b6a6291 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/spf13/pflag v1.0.6 github.com/vmware-tanzu/velero v1.14.0 golang.org/x/sync v0.20.0 + golang.org/x/term v0.41.0 gopkg.in/yaml.v2 v2.4.0 k8s.io/api v0.33.3 k8s.io/apimachinery v0.33.3 @@ -97,7 +98,6 @@ require ( golang.org/x/net v0.52.0 // indirect golang.org/x/oauth2 v0.33.0 // indirect golang.org/x/sys v0.42.0 // indirect - golang.org/x/term v0.41.0 // indirect golang.org/x/text v0.35.0 // indirect golang.org/x/time v0.14.0 // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect From 16a80df01bf3e3f151747a40736c6a43c3ee64e8 Mon Sep 17 00:00:00 2001 From: Nicholas Yancey Date: Thu, 4 Jun 2026 15:59:28 -0400 Subject: [PATCH 3/7] Write NABSL prompt to stderr and require TTY on both streams --- cmd/non-admin/backup/create.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/non-admin/backup/create.go b/cmd/non-admin/backup/create.go index 4c1df9d7..d03b6f3d 100644 --- a/cmd/non-admin/backup/create.go +++ b/cmd/non-admin/backup/create.go @@ -313,7 +313,7 @@ func (o *CreateOptions) resolveStorageLocationFromList(items []nacv1alpha1.NonAd } return nil default: - selected, err := promptForNABSLSelection(items, os.Stdin, os.Stdout) + selected, err := promptForNABSLSelection(items, os.Stdin, os.Stderr) if err != nil { return err } @@ -324,8 +324,9 @@ func (o *CreateOptions) resolveStorageLocationFromList(items []nacv1alpha1.NonAd } func promptForNABSLSelection(items []nacv1alpha1.NonAdminBackupStorageLocation, in io.Reader, out io.Writer) (string, error) { - stdin, ok := in.(*os.File) - if !ok || !term.IsTerminal(int(stdin.Fd())) { + inFile, inOk := in.(*os.File) + outFile, outOk := out.(*os.File) + if !inOk || !outOk || !term.IsTerminal(int(inFile.Fd())) || !term.IsTerminal(int(outFile.Fd())) { return "", fmt.Errorf("multiple NonAdminBackupStorageLocations found; specify one with --storage-location\n" + "To list available locations, run: oc oadp nonadmin bsl get") } From 4fa44075a7c006c7f574e615d3951f83648580d6 Mon Sep 17 00:00:00 2001 From: Nicholas Yancey Date: Fri, 5 Jun 2026 09:03:37 -0400 Subject: [PATCH 4/7] Only using nabsl fix consistent with pr #193, only uses usable nabsl Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- cmd/non-admin/backup/create.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/cmd/non-admin/backup/create.go b/cmd/non-admin/backup/create.go index d03b6f3d..62b56099 100644 --- a/cmd/non-admin/backup/create.go +++ b/cmd/non-admin/backup/create.go @@ -303,17 +303,23 @@ func (o *CreateOptions) resolveStorageLocation(namespace string) error { } func (o *CreateOptions) resolveStorageLocationFromList(items []nacv1alpha1.NonAdminBackupStorageLocation) error { - switch len(items) { + // Filter to only approved/created NABSLs (exclude pending/rejected) + usable := make([]nacv1alpha1.NonAdminBackupStorageLocation, 0, len(items)) + for _, nabsl := range items { + if nabsl.Status.Phase == nacv1alpha1.NonAdminPhaseCreated { + usable = append(usable, nabsl) + } + } + + switch len(usable) { case 0: return nil case 1: - if items[0].Status.Phase == nacv1alpha1.NonAdminPhaseCreated { - o.StorageLocation = items[0].Name - o.storageLocationAutoSelected = true - } + o.StorageLocation = usable[0].Name + o.storageLocationAutoSelected = true return nil default: - selected, err := promptForNABSLSelection(items, os.Stdin, os.Stderr) + selected, err := promptForNABSLSelection(usable, os.Stdin, os.Stderr) if err != nil { return err } @@ -322,6 +328,7 @@ func (o *CreateOptions) resolveStorageLocationFromList(items []nacv1alpha1.NonAd return nil } } +} func promptForNABSLSelection(items []nacv1alpha1.NonAdminBackupStorageLocation, in io.Reader, out io.Writer) (string, error) { inFile, inOk := in.(*os.File) From 6f95b14fd4dcbff26d39254900a9bd1fcf8c1c97 Mon Sep 17 00:00:00 2001 From: Nicholas Yancey Date: Fri, 5 Jun 2026 09:13:00 -0400 Subject: [PATCH 5/7] Fix extra closing brace in resolveStorageLocationFromList --- cmd/non-admin/backup/create.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/non-admin/backup/create.go b/cmd/non-admin/backup/create.go index 62b56099..a992c1b9 100644 --- a/cmd/non-admin/backup/create.go +++ b/cmd/non-admin/backup/create.go @@ -328,7 +328,6 @@ func (o *CreateOptions) resolveStorageLocationFromList(items []nacv1alpha1.NonAd return nil } } -} func promptForNABSLSelection(items []nacv1alpha1.NonAdminBackupStorageLocation, in io.Reader, out io.Writer) (string, error) { inFile, inOk := in.(*os.File) From 9592206ee743fe869c6e31d0e12ec9f8c56af016 Mon Sep 17 00:00:00 2001 From: Nicholas Yancey Date: Fri, 5 Jun 2026 09:19:50 -0400 Subject: [PATCH 6/7] Returns error when no usable NABSL exists in namespace --- cmd/non-admin/backup/create.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/cmd/non-admin/backup/create.go b/cmd/non-admin/backup/create.go index a992c1b9..5fe663b2 100644 --- a/cmd/non-admin/backup/create.go +++ b/cmd/non-admin/backup/create.go @@ -299,10 +299,10 @@ func (o *CreateOptions) resolveStorageLocation(namespace string) error { return fmt.Errorf("failed to list NonAdminBackupStorageLocations: %w", err) } - return o.resolveStorageLocationFromList(nabslList.Items) + return o.resolveStorageLocationFromList(namespace, nabslList.Items) } -func (o *CreateOptions) resolveStorageLocationFromList(items []nacv1alpha1.NonAdminBackupStorageLocation) error { +func (o *CreateOptions) resolveStorageLocationFromList(namespace string, items []nacv1alpha1.NonAdminBackupStorageLocation) error { // Filter to only approved/created NABSLs (exclude pending/rejected) usable := make([]nacv1alpha1.NonAdminBackupStorageLocation, 0, len(items)) for _, nabsl := range items { @@ -313,7 +313,13 @@ func (o *CreateOptions) resolveStorageLocationFromList(items []nacv1alpha1.NonAd switch len(usable) { case 0: - return nil + if len(items) == 0 { + return fmt.Errorf("no NonAdminBackupStorageLocations found in namespace %q\n"+ + "Create one with `oc oadp nonadmin bsl create` or specify `--storage-location`", namespace) + } + return fmt.Errorf("no usable NonAdminBackupStorageLocation with phase %q found in namespace %q\n"+ + "Check status with `oc oadp nonadmin bsl get` or specify `--storage-location`", + nacv1alpha1.NonAdminPhaseCreated, namespace) case 1: o.StorageLocation = usable[0].Name o.storageLocationAutoSelected = true From aaa30cb48e21f3b389164ad9a2b803ecbfadfb67 Mon Sep 17 00:00:00 2001 From: Nicholas Yancey Date: Fri, 5 Jun 2026 10:07:58 -0400 Subject: [PATCH 7/7] Use slice element pointer in NABSL prompt loop --- cmd/non-admin/backup/create.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/non-admin/backup/create.go b/cmd/non-admin/backup/create.go index 5fe663b2..78dcaa6b 100644 --- a/cmd/non-admin/backup/create.go +++ b/cmd/non-admin/backup/create.go @@ -344,8 +344,9 @@ func promptForNABSLSelection(items []nacv1alpha1.NonAdminBackupStorageLocation, } fmt.Fprintln(out, "Multiple non-admin backup storage locations found. Select one:") - for i, nabsl := range items { - fmt.Fprintf(out, " %d) %s (%s)\n", i+1, nabsl.Name, formatNABSLPhase(&nabsl)) + for i := range items { + nabsl := &items[i] + fmt.Fprintf(out, " %d) %s (%s)\n", i+1, nabsl.Name, formatNABSLPhase(nabsl)) } reader := bufio.NewReader(in)