From 6a166e91a6f28334d72121047cc7a1bd18a48835 Mon Sep 17 00:00:00 2001 From: Stephen Kitt Date: Thu, 22 Aug 2024 11:45:24 +0200 Subject: [PATCH] Use joined errors Go 1.20 added support for joined errors; using those instead of error slices allows typical nil/non-nil semantics to be used, and allows all error messages to be shown simultaneously when asserting. This gets rid of HandleErrorList, and with it the implied generic/validation error distinction. To handle that, HandleError takes an additional error type parameter, making the distinction explicit. Signed-off-by: Stephen Kitt --- cmd/skupper/main.go | 2 +- internal/cmd/generate-doc/main.go | 5 +- internal/cmd/skupper/common/command.go | 8 +- internal/cmd/skupper/common/command_test.go | 2 +- .../common/testutils/validate_input.go | 21 +++ .../cmd/skupper/common/utils/handle_error.go | 28 +--- .../connector/kube/connector_create.go | 11 +- .../connector/kube/connector_create_test.go | 108 ++++++--------- .../connector/kube/connector_delete.go | 7 +- .../connector/kube/connector_delete_test.go | 52 ++++--- .../connector/kube/connector_status.go | 11 +- .../connector/kube/connector_status_test.go | 43 +++--- .../connector/kube/connector_update.go | 11 +- .../connector/kube/connector_update_test.go | 128 +++++++----------- .../connector/nonkube/connector_create.go | 5 +- .../nonkube/connector_create_test.go | 125 ++++++++--------- .../connector/nonkube/connector_delete.go | 5 +- .../nonkube/connector_delete_test.go | 59 ++++---- .../connector/nonkube/connector_status.go | 5 +- .../nonkube/connector_status_test.go | 65 ++++----- .../connector/nonkube/connector_update.go | 5 +- .../nonkube/connector_update_test.go | 103 +++++++------- internal/cmd/skupper/debug/kube/debug.go | 3 +- internal/cmd/skupper/debug/nonkube/debug.go | 3 +- internal/cmd/skupper/link/kube/link_delete.go | 10 +- .../cmd/skupper/link/kube/link_delete_test.go | 34 +++-- .../cmd/skupper/link/kube/link_generate.go | 20 +-- .../skupper/link/kube/link_generate_test.go | 62 ++++----- internal/cmd/skupper/link/kube/link_status.go | 12 +- .../cmd/skupper/link/kube/link_status_test.go | 26 ++-- internal/cmd/skupper/link/kube/link_update.go | 12 +- .../cmd/skupper/link/kube/link_update_test.go | 47 +++---- .../cmd/skupper/link/nonkube/link_delete.go | 5 +- .../cmd/skupper/link/nonkube/link_generate.go | 5 +- .../cmd/skupper/link/nonkube/link_status.go | 5 +- .../cmd/skupper/link/nonkube/link_update.go | 5 +- .../skupper/listener/kube/listener_create.go | 14 +- .../listener/kube/listener_create_test.go | 118 ++++++++-------- .../skupper/listener/kube/listener_delete.go | 7 +- .../listener/kube/listener_delete_test.go | 51 ++++--- .../skupper/listener/kube/listener_status.go | 11 +- .../listener/kube/listener_status_test.go | 48 +++---- .../skupper/listener/kube/listener_update.go | 11 +- .../listener/kube/listener_update_test.go | 71 +++++----- .../listener/nonkube/listener_create.go | 5 +- .../listener/nonkube/listener_create_test.go | 117 ++++++++-------- .../listener/nonkube/listener_delete.go | 5 +- .../listener/nonkube/listener_delete_test.go | 59 ++++---- .../listener/nonkube/listener_status.go | 5 +- .../listener/nonkube/listener_status_test.go | 65 ++++----- .../listener/nonkube/listener_update.go | 5 +- .../listener/nonkube/listener_update_test.go | 103 +++++++------- internal/cmd/skupper/site/kube/site_create.go | 7 +- .../cmd/skupper/site/kube/site_create_test.go | 101 ++++++-------- internal/cmd/skupper/site/kube/site_delete.go | 7 +- .../cmd/skupper/site/kube/site_delete_test.go | 42 +++--- internal/cmd/skupper/site/kube/site_status.go | 11 +- .../cmd/skupper/site/kube/site_status_test.go | 13 +- internal/cmd/skupper/site/kube/site_update.go | 10 +- .../cmd/skupper/site/kube/site_update_test.go | 71 ++++------ .../cmd/skupper/site/nonkube/site_create.go | 5 +- .../skupper/site/nonkube/site_create_test.go | 79 +++++------ .../cmd/skupper/site/nonkube/site_delete.go | 5 +- .../skupper/site/nonkube/site_delete_test.go | 59 ++++---- .../cmd/skupper/site/nonkube/site_status.go | 5 +- .../skupper/site/nonkube/site_status_test.go | 55 ++++---- .../cmd/skupper/site/nonkube/site_update.go | 5 +- .../skupper/site/nonkube/site_update_test.go | 79 +++++------ .../cmd/skupper/system/kube/system_reload.go | 3 +- .../cmd/skupper/system/kube/system_setup.go | 3 +- .../cmd/skupper/system/kube/system_start.go | 3 +- .../cmd/skupper/system/kube/system_stop.go | 3 +- .../skupper/system/kube/system_teardown.go | 3 +- .../skupper/system/nonkube/system_reload.go | 11 +- .../system/nonkube/system_reload_test.go | 24 ++-- .../skupper/system/nonkube/system_setup.go | 5 +- .../system/nonkube/system_setup_test.go | 30 ++-- .../skupper/system/nonkube/system_start.go | 11 +- .../system/nonkube/system_start_test.go | 22 ++- .../cmd/skupper/system/nonkube/system_stop.go | 11 +- .../system/nonkube/system_stop_test.go | 20 ++- .../skupper/system/nonkube/system_teardown.go | 11 +- .../system/nonkube/system_teardown_test.go | 19 ++- .../cmd/skupper/token/kube/token_issue.go | 11 +- .../skupper/token/kube/token_issue_test.go | 40 +++--- .../cmd/skupper/token/kube/token_redeem.go | 7 +- .../skupper/token/kube/token_redeem_test.go | 48 +++---- .../cmd/skupper/token/nonkube/token_issue.go | 5 +- .../cmd/skupper/token/nonkube/token_redeem.go | 5 +- internal/cmd/skupper/version/kube/version.go | 8 +- .../cmd/skupper/version/kube/version_test.go | 30 ++-- .../cmd/skupper/version/nonkube/version.go | 5 +- .../skupper/version/nonkube/version_test.go | 30 ++-- internal/nonkube/bundle/extract_test.go | 13 +- internal/nonkube/bundle/tarball_test.go | 13 +- .../nonkube/client/compat/container_test.go | 5 +- 96 files changed, 1250 insertions(+), 1456 deletions(-) create mode 100644 internal/cmd/skupper/common/testutils/validate_input.go diff --git a/cmd/skupper/main.go b/cmd/skupper/main.go index 2810395e6..d4ccdc075 100644 --- a/cmd/skupper/main.go +++ b/cmd/skupper/main.go @@ -13,5 +13,5 @@ func main() { rootCmd := root.NewSkupperRootCommand() err := rootCmd.Execute() - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) } diff --git a/internal/cmd/generate-doc/main.go b/internal/cmd/generate-doc/main.go index b0eee0d0a..eb7832cfe 100644 --- a/internal/cmd/generate-doc/main.go +++ b/internal/cmd/generate-doc/main.go @@ -2,9 +2,10 @@ package main import ( "fmt" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" "os" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/root" "github.com/spf13/cobra/doc" ) @@ -15,7 +16,7 @@ func main() { fmt.Printf("%s\n\nUsage: generate-doc ./docsoutput\n", err) os.Exit(1) } - utils.HandleError(doc.GenMarkdownTree(root.NewSkupperRootCommand(), path)) + utils.HandleError(utils.GenericError, doc.GenMarkdownTree(root.NewSkupperRootCommand(), path)) } diff --git a/internal/cmd/skupper/common/command.go b/internal/cmd/skupper/common/command.go index feea41b1f..a4c298fe3 100644 --- a/internal/cmd/skupper/common/command.go +++ b/internal/cmd/skupper/common/command.go @@ -10,7 +10,7 @@ import ( type SkupperCommand interface { NewClient(cobraCommand *cobra.Command, args []string) - ValidateInput(args []string) []error + ValidateInput(args []string) error InputToOptions() Run() error WaitUntil() error @@ -52,12 +52,12 @@ func ConfigureCobraCommand(configuredPlatform types.Platform, description Skuppe return nil }, Run: func(cmd *cobra.Command, args []string) { - utils.HandleErrorList(skupperCommand.ValidateInput(args)) + utils.HandleError(utils.ValidationError, skupperCommand.ValidateInput(args)) skupperCommand.InputToOptions() - utils.HandleError(skupperCommand.Run()) + utils.HandleError(utils.GenericError, skupperCommand.Run()) }, PostRun: func(cmd *cobra.Command, args []string) { - utils.HandleError(skupperCommand.WaitUntil()) + utils.HandleError(utils.GenericError, skupperCommand.WaitUntil()) }, } diff --git a/internal/cmd/skupper/common/command_test.go b/internal/cmd/skupper/common/command_test.go index d26ff6fb0..aef732536 100644 --- a/internal/cmd/skupper/common/command_test.go +++ b/internal/cmd/skupper/common/command_test.go @@ -20,7 +20,7 @@ func (m *MockSkupperCommand) NewClient(cmd *cobra.Command, args []string) { m.CalledNewClient = true } -func (m *MockSkupperCommand) ValidateInput(args []string) []error { +func (m *MockSkupperCommand) ValidateInput(args []string) error { m.CalledValidateInput = true return nil } diff --git a/internal/cmd/skupper/common/testutils/validate_input.go b/internal/cmd/skupper/common/testutils/validate_input.go new file mode 100644 index 000000000..810d23c2a --- /dev/null +++ b/internal/cmd/skupper/common/testutils/validate_input.go @@ -0,0 +1,21 @@ +package testutils + +import ( + "testing" + + "gotest.tools/v3/assert" +) + +type inputValidatingCommand interface { + ValidateInput(args []string) error +} + +func CheckValidateInput(t *testing.T, command inputValidatingCommand, expectedError string, args []string) { + actualError := command.ValidateInput(args) + + if expectedError == "" { + assert.NilError(t, actualError) + } else { + assert.Error(t, actualError, expectedError) + } +} diff --git a/internal/cmd/skupper/common/utils/handle_error.go b/internal/cmd/skupper/common/utils/handle_error.go index 51623f213..a88d0cc2d 100644 --- a/internal/cmd/skupper/common/utils/handle_error.go +++ b/internal/cmd/skupper/common/utils/handle_error.go @@ -5,32 +5,16 @@ import ( "syscall" ) +type ErrorType int + const ( - GenericError = 1 - ValidationError = 2 + GenericError ErrorType = 1 + ValidationError ErrorType = 2 ) -func HandleError(err error) { +func HandleError(errType ErrorType, err error) { if err != nil { fmt.Println(err) - syscall.Exit(GenericError) - } -} - -func HandleErrorList(errList []error) { - if errList != nil && len(errList) > 0 { - for _, err := range errList { - fmt.Println(err) - } - - syscall.Exit(ValidationError) - } -} - -func ErrorsToMessages(errs []error) []string { - messages := make([]string, len(errs)) - for i, err := range errs { - messages[i] = err.Error() + syscall.Exit(int(errType)) } - return messages } diff --git a/internal/cmd/skupper/connector/kube/connector_create.go b/internal/cmd/skupper/connector/kube/connector_create.go index eca7875a7..60eb24886 100644 --- a/internal/cmd/skupper/connector/kube/connector_create.go +++ b/internal/cmd/skupper/connector/kube/connector_create.go @@ -2,6 +2,7 @@ package kube import ( "context" + "errors" "fmt" "k8s.io/apimachinery/pkg/api/meta" "strconv" @@ -16,7 +17,7 @@ import ( "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" skupperv2alpha1 "github.com/skupperproject/skupper/pkg/generated/client/clientset/versioned/typed/skupper/v2alpha1" "github.com/spf13/cobra" - "k8s.io/apimachinery/pkg/api/errors" + k8serrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" ) @@ -47,14 +48,14 @@ func NewCmdConnectorCreate() *CmdConnectorCreate { func (cmd *CmdConnectorCreate) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.client = cli.GetSkupperClient().SkupperV2alpha1() cmd.namespace = cli.Namespace cmd.KubeClient = cli.Kube } -func (cmd *CmdConnectorCreate) ValidateInput(args []string) []error { +func (cmd *CmdConnectorCreate) ValidateInput(args []string) error { var validationErrors []error resourceStringValidator := validator.NewResourceStringValidator() numberValidator := validator.NewNumberValidator() @@ -95,7 +96,7 @@ func (cmd *CmdConnectorCreate) ValidateInput(args []string) []error { // Validate if there is already a Connector with this name in the namespace if cmd.name != "" { connector, err := cmd.client.Connectors(cmd.namespace).Get(context.TODO(), cmd.name, metav1.GetOptions{}) - if connector != nil && !errors.IsNotFound(err) { + if connector != nil && !k8serrs.IsNotFound(err) { validationErrors = append(validationErrors, fmt.Errorf("there is already a connector %s created for namespace %s", cmd.name, cmd.namespace)) } } @@ -214,7 +215,7 @@ func (cmd *CmdConnectorCreate) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdConnectorCreate) InputToOptions() { diff --git a/internal/cmd/skupper/connector/kube/connector_create_test.go b/internal/cmd/skupper/connector/kube/connector_create_test.go index 0d60839c8..2fae1ee02 100644 --- a/internal/cmd/skupper/connector/kube/connector_create_test.go +++ b/internal/cmd/skupper/connector/kube/connector_create_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/skupperproject/skupper/internal/cmd/skupper/common" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" @@ -23,7 +24,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { flags common.CommandConnectorCreateFlags k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } testTable := []test{ @@ -57,7 +58,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"there is already a connector my-connector created for namespace test"}, + expectedError: "there is already a connector my-connector created for namespace test", }, { name: "connector name and port are not specified", @@ -66,7 +67,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Selector: "backend", Timeout: 1 * time.Minute, }, - expectedErrors: []string{"connector name and port must be configured"}, + expectedError: "connector name and port must be configured", }, { name: "connector name empty", @@ -75,7 +76,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Selector: "backend", Timeout: 1 * time.Minute, }, - expectedErrors: []string{"connector name must not be empty"}, + expectedError: "connector name must not be empty", }, { name: "connector port empty", @@ -84,7 +85,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Selector: "backend", Timeout: 1 * time.Minute, }, - expectedErrors: []string{"connector port must not be empty"}, + expectedError: "connector port must not be empty", }, { name: "connector port not positive", @@ -93,7 +94,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Selector: "backend", Timeout: 1 * time.Minute, }, - expectedErrors: []string{"connector port is not valid: value is not positive"}, + expectedError: "connector port is not valid: value is not positive", }, { name: "connector name and port are not specified", @@ -102,7 +103,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Selector: "backend", Timeout: 1 * time.Minute, }, - expectedErrors: []string{"connector name and port must be configured"}, + expectedError: "connector name and port must be configured", }, { name: "connector port is not specified", @@ -111,7 +112,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Selector: "backend", Timeout: 1 * time.Minute, }, - expectedErrors: []string{"connector name and port must be configured"}, + expectedError: "connector name and port must be configured", }, { name: "more than two arguments are specified", @@ -120,7 +121,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Selector: "backend", Timeout: 1 * time.Minute, }, - expectedErrors: []string{"only two arguments are allowed for this command"}, + expectedError: "only two arguments are allowed for this command", }, { name: "connector name is not valid.", @@ -129,7 +130,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Selector: "backend", Timeout: 1 * time.Minute, }, - expectedErrors: []string{"connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + expectedError: "connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { name: "port is not valid.", @@ -138,7 +139,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Selector: "backend", Timeout: 1 * time.Minute, }, - expectedErrors: []string{"connector port is not valid: strconv.Atoi: parsing \"abcd\": invalid syntax"}, + expectedError: "connector port is not valid: strconv.Atoi: parsing \"abcd\": invalid syntax", }, { name: "connector type is not valid", @@ -148,8 +149,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Timeout: 1 * time.Minute, Selector: "backend", }, - expectedErrors: []string{ - "connector type is not valid: value not-valid not allowed. It should be one of this options: [tcp]"}, + expectedError: "connector type is not valid: value not-valid not allowed. It should be one of this options: [tcp]", }, { name: "routing key is not valid", @@ -159,8 +159,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Timeout: 1 * time.Minute, Selector: "backend", }, - expectedErrors: []string{ - "routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + expectedError: "routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { name: "tls-secret does not exist", @@ -170,7 +169,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Timeout: 1 * time.Minute, Selector: "backend", }, - expectedErrors: []string{"tls-secret is not valid: does not exist"}, + expectedError: "tls-secret is not valid: does not exist", }, { name: "workload is not valid", @@ -179,8 +178,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Workload: "@345", Timeout: 1 * time.Minute, }, - expectedErrors: []string{ - "workload is not valid: workload must include /"}, + expectedError: "workload is not valid: workload must include /", }, { name: "selector is not valid", @@ -189,8 +187,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Selector: "@#$%", Timeout: 20 * time.Second, }, - expectedErrors: []string{ - "selector is not valid: value does not match this regular expression: ^[A-Za-z0-9=:./-]+$"}, + expectedError: "selector is not valid: value does not match this regular expression: ^[A-Za-z0-9=:./-]+$", }, { name: "timeout is not valid", @@ -199,7 +196,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Host: "host", Timeout: 0 * time.Second, }, - expectedErrors: []string{"timeout is not valid: duration must not be less than 10s; got 0s"}, + expectedError: "timeout is not valid: duration must not be less than 10s; got 0s", }, { name: "output is not valid", @@ -209,8 +206,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Timeout: 10 * time.Second, Output: "not-supported", }, - expectedErrors: []string{ - "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]"}, + expectedError: "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]", }, { name: "selector/host", @@ -221,9 +217,8 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { Selector: "app=test", Host: "test", }, - expectedErrors: []string{ - "If host is configured, cannot configure workload or selector", - "If selector is configured, cannot configure workload or host"}, + expectedError: "If host is configured, cannot configure workload or selector\n" + + "If selector is configured, cannot configure workload or host", }, { name: "workload/host", @@ -253,9 +248,8 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "If host is configured, cannot configure workload or selector", - "If workload is configured, cannot configure selector or host"}, + expectedError: "If host is configured, cannot configure workload or selector\n" + + "If workload is configured, cannot configure selector or host", }, } @@ -267,12 +261,7 @@ func TestCmdConnectorCreate_ValidateInput(t *testing.T) { command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } @@ -284,7 +273,7 @@ func TestCmdConnectorCreate_ValidateWorkload(t *testing.T) { flags common.CommandConnectorCreateFlags k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string expectedSelector string } @@ -296,8 +285,7 @@ func TestCmdConnectorCreate_ValidateWorkload(t *testing.T) { Output: "json", Workload: "deployment/backend", }, - expectedErrors: []string{ - "failed trying to get Deployment specified by workload: deployments.apps \"backend\" not found"}, + expectedError: "failed trying to get Deployment specified by workload: deployments.apps \"backend\" not found", }, { name: "workload-deployment-no-labels", @@ -321,7 +309,7 @@ func TestCmdConnectorCreate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{"workload, no selector Matchlabels found"}, + expectedError: "workload, no selector Matchlabels found", }, { name: "workload-deployment", @@ -349,7 +337,7 @@ func TestCmdConnectorCreate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{}, + expectedError: "", expectedSelector: "app=backend", }, { @@ -359,9 +347,7 @@ func TestCmdConnectorCreate_ValidateWorkload(t *testing.T) { Output: "json", Workload: "service/backend", }, - expectedErrors: []string{ - "failed trying to get Service specified by workload: services \"backend\" not found", - }, + expectedError: "failed trying to get Service specified by workload: services \"backend\" not found", }, { name: "workload-service-no-labels", @@ -383,7 +369,7 @@ func TestCmdConnectorCreate_ValidateWorkload(t *testing.T) { Spec: v12.ServiceSpec{}, }, }, - expectedErrors: []string{"workload, no selector labels found"}, + expectedError: "workload, no selector labels found", }, { name: "workload-service", @@ -409,7 +395,7 @@ func TestCmdConnectorCreate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{}, + expectedError: "", expectedSelector: "app=backend", }, { @@ -419,9 +405,7 @@ func TestCmdConnectorCreate_ValidateWorkload(t *testing.T) { Output: "json", Workload: "daemonset/backend", }, - expectedErrors: []string{ - "failed trying to get DaemonSet specified by workload: daemonsets.apps \"backend\" not found", - }, + expectedError: "failed trying to get DaemonSet specified by workload: daemonsets.apps \"backend\" not found", }, { name: "workload-daemonset-no-labels", @@ -445,7 +429,7 @@ func TestCmdConnectorCreate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{"workload, no selector Matchlabels found"}, + expectedError: "workload, no selector Matchlabels found", }, { name: "workload-daemonset", @@ -473,7 +457,7 @@ func TestCmdConnectorCreate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{}, + expectedError: "", expectedSelector: "app=backend", }, { @@ -483,9 +467,7 @@ func TestCmdConnectorCreate_ValidateWorkload(t *testing.T) { Output: "json", Workload: "StatefulSet/backend", }, - expectedErrors: []string{ - "failed trying to get StatefulSet specified by workload: statefulsets.apps \"backend\" not found", - }, + expectedError: "failed trying to get StatefulSet specified by workload: statefulsets.apps \"backend\" not found", }, { name: "workload-statefulset-no-labels", @@ -509,7 +491,7 @@ func TestCmdConnectorCreate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{"workload, no selector Matchlabels found"}, + expectedError: "workload, no selector Matchlabels found", }, { name: "workload-statefulset", @@ -537,16 +519,14 @@ func TestCmdConnectorCreate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{}, + expectedError: "", expectedSelector: "app=backend", }, { - name: "wait status is not valid", - args: []string{"workload-deployment", "1234"}, - flags: common.CommandConnectorCreateFlags{Timeout: time.Minute, Wait: "created"}, - expectedErrors: []string{ - "status is not valid: value created not allowed. It should be one of this options: [ready configured none]", - }, + name: "wait status is not valid", + args: []string{"workload-deployment", "1234"}, + flags: common.CommandConnectorCreateFlags{Timeout: time.Minute, Wait: "created"}, + expectedError: "status is not valid: value created not allowed. It should be one of this options: [ready configured none]", }, } @@ -558,11 +538,7 @@ func TestCmdConnectorCreate_ValidateWorkload(t *testing.T) { command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) + testutils.CheckValidateInput(t, command, test.expectedError, test.args) //validate selector is correct assert.Check(t, command.selector == test.expectedSelector) diff --git a/internal/cmd/skupper/connector/kube/connector_delete.go b/internal/cmd/skupper/connector/kube/connector_delete.go index 9c85297bd..edc02ddae 100644 --- a/internal/cmd/skupper/connector/kube/connector_delete.go +++ b/internal/cmd/skupper/connector/kube/connector_delete.go @@ -2,6 +2,7 @@ package kube import ( "context" + "errors" "fmt" "github.com/skupperproject/skupper/internal/cmd/skupper/common" @@ -30,13 +31,13 @@ func NewCmdConnectorDelete() *CmdConnectorDelete { func (cmd *CmdConnectorDelete) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.client = cli.GetSkupperClient().SkupperV2alpha1() cmd.namespace = cli.Namespace } -func (cmd *CmdConnectorDelete) ValidateInput(args []string) []error { +func (cmd *CmdConnectorDelete) ValidateInput(args []string) error { var validationErrors []error resourceStringValidator := validator.NewResourceStringValidator() timeoutValidator := validator.NewTimeoutInSecondsValidator() @@ -71,7 +72,7 @@ func (cmd *CmdConnectorDelete) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdConnectorDelete) Run() error { diff --git a/internal/cmd/skupper/connector/kube/connector_delete_test.go b/internal/cmd/skupper/connector/kube/connector_delete_test.go index c54a0b325..62fc34373 100644 --- a/internal/cmd/skupper/connector/kube/connector_delete_test.go +++ b/internal/cmd/skupper/connector/kube/connector_delete_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/skupperproject/skupper/internal/cmd/skupper/common" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" @@ -21,39 +22,39 @@ func TestCmdConnectorDelete_ValidateInput(t *testing.T) { flags common.CommandConnectorDeleteFlags k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } testTable := []test{ { - name: "connector is not Deleted because connector does not exist in the namespace", - args: []string{"my-connector"}, - flags: common.CommandConnectorDeleteFlags{Timeout: 30 * time.Second}, - expectedErrors: []string{"connector my-connector does not exist in namespace test"}, + name: "connector is not Deleted because connector does not exist in the namespace", + args: []string{"my-connector"}, + flags: common.CommandConnectorDeleteFlags{Timeout: 30 * time.Second}, + expectedError: "connector my-connector does not exist in namespace test", }, { - name: "connector name is not specified", - args: []string{}, - flags: common.CommandConnectorDeleteFlags{Timeout: 10 * time.Second}, - expectedErrors: []string{"connector name must be specified"}, + name: "connector name is not specified", + args: []string{}, + flags: common.CommandConnectorDeleteFlags{Timeout: 10 * time.Second}, + expectedError: "connector name must be specified", }, { - name: "connector name is nil", - args: []string{""}, - flags: common.CommandConnectorDeleteFlags{Timeout: 10 * time.Second}, - expectedErrors: []string{"connector name must not be empty"}, + name: "connector name is nil", + args: []string{""}, + flags: common.CommandConnectorDeleteFlags{Timeout: 10 * time.Second}, + expectedError: "connector name must not be empty", }, { - name: "connector name is not valid", - args: []string{"my name"}, - flags: common.CommandConnectorDeleteFlags{Timeout: 10 * time.Second}, - expectedErrors: []string{"connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "connector name is not valid", + args: []string{"my name"}, + flags: common.CommandConnectorDeleteFlags{Timeout: 10 * time.Second}, + expectedError: "connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "more than one argument is specified", - args: []string{"my", "connector"}, - flags: common.CommandConnectorDeleteFlags{Timeout: 10 * time.Second}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument is specified", + args: []string{"my", "connector"}, + flags: common.CommandConnectorDeleteFlags{Timeout: 10 * time.Second}, + expectedError: "only one argument is allowed for this command", }, { name: "timeout is not valid", @@ -77,7 +78,7 @@ func TestCmdConnectorDelete_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"timeout is not valid: duration must not be less than 10s; got 5s"}, + expectedError: "timeout is not valid: duration must not be less than 10s; got 5s", }, } @@ -89,12 +90,7 @@ func TestCmdConnectorDelete_ValidateInput(t *testing.T) { command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/connector/kube/connector_status.go b/internal/cmd/skupper/connector/kube/connector_status.go index 534c51d02..8699b9eed 100644 --- a/internal/cmd/skupper/connector/kube/connector_status.go +++ b/internal/cmd/skupper/connector/kube/connector_status.go @@ -2,6 +2,7 @@ package kube import ( "context" + "errors" "fmt" "os" "text/tabwriter" @@ -13,7 +14,7 @@ import ( "github.com/skupperproject/skupper/internal/utils/validator" skupperv2alpha1 "github.com/skupperproject/skupper/pkg/generated/client/clientset/versioned/typed/skupper/v2alpha1" "github.com/spf13/cobra" - "k8s.io/apimachinery/pkg/api/errors" + k8serrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -33,13 +34,13 @@ func NewCmdConnectorStatus() *CmdConnectorStatus { func (cmd *CmdConnectorStatus) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.client = cli.GetSkupperClient().SkupperV2alpha1() cmd.namespace = cli.Namespace } -func (cmd *CmdConnectorStatus) ValidateInput(args []string) []error { +func (cmd *CmdConnectorStatus) ValidateInput(args []string) error { var validationErrors []error resourceStringValidator := validator.NewResourceStringValidator() outputTypeValidator := validator.NewOptionValidator(common.OutputTypes) @@ -63,7 +64,7 @@ func (cmd *CmdConnectorStatus) ValidateInput(args []string) []error { // Validate that there is a connector with this name in the namespace if cmd.name != "" { connector, err := cmd.client.Connectors(cmd.namespace).Get(context.TODO(), cmd.name, metav1.GetOptions{}) - if connector == nil || errors.IsNotFound(err) { + if connector == nil || k8serrs.IsNotFound(err) { validationErrors = append(validationErrors, fmt.Errorf("connector %s does not exist in namespace %s", cmd.name, cmd.namespace)) } } @@ -77,7 +78,7 @@ func (cmd *CmdConnectorStatus) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdConnectorStatus) Run() error { if cmd.name == "" { diff --git a/internal/cmd/skupper/connector/kube/connector_status_test.go b/internal/cmd/skupper/connector/kube/connector_status_test.go index ff4da1177..fd1388797 100644 --- a/internal/cmd/skupper/connector/kube/connector_status_test.go +++ b/internal/cmd/skupper/connector/kube/connector_status_test.go @@ -4,7 +4,7 @@ import ( "testing" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" @@ -20,33 +20,33 @@ func TestCmdConnectorStatus_ValidateInput(t *testing.T) { flags common.CommandConnectorStatusFlags k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } testTable := []test{ { - name: "connector is not shown because connector does not exist in the namespace", - args: []string{"my-connector"}, - expectedErrors: []string{"connector my-connector does not exist in namespace test"}, + name: "connector is not shown because connector does not exist in the namespace", + args: []string{"my-connector"}, + expectedError: "connector my-connector does not exist in namespace test", }, { - name: "connector name is nil", - args: []string{""}, - expectedErrors: []string{"connector name must not be empty"}, + name: "connector name is nil", + args: []string{""}, + expectedError: "connector name must not be empty", }, { - name: "more than one argument is specified", - args: []string{"my", "connector"}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument is specified", + args: []string{"my", "connector"}, + expectedError: "only one argument is allowed for this command", }, { - name: "connector name is not valid.", - args: []string{"my new connector"}, - expectedErrors: []string{"connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "connector name is not valid.", + args: []string{"my new connector"}, + expectedError: "connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "no args", - expectedErrors: []string{}, + name: "no args", + expectedError: "", }, { name: "bad output status", @@ -74,7 +74,7 @@ func TestCmdConnectorStatus_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]"}, + expectedError: "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]", }, { name: "good output status", @@ -102,7 +102,7 @@ func TestCmdConnectorStatus_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{}, + expectedError: "", }, } @@ -114,12 +114,7 @@ func TestCmdConnectorStatus_ValidateInput(t *testing.T) { command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/connector/kube/connector_update.go b/internal/cmd/skupper/connector/kube/connector_update.go index 1132d2e64..c84ba81a7 100644 --- a/internal/cmd/skupper/connector/kube/connector_update.go +++ b/internal/cmd/skupper/connector/kube/connector_update.go @@ -2,6 +2,7 @@ package kube import ( "context" + "errors" "fmt" "k8s.io/apimachinery/pkg/api/meta" "time" @@ -15,7 +16,7 @@ import ( "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" skupperv2alpha1 "github.com/skupperproject/skupper/pkg/generated/client/clientset/versioned/typed/skupper/v2alpha1" "github.com/spf13/cobra" - "k8s.io/apimachinery/pkg/api/errors" + k8serrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" ) @@ -55,14 +56,14 @@ func NewCmdConnectorUpdate() *CmdConnectorUpdate { func (cmd *CmdConnectorUpdate) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.client = cli.GetSkupperClient().SkupperV2alpha1() cmd.namespace = cli.Namespace cmd.KubeClient = cli.Kube } -func (cmd *CmdConnectorUpdate) ValidateInput(args []string) []error { +func (cmd *CmdConnectorUpdate) ValidateInput(args []string) error { var validationErrors []error resourceStringValidator := validator.NewResourceStringValidator() numberValidator := validator.NewNumberValidator() @@ -92,7 +93,7 @@ func (cmd *CmdConnectorUpdate) ValidateInput(args []string) []error { // Validate that there is already a connector with this name in the namespace if cmd.name != "" { connector, err := cmd.client.Connectors(cmd.namespace).Get(context.TODO(), cmd.name, metav1.GetOptions{}) - if connector == nil || errors.IsNotFound(err) { + if connector == nil || k8serrs.IsNotFound(err) { validationErrors = append(validationErrors, fmt.Errorf("connector %s must exist in namespace %s to be updated", cmd.name, cmd.namespace)) } else { // save existing values @@ -246,7 +247,7 @@ func (cmd *CmdConnectorUpdate) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdConnectorUpdate) Run() error { diff --git a/internal/cmd/skupper/connector/kube/connector_update_test.go b/internal/cmd/skupper/connector/kube/connector_update_test.go index 130df8722..0622de96d 100644 --- a/internal/cmd/skupper/connector/kube/connector_update_test.go +++ b/internal/cmd/skupper/connector/kube/connector_update_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/skupperproject/skupper/internal/cmd/skupper/common" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" @@ -23,39 +24,39 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { flags common.CommandConnectorUpdateFlags k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } testTable := []test{ { - name: "connector is not updated because get connector returned error", - args: []string{"my-connector"}, - flags: common.CommandConnectorUpdateFlags{Timeout: 10 * time.Second}, - expectedErrors: []string{"connector my-connector must exist in namespace test to be updated"}, + name: "connector is not updated because get connector returned error", + args: []string{"my-connector"}, + flags: common.CommandConnectorUpdateFlags{Timeout: 10 * time.Second}, + expectedError: "connector my-connector must exist in namespace test to be updated", }, { - name: "connector name is not specified", - args: []string{}, - flags: common.CommandConnectorUpdateFlags{Timeout: 10 * time.Second}, - expectedErrors: []string{"connector name must be configured"}, + name: "connector name is not specified", + args: []string{}, + flags: common.CommandConnectorUpdateFlags{Timeout: 10 * time.Second}, + expectedError: "connector name must be configured", }, { - name: "connector name is nil", - args: []string{""}, - flags: common.CommandConnectorUpdateFlags{Timeout: 10 * time.Minute}, - expectedErrors: []string{"connector name must not be empty"}, + name: "connector name is nil", + args: []string{""}, + flags: common.CommandConnectorUpdateFlags{Timeout: 10 * time.Minute}, + expectedError: "connector name must not be empty", }, { - name: "more than one argument is specified", - args: []string{"my", "connector"}, - flags: common.CommandConnectorUpdateFlags{Timeout: 10 * time.Minute}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument is specified", + args: []string{"my", "connector"}, + flags: common.CommandConnectorUpdateFlags{Timeout: 10 * time.Minute}, + expectedError: "only one argument is allowed for this command", }, { - name: "connector name is not valid.", - args: []string{"my new connector"}, - flags: common.CommandConnectorUpdateFlags{Timeout: 10 * time.Minute}, - expectedErrors: []string{"connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "connector name is not valid.", + args: []string{"my new connector"}, + flags: common.CommandConnectorUpdateFlags{Timeout: 10 * time.Minute}, + expectedError: "connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { name: "connector type is not valid", @@ -82,8 +83,7 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "connector type is not valid: value not-valid not allowed. It should be one of this options: [tcp]"}, + expectedError: "connector type is not valid: value not-valid not allowed. It should be one of this options: [tcp]", }, { name: "routing key is not valid", @@ -110,8 +110,7 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + expectedError: "routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { name: "tls-secret is not valid", @@ -138,7 +137,7 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"tls-secret is not valid: does not exist"}, + expectedError: "tls-secret is not valid: does not exist", }, { name: "port is not valid", @@ -165,8 +164,7 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "connector port is not valid: value is not positive"}, + expectedError: "connector port is not valid: value is not positive", }, { name: "workload is not valid", @@ -193,8 +191,7 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "workload is not valid: workload must include /"}, + expectedError: "workload is not valid: workload must include /", }, { name: "selector is not valid", @@ -221,8 +218,7 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "selector is not valid: value does not match this regular expression: ^[A-Za-z0-9=:./-]+$"}, + expectedError: "selector is not valid: value does not match this regular expression: ^[A-Za-z0-9=:./-]+$", }, { name: "selector/host", @@ -251,9 +247,8 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "If host is configured, cannot configure workload or selector", - "If selector is configured, cannot configure workload or host"}, + expectedError: "If host is configured, cannot configure workload or selector\n" + + "If selector is configured, cannot configure workload or host", }, { name: "workload/host", @@ -301,9 +296,8 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "If host is configured, cannot configure workload or selector", - "If workload is configured, cannot configure selector or host"}, + expectedError: "If host is configured, cannot configure workload or selector\n" + + "If workload is configured, cannot configure selector or host", }, { name: "timeout is not valid", @@ -330,7 +324,7 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"timeout is not valid: duration must not be less than 10s; got 1s"}, + expectedError: "timeout is not valid: duration must not be less than 10s; got 1s", }, { name: "output is not valid", @@ -357,8 +351,7 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]"}, + expectedError: "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]", }, { name: "flags all valid", @@ -398,7 +391,7 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{}, + expectedError: "", }, { name: "wait status is not valid", @@ -422,9 +415,7 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "status is not valid: value created not allowed. It should be one of this options: [ready configured none]", - }, + expectedError: "status is not valid: value created not allowed. It should be one of this options: [ready configured none]", }, } @@ -436,12 +427,7 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } @@ -453,7 +439,7 @@ func TestCmdConnectorUpdate_ValidateWorkload(t *testing.T) { flags common.CommandConnectorUpdateFlags k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string expectedSelector string expectedStatus string } @@ -484,9 +470,7 @@ func TestCmdConnectorUpdate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{ - "failed trying to get Deployment specified by workload: deployments.apps \"backend\" not found", - }, + expectedError: "failed trying to get Deployment specified by workload: deployments.apps \"backend\" not found", }, { name: "workload-deployment-no-labels", @@ -528,7 +512,7 @@ func TestCmdConnectorUpdate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{"workload, no selector Matchlabels found"}, + expectedError: "workload, no selector Matchlabels found", }, { name: "workload-deployment", @@ -574,7 +558,7 @@ func TestCmdConnectorUpdate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{}, + expectedError: "", expectedSelector: "app=backend", }, { @@ -602,9 +586,7 @@ func TestCmdConnectorUpdate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{ - "failed trying to get Service specified by workload: services \"backend\" not found", - }, + expectedError: "failed trying to get Service specified by workload: services \"backend\" not found", }, { name: "workload-service-no-labels", @@ -644,7 +626,7 @@ func TestCmdConnectorUpdate_ValidateWorkload(t *testing.T) { Spec: v12.ServiceSpec{}, }, }, - expectedErrors: []string{"workload, no selector labels found"}, + expectedError: "workload, no selector labels found", }, { name: "workload-service", @@ -688,7 +670,7 @@ func TestCmdConnectorUpdate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{}, + expectedError: "", expectedSelector: "app=backend", }, { @@ -716,9 +698,7 @@ func TestCmdConnectorUpdate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{ - "failed trying to get DaemonSet specified by workload: daemonsets.apps \"backend\" not found", - }, + expectedError: "failed trying to get DaemonSet specified by workload: daemonsets.apps \"backend\" not found", }, { name: "workload-daemonset-no-labels", @@ -760,7 +740,7 @@ func TestCmdConnectorUpdate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{"workload, no selector Matchlabels found"}, + expectedError: "workload, no selector Matchlabels found", }, { name: "workload-daemonset", @@ -806,7 +786,7 @@ func TestCmdConnectorUpdate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{}, + expectedError: "", expectedSelector: "app=backend", }, { @@ -834,9 +814,7 @@ func TestCmdConnectorUpdate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{ - "failed trying to get StatefulSet specified by workload: statefulsets.apps \"backend\" not found", - }, + expectedError: "failed trying to get StatefulSet specified by workload: statefulsets.apps \"backend\" not found", }, { name: "workload-statefulset-no-labels", @@ -878,7 +856,7 @@ func TestCmdConnectorUpdate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{"workload, no selector Matchlabels found"}, + expectedError: "workload, no selector Matchlabels found", }, { name: "workload-statefulset", @@ -924,7 +902,7 @@ func TestCmdConnectorUpdate_ValidateWorkload(t *testing.T) { }, }, }, - expectedErrors: []string{}, + expectedError: "", expectedSelector: "app=backend", }, } @@ -937,11 +915,7 @@ func TestCmdConnectorUpdate_ValidateWorkload(t *testing.T) { command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) + testutils.CheckValidateInput(t, command, test.expectedError, test.args) //validate selector is correct assert.Check(t, command.newSettings.selector == test.expectedSelector) diff --git a/internal/cmd/skupper/connector/nonkube/connector_create.go b/internal/cmd/skupper/connector/nonkube/connector_create.go index 0e2cd8f5d..41c8e361a 100644 --- a/internal/cmd/skupper/connector/nonkube/connector_create.go +++ b/internal/cmd/skupper/connector/nonkube/connector_create.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "net" "strconv" @@ -40,7 +41,7 @@ func (cmd *CmdConnectorCreate) NewClient(cobraCommand *cobra.Command, args []str cmd.connectorHandler = fs.NewConnectorHandler(cmd.namespace) } -func (cmd *CmdConnectorCreate) ValidateInput(args []string) []error { +func (cmd *CmdConnectorCreate) ValidateInput(args []string) error { var validationErrors []error if cmd.CobraCmd != nil && cmd.CobraCmd.Flag(common.FlagNameContext) != nil && cmd.CobraCmd.Flag(common.FlagNameContext).Value.String() != "" { @@ -118,7 +119,7 @@ func (cmd *CmdConnectorCreate) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdConnectorCreate) InputToOptions() { diff --git a/internal/cmd/skupper/connector/nonkube/connector_create_test.go b/internal/cmd/skupper/connector/nonkube/connector_create_test.go index ed977eb08..7479d1b46 100644 --- a/internal/cmd/skupper/connector/nonkube/connector_create_test.go +++ b/internal/cmd/skupper/connector/nonkube/connector_create_test.go @@ -4,7 +4,7 @@ import ( "testing" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/nonkube/client/fs" "github.com/spf13/cobra" @@ -20,93 +20,93 @@ func TestNonKubeCmdConnectorCreate_ValidateInput(t *testing.T) { skupperObjects []runtime.Object flags *common.CommandConnectorCreateFlags cobraGenericFlags map[string]string - expectedErrors []string + expectedError string } testTable := []test{ { - name: "Connector name and port are not specified", - args: []string{}, - flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{"connector name and port must be configured"}, + name: "Connector name and port are not specified", + args: []string{}, + flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, + expectedError: "connector name and port must be configured", }, { - name: "Connector name is not valid", - args: []string{"my new Connector", "8080"}, - flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{"connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "Connector name is not valid", + args: []string{"my new Connector", "8080"}, + flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, + expectedError: "connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "Connector name is empty", - args: []string{"", "1234"}, - flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{"connector name must not be empty"}, + name: "Connector name is empty", + args: []string{"", "1234"}, + flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, + expectedError: "connector name must not be empty", }, { - name: "connector port empty", - args: []string{"my-name-port-empty", ""}, - flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{"connector port must not be empty"}, + name: "connector port empty", + args: []string{"my-name-port-empty", ""}, + flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, + expectedError: "connector port must not be empty", }, { - name: "port is not valid", - args: []string{"my-connector-port", "abcd"}, - flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{"connector port is not valid: strconv.Atoi: parsing \"abcd\": invalid syntax"}, + name: "port is not valid", + args: []string{"my-connector-port", "abcd"}, + flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, + expectedError: "connector port is not valid: strconv.Atoi: parsing \"abcd\": invalid syntax", }, { - name: "port not positive", - args: []string{"my-port-positive", "-45"}, - flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{"connector port is not valid: value is not positive"}, + name: "port not positive", + args: []string{"my-port-positive", "-45"}, + flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, + expectedError: "connector port is not valid: value is not positive", }, { - name: "more than two arguments was specified", - args: []string{"my", "Connector", "test"}, - flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{"only two arguments are allowed for this command"}, + name: "more than two arguments was specified", + args: []string{"my", "Connector", "test"}, + flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, + expectedError: "only two arguments are allowed for this command", }, { - name: "type is not valid", - args: []string{"my-connector", "8080"}, - flags: &common.CommandConnectorCreateFlags{ConnectorType: "not-valid", Host: "1.2.3.4"}, - expectedErrors: []string{"connector type is not valid: value not-valid not allowed. It should be one of this options: [tcp]"}, + name: "type is not valid", + args: []string{"my-connector", "8080"}, + flags: &common.CommandConnectorCreateFlags{ConnectorType: "not-valid", Host: "1.2.3.4"}, + expectedError: "connector type is not valid: value not-valid not allowed. It should be one of this options: [tcp]", }, { - name: "routing key is not valid", - args: []string{"my-connector-rk", "8080"}, - flags: &common.CommandConnectorCreateFlags{RoutingKey: "not-valid$", Host: "1.2.3.4"}, - expectedErrors: []string{"routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "routing key is not valid", + args: []string{"my-connector-rk", "8080"}, + flags: &common.CommandConnectorCreateFlags{RoutingKey: "not-valid$", Host: "1.2.3.4"}, + expectedError: "routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "TlsCredentials is not valid", - args: []string{"my-connector-tls", "8080"}, - flags: &common.CommandConnectorCreateFlags{TlsCredentials: "not-valid$", Host: "1.2.3.4"}, - expectedErrors: []string{"tlsCredentials value is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "TlsCredentials is not valid", + args: []string{"my-connector-tls", "8080"}, + flags: &common.CommandConnectorCreateFlags{TlsCredentials: "not-valid$", Host: "1.2.3.4"}, + expectedError: "tlsCredentials value is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "host is not valid", - args: []string{"my-connector-host", "8080"}, - flags: &common.CommandConnectorCreateFlags{Host: "not-valid$"}, - expectedErrors: []string{"host is not valid: a valid IP address or hostname is expected"}, + name: "host is not valid", + args: []string{"my-connector-host", "8080"}, + flags: &common.CommandConnectorCreateFlags{Host: "not-valid$"}, + expectedError: "host is not valid: a valid IP address or hostname is expected", }, { - name: "host is not configued", - args: []string{"my-connector-host", "8080"}, - flags: &common.CommandConnectorCreateFlags{}, - expectedErrors: []string{"host name must be configured: an IP address or hostname is expected"}, + name: "host is not configued", + args: []string{"my-connector-host", "8080"}, + flags: &common.CommandConnectorCreateFlags{}, + expectedError: "host name must be configured: an IP address or hostname is expected", }, { - name: "output format is not valid", - args: []string{"my-connector", "8080"}, - flags: &common.CommandConnectorCreateFlags{Output: "not-valid", Host: "1.2.3.4"}, - expectedErrors: []string{"output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]"}, + name: "output format is not valid", + args: []string{"my-connector", "8080"}, + flags: &common.CommandConnectorCreateFlags{Output: "not-valid", Host: "1.2.3.4"}, + expectedError: "output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]", }, { - name: "kubernetes flags are not valid on this platform", - args: []string{"my-connector", "8080"}, - flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{}, + name: "kubernetes flags are not valid on this platform", + args: []string{"my-connector", "8080"}, + flags: &common.CommandConnectorCreateFlags{Host: "1.2.3.4"}, + expectedError: "", cobraGenericFlags: map[string]string{ common.FlagNameContext: "test", common.FlagNameKubeconfig: "test", @@ -122,7 +122,7 @@ func TestNonKubeCmdConnectorCreate_ValidateInput(t *testing.T) { Output: "json", Host: "1.2.3.4", }, - expectedErrors: []string{}, + expectedError: "", }, } @@ -141,12 +141,7 @@ func TestNonKubeCmdConnectorCreate_ValidateInput(t *testing.T) { } } - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/connector/nonkube/connector_delete.go b/internal/cmd/skupper/connector/nonkube/connector_delete.go index 2e5e619e6..c52bbbcad 100644 --- a/internal/cmd/skupper/connector/nonkube/connector_delete.go +++ b/internal/cmd/skupper/connector/nonkube/connector_delete.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "github.com/skupperproject/skupper/internal/cmd/skupper/common" @@ -29,7 +30,7 @@ func (cmd *CmdConnectorDelete) NewClient(cobraCommand *cobra.Command, args []str cmd.connectorHandler = fs.NewConnectorHandler(cmd.namespace) } -func (cmd *CmdConnectorDelete) ValidateInput(args []string) []error { +func (cmd *CmdConnectorDelete) ValidateInput(args []string) error { var validationErrors []error opts := fs.GetOptions{RuntimeFirst: false, LogWarning: false} resourceStringValidator := validator.NewResourceStringValidator() @@ -66,7 +67,7 @@ func (cmd *CmdConnectorDelete) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdConnectorDelete) InputToOptions() { diff --git a/internal/cmd/skupper/connector/nonkube/connector_delete_test.go b/internal/cmd/skupper/connector/nonkube/connector_delete_test.go index 45dffefef..1e0f4ed83 100644 --- a/internal/cmd/skupper/connector/nonkube/connector_delete_test.go +++ b/internal/cmd/skupper/connector/nonkube/connector_delete_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/nonkube/client/fs" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "github.com/skupperproject/skupper/pkg/nonkube/api" @@ -24,7 +24,7 @@ func TestCmdConnectorDelete_ValidateInput(t *testing.T) { cobraGenericFlags map[string]string k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } homeDir, err := os.UserHomeDir() @@ -33,40 +33,40 @@ func TestCmdConnectorDelete_ValidateInput(t *testing.T) { testTable := []test{ { - name: "connector name is not specified", - args: []string{}, - flags: &common.CommandConnectorDeleteFlags{}, - expectedErrors: []string{"connector name must be configured"}, + name: "connector name is not specified", + args: []string{}, + flags: &common.CommandConnectorDeleteFlags{}, + expectedError: "connector name must be configured", }, { - name: "connector name is nil", - args: []string{""}, - flags: &common.CommandConnectorDeleteFlags{}, - expectedErrors: []string{"connector name must not be empty"}, + name: "connector name is nil", + args: []string{""}, + flags: &common.CommandConnectorDeleteFlags{}, + expectedError: "connector name must not be empty", }, { - name: "connector name is not valid", - args: []string{"my name"}, - flags: &common.CommandConnectorDeleteFlags{}, - expectedErrors: []string{"connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "connector name is not valid", + args: []string{"my name"}, + flags: &common.CommandConnectorDeleteFlags{}, + expectedError: "connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "more than one argument is specified", - args: []string{"my", "connector"}, - flags: &common.CommandConnectorDeleteFlags{}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument is specified", + args: []string{"my", "connector"}, + flags: &common.CommandConnectorDeleteFlags{}, + expectedError: "only one argument is allowed for this command", }, { - name: "connector doesn't exist ", - args: []string{"no-connector"}, - flags: &common.CommandConnectorDeleteFlags{}, - expectedErrors: []string{"connector no-connector does not exist"}, + name: "connector doesn't exist ", + args: []string{"no-connector"}, + flags: &common.CommandConnectorDeleteFlags{}, + expectedError: "connector no-connector does not exist", }, { - name: "kubernetes flags are not valid on this platform", - args: []string{"my-connector"}, - flags: &common.CommandConnectorDeleteFlags{}, - expectedErrors: []string{}, + name: "kubernetes flags are not valid on this platform", + args: []string{"my-connector"}, + flags: &common.CommandConnectorDeleteFlags{}, + expectedError: "", cobraGenericFlags: map[string]string{ common.FlagNameContext: "test", common.FlagNameKubeconfig: "test", @@ -110,12 +110,7 @@ func TestCmdConnectorDelete_ValidateInput(t *testing.T) { } } - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/connector/nonkube/connector_status.go b/internal/cmd/skupper/connector/nonkube/connector_status.go index e62489547..98fc3ddb9 100644 --- a/internal/cmd/skupper/connector/nonkube/connector_status.go +++ b/internal/cmd/skupper/connector/nonkube/connector_status.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "os" "text/tabwriter" @@ -33,7 +34,7 @@ func (cmd *CmdConnectorStatus) NewClient(cobraCommand *cobra.Command, args []str cmd.connectorHandler = fs.NewConnectorHandler(cmd.namespace) } -func (cmd *CmdConnectorStatus) ValidateInput(args []string) []error { +func (cmd *CmdConnectorStatus) ValidateInput(args []string) error { var validationErrors []error opts := fs.GetOptions{RuntimeFirst: true, LogWarning: false} resourceStringValidator := validator.NewResourceStringValidator() @@ -71,7 +72,7 @@ func (cmd *CmdConnectorStatus) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdConnectorStatus) Run() error { diff --git a/internal/cmd/skupper/connector/nonkube/connector_status_test.go b/internal/cmd/skupper/connector/nonkube/connector_status_test.go index a51e2d642..44e738ae4 100644 --- a/internal/cmd/skupper/connector/nonkube/connector_status_test.go +++ b/internal/cmd/skupper/connector/nonkube/connector_status_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/nonkube/client/fs" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "github.com/skupperproject/skupper/pkg/nonkube/api" @@ -23,7 +23,7 @@ func TestCmdConnectorStatus_ValidateInput(t *testing.T) { cobraGenericFlags map[string]string k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } homeDir, err := os.UserHomeDir() @@ -32,45 +32,45 @@ func TestCmdConnectorStatus_ValidateInput(t *testing.T) { testTable := []test{ { - name: "connector is not shown because connector does not exist in the namespace", - args: []string{"no-connector"}, - flags: &common.CommandConnectorStatusFlags{}, - expectedErrors: []string{"connector no-connector does not exist in namespace test"}, + name: "connector is not shown because connector does not exist in the namespace", + args: []string{"no-connector"}, + flags: &common.CommandConnectorStatusFlags{}, + expectedError: "connector no-connector does not exist in namespace test", }, { - name: "connector name is nil", - args: []string{""}, - flags: &common.CommandConnectorStatusFlags{}, - expectedErrors: []string{"connector name must not be empty"}, + name: "connector name is nil", + args: []string{""}, + flags: &common.CommandConnectorStatusFlags{}, + expectedError: "connector name must not be empty", }, { - name: "more than one argument is specified", - args: []string{"my", "connector"}, - flags: &common.CommandConnectorStatusFlags{}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument is specified", + args: []string{"my", "connector"}, + flags: &common.CommandConnectorStatusFlags{}, + expectedError: "only one argument is allowed for this command", }, { - name: "connector name is not valid.", - args: []string{"my new connector"}, - flags: &common.CommandConnectorStatusFlags{}, - expectedErrors: []string{"connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "connector name is not valid.", + args: []string{"my new connector"}, + flags: &common.CommandConnectorStatusFlags{}, + expectedError: "connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "no args", - flags: &common.CommandConnectorStatusFlags{}, - expectedErrors: []string{}, + name: "no args", + flags: &common.CommandConnectorStatusFlags{}, + expectedError: "", }, { - name: "bad output status", - args: []string{"my-connector"}, - flags: &common.CommandConnectorStatusFlags{Output: "not-supported"}, - expectedErrors: []string{"output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]"}, + name: "bad output status", + args: []string{"my-connector"}, + flags: &common.CommandConnectorStatusFlags{Output: "not-supported"}, + expectedError: "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]", }, { - name: "good output status", - args: []string{"my-connector"}, - flags: &common.CommandConnectorStatusFlags{Output: "json"}, - expectedErrors: []string{}, + name: "good output status", + args: []string{"my-connector"}, + flags: &common.CommandConnectorStatusFlags{Output: "json"}, + expectedError: "", }, } @@ -110,12 +110,7 @@ func TestCmdConnectorStatus_ValidateInput(t *testing.T) { } } - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/connector/nonkube/connector_update.go b/internal/cmd/skupper/connector/nonkube/connector_update.go index e52bd0ffc..7476571cf 100644 --- a/internal/cmd/skupper/connector/nonkube/connector_update.go +++ b/internal/cmd/skupper/connector/nonkube/connector_update.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "net" @@ -42,7 +43,7 @@ func (cmd *CmdConnectorUpdate) NewClient(cobraCommand *cobra.Command, args []str cmd.connectorHandler = fs.NewConnectorHandler(cmd.namespace) } -func (cmd *CmdConnectorUpdate) ValidateInput(args []string) []error { +func (cmd *CmdConnectorUpdate) ValidateInput(args []string) error { var validationErrors []error opts := fs.GetOptions{RuntimeFirst: false, LogWarning: false} resourceStringValidator := validator.NewResourceStringValidator() @@ -140,7 +141,7 @@ func (cmd *CmdConnectorUpdate) ValidateInput(args []string) []error { cmd.newSettings.tlsCredentials = cmd.Flags.TlsCredentials } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdConnectorUpdate) InputToOptions() { diff --git a/internal/cmd/skupper/connector/nonkube/connector_update_test.go b/internal/cmd/skupper/connector/nonkube/connector_update_test.go index 6d38bc578..9f4e1be09 100644 --- a/internal/cmd/skupper/connector/nonkube/connector_update_test.go +++ b/internal/cmd/skupper/connector/nonkube/connector_update_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/nonkube/client/fs" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "github.com/skupperproject/skupper/pkg/nonkube/api" @@ -24,7 +24,7 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { k8sObjects []runtime.Object skupperObjects []runtime.Object cobraGenericFlags map[string]string - expectedErrors []string + expectedError string } homeDir, err := os.UserHomeDir() @@ -32,70 +32,70 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { path := filepath.Join(homeDir, "/.local/share/skupper/namespaces/test/", string(api.InputSiteStatePath)) testTable := []test{ { - name: "connector is not updated because get connector returned error", - args: []string{"no-connector"}, - flags: &common.CommandConnectorUpdateFlags{}, - expectedErrors: []string{"connector no-connector must exist in namespace test to be updated"}, + name: "connector is not updated because get connector returned error", + args: []string{"no-connector"}, + flags: &common.CommandConnectorUpdateFlags{}, + expectedError: "connector no-connector must exist in namespace test to be updated", }, { - name: "connector name is not specified", - args: []string{}, - flags: &common.CommandConnectorUpdateFlags{}, - expectedErrors: []string{"connector name must be configured"}, + name: "connector name is not specified", + args: []string{}, + flags: &common.CommandConnectorUpdateFlags{}, + expectedError: "connector name must be configured", }, { - name: "connector name is nil", - args: []string{""}, - flags: &common.CommandConnectorUpdateFlags{}, - expectedErrors: []string{"connector name must not be empty"}, + name: "connector name is nil", + args: []string{""}, + flags: &common.CommandConnectorUpdateFlags{}, + expectedError: "connector name must not be empty", }, { - name: "more than one argument is specified", - args: []string{"my", "connector"}, - flags: &common.CommandConnectorUpdateFlags{}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument is specified", + args: []string{"my", "connector"}, + flags: &common.CommandConnectorUpdateFlags{}, + expectedError: "only one argument is allowed for this command", }, { - name: "connector name is not valid.", - args: []string{"my new connector"}, - flags: &common.CommandConnectorUpdateFlags{}, - expectedErrors: []string{"connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "connector name is not valid.", + args: []string{"my new connector"}, + flags: &common.CommandConnectorUpdateFlags{}, + expectedError: "connector name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "connector type is not valid", - args: []string{"my-connector"}, - flags: &common.CommandConnectorUpdateFlags{ConnectorType: "not-valid"}, - expectedErrors: []string{"connector type is not valid: value not-valid not allowed. It should be one of this options: [tcp]"}, + name: "connector type is not valid", + args: []string{"my-connector"}, + flags: &common.CommandConnectorUpdateFlags{ConnectorType: "not-valid"}, + expectedError: "connector type is not valid: value not-valid not allowed. It should be one of this options: [tcp]", }, { - name: "routing key is not valid", - args: []string{"my-connector"}, - flags: &common.CommandConnectorUpdateFlags{RoutingKey: "not-valid$"}, - expectedErrors: []string{"routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "routing key is not valid", + args: []string{"my-connector"}, + flags: &common.CommandConnectorUpdateFlags{RoutingKey: "not-valid$"}, + expectedError: "routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "tlsCredentials is not valid", - args: []string{"my-connector"}, - flags: &common.CommandConnectorUpdateFlags{TlsCredentials: "not-valid$", Host: "1.2.3.4"}, - expectedErrors: []string{"tlsCredentials value is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "tlsCredentials is not valid", + args: []string{"my-connector"}, + flags: &common.CommandConnectorUpdateFlags{TlsCredentials: "not-valid$", Host: "1.2.3.4"}, + expectedError: "tlsCredentials value is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "host is not valid", - args: []string{"my-connector"}, - flags: &common.CommandConnectorUpdateFlags{Host: "not-valid$"}, - expectedErrors: []string{"host is not valid: a valid IP address or hostname is expected"}, + name: "host is not valid", + args: []string{"my-connector"}, + flags: &common.CommandConnectorUpdateFlags{Host: "not-valid$"}, + expectedError: "host is not valid: a valid IP address or hostname is expected", }, { - name: "port is not valid", - args: []string{"my-connector"}, - flags: &common.CommandConnectorUpdateFlags{Port: -1}, - expectedErrors: []string{"connector port is not valid: value is not positive"}, + name: "port is not valid", + args: []string{"my-connector"}, + flags: &common.CommandConnectorUpdateFlags{Port: -1}, + expectedError: "connector port is not valid: value is not positive", }, { - name: "output is not valid", - args: []string{"my-connector"}, - flags: &common.CommandConnectorUpdateFlags{Output: "not-supported"}, - expectedErrors: []string{"output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]"}, + name: "output is not valid", + args: []string{"my-connector"}, + flags: &common.CommandConnectorUpdateFlags{Output: "not-supported"}, + expectedError: "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]", }, { name: "kubernetes flags are not valid on this platform", @@ -105,7 +105,7 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { common.FlagNameContext: "test", common.FlagNameKubeconfig: "test", }, - expectedErrors: []string{}, + expectedError: "", }, { name: "flags all valid", @@ -118,7 +118,7 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { Output: "json", Host: "1.2.3.4", }, - expectedErrors: []string{}, + expectedError: "", }, } @@ -158,12 +158,7 @@ func TestCmdConnectorUpdate_ValidateInput(t *testing.T) { } } - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/debug/kube/debug.go b/internal/cmd/skupper/debug/kube/debug.go index 136050c15..8357302d9 100644 --- a/internal/cmd/skupper/debug/kube/debug.go +++ b/internal/cmd/skupper/debug/kube/debug.go @@ -2,6 +2,7 @@ package kube import ( "fmt" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" "github.com/skupperproject/skupper/internal/kube/client" skupperv2alpha1 "github.com/skupperproject/skupper/pkg/generated/client/clientset/versioned/typed/skupper/v2alpha1" @@ -33,7 +34,7 @@ func (cmd *CmdDebug) NewClient(cobraCommand *cobra.Command, args []string) { } } -func (cmd *CmdDebug) ValidateInput(args []string) []error { return nil } +func (cmd *CmdDebug) ValidateInput(args []string) error { return nil } func (cmd *CmdDebug) InputToOptions() {} diff --git a/internal/cmd/skupper/debug/nonkube/debug.go b/internal/cmd/skupper/debug/nonkube/debug.go index 18cf4dc35..976a80ca1 100644 --- a/internal/cmd/skupper/debug/nonkube/debug.go +++ b/internal/cmd/skupper/debug/nonkube/debug.go @@ -2,6 +2,7 @@ package nonkube import ( "fmt" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" "github.com/spf13/cobra" ) @@ -23,7 +24,7 @@ func (cmd *CmdDebug) NewClient(cobraCommand *cobra.Command, args []string) { } -func (cmd *CmdDebug) ValidateInput(args []string) []error { return nil } +func (cmd *CmdDebug) ValidateInput(args []string) error { return nil } func (cmd *CmdDebug) InputToOptions() {} diff --git a/internal/cmd/skupper/link/kube/link_delete.go b/internal/cmd/skupper/link/kube/link_delete.go index 90b967bbc..e7000259e 100644 --- a/internal/cmd/skupper/link/kube/link_delete.go +++ b/internal/cmd/skupper/link/kube/link_delete.go @@ -2,7 +2,10 @@ package kube import ( "context" + "errors" "fmt" + "time" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" "github.com/skupperproject/skupper/internal/kube/client" @@ -10,7 +13,6 @@ import ( skupperv2alpha1 "github.com/skupperproject/skupper/pkg/generated/client/clientset/versioned/typed/skupper/v2alpha1" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "time" ) type CmdLinkDelete struct { @@ -29,13 +31,13 @@ func NewCmdLinkDelete() *CmdLinkDelete { func (cmd *CmdLinkDelete) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.Client = cli.GetSkupperClient().SkupperV2alpha1() cmd.Namespace = cli.Namespace } -func (cmd *CmdLinkDelete) ValidateInput(args []string) []error { +func (cmd *CmdLinkDelete) ValidateInput(args []string) error { var validationErrors []error timeoutValidator := validator.NewTimeoutInSecondsValidator() @@ -63,7 +65,7 @@ func (cmd *CmdLinkDelete) ValidateInput(args []string) []error { validationErrors = append(validationErrors, fmt.Errorf("timeout is not valid: %s", err)) } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdLinkDelete) InputToOptions() { cmd.timeout = cmd.Flags.Timeout diff --git a/internal/cmd/skupper/link/kube/link_delete_test.go b/internal/cmd/skupper/link/kube/link_delete_test.go index 918287b7f..751e33ae1 100644 --- a/internal/cmd/skupper/link/kube/link_delete_test.go +++ b/internal/cmd/skupper/link/kube/link_delete_test.go @@ -1,15 +1,17 @@ package kube import ( + "testing" + "time" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "gotest.tools/v3/assert" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "testing" - "time" ) func TestCmdLinkDelete_ValidateInput(t *testing.T) { @@ -19,15 +21,16 @@ func TestCmdLinkDelete_ValidateInput(t *testing.T) { flags common.CommandLinkDeleteFlags k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } testTable := []test{ { - name: "there is no active skupper site in this namespace", - args: []string{"my-link"}, - flags: common.CommandLinkDeleteFlags{Timeout: time.Minute}, - expectedErrors: []string{"there is no skupper site in this namespace", "the link \"my-link\" is not available in the namespace"}, + name: "there is no active skupper site in this namespace", + args: []string{"my-link"}, + flags: common.CommandLinkDeleteFlags{Timeout: time.Minute}, + expectedError: "there is no skupper site in this namespace\n" + + "the link \"my-link\" is not available in the namespace", }, { name: "link is not deleted because it does not exist", @@ -45,7 +48,7 @@ func TestCmdLinkDelete_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"the link \"my-link\" is not available in the namespace"}, + expectedError: "the link \"my-link\" is not available in the namespace", }, { name: "more than one argument was specified", @@ -63,7 +66,7 @@ func TestCmdLinkDelete_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"only one argument is allowed for this command"}, + expectedError: "only one argument is allowed for this command", }, { name: "trying to delete without specifying a name", @@ -81,7 +84,7 @@ func TestCmdLinkDelete_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"link name must not be empty"}, + expectedError: "link name must not be empty", }, { name: "link deleted successfully", @@ -105,7 +108,7 @@ func TestCmdLinkDelete_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{}, + expectedError: "", }, { name: "timeout is not valid because it is zero", @@ -129,7 +132,7 @@ func TestCmdLinkDelete_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"timeout is not valid: duration must not be less than 10s; got 0s"}, + expectedError: "timeout is not valid: duration must not be less than 10s; got 0s", }, } @@ -140,12 +143,7 @@ func TestCmdLinkDelete_ValidateInput(t *testing.T) { assert.Assert(t, err) command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/link/kube/link_generate.go b/internal/cmd/skupper/link/kube/link_generate.go index ebe92e781..353896c36 100644 --- a/internal/cmd/skupper/link/kube/link_generate.go +++ b/internal/cmd/skupper/link/kube/link_generate.go @@ -5,9 +5,14 @@ package kube import ( "context" + "errors" "fmt" + "strconv" + "strings" + "time" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" - commonutils "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" "github.com/skupperproject/skupper/internal/kube/client" pkgutils "github.com/skupperproject/skupper/internal/utils" "github.com/skupperproject/skupper/internal/utils/validator" @@ -17,9 +22,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" - "strconv" - "strings" - "time" ) type CmdLinkGenerate struct { @@ -47,14 +49,14 @@ func NewCmdLinkGenerate() *CmdLinkGenerate { func (cmd *CmdLinkGenerate) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - commonutils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.Client = cli.GetSkupperClient().SkupperV2alpha1() cmd.KubeClient = cli.GetKubeClient() cmd.Namespace = cli.Namespace } -func (cmd *CmdLinkGenerate) ValidateInput(args []string) []error { +func (cmd *CmdLinkGenerate) ValidateInput(args []string) error { var validationErrors []error resourceStringValidator := validator.NewResourceStringValidator() @@ -112,7 +114,7 @@ func (cmd *CmdLinkGenerate) ValidateInput(args []string) []error { validationErrors = append(validationErrors, fmt.Errorf("timeout is not valid: %s", err)) } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdLinkGenerate) InputToOptions() { @@ -195,7 +197,7 @@ func (cmd *CmdLinkGenerate) Run() error { func (cmd *CmdLinkGenerate) WaitUntil() error { var resourcesToPrint []string - encodedOutput, err := commonutils.Encode(cmd.output, cmd.generatedLink) + encodedOutput, err := utils.Encode(cmd.output, cmd.generatedLink) if err != nil { return err } @@ -230,7 +232,7 @@ func (cmd *CmdLinkGenerate) WaitUntil() error { }, } - encodedSecret, _ := commonutils.Encode(cmd.output, secretResource) + encodedSecret, _ := utils.Encode(cmd.output, secretResource) resourcesToPrint = append(resourcesToPrint, encodedSecret) } return nil diff --git a/internal/cmd/skupper/link/kube/link_generate_test.go b/internal/cmd/skupper/link/kube/link_generate_test.go index acba38cf5..1b66f7dff 100644 --- a/internal/cmd/skupper/link/kube/link_generate_test.go +++ b/internal/cmd/skupper/link/kube/link_generate_test.go @@ -5,12 +5,12 @@ import ( "time" "github.com/skupperproject/skupper/internal/cmd/skupper/common" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "gotest.tools/v3/assert" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -22,14 +22,15 @@ func TestCmdLinkGenerate_ValidateInput(t *testing.T) { flags common.CommandLinkGenerateFlags k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } testTable := []test{ { - name: "link yaml is not generated because there is no site in the namespace.", - expectedErrors: []string{"there is no skupper site in this namespace", "there is no active skupper site in this namespace"}, - flags: common.CommandLinkGenerateFlags{Cost: "1", TlsCredentials: "secret", Timeout: time.Minute}, + name: "link yaml is not generated because there is no site in the namespace.", + expectedError: "there is no skupper site in this namespace\n" + + "there is no active skupper site in this namespace", + flags: common.CommandLinkGenerateFlags{Cost: "1", TlsCredentials: "secret", Timeout: time.Minute}, }, { name: "arguments were specified and they are not needed", @@ -81,8 +82,8 @@ func TestCmdLinkGenerate_ValidateInput(t *testing.T) { }, }, }, - flags: common.CommandLinkGenerateFlags{Cost: "1", TlsCredentials: "secret", Timeout: time.Minute}, - expectedErrors: []string{"arguments are not allowed in this command"}, + flags: common.CommandLinkGenerateFlags{Cost: "1", TlsCredentials: "secret", Timeout: time.Minute}, + expectedError: "arguments are not allowed in this command", }, { name: "tls secret was not specified", @@ -133,8 +134,8 @@ func TestCmdLinkGenerate_ValidateInput(t *testing.T) { }, }, }, - flags: common.CommandLinkGenerateFlags{Cost: "1", Timeout: time.Minute}, - expectedErrors: []string{"the TLS secret name was not specified"}, + flags: common.CommandLinkGenerateFlags{Cost: "1", Timeout: time.Minute}, + expectedError: "the TLS secret name was not specified", }, { name: "cost is not valid.", @@ -185,8 +186,8 @@ func TestCmdLinkGenerate_ValidateInput(t *testing.T) { }, }, }, - flags: common.CommandLinkGenerateFlags{Cost: "one", TlsCredentials: "secret", Timeout: time.Minute}, - expectedErrors: []string{"link cost is not valid: strconv.Atoi: parsing \"one\": invalid syntax"}, + flags: common.CommandLinkGenerateFlags{Cost: "one", TlsCredentials: "secret", Timeout: time.Minute}, + expectedError: "link cost is not valid: strconv.Atoi: parsing \"one\": invalid syntax", }, { name: "cost is not positive", @@ -237,10 +238,8 @@ func TestCmdLinkGenerate_ValidateInput(t *testing.T) { }, }, }, - flags: common.CommandLinkGenerateFlags{Cost: "-4", TlsCredentials: "secret", Timeout: time.Minute}, - expectedErrors: []string{ - "link cost is not valid: value is not positive", - }, + flags: common.CommandLinkGenerateFlags{Cost: "-4", TlsCredentials: "secret", Timeout: time.Minute}, + expectedError: "link cost is not valid: value is not positive", }, { name: "output format is not valid", @@ -291,10 +290,8 @@ func TestCmdLinkGenerate_ValidateInput(t *testing.T) { }, }, }, - flags: common.CommandLinkGenerateFlags{Cost: "1", TlsCredentials: "secret", Output: "not-valid", Timeout: time.Minute}, - expectedErrors: []string{ - "output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]", - }, + flags: common.CommandLinkGenerateFlags{Cost: "1", TlsCredentials: "secret", Output: "not-valid", Timeout: time.Minute}, + expectedError: "output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]", }, { name: "tls secret name is not valid", @@ -345,10 +342,8 @@ func TestCmdLinkGenerate_ValidateInput(t *testing.T) { }, }, }, - flags: common.CommandLinkGenerateFlags{Cost: "1", TlsCredentials: "tls secret", Timeout: time.Minute}, - expectedErrors: []string{ - "the name of the tls secret is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", - }, + flags: common.CommandLinkGenerateFlags{Cost: "1", TlsCredentials: "tls secret", Timeout: time.Minute}, + expectedError: "the name of the tls secret is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { name: "timeout is not valid", @@ -399,10 +394,8 @@ func TestCmdLinkGenerate_ValidateInput(t *testing.T) { }, }, }, - flags: common.CommandLinkGenerateFlags{Cost: "1", TlsCredentials: "secret", Timeout: time.Second * 0}, - expectedErrors: []string{ - "timeout is not valid: duration must not be less than 10s; got 0s", - }, + flags: common.CommandLinkGenerateFlags{Cost: "1", TlsCredentials: "secret", Timeout: time.Second * 0}, + expectedError: "timeout is not valid: duration must not be less than 10s; got 0s", }, } @@ -413,12 +406,7 @@ func TestCmdLinkGenerate_ValidateInput(t *testing.T) { assert.Assert(t, err) command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } @@ -845,11 +833,11 @@ func TestCmdLinkGenerate_WaitUntil(t *testing.T) { tlsCredentials: "link-test", k8sObjects: []runtime.Object{ &corev1.Secret{ - TypeMeta: metav1.TypeMeta{ + TypeMeta: v1.TypeMeta{ APIVersion: "v1", Kind: "Secret", }, - ObjectMeta: metav1.ObjectMeta{ + ObjectMeta: v1.ObjectMeta{ Name: "link-test", Namespace: "test", }, @@ -879,11 +867,11 @@ func TestCmdLinkGenerate_WaitUntil(t *testing.T) { tlsCredentials: "link-test", k8sObjects: []runtime.Object{ &corev1.Secret{ - TypeMeta: metav1.TypeMeta{ + TypeMeta: v1.TypeMeta{ APIVersion: "v1", Kind: "Secret", }, - ObjectMeta: metav1.ObjectMeta{ + ObjectMeta: v1.ObjectMeta{ Name: "link-test", Namespace: "test", }, diff --git a/internal/cmd/skupper/link/kube/link_status.go b/internal/cmd/skupper/link/kube/link_status.go index 7bc14bb79..dffa58d57 100644 --- a/internal/cmd/skupper/link/kube/link_status.go +++ b/internal/cmd/skupper/link/kube/link_status.go @@ -2,7 +2,11 @@ package kube import ( "context" + "errors" "fmt" + "os" + "text/tabwriter" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" "github.com/skupperproject/skupper/internal/kube/client" @@ -11,8 +15,6 @@ import ( skupperv2alpha1 "github.com/skupperproject/skupper/pkg/generated/client/clientset/versioned/typed/skupper/v2alpha1" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "os" - "text/tabwriter" ) type CmdLinkStatus struct { @@ -31,13 +33,13 @@ func NewCmdLinkStatus() *CmdLinkStatus { func (cmd *CmdLinkStatus) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.Client = cli.GetSkupperClient().SkupperV2alpha1() cmd.Namespace = cli.Namespace } -func (cmd *CmdLinkStatus) ValidateInput(args []string) []error { +func (cmd *CmdLinkStatus) ValidateInput(args []string) error { var validationErrors []error outputTypeValidator := validator.NewOptionValidator(common.OutputTypes) @@ -62,7 +64,7 @@ func (cmd *CmdLinkStatus) ValidateInput(args []string) []error { cmd.linkName = args[0] } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdLinkStatus) InputToOptions() { diff --git a/internal/cmd/skupper/link/kube/link_status_test.go b/internal/cmd/skupper/link/kube/link_status_test.go index 1fbd5b457..f5c7d9200 100644 --- a/internal/cmd/skupper/link/kube/link_status_test.go +++ b/internal/cmd/skupper/link/kube/link_status_test.go @@ -1,14 +1,15 @@ package kube import ( + "testing" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "gotest.tools/v3/assert" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "testing" ) func TestCmdLinkStatus_ValidateInput(t *testing.T) { @@ -19,7 +20,7 @@ func TestCmdLinkStatus_ValidateInput(t *testing.T) { k8sObjects []runtime.Object skupperObjects []runtime.Object skupperErrorMessage string - expectedErrors []string + expectedError string } testTable := []test{ @@ -38,12 +39,12 @@ func TestCmdLinkStatus_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"this command only accepts one argument"}, + expectedError: "this command only accepts one argument", }, { - name: "there are no sites", - args: []string{}, - expectedErrors: []string{"there is no skupper site available"}, + name: "there are no sites", + args: []string{}, + expectedError: "there is no skupper site available", }, { name: "output format is not valid", @@ -67,9 +68,7 @@ func TestCmdLinkStatus_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]", - }, + expectedError: "output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]", }, } @@ -81,12 +80,7 @@ func TestCmdLinkStatus_ValidateInput(t *testing.T) { command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/link/kube/link_update.go b/internal/cmd/skupper/link/kube/link_update.go index eda612887..474226536 100644 --- a/internal/cmd/skupper/link/kube/link_update.go +++ b/internal/cmd/skupper/link/kube/link_update.go @@ -5,7 +5,11 @@ package kube import ( "context" + "errors" "fmt" + "strconv" + "time" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" "github.com/skupperproject/skupper/internal/kube/client" @@ -16,8 +20,6 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" - "strconv" - "time" ) type CmdLinkUpdate struct { @@ -40,14 +42,14 @@ func NewCmdLinkUpdate() *CmdLinkUpdate { func (cmd *CmdLinkUpdate) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.Client = cli.GetSkupperClient().SkupperV2alpha1() cmd.KubeClient = cli.GetKubeClient() cmd.Namespace = cli.Namespace } -func (cmd *CmdLinkUpdate) ValidateInput(args []string) []error { +func (cmd *CmdLinkUpdate) ValidateInput(args []string) error { var validationErrors []error numberValidator := validator.NewNumberValidator() @@ -108,7 +110,7 @@ func (cmd *CmdLinkUpdate) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdLinkUpdate) InputToOptions() { diff --git a/internal/cmd/skupper/link/kube/link_update_test.go b/internal/cmd/skupper/link/kube/link_update_test.go index 9ff6fcb98..a837f9f78 100644 --- a/internal/cmd/skupper/link/kube/link_update_test.go +++ b/internal/cmd/skupper/link/kube/link_update_test.go @@ -1,7 +1,11 @@ package kube import ( + "testing" + "time" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" @@ -9,8 +13,6 @@ import ( v12 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "testing" - "time" ) func TestCmdLinkUpdate_ValidateInput(t *testing.T) { @@ -20,7 +22,7 @@ func TestCmdLinkUpdate_ValidateInput(t *testing.T) { flags common.CommandLinkUpdateFlags k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } testTable := []test{ @@ -36,7 +38,7 @@ func TestCmdLinkUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"there is no skupper site in this namespace"}, + expectedError: "there is no skupper site in this namespace", }, { name: "link is not available", @@ -54,7 +56,7 @@ func TestCmdLinkUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"the link \"my-link\" is not available in the namespace: links.skupper.io \"my-link\" not found"}, + expectedError: "the link \"my-link\" is not available in the namespace: links.skupper.io \"my-link\" not found", }, { name: "selected link does not exist", @@ -78,7 +80,7 @@ func TestCmdLinkUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"the link \"my\" is not available in the namespace: links.skupper.io \"my\" not found"}, + expectedError: "the link \"my\" is not available in the namespace: links.skupper.io \"my\" not found", }, { name: "link name is not specified.", @@ -102,7 +104,7 @@ func TestCmdLinkUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"link name must not be empty"}, + expectedError: "link name must not be empty", }, { name: "more than one argument was specified", @@ -126,7 +128,7 @@ func TestCmdLinkUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"only one argument is allowed for this command"}, + expectedError: "only one argument is allowed for this command", }, { name: "Cost is not valid.", @@ -150,7 +152,7 @@ func TestCmdLinkUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"link cost is not valid: strconv.Atoi: parsing \"one\": invalid syntax"}, + expectedError: "link cost is not valid: strconv.Atoi: parsing \"one\": invalid syntax", }, { name: "Cost is not positive", @@ -174,9 +176,7 @@ func TestCmdLinkUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "link cost is not valid: value is not positive", - }, + expectedError: "link cost is not valid: value is not positive", }, { name: "output format is not valid", @@ -200,9 +200,7 @@ func TestCmdLinkUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]", - }, + expectedError: "output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]", }, { name: "tls secret not available", @@ -226,9 +224,7 @@ func TestCmdLinkUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "the TLS secret \"secret\" is not available in the namespace: secrets \"secret\" not found", - }, + expectedError: "the TLS secret \"secret\" is not available in the namespace: secrets \"secret\" not found", }, { name: "Timeout value is 0", @@ -260,9 +256,7 @@ func TestCmdLinkUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "timeout is not valid: duration must not be less than 10s; got 0s", - }, + expectedError: "timeout is not valid: duration must not be less than 10s; got 0s", }, { name: "wait status is not valid", @@ -287,9 +281,7 @@ func TestCmdLinkUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "status is not valid: value created not allowed. It should be one of this options: [ready configured none]", - }, + expectedError: "status is not valid: value created not allowed. It should be one of this options: [ready configured none]", }, } @@ -301,12 +293,7 @@ func TestCmdLinkUpdate_ValidateInput(t *testing.T) { command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/link/nonkube/link_delete.go b/internal/cmd/skupper/link/nonkube/link_delete.go index 4b90c047b..7e90879f8 100644 --- a/internal/cmd/skupper/link/nonkube/link_delete.go +++ b/internal/cmd/skupper/link/nonkube/link_delete.go @@ -2,6 +2,7 @@ package nonkube import ( "fmt" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" "github.com/spf13/cobra" ) @@ -21,8 +22,8 @@ func (cmd *CmdLinkDelete) NewClient(cobraCommand *cobra.Command, args []string) //TODO } -func (cmd *CmdLinkDelete) ValidateInput(args []string) []error { return nil } -func (cmd *CmdLinkDelete) InputToOptions() {} +func (cmd *CmdLinkDelete) ValidateInput(args []string) error { return nil } +func (cmd *CmdLinkDelete) InputToOptions() {} func (cmd *CmdLinkDelete) Run() error { return fmt.Errorf("command not supported by the selected platform") } diff --git a/internal/cmd/skupper/link/nonkube/link_generate.go b/internal/cmd/skupper/link/nonkube/link_generate.go index 9a8d99dd5..9839584d9 100644 --- a/internal/cmd/skupper/link/nonkube/link_generate.go +++ b/internal/cmd/skupper/link/nonkube/link_generate.go @@ -5,6 +5,7 @@ package nonkube import ( "fmt" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" "github.com/spf13/cobra" ) @@ -24,8 +25,8 @@ func (cmd *CmdLinkGenerate) NewClient(cobraCommand *cobra.Command, args []string //TODO } -func (cmd *CmdLinkGenerate) ValidateInput(args []string) []error { return nil } -func (cmd *CmdLinkGenerate) InputToOptions() {} +func (cmd *CmdLinkGenerate) ValidateInput(args []string) error { return nil } +func (cmd *CmdLinkGenerate) InputToOptions() {} func (cmd *CmdLinkGenerate) Run() error { return fmt.Errorf("command not supported by the selected platform") } diff --git a/internal/cmd/skupper/link/nonkube/link_status.go b/internal/cmd/skupper/link/nonkube/link_status.go index 5145d4ada..7fce2d534 100644 --- a/internal/cmd/skupper/link/nonkube/link_status.go +++ b/internal/cmd/skupper/link/nonkube/link_status.go @@ -2,6 +2,7 @@ package nonkube import ( "fmt" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" "github.com/spf13/cobra" ) @@ -21,8 +22,8 @@ func (cmd *CmdLinkStatus) NewClient(cobraCommand *cobra.Command, args []string) //TODO } -func (cmd *CmdLinkStatus) ValidateInput(args []string) []error { return nil } -func (cmd *CmdLinkStatus) InputToOptions() {} +func (cmd *CmdLinkStatus) ValidateInput(args []string) error { return nil } +func (cmd *CmdLinkStatus) InputToOptions() {} func (cmd *CmdLinkStatus) Run() error { return fmt.Errorf("command not supported by the selected platform") } diff --git a/internal/cmd/skupper/link/nonkube/link_update.go b/internal/cmd/skupper/link/nonkube/link_update.go index 1aac394d2..be50a0e37 100644 --- a/internal/cmd/skupper/link/nonkube/link_update.go +++ b/internal/cmd/skupper/link/nonkube/link_update.go @@ -6,6 +6,7 @@ package nonkube import ( "fmt" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" "github.com/spf13/cobra" ) @@ -25,8 +26,8 @@ func (cmd *CmdLinkUpdate) NewClient(cobraCommand *cobra.Command, args []string) //TODO } -func (cmd *CmdLinkUpdate) ValidateInput(args []string) []error { return nil } -func (cmd *CmdLinkUpdate) InputToOptions() {} +func (cmd *CmdLinkUpdate) ValidateInput(args []string) error { return nil } +func (cmd *CmdLinkUpdate) InputToOptions() {} func (cmd *CmdLinkUpdate) Run() error { return fmt.Errorf("command not supported by the selected platform") } diff --git a/internal/cmd/skupper/listener/kube/listener_create.go b/internal/cmd/skupper/listener/kube/listener_create.go index f2a767594..75a9b5b6e 100644 --- a/internal/cmd/skupper/listener/kube/listener_create.go +++ b/internal/cmd/skupper/listener/kube/listener_create.go @@ -2,11 +2,13 @@ package kube import ( "context" + "errors" "fmt" - "k8s.io/apimachinery/pkg/api/meta" "strconv" "time" + "k8s.io/apimachinery/pkg/api/meta" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" @@ -15,7 +17,7 @@ import ( "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" skupperv2alpha1 "github.com/skupperproject/skupper/pkg/generated/client/clientset/versioned/typed/skupper/v2alpha1" "github.com/spf13/cobra" - "k8s.io/apimachinery/pkg/api/errors" + k8serrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" ) @@ -45,14 +47,14 @@ func NewCmdListenerCreate() *CmdListenerCreate { func (cmd *CmdListenerCreate) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.client = cli.GetSkupperClient().SkupperV2alpha1() cmd.namespace = cli.Namespace cmd.KubeClient = cli.Kube } -func (cmd *CmdListenerCreate) ValidateInput(args []string) []error { +func (cmd *CmdListenerCreate) ValidateInput(args []string) error { var validationErrors []error resourceStringValidator := validator.NewResourceStringValidator() numberValidator := validator.NewNumberValidator() @@ -91,7 +93,7 @@ func (cmd *CmdListenerCreate) ValidateInput(args []string) []error { // Validate if there is already a listener with this name in the namespace if cmd.name != "" { listener, err := cmd.client.Listeners(cmd.namespace).Get(context.TODO(), cmd.name, metav1.GetOptions{}) - if listener != nil && !errors.IsNotFound(err) { + if listener != nil && !k8serrs.IsNotFound(err) { validationErrors = append(validationErrors, fmt.Errorf("there is already a listener %s created for namespace %s", cmd.name, cmd.namespace)) } } @@ -140,7 +142,7 @@ func (cmd *CmdListenerCreate) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdListenerCreate) InputToOptions() { diff --git a/internal/cmd/skupper/listener/kube/listener_create_test.go b/internal/cmd/skupper/listener/kube/listener_create_test.go index 38e2c4455..3d7f8a894 100644 --- a/internal/cmd/skupper/listener/kube/listener_create_test.go +++ b/internal/cmd/skupper/listener/kube/listener_create_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/skupperproject/skupper/internal/cmd/skupper/common" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" @@ -23,7 +24,7 @@ func TestCmdListenerCreate_ValidateInput(t *testing.T) { k8sObjects []runtime.Object skupperObjects []runtime.Object skupperErrorMessage string - expectedErrors []string + expectedError string } testTable := []test{ @@ -50,64 +51,61 @@ func TestCmdListenerCreate_ValidateInput(t *testing.T) { }, }, skupperErrorMessage: "AllReadyExists", - expectedErrors: []string{ - "there is already a listener my-listener created for namespace test"}, + expectedError: "there is already a listener my-listener created for namespace test", }, { - name: "listener name and port are not specified", - args: []string{}, - flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"listener name and port must be configured"}, + name: "listener name and port are not specified", + args: []string{}, + flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, + expectedError: "listener name and port must be configured", }, { - name: "listener name empty", - args: []string{"", "8090"}, - flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"listener name must not be empty"}, + name: "listener name empty", + args: []string{"", "8090"}, + flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, + expectedError: "listener name must not be empty", }, { - name: "listener port empty", - args: []string{"my-name-port-empty", ""}, - flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"listener port must not be empty"}, + name: "listener port empty", + args: []string{"my-name-port-empty", ""}, + flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, + expectedError: "listener port must not be empty", }, { - name: "listener port not positive", - args: []string{"my-port-positive", "-45"}, - flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"listener port is not valid: value is not positive"}, + name: "listener port not positive", + args: []string{"my-port-positive", "-45"}, + flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, + expectedError: "listener port is not valid: value is not positive", }, { - name: "listener name and port are not specified", - args: []string{}, - flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"listener name and port must be configured"}, + name: "listener name and port are not specified", + args: []string{}, + flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, + expectedError: "listener name and port must be configured", }, { - name: "listener port is not specified", - args: []string{"my-name"}, - flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"listener name and port must be configured"}, + name: "listener port is not specified", + args: []string{"my-name"}, + flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, + expectedError: "listener name and port must be configured", }, { - name: "more than two arguments are specified", - args: []string{"my", "listener", "8080"}, - flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"only two arguments are allowed for this command"}, + name: "more than two arguments are specified", + args: []string{"my", "listener", "8080"}, + flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, + expectedError: "only two arguments are allowed for this command", }, { - name: "listener name is not valid.", - args: []string{"my new listener", "8080"}, - flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{ - "listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "listener name is not valid.", + args: []string{"my new listener", "8080"}, + flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, + expectedError: "listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "port is not valid.", - args: []string{"my-listener-port", "abcd"}, - flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{ - "listener port is not valid: strconv.Atoi: parsing \"abcd\": invalid syntax"}, + name: "port is not valid.", + args: []string{"my-listener-port", "abcd"}, + flags: common.CommandListenerCreateFlags{Timeout: 1 * time.Minute}, + expectedError: "listener port is not valid: strconv.Atoi: parsing \"abcd\": invalid syntax", }, { name: "listener type is not valid", @@ -116,8 +114,7 @@ func TestCmdListenerCreate_ValidateInput(t *testing.T) { Timeout: 1 * time.Minute, ListenerType: "not-valid", }, - expectedErrors: []string{ - "listener type is not valid: value not-valid not allowed. It should be one of this options: [tcp]"}, + expectedError: "listener type is not valid: value not-valid not allowed. It should be one of this options: [tcp]", }, { name: "routing key is not valid", @@ -126,8 +123,7 @@ func TestCmdListenerCreate_ValidateInput(t *testing.T) { Timeout: 60 * time.Second, RoutingKey: "not-valid$", }, - expectedErrors: []string{ - "routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + expectedError: "routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { name: "tls-secret does not exist", @@ -136,13 +132,13 @@ func TestCmdListenerCreate_ValidateInput(t *testing.T) { Timeout: 1 * time.Minute, TlsCredentials: "not-valid", }, - expectedErrors: []string{"tls-secret is not valid: does not exist"}, + expectedError: "tls-secret is not valid: does not exist", }, { - name: "timeout is not valid", - args: []string{"bad-timeout", "8080"}, - flags: common.CommandListenerCreateFlags{Timeout: 0 * time.Second}, - expectedErrors: []string{"timeout is not valid: duration must not be less than 10s; got 0s"}, + name: "timeout is not valid", + args: []string{"bad-timeout", "8080"}, + flags: common.CommandListenerCreateFlags{Timeout: 0 * time.Second}, + expectedError: "timeout is not valid: duration must not be less than 10s; got 0s", }, { name: "output is not valid", @@ -151,8 +147,7 @@ func TestCmdListenerCreate_ValidateInput(t *testing.T) { Timeout: 30 * time.Second, Output: "not-supported", }, - expectedErrors: []string{ - "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]"}, + expectedError: "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]", }, { name: "flags all valid", @@ -191,15 +186,13 @@ func TestCmdListenerCreate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{}, + expectedError: "", }, { - name: "wait status is not valid", - args: []string{"my-listener-tls", "8080"}, - flags: common.CommandListenerCreateFlags{Timeout: time.Minute, Wait: "created"}, - expectedErrors: []string{ - "status is not valid: value created not allowed. It should be one of this options: [ready configured none]", - }, + name: "wait status is not valid", + args: []string{"my-listener-tls", "8080"}, + flags: common.CommandListenerCreateFlags{Timeout: time.Minute, Wait: "created"}, + expectedError: "status is not valid: value created not allowed. It should be one of this options: [ready configured none]", }, } @@ -211,12 +204,7 @@ func TestCmdListenerCreate_ValidateInput(t *testing.T) { command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/listener/kube/listener_delete.go b/internal/cmd/skupper/listener/kube/listener_delete.go index 7bc9c6aa4..3f4edce8c 100644 --- a/internal/cmd/skupper/listener/kube/listener_delete.go +++ b/internal/cmd/skupper/listener/kube/listener_delete.go @@ -2,6 +2,7 @@ package kube import ( "context" + "errors" "fmt" "github.com/skupperproject/skupper/internal/cmd/skupper/common" @@ -30,13 +31,13 @@ func NewCmdListenerDelete() *CmdListenerDelete { func (cmd *CmdListenerDelete) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.client = cli.GetSkupperClient().SkupperV2alpha1() cmd.namespace = cli.Namespace } -func (cmd *CmdListenerDelete) ValidateInput(args []string) []error { +func (cmd *CmdListenerDelete) ValidateInput(args []string) error { var validationErrors []error resourceStringValidator := validator.NewResourceStringValidator() timeoutValidator := validator.NewTimeoutInSecondsValidator() @@ -72,7 +73,7 @@ func (cmd *CmdListenerDelete) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdListenerDelete) Run() error { diff --git a/internal/cmd/skupper/listener/kube/listener_delete_test.go b/internal/cmd/skupper/listener/kube/listener_delete_test.go index 495ace8e0..66c3b1c59 100644 --- a/internal/cmd/skupper/listener/kube/listener_delete_test.go +++ b/internal/cmd/skupper/listener/kube/listener_delete_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/skupperproject/skupper/internal/cmd/skupper/common" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" @@ -22,39 +23,39 @@ func TestCmdListenerDelete_ValidateInput(t *testing.T) { k8sObjects []runtime.Object skupperObjects []runtime.Object skupperErrorMessage string - expectedErrors []string + expectedError string } testTable := []test{ { - name: "listener is not deleted because listener does not exist in the namespace", - args: []string{"my-listener"}, - flags: common.CommandListenerDeleteFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"listener my-listener does not exist in namespace test"}, + name: "listener is not deleted because listener does not exist in the namespace", + args: []string{"my-listener"}, + flags: common.CommandListenerDeleteFlags{Timeout: 1 * time.Minute}, + expectedError: "listener my-listener does not exist in namespace test", }, { - name: "listener name is not specified", - args: []string{}, - flags: common.CommandListenerDeleteFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"listener name must be specified"}, + name: "listener name is not specified", + args: []string{}, + flags: common.CommandListenerDeleteFlags{Timeout: 1 * time.Minute}, + expectedError: "listener name must be specified", }, { - name: "listener name is nil", - args: []string{""}, - flags: common.CommandListenerDeleteFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"listener name must not be empty"}, + name: "listener name is nil", + args: []string{""}, + flags: common.CommandListenerDeleteFlags{Timeout: 1 * time.Minute}, + expectedError: "listener name must not be empty", }, { - name: "listener name is not valid", - args: []string{"my name"}, - flags: common.CommandListenerDeleteFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "listener name is not valid", + args: []string{"my name"}, + flags: common.CommandListenerDeleteFlags{Timeout: 1 * time.Minute}, + expectedError: "listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "more than one argument is specified", - args: []string{"my", "listener"}, - flags: common.CommandListenerDeleteFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument is specified", + args: []string{"my", "listener"}, + flags: common.CommandListenerDeleteFlags{Timeout: 1 * time.Minute}, + expectedError: "only one argument is allowed for this command", }, { name: "timeout is not valid", @@ -79,7 +80,7 @@ func TestCmdListenerDelete_ValidateInput(t *testing.T) { }, flags: common.CommandListenerDeleteFlags{Timeout: 0 * time.Minute}, skupperErrorMessage: "timeout is not valid", - expectedErrors: []string{"timeout is not valid: duration must not be less than 10s; got 0s"}, + expectedError: "timeout is not valid: duration must not be less than 10s; got 0s", }, } @@ -91,11 +92,7 @@ func TestCmdListenerDelete_ValidateInput(t *testing.T) { command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/listener/kube/listener_status.go b/internal/cmd/skupper/listener/kube/listener_status.go index 6ea0b5998..ae985c9c7 100644 --- a/internal/cmd/skupper/listener/kube/listener_status.go +++ b/internal/cmd/skupper/listener/kube/listener_status.go @@ -2,6 +2,7 @@ package kube import ( "context" + "errors" "fmt" "os" "text/tabwriter" @@ -13,7 +14,7 @@ import ( "github.com/skupperproject/skupper/internal/utils/validator" skupperv2alpha1 "github.com/skupperproject/skupper/pkg/generated/client/clientset/versioned/typed/skupper/v2alpha1" "github.com/spf13/cobra" - "k8s.io/apimachinery/pkg/api/errors" + k8serrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -33,13 +34,13 @@ func NewCmdListenerStatus() *CmdListenerStatus { func (cmd *CmdListenerStatus) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.client = cli.GetSkupperClient().SkupperV2alpha1() cmd.namespace = cli.Namespace } -func (cmd *CmdListenerStatus) ValidateInput(args []string) []error { +func (cmd *CmdListenerStatus) ValidateInput(args []string) error { var validationErrors []error resourceStringValidator := validator.NewResourceStringValidator() outputTypeValidator := validator.NewOptionValidator(common.OutputTypes) @@ -77,7 +78,7 @@ func (cmd *CmdListenerStatus) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdListenerStatus) Run() error { if cmd.name == "" { @@ -107,7 +108,7 @@ func (cmd *CmdListenerStatus) Run() error { } } else { resource, err := cmd.client.Listeners(cmd.namespace).Get(context.TODO(), cmd.name, metav1.GetOptions{}) - if resource == nil || errors.IsNotFound(err) { + if resource == nil || k8serrs.IsNotFound(err) { fmt.Println("No listeners found") return err } diff --git a/internal/cmd/skupper/listener/kube/listener_status_test.go b/internal/cmd/skupper/listener/kube/listener_status_test.go index 521a7047b..1be494553 100644 --- a/internal/cmd/skupper/listener/kube/listener_status_test.go +++ b/internal/cmd/skupper/listener/kube/listener_status_test.go @@ -1,10 +1,10 @@ package kube import ( - "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" "testing" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "gotest.tools/v3/assert" @@ -19,33 +19,33 @@ func TestCmdListenerStatus_ValidateInput(t *testing.T) { flags common.CommandListenerStatusFlags k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } testTable := []test{ { - name: "listener is not shown because listener does not exist in the namespace", - args: []string{"my-listener"}, - expectedErrors: []string{"listener my-listener does not exist in namespace test"}, + name: "listener is not shown because listener does not exist in the namespace", + args: []string{"my-listener"}, + expectedError: "listener my-listener does not exist in namespace test", }, { - name: "listener name is nil", - args: []string{""}, - expectedErrors: []string{"listener name must not be empty"}, + name: "listener name is nil", + args: []string{""}, + expectedError: "listener name must not be empty", }, { - name: "more than one argument is specified", - args: []string{"my", "listener"}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument is specified", + args: []string{"my", "listener"}, + expectedError: "only one argument is allowed for this command", }, { - name: "listener name is not valid.", - args: []string{"my new listener"}, - expectedErrors: []string{"listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "listener name is not valid.", + args: []string{"my new listener"}, + expectedError: "listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "no args", - expectedErrors: []string{}, + name: "no args", + expectedError: "", }, { name: "bad output status", @@ -78,12 +78,12 @@ func TestCmdListenerStatus_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]"}, + expectedError: "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]", }, { - name: "good output status", - flags: common.CommandListenerStatusFlags{Output: "json"}, - expectedErrors: []string{}, + name: "good output status", + flags: common.CommandListenerStatusFlags{Output: "json"}, + expectedError: "", }, } @@ -95,11 +95,7 @@ func TestCmdListenerStatus_ValidateInput(t *testing.T) { command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/listener/kube/listener_update.go b/internal/cmd/skupper/listener/kube/listener_update.go index 5790c3e69..030f2c8ec 100644 --- a/internal/cmd/skupper/listener/kube/listener_update.go +++ b/internal/cmd/skupper/listener/kube/listener_update.go @@ -2,6 +2,7 @@ package kube import ( "context" + "errors" "fmt" "k8s.io/apimachinery/pkg/api/meta" "time" @@ -14,7 +15,7 @@ import ( "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" skupperv2alpha1 "github.com/skupperproject/skupper/pkg/generated/client/clientset/versioned/typed/skupper/v2alpha1" "github.com/spf13/cobra" - "k8s.io/apimachinery/pkg/api/errors" + k8serrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" ) @@ -47,14 +48,14 @@ func NewCmdListenerUpdate() *CmdListenerUpdate { func (cmd *CmdListenerUpdate) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.client = cli.GetSkupperClient().SkupperV2alpha1() cmd.namespace = cli.Namespace cmd.KubeClient = cli.Kube } -func (cmd *CmdListenerUpdate) ValidateInput(args []string) []error { +func (cmd *CmdListenerUpdate) ValidateInput(args []string) error { var validationErrors []error resourceStringValidator := validator.NewResourceStringValidator() numberValidator := validator.NewNumberValidator() @@ -82,7 +83,7 @@ func (cmd *CmdListenerUpdate) ValidateInput(args []string) []error { // Validate that there is already a listener with this name in the namespace if cmd.name != "" { listener, err := cmd.client.Listeners(cmd.namespace).Get(context.TODO(), cmd.name, metav1.GetOptions{}) - if listener == nil || errors.IsNotFound(err) { + if listener == nil || k8serrs.IsNotFound(err) { validationErrors = append(validationErrors, fmt.Errorf("listener %s must exist in namespace %s to be updated", cmd.name, cmd.namespace)) } else { // save existing values @@ -153,7 +154,7 @@ func (cmd *CmdListenerUpdate) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdListenerUpdate) Run() error { diff --git a/internal/cmd/skupper/listener/kube/listener_update_test.go b/internal/cmd/skupper/listener/kube/listener_update_test.go index 998a31a3a..408592210 100644 --- a/internal/cmd/skupper/listener/kube/listener_update_test.go +++ b/internal/cmd/skupper/listener/kube/listener_update_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/skupperproject/skupper/internal/cmd/skupper/common" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" @@ -22,39 +23,39 @@ func TestCmdListenerUpdate_ValidateInput(t *testing.T) { flags common.CommandListenerUpdateFlags k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } testTable := []test{ { - name: "listener is not updated because listener does not exist in the namespace", - args: []string{"my-listener"}, - flags: common.CommandListenerUpdateFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"listener my-listener must exist in namespace test to be updated"}, + name: "listener is not updated because listener does not exist in the namespace", + args: []string{"my-listener"}, + flags: common.CommandListenerUpdateFlags{Timeout: 1 * time.Minute}, + expectedError: "listener my-listener must exist in namespace test to be updated", }, { - name: "listener name is not specified", - args: []string{}, - flags: common.CommandListenerUpdateFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"listener name must be configured"}, + name: "listener name is not specified", + args: []string{}, + flags: common.CommandListenerUpdateFlags{Timeout: 1 * time.Minute}, + expectedError: "listener name must be configured", }, { - name: "listener name is nil", - args: []string{""}, - flags: common.CommandListenerUpdateFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"listener name must not be empty"}, + name: "listener name is nil", + args: []string{""}, + flags: common.CommandListenerUpdateFlags{Timeout: 1 * time.Minute}, + expectedError: "listener name must not be empty", }, { - name: "more than one argument is specified", - args: []string{"my", "listener"}, - flags: common.CommandListenerUpdateFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument is specified", + args: []string{"my", "listener"}, + flags: common.CommandListenerUpdateFlags{Timeout: 1 * time.Minute}, + expectedError: "only one argument is allowed for this command", }, { - name: "listener name is not valid.", - args: []string{"my new listener"}, - flags: common.CommandListenerUpdateFlags{Timeout: 1 * time.Minute}, - expectedErrors: []string{"listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "listener name is not valid.", + args: []string{"my new listener"}, + flags: common.CommandListenerUpdateFlags{Timeout: 1 * time.Minute}, + expectedError: "listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { name: "listener type is not valid", @@ -81,8 +82,7 @@ func TestCmdListenerUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "listener type is not valid: value not-valid not allowed. It should be one of this options: [tcp]"}, + expectedError: "listener type is not valid: value not-valid not allowed. It should be one of this options: [tcp]", }, { name: "routing key is not valid", @@ -108,8 +108,7 @@ func TestCmdListenerUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + expectedError: "routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { name: "tls-secret is not valid", @@ -136,7 +135,7 @@ func TestCmdListenerUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"tls-secret is not valid: does not exist"}, + expectedError: "tls-secret is not valid: does not exist", }, { name: "port is not valid", @@ -163,7 +162,7 @@ func TestCmdListenerUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"listener port is not valid: value is not positive"}, + expectedError: "listener port is not valid: value is not positive", }, { name: "timeout is not valid", @@ -187,7 +186,7 @@ func TestCmdListenerUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"timeout is not valid: duration must not be less than 10s; got 5s"}, + expectedError: "timeout is not valid: duration must not be less than 10s; got 5s", }, { name: "output is not valid", @@ -214,8 +213,7 @@ func TestCmdListenerUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]"}, + expectedError: "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]", }, { name: "flags all valid", @@ -255,7 +253,7 @@ func TestCmdListenerUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{}, + expectedError: "", }, { name: "wait status is not valid", @@ -280,9 +278,7 @@ func TestCmdListenerUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "status is not valid: value created not allowed. It should be one of this options: [ready configured none]", - }, + expectedError: "status is not valid: value created not allowed. It should be one of this options: [ready configured none]", }, } @@ -294,12 +290,7 @@ func TestCmdListenerUpdate_ValidateInput(t *testing.T) { command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/listener/nonkube/listener_create.go b/internal/cmd/skupper/listener/nonkube/listener_create.go index eca1861a1..7ba23bacc 100644 --- a/internal/cmd/skupper/listener/nonkube/listener_create.go +++ b/internal/cmd/skupper/listener/nonkube/listener_create.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "net" "strconv" @@ -40,7 +41,7 @@ func (cmd *CmdListenerCreate) NewClient(cobraCommand *cobra.Command, args []stri cmd.listenerHandler = fs.NewListenerHandler(cmd.namespace) } -func (cmd *CmdListenerCreate) ValidateInput(args []string) []error { +func (cmd *CmdListenerCreate) ValidateInput(args []string) error { var validationErrors []error if cmd.CobraCmd != nil && cmd.CobraCmd.Flag(common.FlagNameContext) != nil && cmd.CobraCmd.Flag(common.FlagNameContext).Value.String() != "" { @@ -120,7 +121,7 @@ func (cmd *CmdListenerCreate) ValidateInput(args []string) []error { validationErrors = append(validationErrors, fmt.Errorf("output type is not valid: %s", err)) } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdListenerCreate) InputToOptions() { diff --git a/internal/cmd/skupper/listener/nonkube/listener_create_test.go b/internal/cmd/skupper/listener/nonkube/listener_create_test.go index 3027cdf34..65d2e5c1b 100644 --- a/internal/cmd/skupper/listener/nonkube/listener_create_test.go +++ b/internal/cmd/skupper/listener/nonkube/listener_create_test.go @@ -4,7 +4,7 @@ import ( "testing" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/nonkube/client/fs" "github.com/spf13/cobra" @@ -20,87 +20,87 @@ func TestNonKubeCmdListenerCreate_ValidateInput(t *testing.T) { skupperObjects []runtime.Object flags *common.CommandListenerCreateFlags cobraGenericFlags map[string]string - expectedErrors []string + expectedError string } testTable := []test{ { - name: "listener name and port are not specified", - args: []string{}, - flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{"listener name and port must be configured"}, + name: "listener name and port are not specified", + args: []string{}, + flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, + expectedError: "listener name and port must be configured", }, { - name: "listener name is not valid", - args: []string{"my new Listener", "8080"}, - flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{"listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "listener name is not valid", + args: []string{"my new Listener", "8080"}, + flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, + expectedError: "listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "listener name is empty", - args: []string{"", "1234"}, - flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{"listener name must not be empty"}, + name: "listener name is empty", + args: []string{"", "1234"}, + flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, + expectedError: "listener name must not be empty", }, { - name: "listener port empty", - args: []string{"my-name-port-empty", ""}, - flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{"listener port must not be empty"}, + name: "listener port empty", + args: []string{"my-name-port-empty", ""}, + flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, + expectedError: "listener port must not be empty", }, { - name: "port is not valid", - args: []string{"my-listener-port", "abcd"}, - flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{"listener port is not valid: strconv.Atoi: parsing \"abcd\": invalid syntax"}, + name: "port is not valid", + args: []string{"my-listener-port", "abcd"}, + flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, + expectedError: "listener port is not valid: strconv.Atoi: parsing \"abcd\": invalid syntax", }, { - name: "listener port not positive", - args: []string{"my-port-positive", "-45"}, - flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{"listener port is not valid: value is not positive"}, + name: "listener port not positive", + args: []string{"my-port-positive", "-45"}, + flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, + expectedError: "listener port is not valid: value is not positive", }, { - name: "more than two arguments was specified", - args: []string{"my", "listener", "test"}, - flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{"only two arguments are allowed for this command"}, + name: "more than two arguments was specified", + args: []string{"my", "listener", "test"}, + flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, + expectedError: "only two arguments are allowed for this command", }, { - name: "type is not valid", - args: []string{"my-listener", "8080"}, - flags: &common.CommandListenerCreateFlags{ListenerType: "not-valid", Host: "1.2.3.4"}, - expectedErrors: []string{"listener type is not valid: value not-valid not allowed. It should be one of this options: [tcp]"}, + name: "type is not valid", + args: []string{"my-listener", "8080"}, + flags: &common.CommandListenerCreateFlags{ListenerType: "not-valid", Host: "1.2.3.4"}, + expectedError: "listener type is not valid: value not-valid not allowed. It should be one of this options: [tcp]", }, { - name: "routing key is not valid", - args: []string{"my-listener-rk", "8080"}, - flags: &common.CommandListenerCreateFlags{RoutingKey: "not-valid$", Host: "1.2.3.4"}, - expectedErrors: []string{"routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "routing key is not valid", + args: []string{"my-listener-rk", "8080"}, + flags: &common.CommandListenerCreateFlags{RoutingKey: "not-valid$", Host: "1.2.3.4"}, + expectedError: "routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "TlsCredentials key is not valid", - args: []string{"my-listener-tls", "8080"}, - flags: &common.CommandListenerCreateFlags{TlsCredentials: "not-valid$", Host: "1.2.3.4"}, - expectedErrors: []string{"tlsCredentials value is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "TlsCredentials key is not valid", + args: []string{"my-listener-tls", "8080"}, + flags: &common.CommandListenerCreateFlags{TlsCredentials: "not-valid$", Host: "1.2.3.4"}, + expectedError: "tlsCredentials value is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "host is not valid", - args: []string{"my-listener-host", "8080"}, - flags: &common.CommandListenerCreateFlags{Host: "not-valid$"}, - expectedErrors: []string{"host is not valid: a valid IP address or hostname is expected"}, + name: "host is not valid", + args: []string{"my-listener-host", "8080"}, + flags: &common.CommandListenerCreateFlags{Host: "not-valid$"}, + expectedError: "host is not valid: a valid IP address or hostname is expected", }, { - name: "output format is not valid", - args: []string{"my-listener", "8080"}, - flags: &common.CommandListenerCreateFlags{Output: "not-valid", Host: "1.2.3.4"}, - expectedErrors: []string{"output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]"}, + name: "output format is not valid", + args: []string{"my-listener", "8080"}, + flags: &common.CommandListenerCreateFlags{Output: "not-valid", Host: "1.2.3.4"}, + expectedError: "output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]", }, { - name: "kubernetes flags are not valid on this platform", - args: []string{"my-listener", "8080"}, - flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, - expectedErrors: []string{}, + name: "kubernetes flags are not valid on this platform", + args: []string{"my-listener", "8080"}, + flags: &common.CommandListenerCreateFlags{Host: "1.2.3.4"}, + expectedError: "", cobraGenericFlags: map[string]string{ common.FlagNameContext: "test", common.FlagNameKubeconfig: "test", @@ -116,7 +116,7 @@ func TestNonKubeCmdListenerCreate_ValidateInput(t *testing.T) { Output: "json", Host: "1.2.3.4", }, - expectedErrors: []string{}, + expectedError: "", }, } @@ -135,12 +135,7 @@ func TestNonKubeCmdListenerCreate_ValidateInput(t *testing.T) { } } - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/listener/nonkube/listener_delete.go b/internal/cmd/skupper/listener/nonkube/listener_delete.go index 1c719bf77..f5b53945b 100644 --- a/internal/cmd/skupper/listener/nonkube/listener_delete.go +++ b/internal/cmd/skupper/listener/nonkube/listener_delete.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "github.com/skupperproject/skupper/internal/cmd/skupper/common" @@ -30,7 +31,7 @@ func (cmd *CmdListenerDelete) NewClient(cobraCommand *cobra.Command, args []stri } -func (cmd *CmdListenerDelete) ValidateInput(args []string) []error { +func (cmd *CmdListenerDelete) ValidateInput(args []string) error { var validationErrors []error opts := fs.GetOptions{RuntimeFirst: false, LogWarning: false} @@ -68,7 +69,7 @@ func (cmd *CmdListenerDelete) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdListenerDelete) InputToOptions() { diff --git a/internal/cmd/skupper/listener/nonkube/listener_delete_test.go b/internal/cmd/skupper/listener/nonkube/listener_delete_test.go index 6405fc0f0..ea9baece2 100644 --- a/internal/cmd/skupper/listener/nonkube/listener_delete_test.go +++ b/internal/cmd/skupper/listener/nonkube/listener_delete_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/nonkube/client/fs" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "github.com/skupperproject/skupper/pkg/nonkube/api" @@ -24,7 +24,7 @@ func TestCmdListenerDelete_ValidateInput(t *testing.T) { cobraGenericFlags map[string]string k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } homeDir, err := os.UserHomeDir() @@ -33,40 +33,40 @@ func TestCmdListenerDelete_ValidateInput(t *testing.T) { testTable := []test{ { - name: "listener name is not specified", - args: []string{}, - flags: &common.CommandListenerDeleteFlags{}, - expectedErrors: []string{"listener name must be specified"}, + name: "listener name is not specified", + args: []string{}, + flags: &common.CommandListenerDeleteFlags{}, + expectedError: "listener name must be specified", }, { - name: "listener name is nil", - args: []string{""}, - flags: &common.CommandListenerDeleteFlags{}, - expectedErrors: []string{"listener name must not be empty"}, + name: "listener name is nil", + args: []string{""}, + flags: &common.CommandListenerDeleteFlags{}, + expectedError: "listener name must not be empty", }, { - name: "listener name is not valid", - args: []string{"my name"}, - flags: &common.CommandListenerDeleteFlags{}, - expectedErrors: []string{"listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "listener name is not valid", + args: []string{"my name"}, + flags: &common.CommandListenerDeleteFlags{}, + expectedError: "listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "more than one argument is specified", - args: []string{"my", "listener"}, - flags: &common.CommandListenerDeleteFlags{}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument is specified", + args: []string{"my", "listener"}, + flags: &common.CommandListenerDeleteFlags{}, + expectedError: "only one argument is allowed for this command", }, { - name: "listener doesn't exist", - args: []string{"no-listener"}, - flags: &common.CommandListenerDeleteFlags{}, - expectedErrors: []string{"listener no-listener does not exist"}, + name: "listener doesn't exist", + args: []string{"no-listener"}, + flags: &common.CommandListenerDeleteFlags{}, + expectedError: "listener no-listener does not exist", }, { - name: "kubernetes flags are not valid on this platform", - args: []string{"my-listener"}, - flags: &common.CommandListenerDeleteFlags{}, - expectedErrors: []string{}, + name: "kubernetes flags are not valid on this platform", + args: []string{"my-listener"}, + flags: &common.CommandListenerDeleteFlags{}, + expectedError: "", cobraGenericFlags: map[string]string{ common.FlagNameContext: "test", common.FlagNameKubeconfig: "test", @@ -110,12 +110,7 @@ func TestCmdListenerDelete_ValidateInput(t *testing.T) { } } - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/listener/nonkube/listener_status.go b/internal/cmd/skupper/listener/nonkube/listener_status.go index 6ba8ad761..f65d09295 100644 --- a/internal/cmd/skupper/listener/nonkube/listener_status.go +++ b/internal/cmd/skupper/listener/nonkube/listener_status.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "os" "text/tabwriter" @@ -33,7 +34,7 @@ func (cmd *CmdListenerStatus) NewClient(cobraCommand *cobra.Command, args []stri cmd.listenerHandler = fs.NewListenerHandler(cmd.namespace) } -func (cmd *CmdListenerStatus) ValidateInput(args []string) []error { +func (cmd *CmdListenerStatus) ValidateInput(args []string) error { var validationErrors []error opts := fs.GetOptions{RuntimeFirst: true, LogWarning: false} resourceStringValidator := validator.NewResourceStringValidator() @@ -71,7 +72,7 @@ func (cmd *CmdListenerStatus) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdListenerStatus) Run() error { diff --git a/internal/cmd/skupper/listener/nonkube/listener_status_test.go b/internal/cmd/skupper/listener/nonkube/listener_status_test.go index a9d92edee..a6409c1e2 100644 --- a/internal/cmd/skupper/listener/nonkube/listener_status_test.go +++ b/internal/cmd/skupper/listener/nonkube/listener_status_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/nonkube/client/fs" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "github.com/skupperproject/skupper/pkg/nonkube/api" @@ -23,7 +23,7 @@ func TestCmdListenerStatus_ValidateInput(t *testing.T) { cobraGenericFlags map[string]string k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } homeDir, err := os.UserHomeDir() @@ -32,45 +32,45 @@ func TestCmdListenerStatus_ValidateInput(t *testing.T) { testTable := []test{ { - name: "listener is not shown because listener does not exist in the namespace", - args: []string{"no-listener"}, - flags: &common.CommandListenerStatusFlags{}, - expectedErrors: []string{"listener no-listener does not exist in namespace test"}, + name: "listener is not shown because listener does not exist in the namespace", + args: []string{"no-listener"}, + flags: &common.CommandListenerStatusFlags{}, + expectedError: "listener no-listener does not exist in namespace test", }, { - name: "listener name is nil", - args: []string{""}, - flags: &common.CommandListenerStatusFlags{}, - expectedErrors: []string{"listener name must not be empty"}, + name: "listener name is nil", + args: []string{""}, + flags: &common.CommandListenerStatusFlags{}, + expectedError: "listener name must not be empty", }, { - name: "more than one argument is specified", - args: []string{"my", "listener"}, - flags: &common.CommandListenerStatusFlags{}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument is specified", + args: []string{"my", "listener"}, + flags: &common.CommandListenerStatusFlags{}, + expectedError: "only one argument is allowed for this command", }, { - name: "listener name is not valid.", - args: []string{"my new listener"}, - flags: &common.CommandListenerStatusFlags{}, - expectedErrors: []string{"listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "listener name is not valid.", + args: []string{"my new listener"}, + flags: &common.CommandListenerStatusFlags{}, + expectedError: "listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "no args", - flags: &common.CommandListenerStatusFlags{}, - expectedErrors: []string{}, + name: "no args", + flags: &common.CommandListenerStatusFlags{}, + expectedError: "", }, { - name: "bad output status", - args: []string{"my-listener"}, - flags: &common.CommandListenerStatusFlags{Output: "not-supported"}, - expectedErrors: []string{"output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]"}, + name: "bad output status", + args: []string{"my-listener"}, + flags: &common.CommandListenerStatusFlags{Output: "not-supported"}, + expectedError: "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]", }, { - name: "good output status", - args: []string{"my-listener"}, - flags: &common.CommandListenerStatusFlags{Output: "json"}, - expectedErrors: []string{}, + name: "good output status", + args: []string{"my-listener"}, + flags: &common.CommandListenerStatusFlags{Output: "json"}, + expectedError: "", }, } @@ -110,12 +110,7 @@ func TestCmdListenerStatus_ValidateInput(t *testing.T) { } } - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/listener/nonkube/listener_update.go b/internal/cmd/skupper/listener/nonkube/listener_update.go index 88b71cb53..761474448 100644 --- a/internal/cmd/skupper/listener/nonkube/listener_update.go +++ b/internal/cmd/skupper/listener/nonkube/listener_update.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "net" @@ -42,7 +43,7 @@ func (cmd *CmdListenerUpdate) NewClient(cobraCommand *cobra.Command, args []stri cmd.listenerHandler = fs.NewListenerHandler(cmd.namespace) } -func (cmd *CmdListenerUpdate) ValidateInput(args []string) []error { +func (cmd *CmdListenerUpdate) ValidateInput(args []string) error { var validationErrors []error opts := fs.GetOptions{RuntimeFirst: false, LogWarning: false} resourceStringValidator := validator.NewResourceStringValidator() @@ -141,7 +142,7 @@ func (cmd *CmdListenerUpdate) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdListenerUpdate) InputToOptions() { diff --git a/internal/cmd/skupper/listener/nonkube/listener_update_test.go b/internal/cmd/skupper/listener/nonkube/listener_update_test.go index 91d08ca2d..eec08956b 100644 --- a/internal/cmd/skupper/listener/nonkube/listener_update_test.go +++ b/internal/cmd/skupper/listener/nonkube/listener_update_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/nonkube/client/fs" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "github.com/skupperproject/skupper/pkg/nonkube/api" @@ -24,7 +24,7 @@ func TestCmdListenerUpdate_ValidateInput(t *testing.T) { k8sObjects []runtime.Object skupperObjects []runtime.Object cobraGenericFlags map[string]string - expectedErrors []string + expectedError string } homeDir, err := os.UserHomeDir() @@ -33,70 +33,70 @@ func TestCmdListenerUpdate_ValidateInput(t *testing.T) { testTable := []test{ { - name: "Listener is not updated because get listener returned error", - args: []string{"no-listener"}, - flags: &common.CommandListenerUpdateFlags{}, - expectedErrors: []string{"listener no-listener must exist in namespace test to be updated"}, + name: "Listener is not updated because get listener returned error", + args: []string{"no-listener"}, + flags: &common.CommandListenerUpdateFlags{}, + expectedError: "listener no-listener must exist in namespace test to be updated", }, { - name: "listener name is not specified", - args: []string{}, - flags: &common.CommandListenerUpdateFlags{}, - expectedErrors: []string{"listener name must be configured"}, + name: "listener name is not specified", + args: []string{}, + flags: &common.CommandListenerUpdateFlags{}, + expectedError: "listener name must be configured", }, { - name: "listener name is nil", - args: []string{""}, - flags: &common.CommandListenerUpdateFlags{}, - expectedErrors: []string{"listener name must not be empty"}, + name: "listener name is nil", + args: []string{""}, + flags: &common.CommandListenerUpdateFlags{}, + expectedError: "listener name must not be empty", }, { - name: "more than one argument is specified", - args: []string{"my", "listener"}, - flags: &common.CommandListenerUpdateFlags{}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument is specified", + args: []string{"my", "listener"}, + flags: &common.CommandListenerUpdateFlags{}, + expectedError: "only one argument is allowed for this command", }, { - name: "listener name is not valid.", - args: []string{"my new listener"}, - flags: &common.CommandListenerUpdateFlags{}, - expectedErrors: []string{"listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "listener name is not valid.", + args: []string{"my new listener"}, + flags: &common.CommandListenerUpdateFlags{}, + expectedError: "listener name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "listener type is not valid", - args: []string{"my-listener"}, - flags: &common.CommandListenerUpdateFlags{ListenerType: "not-valid"}, - expectedErrors: []string{"listener type is not valid: value not-valid not allowed. It should be one of this options: [tcp]"}, + name: "listener type is not valid", + args: []string{"my-listener"}, + flags: &common.CommandListenerUpdateFlags{ListenerType: "not-valid"}, + expectedError: "listener type is not valid: value not-valid not allowed. It should be one of this options: [tcp]", }, { - name: "routing key is not valid", - args: []string{"my-listener"}, - flags: &common.CommandListenerUpdateFlags{RoutingKey: "not-valid$"}, - expectedErrors: []string{"routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "routing key is not valid", + args: []string{"my-listener"}, + flags: &common.CommandListenerUpdateFlags{RoutingKey: "not-valid$"}, + expectedError: "routing key is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "TlsCredentials key is not valid", - args: []string{"my-listener"}, - flags: &common.CommandListenerUpdateFlags{TlsCredentials: "not-valid$", Host: "1.2.3.4"}, - expectedErrors: []string{"tlsCredentials value is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "TlsCredentials key is not valid", + args: []string{"my-listener"}, + flags: &common.CommandListenerUpdateFlags{TlsCredentials: "not-valid$", Host: "1.2.3.4"}, + expectedError: "tlsCredentials value is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "port is not valid", - args: []string{"my-listener"}, - flags: &common.CommandListenerUpdateFlags{Port: -1}, - expectedErrors: []string{"listener port is not valid: value is not positive"}, + name: "port is not valid", + args: []string{"my-listener"}, + flags: &common.CommandListenerUpdateFlags{Port: -1}, + expectedError: "listener port is not valid: value is not positive", }, { - name: "host is not valid", - args: []string{"my-listener"}, - flags: &common.CommandListenerUpdateFlags{Host: "not-valid$"}, - expectedErrors: []string{"host is not valid: a valid IP address or hostname is expected"}, + name: "host is not valid", + args: []string{"my-listener"}, + flags: &common.CommandListenerUpdateFlags{Host: "not-valid$"}, + expectedError: "host is not valid: a valid IP address or hostname is expected", }, { - name: "output is not valid", - args: []string{"my-listener"}, - flags: &common.CommandListenerUpdateFlags{Output: "not-supported"}, - expectedErrors: []string{"output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]"}, + name: "output is not valid", + args: []string{"my-listener"}, + flags: &common.CommandListenerUpdateFlags{Output: "not-supported"}, + expectedError: "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]", }, { name: "kubernetes flags are not valid on this platform", @@ -106,7 +106,7 @@ func TestCmdListenerUpdate_ValidateInput(t *testing.T) { common.FlagNameContext: "test", common.FlagNameKubeconfig: "test", }, - expectedErrors: []string{}, + expectedError: "", }, { name: "flags all valid", @@ -119,7 +119,7 @@ func TestCmdListenerUpdate_ValidateInput(t *testing.T) { Output: "json", Host: "1.2.3.4", }, - expectedErrors: []string{}, + expectedError: "", }, } @@ -160,12 +160,7 @@ func TestCmdListenerUpdate_ValidateInput(t *testing.T) { } } - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/site/kube/site_create.go b/internal/cmd/skupper/site/kube/site_create.go index 75250dfb9..014d14408 100644 --- a/internal/cmd/skupper/site/kube/site_create.go +++ b/internal/cmd/skupper/site/kube/site_create.go @@ -5,6 +5,7 @@ package kube import ( "context" + "errors" "fmt" "k8s.io/apimachinery/pkg/api/meta" "time" @@ -43,14 +44,14 @@ func NewCmdSiteCreate() *CmdSiteCreate { func (cmd *CmdSiteCreate) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.Client = cli.GetSkupperClient().SkupperV2alpha1() cmd.KubeClient = cli.GetKubeClient() cmd.Namespace = cli.Namespace } -func (cmd *CmdSiteCreate) ValidateInput(args []string) []error { +func (cmd *CmdSiteCreate) ValidateInput(args []string) error { var validationErrors []error resourceStringValidator := validator.NewResourceStringValidator() @@ -121,7 +122,7 @@ func (cmd *CmdSiteCreate) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdSiteCreate) InputToOptions() { diff --git a/internal/cmd/skupper/site/kube/site_create_test.go b/internal/cmd/skupper/site/kube/site_create_test.go index 2531ed8dd..d97666548 100644 --- a/internal/cmd/skupper/site/kube/site_create_test.go +++ b/internal/cmd/skupper/site/kube/site_create_test.go @@ -1,12 +1,14 @@ package kube import ( + "testing" + "time" + "github.com/skupperproject/skupper/api/types" "github.com/skupperproject/skupper/internal/cmd/skupper/common" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" - "testing" - "time" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "gotest.tools/v3/assert" @@ -21,7 +23,7 @@ func TestCmdSiteCreate_ValidateInput(t *testing.T) { k8sObjects []runtime.Object skupperObjects []runtime.Object flags *common.CommandSiteCreateFlags - expectedErrors []string + expectedError string } testTable := []test{ @@ -41,79 +43,71 @@ func TestCmdSiteCreate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"there is already a site created for this namespace"}, + expectedError: "there is already a site created for this namespace", }, { - name: "site name is not valid.", - args: []string{"my new site"}, - expectedErrors: []string{"site name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "site name is not valid.", + args: []string{"my new site"}, + expectedError: "site name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "site name is not specified.", - args: []string{}, - expectedErrors: []string{"site name must not be empty"}, + name: "site name is not specified.", + args: []string{}, + expectedError: "site name must not be empty", }, { - name: "more than one argument was specified", - args: []string{"my", "site"}, - expectedErrors: []string{"only one argument is allowed for this command."}, + name: "more than one argument was specified", + args: []string{"my", "site"}, + expectedError: "only one argument is allowed for this command.", }, { - name: "service account name is not valid.", - args: []string{"my-site"}, - flags: &common.CommandSiteCreateFlags{ServiceAccount: "not valid service account name", Timeout: time.Minute}, - expectedErrors: []string{"service account name is not valid: serviceaccounts \"not valid service account name\" not found"}, + name: "service account name is not valid.", + args: []string{"my-site"}, + flags: &common.CommandSiteCreateFlags{ServiceAccount: "not valid service account name", Timeout: time.Minute}, + expectedError: "service account name is not valid: serviceaccounts \"not valid service account name\" not found", }, { - name: "host name was specified, but this flag does not work on kube platforms", - args: []string{"my-site"}, - flags: &common.CommandSiteCreateFlags{Host: "host", Timeout: time.Minute}, - expectedErrors: []string{}, + name: "host name was specified, but this flag does not work on kube platforms", + args: []string{"my-site"}, + flags: &common.CommandSiteCreateFlags{Host: "host", Timeout: time.Minute}, + expectedError: "", }, { name: "link access type is not valid", args: []string{"my-site"}, flags: &common.CommandSiteCreateFlags{LinkAccessType: "not-valid", Timeout: time.Minute}, - expectedErrors: []string{ - "link access type is not valid: value not-valid not allowed. It should be one of this options: [route loadbalancer default]", + expectedError: "link access type is not valid: value not-valid not allowed. It should be one of this options: [route loadbalancer default]\n" + "for the site to work with this type of linkAccess, the --enable-link-access option must be set to true", - }, }, { - name: "output format is not valid", - args: []string{"my-site"}, - flags: &common.CommandSiteCreateFlags{Output: "not-valid", Timeout: time.Minute}, - expectedErrors: []string{ - "output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]", - }, + name: "output format is not valid", + args: []string{"my-site"}, + flags: &common.CommandSiteCreateFlags{Output: "not-valid", Timeout: time.Minute}, + expectedError: "output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]", }, { - name: "host flag is not valid for this platform", - args: []string{"my-site"}, - flags: &common.CommandSiteCreateFlags{Host: "host", Timeout: time.Minute}, - expectedErrors: []string{}, + name: "host flag is not valid for this platform", + args: []string{"my-site"}, + flags: &common.CommandSiteCreateFlags{Host: "host", Timeout: time.Minute}, + expectedError: "", }, { - name: "subject alternative names flag is not valid for this platform", - args: []string{"my-site"}, - flags: &common.CommandSiteCreateFlags{SubjectAlternativeNames: []string{"test"}, Timeout: time.Minute}, - expectedErrors: []string{}, + name: "subject alternative names flag is not valid for this platform", + args: []string{"my-site"}, + flags: &common.CommandSiteCreateFlags{SubjectAlternativeNames: []string{"test"}, Timeout: time.Minute}, + expectedError: "", }, { - name: "timeout is not valid", - args: []string{"my-site"}, - flags: &common.CommandSiteCreateFlags{Timeout: time.Second * 0}, - expectedErrors: []string{ - "timeout is not valid: duration must not be less than 10s; got 0s", - }, + name: "timeout is not valid", + args: []string{"my-site"}, + flags: &common.CommandSiteCreateFlags{Timeout: time.Second * 0}, + expectedError: "timeout is not valid: duration must not be less than 10s; got 0s", }, { - name: "wait status is not valid", - args: []string{"my-site"}, - flags: &common.CommandSiteCreateFlags{Timeout: time.Minute, Wait: "created"}, - expectedErrors: []string{ - "status is not valid: value created not allowed. It should be one of this options: [ready configured none]", - }, + name: "wait status is not valid", + args: []string{"my-site"}, + flags: &common.CommandSiteCreateFlags{Timeout: time.Minute, Wait: "created"}, + expectedError: "status is not valid: value created not allowed. It should be one of this options: [ready configured none]", }, } @@ -136,12 +130,7 @@ func TestCmdSiteCreate_ValidateInput(t *testing.T) { command.Flags = test.flags } - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/site/kube/site_delete.go b/internal/cmd/skupper/site/kube/site_delete.go index 9d8ebc95b..01dac6dda 100644 --- a/internal/cmd/skupper/site/kube/site_delete.go +++ b/internal/cmd/skupper/site/kube/site_delete.go @@ -2,6 +2,7 @@ package kube import ( "context" + "errors" "fmt" "time" @@ -35,13 +36,13 @@ func NewCmdSiteDelete() *CmdSiteDelete { func (cmd *CmdSiteDelete) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.Client = cli.GetSkupperClient().SkupperV2alpha1() cmd.Namespace = cli.Namespace } -func (cmd *CmdSiteDelete) ValidateInput(args []string) []error { +func (cmd *CmdSiteDelete) ValidateInput(args []string) error { var validationErrors []error timeoutValidator := validator.NewTimeoutInSecondsValidator() @@ -84,7 +85,7 @@ func (cmd *CmdSiteDelete) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdSiteDelete) InputToOptions() { cmd.timeout = cmd.Flags.Timeout diff --git a/internal/cmd/skupper/site/kube/site_delete_test.go b/internal/cmd/skupper/site/kube/site_delete_test.go index fd00ca150..53a03e86d 100644 --- a/internal/cmd/skupper/site/kube/site_delete_test.go +++ b/internal/cmd/skupper/site/kube/site_delete_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/skupperproject/skupper/internal/cmd/skupper/common" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" @@ -21,7 +22,7 @@ func TestCmdSiteDelete_ValidateInput(t *testing.T) { args []string k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string skupperError string flags *common.CommandSiteDeleteFlags } @@ -32,7 +33,7 @@ func TestCmdSiteDelete_ValidateInput(t *testing.T) { args: []string{"my-site"}, k8sObjects: nil, skupperObjects: nil, - expectedErrors: []string{"there is no existing Skupper site resource to delete"}, + expectedError: "there is no existing Skupper site resource to delete", skupperError: "", }, { @@ -40,7 +41,7 @@ func TestCmdSiteDelete_ValidateInput(t *testing.T) { args: []string{"my-site"}, k8sObjects: nil, skupperObjects: nil, - expectedErrors: []string{"error getting the site"}, + expectedError: "error getting the site", skupperError: "error getting the site", }, { @@ -60,8 +61,8 @@ func TestCmdSiteDelete_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"only one argument is allowed for this command"}, - skupperError: "", + expectedError: "only one argument is allowed for this command", + skupperError: "", }, { name: "there are several skupper sites and no site name was specified", @@ -91,8 +92,8 @@ func TestCmdSiteDelete_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"site name is required because there are several sites in this namespace"}, - skupperError: "", + expectedError: "site name is required because there are several sites in this namespace", + skupperError: "", }, { name: "there are several skupper sites but not the one specified by the user", @@ -122,8 +123,8 @@ func TestCmdSiteDelete_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"site with name \"special-site\" is not available"}, - skupperError: "", + expectedError: "site with name \"special-site\" is not available", + skupperError: "", }, { name: "there are several skupper sites and the user specifies one of them", @@ -153,8 +154,8 @@ func TestCmdSiteDelete_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{}, - skupperError: "", + expectedError: "", + skupperError: "", }, { name: "trying to delete a site that does not exist", @@ -173,8 +174,8 @@ func TestCmdSiteDelete_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"site with name \"siteb\" is not available"}, - skupperError: "", + expectedError: "site with name \"siteb\" is not available", + skupperError: "", }, { name: "deleting the site successfully", @@ -193,8 +194,8 @@ func TestCmdSiteDelete_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{}, - skupperError: "", + expectedError: "", + skupperError: "", }, { name: "timeout is not valid", @@ -214,8 +215,8 @@ func TestCmdSiteDelete_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"timeout is not valid: duration must not be less than 10s; got 0s"}, - skupperError: "", + expectedError: "timeout is not valid: duration must not be less than 10s; got 0s", + skupperError: "", }, } @@ -235,12 +236,7 @@ func TestCmdSiteDelete_ValidateInput(t *testing.T) { command.Flags = test.flags } - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/site/kube/site_status.go b/internal/cmd/skupper/site/kube/site_status.go index acc7946b8..472319f6d 100644 --- a/internal/cmd/skupper/site/kube/site_status.go +++ b/internal/cmd/skupper/site/kube/site_status.go @@ -2,6 +2,7 @@ package kube import ( "context" + "errors" "fmt" "os" "text/tabwriter" @@ -31,20 +32,18 @@ func NewCmdSiteStatus() *CmdSiteStatus { func (cmd *CmdSiteStatus) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.Client = cli.GetSkupperClient().SkupperV2alpha1() cmd.Namespace = cli.Namespace } -func (cmd *CmdSiteStatus) ValidateInput(args []string) []error { - var validationErrors []error - +func (cmd *CmdSiteStatus) ValidateInput(args []string) error { if len(args) > 0 { - validationErrors = append(validationErrors, fmt.Errorf("this command does not need any arguments")) + return errors.New("this command does not need any arguments") } - return validationErrors + return nil } func (cmd *CmdSiteStatus) InputToOptions() {} diff --git a/internal/cmd/skupper/site/kube/site_status_test.go b/internal/cmd/skupper/site/kube/site_status_test.go index 601a530ad..422c6c7cc 100644 --- a/internal/cmd/skupper/site/kube/site_status_test.go +++ b/internal/cmd/skupper/site/kube/site_status_test.go @@ -1,9 +1,9 @@ package kube import ( - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" "testing" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "gotest.tools/v3/assert" @@ -18,7 +18,7 @@ func TestCmdSiteStatus_ValidateInput(t *testing.T) { k8sObjects []runtime.Object skupperObjects []runtime.Object skupperError string - expectedErrors []string + expectedError string } testTable := []test{ @@ -28,7 +28,7 @@ func TestCmdSiteStatus_ValidateInput(t *testing.T) { k8sObjects: nil, skupperObjects: nil, skupperError: "", - expectedErrors: []string{"this command does not need any arguments"}, + expectedError: "this command does not need any arguments", }, } @@ -42,12 +42,7 @@ func TestCmdSiteStatus_ValidateInput(t *testing.T) { assert.Assert(t, err) command.Client = fakeSkupperClient.GetSkupperClient().SkupperV2alpha1() - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/site/kube/site_update.go b/internal/cmd/skupper/site/kube/site_update.go index 3f44b5c34..c49365fd1 100644 --- a/internal/cmd/skupper/site/kube/site_update.go +++ b/internal/cmd/skupper/site/kube/site_update.go @@ -2,12 +2,14 @@ package kube import ( "context" + "errors" "fmt" + "time" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/client-go/kubernetes" - "time" "github.com/skupperproject/skupper/internal/kube/client" "github.com/skupperproject/skupper/internal/utils/validator" @@ -40,14 +42,14 @@ func NewCmdSiteUpdate() *CmdSiteUpdate { func (cmd *CmdSiteUpdate) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.Client = cli.GetSkupperClient().SkupperV2alpha1() cmd.KubeClient = cli.GetKubeClient() cmd.Namespace = cli.Namespace } -func (cmd *CmdSiteUpdate) ValidateInput(args []string) []error { +func (cmd *CmdSiteUpdate) ValidateInput(args []string) error { var validationErrors []error linkAccessTypeValidator := validator.NewOptionValidator(common.LinkAccessTypes) @@ -127,7 +129,7 @@ func (cmd *CmdSiteUpdate) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdSiteUpdate) InputToOptions() { cmd.serviceAccountName = cmd.Flags.ServiceAccount diff --git a/internal/cmd/skupper/site/kube/site_update_test.go b/internal/cmd/skupper/site/kube/site_update_test.go index a96da0ad9..9c8a3fbcb 100644 --- a/internal/cmd/skupper/site/kube/site_update_test.go +++ b/internal/cmd/skupper/site/kube/site_update_test.go @@ -2,11 +2,13 @@ package kube import ( "fmt" - "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" "testing" "time" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "gotest.tools/v3/assert" @@ -22,7 +24,7 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { k8sObjects []runtime.Object skupperObjects []runtime.Object skupperError string - expectedErrors []string + expectedError string } testTable := []test{ @@ -44,8 +46,8 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { }, }, }, - skupperError: "", - expectedErrors: []string{}, + skupperError: "", + expectedError: "", }, { name: "site name is not specified.", @@ -65,8 +67,8 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { }, }, }, - skupperError: "", - expectedErrors: []string{}, + skupperError: "", + expectedError: "", }, { name: "more than one argument was specified", @@ -86,8 +88,8 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { }, }, }, - skupperError: "", - expectedErrors: []string{"only one argument is allowed for this command"}, + skupperError: "", + expectedError: "only one argument is allowed for this command", }, { name: "service account name is not valid.", @@ -107,8 +109,8 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { }, }, }, - skupperError: "", - expectedErrors: []string{"service account name is not valid: serviceaccounts \"not valid service account name\" not found"}, + skupperError: "", + expectedError: "service account name is not valid: serviceaccounts \"not valid service account name\" not found", }, { name: "host name was specified, but this flag does not work on kube platforms", @@ -127,7 +129,7 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"--host flag is not supported on this platform"}, + expectedError: "--host flag is not supported on this platform", }, { name: "link access type is not valid", @@ -148,10 +150,8 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { }, }, skupperError: "", - expectedErrors: []string{ - "link access type is not valid: value not-valid not allowed. It should be one of this options: [route loadbalancer default]", + expectedError: "link access type is not valid: value not-valid not allowed. It should be one of this options: [route loadbalancer default]\n" + "for the site to work with this type of linkAccess, the --enable-link-access option must be set to true", - }, }, { name: "output format is not valid", @@ -171,9 +171,7 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]", - }, + expectedError: "output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]", }, { name: "there is no skupper site", @@ -182,9 +180,7 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { k8sObjects: nil, skupperObjects: nil, skupperError: "", - expectedErrors: []string{ - "there is no existing Skupper site resource to update", - }, + expectedError: "there is no existing Skupper site resource to update", }, { name: "there are several skupper sites and no site name was specified", @@ -215,8 +211,8 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { }, }, }, - skupperError: "", - expectedErrors: []string{"site name is required because there are several sites in this namespace"}, + skupperError: "", + expectedError: "site name is required because there are several sites in this namespace", }, { name: "there are several skupper sites but not the one specified by the user", @@ -247,8 +243,8 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { }, }, }, - skupperError: "", - expectedErrors: []string{"site with name \"special-site\" is not available"}, + skupperError: "", + expectedError: "site with name \"special-site\" is not available", }, { name: "there are several skupper sites and the user specifies one of them", @@ -279,8 +275,8 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { }, }, }, - skupperError: "", - expectedErrors: []string{}, + skupperError: "", + expectedError: "", }, { name: "the name specified in the arguments does not match with the current site", @@ -300,10 +296,8 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { }, }, }, - skupperError: "", - expectedErrors: []string{ - "site with name \"a-site\" is not available", - }, + skupperError: "", + expectedError: "site with name \"a-site\" is not available", }, { name: "timeout format is not valid", @@ -323,9 +317,7 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "timeout is not valid: duration must not be less than 10s; got 0s", - }, + expectedError: "timeout is not valid: duration must not be less than 10s; got 0s", }, { name: "wait status is not valid", @@ -345,9 +337,7 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "status is not valid: value created not allowed. It should be one of this options: [ready configured none]", - }, + expectedError: "status is not valid: value created not allowed. It should be one of this options: [ready configured none]", }, } @@ -367,12 +357,7 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { command.Flags = test.flags } - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/site/nonkube/site_create.go b/internal/cmd/skupper/site/nonkube/site_create.go index ad8be434b..18c0a7955 100644 --- a/internal/cmd/skupper/site/nonkube/site_create.go +++ b/internal/cmd/skupper/site/nonkube/site_create.go @@ -4,6 +4,7 @@ Copyright © 2024 Skupper Team package nonkube import ( + "errors" "fmt" "net" @@ -45,7 +46,7 @@ func (cmd *CmdSiteCreate) NewClient(cobraCommand *cobra.Command, args []string) } -func (cmd *CmdSiteCreate) ValidateInput(args []string) []error { +func (cmd *CmdSiteCreate) ValidateInput(args []string) error { var validationErrors []error hostStringValidator := validator.NewHostStringValidator() resourceStringValidator := validator.NewResourceStringValidator() @@ -99,7 +100,7 @@ func (cmd *CmdSiteCreate) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdSiteCreate) InputToOptions() { diff --git a/internal/cmd/skupper/site/nonkube/site_create_test.go b/internal/cmd/skupper/site/nonkube/site_create_test.go index bfda53471..3f1f36ce7 100644 --- a/internal/cmd/skupper/site/nonkube/site_create_test.go +++ b/internal/cmd/skupper/site/nonkube/site_create_test.go @@ -4,7 +4,7 @@ import ( "testing" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/nonkube/client/fs" "github.com/spf13/cobra" "gotest.tools/v3/assert" @@ -19,59 +19,57 @@ func TestNonKubeCmdSiteCreate_ValidateInput(t *testing.T) { skupperObjects []runtime.Object flags *common.CommandSiteCreateFlags cobraGenericFlags map[string]string - expectedErrors []string + expectedError string } testTable := []test{ { - name: "site name is not valid.", - args: []string{"my new site"}, - flags: &common.CommandSiteCreateFlags{BindHost: "bindhost", EnableLinkAccess: true}, - expectedErrors: []string{"site name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "site name is not valid.", + args: []string{"my new site"}, + flags: &common.CommandSiteCreateFlags{BindHost: "bindhost", EnableLinkAccess: true}, + expectedError: "site name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "site name is not specified.", - args: []string{}, - flags: &common.CommandSiteCreateFlags{BindHost: "bindhost"}, - expectedErrors: []string{"site name must not be empty"}, + name: "site name is not specified.", + args: []string{}, + flags: &common.CommandSiteCreateFlags{BindHost: "bindhost"}, + expectedError: "site name must not be empty", }, { - name: "more than one argument was specified", - args: []string{"my", "site"}, - flags: &common.CommandSiteCreateFlags{BindHost: "bindhost"}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument was specified", + args: []string{"my", "site"}, + flags: &common.CommandSiteCreateFlags{BindHost: "bindhost"}, + expectedError: "only one argument is allowed for this command", }, { - name: "output format is not valid", - args: []string{"my-site"}, - flags: &common.CommandSiteCreateFlags{BindHost: "127.0.0.1", Output: "not-valid"}, - expectedErrors: []string{ - "output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]", - }, + name: "output format is not valid", + args: []string{"my-site"}, + flags: &common.CommandSiteCreateFlags{BindHost: "127.0.0.1", Output: "not-valid"}, + expectedError: "output type is not valid: value not-valid not allowed. It should be one of this options: [json yaml]", }, { - name: "bindHost was not specified ok", - args: []string{"my-site"}, - flags: &common.CommandSiteCreateFlags{EnableLinkAccess: true}, - expectedErrors: []string{}, + name: "bindHost was not specified ok", + args: []string{"my-site"}, + flags: &common.CommandSiteCreateFlags{EnableLinkAccess: true}, + expectedError: "", }, { - name: "bindHost was not valid", - args: []string{"my-site"}, - flags: &common.CommandSiteCreateFlags{EnableLinkAccess: true, BindHost: "not-valid$"}, - expectedErrors: []string{"bindhost is not valid: a valid IP address or hostname is expected"}, + name: "bindHost was not valid", + args: []string{"my-site"}, + flags: &common.CommandSiteCreateFlags{EnableLinkAccess: true, BindHost: "not-valid$"}, + expectedError: "bindhost is not valid: a valid IP address or hostname is expected", }, { - name: "subjectAlternativeNames was not valid", - args: []string{"my-site"}, - flags: &common.CommandSiteCreateFlags{EnableLinkAccess: true, BindHost: "not-valid", SubjectAlternativeNames: []string{"not-valid$"}}, - expectedErrors: []string{"SubjectAlternativeNames is not valid: a valid IP address or hostname is expected"}, + name: "subjectAlternativeNames was not valid", + args: []string{"my-site"}, + flags: &common.CommandSiteCreateFlags{EnableLinkAccess: true, BindHost: "not-valid", SubjectAlternativeNames: []string{"not-valid$"}}, + expectedError: "SubjectAlternativeNames is not valid: a valid IP address or hostname is expected", }, { - name: "kubernetes flags are not valid on this platform", - args: []string{"my-site"}, - flags: &common.CommandSiteCreateFlags{BindHost: "bindhost"}, - expectedErrors: []string{}, + name: "kubernetes flags are not valid on this platform", + args: []string{"my-site"}, + flags: &common.CommandSiteCreateFlags{BindHost: "bindhost"}, + expectedError: "", cobraGenericFlags: map[string]string{ common.FlagNameContext: "test", common.FlagNameKubeconfig: "test", @@ -86,7 +84,7 @@ func TestNonKubeCmdSiteCreate_ValidateInput(t *testing.T) { EnableLinkAccess: true, SubjectAlternativeNames: []string{"3.3.3.3"}, }, - expectedErrors: []string{}, + expectedError: "", }, } @@ -106,12 +104,7 @@ func TestNonKubeCmdSiteCreate_ValidateInput(t *testing.T) { } } - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/site/nonkube/site_delete.go b/internal/cmd/skupper/site/nonkube/site_delete.go index d7ba614c6..c9ef70fb7 100644 --- a/internal/cmd/skupper/site/nonkube/site_delete.go +++ b/internal/cmd/skupper/site/nonkube/site_delete.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "github.com/skupperproject/skupper/internal/cmd/skupper/common" @@ -31,7 +32,7 @@ func (cmd *CmdSiteDelete) NewClient(cobraCommand *cobra.Command, args []string) cmd.routerAccessHandler = fs.NewRouterAccessHandler(cmd.namespace) } -func (cmd *CmdSiteDelete) ValidateInput(args []string) []error { +func (cmd *CmdSiteDelete) ValidateInput(args []string) error { var validationErrors []error opts := fs.GetOptions{RuntimeFirst: false, LogWarning: false} resourceStringValidator := validator.NewResourceStringValidator() @@ -67,7 +68,7 @@ func (cmd *CmdSiteDelete) ValidateInput(args []string) []error { validationErrors = append(validationErrors, fmt.Errorf("site %s does not exist", cmd.siteName)) } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdSiteDelete) InputToOptions() { diff --git a/internal/cmd/skupper/site/nonkube/site_delete_test.go b/internal/cmd/skupper/site/nonkube/site_delete_test.go index ae2a437fb..3c7da5b87 100644 --- a/internal/cmd/skupper/site/nonkube/site_delete_test.go +++ b/internal/cmd/skupper/site/nonkube/site_delete_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/nonkube/client/fs" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "github.com/skupperproject/skupper/pkg/nonkube/api" @@ -24,7 +24,7 @@ func TestCmdSiteDelete_ValidateInput(t *testing.T) { cobraGenericFlags map[string]string k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } homeDir, err := os.UserHomeDir() @@ -33,40 +33,40 @@ func TestCmdSiteDelete_ValidateInput(t *testing.T) { testTable := []test{ { - name: "site name is not specified", - args: []string{}, - flags: &common.CommandSiteDeleteFlags{}, - expectedErrors: []string{"site name must be specified"}, + name: "site name is not specified", + args: []string{}, + flags: &common.CommandSiteDeleteFlags{}, + expectedError: "site name must be specified", }, { - name: "site name is nil", - args: []string{""}, - flags: &common.CommandSiteDeleteFlags{}, - expectedErrors: []string{"site name must not be empty"}, + name: "site name is nil", + args: []string{""}, + flags: &common.CommandSiteDeleteFlags{}, + expectedError: "site name must not be empty", }, { - name: "site name is not valid", - args: []string{"my name"}, - flags: &common.CommandSiteDeleteFlags{}, - expectedErrors: []string{"site name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "site name is not valid", + args: []string{"my name"}, + flags: &common.CommandSiteDeleteFlags{}, + expectedError: "site name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "more than one argument is specified", - args: []string{"my", "site"}, - flags: &common.CommandSiteDeleteFlags{}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument is specified", + args: []string{"my", "site"}, + flags: &common.CommandSiteDeleteFlags{}, + expectedError: "only one argument is allowed for this command", }, { - name: "site doesn't exist", - args: []string{"no-site"}, - flags: &common.CommandSiteDeleteFlags{}, - expectedErrors: []string{"site no-site does not exist"}, + name: "site doesn't exist", + args: []string{"no-site"}, + flags: &common.CommandSiteDeleteFlags{}, + expectedError: "site no-site does not exist", }, { - name: "kubernetes flags are not valid on this platform", - args: []string{"my-site"}, - flags: &common.CommandSiteDeleteFlags{}, - expectedErrors: []string{}, + name: "kubernetes flags are not valid on this platform", + args: []string{"my-site"}, + flags: &common.CommandSiteDeleteFlags{}, + expectedError: "", cobraGenericFlags: map[string]string{ common.FlagNameContext: "test", common.FlagNameKubeconfig: "test", @@ -110,12 +110,7 @@ func TestCmdSiteDelete_ValidateInput(t *testing.T) { } } - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/site/nonkube/site_status.go b/internal/cmd/skupper/site/nonkube/site_status.go index 36b931f3d..b26cf3c58 100644 --- a/internal/cmd/skupper/site/nonkube/site_status.go +++ b/internal/cmd/skupper/site/nonkube/site_status.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "os" "text/tabwriter" @@ -33,7 +34,7 @@ func (cmd *CmdSiteStatus) NewClient(cobraCommand *cobra.Command, args []string) cmd.siteHandler = fs.NewSiteHandler(cmd.namespace) } -func (cmd *CmdSiteStatus) ValidateInput(args []string) []error { +func (cmd *CmdSiteStatus) ValidateInput(args []string) error { var validationErrors []error opts := fs.GetOptions{RuntimeFirst: true, LogWarning: false} resourceStringValidator := validator.NewResourceStringValidator() @@ -71,7 +72,7 @@ func (cmd *CmdSiteStatus) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdSiteStatus) Run() error { diff --git a/internal/cmd/skupper/site/nonkube/site_status_test.go b/internal/cmd/skupper/site/nonkube/site_status_test.go index 42bec6e9a..b8172d5ec 100644 --- a/internal/cmd/skupper/site/nonkube/site_status_test.go +++ b/internal/cmd/skupper/site/nonkube/site_status_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/nonkube/client/fs" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "github.com/skupperproject/skupper/pkg/nonkube/api" @@ -23,7 +23,7 @@ func TestCmdSiteStatus_ValidateInput(t *testing.T) { cobraGenericFlags map[string]string k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } homeDir, err := os.UserHomeDir() @@ -32,40 +32,40 @@ func TestCmdSiteStatus_ValidateInput(t *testing.T) { testTable := []test{ { - name: "site does not exist in the namespace", - args: []string{"no-site"}, - expectedErrors: []string{"site no-site does not exist"}, + name: "site does not exist in the namespace", + args: []string{"no-site"}, + expectedError: "site no-site does not exist", }, { - name: "site name is nil", - args: []string{""}, - expectedErrors: []string{"site name must not be empty"}, + name: "site name is nil", + args: []string{""}, + expectedError: "site name must not be empty", }, { - name: "more than one argument is specified", - args: []string{"my", "site"}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument is specified", + args: []string{"my", "site"}, + expectedError: "only one argument is allowed for this command", }, { - name: "site name is not valid.", - args: []string{"my new site"}, - expectedErrors: []string{"site name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "site name is not valid.", + args: []string{"my new site"}, + expectedError: "site name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "no args", - expectedErrors: []string{}, + name: "no args", + expectedError: "", }, { - name: "bad output", - args: []string{"my-site"}, - flags: &common.CommandSiteStatusFlags{Output: "yaml$"}, - expectedErrors: []string{"output type is not valid: value yaml$ not allowed. It should be one of this options: [json yaml]"}, + name: "bad output", + args: []string{"my-site"}, + flags: &common.CommandSiteStatusFlags{Output: "yaml$"}, + expectedError: "output type is not valid: value yaml$ not allowed. It should be one of this options: [json yaml]", }, { - name: "good flags", - args: []string{"my-site"}, - flags: &common.CommandSiteStatusFlags{Output: "yaml"}, - expectedErrors: []string{}, + name: "good flags", + args: []string{"my-site"}, + flags: &common.CommandSiteStatusFlags{Output: "yaml"}, + expectedError: "", }, } @@ -104,12 +104,7 @@ func TestCmdSiteStatus_ValidateInput(t *testing.T) { } } - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/site/nonkube/site_update.go b/internal/cmd/skupper/site/nonkube/site_update.go index e156bdcff..26de4f7a5 100644 --- a/internal/cmd/skupper/site/nonkube/site_update.go +++ b/internal/cmd/skupper/site/nonkube/site_update.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "net" @@ -48,7 +49,7 @@ func (cmd *CmdSiteUpdate) NewClient(cobraCommand *cobra.Command, args []string) cmd.routerAccessHandler = fs.NewRouterAccessHandler(cmd.namespace) } -func (cmd *CmdSiteUpdate) ValidateInput(args []string) []error { +func (cmd *CmdSiteUpdate) ValidateInput(args []string) error { var validationErrors []error opts := fs.GetOptions{RuntimeFirst: false, LogWarning: false} resourceStringValidator := validator.NewResourceStringValidator() @@ -131,7 +132,7 @@ func (cmd *CmdSiteUpdate) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdSiteUpdate) InputToOptions() { diff --git a/internal/cmd/skupper/site/nonkube/site_update_test.go b/internal/cmd/skupper/site/nonkube/site_update_test.go index 05ef5d5ca..ed480bee7 100644 --- a/internal/cmd/skupper/site/nonkube/site_update_test.go +++ b/internal/cmd/skupper/site/nonkube/site_update_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/nonkube/client/fs" "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" "github.com/skupperproject/skupper/pkg/nonkube/api" @@ -24,7 +24,7 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { k8sObjects []runtime.Object skupperObjects []runtime.Object cobraGenericFlags map[string]string - expectedErrors []string + expectedError string } homeDir, err := os.UserHomeDir() @@ -33,52 +33,52 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { testTable := []test{ { - name: "site is not updated because get site returned error", - args: []string{"no-site"}, - flags: &common.CommandSiteUpdateFlags{}, - expectedErrors: []string{"site no-site must exist to be updated"}, + name: "site is not updated because get site returned error", + args: []string{"no-site"}, + flags: &common.CommandSiteUpdateFlags{}, + expectedError: "site no-site must exist to be updated", }, { - name: "site name is not specified", - args: []string{}, - flags: &common.CommandSiteUpdateFlags{}, - expectedErrors: []string{"site name must be configured"}, + name: "site name is not specified", + args: []string{}, + flags: &common.CommandSiteUpdateFlags{}, + expectedError: "site name must be configured", }, { - name: "site name is nil", - args: []string{""}, - flags: &common.CommandSiteUpdateFlags{}, - expectedErrors: []string{"site name must not be empty"}, + name: "site name is nil", + args: []string{""}, + flags: &common.CommandSiteUpdateFlags{}, + expectedError: "site name must not be empty", }, { - name: "more than one argument is specified", - args: []string{"my", "site"}, - flags: &common.CommandSiteUpdateFlags{}, - expectedErrors: []string{"only one argument is allowed for this command"}, + name: "more than one argument is specified", + args: []string{"my", "site"}, + flags: &common.CommandSiteUpdateFlags{}, + expectedError: "only one argument is allowed for this command", }, { - name: "site name is not valid.", - args: []string{"my new site"}, - flags: &common.CommandSiteUpdateFlags{}, - expectedErrors: []string{"site name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + name: "site name is not valid.", + args: []string{"my new site"}, + flags: &common.CommandSiteUpdateFlags{}, + expectedError: "site name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { - name: "bind-host is not valid", - args: []string{"my-site"}, - flags: &common.CommandSiteUpdateFlags{BindHost: "not-valid$"}, - expectedErrors: []string{"bindhost is not valid: a valid IP address or hostname is expected"}, + name: "bind-host is not valid", + args: []string{"my-site"}, + flags: &common.CommandSiteUpdateFlags{BindHost: "not-valid$"}, + expectedError: "bindhost is not valid: a valid IP address or hostname is expected", }, { - name: "subjectAlternativeNames are not valid", - args: []string{"my-site"}, - flags: &common.CommandSiteUpdateFlags{SubjectAlternativeNames: []string{"not-valid$"}}, - expectedErrors: []string{"SubjectAlternativeNames are not valid: a valid IP address or hostname is expected"}, + name: "subjectAlternativeNames are not valid", + args: []string{"my-site"}, + flags: &common.CommandSiteUpdateFlags{SubjectAlternativeNames: []string{"not-valid$"}}, + expectedError: "SubjectAlternativeNames are not valid: a valid IP address or hostname is expected", }, { - name: "output is not valid", - args: []string{"my-site"}, - flags: &common.CommandSiteUpdateFlags{Output: "not-supported"}, - expectedErrors: []string{"output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]"}, + name: "output is not valid", + args: []string{"my-site"}, + flags: &common.CommandSiteUpdateFlags{Output: "not-supported"}, + expectedError: "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]", }, { name: "kubernetes flags are not valid on this platform", @@ -88,7 +88,7 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { common.FlagNameContext: "test", common.FlagNameKubeconfig: "test", }, - expectedErrors: []string{}, + expectedError: "", }, { name: "flags all valid", @@ -99,7 +99,7 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { EnableLinkAccess: true, SubjectAlternativeNames: []string{"3.3.3.3"}, }, - expectedErrors: []string{}, + expectedError: "", }, } @@ -170,12 +170,7 @@ func TestCmdSiteUpdate_ValidateInput(t *testing.T) { } } - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/system/kube/system_reload.go b/internal/cmd/skupper/system/kube/system_reload.go index 7499a2a4e..b5f2256f9 100644 --- a/internal/cmd/skupper/system/kube/system_reload.go +++ b/internal/cmd/skupper/system/kube/system_reload.go @@ -2,6 +2,7 @@ package kube import ( "fmt" + skupperv2alpha1 "github.com/skupperproject/skupper/pkg/generated/client/clientset/versioned/typed/skupper/v2alpha1" "github.com/spf13/cobra" "k8s.io/client-go/kubernetes" @@ -23,7 +24,7 @@ func NewCmdSystemReload() *CmdSystemReload { func (cmd *CmdSystemReload) NewClient(cobraCommand *cobra.Command, args []string) {} -func (cmd *CmdSystemReload) ValidateInput(args []string) []error { return nil } +func (cmd *CmdSystemReload) ValidateInput(args []string) error { return nil } func (cmd *CmdSystemReload) InputToOptions() {} diff --git a/internal/cmd/skupper/system/kube/system_setup.go b/internal/cmd/skupper/system/kube/system_setup.go index 9c9c8cd20..396cc81ec 100644 --- a/internal/cmd/skupper/system/kube/system_setup.go +++ b/internal/cmd/skupper/system/kube/system_setup.go @@ -2,6 +2,7 @@ package kube import ( "fmt" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" skupperv2alpha1 "github.com/skupperproject/skupper/pkg/generated/client/clientset/versioned/typed/skupper/v2alpha1" "github.com/spf13/cobra" @@ -25,7 +26,7 @@ func NewCmdSystemSetup() *CmdSystemSetup { func (cmd *CmdSystemSetup) NewClient(cobraCommand *cobra.Command, args []string) {} -func (cmd *CmdSystemSetup) ValidateInput(args []string) []error { return nil } +func (cmd *CmdSystemSetup) ValidateInput(args []string) error { return nil } func (cmd *CmdSystemSetup) InputToOptions() {} diff --git a/internal/cmd/skupper/system/kube/system_start.go b/internal/cmd/skupper/system/kube/system_start.go index 971cf1849..e849f0ee5 100644 --- a/internal/cmd/skupper/system/kube/system_start.go +++ b/internal/cmd/skupper/system/kube/system_start.go @@ -2,6 +2,7 @@ package kube import ( "fmt" + skupperv2alpha1 "github.com/skupperproject/skupper/pkg/generated/client/clientset/versioned/typed/skupper/v2alpha1" "github.com/spf13/cobra" "k8s.io/client-go/kubernetes" @@ -23,7 +24,7 @@ func NewCmdCmdSystemStart() *CmdSystemStart { func (cmd *CmdSystemStart) NewClient(cobraCommand *cobra.Command, args []string) {} -func (cmd *CmdSystemStart) ValidateInput(args []string) []error { return nil } +func (cmd *CmdSystemStart) ValidateInput(args []string) error { return nil } func (cmd *CmdSystemStart) InputToOptions() {} diff --git a/internal/cmd/skupper/system/kube/system_stop.go b/internal/cmd/skupper/system/kube/system_stop.go index d4afe8c13..ab1a0e47b 100644 --- a/internal/cmd/skupper/system/kube/system_stop.go +++ b/internal/cmd/skupper/system/kube/system_stop.go @@ -2,6 +2,7 @@ package kube import ( "fmt" + skupperv2alpha1 "github.com/skupperproject/skupper/pkg/generated/client/clientset/versioned/typed/skupper/v2alpha1" "github.com/spf13/cobra" "k8s.io/client-go/kubernetes" @@ -23,7 +24,7 @@ func NewCmdSystemStop() *CmdSystemStop { func (cmd *CmdSystemStop) NewClient(cobraCommand *cobra.Command, args []string) {} -func (cmd *CmdSystemStop) ValidateInput(args []string) []error { return nil } +func (cmd *CmdSystemStop) ValidateInput(args []string) error { return nil } func (cmd *CmdSystemStop) InputToOptions() {} diff --git a/internal/cmd/skupper/system/kube/system_teardown.go b/internal/cmd/skupper/system/kube/system_teardown.go index 73d7c68de..7132bd7b6 100644 --- a/internal/cmd/skupper/system/kube/system_teardown.go +++ b/internal/cmd/skupper/system/kube/system_teardown.go @@ -2,6 +2,7 @@ package kube import ( "fmt" + skupperv2alpha1 "github.com/skupperproject/skupper/pkg/generated/client/clientset/versioned/typed/skupper/v2alpha1" "github.com/spf13/cobra" "k8s.io/client-go/kubernetes" @@ -23,7 +24,7 @@ func NewCmdSystemTeardown() *CmdSystemTeardown { func (cmd *CmdSystemTeardown) NewClient(cobraCommand *cobra.Command, args []string) {} -func (cmd *CmdSystemTeardown) ValidateInput(args []string) []error { return nil } +func (cmd *CmdSystemTeardown) ValidateInput(args []string) error { return nil } func (cmd *CmdSystemTeardown) InputToOptions() {} diff --git a/internal/cmd/skupper/system/nonkube/system_reload.go b/internal/cmd/skupper/system/nonkube/system_reload.go index dfda83564..3ddef2355 100644 --- a/internal/cmd/skupper/system/nonkube/system_reload.go +++ b/internal/cmd/skupper/system/nonkube/system_reload.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "github.com/skupperproject/skupper/api/types" @@ -33,14 +34,12 @@ func (cmd *CmdSystemReload) NewClient(cobraCommand *cobra.Command, args []string cmd.Namespace = cobraCommand.Flag("namespace").Value.String() } -func (cmd *CmdSystemReload) ValidateInput(args []string) []error { - var validationErrors []error - - if args != nil && len(args) > 0 { - validationErrors = append(validationErrors, fmt.Errorf("this command does not accept arguments")) +func (cmd *CmdSystemReload) ValidateInput(args []string) error { + if len(args) > 0 { + return errors.New("this command does not accept arguments") } - return validationErrors + return nil } func (cmd *CmdSystemReload) InputToOptions() { diff --git a/internal/cmd/skupper/system/nonkube/system_reload_test.go b/internal/cmd/skupper/system/nonkube/system_reload_test.go index 4fac99b37..41ceef090 100644 --- a/internal/cmd/skupper/system/nonkube/system_reload_test.go +++ b/internal/cmd/skupper/system/nonkube/system_reload_test.go @@ -2,28 +2,29 @@ package nonkube import ( "fmt" + "os" + "testing" + "github.com/skupperproject/skupper/api/types" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/pkg/nonkube/api" "github.com/skupperproject/skupper/pkg/nonkube/bootstrap" "gotest.tools/v3/assert" - "os" - "testing" ) func TestCmdSystemReload_ValidateInput(t *testing.T) { type test struct { - name string - args []string - expectedErrors []string + name string + args []string + expectedError string } testTable := []test{ { - name: "args-are-not-accepted", - args: []string{"something"}, - expectedErrors: []string{"this command does not accept arguments"}, + name: "args-are-not-accepted", + args: []string{"something"}, + expectedError: "this command does not accept arguments", }, } @@ -33,10 +34,7 @@ func TestCmdSystemReload_ValidateInput(t *testing.T) { command := &CmdSystemReload{} command.CobraCmd = common.ConfigureCobraCommand(types.PlatformLinux, common.SkupperCmdDescription{}, command, nil) - actualErrors := command.ValidateInput(test.args) - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/system/nonkube/system_setup.go b/internal/cmd/skupper/system/nonkube/system_setup.go index 74285eb07..2135b9f60 100644 --- a/internal/cmd/skupper/system/nonkube/system_setup.go +++ b/internal/cmd/skupper/system/nonkube/system_setup.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "os" "path/filepath" @@ -38,7 +39,7 @@ func (cmd *CmdSystemSetup) NewClient(cobraCommand *cobra.Command, args []string) cmd.Namespace = cobraCommand.Flag("namespace").Value.String() } -func (cmd *CmdSystemSetup) ValidateInput(args []string) []error { +func (cmd *CmdSystemSetup) ValidateInput(args []string) error { var validationErrors []error if args != nil && len(args) > 0 { @@ -77,7 +78,7 @@ func (cmd *CmdSystemSetup) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdSystemSetup) InputToOptions() { diff --git a/internal/cmd/skupper/system/nonkube/system_setup_test.go b/internal/cmd/skupper/system/nonkube/system_setup_test.go index 4c93554ae..0d9af386b 100644 --- a/internal/cmd/skupper/system/nonkube/system_setup_test.go +++ b/internal/cmd/skupper/system/nonkube/system_setup_test.go @@ -2,37 +2,38 @@ package nonkube import ( "fmt" + "os" + "strings" + "testing" + "github.com/skupperproject/skupper/api/types" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/pkg/nonkube/api" "github.com/skupperproject/skupper/pkg/nonkube/bootstrap" "gotest.tools/v3/assert" - "os" - "strings" - "testing" ) func TestCmdSystemSetup_ValidateInput(t *testing.T) { type test struct { - name string - args []string - flags *common.CommandSystemSetupFlags - expectedErrors []string + name string + args []string + flags *common.CommandSystemSetupFlags + expectedError string } testTable := []test{ { - name: "args-are-not-accepted", - args: []string{"something"}, - expectedErrors: []string{"this command does not accept arguments"}, + name: "args-are-not-accepted", + args: []string{"something"}, + expectedError: "this command does not accept arguments", }, { name: "invalid-bundle-strategy", flags: &common.CommandSystemSetupFlags{ Strategy: "not-valid", }, - expectedErrors: []string{"invalid bundle strategy: not-valid"}, + expectedError: "invalid bundle strategy: not-valid", }, } @@ -46,10 +47,7 @@ func TestCmdSystemSetup_ValidateInput(t *testing.T) { command.Flags = test.flags } - actualErrors := command.ValidateInput(test.args) - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/system/nonkube/system_start.go b/internal/cmd/skupper/system/nonkube/system_start.go index 027f820b3..2bbde238b 100644 --- a/internal/cmd/skupper/system/nonkube/system_start.go +++ b/internal/cmd/skupper/system/nonkube/system_start.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "github.com/skupperproject/skupper/pkg/nonkube/bootstrap" @@ -25,14 +26,12 @@ func (cmd *CmdSystemStart) NewClient(cobraCommand *cobra.Command, args []string) cmd.Namespace = cobraCommand.Flag("namespace").Value.String() } -func (cmd *CmdSystemStart) ValidateInput(args []string) []error { - var validationErrors []error - - if args != nil && len(args) > 0 { - validationErrors = append(validationErrors, fmt.Errorf("this command does not accept arguments")) +func (cmd *CmdSystemStart) ValidateInput(args []string) error { + if len(args) > 0 { + return errors.New("this command does not accept arguments") } - return validationErrors + return nil } func (cmd *CmdSystemStart) InputToOptions() { diff --git a/internal/cmd/skupper/system/nonkube/system_start_test.go b/internal/cmd/skupper/system/nonkube/system_start_test.go index e6852ca69..9871e3692 100644 --- a/internal/cmd/skupper/system/nonkube/system_start_test.go +++ b/internal/cmd/skupper/system/nonkube/system_start_test.go @@ -2,25 +2,26 @@ package nonkube import ( "fmt" + "testing" + "github.com/skupperproject/skupper/api/types" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "gotest.tools/v3/assert" - "testing" ) func TestCmdSystemStart_ValidateInput(t *testing.T) { type test struct { - name string - args []string - expectedErrors []string + name string + args []string + expectedError string } testTable := []test{ { - name: "arg-not-accepted", - args: []string{"namespace"}, - expectedErrors: []string{"this command does not accept arguments"}, + name: "arg-not-accepted", + args: []string{"namespace"}, + expectedError: "this command does not accept arguments", }, } @@ -30,10 +31,7 @@ func TestCmdSystemStart_ValidateInput(t *testing.T) { command := &CmdSystemStart{} command.CobraCmd = common.ConfigureCobraCommand(types.PlatformLinux, common.SkupperCmdDescription{}, command, nil) - actualErrors := command.ValidateInput(test.args) - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/system/nonkube/system_stop.go b/internal/cmd/skupper/system/nonkube/system_stop.go index c46f7c805..4fb4c8b8f 100644 --- a/internal/cmd/skupper/system/nonkube/system_stop.go +++ b/internal/cmd/skupper/system/nonkube/system_stop.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "github.com/skupperproject/skupper/pkg/nonkube/bootstrap" @@ -25,14 +26,12 @@ func (cmd *CmdSystemStop) NewClient(cobraCommand *cobra.Command, args []string) cmd.Namespace = cobraCommand.Flag("namespace").Value.String() } -func (cmd *CmdSystemStop) ValidateInput(args []string) []error { - var validationErrors []error - - if args != nil && len(args) > 0 { - validationErrors = append(validationErrors, fmt.Errorf("this command does not accept arguments")) +func (cmd *CmdSystemStop) ValidateInput(args []string) error { + if len(args) > 0 { + return errors.New("this command does not accept arguments") } - return validationErrors + return nil } func (cmd *CmdSystemStop) InputToOptions() { diff --git a/internal/cmd/skupper/system/nonkube/system_stop_test.go b/internal/cmd/skupper/system/nonkube/system_stop_test.go index 82436df3f..d037f8815 100644 --- a/internal/cmd/skupper/system/nonkube/system_stop_test.go +++ b/internal/cmd/skupper/system/nonkube/system_stop_test.go @@ -2,9 +2,10 @@ package nonkube import ( "fmt" + "github.com/skupperproject/skupper/api/types" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "gotest.tools/v3/assert" "testing" @@ -12,16 +13,16 @@ import ( func TestCmdSystemStop_ValidateInput(t *testing.T) { type test struct { - name string - args []string - expectedErrors []string + name string + args []string + expectedError string } testTable := []test{ { - name: "arg-not-accepted", - args: []string{"namespace"}, - expectedErrors: []string{"this command does not accept arguments"}, + name: "arg-not-accepted", + args: []string{"namespace"}, + expectedError: "this command does not accept arguments", }, } @@ -31,10 +32,7 @@ func TestCmdSystemStop_ValidateInput(t *testing.T) { command := &CmdSystemStop{} command.CobraCmd = common.ConfigureCobraCommand(types.PlatformLinux, common.SkupperCmdDescription{}, command, nil) - actualErrors := command.ValidateInput(test.args) - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/system/nonkube/system_teardown.go b/internal/cmd/skupper/system/nonkube/system_teardown.go index f3f693806..1194675d8 100644 --- a/internal/cmd/skupper/system/nonkube/system_teardown.go +++ b/internal/cmd/skupper/system/nonkube/system_teardown.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "github.com/skupperproject/skupper/internal/config" @@ -28,14 +29,12 @@ func (cmd *CmdSystemTeardown) NewClient(cobraCommand *cobra.Command, args []stri cmd.Platform = string(config.GetPlatform()) } -func (cmd *CmdSystemTeardown) ValidateInput(args []string) []error { - var validationErrors []error - - if args != nil && len(args) > 0 { - validationErrors = append(validationErrors, fmt.Errorf("this command does not accept arguments")) +func (cmd *CmdSystemTeardown) ValidateInput(args []string) error { + if len(args) > 0 { + return errors.New("this command does not accept arguments") } - return validationErrors + return nil } func (cmd *CmdSystemTeardown) InputToOptions() { diff --git a/internal/cmd/skupper/system/nonkube/system_teardown_test.go b/internal/cmd/skupper/system/nonkube/system_teardown_test.go index df977689f..8bf4fc74a 100644 --- a/internal/cmd/skupper/system/nonkube/system_teardown_test.go +++ b/internal/cmd/skupper/system/nonkube/system_teardown_test.go @@ -6,22 +6,22 @@ import ( "github.com/skupperproject/skupper/api/types" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "gotest.tools/v3/assert" ) func TestCmdSystemTearDown_ValidateInput(t *testing.T) { type test struct { - name string - args []string - expectedErrors []string + name string + args []string + expectedError string } testTable := []test{ { - name: "arg-not-accepted", - args: []string{"namespace"}, - expectedErrors: []string{"this command does not accept arguments"}, + name: "arg-not-accepted", + args: []string{"namespace"}, + expectedError: "this command does not accept arguments", }, } @@ -31,10 +31,7 @@ func TestCmdSystemTearDown_ValidateInput(t *testing.T) { command := &CmdSystemTeardown{} command.CobraCmd = common.ConfigureCobraCommand(types.PlatformLinux, common.SkupperCmdDescription{}, command, nil) - actualErrors := command.ValidateInput(test.args) - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/token/kube/token_issue.go b/internal/cmd/skupper/token/kube/token_issue.go index 1d327f519..65ea15177 100644 --- a/internal/cmd/skupper/token/kube/token_issue.go +++ b/internal/cmd/skupper/token/kube/token_issue.go @@ -2,6 +2,7 @@ package kube import ( "context" + "errors" "fmt" "os" "strconv" @@ -15,7 +16,7 @@ import ( "github.com/skupperproject/skupper/pkg/apis/skupper/v2alpha1" skupperv2alpha1 "github.com/skupperproject/skupper/pkg/generated/client/clientset/versioned/typed/skupper/v2alpha1" "github.com/spf13/cobra" - "k8s.io/apimachinery/pkg/api/errors" + k8serrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -37,13 +38,13 @@ func NewCmdTokenIssue() *CmdTokenIssue { func (cmd *CmdTokenIssue) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.client = cli.GetSkupperClient().SkupperV2alpha1() cmd.namespace = cli.Namespace } -func (cmd *CmdTokenIssue) ValidateInput(args []string) []error { +func (cmd *CmdTokenIssue) ValidateInput(args []string) error { var validationErrors []error resourceStringValidator := validator.NewResourceStringValidator() tokenStringValidator := validator.NewFilePathStringValidator() @@ -97,7 +98,7 @@ func (cmd *CmdTokenIssue) ValidateInput(args []string) []error { // Validate if we already have a token with this name in the namespace if cmd.grantName != "" { grant, err := cmd.client.AccessGrants(cmd.namespace).Get(context.TODO(), cmd.grantName, metav1.GetOptions{}) - if grant != nil && !errors.IsNotFound(err) { + if grant != nil && !k8serrs.IsNotFound(err) { validationErrors = append(validationErrors, fmt.Errorf("there is already a token %s created in namespace %s", cmd.grantName, cmd.namespace)) } } @@ -132,7 +133,7 @@ func (cmd *CmdTokenIssue) ValidateInput(args []string) []error { cmd.cost = selectedCost } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdTokenIssue) Run() error { diff --git a/internal/cmd/skupper/token/kube/token_issue_test.go b/internal/cmd/skupper/token/kube/token_issue_test.go index 3c90f3762..e650ab3fb 100644 --- a/internal/cmd/skupper/token/kube/token_issue_test.go +++ b/internal/cmd/skupper/token/kube/token_issue_test.go @@ -6,6 +6,7 @@ import ( "time" "github.com/skupperproject/skupper/internal/cmd/skupper/common" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" @@ -22,7 +23,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { flags common.CommandTokenIssueFlags k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } testTable := []test{ @@ -86,7 +87,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"there is already a token my-token created in namespace test"}, + expectedError: "there is already a token my-token created in namespace test", }, { name: "token no site", @@ -115,7 +116,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"A site must exist in namespace test before a token can be created"}, + expectedError: "A site must exist in namespace test before a token can be created", }, { name: "token no site with OK status", @@ -147,7 +148,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"there is no active skupper site in this namespace"}, + expectedError: "there is no active skupper site in this namespace", }, { name: "file name is not specified", @@ -188,7 +189,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"file name must be configured"}, + expectedError: "file name must be configured", }, { name: "token file name empty", @@ -229,7 +230,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"file name must not be empty"}, + expectedError: "file name must not be empty", }, { name: "more than one arguments is specified", @@ -270,7 +271,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"only one argument is allowed for this command"}, + expectedError: "only one argument is allowed for this command", }, { name: "token name is not valid.", @@ -312,7 +313,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"token name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$"}, + expectedError: "token name is not valid: value does not match this regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])*(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])*)*$", }, { name: "token file name is not valid.", @@ -353,7 +354,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"token file name is not valid: value does not match this regular expression: ^[A-Za-z0-9./~-]+$"}, + expectedError: "token file name is not valid: value does not match this regular expression: ^[A-Za-z0-9./~-]+$", }, { name: "token name is a directory.", @@ -394,7 +395,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"token file name is a directory"}, + expectedError: "token file name is a directory", }, { name: "redemptions is not valid", @@ -435,8 +436,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "number of redemptions is not valid"}, + expectedError: "number of redemptions is not valid", }, { name: "expiration is not valid", @@ -477,7 +477,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"expiration time is not valid: duration must not be less than 1m0s; got 10s"}, + expectedError: "expiration time is not valid: duration must not be less than 1m0s; got 10s", }, { name: "timeout is not valid", @@ -518,7 +518,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"timeout is not valid: duration must not be less than 10s; got 0s"}, + expectedError: "timeout is not valid: duration must not be less than 10s; got 0s", }, { name: "cost is not valid", @@ -559,8 +559,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - `link cost is not valid: strconv.Atoi: parsing "Not-valid": invalid syntax`}, + expectedError: `link cost is not valid: strconv.Atoi: parsing "Not-valid": invalid syntax`, }, { name: "flags all valid", @@ -621,7 +620,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{}, + expectedError: "", }, } @@ -633,12 +632,7 @@ func TestCmdTokenIssue_ValidateInput(t *testing.T) { command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/token/kube/token_redeem.go b/internal/cmd/skupper/token/kube/token_redeem.go index 8f152eb9a..5205a2307 100644 --- a/internal/cmd/skupper/token/kube/token_redeem.go +++ b/internal/cmd/skupper/token/kube/token_redeem.go @@ -2,6 +2,7 @@ package kube import ( "context" + "errors" "fmt" "os" @@ -33,13 +34,13 @@ func NewCmdTokenRedeem() *CmdTokenRedeem { func (cmd *CmdTokenRedeem) NewClient(cobraCommand *cobra.Command, args []string) { cli, err := client.NewClient(cobraCommand.Flag("namespace").Value.String(), cobraCommand.Flag("context").Value.String(), cobraCommand.Flag("kubeconfig").Value.String()) - utils.HandleError(err) + utils.HandleError(utils.GenericError, err) cmd.client = cli.GetSkupperClient().SkupperV2alpha1() cmd.namespace = cli.Namespace } -func (cmd *CmdTokenRedeem) ValidateInput(args []string) []error { +func (cmd *CmdTokenRedeem) ValidateInput(args []string) error { var validationErrors []error tokenStringValidator := validator.NewFilePathStringValidator() timeoutValidator := validator.NewTimeoutInSecondsValidator() @@ -86,7 +87,7 @@ func (cmd *CmdTokenRedeem) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdTokenRedeem) Run() error { diff --git a/internal/cmd/skupper/token/kube/token_redeem_test.go b/internal/cmd/skupper/token/kube/token_redeem_test.go index c0ec2cb3f..ae8ee34d2 100644 --- a/internal/cmd/skupper/token/kube/token_redeem_test.go +++ b/internal/cmd/skupper/token/kube/token_redeem_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/skupperproject/skupper/internal/cmd/skupper/common" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" fakeclient "github.com/skupperproject/skupper/internal/kube/client/fake" @@ -26,7 +27,7 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { flags common.CommandTokenRedeemFlags k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } // create temp token file for tests @@ -37,10 +38,10 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { testTable := []test{ { - name: "token no site", - args: []string{"/tmp/token-redeem.yaml"}, - flags: common.CommandTokenRedeemFlags{Timeout: 60 * time.Second}, - expectedErrors: []string{"A site must exist in namespace test before a token can be redeemed"}, + name: "token no site", + args: []string{"/tmp/token-redeem.yaml"}, + flags: common.CommandTokenRedeemFlags{Timeout: 60 * time.Second}, + expectedError: "A site must exist in namespace test before a token can be redeemed", }, { name: "token not site ok", @@ -50,7 +51,7 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { &v2alpha1.SiteList{ Items: []v2alpha1.Site{ { - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "the-site", Namespace: "test", }, @@ -58,7 +59,7 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"there is no active skupper site in this namespace"}, + expectedError: "there is no active skupper site in this namespace", }, { name: "file name is not specified", @@ -68,7 +69,7 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { &v2alpha1.SiteList{ Items: []v2alpha1.Site{ { - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "the-site", Namespace: "test", }, @@ -94,7 +95,7 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"token file name must be configured"}, + expectedError: "token file name must be configured", }, { name: "file name empty", @@ -104,7 +105,7 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { &v2alpha1.SiteList{ Items: []v2alpha1.Site{ { - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "the-site", Namespace: "test", }, @@ -130,7 +131,7 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"file name must not be empty"}, + expectedError: "file name must not be empty", }, { name: "more than one argument is specified", @@ -140,7 +141,7 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { &v2alpha1.SiteList{ Items: []v2alpha1.Site{ { - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "the-site", Namespace: "test", }, @@ -166,7 +167,7 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"only one argument is allowed for this command"}, + expectedError: "only one argument is allowed for this command", }, { name: "token file name is not valid.", @@ -176,7 +177,7 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { &v2alpha1.SiteList{ Items: []v2alpha1.Site{ { - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "the-site", Namespace: "test", }, @@ -202,7 +203,7 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{"token file name is not valid: value does not match this regular expression: ^[A-Za-z0-9./~-]+$"}, + expectedError: "token file name is not valid: value does not match this regular expression: ^[A-Za-z0-9./~-]+$", }, { name: "timeout is not valid", @@ -212,7 +213,7 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { &v2alpha1.SiteList{ Items: []v2alpha1.Site{ { - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "the-site", Namespace: "test", }, @@ -238,10 +239,8 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{ - "token file does not exist: stat ~/token.yaml: no such file or directory", + expectedError: "token file does not exist: stat ~/token.yaml: no such file or directory\n" + "timeout is not valid: duration must not be less than 10s; got 0s"}, - }, { name: "flags all valid", args: []string{"/tmp/token-redeem.yaml"}, @@ -250,7 +249,7 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { &v2alpha1.SiteList{ Items: []v2alpha1.Site{ { - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "the-site", Namespace: "test", }, @@ -276,7 +275,7 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { }, }, }, - expectedErrors: []string{}, + expectedError: "", }, } @@ -288,12 +287,7 @@ func TestCmdTokenRedeem_ValidateInput(t *testing.T) { command.Flags = &test.flags - actualErrors := command.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) - + testutils.CheckValidateInput(t, command, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/token/nonkube/token_issue.go b/internal/cmd/skupper/token/nonkube/token_issue.go index 9fd3c2f87..cedbf45cd 100644 --- a/internal/cmd/skupper/token/nonkube/token_issue.go +++ b/internal/cmd/skupper/token/nonkube/token_issue.go @@ -2,6 +2,7 @@ package nonkube import ( "fmt" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" "github.com/spf13/cobra" ) @@ -21,8 +22,8 @@ func (cmd *CmdTokenIssue) NewClient(cobraCommand *cobra.Command, args []string) //TODO } -func (cmd *CmdTokenIssue) ValidateInput(args []string) []error { return nil } -func (cmd *CmdTokenIssue) InputToOptions() {} +func (cmd *CmdTokenIssue) ValidateInput(args []string) error { return nil } +func (cmd *CmdTokenIssue) InputToOptions() {} func (cmd *CmdTokenIssue) Run() error { return fmt.Errorf("command not supported by the selected platform") } diff --git a/internal/cmd/skupper/token/nonkube/token_redeem.go b/internal/cmd/skupper/token/nonkube/token_redeem.go index 4972f4904..1fe651cdf 100644 --- a/internal/cmd/skupper/token/nonkube/token_redeem.go +++ b/internal/cmd/skupper/token/nonkube/token_redeem.go @@ -2,6 +2,7 @@ package nonkube import ( "fmt" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" "github.com/spf13/cobra" ) @@ -21,8 +22,8 @@ func (cmd *CmdTokenRedeem) NewClient(cobraCommand *cobra.Command, args []string) //TODO } -func (cmd *CmdTokenRedeem) ValidateInput(args []string) []error { return nil } -func (cmd *CmdTokenRedeem) InputToOptions() {} +func (cmd *CmdTokenRedeem) ValidateInput(args []string) error { return nil } +func (cmd *CmdTokenRedeem) InputToOptions() {} func (cmd *CmdTokenRedeem) Run() error { return fmt.Errorf("command not supported by the selected platform") } diff --git a/internal/cmd/skupper/version/kube/version.go b/internal/cmd/skupper/version/kube/version.go index fd28a7565..265611e57 100644 --- a/internal/cmd/skupper/version/kube/version.go +++ b/internal/cmd/skupper/version/kube/version.go @@ -2,11 +2,13 @@ package kube import ( "context" + "errors" "fmt" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "os" "text/tabwriter" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/skupperproject/skupper/internal/cmd/skupper/common" "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" "github.com/skupperproject/skupper/internal/images" @@ -44,7 +46,7 @@ func (cmd *CmdVersion) NewClient(cobraCommand *cobra.Command, args []string) { } } -func (cmd *CmdVersion) ValidateInput(args []string) []error { +func (cmd *CmdVersion) ValidateInput(args []string) error { var validationErrors []error outputTypeValidator := validator.NewOptionValidator(common.OutputTypes) @@ -57,7 +59,7 @@ func (cmd *CmdVersion) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdVersion) InputToOptions() { diff --git a/internal/cmd/skupper/version/kube/version_test.go b/internal/cmd/skupper/version/kube/version_test.go index ebc2c455f..b77ffa327 100644 --- a/internal/cmd/skupper/version/kube/version_test.go +++ b/internal/cmd/skupper/version/kube/version_test.go @@ -4,7 +4,7 @@ import ( "testing" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/utils/configs" "gotest.tools/v3/assert" "k8s.io/apimachinery/pkg/runtime" @@ -21,25 +21,25 @@ func TestCmdVersion_ValidateInput(t *testing.T) { flags common.CommandVersionFlags k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } testTable := []test{ { - name: "bad output", - args: []string{""}, - flags: common.CommandVersionFlags{Output: "not-supported"}, - expectedErrors: []string{"output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]"}, + name: "bad output", + args: []string{""}, + flags: common.CommandVersionFlags{Output: "not-supported"}, + expectedError: "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]", }, { - name: "good output", - flags: common.CommandVersionFlags{Output: "json"}, - expectedErrors: []string{}, + name: "good output", + flags: common.CommandVersionFlags{Output: "json"}, + expectedError: "", }, { - name: "ok no output", - flags: common.CommandVersionFlags{}, - expectedErrors: []string{}, + name: "ok no output", + flags: common.CommandVersionFlags{}, + expectedError: "", }, } @@ -51,11 +51,7 @@ func TestCmdVersion_ValidateInput(t *testing.T) { cmd.Flags = &test.flags - actualErrors := cmd.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) + testutils.CheckValidateInput(t, cmd, test.expectedError, test.args) }) } } diff --git a/internal/cmd/skupper/version/nonkube/version.go b/internal/cmd/skupper/version/nonkube/version.go index c96bedd6e..76c669e36 100644 --- a/internal/cmd/skupper/version/nonkube/version.go +++ b/internal/cmd/skupper/version/nonkube/version.go @@ -1,6 +1,7 @@ package nonkube import ( + "errors" "fmt" "os" "text/tabwriter" @@ -41,7 +42,7 @@ func (cmd *CmdVersion) NewClient(cobraCommand *cobra.Command, args []string) { } } -func (cmd *CmdVersion) ValidateInput(args []string) []error { +func (cmd *CmdVersion) ValidateInput(args []string) error { var validationErrors []error outputTypeValidator := validator.NewOptionValidator(common.OutputTypes) @@ -54,7 +55,7 @@ func (cmd *CmdVersion) ValidateInput(args []string) []error { } } - return validationErrors + return errors.Join(validationErrors...) } func (cmd *CmdVersion) InputToOptions() { diff --git a/internal/cmd/skupper/version/nonkube/version_test.go b/internal/cmd/skupper/version/nonkube/version_test.go index 4f847e269..01f9850ea 100644 --- a/internal/cmd/skupper/version/nonkube/version_test.go +++ b/internal/cmd/skupper/version/nonkube/version_test.go @@ -4,7 +4,7 @@ import ( "testing" "github.com/skupperproject/skupper/internal/cmd/skupper/common" - "github.com/skupperproject/skupper/internal/cmd/skupper/common/utils" + "github.com/skupperproject/skupper/internal/cmd/skupper/common/testutils" "github.com/skupperproject/skupper/internal/utils/configs" "gotest.tools/v3/assert" "k8s.io/apimachinery/pkg/runtime" @@ -21,25 +21,25 @@ func TestCmdVersion_ValidateInput(t *testing.T) { flags common.CommandVersionFlags k8sObjects []runtime.Object skupperObjects []runtime.Object - expectedErrors []string + expectedError string } testTable := []test{ { - name: "bad output", - args: []string{""}, - flags: common.CommandVersionFlags{Output: "not-supported"}, - expectedErrors: []string{"output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]"}, + name: "bad output", + args: []string{""}, + flags: common.CommandVersionFlags{Output: "not-supported"}, + expectedError: "output type is not valid: value not-supported not allowed. It should be one of this options: [json yaml]", }, { - name: "good output", - flags: common.CommandVersionFlags{Output: "json"}, - expectedErrors: []string{}, + name: "good output", + flags: common.CommandVersionFlags{Output: "json"}, + expectedError: "", }, { - name: "ok no output", - flags: common.CommandVersionFlags{}, - expectedErrors: []string{}, + name: "ok no output", + flags: common.CommandVersionFlags{}, + expectedError: "", }, } @@ -51,11 +51,7 @@ func TestCmdVersion_ValidateInput(t *testing.T) { cmd.Flags = &test.flags - actualErrors := cmd.ValidateInput(test.args) - - actualErrorsMessages := utils.ErrorsToMessages(actualErrors) - - assert.DeepEqual(t, actualErrorsMessages, test.expectedErrors) + testutils.CheckValidateInput(t, cmd, test.expectedError, test.args) }) } } diff --git a/internal/nonkube/bundle/extract_test.go b/internal/nonkube/bundle/extract_test.go index 29db11bc0..edc17d59f 100644 --- a/internal/nonkube/bundle/extract_test.go +++ b/internal/nonkube/bundle/extract_test.go @@ -1,6 +1,7 @@ package bundle import ( + "errors" "fmt" "os" "os/exec" @@ -34,17 +35,11 @@ func TestSelfExtractingBundle_Generate(t *testing.T) { } // cleanup function defer func() { - var errors []error - appendError := func(e error) { - if e == nil { - return - } - errors = append(errors, e) - } + var errs []error for _, cleanupPath := range cleanupPaths { - appendError(os.RemoveAll(cleanupPath)) + errs = append(errs, os.RemoveAll(cleanupPath)) } - assert.Equal(t, len(errors), 0, "No errors expected during cleanup, but found: %v", errors) + assert.NilError(t, errors.Join(errs...), "No errors expected during cleanup, but found: %v", errs) }() var sitePath string var err error diff --git a/internal/nonkube/bundle/tarball_test.go b/internal/nonkube/bundle/tarball_test.go index 1cdb7cdfa..fac1ec8a0 100644 --- a/internal/nonkube/bundle/tarball_test.go +++ b/internal/nonkube/bundle/tarball_test.go @@ -1,6 +1,7 @@ package bundle import ( + "errors" "fmt" "os" "os/exec" @@ -32,17 +33,11 @@ func TestTarballBundle_Generate(t *testing.T) { } // cleanup function defer func() { - var errors []error - appendError := func(e error) { - if e == nil { - return - } - errors = append(errors, e) - } + var errs []error for _, cleanupPath := range cleanupPaths { - appendError(os.RemoveAll(cleanupPath)) + errs = append(errs, os.RemoveAll(cleanupPath)) } - assert.Equal(t, len(errors), 0, "No errors expected during cleanup, but found: %v", errors) + assert.NilError(t, errors.Join(errs...), "No errors expected during cleanup, but found: %v", errs) }() var sitePath string var extractPath string diff --git a/internal/nonkube/client/compat/container_test.go b/internal/nonkube/client/compat/container_test.go index e0f4e3ec1..5e5706719 100644 --- a/internal/nonkube/client/compat/container_test.go +++ b/internal/nonkube/client/compat/container_test.go @@ -3,6 +3,7 @@ package compat import ( "bytes" "context" + "errors" "fmt" "os" "os/exec" @@ -249,9 +250,7 @@ func TestContainer(t *testing.T) { cleanupErrors = append(cleanupErrors, cli.ContainerRemove(name)) cleanupErrors = append(cleanupErrors, cli.VolumeRemove(name)) cleanupErrors = append(cleanupErrors, cli.NetworkRemove(name)) - for _, e := range cleanupErrors { - assert.Assert(t, e) - } + assert.NilError(t, errors.Join(cleanupErrors...)) }) }