From 7a7cd9a58f69970ba14ddd895e93550d544debb9 Mon Sep 17 00:00:00 2001 From: Marcel Jacek Date: Wed, 19 Feb 2025 17:14:17 +0100 Subject: [PATCH 1/2] - add nil pointer checks - add tests for the outputResult functions within the image commands --- internal/cmd/beta/image/create/create.go | 9 ++- internal/cmd/beta/image/create/create_test.go | 57 +++++++++++++++++-- internal/cmd/beta/image/delete/delete.go | 3 + internal/cmd/beta/image/describe/describe.go | 9 ++- .../cmd/beta/image/describe/describe_test.go | 34 +++++++++++ internal/cmd/beta/image/list/list.go | 13 ++++- internal/cmd/beta/image/list/list_test.go | 38 +++++++++++++ internal/cmd/beta/image/update/update.go | 3 + 8 files changed, 157 insertions(+), 9 deletions(-) diff --git a/internal/cmd/beta/image/create/create.go b/internal/cmd/beta/image/create/create.go index 1203f70a6..149aae01e 100644 --- a/internal/cmd/beta/image/create/create.go +++ b/internal/cmd/beta/image/create/create.go @@ -377,7 +377,14 @@ func createPayload(_ context.Context, model *inputModel) iaas.CreateImagePayload } func outputResult(p *print.Printer, model *inputModel, resp *iaas.ImageCreateResponse) error { - switch model.OutputFormat { + if model == nil { + return fmt.Errorf("input model is nil") + } + var outputFormat string + if model.GlobalFlagModel != nil { + outputFormat = model.OutputFormat + } + switch outputFormat { case print.JSONOutputFormat: details, err := json.MarshalIndent(resp, "", " ") if err != nil { diff --git a/internal/cmd/beta/image/create/create_test.go b/internal/cmd/beta/image/create/create_test.go index 5777a9cf8..5e0bee59d 100644 --- a/internal/cmd/beta/image/create/create_test.go +++ b/internal/cmd/beta/image/create/create_test.go @@ -6,13 +6,12 @@ import ( "strings" "testing" - "github.com/stackitcloud/stackit-cli/internal/pkg/globalflags" - "github.com/stackitcloud/stackit-cli/internal/pkg/print" - "github.com/stackitcloud/stackit-cli/internal/pkg/utils" - "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/google/uuid" + "github.com/stackitcloud/stackit-cli/internal/pkg/globalflags" + "github.com/stackitcloud/stackit-cli/internal/pkg/print" + "github.com/stackitcloud/stackit-cli/internal/pkg/utils" "github.com/stackitcloud/stackit-sdk-go/services/iaas" ) @@ -372,3 +371,53 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + model *inputModel + resp *iaas.ImageCreateResponse + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "nil", + args: args{ + model: nil, + resp: nil, + }, + wantErr: true, + }, + { + name: "empty input", + args: args{ + model: &inputModel{}, + resp: &iaas.ImageCreateResponse{}, + }, + wantErr: false, + }, + { + name: "output json", + args: args{ + model: &inputModel{ + GlobalFlagModel: &globalflags.GlobalFlagModel{ + OutputFormat: print.JSONOutputFormat, + }, + }, + resp: nil, + }, + 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.model, tt.args.resp); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/beta/image/delete/delete.go b/internal/cmd/beta/image/delete/delete.go index 3e2b44260..4c9709c3a 100644 --- a/internal/cmd/beta/image/delete/delete.go +++ b/internal/cmd/beta/image/delete/delete.go @@ -57,6 +57,9 @@ func NewCmd(p *print.Printer) *cobra.Command { p.Debug(print.ErrorLevel, "get image name: %v", err) imageName = model.ImageId } + if imageName == "" { + imageName = model.ImageId + } if !model.AssumeYes { prompt := fmt.Sprintf("Are you sure you want to delete the image %q for %q?", imageName, projectLabel) diff --git a/internal/cmd/beta/image/describe/describe.go b/internal/cmd/beta/image/describe/describe.go index 6da572ad4..8c8b27023 100644 --- a/internal/cmd/beta/image/describe/describe.go +++ b/internal/cmd/beta/image/describe/describe.go @@ -56,7 +56,7 @@ func NewCmd(p *print.Printer) *cobra.Command { return fmt.Errorf("get image: %w", err) } - if err := outputResult(p, model, image); err != nil { + if err := outputResult(p, model.OutputFormat, image); err != nil { return err } @@ -95,8 +95,11 @@ func parseInput(p *print.Printer, cmd *cobra.Command, cliArgs []string) (*inputM return &model, nil } -func outputResult(p *print.Printer, model *inputModel, resp *iaas.Image) error { - switch model.OutputFormat { +func outputResult(p *print.Printer, outputFormat string, resp *iaas.Image) error { + if resp == nil { + return fmt.Errorf("image not found") + } + switch outputFormat { case print.JSONOutputFormat: details, err := json.MarshalIndent(resp, "", " ") if err != nil { diff --git a/internal/cmd/beta/image/describe/describe_test.go b/internal/cmd/beta/image/describe/describe_test.go index 4003e78d1..036648fee 100644 --- a/internal/cmd/beta/image/describe/describe_test.go +++ b/internal/cmd/beta/image/describe/describe_test.go @@ -192,3 +192,37 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + resp *iaas.Image + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{ + resp: &iaas.Image{}, + }, + wantErr: false, + }, + { + name: "nil", + args: args{}, + wantErr: true, + }, + } + 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.resp); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/beta/image/list/list.go b/internal/cmd/beta/image/list/list.go index 2452c7443..a92e70e32 100644 --- a/internal/cmd/beta/image/list/list.go +++ b/internal/cmd/beta/image/list/list.go @@ -69,6 +69,9 @@ func NewCmd(p *print.Printer) *cobra.Command { p.Debug(print.ErrorLevel, "get project name: %v", err) projectLabel = model.ProjectId } + if projectLabel == "" { + projectLabel = model.ProjectId + } // Call API request := buildRequest(ctx, model, apiClient) @@ -108,10 +111,18 @@ func parseInput(p *print.Printer, cmd *cobra.Command) (*inputModel, error) { return nil, &errors.ProjectIdError{} } + limit := flags.FlagToInt64Pointer(p, cmd, limitFlag) + if limit != nil && *limit < 1 { + return nil, &errors.FlagValidationError{ + Flag: limitFlag, + Details: "must be greater than 0", + } + } + model := inputModel{ GlobalFlagModel: globalFlags, LabelSelector: flags.FlagToStringPointer(p, cmd, labelSelectorFlag), - Limit: flags.FlagToInt64Pointer(p, cmd, limitFlag), + Limit: limit, } if p.IsVerbosityDebug() { diff --git a/internal/cmd/beta/image/list/list_test.go b/internal/cmd/beta/image/list/list_test.go index 70c6112cb..49a27a16e 100644 --- a/internal/cmd/beta/image/list/list_test.go +++ b/internal/cmd/beta/image/list/list_test.go @@ -210,3 +210,41 @@ func TestBuildRequest(t *testing.T) { }) } } + +func Test_outputResult(t *testing.T) { + type args struct { + outputFormat string + items []iaas.Image + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{ + outputFormat: "", + items: []iaas.Image{}, + }, + wantErr: false, + }, + { + name: "output format json", + args: args{ + outputFormat: print.JSONOutputFormat, + items: []iaas.Image{}, + }, + 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.items); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/beta/image/update/update.go b/internal/cmd/beta/image/update/update.go index 8e8becebd..afc0b83ec 100644 --- a/internal/cmd/beta/image/update/update.go +++ b/internal/cmd/beta/image/update/update.go @@ -135,6 +135,9 @@ func NewCmd(p *print.Printer) *cobra.Command { p.Debug(print.WarningLevel, "cannot retrieve image name: %v", err) imageLabel = model.Id } + if imageLabel == "" { + imageLabel = model.Id + } if !model.AssumeYes { prompt := fmt.Sprintf("Are you sure you want to update the image %q?", imageLabel) From 77d07785864927695ab968c2177f3b2f11e04a58 Mon Sep 17 00:00:00 2001 From: Marcel Jacek Date: Thu, 20 Feb 2025 10:30:08 +0100 Subject: [PATCH 2/2] fix: consistent `else if`-clause for labels --- internal/cmd/beta/image/delete/delete.go | 3 +-- internal/cmd/beta/image/list/list.go | 3 +-- internal/cmd/beta/image/update/update.go | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/internal/cmd/beta/image/delete/delete.go b/internal/cmd/beta/image/delete/delete.go index 4c9709c3a..609ced9ec 100644 --- a/internal/cmd/beta/image/delete/delete.go +++ b/internal/cmd/beta/image/delete/delete.go @@ -56,8 +56,7 @@ func NewCmd(p *print.Printer) *cobra.Command { if err != nil { p.Debug(print.ErrorLevel, "get image name: %v", err) imageName = model.ImageId - } - if imageName == "" { + } else if imageName == "" { imageName = model.ImageId } diff --git a/internal/cmd/beta/image/list/list.go b/internal/cmd/beta/image/list/list.go index a92e70e32..035771d89 100644 --- a/internal/cmd/beta/image/list/list.go +++ b/internal/cmd/beta/image/list/list.go @@ -68,8 +68,7 @@ func NewCmd(p *print.Printer) *cobra.Command { if err != nil { p.Debug(print.ErrorLevel, "get project name: %v", err) projectLabel = model.ProjectId - } - if projectLabel == "" { + } else if projectLabel == "" { projectLabel = model.ProjectId } diff --git a/internal/cmd/beta/image/update/update.go b/internal/cmd/beta/image/update/update.go index afc0b83ec..9a4603882 100644 --- a/internal/cmd/beta/image/update/update.go +++ b/internal/cmd/beta/image/update/update.go @@ -134,8 +134,7 @@ func NewCmd(p *print.Printer) *cobra.Command { if err != nil { p.Debug(print.WarningLevel, "cannot retrieve image name: %v", err) imageLabel = model.Id - } - if imageLabel == "" { + } else if imageLabel == "" { imageLabel = model.Id }