feat(account): delete subcommand (#287)

Author: villevsv-upcloud

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Add `gateway` commands (`delete`, `list`) for Network gateway management.
 - In machine readable outputs of server list, add support for `--show-ip-addresses` parameter.
+- Support for sub-account deletion via `account delete` command
 
 ## [3.3.0] - 2024-01-23
 

internal/commands/account/delete.go

@@ -0,0 +1,46 @@
+package account
+
+import (
+	"fmt"
+
+	"github.com/UpCloudLtd/upcloud-cli/v3/internal/commands"
+	"github.com/UpCloudLtd/upcloud-cli/v3/internal/completion"
+	"github.com/UpCloudLtd/upcloud-cli/v3/internal/output"
+	"github.com/UpCloudLtd/upcloud-cli/v3/internal/resolver"
+	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud/request"
+)
+
+// DeleteCommand creates the "account delete" command
+func DeleteCommand() commands.Command {
+	return &deleteCommand{
+		BaseCommand: commands.New(
+			"delete",
+			"Delete a sub-account",
+			"upctl account delete my-sub-account",
+		),
+	}
+}
+
+type deleteCommand struct {
+	*commands.BaseCommand
+	resolver.CachingAccount
+	completion.Account
+}
+
+// Execute implements commands.MultipleArgumentCommand
+func (s *deleteCommand) Execute(exec commands.Executor, arg string) (output.Output, error) {
+	svc := exec.All()
+	msg := fmt.Sprintf("Deleting sub-account %v", arg)
+	exec.PushProgressStarted(msg)
+
+	err := svc.DeleteSubaccount(exec.Context(), &request.DeleteSubaccountRequest{
+		Username: arg,
+	})
+	if err != nil {
+		return commands.HandleError(exec, fmt.Sprintf("%s: failed", msg), err)
+	}
+
+	exec.PushProgressSuccess(msg)
+
+	return output.None{}, nil
+}

internal/commands/account/delete_test.go

@@ -0,0 +1,52 @@
+package account
+
+import (
+	"testing"
+
+	"github.com/UpCloudLtd/upcloud-cli/v3/internal/commands"
+	"github.com/UpCloudLtd/upcloud-cli/v3/internal/config"
+	smock "github.com/UpCloudLtd/upcloud-cli/v3/internal/mock"
+
+	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud"
+	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud/request"
+	"github.com/gemalto/flume"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDeleteCommand(t *testing.T) {
+	targetMethod := "DeleteSubaccount"
+
+	account := upcloud.Account{
+		UserName: "my-sub-account",
+	}
+
+	for _, test := range []struct {
+		name  string
+		arg   string
+		error string
+		req   request.DeleteSubaccountRequest
+	}{
+		{
+			name: "delete with username",
+			arg:  account.UserName,
+			req:  request.DeleteSubaccountRequest{Username: account.UserName},
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			mService := smock.Service{}
+			mService.On(targetMethod, &test.req).Return(nil)
+
+			conf := config.New()
+			c := commands.BuildCommand(DeleteCommand(), nil, conf)
+
+			_, err := c.(commands.MultipleArgumentCommand).Execute(commands.NewExecutor(conf, &mService, flume.New("test")), test.arg)
+
+			if test.error != "" {
+				assert.EqualError(t, err, test.error)
+			} else {
+				assert.NoError(t, err)
+				mService.AssertNumberOfCalls(t, targetMethod, 1)
+			}
+		})
+	}
+}

internal/commands/all/all.go

@@ -106,6 +106,7 @@ func BuildCommands(rootCmd *cobra.Command, conf *config.Config) {
 	accountCommand := commands.BuildCommand(account.BaseAccountCommand(), rootCmd, conf)
 	commands.BuildCommand(account.ShowCommand(), accountCommand.Cobra(), conf)
 	commands.BuildCommand(account.ListCommand(), accountCommand.Cobra(), conf)
+	commands.BuildCommand(account.DeleteCommand(), accountCommand.Cobra(), conf)
 
 	// Account permissions
 	permissionsCommand := commands.BuildCommand(permissions.BasePermissionsCommand(), accountCommand.Cobra(), conf)

internal/completion/account.go

@@ -0,0 +1,27 @@
+package completion
+
+import (
+	"context"
+
+	"github.com/UpCloudLtd/upcloud-cli/v3/internal/service"
+	"github.com/spf13/cobra"
+)
+
+// Account implements argument completion for accounts by username.
+type Account struct{}
+
+// make sure Account implements the interface
+var _ Provider = Account{}
+
+// CompleteArgument implements completion.Provider
+func (s Account) CompleteArgument(ctx context.Context, svc service.AllServices, toComplete string) ([]string, cobra.ShellCompDirective) {
+	accounts, err := svc.GetAccountList(ctx)
+	if err != nil {
+		return None(toComplete)
+	}
+	var vals []string
+	for _, v := range accounts {
+		vals = append(vals, v.Username)
+	}
+	return MatchStringPrefix(vals, toComplete, true), cobra.ShellCompDirectiveNoFileComp
+}

internal/completion/account_test.go

@@ -0,0 +1,50 @@
+package completion_test
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/UpCloudLtd/upcloud-cli/v3/internal/completion"
+	smock "github.com/UpCloudLtd/upcloud-cli/v3/internal/mock"
+	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud"
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+)
+
+var mockAccounts = upcloud.AccountList{
+	{Username: "mock1"},
+	{Username: "mock2"},
+	{Username: "bock1"},
+	{Username: "bock2"},
+	{Username: "dock1"},
+}
+
+func TestAccount_CompleteArgument(t *testing.T) {
+	for _, test := range []struct {
+		name              string
+		complete          string
+		expectedMatches   []string
+		expectedDirective cobra.ShellCompDirective
+	}{
+		{name: "basic username", complete: "dock", expectedMatches: []string{"dock1"}, expectedDirective: cobra.ShellCompDirectiveNoFileComp},
+		{name: "multiple usernames", complete: "mock", expectedMatches: []string{"mock1", "mock2"}, expectedDirective: cobra.ShellCompDirectiveNoFileComp},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			mService := new(smock.Service)
+			mService.On("GetAccountList", mock.Anything).Return(mockAccounts, nil)
+			accounts, directive := completion.Account{}.CompleteArgument(context.TODO(), mService, test.complete)
+			assert.Equal(t, test.expectedMatches, accounts)
+			assert.Equal(t, test.expectedDirective, directive)
+		})
+	}
+}
+
+func TestAccount_CompleteArgumentServiceFail(t *testing.T) {
+	mService := new(smock.Service)
+	mService.On("GetAccountList", mock.Anything).Return(nil, fmt.Errorf("MOCKFAIL"))
+	ips, directive := completion.Account{}.CompleteArgument(context.TODO(), mService, "127")
+	assert.Nil(t, ips)
+	assert.Equal(t, cobra.ShellCompDirectiveDefault, directive)
+}

