From 59094c58c8fbc31d2d6dd8915904d9b4f240ed1b Mon Sep 17 00:00:00 2001 From: Marcel Jacek Date: Wed, 19 Feb 2025 17:17:09 +0100 Subject: [PATCH 1/3] - add nil pointer checks - add tests for the outputResult functions within the network-interface commands --- .../beta/network-interface/create/create.go | 14 +++++--- .../network-interface/create/create_test.go | 35 +++++++++++++++++++ .../network-interface/describe/describe.go | 17 +++++---- .../describe/describe_test.go | 34 ++++++++++++++++++ .../cmd/beta/network-interface/list/list.go | 3 ++ .../beta/network-interface/list/list_test.go | 27 ++++++++++++++ .../beta/network-interface/update/update.go | 8 ++--- .../network-interface/update/update_test.go | 35 +++++++++++++++++++ 8 files changed, 158 insertions(+), 15 deletions(-) diff --git a/internal/cmd/beta/network-interface/create/create.go b/internal/cmd/beta/network-interface/create/create.go index 99fa2dc7f..214837a0e 100644 --- a/internal/cmd/beta/network-interface/create/create.go +++ b/internal/cmd/beta/network-interface/create/create.go @@ -83,6 +83,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 network interface for project %q?", projectLabel) @@ -99,7 +102,7 @@ func NewCmd(p *print.Printer) *cobra.Command { return fmt.Errorf("create network interface: %w", err) } - return outputResult(p, model, resp) + return outputResult(p, model.OutputFormat, model.ProjectId, resp) }, } configureFlags(cmd) @@ -226,8 +229,11 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient *iaas.APICli return req.CreateNicPayload(payload) } -func outputResult(p *print.Printer, model *inputModel, nic *iaas.NIC) error { - switch model.OutputFormat { +func outputResult(p *print.Printer, outputFormat, projectId string, nic *iaas.NIC) error { + if nic == nil { + return fmt.Errorf("nic is empty") + } + switch outputFormat { case print.JSONOutputFormat: details, err := json.MarshalIndent(nic, "", " ") if err != nil { @@ -245,7 +251,7 @@ func outputResult(p *print.Printer, model *inputModel, nic *iaas.NIC) error { return nil default: - p.Outputf("Created network interface for project %q.\nNIC ID: %s\n", model.ProjectId, utils.PtrString(nic.Id)) + p.Outputf("Created network interface for project %q.\nNIC ID: %s\n", projectId, utils.PtrString(nic.Id)) return nil } } diff --git a/internal/cmd/beta/network-interface/create/create_test.go b/internal/cmd/beta/network-interface/create/create_test.go index 2173cf1ea..843b15f69 100644 --- a/internal/cmd/beta/network-interface/create/create_test.go +++ b/internal/cmd/beta/network-interface/create/create_test.go @@ -260,3 +260,38 @@ func TestBuildRequest(t *testing.T) { }) } } + +func Test_outputResult(t *testing.T) { + type args struct { + outputFormat string + projectId string + nic *iaas.NIC + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: true, + }, + { + name: "set empty nic", + args: args{ + nic: &iaas.NIC{}, + }, + 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.projectId, tt.args.nic); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/beta/network-interface/describe/describe.go b/internal/cmd/beta/network-interface/describe/describe.go index c83e79897..a1d741bdf 100644 --- a/internal/cmd/beta/network-interface/describe/describe.go +++ b/internal/cmd/beta/network-interface/describe/describe.go @@ -117,6 +117,9 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient *iaas.APICli } func outputResult(p *print.Printer, outputFormat string, nic *iaas.NIC) error { + if nic == nil { + return fmt.Errorf("nic is empty") + } switch outputFormat { case print.JSONOutputFormat: details, err := json.MarshalIndent(nic, "", " ") @@ -136,27 +139,27 @@ func outputResult(p *print.Printer, outputFormat string, nic *iaas.NIC) error { return nil default: table := tables.NewTable() - table.AddRow("ID", *nic.Id) + table.AddRow("ID", utils.PtrString(nic.Id)) table.AddSeparator() - table.AddRow("NETWORK ID", *nic.NetworkId) + table.AddRow("NETWORK ID", utils.PtrString(nic.NetworkId)) table.AddSeparator() if nic.Name != nil { - table.AddRow("NAME", *nic.Name) + table.AddRow("NAME", utils.PtrString(nic.Name)) table.AddSeparator() } if nic.Ipv4 != nil { - table.AddRow("IPV4", *nic.Ipv4) + table.AddRow("IPV4", utils.PtrString(nic.Ipv4)) table.AddSeparator() } if nic.Ipv6 != nil { - table.AddRow("IPV6", *nic.Ipv6) + table.AddRow("IPV6", utils.PtrString(nic.Ipv6)) table.AddSeparator() } table.AddRow("MAC", utils.PtrString(nic.Mac)) table.AddSeparator() table.AddRow("NIC SECURITY", utils.PtrString(nic.NicSecurity)) if nic.AllowedAddresses != nil && len(*nic.AllowedAddresses) > 0 { - allowedAddresses := []string{} + var allowedAddresses []string for _, value := range *nic.AllowedAddresses { allowedAddresses = append(allowedAddresses, *value.String) } @@ -164,7 +167,7 @@ func outputResult(p *print.Printer, outputFormat string, nic *iaas.NIC) error { table.AddRow("ALLOWED ADDRESSES", strings.Join(allowedAddresses, "\n")) } if nic.Labels != nil && len(*nic.Labels) > 0 { - labels := []string{} + var labels []string for key, value := range *nic.Labels { labels = append(labels, fmt.Sprintf("%s: %s", key, value)) } diff --git a/internal/cmd/beta/network-interface/describe/describe_test.go b/internal/cmd/beta/network-interface/describe/describe_test.go index 145a12cc8..967057b02 100644 --- a/internal/cmd/beta/network-interface/describe/describe_test.go +++ b/internal/cmd/beta/network-interface/describe/describe_test.go @@ -200,3 +200,37 @@ func TestBuildRequest(t *testing.T) { }) } } + +func Test_outputResult(t *testing.T) { + type args struct { + outputFormat string + nic *iaas.NIC + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: true, + }, + { + name: "set empty nic", + args: args{ + nic: &iaas.NIC{}, + }, + 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.nic); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/beta/network-interface/list/list.go b/internal/cmd/beta/network-interface/list/list.go index 4b8d6b314..0309cb8e9 100644 --- a/internal/cmd/beta/network-interface/list/list.go +++ b/internal/cmd/beta/network-interface/list/list.go @@ -83,6 +83,9 @@ func NewCmd(p *print.Printer) *cobra.Command { p.Debug(print.ErrorLevel, "get network name: %v", err) networkLabel = *model.NetworkId } + if networkLabel == "" { + networkLabel = *model.NetworkId + } p.Info("No network interfaces found for network %q\n", networkLabel) return nil } diff --git a/internal/cmd/beta/network-interface/list/list_test.go b/internal/cmd/beta/network-interface/list/list_test.go index 3058072b0..97610156a 100644 --- a/internal/cmd/beta/network-interface/list/list_test.go +++ b/internal/cmd/beta/network-interface/list/list_test.go @@ -205,3 +205,30 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + nics []iaas.NIC + } + 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.nics); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/beta/network-interface/update/update.go b/internal/cmd/beta/network-interface/update/update.go index 2b74557ac..91ab36f27 100644 --- a/internal/cmd/beta/network-interface/update/update.go +++ b/internal/cmd/beta/network-interface/update/update.go @@ -95,7 +95,7 @@ func NewCmd(p *print.Printer) *cobra.Command { return fmt.Errorf("update network interface: %w", err) } - return outputResult(p, model, resp) + return outputResult(p, model.OutputFormat, model.ProjectId, resp) }, } configureFlags(cmd) @@ -218,8 +218,8 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient *iaas.APICli return req.UpdateNicPayload(payload) } -func outputResult(p *print.Printer, model *inputModel, nic *iaas.NIC) error { - switch model.OutputFormat { +func outputResult(p *print.Printer, outputFormat, projectId string, nic *iaas.NIC) error { + switch outputFormat { case print.JSONOutputFormat: details, err := json.MarshalIndent(nic, "", " ") if err != nil { @@ -237,7 +237,7 @@ func outputResult(p *print.Printer, model *inputModel, nic *iaas.NIC) error { return nil default: - p.Outputf("Updated network interface for project %q.\n", model.ProjectId) + p.Outputf("Updated network interface for project %q.\n", projectId) return nil } } diff --git a/internal/cmd/beta/network-interface/update/update_test.go b/internal/cmd/beta/network-interface/update/update_test.go index 00eeda71f..3bebdccf8 100644 --- a/internal/cmd/beta/network-interface/update/update_test.go +++ b/internal/cmd/beta/network-interface/update/update_test.go @@ -291,3 +291,38 @@ func TestBuildRequest(t *testing.T) { }) } } + +func Test_outputResult(t *testing.T) { + type args struct { + outputFormat string + projectId string + nic *iaas.NIC + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: false, + }, + { + name: "set empty nic", + args: args{ + nic: &iaas.NIC{}, + }, + 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.projectId, tt.args.nic); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} From 0ce6cdd8c7852d07077121774f145cfda9201b2e Mon Sep 17 00:00:00 2001 From: Marcel Jacek Date: Wed, 19 Feb 2025 17:33:26 +0100 Subject: [PATCH 2/3] fix: review feedback --- internal/cmd/beta/network-interface/list/list.go | 3 +-- internal/cmd/beta/network-interface/update/update.go | 3 +++ internal/cmd/beta/network-interface/update/update_test.go | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/cmd/beta/network-interface/list/list.go b/internal/cmd/beta/network-interface/list/list.go index 0309cb8e9..b4925b7a5 100644 --- a/internal/cmd/beta/network-interface/list/list.go +++ b/internal/cmd/beta/network-interface/list/list.go @@ -82,8 +82,7 @@ func NewCmd(p *print.Printer) *cobra.Command { if err != nil { p.Debug(print.ErrorLevel, "get network name: %v", err) networkLabel = *model.NetworkId - } - if networkLabel == "" { + } else if networkLabel == "" { networkLabel = *model.NetworkId } p.Info("No network interfaces found for network %q\n", networkLabel) diff --git a/internal/cmd/beta/network-interface/update/update.go b/internal/cmd/beta/network-interface/update/update.go index 91ab36f27..9e7c12022 100644 --- a/internal/cmd/beta/network-interface/update/update.go +++ b/internal/cmd/beta/network-interface/update/update.go @@ -219,6 +219,9 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient *iaas.APICli } func outputResult(p *print.Printer, outputFormat, projectId string, nic *iaas.NIC) error { + if nic == nil { + return fmt.Errorf("nic is empty") + } switch outputFormat { case print.JSONOutputFormat: details, err := json.MarshalIndent(nic, "", " ") diff --git a/internal/cmd/beta/network-interface/update/update_test.go b/internal/cmd/beta/network-interface/update/update_test.go index 3bebdccf8..98987b829 100644 --- a/internal/cmd/beta/network-interface/update/update_test.go +++ b/internal/cmd/beta/network-interface/update/update_test.go @@ -306,7 +306,7 @@ func Test_outputResult(t *testing.T) { { name: "empty", args: args{}, - wantErr: false, + wantErr: true, }, { name: "set empty nic", From a5a52de45c8a915f51f161cf765cb09a7576d907 Mon Sep 17 00:00:00 2001 From: Marcel Jacek Date: Thu, 20 Feb 2025 10:21:08 +0100 Subject: [PATCH 3/3] fix: consistent `else if`-clause for labels --- internal/cmd/beta/network-interface/create/create.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/cmd/beta/network-interface/create/create.go b/internal/cmd/beta/network-interface/create/create.go index 214837a0e..eb6bb301b 100644 --- a/internal/cmd/beta/network-interface/create/create.go +++ b/internal/cmd/beta/network-interface/create/create.go @@ -82,8 +82,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 }