Refactor non-admin backup storage location commands

- Introduced new subcommands for managing backup storage location requests: approve, deny, describe, and get.
- Updated the create command to enhance its functionality and examples.
- Improved help text for commands to provide clearer guidance on usage.
- Added tests to ensure help messages are accurate and comprehensive.
- Enhanced the overall structure and organization of the non-admin command set.

Author: Joeavaikath

cmd/non-admin/bsl/bsl.go

@@ -25,12 +25,13 @@ import (
 func NewBSLCommand(f client.Factory) *cobra.Command {
 	c := &cobra.Command{
 		Use:   "bsl",
-		Short: "Work with non-admin backup storage locations",
-		Long:  "Work with non-admin backup storage locations",
+		Short: "Create and manage backup storage locations",
+		Long:  "Create and manage non-admin backup storage locations and their approval requests",
 	}
 
 	c.AddCommand(
-		NewCreateCommand(f, "create"),
+		NewCreateCommand(f),
+		NewRequestCommand(f),
 	)
 
 	return c

cmd/non-admin/bsl/create.go

@@ -30,32 +30,44 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/client"
 	"github.com/vmware-tanzu/velero/pkg/cmd"
 	"github.com/vmware-tanzu/velero/pkg/cmd/util/output"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func NewCreateCommand(f client.Factory, use string) *cobra.Command {
+func NewCreateCommand(f client.Factory) *cobra.Command {
 	o := NewCreateOptions()
 
 	c := &cobra.Command{
-		Use:   use + " NAME",
+		Use:   "create NAME",
 		Short: "Create a non-admin backup storage location",
 		Args:  cobra.ExactArgs(1),
 		Run: func(c *cobra.Command, args []string) {
 			cmd.CheckError(o.Complete(args, f))
 			cmd.CheckError(o.Validate(c, args, f))
 			cmd.CheckError(o.Run(c, f))
 		},
-		Example: `  # Create a non-admin backup storage location
-  kubectl oadp nonadmin bsl create my-bsl --backup-storage-location default
-
-  # Create a non-admin backup storage location with specific namespace
-  kubectl oadp nonadmin bsl create my-bsl --backup-storage-location aws-bsl --namespace my-namespace
-
-  # Create with custom BSL namespace (if OADP operator is not in openshift-adp)
-  kubectl oadp nonadmin bsl create my-bsl --backup-storage-location default --bsl-namespace velero
-
-  # View the YAML for a non-admin backup storage location without sending it to the server
-  kubectl oadp nonadmin bsl create my-bsl --backup-storage-location default -o yaml`,
+		Example: `  # Create a non-admin backup storage location for AWS
+  kubectl oadp nonadmin bsl create my-storage \
+    --provider aws \
+    --bucket my-velero-bucket \
+    --credential-name cloud-credentials \
+    --region us-east-1
+
+  # Create with prefix for organizing backups
+  kubectl oadp nonadmin bsl create my-storage \
+    --provider aws \
+    --bucket my-velero-bucket \
+    --prefix velero-backups \
+    --credential-name cloud-credentials \
+    --region us-east-1
+
+  # View the YAML without creating the resource
+  kubectl oadp nonadmin bsl create my-storage \
+    --provider aws \
+    --bucket my-bucket \
+    --credential-name cloud-credentials \
+    --region us-east-1 \
+    -o yaml`,
 	}
 
 	o.BindFlags(c.Flags())
@@ -66,89 +78,115 @@ func NewCreateCommand(f client.Factory, use string) *cobra.Command {
 }
 
 type CreateOptions struct {
-	Name                  string
-	BackupStorageLocation string
-	NonAdminNamespace     string
-	BSLNamespace          string
-	client                kbclient.WithWatch
+	Name           string
+	Namespace      string
+	Provider       string
+	Bucket         string
+	Prefix         string
+	CredentialName string
+	Region         string
+	Config         map[string]string
+	client         kbclient.WithWatch
 }
 
 func NewCreateOptions() *CreateOptions {
 	return &CreateOptions{
-		BSLNamespace: "openshift-adp", // Default OADP operator namespace
+		Config: make(map[string]string),
 	}
 }
 
 func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) {
-	flags.StringVar(&o.BackupStorageLocation, "backup-storage-location", "", "Name of the BackupStorageLocation to reference.")
-	flags.StringVar(&o.NonAdminNamespace, "namespace", "", "Namespace for the NonAdminBackupStorageLocation (defaults to current context namespace).")
-	flags.StringVar(&o.BSLNamespace, "bsl-namespace", "openshift-adp", "Namespace where the BackupStorageLocation exists.")
+	flags.StringVar(&o.Provider, "provider", "", "Storage provider (required). Examples: aws, azure, gcp")
+	flags.StringVar(&o.Bucket, "bucket", "", "Object storage bucket name (required)")
+	flags.StringVar(&o.Prefix, "prefix", "", "Prefix for backup objects in the bucket")
+	flags.StringVar(&o.CredentialName, "credential-name", "", "Name of the credential secret (required)")
+	flags.StringVar(&o.Region, "region", "", "Storage region (required for some providers like AWS)")
+	flags.StringToStringVar(&o.Config, "config", nil, "Additional provider-specific configuration (key=value pairs)")
 }
 
 func (o *CreateOptions) Complete(args []string, f client.Factory) error {
 	o.Name = args[0]
 
-	// Create client with Velero scheme for BackupStorageLocation access
-	client, err := shared.NewClientWithScheme(f, shared.ClientOptions{
-		IncludeVeleroTypes: true,
-	})
+	// Create client with full scheme including NonAdmin and Velero types
+	client, err := shared.NewClientWithFullScheme(f)
 	if err != nil {
 		return err
 	}
 
 	o.client = client
 
-	if o.NonAdminNamespace == "" {
-		namespace := f.Namespace()
-		o.NonAdminNamespace = namespace
+	// Get the current namespace
+	currentNS, err := shared.GetCurrentNamespace()
+	if err != nil {
+		return fmt.Errorf("failed to determine current namespace: %w", err)
 	}
+	o.Namespace = currentNS
 
 	return nil
 }
 
 func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Factory) error {
-	if o.BackupStorageLocation == "" {
-		return fmt.Errorf("--backup-storage-location is required")
+	if o.Provider == "" {
+		return fmt.Errorf("--provider is required")
+	}
+	if o.Bucket == "" {
+		return fmt.Errorf("--bucket is required")
+	}
+	if o.CredentialName == "" {
+		return fmt.Errorf("--credential-name is required")
 	}
 
 	return nil
 }
 
 func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error {
-	// If we have a BackupStorageLocation name, we need to fetch its spec
-	var bslSpec *velerov1.BackupStorageLocationSpec
-	if o.BackupStorageLocation != "" {
-		// Get the existing BackupStorageLocation to copy its spec
-		existingBSL := &velerov1.BackupStorageLocation{}
-		err := o.client.Get(context.Background(), kbclient.ObjectKey{
-			Name:      o.BackupStorageLocation,
-			Namespace: o.BSLNamespace, // Use the BSLNamespace flag
-		}, existingBSL)
-		if err != nil {
-			return fmt.Errorf("failed to get BackupStorageLocation %q: %w", o.BackupStorageLocation, err)
-		}
-		bslSpec = &existingBSL.Spec
+	// Build config map
+	config := make(map[string]string)
+	if o.Region != "" {
+		config["region"] = o.Region
+	}
+	// Add any additional config provided via --config flag
+	for k, v := range o.Config {
+		config[k] = v
 	}
 
-	bsl := &nacv1alpha1.NonAdminBackupStorageLocation{
+	// Create the NABSL
+	nabsl := &nacv1alpha1.NonAdminBackupStorageLocation{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      o.Name,
-			Namespace: o.NonAdminNamespace,
+			Namespace: o.Namespace,
 		},
 		Spec: nacv1alpha1.NonAdminBackupStorageLocationSpec{
-			BackupStorageLocationSpec: bslSpec,
+			BackupStorageLocationSpec: &velerov1.BackupStorageLocationSpec{
+				Provider: o.Provider,
+				Config:   config,
+				Credential: &corev1.SecretKeySelector{
+					LocalObjectReference: corev1.LocalObjectReference{
+						Name: o.CredentialName,
+					},
+					Key: "cloud", // Standard key name for cloud credentials
+				},
+				StorageType: velerov1.StorageType{
+					ObjectStorage: &velerov1.ObjectStorageLocation{
+						Bucket: o.Bucket,
+						Prefix: o.Prefix,
+					},
+				},
+			},
 		},
 	}
 
-	if printed, err := output.PrintWithFormat(c, bsl); printed || err != nil {
+	if printed, err := output.PrintWithFormat(c, nabsl); printed || err != nil {
 		return err
 	}
 
-	err := o.client.Create(context.Background(), bsl)
+	err := o.client.Create(context.Background(), nabsl)
 	if err != nil {
 		return err
 	}
 
-	fmt.Printf("NonAdminBackupStorageLocation %q created successfully.\n", bsl.Name)
+	fmt.Printf("NonAdminBackupStorageLocation %q created successfully.\n", nabsl.Name)
+	fmt.Printf("The controller will create a request for admin approval.\n")
+	fmt.Printf("Use 'kubectl oadp nonadmin bsl request get' to view auto-created requests.\n")
 	return nil
 }

cmd/non-admin/bsl/request.go

@@ -0,0 +1,40 @@
+/*
+Copyright 2025 The OADP CLI 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 bsl
+
+import (
+	"github.com/spf13/cobra"
+	"github.com/vmware-tanzu/velero/pkg/client"
+)
+
+// NewRequestCommand creates the "request" subcommand under bsl
+func NewRequestCommand(f client.Factory) *cobra.Command {
+	c := &cobra.Command{
+		Use:   "request",
+		Short: "Manage backup storage location approval requests",
+		Long:  "View and manage approval requests for backup storage locations. Requests are automatically created when users create backup storage locations and require admin approval.",
+	}
+
+	c.AddCommand(
+		NewRequestGetCommand(f),
+		NewRequestDescribeCommand(f),
+		NewRequestApproveCommand(f),
+		NewRequestDenyCommand(f),
+	)
+
+	return c
+}