refactor(server): use cobra to validate required flags in server storage

Author: kangasta

internal/commands/serverstorage/attach.go

@@ -58,6 +58,7 @@ func (s *attachCommand) InitCommand() {
 	config.AddToggleFlag(flagSet, &s.params.bootable, "boot-disk", false, "Set attached device as the server's boot disk.")
 
 	s.AddFlags(flagSet)
+	s.Cobra().MarkFlagRequired("storage") //nolint:errcheck
 }
 
 // MaximumExecutions implements command.Command
@@ -69,10 +70,6 @@ func (s *attachCommand) MaximumExecutions() int {
 func (s *attachCommand) ExecuteSingleArgument(exec commands.Executor, uuid string) (output.Output, error) {
 	storageSvc := exec.Storage()
 
-	if s.params.StorageUUID == "" {
-		return nil, fmt.Errorf("storage is required")
-	}
-
 	strg, err := storage.SearchSingleStorage(s.params.StorageUUID, storageSvc)
 	if err != nil {
 		return nil, err

internal/commands/serverstorage/attach_test.go

@@ -6,11 +6,11 @@ import (
 	"github.com/UpCloudLtd/upcloud-cli/internal/commands"
 	"github.com/UpCloudLtd/upcloud-cli/internal/config"
 	smock "github.com/UpCloudLtd/upcloud-cli/internal/mock"
+	"github.com/UpCloudLtd/upcloud-cli/internal/mockexecute"
 	internal "github.com/UpCloudLtd/upcloud-cli/internal/service"
 
 	"github.com/UpCloudLtd/upcloud-go-api/v4/upcloud"
 	"github.com/UpCloudLtd/upcloud-go-api/v4/upcloud/request"
-	"github.com/gemalto/flume"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 )
@@ -72,7 +72,7 @@ func TestAttachStorageCommand(t *testing.T) {
 		{
 			name:  "storage is missing",
 			args:  []string{},
-			error: "storage is required",
+			error: `required flag(s) "storage" not set`,
 		},
 		{
 			name: "use default values",
@@ -113,15 +113,15 @@ func TestAttachStorageCommand(t *testing.T) {
 			mService.On("GetStorages", mock.Anything).Return(storages, nil)
 
 			c := commands.BuildCommand(AttachCommand(), nil, conf)
-			err := c.Cobra().Flags().Parse(test.args)
-			assert.NoError(t, err)
 
-			_, err = c.(commands.SingleArgumentCommand).ExecuteSingleArgument(commands.NewExecutor(conf, mService, flume.New("test")), Server1.UUID)
+			c.Cobra().SetArgs(append(test.args, Server1.UUID))
+			_, err := mockexecute.MockExecute(c, mService, conf)
 
 			if test.error != "" {
 				assert.Error(t, err)
 				assert.Equal(t, test.error, err.Error())
 			} else {
+				assert.NoError(t, err)
 				mService.AssertNumberOfCalls(t, targetMethod, 1)
 			}
 		})

internal/commands/serverstorage/detach.go

@@ -47,6 +47,7 @@ func (s *detachCommand) InitCommand() {
 	flagSet.StringVar(&s.params.Address, "address", defaultDetachParams.Address, "Detach the storage attached to this address.")
 
 	s.AddFlags(flagSet)
+	s.Cobra().MarkFlagRequired("address") //nolint:errcheck
 }
 
 // MaximumExecutions implements command.Command
@@ -58,10 +59,6 @@ func (s *detachCommand) MaximumExecutions() int {
 func (s *detachCommand) ExecuteSingleArgument(exec commands.Executor, uuid string) (output.Output, error) {
 	storageSvc := exec.Storage()
 
-	if s.params.Address == "" {
-		return nil, fmt.Errorf("address is required")
-	}
-
 	req := s.params.DetachStorageRequest
 	req.ServerUUID = uuid
 

internal/commands/serverstorage/detach_test.go

@@ -6,11 +6,11 @@ import (
 	"github.com/UpCloudLtd/upcloud-cli/internal/commands"
 	"github.com/UpCloudLtd/upcloud-cli/internal/config"
 	smock "github.com/UpCloudLtd/upcloud-cli/internal/mock"
+	"github.com/UpCloudLtd/upcloud-cli/internal/mockexecute"
 	internal "github.com/UpCloudLtd/upcloud-cli/internal/service"
 
 	"github.com/UpCloudLtd/upcloud-go-api/v4/upcloud"
 	"github.com/UpCloudLtd/upcloud-go-api/v4/upcloud/request"
-	"github.com/gemalto/flume"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 )
@@ -50,7 +50,7 @@ func TestDetachCommand(t *testing.T) {
 		{
 			name:  "Address missing",
 			args:  []string{},
-			error: "address is required",
+			error: `required flag(s) "address" not set`,
 		},
 		{
 			name: "Address provided",
@@ -71,15 +71,15 @@ func TestDetachCommand(t *testing.T) {
 			mService.On(targetMethod, &test.detachReq).Return(&details, nil)
 
 			c := commands.BuildCommand(DetachCommand(), nil, conf)
-			err := c.Cobra().Flags().Parse(test.args)
-			assert.NoError(t, err)
 
-			_, err = c.(commands.SingleArgumentCommand).ExecuteSingleArgument(commands.NewExecutor(conf, mService, flume.New("test")), Server1.UUID)
+			c.Cobra().SetArgs(append(test.args, Server1.UUID))
+			_, err := mockexecute.MockExecute(c, mService, conf)
 
 			if test.error != "" {
 				assert.Error(t, err)
 				assert.Equal(t, test.error, err.Error())
 			} else {
+				assert.NoError(t, err)
 				mService.AssertNumberOfCalls(t, targetMethod, 1)
 			}
 		})