feat: allow deletion of multiple resources at once (#719)

Currently, the CLI only allows you to delete one resource at a time.
This can be time consuming and tedious if you want to delete multiple
resources, because you have to wait for each resource to be deleted
before you can delete the next one.
This PR adds the ability to delete multiple resources at once by passing
them to the `delete` subcommand as multiple positional arguments, e.g.
`hcloud server delete server1 server2 ...`. It also adds tests to verify
that this works as intended.

Author: phm07

internal/cmd/base/delete.go

@@ -1,6 +1,7 @@
 package base
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 
@@ -31,10 +32,10 @@ func (dc *DeleteCmd) CobraCommand(s state.State) *cobra.Command {
 	}
 
 	cmd := &cobra.Command{
-		Use:                   fmt.Sprintf("delete %s<%s>", opts, util.ToKebabCase(dc.ResourceNameSingular)),
+		Use:                   fmt.Sprintf("delete %s<%s>...", opts, util.ToKebabCase(dc.ResourceNameSingular)),
 		Short:                 dc.ShortDescription,
 		Args:                  util.Validate,
-		ValidArgsFunction:     cmpl.SuggestArgs(cmpl.SuggestCandidatesF(dc.NameSuggestions(s.Client()))),
+		ValidArgsFunction:     cmpl.SuggestCandidatesF(dc.NameSuggestions(s.Client())),
 		TraverseChildren:      true,
 		DisableFlagsInUseLine: true,
 		PreRunE:               util.ChainRunE(s.EnsureToken),
@@ -50,22 +51,27 @@ func (dc *DeleteCmd) CobraCommand(s state.State) *cobra.Command {
 
 // Run executes a describe command.
 func (dc *DeleteCmd) Run(s state.State, cmd *cobra.Command, args []string) error {
+	var cmdErr error
 
-	idOrName := args[0]
-	resource, _, err := dc.Fetch(s, cmd, idOrName)
-	if err != nil {
-		return err
-	}
+	for _, idOrName := range args {
+		resource, _, err := dc.Fetch(s, cmd, idOrName)
+		if err != nil {
+			cmdErr = errors.Join(cmdErr, err)
+			continue
+		}
 
-	// resource is an interface that always has a type, so the interface is never nil
-	// (i.e. == nil) is always false.
-	if reflect.ValueOf(resource).IsNil() {
-		return fmt.Errorf("%s not found: %s", dc.ResourceNameSingular, idOrName)
-	}
+		// resource is an interface that always has a type, so the interface is never nil
+		// (i.e. == nil) is always false.
+		if reflect.ValueOf(resource).IsNil() {
+			cmdErr = errors.Join(cmdErr, fmt.Errorf("%s not found: %s", dc.ResourceNameSingular, idOrName))
+			continue
+		}
 
-	if err := dc.Delete(s, cmd, resource); err != nil {
-		return fmt.Errorf("deleting %s %s failed: %s", dc.ResourceNameSingular, idOrName, err)
+		if err = dc.Delete(s, cmd, resource); err != nil {
+			cmdErr = errors.Join(cmdErr, fmt.Errorf("deleting %s %s failed: %s", dc.ResourceNameSingular, idOrName, err))
+		}
+		cmd.Printf("%s %v deleted\n", dc.ResourceNameSingular, idOrName)
 	}
-	cmd.Printf("%s %v deleted\n", dc.ResourceNameSingular, idOrName)
-	return nil
+
+	return cmdErr
 }

internal/cmd/base/delete_test.go

@@ -41,8 +41,16 @@ func TestDelete(t *testing.T) {
 			Args:   []string{"delete", "123"},
 			ExpOut: "Fetching fake resource\nDeleting fake resource\nFake resource 123 deleted\n",
 		},
+		"no flags multiple": {
+			Args: []string{"delete", "123", "456", "789"},
+			ExpOut: "Fetching fake resource\nDeleting fake resource\nFake resource 123 deleted\nFetching fake resource\n" +
+				"Deleting fake resource\nFake resource 456 deleted\nFetching fake resource\nDeleting fake resource\nFake resource 789 deleted\n",
+		},
 		"quiet": {
 			Args: []string{"delete", "123", "--quiet"},
 		},
+		"quiet multiple": {
+			Args: []string{"delete", "123", "456", "789", "--quiet"},
+		},
 	})
 }

internal/cmd/certificate/delete_test.go

@@ -1,6 +1,8 @@
 package certificate_test
 
 import (
+	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/golang/mock/gomock"
@@ -38,3 +40,47 @@ func TestDelete(t *testing.T) {
 	assert.Empty(t, errOut)
 	assert.Equal(t, expOut, out)
 }
+
+func TestDeleteMultiple(t *testing.T) {
+	fx := testutil.NewFixture(t)
+	defer fx.Finish()
+
+	cmd := certificate.DeleteCmd.CobraCommand(fx.State())
+	fx.ExpectEnsureToken()
+
+	certs := []*hcloud.Certificate{
+		{
+			ID:   123,
+			Name: "test1",
+		},
+		{
+			ID:   456,
+			Name: "test2",
+		},
+		{
+			ID:   789,
+			Name: "test3",
+		},
+	}
+
+	expOutBuilder := strings.Builder{}
+
+	var names []string
+	for _, cert := range certs {
+		names = append(names, cert.Name)
+		expOutBuilder.WriteString(fmt.Sprintf("certificate %s deleted\n", cert.Name))
+		fx.Client.CertificateClient.EXPECT().
+			Get(gomock.Any(), cert.Name).
+			Return(cert, nil, nil)
+		fx.Client.CertificateClient.EXPECT().
+			Delete(gomock.Any(), cert).
+			Return(nil, nil)
+	}
+
+	out, errOut, err := fx.Run(cmd, names)
+	expOut := expOutBuilder.String()
+
+	assert.NoError(t, err)
+	assert.Empty(t, errOut)
+	assert.Equal(t, expOut, out)
+}

internal/cmd/firewall/delete_test.go

@@ -1,6 +1,8 @@
 package firewall_test
 
 import (
+	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/golang/mock/gomock"
@@ -38,3 +40,47 @@ func TestDelete(t *testing.T) {
 	assert.Empty(t, errOut)
 	assert.Equal(t, expOut, out)
 }
+
+func TestDeleteMultiple(t *testing.T) {
+	fx := testutil.NewFixture(t)
+	defer fx.Finish()
+
+	cmd := firewall.DeleteCmd.CobraCommand(fx.State())
+	fx.ExpectEnsureToken()
+
+	firewalls := []*hcloud.Firewall{
+		{
+			ID:   123,
+			Name: "test1",
+		},
+		{
+			ID:   456,
+			Name: "test2",
+		},
+		{
+			ID:   789,
+			Name: "test3",
+		},
+	}
+
+	expOutBuilder := strings.Builder{}
+
+	var names []string
+	for _, fw := range firewalls {
+		names = append(names, fw.Name)
+		expOutBuilder.WriteString(fmt.Sprintf("firewall %s deleted\n", fw.Name))
+		fx.Client.FirewallClient.EXPECT().
+			Get(gomock.Any(), fw.Name).
+			Return(fw, nil, nil)
+		fx.Client.FirewallClient.EXPECT().
+			Delete(gomock.Any(), fw).
+			Return(nil, nil)
+	}
+
+	out, errOut, err := fx.Run(cmd, names)
+	expOut := expOutBuilder.String()
+
+	assert.NoError(t, err)
+	assert.Empty(t, errOut)
+	assert.Equal(t, expOut, out)
+}

internal/cmd/floatingip/delete_test.go

@@ -1,6 +1,8 @@
 package floatingip_test
 
 import (
+	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/golang/mock/gomock"
@@ -38,3 +40,47 @@ func TestDelete(t *testing.T) {
 	assert.Empty(t, errOut)
 	assert.Equal(t, expOut, out)
 }
+
+func TestDeleteMultiple(t *testing.T) {
+	fx := testutil.NewFixture(t)
+	defer fx.Finish()
+
+	cmd := floatingip.DeleteCmd.CobraCommand(fx.State())
+	fx.ExpectEnsureToken()
+
+	ips := []*hcloud.FloatingIP{
+		{
+			ID:   123,
+			Name: "test1",
+		},
+		{
+			ID:   456,
+			Name: "test2",
+		},
+		{
+			ID:   789,
+			Name: "test3",
+		},
+	}
+
+	expOutBuilder := strings.Builder{}
+
+	var names []string
+	for _, ip := range ips {
+		names = append(names, ip.Name)
+		expOutBuilder.WriteString(fmt.Sprintf("Floating IP %s deleted\n", ip.Name))
+		fx.Client.FloatingIPClient.EXPECT().
+			Get(gomock.Any(), ip.Name).
+			Return(ip, nil, nil)
+		fx.Client.FloatingIPClient.EXPECT().
+			Delete(gomock.Any(), ip).
+			Return(nil, nil)
+	}
+
+	out, errOut, err := fx.Run(cmd, names)
+	expOut := expOutBuilder.String()
+
+	assert.NoError(t, err)
+	assert.Empty(t, errOut)
+	assert.Equal(t, expOut, out)
+}

internal/cmd/image/delete_test.go

@@ -1,6 +1,8 @@
 package image_test
 
 import (
+	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/golang/mock/gomock"
@@ -38,3 +40,47 @@ func TestDelete(t *testing.T) {
 	assert.Empty(t, errOut)
 	assert.Equal(t, expOut, out)
 }
+
+func TestDeleteMultiple(t *testing.T) {
+	fx := testutil.NewFixture(t)
+	defer fx.Finish()
+
+	cmd := image.DeleteCmd.CobraCommand(fx.State())
+	fx.ExpectEnsureToken()
+
+	images := []*hcloud.Image{
+		{
+			ID:   123,
+			Name: "test1",
+		},
+		{
+			ID:   456,
+			Name: "test2",
+		},
+		{
+			ID:   789,
+			Name: "test3",
+		},
+	}
+
+	expOutBuilder := strings.Builder{}
+
+	var names []string
+	for _, img := range images {
+		names = append(names, img.Name)
+		expOutBuilder.WriteString(fmt.Sprintf("image %s deleted\n", img.Name))
+		fx.Client.ImageClient.EXPECT().
+			Get(gomock.Any(), img.Name).
+			Return(img, nil, nil)
+		fx.Client.ImageClient.EXPECT().
+			Delete(gomock.Any(), img).
+			Return(nil, nil)
+	}
+
+	out, errOut, err := fx.Run(cmd, names)
+	expOut := expOutBuilder.String()
+
+	assert.NoError(t, err)
+	assert.Empty(t, errOut)
+	assert.Equal(t, expOut, out)
+}

internal/cmd/loadbalancer/delete_test.go

@@ -1,6 +1,8 @@
 package loadbalancer_test
 
 import (
+	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/golang/mock/gomock"
@@ -38,3 +40,47 @@ func TestDelete(t *testing.T) {
 	assert.Empty(t, errOut)
 	assert.Equal(t, expOut, out)
 }
+
+func TestDeleteMultiple(t *testing.T) {
+	fx := testutil.NewFixture(t)
+	defer fx.Finish()
+
+	cmd := loadbalancer.DeleteCmd.CobraCommand(fx.State())
+	fx.ExpectEnsureToken()
+
+	loadBalancers := []*hcloud.LoadBalancer{
+		{
+			ID:   123,
+			Name: "test1",
+		},
+		{
+			ID:   456,
+			Name: "test2",
+		},
+		{
+			ID:   789,
+			Name: "test3",
+		},
+	}
+
+	expOutBuilder := strings.Builder{}
+
+	var names []string
+	for _, lb := range loadBalancers {
+		names = append(names, lb.Name)
+		expOutBuilder.WriteString(fmt.Sprintf("Load Balancer %s deleted\n", lb.Name))
+		fx.Client.LoadBalancerClient.EXPECT().
+			Get(gomock.Any(), lb.Name).
+			Return(lb, nil, nil)
+		fx.Client.LoadBalancerClient.EXPECT().
+			Delete(gomock.Any(), lb).
+			Return(nil, nil)
+	}
+
+	out, errOut, err := fx.Run(cmd, names)
+	expOut := expOutBuilder.String()
+
+	assert.NoError(t, err)
+	assert.Empty(t, errOut)
+	assert.Equal(t, expOut, out)
+}