From 605f3243ea920639946dd4abdc53ce06d6143518 Mon Sep 17 00:00:00 2001 From: Ruben Hoenle Date: Tue, 25 Feb 2025 16:05:08 +0100 Subject: [PATCH] fix(ske): add nil pointer checks for cmd outputs relates to STACKITCLI-115 --- internal/cmd/ske/cluster/create/create.go | 18 ++-- .../cmd/ske/cluster/create/create_test.go | 36 +++++++ internal/cmd/ske/cluster/describe/describe.go | 16 +++- .../cmd/ske/cluster/describe/describe_test.go | 34 +++++++ .../generate-payload/generate_payload.go | 4 + .../generate-payload/generate_payload_test.go | 42 +++++++++ internal/cmd/ske/cluster/list/list.go | 17 +++- internal/cmd/ske/cluster/list/list_test.go | 41 ++++++++ internal/cmd/ske/cluster/update/update.go | 18 ++-- .../cmd/ske/cluster/update/update_test.go | 36 +++++++ internal/cmd/ske/describe/describe.go | 8 +- internal/cmd/ske/describe/describe_test.go | 35 +++++++ internal/cmd/ske/kubeconfig/create/create.go | 8 +- .../cmd/ske/kubeconfig/create/create_test.go | 44 +++++++++ internal/cmd/ske/options/options.go | 10 ++ internal/cmd/ske/options/options_test.go | 94 +++++++++++++++++++ 16 files changed, 435 insertions(+), 26 deletions(-) diff --git a/internal/cmd/ske/cluster/create/create.go b/internal/cmd/ske/cluster/create/create.go index 967b1fe37..014e69bc2 100644 --- a/internal/cmd/ske/cluster/create/create.go +++ b/internal/cmd/ske/cluster/create/create.go @@ -143,7 +143,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) @@ -197,10 +197,14 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient *ske.APIClie return req } -func outputResult(p *print.Printer, model *inputModel, projectLabel string, resp *ske.Cluster) error { - switch model.OutputFormat { +func outputResult(p *print.Printer, outputFormat string, async bool, projectLabel string, cluster *ske.Cluster) error { + if cluster == nil { + return fmt.Errorf("cluster is nil") + } + + switch outputFormat { case print.JSONOutputFormat: - details, err := json.MarshalIndent(resp, "", " ") + details, err := json.MarshalIndent(cluster, "", " ") if err != nil { return fmt.Errorf("marshal SKE cluster: %w", err) } @@ -208,7 +212,7 @@ func outputResult(p *print.Printer, model *inputModel, projectLabel string, resp return nil case print.YAMLOutputFormat: - details, err := yaml.MarshalWithOptions(resp, yaml.IndentSequence(true), yaml.UseJSONMarshaler()) + details, err := yaml.MarshalWithOptions(cluster, yaml.IndentSequence(true), yaml.UseJSONMarshaler()) if err != nil { return fmt.Errorf("marshal SKE cluster: %w", err) } @@ -217,10 +221,10 @@ 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 cluster for project %q. Cluster name: %s\n", operationState, projectLabel, utils.PtrString(resp.Name)) + p.Outputf("%s cluster for project %q. Cluster name: %s\n", operationState, projectLabel, utils.PtrString(cluster.Name)) return nil } } diff --git a/internal/cmd/ske/cluster/create/create_test.go b/internal/cmd/ske/cluster/create/create_test.go index 7e2496c84..6e2e92b3f 100644 --- a/internal/cmd/ske/cluster/create/create_test.go +++ b/internal/cmd/ske/cluster/create/create_test.go @@ -315,3 +315,39 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + async bool + projectLabel string + cluster *ske.Cluster + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: true, + }, + { + name: "empty cluster", + args: args{ + cluster: &ske.Cluster{}, + }, + 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.cluster); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/ske/cluster/describe/describe.go b/internal/cmd/ske/cluster/describe/describe.go index 9740abf39..52fb7af4d 100644 --- a/internal/cmd/ske/cluster/describe/describe.go +++ b/internal/cmd/ske/cluster/describe/describe.go @@ -97,6 +97,10 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient *ske.APIClie } func outputResult(p *print.Printer, outputFormat string, cluster *ske.Cluster) error { + if cluster == nil { + return fmt.Errorf("cluster is nil") + } + switch outputFormat { case print.JSONOutputFormat: details, err := json.MarshalIndent(cluster, "", " ") @@ -123,10 +127,14 @@ func outputResult(p *print.Printer, outputFormat string, cluster *ske.Cluster) e table := tables.NewTable() table.AddRow("NAME", utils.PtrString(cluster.Name)) table.AddSeparator() - table.AddRow("STATE", utils.PtrString(cluster.Status.Aggregated)) - table.AddSeparator() - table.AddRow("VERSION", utils.PtrString(cluster.Kubernetes.Version)) - table.AddSeparator() + if cluster.HasStatus() { + table.AddRow("STATE", utils.PtrString(cluster.Status.Aggregated)) + table.AddSeparator() + } + if cluster.Kubernetes != nil { + table.AddRow("VERSION", utils.PtrString(cluster.Kubernetes.Version)) + table.AddSeparator() + } table.AddRow("ACL", acl) err := table.Display(p) if err != nil { diff --git a/internal/cmd/ske/cluster/describe/describe_test.go b/internal/cmd/ske/cluster/describe/describe_test.go index 335bb39c0..99e644af4 100644 --- a/internal/cmd/ske/cluster/describe/describe_test.go +++ b/internal/cmd/ske/cluster/describe/describe_test.go @@ -206,3 +206,37 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + cluster *ske.Cluster + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: true, + }, + { + name: "empty cluster", + args: args{ + cluster: &ske.Cluster{}, + }, + 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.cluster); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/ske/cluster/generate-payload/generate_payload.go b/internal/cmd/ske/cluster/generate-payload/generate_payload.go index 461842528..299b5e9ab 100644 --- a/internal/cmd/ske/cluster/generate-payload/generate_payload.go +++ b/internal/cmd/ske/cluster/generate-payload/generate_payload.go @@ -134,6 +134,10 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient *ske.APIClie } func outputResult(p *print.Printer, filePath *string, payload *ske.CreateOrUpdateClusterPayload) error { + if payload == nil { + return fmt.Errorf("payload is nil") + } + payloadBytes, err := json.MarshalIndent(*payload, "", " ") if err != nil { return fmt.Errorf("marshal payload: %w", err) diff --git a/internal/cmd/ske/cluster/generate-payload/generate_payload_test.go b/internal/cmd/ske/cluster/generate-payload/generate_payload_test.go index 3cf60e949..5516ccf26 100644 --- a/internal/cmd/ske/cluster/generate-payload/generate_payload_test.go +++ b/internal/cmd/ske/cluster/generate-payload/generate_payload_test.go @@ -208,3 +208,45 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + filePath *string + payload *ske.CreateOrUpdateClusterPayload + } + filePathDummy := "/dummy.txt" + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: true, + }, + { + name: "missing payload", + args: args{ + filePath: &filePathDummy, + }, + wantErr: true, + }, + { + name: "missing file path", + args: args{ + payload: &ske.CreateOrUpdateClusterPayload{}, + }, + 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.filePath, tt.args.payload); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/ske/cluster/list/list.go b/internal/cmd/ske/cluster/list/list.go index 0a39805aa..8f6dd8829 100644 --- a/internal/cmd/ske/cluster/list/list.go +++ b/internal/cmd/ske/cluster/list/list.go @@ -174,11 +174,22 @@ func outputResult(p *print.Printer, outputFormat string, clusters []ske.Cluster) if c.Extensions != nil && c.Extensions.Argus != nil && *c.Extensions.Argus.Enabled { monitoring = "Enabled" } + statusAggregated, kubernetesVersion := "", "" + if c.HasStatus() { + statusAggregated = utils.PtrString(c.Status.Aggregated) + } + if c.Kubernetes != nil { + kubernetesVersion = utils.PtrString(c.Kubernetes.Version) + } + countNodepools := 0 + if c.Nodepools != nil { + countNodepools = len(*c.Nodepools) + } table.AddRow( utils.PtrString(c.Name), - utils.PtrString(c.Status.Aggregated), - utils.PtrString(c.Kubernetes.Version), - len(*c.Nodepools), + statusAggregated, + kubernetesVersion, + countNodepools, monitoring, ) } diff --git a/internal/cmd/ske/cluster/list/list_test.go b/internal/cmd/ske/cluster/list/list_test.go index 9e9a8c6a3..2959d6be6 100644 --- a/internal/cmd/ske/cluster/list/list_test.go +++ b/internal/cmd/ske/cluster/list/list_test.go @@ -186,3 +186,44 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + clusters []ske.Cluster + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: false, + }, + { + name: "empty clusters slice", + args: args{ + clusters: []ske.Cluster{}, + }, + wantErr: false, + }, + { + name: "empty cluster in clusters slice", + args: args{ + clusters: []ske.Cluster{{}}, + }, + 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.clusters); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/ske/cluster/update/update.go b/internal/cmd/ske/cluster/update/update.go index 48d73784b..c407b7fb8 100644 --- a/internal/cmd/ske/cluster/update/update.go +++ b/internal/cmd/ske/cluster/update/update.go @@ -105,7 +105,7 @@ func NewCmd(p *print.Printer) *cobra.Command { s.Stop() } - return outputResult(p, model, resp) + return outputResult(p, model.OutputFormat, model.Async, model.ClusterName, resp) }, } configureFlags(cmd) @@ -159,10 +159,14 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient *ske.APIClie return req } -func outputResult(p *print.Printer, model *inputModel, resp *ske.Cluster) error { - switch model.OutputFormat { +func outputResult(p *print.Printer, outputFormat string, async bool, clusterName string, cluster *ske.Cluster) error { + if cluster == nil { + return fmt.Errorf("cluster is nil") + } + + switch outputFormat { case print.JSONOutputFormat: - details, err := json.MarshalIndent(resp, "", " ") + details, err := json.MarshalIndent(cluster, "", " ") if err != nil { return fmt.Errorf("marshal SKE cluster: %w", err) } @@ -170,7 +174,7 @@ func outputResult(p *print.Printer, model *inputModel, resp *ske.Cluster) error return nil case print.YAMLOutputFormat: - details, err := yaml.MarshalWithOptions(resp, yaml.IndentSequence(true), yaml.UseJSONMarshaler()) + details, err := yaml.MarshalWithOptions(cluster, yaml.IndentSequence(true), yaml.UseJSONMarshaler()) if err != nil { return fmt.Errorf("marshal SKE cluster: %w", err) } @@ -179,10 +183,10 @@ func outputResult(p *print.Printer, model *inputModel, resp *ske.Cluster) error return nil default: operationState := "Updated" - if model.Async { + if async { operationState = "Triggered update of" } - p.Info("%s cluster %q\n", operationState, model.ClusterName) + p.Info("%s cluster %q\n", operationState, clusterName) return nil } } diff --git a/internal/cmd/ske/cluster/update/update_test.go b/internal/cmd/ske/cluster/update/update_test.go index 32a1cfed5..53bce8b78 100644 --- a/internal/cmd/ske/cluster/update/update_test.go +++ b/internal/cmd/ske/cluster/update/update_test.go @@ -295,3 +295,39 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + async bool + clusterName string + cluster *ske.Cluster + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: true, + }, + { + name: "empty cluster", + args: args{ + cluster: &ske.Cluster{}, + }, + 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.clusterName, tt.args.cluster); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/ske/describe/describe.go b/internal/cmd/ske/describe/describe.go index 025cd78a1..4912b997d 100644 --- a/internal/cmd/ske/describe/describe.go +++ b/internal/cmd/ske/describe/describe.go @@ -87,6 +87,10 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient *serviceenab } func outputResult(p *print.Printer, outputFormat string, project *serviceenablement.ServiceStatus, projectId string) error { + if project == nil { + return fmt.Errorf("project is nil") + } + switch outputFormat { case print.JSONOutputFormat: details, err := json.MarshalIndent(project, "", " ") @@ -108,7 +112,9 @@ func outputResult(p *print.Printer, outputFormat string, project *serviceenablem table := tables.NewTable() table.AddRow("ID", projectId) table.AddSeparator() - table.AddRow("STATE", utils.PtrString(project.State)) + if project.HasState() { + table.AddRow("STATE", utils.PtrString(project.State)) + } err := table.Display(p) if err != nil { return fmt.Errorf("render table: %w", err) diff --git a/internal/cmd/ske/describe/describe_test.go b/internal/cmd/ske/describe/describe_test.go index fe8e39e71..983e061e5 100644 --- a/internal/cmd/ske/describe/describe_test.go +++ b/internal/cmd/ske/describe/describe_test.go @@ -169,3 +169,38 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + outputFormat string + project *serviceenablement.ServiceStatus + projectId string + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: true, + }, + { + name: "empty project", + args: args{ + project: &serviceenablement.ServiceStatus{}, + }, + 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.project, tt.args.projectId); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/ske/kubeconfig/create/create.go b/internal/cmd/ske/kubeconfig/create/create.go index a8fb68437..8677b033c 100644 --- a/internal/cmd/ske/kubeconfig/create/create.go +++ b/internal/cmd/ske/kubeconfig/create/create.go @@ -159,7 +159,7 @@ func NewCmd(p *print.Printer) *cobra.Command { p.Outputf("\nSet kubectl context to %s with: kubectl config use-context %s\n", model.ClusterName, model.ClusterName) } - return outputResult(p, model, kubeconfigPath, respKubeconfig, respLogin) + return outputResult(p, model.OutputFormat, model.ClusterName, kubeconfigPath, respKubeconfig, respLogin) }, } configureFlags(cmd) @@ -242,8 +242,8 @@ func buildRequestLogin(ctx context.Context, model *inputModel, apiClient *ske.AP return apiClient.GetLoginKubeconfig(ctx, model.ProjectId, model.ClusterName), nil } -func outputResult(p *print.Printer, model *inputModel, kubeconfigPath string, respKubeconfig *ske.Kubeconfig, respLogin *ske.LoginKubeconfig) error { - switch model.OutputFormat { +func outputResult(p *print.Printer, outputFormat, clusterName, kubeconfigPath string, respKubeconfig *ske.Kubeconfig, respLogin *ske.LoginKubeconfig) error { + switch outputFormat { case print.JSONOutputFormat: var err error var details []byte @@ -277,7 +277,7 @@ func outputResult(p *print.Printer, model *inputModel, kubeconfigPath string, re if respKubeconfig != nil { expiration = fmt.Sprintf(", with expiration date %v (UTC)", utils.ConvertTimePToDateTimeString(respKubeconfig.ExpirationTimestamp)) } - p.Outputf("Updated kubeconfig file for cluster %s in %q%s\n", model.ClusterName, kubeconfigPath, expiration) + p.Outputf("Updated kubeconfig file for cluster %s in %q%s\n", clusterName, kubeconfigPath, expiration) return nil } diff --git a/internal/cmd/ske/kubeconfig/create/create_test.go b/internal/cmd/ske/kubeconfig/create/create_test.go index 85cfe1560..9743067f8 100644 --- a/internal/cmd/ske/kubeconfig/create/create_test.go +++ b/internal/cmd/ske/kubeconfig/create/create_test.go @@ -289,3 +289,47 @@ func TestBuildRequestCreate(t *testing.T) { }) } } + +func Test_outputResult(t *testing.T) { + type args struct { + outputFormat string + clusterName string + kubeconfigPath string + respKubeconfig *ske.Kubeconfig + respLogin *ske.LoginKubeconfig + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: false, + }, + { + name: "missing kubeconfig", + args: args{ + respLogin: &ske.LoginKubeconfig{}, + }, + wantErr: false, + }, + { + name: "missing login", + args: args{ + respKubeconfig: &ske.Kubeconfig{}, + }, + 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.clusterName, tt.args.kubeconfigPath, tt.args.respKubeconfig, tt.args.respLogin); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/cmd/ske/options/options.go b/internal/cmd/ske/options/options.go index 82105321c..419721d29 100644 --- a/internal/cmd/ske/options/options.go +++ b/internal/cmd/ske/options/options.go @@ -136,6 +136,12 @@ func buildRequest(ctx context.Context, apiClient *ske.APIClient) ske.ApiListProv } func outputResult(p *print.Printer, model *inputModel, options *ske.ProviderOptions) error { + if model == nil || model.GlobalFlagModel == nil { + return fmt.Errorf("model is nil") + } else if options == nil { + return fmt.Errorf("options is nil") + } + // filter output based on the flags if !model.AvailabilityZones { options.AvailabilityZones = nil @@ -179,6 +185,10 @@ func outputResult(p *print.Printer, model *inputModel, options *ske.ProviderOpti } func outputResultAsTable(p *print.Printer, options *ske.ProviderOptions) error { + if options == nil { + return fmt.Errorf("options is nil") + } + content := []tables.Table{} if options.AvailabilityZones != nil && len(*options.AvailabilityZones) != 0 { content = append(content, buildAvailabilityZonesTable(options)) diff --git a/internal/cmd/ske/options/options_test.go b/internal/cmd/ske/options/options_test.go index d6045f823..6b1c1ae93 100644 --- a/internal/cmd/ske/options/options_test.go +++ b/internal/cmd/ske/options/options_test.go @@ -186,3 +186,97 @@ func TestBuildRequest(t *testing.T) { }) } } + +func TestOutputResult(t *testing.T) { + type args struct { + model *inputModel + options *ske.ProviderOptions + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: true, + }, + { + name: "missing model", + args: args{ + options: &ske.ProviderOptions{}, + }, + wantErr: true, + }, + { + name: "missing options", + args: args{ + model: &inputModel{ + GlobalFlagModel: &globalflags.GlobalFlagModel{}, + }, + }, + wantErr: true, + }, + { + name: "missing global flags in model", + args: args{ + model: &inputModel{}, + options: &ske.ProviderOptions{}, + }, + wantErr: true, + }, + { + name: "set model and options", + args: args{ + model: &inputModel{ + GlobalFlagModel: &globalflags.GlobalFlagModel{}, + }, + options: &ske.ProviderOptions{}, + }, + 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.options); (err != nil) != tt.wantErr { + t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestOutputResultAsTable(t *testing.T) { + type args struct { + options *ske.ProviderOptions + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "empty", + args: args{}, + wantErr: true, + }, + { + name: "empty options", + args: args{ + options: &ske.ProviderOptions{}, + }, + wantErr: false, + }, + } + p := print.NewPrinter() + p.Cmd = NewCmd(p) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := outputResultAsTable(p, tt.args.options); (err != nil) != tt.wantErr { + t.Errorf("outputResultAsTable() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +}