Skip to content

Commit c955f49

Browse files
committed
Fix verb commands
Signed-off-by: Joseph <jvaikath@redhat.com>
1 parent 913bd65 commit c955f49

4 files changed

Lines changed: 161 additions & 4 deletions

File tree

cmd/non-admin/bsl/bsl.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func NewBSLCommand(f client.Factory) *cobra.Command {
3030
}
3131

3232
c.AddCommand(
33-
NewCreateCommand(f),
33+
NewCreateCommand(f, "create"),
3434
NewGetCommand(f, "get"),
3535
)
3636

cmd/non-admin/bsl/create.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ import (
3636
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3737
)
3838

39-
func NewCreateCommand(f client.Factory) *cobra.Command {
39+
func NewCreateCommand(f client.Factory, use string) *cobra.Command {
4040
o := NewCreateOptions()
4141

4242
c := &cobra.Command{
43-
Use: "create NAME",
43+
Use: use + " NAME",
4444
Short: "Create a non-admin backup storage location",
4545
Args: cobra.ExactArgs(1),
4646
Run: func(c *cobra.Command, args []string) {

cmd/non-admin/verbs/builder.go

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,36 @@ func (vb *NonAdminVerbBuilder) runEFunc(verb string) func(cmd *cobra.Command, ar
108108
// This ensures the RunE closure uses the correct flag variables
109109
subCmd.SetArgs(remainingArgs)
110110

111-
return subCmd.Execute()
111+
// Parse flags from the args we set
112+
// IMPORTANT: We cannot use subCmd.Execute() here because it causes Cobra to
113+
// re-parse os.Args from the root command, which results in incorrect command
114+
// path parsing (e.g., "unknown command 'na' for 'backup'" error).
115+
// Instead, we manually parse flags and execute the RunE function directly.
116+
if err := subCmd.ParseFlags(remainingArgs); err != nil {
117+
return err
118+
}
119+
120+
// Validate arguments
121+
if subCmd.Args != nil {
122+
// Get the args after flag parsing
123+
flags := subCmd.Flags()
124+
argsList := flags.Args()
125+
if err := subCmd.Args(subCmd, argsList); err != nil {
126+
return err
127+
}
128+
}
129+
130+
// Execute the RunE function directly with parsed args
131+
if subCmd.RunE != nil {
132+
flags := subCmd.Flags()
133+
return subCmd.RunE(subCmd, flags.Args())
134+
} else if subCmd.Run != nil {
135+
flags := subCmd.Flags()
136+
subCmd.Run(subCmd, flags.Args())
137+
return nil
138+
}
139+
140+
return fmt.Errorf("command %s does not have Run or RunE", subCmd.Name())
112141
}
113142
}
114143

cmd/non-admin/verbs/builder_test.go

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,3 +408,131 @@ func TestNonAdminVerbBuilder_NilHandler(t *testing.T) {
408408
t.Logf("Got error (as expected): %v", err)
409409
}
410410
}
411+
412+
// TestNonAdminVerbBuilder_VerbNounExecution tests that verb-noun command execution works correctly
413+
// This test specifically verifies that the command delegation doesn't cause os.Args re-parsing issues
414+
func TestNonAdminVerbBuilder_VerbNounExecution(t *testing.T) {
415+
// Track that the delegated command was actually executed
416+
executionCalled := false
417+
var receivedArgs []string
418+
419+
// Create a mock subcommand that tracks execution
420+
mockSubCmd := &cobra.Command{
421+
Use: "get",
422+
Short: "Get a backup",
423+
RunE: func(cmd *cobra.Command, args []string) error {
424+
executionCalled = true
425+
receivedArgs = args
426+
return nil
427+
},
428+
}
429+
430+
mockResourceCmd := &cobra.Command{
431+
Use: "backup",
432+
Short: "Work with backups",
433+
}
434+
mockResourceCmd.AddCommand(mockSubCmd)
435+
436+
handler := NonAdminResourceHandler{
437+
GetCommandFunc: func(f client.Factory) *cobra.Command {
438+
return mockResourceCmd
439+
},
440+
GetSubCommandFunc: func(cmd *cobra.Command) *cobra.Command {
441+
return mockSubCmd
442+
},
443+
}
444+
445+
// Build the verb command
446+
builder := NewNonAdminVerbBuilder(nil)
447+
builder.RegisterResource("backup", handler)
448+
449+
verbCmd := builder.BuildVerbCommand(NonAdminVerbConfig{
450+
Use: "get",
451+
Short: "Get resources",
452+
})
453+
454+
// Execute with verb-noun pattern: "get backup my-backup"
455+
verbCmd.SetArgs([]string{"backup", "my-backup"})
456+
457+
err := verbCmd.Execute()
458+
if err != nil {
459+
// Check that the error is NOT the "unknown command" error that indicates os.Args re-parsing
460+
if strings.Contains(err.Error(), "unknown command") {
461+
t.Fatalf("Got 'unknown command' error, indicating os.Args re-parsing bug: %v", err)
462+
}
463+
// Other errors are okay (e.g., no cluster connection)
464+
t.Logf("Got expected error (no cluster): %v", err)
465+
}
466+
467+
// Verify the delegated command was actually called
468+
if !executionCalled {
469+
t.Error("Expected delegated command to be executed, but it wasn't called")
470+
}
471+
472+
// Verify the arguments were passed correctly
473+
expectedArgs := []string{"my-backup"}
474+
if len(receivedArgs) != len(expectedArgs) {
475+
t.Errorf("Expected %d args, got %d: %v", len(expectedArgs), len(receivedArgs), receivedArgs)
476+
} else if len(receivedArgs) > 0 && receivedArgs[0] != expectedArgs[0] {
477+
t.Errorf("Expected args %v, got %v", expectedArgs, receivedArgs)
478+
}
479+
480+
t.Log("Verb-noun execution test passed successfully")
481+
}
482+
483+
// TestNonAdminVerbBuilder_VerbNounWithFlags tests verb-noun pattern with flags
484+
func TestNonAdminVerbBuilder_VerbNounWithFlags(t *testing.T) {
485+
executionCalled := false
486+
var receivedFlagValue string
487+
488+
mockSubCmd := &cobra.Command{
489+
Use: "get",
490+
Short: "Get a backup",
491+
RunE: func(cmd *cobra.Command, args []string) error {
492+
executionCalled = true
493+
flag := cmd.Flags().Lookup("output")
494+
if flag != nil {
495+
receivedFlagValue = flag.Value.String()
496+
}
497+
return nil
498+
},
499+
}
500+
mockSubCmd.Flags().String("output", "", "output format")
501+
502+
mockResourceCmd := &cobra.Command{Use: "backup"}
503+
mockResourceCmd.AddCommand(mockSubCmd)
504+
505+
handler := NonAdminResourceHandler{
506+
GetCommandFunc: func(f client.Factory) *cobra.Command { return mockResourceCmd },
507+
GetSubCommandFunc: func(cmd *cobra.Command) *cobra.Command { return mockSubCmd },
508+
}
509+
510+
builder := NewNonAdminVerbBuilder(nil)
511+
builder.RegisterResource("backup", handler)
512+
513+
verbCmd := builder.BuildVerbCommand(NonAdminVerbConfig{
514+
Use: "get",
515+
Short: "Get resources",
516+
})
517+
518+
// Execute with verb-noun pattern and flags: "get backup my-backup --output=json"
519+
verbCmd.SetArgs([]string{"backup", "my-backup", "--output=json"})
520+
521+
err := verbCmd.Execute()
522+
if err != nil {
523+
if strings.Contains(err.Error(), "unknown command") {
524+
t.Fatalf("Got 'unknown command' error, indicating os.Args re-parsing bug: %v", err)
525+
}
526+
t.Logf("Got expected error (no cluster): %v", err)
527+
}
528+
529+
if !executionCalled {
530+
t.Error("Expected delegated command to be executed")
531+
}
532+
533+
if receivedFlagValue != "json" {
534+
t.Errorf("Expected flag value 'json', got '%s'", receivedFlagValue)
535+
}
536+
537+
t.Log("Verb-noun with flags test passed successfully")
538+
}

0 commit comments

Comments
 (0)