From ec6c3b0a228a744309b2018cb786117d881ae97c Mon Sep 17 00:00:00 2001 From: Toni Kangas Date: Mon, 16 Oct 2023 20:09:01 +0300 Subject: [PATCH 1/4] refactor(account): remove unnecessary variable assignment --- internal/commands/account/show.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/commands/account/show.go b/internal/commands/account/show.go index 188804354..cb9aef67a 100644 --- a/internal/commands/account/show.go +++ b/internal/commands/account/show.go @@ -27,7 +27,7 @@ func (s *showCommand) ExecuteWithoutArguments(exec commands.Executor) (output.Ou return nil, err } - details, err := output.Details{ + details := output.Details{ Sections: []output.DetailSection{ { Rows: []output.DetailRow{ @@ -80,12 +80,12 @@ func (s *showCommand) ExecuteWithoutArguments(exec commands.Executor) (output.Ou }, }, }, - }, nil + } return output.MarshaledWithHumanOutput{ Value: account, Output: details, - }, err + }, nil } func formatCredits(credits float64) string { From ec778338737ba0de3779d6a17dfc93bc94eccc5d Mon Sep 17 00:00:00 2001 From: Toni Kangas Date: Mon, 16 Oct 2023 20:25:24 +0300 Subject: [PATCH 2/4] fix(ipaddress): show full API response in marshaled outputs --- CHANGELOG.md | 4 ++ internal/commands/ipaddress/list.go | 80 +++++++++++++++-------------- internal/commands/ipaddress/show.go | 20 +++++--- 3 files changed, 59 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b38b8108..98322d798 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,8 +24,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - In JSON and YAML output of `kubernetes list`: display full API response. - **Breaking**: In JSON and YAML output of `database list`: display the full API response. Value of `title` is not replaced with value from `name`, if `title` is empty. - **Breaking**: In JSON and YAML output of `database types`: display the full API response. This changes the top level datatype from list to object, where keys are the available database type, e.g., `pg` and `mysql`. +- **Breaking**: In JSON and YAML output of `ip-address list`: display full API response. This changes `partofplan` key to `part_of_plan` and `ptrrecord` key to `ptr_record`. - In human readable output of `kubernetes show` command, show node-groups as table. Node-group details are available with `kubernetes nodegroup show` command. +## Fixed +- **Breaking**: In JSON and YAML output of `ip-address show`: use same JSON keys as in API documentation. This removes `credits` key that was used in place of `floating`. + ## Removed - **Breaking**: Remove `database connection list` and `database connection cancel` commands in favor of `database session` counterparts - **Breaking**: In JSON and YAML output of `database properties * show`: pass-through the API response. This removes `key` field from the output. diff --git a/internal/commands/ipaddress/list.go b/internal/commands/ipaddress/list.go index 7d537cd9d..215e0c21d 100644 --- a/internal/commands/ipaddress/list.go +++ b/internal/commands/ipaddress/list.go @@ -36,45 +36,49 @@ func (s *listCommand) ExecuteWithoutArguments(exec commands.Executor) (output.Ou for i, ipAddress := range ips.IPAddresses { rows[i] = output.TableRow{ipAddress.Address, ipAddress.Access, ipAddress.Family, ipAddress.PartOfPlan, ipAddress.PTRRecord, ipAddress.ServerUUID, ipAddress.Floating, ipAddress.Zone} } - return output.Table{ - Columns: []output.TableColumn{ - { - Header: "Address", - Key: "address", - Colour: ui.DefaultAddressColours, - }, - { - Header: "Access", - Key: "access", - }, - { - Header: "Family", - Key: "family", - }, - { - Header: "Part of Plan", - Key: "partofplan", - Format: format.Boolean, - }, - { - Header: "PTR Record", - Key: "ptrrecord", - }, - { - Header: "Server", - Key: "server", - Colour: ui.DefaultUUUIDColours, - }, - { - Header: "Floating", - Key: "floating", - Format: format.Boolean, - }, - { - Header: "Zone", - Key: "zone", + + return output.MarshaledWithHumanOutput{ + Value: ips.IPAddresses, + Output: output.Table{ + Columns: []output.TableColumn{ + { + Header: "Address", + Key: "address", + Colour: ui.DefaultAddressColours, + }, + { + Header: "Access", + Key: "access", + }, + { + Header: "Family", + Key: "family", + }, + { + Header: "Part of Plan", + Key: "part_of_plan", + Format: format.Boolean, + }, + { + Header: "PTR Record", + Key: "ptr_record", + }, + { + Header: "Server", + Key: "server", + Colour: ui.DefaultUUUIDColours, + }, + { + Header: "Floating", + Key: "floating", + Format: format.Boolean, + }, + { + Header: "Zone", + Key: "zone", + }, }, + Rows: rows, }, - Rows: rows, }, nil } diff --git a/internal/commands/ipaddress/show.go b/internal/commands/ipaddress/show.go index acc257727..20ea7dfa4 100644 --- a/internal/commands/ipaddress/show.go +++ b/internal/commands/ipaddress/show.go @@ -39,25 +39,31 @@ func (s *showCommand) Execute(exec commands.Executor, arg string) (output.Output if err != nil { return nil, err } - return output.Details{ + + details := output.Details{ Sections: []output.DetailSection{ { Rows: []output.DetailRow{ {Title: "Address:", Key: "address", Value: ipAddress.Address, Colour: ui.DefaultAddressColours}, {Title: "Access:", Key: "access", Value: ipAddress.Access}, - {Title: "Family:", Key: "access", Value: ipAddress.Family}, - {Title: "Part of Plan:", Key: "access", Value: ipAddress.PartOfPlan, Format: format.Boolean}, - {Title: "PTR Record:", Key: "access", Value: ipAddress.PTRRecord}, + {Title: "Family:", Key: "family", Value: ipAddress.Family}, + {Title: "Part of Plan:", Key: "part_of_plan", Value: ipAddress.PartOfPlan, Format: format.Boolean}, + {Title: "PTR Record:", Key: "ptr_record", Value: ipAddress.PTRRecord}, { Title: "Server UUID:", - Key: "access", Value: ipAddress.ServerUUID, + Key: "server", Value: ipAddress.ServerUUID, Colour: ui.DefaultUUUIDColours, }, - {Title: "MAC:", Key: "credits", Value: ipAddress.MAC}, - {Title: "Floating:", Key: "credits", Value: ipAddress.Floating, Format: format.Boolean}, + {Title: "MAC:", Key: "mac", Value: ipAddress.MAC}, + {Title: "Floating:", Key: "floating", Value: ipAddress.Floating, Format: format.Boolean}, {Title: "Zone:", Key: "zone", Value: ipAddress.Zone}, }, }, }, + } + + return output.MarshaledWithHumanOutput{ + Value: ipAddress, + Output: details, }, nil } From 9711470590683c6391155e6ef12d3e1ffeec1566 Mon Sep 17 00:00:00 2001 From: Toni Kangas Date: Tue, 17 Oct 2023 14:25:16 +0300 Subject: [PATCH 3/4] test(ipaddress): validate list JSON output is a list --- internal/commands/ipaddress/list.go | 2 +- internal/commands/ipaddress/list_test.go | 46 ++++++++++++++++++++++++ internal/testutils/json_output.go | 26 ++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 internal/commands/ipaddress/list_test.go create mode 100644 internal/testutils/json_output.go diff --git a/internal/commands/ipaddress/list.go b/internal/commands/ipaddress/list.go index 215e0c21d..406ecf1b1 100644 --- a/internal/commands/ipaddress/list.go +++ b/internal/commands/ipaddress/list.go @@ -38,7 +38,7 @@ func (s *listCommand) ExecuteWithoutArguments(exec commands.Executor) (output.Ou } return output.MarshaledWithHumanOutput{ - Value: ips.IPAddresses, + Value: ips, Output: output.Table{ Columns: []output.TableColumn{ { diff --git a/internal/commands/ipaddress/list_test.go b/internal/commands/ipaddress/list_test.go new file mode 100644 index 000000000..a3c62433f --- /dev/null +++ b/internal/commands/ipaddress/list_test.go @@ -0,0 +1,46 @@ +package ipaddress + +import ( + "testing" + + "github.com/UpCloudLtd/upcloud-cli/v2/internal/commands" + "github.com/UpCloudLtd/upcloud-cli/v2/internal/config" + smock "github.com/UpCloudLtd/upcloud-cli/v2/internal/mock" + "github.com/UpCloudLtd/upcloud-cli/v2/internal/mockexecute" + "github.com/UpCloudLtd/upcloud-cli/v2/internal/testutils" + "github.com/stretchr/testify/assert" + + "github.com/UpCloudLtd/upcloud-go-api/v6/upcloud" +) + +func TestList_MarshaledOutput(t *testing.T) { + ipAddress := upcloud.IPAddress{ + Address: "94.237.117.150", + Access: "public", + Family: "IPv4", + PartOfPlan: upcloud.FromBool(true), + PTRRecord: "94-237-117-150.fi-hel1.upcloud.host", + ServerUUID: "005ab220-7ff6-42c9-8615-e4c02eb4104e", + MAC: "ee:1b:db:ca:6b:80", + Floating: upcloud.FromBool(false), + Zone: "fi-hel1", + } + ipAddresses := upcloud.IPAddresses{ + IPAddresses: []upcloud.IPAddress{ + ipAddress, + }, + } + + mService := smock.Service{} + mService.On("GetIPAddresses").Return(&ipAddresses, nil) + + conf := config.New() + conf.Viper().Set(config.KeyOutput, config.ValueOutputJSON) + + command := commands.BuildCommand(ListCommand(), nil, conf) + + output, err := mockexecute.MockExecute(command, &mService, conf) + + assert.Nil(t, err) + testutils.AssertOutputHasType(t, output, &upcloud.IPAddressSlice{}) +} diff --git a/internal/testutils/json_output.go b/internal/testutils/json_output.go new file mode 100644 index 000000000..64cdfb842 --- /dev/null +++ b/internal/testutils/json_output.go @@ -0,0 +1,26 @@ +package testutils + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" +) + +func AssertOutputHasType(t *testing.T, output string, expected interface{}) { + t.Helper() + + err := json.Unmarshal([]byte(output), expected) + if typeError, ok := err.(*json.UnmarshalTypeError); ok { + assert.Nil(t, err, "Expected %t, got %s: %s", expected, typeError.Value, output) + } else { + assert.Nil(t, err) + } +} + +func AssertOutputIsList(t *testing.T, output string) { + t.Helper() + + list := make([]interface{}, 0) + AssertOutputHasType(t, output, &list) +} From c618de419c45e30a963ae194a923b661c83b9656 Mon Sep 17 00:00:00 2001 From: Toni Kangas Date: Tue, 17 Oct 2023 16:28:22 +0300 Subject: [PATCH 4/4] test(kubernetes): validate list output type --- internal/commands/kubernetes/list_test.go | 30 ++++ internal/commands/kubernetes/show_test.go | 165 +++++++++++----------- 2 files changed, 113 insertions(+), 82 deletions(-) create mode 100644 internal/commands/kubernetes/list_test.go diff --git a/internal/commands/kubernetes/list_test.go b/internal/commands/kubernetes/list_test.go new file mode 100644 index 000000000..ac3d1c0df --- /dev/null +++ b/internal/commands/kubernetes/list_test.go @@ -0,0 +1,30 @@ +package kubernetes + +import ( + "testing" + + "github.com/UpCloudLtd/upcloud-cli/v2/internal/commands" + "github.com/UpCloudLtd/upcloud-cli/v2/internal/config" + smock "github.com/UpCloudLtd/upcloud-cli/v2/internal/mock" + "github.com/UpCloudLtd/upcloud-cli/v2/internal/mockexecute" + "github.com/UpCloudLtd/upcloud-cli/v2/internal/testutils" + + "github.com/UpCloudLtd/upcloud-go-api/v6/upcloud" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +func TestList_MarshaledOutput(t *testing.T) { + mService := smock.Service{} + mService.On("GetKubernetesClusters", mock.Anything).Return([]upcloud.KubernetesCluster{testCluster}, nil) + + conf := config.New() + conf.Viper().Set(config.KeyOutput, config.ValueOutputJSON) + + command := commands.BuildCommand(ListCommand(), nil, conf) + + output, err := mockexecute.MockExecute(command, &mService, conf) + + assert.Nil(t, err) + testutils.AssertOutputIsList(t, output) +} diff --git a/internal/commands/kubernetes/show_test.go b/internal/commands/kubernetes/show_test.go index 7bb05669e..4759a794f 100644 --- a/internal/commands/kubernetes/show_test.go +++ b/internal/commands/kubernetes/show_test.go @@ -15,97 +15,98 @@ import ( "github.com/stretchr/testify/mock" ) -func TestShowCommand(t *testing.T) { - text.DisableColors() - cluster1 := upcloud.KubernetesCluster{ - ControlPlaneIPFilter: []string{"10.144.1.100", "10.144.2.0/24"}, - Name: "upcloud-go-sdk-unit-test", - Network: "03a98be3-7daa-443f-bb25-4bc6854b396c", - NetworkCIDR: "172.16.1.0/24", - NodeGroups: []upcloud.KubernetesNodeGroup{ - { - Count: 4, - Labels: []upcloud.Label{ - { - Key: "managedBy", - Value: "upcloud-go-sdk-unit-test", - }, - { - Key: "another", - Value: "label-thing", - }, +var testCluster = upcloud.KubernetesCluster{ + ControlPlaneIPFilter: []string{"10.144.1.100", "10.144.2.0/24"}, + Name: "upcloud-upctl-unit-test", + Network: "03a98be3-7daa-443f-bb25-4bc6854b396c", + NetworkCIDR: "172.16.1.0/24", + NodeGroups: []upcloud.KubernetesNodeGroup{ + { + Count: 4, + Labels: []upcloud.Label{ + { + Key: "managedBy", + Value: "upcloud-go-sdk-unit-test", + }, + { + Key: "another", + Value: "label-thing", + }, + }, + Name: "upcloud-go-sdk-unit-test", + Plan: "2xCPU-4GB", + State: upcloud.KubernetesNodeGroupStateRunning, + KubeletArgs: []upcloud.KubernetesKubeletArg{ + { + Key: "somekubeletkey", + Value: "somekubeletvalue", + }, + }, + Taints: []upcloud.KubernetesTaint{ + { + Effect: "NoExecute", + Key: "sometaintkey", + Value: "sometaintvalue", + }, + { + Effect: "NoExecute", + Key: "sometaintkey", + Value: "sometaintvalue", }, - Name: "upcloud-go-sdk-unit-test", - Plan: "2xCPU-4GB", - State: upcloud.KubernetesNodeGroupStateRunning, - KubeletArgs: []upcloud.KubernetesKubeletArg{ - { - Key: "somekubeletkey", - Value: "somekubeletvalue", - }, + { + Effect: "NoExecute", + Key: "sometaintkey", + Value: "sometaintvalue", }, - Taints: []upcloud.KubernetesTaint{ - { - Effect: "NoExecute", - Key: "sometaintkey", - Value: "sometaintvalue", - }, - { - Effect: "NoExecute", - Key: "sometaintkey", - Value: "sometaintvalue", - }, - { - Effect: "NoExecute", - Key: "sometaintkey", - Value: "sometaintvalue", - }, + }, + Storage: "storage-uuid", + SSHKeys: []string{"somekey"}, + UtilityNetworkAccess: true, + }, { + Count: 8, + Labels: []upcloud.Label{ + { + Key: "managedBy", + Value: "upcloud-go-sdk-unit-test-2", }, - Storage: "storage-uuid", - SSHKeys: []string{"somekey"}, - UtilityNetworkAccess: true, - }, { - Count: 8, - Labels: []upcloud.Label{ - { - Key: "managedBy", - Value: "upcloud-go-sdk-unit-test-2", - }, - { - Key: "another2", - Value: "label-thing-2", - }, + { + Key: "another2", + Value: "label-thing-2", }, - Name: "upcloud-go-sdk-unit-test-2", - Plan: "4xCPU-8GB", - State: upcloud.KubernetesNodeGroupStatePending, - KubeletArgs: []upcloud.KubernetesKubeletArg{ - { - Key: "somekubeletkey2", - Value: "somekubeletvalue2", - }, + }, + Name: "upcloud-go-sdk-unit-test-2", + Plan: "4xCPU-8GB", + State: upcloud.KubernetesNodeGroupStatePending, + KubeletArgs: []upcloud.KubernetesKubeletArg{ + { + Key: "somekubeletkey2", + Value: "somekubeletvalue2", }, - Taints: []upcloud.KubernetesTaint{ - { - Effect: "NoSchedule", - Key: "sometaintkey2", - Value: "sometaintvalue2", - }, + }, + Taints: []upcloud.KubernetesTaint{ + { + Effect: "NoSchedule", + Key: "sometaintkey2", + Value: "sometaintvalue2", }, - Storage: "storage-uuid-2", - SSHKeys: []string{"somekey2"}, - UtilityNetworkAccess: false, }, + Storage: "storage-uuid-2", + SSHKeys: []string{"somekey2"}, + UtilityNetworkAccess: false, }, - State: upcloud.KubernetesClusterStateRunning, - UUID: "0ddab8f4-97c0-4222-91ba-85a4fff7499b", - Zone: "de-fra1", - } + }, + State: upcloud.KubernetesClusterStateRunning, + UUID: "0ddab8f4-97c0-4222-91ba-85a4fff7499b", + Zone: "de-fra1", +} + +func TestShowCommand(t *testing.T) { + text.DisableColors() expected := ` Overview: UUID: 0ddab8f4-97c0-4222-91ba-85a4fff7499b - Name: upcloud-go-sdk-unit-test + Name: upcloud-upctl-unit-test Network UUID: 03a98be3-7daa-443f-bb25-4bc6854b396c Network name: Test network Network CIDR: 172.16.1.0/24 @@ -125,8 +126,8 @@ func TestShowCommand(t *testing.T) { ` mService := smock.Service{} - mService.On("GetKubernetesClusters", mock.Anything).Return([]upcloud.KubernetesCluster{cluster1}, nil) - mService.On("GetKubernetesCluster", mock.Anything).Return(&cluster1, nil) + mService.On("GetKubernetesClusters", mock.Anything).Return([]upcloud.KubernetesCluster{testCluster}, nil) + mService.On("GetKubernetesCluster", mock.Anything).Return(&testCluster, nil) mService.On("GetNetworkDetails", mock.Anything).Return(&upcloud.Network{Name: "Test network"}, nil) mService.On("GetStorageDetails", mock.Anything).Return(&upcloud.StorageDetails{Storage: upcloud.Storage{Title: "Test storage"}}, nil) @@ -139,7 +140,7 @@ func TestShowCommand(t *testing.T) { t.Fatal(err) } - command.Cobra().SetArgs([]string{cluster1.UUID}) + command.Cobra().SetArgs([]string{testCluster.UUID}) output, err := mockexecute.MockExecute(command, &mService, conf) assert.NoError(t, err)