internal/mock/mock.go

@@ -573,10 +573,7 @@ func (m *Service) ModifySubaccount(_ context.Context, r *request.ModifySubaccoun
 // DeleteSubaccount implements service.Account.DeleteSubaccount
 func (m *Service) DeleteSubaccount(_ context.Context, r *request.DeleteSubaccountRequest) error {
 	args := m.Called(r)
-	if args[0] == nil {
-		return args.Error(1)
-	}
-	return nil
+	return args.Error(0)
 }
 
 func (m *Service) CancelManagedDatabaseConnection(_ context.Context, r *request.CancelManagedDatabaseConnection) error {

internal/resolver/account.go

@@ -0,0 +1,41 @@
+package resolver
+
+import (
+	"context"
+
+	internal "github.com/UpCloudLtd/upcloud-cli/v3/internal/service"
+)
+
+// CachingAccount implements resolver for servers, caching the results
+type CachingAccount struct{}
+
+// make sure we implement the ResolutionProvider interface
+var _ ResolutionProvider = CachingAccount{}
+
+// Get implements ResolutionProvider.Get
+func (s CachingAccount) Get(ctx context.Context, svc internal.AllServices) (Resolver, error) {
+	accounts, err := svc.GetAccountList(ctx)
+	if err != nil {
+		return nil, err
+	}
+	return func(arg string) (uuid string, err error) {
+		rv := ""
+		for _, account := range accounts {
+			if MatchArgWithWhitespace(arg, account.Username) {
+				if rv != "" {
+					return "", AmbiguousResolutionError(arg)
+				}
+				rv = account.Username
+			}
+		}
+		if rv != "" {
+			return rv, nil
+		}
+		return "", NotFoundError(arg)
+	}, nil
+}
+
+// PositionalArgumentHelp implements resolver.ResolutionProvider
+func (s CachingAccount) PositionalArgumentHelp() string {
+	return "<Username...>"
+}

internal/resolver/account_test.go

@@ -0,0 +1,82 @@
+package resolver_test
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	smock "github.com/UpCloudLtd/upcloud-cli/v3/internal/mock"
+	"github.com/UpCloudLtd/upcloud-cli/v3/internal/resolver"
+
+	"github.com/UpCloudLtd/upcloud-go-api/v6/upcloud"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAccountResolution(t *testing.T) {
+	Account1 := upcloud.AccountListItem{
+		Username: "account-1",
+	}
+
+	Account2 := upcloud.AccountListItem{
+		Username: "account-2",
+	}
+
+	Account3 := upcloud.AccountListItem{
+		Username: "account-3",
+	}
+
+	Account4 := upcloud.AccountListItem{
+		Username: "account-4",
+	}
+
+	allAccounts := upcloud.AccountList{
+		Account1,
+		Account2,
+		Account3,
+		Account4,
+	}
+
+	t.Run("resolve username", func(t *testing.T) {
+		mService := &smock.Service{}
+		mService.On("GetAccountList").Return(allAccounts, nil)
+		res := resolver.CachingAccount{}
+		argResolver, err := res.Get(context.TODO(), mService)
+		assert.NoError(t, err)
+		for _, account := range allAccounts {
+			resolved, err := argResolver(account.Username)
+			assert.NoError(t, err)
+			assert.Equal(t, account.Username, resolved)
+		}
+		// make sure caching works, eg. we didn't call GetAccountList more than once
+		mService.AssertNumberOfCalls(t, "GetAccountList", 1)
+	})
+
+	t.Run("failure situations", func(t *testing.T) {
+		mService := &smock.Service{}
+		mService.On("GetAccountList").Return(allAccounts, nil)
+
+		res := resolver.CachingAccount{}
+		argResolver, err := res.Get(context.TODO(), mService)
+		assert.NoError(t, err)
+
+		// not found
+		resolved, err := argResolver("notfound")
+		if !assert.Error(t, err) {
+			t.FailNow()
+		}
+		assert.ErrorIs(t, err, resolver.NotFoundError("notfound"))
+		assert.Equal(t, "", resolved)
+
+		// make sure caching works, eg. we didn't call GetAccountList more than once
+		mService.AssertNumberOfCalls(t, "GetAccountList", 1)
+	})
+}
+
+func TestFailingAccountResolution(t *testing.T) {
+	mService := &smock.Service{}
+	mService.On("GetAccountList").Return(upcloud.AccountList{}, errors.New("MOCKERROR"))
+	res := resolver.CachingAccount{}
+	argResolver, err := res.Get(context.TODO(), mService)
+	assert.EqualError(t, err, "MOCKERROR")
+	assert.Nil(t, argResolver)
+}

internal/resolver/ipaddress_test.go

@@ -139,11 +139,11 @@ func TestIPAddressResolution(t *testing.T) {
 		assert.Equal(t, "", resolved)
 
 		// not found
-		resolved, err = argResolver("notfounf")
+		resolved, err = argResolver("notfound")
 		if !assert.Error(t, err) {
 			t.FailNow()
 		}
-		assert.ErrorIs(t, err, resolver.NotFoundError("notfounf"))
+		assert.ErrorIs(t, err, resolver.NotFoundError("notfound"))
 		assert.Equal(t, "", resolved)
 
 		// make sure caching works, eg. we didn't call GetServers more than once