From f4c54a952a49c3d04689b166f2cffa24eb684fa6 Mon Sep 17 00:00:00 2001 From: Marcel Jacek Date: Wed, 19 Feb 2025 17:17:41 +0100 Subject: [PATCH 1/2] - add nil pointer checks - add tests for the outputResult functions within the public-ip commands --- .../cmd/beta/public-ip/associate/associate.go | 3 ++ internal/cmd/beta/public-ip/create/create.go | 9 ++++-- .../cmd/beta/public-ip/create/create_test.go | 28 +++++++++++++++++++ internal/cmd/beta/public-ip/delete/delete.go | 5 +++- .../cmd/beta/public-ip/describe/describe.go | 4 +-- .../beta/public-ip/describe/describe_test.go | 27 ++++++++++++++++++ .../public-ip/disassociate/disassociate.go | 3 ++ internal/cmd/beta/public-ip/list/list.go | 3 ++ internal/cmd/beta/public-ip/list/list_test.go | 27 ++++++++++++++++++ 9 files changed, 103 insertions(+), 6 deletions(-) diff --git a/internal/cmd/beta/public-ip/associate/associate.go b/internal/cmd/beta/public-ip/associate/associate.go index 1c33b2b8d..f1fd5d29e 100644 --- a/internal/cmd/beta/public-ip/associate/associate.go +++ b/internal/cmd/beta/public-ip/associate/associate.go @@ -60,6 +60,9 @@ func NewCmd(p *print.Printer) *cobra.Command { p.Debug(print.ErrorLevel, "get public IP: %v", err) publicIpLabel = model.PublicIpId } + if publicIpLabel == "" { + publicIpLabel = model.PublicIpId + } if !model.AssumeYes { prompt := fmt.Sprintf("Are you sure you want to associate public IP %q with resource %v?", publicIpLabel, *model.AssociatedResourceId) diff --git a/internal/cmd/beta/public-ip/create/create.go b/internal/cmd/beta/public-ip/create/create.go index ad6196e82..e968bdf0b 100644 --- a/internal/cmd/beta/public-ip/create/create.go +++ b/internal/cmd/beta/public-ip/create/create.go @@ -68,6 +68,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 + } if !model.AssumeYes { prompt := fmt.Sprintf("Are you sure you want to create a public IP for project %q?", projectLabel) @@ -84,7 +87,7 @@ func NewCmd(p *print.Printer) *cobra.Command { return fmt.Errorf("create public IP: %w", err) } - return outputResult(p, model, projectLabel, resp) + return outputResult(p, model.OutputFormat, projectLabel, *resp) }, } configureFlags(cmd) @@ -140,8 +143,8 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient *iaas.APICli return req.CreatePublicIPPayload(payload) } -func outputResult(p *print.Printer, model *inputModel, projectLabel string, publicIp *iaas.PublicIp) error { - switch model.OutputFormat { +func outputResult(p *print.Printer, outputFormat, projectLabel string, publicIp iaas.PublicIp) error { + switch outputFormat { case print.JSONOutputFormat: details, err := json.MarshalIndent(publicIp, "", " ") if err != nil { diff --git a/internal/cmd/beta/public-ip/create/create_test.go b/internal/cmd/beta/public-ip/create/create_test.go index 7411c0a9f..b204d561f 100644 --- a/internal/cmd/beta/public-ip/create/create_test.go +++ b/internal/cmd/beta/public-ip/create/create_test.go @@ -213,3 +213,31 @@ func TestBuildRequest(t *testing.T) { }) } } + +func Test_outputResult(t *testing.T) { + type args struct { + outputFormat string + projectLabel string + publicIp iaas.PublicIp + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + 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.projectLabel, tt.args.publicIp); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/beta/public-ip/delete/delete.go b/internal/cmd/beta/public-ip/delete/delete.go index bd87419da..9edc4784a 100644 --- a/internal/cmd/beta/public-ip/delete/delete.go +++ b/internal/cmd/beta/public-ip/delete/delete.go @@ -58,6 +58,9 @@ func NewCmd(p *print.Printer) *cobra.Command { p.Debug(print.ErrorLevel, "get public IP: %v", err) publicIpLabel = model.PublicIpId } + if publicIpLabel == "" { + publicIpLabel = model.PublicIpId + } if !model.AssumeYes { prompt := fmt.Sprintf("Are you sure you want to delete public IP %q? (This cannot be undone)", publicIpLabel) @@ -74,7 +77,7 @@ func NewCmd(p *print.Printer) *cobra.Command { return fmt.Errorf("delete public IP: %w", err) } - p.Info("Deleted public IP %q\n", model.PublicIpId) + p.Info("Deleted public IP %q\n", publicIpLabel) return nil }, } diff --git a/internal/cmd/beta/public-ip/describe/describe.go b/internal/cmd/beta/public-ip/describe/describe.go index c07e9e981..bc532fd6b 100644 --- a/internal/cmd/beta/public-ip/describe/describe.go +++ b/internal/cmd/beta/public-ip/describe/describe.go @@ -66,7 +66,7 @@ func NewCmd(p *print.Printer) *cobra.Command { return fmt.Errorf("read public IP: %w", err) } - return outputResult(p, model.OutputFormat, resp) + return outputResult(p, model.OutputFormat, *resp) }, } return cmd @@ -101,7 +101,7 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient *iaas.APICli return apiClient.GetPublicIP(ctx, model.ProjectId, model.PublicIpId) } -func outputResult(p *print.Printer, outputFormat string, publicIp *iaas.PublicIp) error { +func outputResult(p *print.Printer, outputFormat string, publicIp iaas.PublicIp) error { switch outputFormat { case print.JSONOutputFormat: details, err := json.MarshalIndent(publicIp, "", " ") diff --git a/internal/cmd/beta/public-ip/describe/describe_test.go b/internal/cmd/beta/public-ip/describe/describe_test.go index 9754de721..44ab876ff 100644 --- a/internal/cmd/beta/public-ip/describe/describe_test.go +++ b/internal/cmd/beta/public-ip/describe/describe_test.go @@ -216,3 +216,30 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + publicIp iaas.PublicIp + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + 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.publicIp); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/beta/public-ip/disassociate/disassociate.go b/internal/cmd/beta/public-ip/disassociate/disassociate.go index 0ad28d773..7b9252c1e 100644 --- a/internal/cmd/beta/public-ip/disassociate/disassociate.go +++ b/internal/cmd/beta/public-ip/disassociate/disassociate.go @@ -56,6 +56,9 @@ func NewCmd(p *print.Printer) *cobra.Command { p.Debug(print.ErrorLevel, "get public IP: %v", err) publicIpLabel = model.PublicIpId } + if publicIpLabel == "" { + publicIpLabel = model.PublicIpId + } if !model.AssumeYes { prompt := fmt.Sprintf("Are you sure you want to disassociate public IP %q from the associated resource %q?", publicIpLabel, associatedResourceId) diff --git a/internal/cmd/beta/public-ip/list/list.go b/internal/cmd/beta/public-ip/list/list.go index 2293600aa..0f7cc9290 100644 --- a/internal/cmd/beta/public-ip/list/list.go +++ b/internal/cmd/beta/public-ip/list/list.go @@ -82,6 +82,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 + } p.Info("No public IPs found for project %q\n", projectLabel) return nil } diff --git a/internal/cmd/beta/public-ip/list/list_test.go b/internal/cmd/beta/public-ip/list/list_test.go index b7f39f61d..0644d83e8 100644 --- a/internal/cmd/beta/public-ip/list/list_test.go +++ b/internal/cmd/beta/public-ip/list/list_test.go @@ -202,3 +202,30 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + publicIps []iaas.PublicIp + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + 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.publicIps); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} From 35fe8c9ff200234d99b8fb2e222d6edc0cb20c6d Mon Sep 17 00:00:00 2001 From: Marcel Jacek Date: Thu, 20 Feb 2025 10:32:39 +0100 Subject: [PATCH 2/2] fix: consistent `else if`-clause for labels --- internal/cmd/beta/public-ip/associate/associate.go | 3 +-- internal/cmd/beta/public-ip/create/create.go | 3 +-- internal/cmd/beta/public-ip/delete/delete.go | 3 +-- internal/cmd/beta/public-ip/disassociate/disassociate.go | 3 +-- internal/cmd/beta/public-ip/list/list.go | 3 +-- 5 files changed, 5 insertions(+), 10 deletions(-) diff --git a/internal/cmd/beta/public-ip/associate/associate.go b/internal/cmd/beta/public-ip/associate/associate.go index f1fd5d29e..d50bafb8d 100644 --- a/internal/cmd/beta/public-ip/associate/associate.go +++ b/internal/cmd/beta/public-ip/associate/associate.go @@ -59,8 +59,7 @@ func NewCmd(p *print.Printer) *cobra.Command { if err != nil { p.Debug(print.ErrorLevel, "get public IP: %v", err) publicIpLabel = model.PublicIpId - } - if publicIpLabel == "" { + } else if publicIpLabel == "" { publicIpLabel = model.PublicIpId } diff --git a/internal/cmd/beta/public-ip/create/create.go b/internal/cmd/beta/public-ip/create/create.go index e968bdf0b..9bc05ab3d 100644 --- a/internal/cmd/beta/public-ip/create/create.go +++ b/internal/cmd/beta/public-ip/create/create.go @@ -67,8 +67,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/public-ip/delete/delete.go b/internal/cmd/beta/public-ip/delete/delete.go index 9edc4784a..0f257d8f0 100644 --- a/internal/cmd/beta/public-ip/delete/delete.go +++ b/internal/cmd/beta/public-ip/delete/delete.go @@ -57,8 +57,7 @@ func NewCmd(p *print.Printer) *cobra.Command { if err != nil { p.Debug(print.ErrorLevel, "get public IP: %v", err) publicIpLabel = model.PublicIpId - } - if publicIpLabel == "" { + } else if publicIpLabel == "" { publicIpLabel = model.PublicIpId } diff --git a/internal/cmd/beta/public-ip/disassociate/disassociate.go b/internal/cmd/beta/public-ip/disassociate/disassociate.go index 7b9252c1e..eb8872d80 100644 --- a/internal/cmd/beta/public-ip/disassociate/disassociate.go +++ b/internal/cmd/beta/public-ip/disassociate/disassociate.go @@ -55,8 +55,7 @@ func NewCmd(p *print.Printer) *cobra.Command { if err != nil { p.Debug(print.ErrorLevel, "get public IP: %v", err) publicIpLabel = model.PublicIpId - } - if publicIpLabel == "" { + } else if publicIpLabel == "" { publicIpLabel = model.PublicIpId } diff --git a/internal/cmd/beta/public-ip/list/list.go b/internal/cmd/beta/public-ip/list/list.go index 0f7cc9290..c071e5069 100644 --- a/internal/cmd/beta/public-ip/list/list.go +++ b/internal/cmd/beta/public-ip/list/list.go @@ -81,8 +81,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 } p.Info("No public IPs found for project %q\n", projectLabel)