Fix nonadmin backup describe --details

Signed-off-by: Joseph <jvaikath@redhat.com>

Author: Joeavaikath

cmd/non-admin/backup/describe.go

@@ -72,7 +72,7 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
 			}
 
 			// Print in Velero-style format
-			printNonAdminBackupDetails(cmd, &nab, kbClient, backupName, userNamespace, effectiveTimeout)
+			printNonAdminBackupDetails(cmd, &nab, kbClient, backupName, userNamespace, effectiveTimeout, details)
 
 			// Add detailed output if --details flag is set
 			if details {
@@ -98,7 +98,7 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
 }
 
 // printNonAdminBackupDetails prints backup details in Velero admin describe format
-func printNonAdminBackupDetails(cmd *cobra.Command, nab *nacv1alpha1.NonAdminBackup, kbClient kbclient.Client, backupName string, userNamespace string, timeout time.Duration) {
+func printNonAdminBackupDetails(cmd *cobra.Command, nab *nacv1alpha1.NonAdminBackup, kbClient kbclient.Client, backupName string, userNamespace string, timeout time.Duration, showDetails bool) {
 	out := cmd.OutOrStdout()
 
 	// Get Velero backup reference if available
@@ -322,22 +322,24 @@ func printNonAdminBackupDetails(cmd *cobra.Command, nab *nacv1alpha1.NonAdminBac
 
 		fmt.Fprintf(out, "\n")
 
-		// Fetch and display Resource List
-		ctx, cancel := context.WithTimeout(context.Background(), timeout)
-		defer cancel()
+		// Fetch and display Resource List (only if showDetails is true)
+		if showDetails {
+			ctx, cancel := context.WithTimeout(context.Background(), timeout)
+			defer cancel()
 
-		resourceList, err := shared.ProcessDownloadRequest(ctx, kbClient, shared.DownloadRequestOptions{
-			BackupName:  backupName,
-			DataType:    "BackupResourceList",
-			Namespace:   userNamespace,
-			HTTPTimeout: timeout,
-		})
+			resourceList, err := shared.ProcessDownloadRequest(ctx, kbClient, shared.DownloadRequestOptions{
+				BackupName:  backupName,
+				DataType:    "BackupResourceList",
+				Namespace:   userNamespace,
+				HTTPTimeout: timeout,
+			})
 
-		if err == nil && resourceList != "" {
-			if formattedList := formatResourceList(resourceList); formattedList != "" {
-				fmt.Fprintf(out, "Resource List:\n")
-				fmt.Fprintf(out, "%s\n", formattedList)
-				fmt.Fprintf(out, "\n")
+			if err == nil && resourceList != "" {
+				if formattedList := formatResourceList(resourceList); formattedList != "" {
+					fmt.Fprintf(out, "Resource List:\n")
+					fmt.Fprintf(out, "%s\n", formattedList)
+					fmt.Fprintf(out, "\n")
+				}
 			}
 		}
 

cmd/non-admin/verbs/builder.go

@@ -88,12 +88,14 @@ func (vb *NonAdminVerbBuilder) runEFunc(verb string) func(cmd *cobra.Command, ar
 		}
 
 		// Get the main command for the resource (e.g., "backup" command)
+		// This creates a FRESH command instance each time, which is important
+		// to avoid flag state persistence across invocations
 		resourceCmd := handler.GetCommandFunc(vb.factory)
 		if resourceCmd == nil {
 			return fmt.Errorf("%s command not found for resource type %s", verb, resourceType)
 		}
 
-		// Get the specific subcommand for the verb (e.g., "backup get" command)
+		// Get the specific subcommand for the verb (e.g., "backup describe" command)
 		subCmd := handler.GetSubCommandFunc(resourceCmd)
 		if subCmd == nil {
 			return fmt.Errorf("%s %s command not found", resourceType, verb)
@@ -102,11 +104,11 @@ func (vb *NonAdminVerbBuilder) runEFunc(verb string) func(cmd *cobra.Command, ar
 		// Add flags to remaining args so they get passed to the delegated command
 		remainingArgs = vb.addFlagsToArgs(cmd, remainingArgs)
 
-		// Create a new command instance to avoid argument inheritance
-		newSubCmd := vb.createCommandInstance(subCmd)
-		newSubCmd.SetArgs(remainingArgs)
+		// Use the original subcommand directly (not a copy) and set its args
+		// This ensures the RunE closure uses the correct flag variables
+		subCmd.SetArgs(remainingArgs)
 
-		return newSubCmd.Execute()
+		return subCmd.Execute()
 	}
 }
 
@@ -138,28 +140,10 @@ func (vb *NonAdminVerbBuilder) addFlagsToArgs(cmd *cobra.Command, remainingArgs
 	return remainingArgs
 }
 
-// createCommandInstance creates a new cobra.Command instance from an existing one to avoid argument/flag inheritance issues.
-func (vb *NonAdminVerbBuilder) createCommandInstance(originalCmd *cobra.Command) *cobra.Command {
-	newCmd := &cobra.Command{
-		Use:   originalCmd.Use,
-		Short: originalCmd.Short,
-		Long:  originalCmd.Long,
-		Run:   originalCmd.Run,
-		RunE:  originalCmd.RunE,
-	}
-
-	// Copy flags from the original command
-	originalCmd.Flags().VisitAll(func(flag *pflag.Flag) {
-		newCmd.Flags().AddFlag(flag)
-	})
-	// Also copy persistent flags
-	originalCmd.PersistentFlags().VisitAll(func(flag *pflag.Flag) {
-		newCmd.PersistentFlags().AddFlag(flag)
-	})
-	return newCmd
-}
-
 // addFlagsFromResources adds flags from all registered resources to the verb command
+// Note: We add flag references (not copies) here for recognition at the verb level.
+// The actual command execution will use fresh command instances created by GetCommandFunc,
+// so flag state won't persist across invocations.
 func (vb *NonAdminVerbBuilder) addFlagsFromResources(verbCmd *cobra.Command, verb string) {
 	addedFlags := make(map[string]bool)
 

cmd/non-admin/verbs/builder_test.go

@@ -386,40 +386,6 @@ func TestNonAdminVerbBuilder_MultipleResourceTypes(t *testing.T) {
 	}
 }
 
-// TestNonAdminVerbBuilder_CreateCommandInstance tests command instance creation
-func TestNonAdminVerbBuilder_CreateCommandInstance(t *testing.T) {
-	builder := NewNonAdminVerbBuilder(nil)
-
-	originalCmd := &cobra.Command{
-		Use:   "create NAME",
-		Short: "Create a resource",
-		Long:  "Create a non-admin resource",
-	}
-	originalCmd.Flags().String("test-flag", "default", "test flag")
-	originalCmd.PersistentFlags().String("persistent-flag", "default", "persistent flag")
-
-	newCmd := builder.createCommandInstance(originalCmd)
-
-	// Verify basic fields are copied
-	if newCmd.Use != originalCmd.Use {
-		t.Errorf("Expected Use to be %s, got %s", originalCmd.Use, newCmd.Use)
-	}
-	if newCmd.Short != originalCmd.Short {
-		t.Errorf("Expected Short to be %s, got %s", originalCmd.Short, newCmd.Short)
-	}
-	if newCmd.Long != originalCmd.Long {
-		t.Errorf("Expected Long to be %s, got %s", originalCmd.Long, newCmd.Long)
-	}
-
-	// Verify flags are copied
-	if newCmd.Flags().Lookup("test-flag") == nil {
-		t.Error("Expected test-flag to be copied to new command")
-	}
-	if newCmd.PersistentFlags().Lookup("persistent-flag") == nil {
-		t.Error("Expected persistent-flag to be copied to new command")
-	}
-}
-
 // TestNonAdminVerbBuilder_NilHandler tests handling of nil handlers
 func TestNonAdminVerbBuilder_NilHandler(t *testing.T) {
 	builder := NewNonAdminVerbBuilder(nil)