From b4d11baa02b664f8fd1f12fc08487cf7b52554c5 Mon Sep 17 00:00:00 2001 From: Oleg Isakov Date: Tue, 11 Mar 2025 20:11:47 +0200 Subject: [PATCH 1/8] fix display nested fields in list mode; silence usage; fixes --- cmd/root.go | 1 + internal/output/entities/handlers.go | 2 +- internal/output/utils.go | 6 +++--- internal/output/utils/main.go | 2 +- main.go | 5 +---- 5 files changed, 7 insertions(+), 9 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 7ca5eaf..b70d390 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -22,6 +22,7 @@ func NewRootCmd(version string) *cobra.Command { Long: `A command line interface for managing servers.com resources`, Version: version, PersistentPreRunE: base.InitCmdContext(cmdContext), + SilenceUsage: true, } // Global flags diff --git a/internal/output/entities/handlers.go b/internal/output/entities/handlers.go index ca7f096..789a373 100644 --- a/internal/output/entities/handlers.go +++ b/internal/output/entities/handlers.go @@ -84,7 +84,7 @@ func mapHandler(w io.Writer, v interface{}, indent string, _ *Field) error { return err } -func structPVHandler(w io.Writer, v interface{}, indent string, f *Field) error { +func structPVHandler(w io.Writer, v any, indent string, f *Field) error { if v == nil { fmt.Fprintf(w, "\t") return nil diff --git a/internal/output/utils.go b/internal/output/utils.go index dbd81bd..960b20d 100644 --- a/internal/output/utils.go +++ b/internal/output/utils.go @@ -118,7 +118,7 @@ func processValue(value reflect.Value, processor func(any) error) error { switch value.Kind() { case reflect.Slice: - for i := 0; i < value.Len(); i++ { + for i := range value.Len() { if err := processor(value.Index(i).Interface()); err != nil { return err } @@ -134,7 +134,7 @@ func (f *Formatter) formatRow(w io.Writer, item any, fields []entities.Field) er values := make([]string, 0, len(fields)) for _, field := range fields { - fieldValue, err := utils.GetFieldValue(item, field.Path) + fieldValue, err := utils.GetFieldValue(item, field.GetPath()) if err != nil { return err } @@ -169,7 +169,7 @@ func (f *Formatter) formatPageView(v any, entity entities.EntityInterface) error switch value.Kind() { case reflect.Slice: - for i := 0; i < value.Len(); i++ { + for i := range value.Len() { if err := f.formatPageViewItem(w, value.Index(i).Interface(), orderedFields); err != nil { return err } diff --git a/internal/output/utils/main.go b/internal/output/utils/main.go index 04bc271..5d92fc5 100644 --- a/internal/output/utils/main.go +++ b/internal/output/utils/main.go @@ -17,7 +17,7 @@ func Humanize(input string) string { } // GetFieldValue returns the value of a given field for an item. -func GetFieldValue(item interface{}, jsonPath string) (interface{}, error) { +func GetFieldValue(item any, jsonPath string) (any, error) { if jsonPath == "" || jsonPath == "." { return item, nil } diff --git a/main.go b/main.go index 2f04b54..fe67a2f 100644 --- a/main.go +++ b/main.go @@ -2,7 +2,6 @@ package main import ( "fmt" - "log" "github.com/serverscom/srvctl/cmd" ) @@ -18,7 +17,5 @@ func main() { } rootCmd := cmd.NewRootCmd(version) - if err := rootCmd.Execute(); err != nil { - log.Fatal(err) - } + _ = rootCmd.Execute() } From df0af5a72eaa414f66676f2afaca95c002e68880 Mon Sep 17 00:00:00 2001 From: Oleg Isakov Date: Wed, 12 Mar 2025 10:46:43 +0200 Subject: [PATCH 2/8] validate output flag --- cmd/base/hooks.go | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/cmd/base/hooks.go b/cmd/base/hooks.go index a8fcea7..5341d4c 100644 --- a/cmd/base/hooks.go +++ b/cmd/base/hooks.go @@ -62,9 +62,27 @@ func CheckFormatterFlags(cmdContext *CmdContext, entities map[string]entities.En manager := cmdContext.GetManager() formatter := cmdContext.GetOrCreateFormatter(cmd) + fieldList, err := manager.GetResolvedBoolValue(cmd, "field-list") + if err != nil { + return err + } + + entity := findEntity(cmd, entities) + if entity == nil { + return fmt.Errorf("can't find entity") + } + if fieldList { + formatter.ListEntityFields(entity.GetFields()) + os.Exit(0) + } + output := formatter.GetOutput() - if output == "json" || output == "yaml" { + switch output { + case "json", "yaml": return nil + case "text": + default: + return fmt.Errorf("invalid output %q, allowed values: json, text, yaml", output) } tmpl := formatter.GetTemplateStr() @@ -81,20 +99,6 @@ func CheckFormatterFlags(cmdContext *CmdContext, entities map[string]entities.En return nil } - fieldList, err := manager.GetResolvedBoolValue(cmd, "field-list") - if err != nil { - return err - } - - entity := findEntity(cmd, entities) - if entity == nil { - return fmt.Errorf("can't find entity") - } - if fieldList { - formatter.ListEntityFields(entity.GetFields()) - os.Exit(0) - } - fields, err := manager.GetResolvedStringSliceValue(cmd, "field") if err != nil { return err From d2915acfe413ff4d4fae2fe986682a6617af2db1 Mon Sep 17 00:00:00 2001 From: Oleg Isakov Date: Wed, 12 Mar 2025 10:50:29 +0200 Subject: [PATCH 3/8] handle invalid subcommand --- cmd/base/utils.go | 10 ++++++++++ cmd/config/config.go | 10 ++++++++-- cmd/context/context.go | 2 ++ cmd/entities/hosts/hosts.go | 5 ++--- cmd/entities/ssh-keys/ssh_keys.go | 5 ++--- 5 files changed, 24 insertions(+), 8 deletions(-) diff --git a/cmd/base/utils.go b/cmd/base/utils.go index 6f81bf4..ea1c45d 100644 --- a/cmd/base/utils.go +++ b/cmd/base/utils.go @@ -145,3 +145,13 @@ func findEntity(cmd *cobra.Command, entities map[string]entities.EntityInterface } return nil } + +func UsageRun(cmd *cobra.Command, args []string) { cmd.Usage() } + +func NoArgs(cmd *cobra.Command, args []string) error { + if len(args) > 0 { + helpStr := fmt.Sprintf("Run '%v --help' for usage.", cmd.CommandPath()) + return fmt.Errorf("unknown command %q for %q\n%s", args[0], cmd.CommandPath(), helpStr) + } + return nil +} diff --git a/cmd/config/config.go b/cmd/config/config.go index b94adee..eae0cff 100644 --- a/cmd/config/config.go +++ b/cmd/config/config.go @@ -15,13 +15,19 @@ func NewCmd(cmdContext *base.CmdContext) *cobra.Command { Short: "Manage configuration", Long: `Manage global and context-specific configurations`, PersistentPreRunE: base.CheckEmptyContexts(cmdContext), + Args: base.NoArgs, + Run: base.UsageRun, } globalCmd := &cobra.Command{ - Use: "global", + Use: "global", + Args: base.NoArgs, + Run: base.UsageRun, } contextCmd := &cobra.Command{ - Use: "context", + Use: "context", + Args: base.NoArgs, + Run: base.UsageRun, } globalCmd.AddCommand(newUpdateCmd(cmdContext)) diff --git a/cmd/context/context.go b/cmd/context/context.go index 8825a2c..01952ec 100644 --- a/cmd/context/context.go +++ b/cmd/context/context.go @@ -10,6 +10,8 @@ func NewCmd(cmdContext *base.CmdContext) *cobra.Command { Use: "context", Short: "Manage contexts", Long: `Manage authentication contexts for different API accounts`, + Args: base.NoArgs, + Run: base.UsageRun, } cmd.AddCommand( diff --git a/cmd/entities/hosts/hosts.go b/cmd/entities/hosts/hosts.go index 7f6bf62..0d7ebbf 100644 --- a/cmd/entities/hosts/hosts.go +++ b/cmd/entities/hosts/hosts.go @@ -91,9 +91,8 @@ func NewCmd(cmdContext *base.CmdContext) *cobra.Command { base.CheckFormatterFlags(cmdContext, entitiesMap), base.CheckEmptyContexts(cmdContext), ), - RunE: func(cmd *cobra.Command, args []string) error { - return cmd.Help() - }, + Args: base.NoArgs, + Run: base.UsageRun, } // hosts list cmd diff --git a/cmd/entities/ssh-keys/ssh_keys.go b/cmd/entities/ssh-keys/ssh_keys.go index ce52126..d36a416 100644 --- a/cmd/entities/ssh-keys/ssh_keys.go +++ b/cmd/entities/ssh-keys/ssh_keys.go @@ -23,9 +23,8 @@ func NewCmd(cmdContext *base.CmdContext) *cobra.Command { base.CheckFormatterFlags(cmdContext, entitiesMap), base.CheckEmptyContexts(cmdContext), ), - RunE: func(cmd *cobra.Command, args []string) error { - return cmd.Help() - }, + Args: base.NoArgs, + Run: base.UsageRun, } cmd.AddCommand( From fb8684ad65cfaf86bc84c68bf7d76c59b5eb9f34 Mon Sep 17 00:00:00 2001 From: Oleg Isakov Date: Wed, 12 Mar 2025 10:50:54 +0200 Subject: [PATCH 4/8] check TTY when login --- cmd/login/login.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/login/login.go b/cmd/login/login.go index 1505c55..3e711f6 100644 --- a/cmd/login/login.go +++ b/cmd/login/login.go @@ -2,6 +2,7 @@ package login import ( "bufio" + "errors" "fmt" "io" "log" @@ -33,6 +34,9 @@ func NewCmd(cmdContext *base.CmdContext, clientFactory client.ClientFactory) *co Example: srvctl login context-name`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + if !term.IsTerminal(int(os.Stdout.Fd())) { + return errors.New("TTY required to enter the password.") + } manager := cmdContext.GetManager() var contextName string @@ -41,7 +45,7 @@ Example: srvctl login context-name`, } else { contextName = manager.GetDefaultContextName() if contextName == "" { - return fmt.Errorf("no contexts found") + return errors.New("no contexts found") } } From fa052f2e7d618b3c213653de58e061a9a2708ba0 Mon Sep 17 00:00:00 2001 From: Oleg Isakov Date: Wed, 12 Mar 2025 11:02:52 +0200 Subject: [PATCH 5/8] fix lint --- cmd/base/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/base/utils.go b/cmd/base/utils.go index ea1c45d..bf1be6e 100644 --- a/cmd/base/utils.go +++ b/cmd/base/utils.go @@ -146,7 +146,7 @@ func findEntity(cmd *cobra.Command, entities map[string]entities.EntityInterface return nil } -func UsageRun(cmd *cobra.Command, args []string) { cmd.Usage() } +func UsageRun(cmd *cobra.Command, args []string) { _ = cmd.Usage() } func NoArgs(cmd *cobra.Command, args []string) error { if len(args) > 0 { From 864921539d7d26d77f7ea1426b0162f7aa0dd014 Mon Sep 17 00:00:00 2001 From: Oleg Isakov Date: Wed, 12 Mar 2025 12:03:27 +0200 Subject: [PATCH 6/8] fix login tests; add login with no tty test case --- cmd/login/login_test.go | 24 ++++++++++++++++++++++++ go.mod | 1 + go.sum | 2 ++ 3 files changed, 27 insertions(+) diff --git a/cmd/login/login_test.go b/cmd/login/login_test.go index 5c90440..610242d 100644 --- a/cmd/login/login_test.go +++ b/cmd/login/login_test.go @@ -2,10 +2,12 @@ package login import ( "io" + "os" "path/filepath" "strings" "testing" + "github.com/creack/pty" . "github.com/onsi/gomega" serverscom "github.com/serverscom/serverscom-go-client/pkg" "github.com/serverscom/srvctl/cmd/testutils" @@ -26,6 +28,7 @@ func TestLoginCmd(t *testing.T) { configureMock func(*mocks.MockCollection[serverscom.Host]) expectedOutput []byte expectError bool + tty bool }{ { name: "login with empty token", @@ -43,6 +46,7 @@ func TestLoginCmd(t *testing.T) { List(gomock.Any()). Return([]serverscom.Host{}, nil) }, + tty: true, }, { name: "login with force", @@ -54,12 +58,20 @@ func TestLoginCmd(t *testing.T) { List(gomock.Any()). Return([]serverscom.Host{}, nil) }, + tty: true, }, { name: "login with invalid context name", args: []string{"_invalid"}, expectError: true, }, + { + name: "no TTY", + args: []string{"notty"}, + input: strings.NewReader("token\n"), + expectError: true, + tty: false, + }, } mockCtrl := gomock.NewController(t) @@ -92,6 +104,18 @@ func TestLoginCmd(t *testing.T) { if tc.configureMock != nil { tc.configureMock(collectionHandler) } + if tc.tty { + ptmx, tty, err := pty.Open() + if err != nil { + t.Fatalf("failed to open pty: %v", err) + } + defer ptmx.Close() + defer tty.Close() + + oldStdout := os.Stdout + defer func() { os.Stdout = oldStdout }() + os.Stdout = tty + } loginCmd := NewCmd(testCmdContext, testClientFactory) diff --git a/go.mod b/go.mod index 57ebbea..0b7b915 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/serverscom/srvctl go 1.23.0 require ( + github.com/creack/pty v1.1.24 github.com/jmespath/go-jmespath v0.4.0 github.com/onsi/gomega v1.36.2 github.com/serverscom/serverscom-go-client v1.0.10 diff --git a/go.sum b/go.sum index 985eb0e..4cf6788 100644 --- a/go.sum +++ b/go.sum @@ -1,4 +1,6 @@ github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s= +github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= From a095fedc85f7991d4a894572d243862c74155a46 Mon Sep 17 00:00:00 2001 From: Oleg Isakov Date: Wed, 12 Mar 2025 12:28:14 +0200 Subject: [PATCH 7/8] add exit code --- main.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/main.go b/main.go index fe67a2f..a113c07 100644 --- a/main.go +++ b/main.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "os" "github.com/serverscom/srvctl/cmd" ) @@ -17,5 +18,7 @@ func main() { } rootCmd := cmd.NewRootCmd(version) - _ = rootCmd.Execute() + if err := rootCmd.Execute(); err != nil { + os.Exit(1) + } } From 80ab997adbb7a35b9e549462f357dae28787aca9 Mon Sep 17 00:00:00 2001 From: Oleg Isakov Date: Wed, 12 Mar 2025 12:36:43 +0200 Subject: [PATCH 8/8] fix tty error desc --- cmd/login/login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/login/login.go b/cmd/login/login.go index 3e711f6..5624b6b 100644 --- a/cmd/login/login.go +++ b/cmd/login/login.go @@ -35,7 +35,7 @@ Example: srvctl login context-name`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if !term.IsTerminal(int(os.Stdout.Fd())) { - return errors.New("TTY required to enter the password.") + return errors.New("TTY required to enter the token.") } manager := cmdContext.GetManager()