From 720a202319b83adf73843ee618bc0f77775d4f21 Mon Sep 17 00:00:00 2001 From: Ruben Hoenle Date: Fri, 21 Feb 2025 09:06:57 +0100 Subject: [PATCH] fix(logme): add nil pointer checks for cmd outputs relates to STACKITCLI-102 --- .../cmd/logme/credentials/create/create.go | 14 ++++--- .../logme/credentials/create/create_test.go | 36 ++++++++++++++++ .../logme/credentials/describe/describe.go | 4 ++ .../credentials/describe/describe_test.go | 34 +++++++++++++++ .../cmd/logme/credentials/list/list_test.go | 41 +++++++++++++++++++ internal/cmd/logme/instance/create/create.go | 12 ++++-- .../cmd/logme/instance/create/create_test.go | 36 ++++++++++++++++ .../cmd/logme/instance/describe/describe.go | 22 ++++++---- .../logme/instance/describe/describe_test.go | 34 +++++++++++++++ internal/cmd/logme/instance/list/list.go | 11 ++++- internal/cmd/logme/instance/list/list_test.go | 41 +++++++++++++++++++ internal/cmd/logme/plans/plans.go | 22 +++++----- internal/cmd/logme/plans/plans_test.go | 41 +++++++++++++++++++ 13 files changed, 319 insertions(+), 29 deletions(-) diff --git a/internal/cmd/logme/credentials/create/create.go b/internal/cmd/logme/credentials/create/create.go index 22685cedc..61df00f15 100644 --- a/internal/cmd/logme/credentials/create/create.go +++ b/internal/cmd/logme/credentials/create/create.go @@ -78,7 +78,7 @@ func NewCmd(p *print.Printer) *cobra.Command { return fmt.Errorf("create LogMe credentials: %w", err) } - return outputResult(p, model, instanceLabel, resp) + return outputResult(p, model.OutputFormat, model.ShowPassword, instanceLabel, resp) }, } configureFlags(cmd) @@ -122,11 +122,15 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient *logme.APICl return req } -func outputResult(p *print.Printer, model *inputModel, instanceLabel string, resp *logme.CredentialsResponse) error { - if !model.ShowPassword { +func outputResult(p *print.Printer, outputFormat string, showPassword bool, instanceLabel string, resp *logme.CredentialsResponse) error { + if resp == nil { + return fmt.Errorf("credentials response is empty") + } + + if !showPassword && resp.HasRaw() && resp.Raw.Credentials != nil { resp.Raw.Credentials.Password = utils.Ptr("hidden") } - switch model.OutputFormat { + switch outputFormat { case print.JSONOutputFormat: details, err := json.MarshalIndent(resp, "", " ") if err != nil { @@ -150,7 +154,7 @@ func outputResult(p *print.Printer, model *inputModel, instanceLabel string, res if username := resp.Raw.Credentials.Username; username != nil && *username != "" { p.Outputf("Username: %s\n", utils.PtrString(username)) } - if !model.ShowPassword { + if !showPassword { p.Outputf("Password: \n") } else { p.Outputf("Password: %s\n", utils.PtrString(resp.Raw.Credentials.Password)) diff --git a/internal/cmd/logme/credentials/create/create_test.go b/internal/cmd/logme/credentials/create/create_test.go index ff0d16fbd..3ad8aec16 100644 --- a/internal/cmd/logme/credentials/create/create_test.go +++ b/internal/cmd/logme/credentials/create/create_test.go @@ -200,3 +200,39 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + showPassword bool + instanceLabel string + credentials *logme.CredentialsResponse + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: true, + }, + { + name: "set empty credentials", + args: args{ + credentials: &logme.CredentialsResponse{}, + }, + wantErr: false, + }, + } + p := print.NewPrinter() + p.Cmd = NewCmd(p) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := outputResult(p, tt.args.outputFormat, tt.args.showPassword, tt.args.instanceLabel, tt.args.credentials); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/logme/credentials/describe/describe.go b/internal/cmd/logme/credentials/describe/describe.go index 968c2b3ef..2a73c9a15 100644 --- a/internal/cmd/logme/credentials/describe/describe.go +++ b/internal/cmd/logme/credentials/describe/describe.go @@ -112,6 +112,10 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient *logme.APICl } func outputResult(p *print.Printer, outputFormat string, credentials *logme.CredentialsResponse) error { + if credentials == nil { + return fmt.Errorf("credentials is nil") + } + switch outputFormat { case print.JSONOutputFormat: details, err := json.MarshalIndent(credentials, "", " ") diff --git a/internal/cmd/logme/credentials/describe/describe_test.go b/internal/cmd/logme/credentials/describe/describe_test.go index 7726b725c..3ef166834 100644 --- a/internal/cmd/logme/credentials/describe/describe_test.go +++ b/internal/cmd/logme/credentials/describe/describe_test.go @@ -243,3 +243,37 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + credentials *logme.CredentialsResponse + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: true, + }, + { + name: "set empty credentials", + args: args{ + credentials: &logme.CredentialsResponse{}, + }, + wantErr: false, + }, + } + p := print.NewPrinter() + p.Cmd = NewCmd(p) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := outputResult(p, tt.args.outputFormat, tt.args.credentials); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/logme/credentials/list/list_test.go b/internal/cmd/logme/credentials/list/list_test.go index 570b58bae..dd9b71e66 100644 --- a/internal/cmd/logme/credentials/list/list_test.go +++ b/internal/cmd/logme/credentials/list/list_test.go @@ -207,3 +207,44 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + credentials []logme.CredentialsListItem + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: false, + }, + { + name: "set empty credentials slice", + args: args{ + credentials: []logme.CredentialsListItem{}, + }, + wantErr: false, + }, + { + name: "set empty credential in credentials slice", + args: args{ + credentials: []logme.CredentialsListItem{{}}, + }, + wantErr: false, + }, + } + p := print.NewPrinter() + p.Cmd = NewCmd(p) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := outputResult(p, tt.args.outputFormat, tt.args.credentials); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/logme/instance/create/create.go b/internal/cmd/logme/instance/create/create.go index 7effe9a74..df11b20a2 100644 --- a/internal/cmd/logme/instance/create/create.go +++ b/internal/cmd/logme/instance/create/create.go @@ -125,7 +125,7 @@ func NewCmd(p *print.Printer) *cobra.Command { s.Stop() } - return outputResult(p, model, projectLabel, resp) + return outputResult(p, model.OutputFormat, model.Async, projectLabel, resp) }, } configureFlags(cmd) @@ -251,8 +251,12 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient logMeClient) return req, nil } -func outputResult(p *print.Printer, model *inputModel, projectLabel string, resp *logme.CreateInstanceResponse) error { - switch model.OutputFormat { +func outputResult(p *print.Printer, outputFormat string, async bool, projectLabel string, resp *logme.CreateInstanceResponse) error { + if resp == nil { + return fmt.Errorf("response is nil") + } + + switch outputFormat { case print.JSONOutputFormat: details, err := json.MarshalIndent(resp, "", " ") if err != nil { @@ -271,7 +275,7 @@ func outputResult(p *print.Printer, model *inputModel, projectLabel string, resp return nil default: operationState := "Created" - if model.Async { + if async { operationState = "Triggered creation of" } p.Outputf("%s instance for project %q. Instance ID: %s\n", operationState, projectLabel, utils.PtrString(resp.InstanceId)) diff --git a/internal/cmd/logme/instance/create/create_test.go b/internal/cmd/logme/instance/create/create_test.go index fe0cfcc0b..ac8928edf 100644 --- a/internal/cmd/logme/instance/create/create_test.go +++ b/internal/cmd/logme/instance/create/create_test.go @@ -463,3 +463,39 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + async bool + projectLabel string + resp *logme.CreateInstanceResponse + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: true, + }, + { + name: "empty response", + args: args{ + resp: &logme.CreateInstanceResponse{}, + }, + wantErr: false, + }, + } + p := print.NewPrinter() + p.Cmd = NewCmd(p) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := outputResult(p, tt.args.outputFormat, tt.args.async, tt.args.projectLabel, tt.args.resp); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/logme/instance/describe/describe.go b/internal/cmd/logme/instance/describe/describe.go index 9a19a4a9c..d32944bdf 100644 --- a/internal/cmd/logme/instance/describe/describe.go +++ b/internal/cmd/logme/instance/describe/describe.go @@ -100,6 +100,10 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient *logme.APICl } func outputResult(p *print.Printer, outputFormat string, instance *logme.Instance) error { + if instance == nil { + return fmt.Errorf("instance is nil") + } + switch outputFormat { case print.JSONOutputFormat: details, err := json.MarshalIndent(instance, "", " ") @@ -123,16 +127,18 @@ func outputResult(p *print.Printer, outputFormat string, instance *logme.Instanc table.AddSeparator() table.AddRow("NAME", utils.PtrString(instance.Name)) table.AddSeparator() - table.AddRow("LAST OPERATION TYPE", utils.PtrString(instance.LastOperation.Type)) - table.AddSeparator() - table.AddRow("LAST OPERATION STATE", utils.PtrString(instance.LastOperation.State)) - table.AddSeparator() + if instance.LastOperation != nil { + table.AddRow("LAST OPERATION TYPE", utils.PtrString(instance.LastOperation.Type)) + table.AddSeparator() + table.AddRow("LAST OPERATION STATE", utils.PtrString(instance.LastOperation.State)) + table.AddSeparator() + } table.AddRow("PLAN ID", utils.PtrString(instance.PlanId)) // Only show ACL if it's present and not empty - acl := (*instance.Parameters)[aclParameterKey] - aclStr, ok := acl.(string) - if ok { - if aclStr != "" { + if instance.Parameters != nil { + acl := (*instance.Parameters)[aclParameterKey] + aclStr, ok := acl.(string) + if ok && aclStr != "" { table.AddSeparator() table.AddRow("ACL", aclStr) } diff --git a/internal/cmd/logme/instance/describe/describe_test.go b/internal/cmd/logme/instance/describe/describe_test.go index 60cdc69ab..3cc0b5e38 100644 --- a/internal/cmd/logme/instance/describe/describe_test.go +++ b/internal/cmd/logme/instance/describe/describe_test.go @@ -216,3 +216,37 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + instance *logme.Instance + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: true, + }, + { + name: "empty instance", + args: args{ + instance: &logme.Instance{}, + }, + wantErr: false, + }, + } + p := print.NewPrinter() + p.Cmd = NewCmd(p) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := outputResult(p, tt.args.outputFormat, tt.args.instance); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/logme/instance/list/list.go b/internal/cmd/logme/instance/list/list.go index 593aff6da..e59d09cb6 100644 --- a/internal/cmd/logme/instance/list/list.go +++ b/internal/cmd/logme/instance/list/list.go @@ -152,11 +152,18 @@ func outputResult(p *print.Printer, outputFormat string, instances []logme.Insta table.SetHeader("ID", "NAME", "LAST OPERATION TYPE", "LAST OPERATION STATE") for i := range instances { instance := instances[i] + + lastOperationType, lastOperationState := "", "" + if instance.LastOperation != nil { + lastOperationType = utils.PtrString(instance.LastOperation.Type) + lastOperationState = utils.PtrString(instance.LastOperation.State) + } + table.AddRow( utils.PtrString(instance.InstanceId), utils.PtrString(instance.Name), - utils.PtrString(instance.LastOperation.Type), - utils.PtrString(instance.LastOperation.State), + lastOperationType, + lastOperationState, ) } err := table.Display(p) diff --git a/internal/cmd/logme/instance/list/list_test.go b/internal/cmd/logme/instance/list/list_test.go index 5524c41e9..97df7394c 100644 --- a/internal/cmd/logme/instance/list/list_test.go +++ b/internal/cmd/logme/instance/list/list_test.go @@ -186,3 +186,44 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + instances []logme.Instance + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: false, + }, + { + name: "empty instances slice", + args: args{ + instances: []logme.Instance{}, + }, + wantErr: false, + }, + { + name: "empty instance in instances slice", + args: args{ + instances: []logme.Instance{{}}, + }, + wantErr: false, + }, + } + p := print.NewPrinter() + p.Cmd = NewCmd(p) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := outputResult(p, tt.args.outputFormat, tt.args.instances); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/logme/plans/plans.go b/internal/cmd/logme/plans/plans.go index 385ee2639..ccfdc098b 100644 --- a/internal/cmd/logme/plans/plans.go +++ b/internal/cmd/logme/plans/plans.go @@ -152,17 +152,19 @@ func outputResult(p *print.Printer, outputFormat string, plans []logme.Offering) table.SetHeader("OFFERING NAME", "VERSION", "ID", "NAME", "DESCRIPTION") for i := range plans { o := plans[i] - for j := range *o.Plans { - p := (*o.Plans)[j] - table.AddRow( - utils.PtrString(o.Name), - utils.PtrString(o.Version), - utils.PtrString(p.Id), - utils.PtrString(p.Name), - utils.PtrString(p.Description), - ) + if o.Plans != nil { + for j := range *o.Plans { + p := (*o.Plans)[j] + table.AddRow( + utils.PtrString(o.Name), + utils.PtrString(o.Version), + utils.PtrString(p.Id), + utils.PtrString(p.Name), + utils.PtrString(p.Description), + ) + } + table.AddSeparator() } - table.AddSeparator() } table.EnableAutoMergeOnColumns(1, 2) err := table.Display(p) diff --git a/internal/cmd/logme/plans/plans_test.go b/internal/cmd/logme/plans/plans_test.go index f8214f2f2..bc8c78bb7 100644 --- a/internal/cmd/logme/plans/plans_test.go +++ b/internal/cmd/logme/plans/plans_test.go @@ -186,3 +186,44 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + plans []logme.Offering + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: false, + }, + { + name: "empty plans slice", + args: args{ + plans: []logme.Offering{}, + }, + wantErr: false, + }, + { + name: "empty plan in plans slice", + args: args{ + plans: []logme.Offering{{}}, + }, + wantErr: false, + }, + } + p := print.NewPrinter() + p.Cmd = NewCmd(p) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := outputResult(p, tt.args.outputFormat, tt.args.plans); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +}