Command cleanup

Makefile

@@ -96,6 +96,10 @@ build: ## Build the kubectl plugin binary (use PLATFORM=os/arch for cross-compil
 .PHONY: install
 install: build ## Build and install the kubectl plugin to ~/.local/bin (no sudo required)
 	@GOOS=$$(go env GOOS); \
+	if [ "$$GOOS" = "" ]; then \
+		echo "Error: GOOS is not set"; \
+		exit 1; \
+	fi; \
 	if [ "$$GOOS" = "windows" ]; then \
 		binary_name="$(BINARY_NAME).exe"; \
 	else \

cmd/nabsl-request/approve.go

@@ -46,7 +46,6 @@ func NewApproveCommand(f client.Factory) *cobra.Command {
   oc oadp nabsl-request approve nacuser01-user-test-bsl-96dfa8b7-3f6f-4c8d-a168-8527b00fbed8 --reason "Approved for production use"`,
 		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))
 		},
 	}
@@ -85,21 +84,15 @@ func (o *ApproveOptions) Complete(args []string, f client.Factory) error {
 	return nil
 }
 
-func (o *ApproveOptions) Validate(c *cobra.Command, args []string, f client.Factory) error {
-	return nil
-}
-
 func (o *ApproveOptions) Run(c *cobra.Command, f client.Factory) error {
-	// Get the admin namespace (from client config) where requests are stored
+
 	adminNS := f.Namespace()
 
-	// Find the request either by UUID or by looking up NABSL name
 	requestName, err := shared.FindNABSLRequestByNameOrUUID(context.Background(), o.client, o.RequestName, adminNS)
 	if err != nil {
 		return err
 	}
 
-	// Get the current request
 	var request nacv1alpha1.NonAdminBackupStorageLocationRequest
 	err = o.client.Get(context.Background(), kbclient.ObjectKey{
 		Name:      requestName,

cmd/nabsl-request/describe.go

@@ -19,8 +19,8 @@ package nabsl
 import (
 	"context"
 	"fmt"
+	"os"
 	"sort"
-	"strings"
 
 	"github.com/spf13/cobra"
 	kbclient "sigs.k8s.io/controller-runtime/pkg/client"
@@ -54,16 +54,16 @@ func NewDescribeCommand(f client.Factory) *cobra.Command {
 }
 
 type DescribeOptions struct {
-	Name   string
-	client kbclient.WithWatch
+	UUID_Name string
+	client    kbclient.WithWatch
 }
 
 func NewDescribeOptions() *DescribeOptions {
 	return &DescribeOptions{}
 }
 
 func (o *DescribeOptions) Complete(args []string, f client.Factory) error {
-	o.Name = args[0]
+	o.UUID_Name = args[0]
 
 	client, err := shared.NewClientWithScheme(f, shared.ClientOptions{
 		IncludeVeleroTypes:   true,
@@ -85,115 +85,103 @@ func (o *DescribeOptions) Run(c *cobra.Command, f client.Factory) error {
 	// Get the admin namespace (from client config) where requests are stored
 	adminNS := f.Namespace()
 
-	// Get the current namespace to find user's NABSLs
-	currentNS, err := shared.GetCurrentNamespace()
-	if err != nil {
-		return fmt.Errorf("failed to determine current namespace: %w", err)
-	}
-
-	// First get all NABSLs in user's namespace to find related requests
-	var nabslList nacv1alpha1.NonAdminBackupStorageLocationList
-	err = o.client.List(context.Background(), &nabslList, kbclient.InNamespace(currentNS))
+	// Get the request from openshift-adp namespace using the UUID
+	var request nacv1alpha1.NonAdminBackupStorageLocationRequest
+	requestName, err := shared.FindNABSLRequestByNameOrUUID(context.Background(), o.client, o.UUID_Name, adminNS)
 	if err != nil {
-		return fmt.Errorf("failed to list NABSLs: %w", err)
-	}
-
-	// Find the target UUID for the request
-	var targetUUID string
-	for _, nabsl := range nabslList.Items {
-		if nabsl.Status.VeleroBackupStorageLocation != nil && nabsl.Status.VeleroBackupStorageLocation.NACUUID != "" {
-			uuid := nabsl.Status.VeleroBackupStorageLocation.NACUUID
-			// Check if o.Name matches the UUID or NABSL name
-			if uuid == o.Name || nabsl.Name == o.Name {
-				targetUUID = uuid
-				break
-			}
-		}
-	}
-
-	if targetUUID == "" {
-		return fmt.Errorf("request %q not found for NABSLs in namespace %s", o.Name, currentNS)
+		return err
 	}
 
-	// Get the request from openshift-adp namespace using the UUID
-	var request nacv1alpha1.NonAdminBackupStorageLocationRequest
 	err = o.client.Get(context.Background(), kbclient.ObjectKey{
-		Name:      targetUUID,
+		Name:      requestName,
 		Namespace: adminNS,
 	}, &request)
+
 	if err != nil {
-		return fmt.Errorf("failed to get request for %q: %w", o.Name, err)
+		return fmt.Errorf("failed to get request for %q: %w", requestName, err)
 	}
 
 	return describeRequest(&request)
 }
 
 func describeRequest(request *nacv1alpha1.NonAdminBackupStorageLocationRequest) error {
-	fmt.Printf("Name:\t%s\n", request.Name)
-	fmt.Printf("Namespace:\t%s\n", request.Namespace)
+	// Name and Namespace
+	fmt.Printf("Name:         %s\n", request.Name)
+	fmt.Printf("Namespace:    %s\n", request.Namespace)
+
+	// Labels
+	shared.PrintLabelsOrAnnotations(os.Stdout, "Labels:       ", request.Labels)
 
-	fmt.Printf("Labels:\t%s\n", formatLabels(request.Labels))
-	fmt.Printf("Annotations:\t%s\n", formatLabels(request.Annotations))
+	// Annotations
+	shared.PrintLabelsOrAnnotations(os.Stdout, "Annotations:  ", request.Annotations)
 
-	fmt.Printf("Phase:\t%s\n", request.Status.Phase)
+	fmt.Printf("\n")
 
+	// Phase (with color)
+	fmt.Printf("Phase:  %s\n", shared.ColorizePhase(string(request.Status.Phase)))
+
+	fmt.Printf("\n")
+
+	// Approval Decision
 	if request.Spec.ApprovalDecision != "" {
-		fmt.Printf("Approval Decision:\t%s\n", request.Spec.ApprovalDecision)
+		fmt.Printf("Approval Decision:  %s\n", request.Spec.ApprovalDecision)
+		fmt.Printf("\n")
 	}
 
+	// Requested NonAdminBackupStorageLocation
 	if request.Status.SourceNonAdminBSL != nil {
 		source := request.Status.SourceNonAdminBSL
 		fmt.Printf("Requested NonAdminBackupStorageLocation:\n")
-		fmt.Printf("  Name:\t%s\n", source.Name)
-		fmt.Printf("  Namespace:\t%s\n", source.Namespace)
+		fmt.Printf("  Name:       %s\n", source.Name)
+		fmt.Printf("  Namespace:  %s\n", source.Namespace)
 
 		if source.NACUUID != "" {
-			fmt.Printf("  NACUUID:\t%s\n", source.NACUUID)
+			fmt.Printf("  NACUUID:    %s\n", source.NACUUID)
 		}
 
+		fmt.Printf("\n")
+
+		// Requested BackupStorageLocation Spec
 		if source.RequestedSpec != nil {
 			spec := source.RequestedSpec
 			fmt.Printf("Requested BackupStorageLocation Spec:\n")
-			fmt.Printf("  Provider:\t%s\n", spec.Provider)
-			fmt.Printf("  Object Storage Bucket:\t%s\n", spec.ObjectStorage.Bucket)
+			fmt.Printf("  Provider:                  %s\n", spec.Provider)
+			fmt.Printf("  Object Storage Bucket:     %s\n", spec.ObjectStorage.Bucket)
 
 			if spec.ObjectStorage.Prefix != "" {
-				fmt.Printf("  Prefix:\t%s\n", spec.ObjectStorage.Prefix)
+				fmt.Printf("  Prefix:                    %s\n", spec.ObjectStorage.Prefix)
 			}
 
 			if len(spec.Config) > 0 {
-				fmt.Printf("  Config:\t%s\n", formatLabels(spec.Config))
+				fmt.Printf("  Config:\n")
+				configKeys := make([]string, 0, len(spec.Config))
+				for k := range spec.Config {
+					configKeys = append(configKeys, k)
+				}
+				sort.Strings(configKeys)
+				for _, k := range configKeys {
+					fmt.Printf("    %s: %s\n", k, spec.Config[k])
+				}
 			}
 
 			if spec.AccessMode != "" {
-				fmt.Printf("  Access Mode:\t%s\n", spec.AccessMode)
+				fmt.Printf("  Access Mode:               %s\n", spec.AccessMode)
 			}
 
 			if spec.BackupSyncPeriod != nil {
-				fmt.Printf("  Backup Sync Period:\t%s\n", spec.BackupSyncPeriod.String())
+				fmt.Printf("  Backup Sync Period:        %s\n", spec.BackupSyncPeriod.String())
 			}
 
 			if spec.ValidationFrequency != nil {
-				fmt.Printf("  Validation Frequency:\t%s\n", spec.ValidationFrequency.String())
+				fmt.Printf("  Validation Frequency:      %s\n", spec.ValidationFrequency.String())
 			}
+
+			fmt.Printf("\n")
 		}
 	}
 
-	fmt.Printf("Creation Timestamp:\t%s\n", request.CreationTimestamp.String())
+	// Creation Timestamp
+	fmt.Printf("Creation Timestamp:  %s\n", request.CreationTimestamp.Time.Format("2006-01-02 15:04:05 -0700 MST"))
 
 	return nil
 }
-
-// formatLabels formats a map of labels into a string
-func formatLabels(labels map[string]string) string {
-	if len(labels) == 0 {
-		return "<none>"
-	}
-
-	var pairs []string
-	for key, value := range labels {
-		pairs = append(pairs, fmt.Sprintf("%s=%s", key, value))
-	}
-	sort.Strings(pairs)
-	return strings.Join(pairs, ",")
-}

cmd/nabsl-request/get.go

@@ -23,7 +23,6 @@ import (
 	"text/tabwriter"
 
 	"github.com/spf13/cobra"
-	"github.com/spf13/pflag"
 	kbclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/migtools/oadp-cli/cmd/shared"
@@ -43,7 +42,6 @@ func NewGetCommand(f client.Factory) *cobra.Command {
 		Args:  cobra.MaximumNArgs(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: `  # Get all backup storage location requests (admin access required)
@@ -59,28 +57,24 @@ func NewGetCommand(f client.Factory) *cobra.Command {
   oc oadp nabsl-request get my-bsl-request -o yaml`,
 	}
 
-	o.BindFlags(c.Flags())
 	output.BindFlags(c.Flags())
 	output.ClearOutputFlagDefault(c)
 
 	return c
 }
 
 type GetOptions struct {
-	Name          string
-	AllNamespaces bool
-	client        kbclient.WithWatch
+	Name   string
+	client kbclient.WithWatch
 }
 
 func NewGetOptions() *GetOptions {
 	return &GetOptions{}
 }
 
-func (o *GetOptions) BindFlags(flags *pflag.FlagSet) {
-	flags.BoolVar(&o.AllNamespaces, "all-namespaces", false, "If present, list requests across all namespaces")
-}
-
+// Complete NABSL request get options
 func (o *GetOptions) Complete(args []string, f client.Factory) error {
+
 	if len(args) > 0 {
 		o.Name = args[0]
 	}
@@ -89,6 +83,7 @@ func (o *GetOptions) Complete(args []string, f client.Factory) error {
 		IncludeVeleroTypes:   true,
 		IncludeNonAdminTypes: true,
 	})
+
 	if err != nil {
 		return err
 	}
@@ -97,23 +92,23 @@ func (o *GetOptions) Complete(args []string, f client.Factory) error {
 	return nil
 }
 
-func (o *GetOptions) Validate(c *cobra.Command, args []string, f client.Factory) error {
-	return nil
-}
-
 func (o *GetOptions) Run(c *cobra.Command, f client.Factory) error {
 	// Get the admin namespace (from client config) where requests are stored
 	adminNS := f.Namespace()
 
 	if o.Name != "" {
 		// Get specific request by name (UUID)
 		var request nacv1alpha1.NonAdminBackupStorageLocationRequest
-		err := o.client.Get(context.Background(), kbclient.ObjectKey{
-			Name:      o.Name,
+		requestName, err := shared.FindNABSLRequestByNameOrUUID(context.Background(), o.client, o.Name, adminNS)
+		if err != nil {
+			return err
+		}
+		err = o.client.Get(context.Background(), kbclient.ObjectKey{
+			Name:      requestName,
 			Namespace: adminNS,
 		}, &request)
 		if err != nil {
-			return fmt.Errorf("failed to get request %q: %w", o.Name, err)
+			return fmt.Errorf("failed to get request %q: %w", requestName, err)
 		}
 
 		if printed, err := output.PrintWithFormat(c, &request); printed || err != nil {
@@ -128,12 +123,10 @@ func (o *GetOptions) Run(c *cobra.Command, f client.Factory) error {
 
 	// List all requests in admin namespace
 	var requestList nacv1alpha1.NonAdminBackupStorageLocationRequestList
-	var err error
-	if o.AllNamespaces {
-		err = o.client.List(context.Background(), &requestList)
-	} else {
-		err = o.client.List(context.Background(), &requestList, kbclient.InNamespace(adminNS))
-	}
+	var err error = o.client.List(context.Background(), &requestList, &kbclient.ListOptions{
+		Namespace: adminNS,
+	})
+
 	if err != nil {
 		return fmt.Errorf("failed to list requests: %w", err)
 	}

cmd/nabsl-request/reject.go

@@ -46,7 +46,6 @@ func NewRejectCommand(f client.Factory) *cobra.Command {
   oc oadp nabsl-request reject nacuser01-user-test-bsl-96dfa8b7-3f6f-4c8d-a168-8527b00fbed8 --reason "Bucket does not exist in specified region"`,
 		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))
 		},
 	}
@@ -85,10 +84,6 @@ func (o *RejectOptions) Complete(args []string, f client.Factory) error {
 	return nil
 }
 
-func (o *RejectOptions) Validate(c *cobra.Command, args []string, f client.Factory) error {
-	return nil
-}
-
 func (o *RejectOptions) Run(c *cobra.Command, f client.Factory) error {
 	// Get the admin namespace (from client config) where requests are stored
 	adminNS := f.Namespace()