storage: handle errors in batch objects delete action (#627)

Batch objects delete API method from `aws-sdk-go-v2` will not return
error as second argument if individual objects failed to be deleted.
Instead it will return both list of deleted objects and list of errors
in result structure.

This prints errors (if any) after the list of successfully deleted
files.

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Bug fixes
+
+- storage: handle errors in batch objects delete action #627
+
 ## 1.80.0
 
 ### Features

cmd/storage_delete.go

@@ -2,9 +2,11 @@ package cmd
 
 import (
 	"fmt"
+	"os"
 	"strings"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/hashicorp/go-multierror"
 	"github.com/spf13/cobra"
 
 	"github.com/exoscale/cli/pkg/storage/sos"
@@ -86,7 +88,15 @@ argument with "/":
 
 		deleted, err := storage.DeleteObjects(gContext, bucket, prefix, recursive)
 		if err != nil {
-			return fmt.Errorf("unable to delete objects: %w", err)
+			if merr, ok := err.(*multierror.Error); ok {
+				// Error in individual files, print to stderr & continue
+				for _, e := range merr.Errors {
+					fmt.Fprintln(os.Stderr, e)
+				}
+			} else {
+				// Global error, exit
+				return fmt.Errorf("unable to delete objects: %w", err)
+			}
 		}
 
 		if verbose {

pkg/storage/sos/object.go

@@ -21,6 +21,7 @@ import (
 	"github.com/aws/aws-sdk-go-v2/service/s3"
 	"github.com/aws/aws-sdk-go-v2/service/s3/types"
 	"github.com/dustin/go-humanize"
+	"github.com/hashicorp/go-multierror"
 	"github.com/vbauerster/mpb/v4"
 	"github.com/vbauerster/mpb/v4/decor"
 
@@ -45,6 +46,7 @@ func (c *Client) DeleteObjects(ctx context.Context, bucket, prefix string, recur
 	// precaution we're batching deletes.
 	maxKeys := 1000
 	deleted := make([]types.DeletedObject, 0)
+	errs := &multierror.Error{}
 
 	for i := 0; i < len(deleteList); i += maxKeys {
 		j := i + maxKeys
@@ -61,9 +63,21 @@ func (c *Client) DeleteObjects(ctx context.Context, bucket, prefix string, recur
 		}
 
 		deleted = append(deleted, res.Deleted...)
+		for _, err := range res.Errors {
+			var e error
+			switch {
+			case err.Message != nil:
+				e = errors.New(*err.Message)
+			case err.Code != nil:
+				e = errors.New(*err.Code)
+			default:
+				e = fmt.Errorf("undefined error")
+			}
+			errs = multierror.Append(errs, e)
+		}
 	}
 
-	return deleted, nil
+	return deleted, errs.ErrorOrNil()
 }
 
 func (c *Client) GenPresignedURL(ctx context.Context, method, bucket, key string, expires time.Duration) (string, error) {

pkg/storage/sos/object_test.go

@@ -15,6 +15,7 @@ import (
 	s3manager "github.com/aws/aws-sdk-go-v2/feature/s3/manager"
 	"github.com/aws/aws-sdk-go-v2/service/s3"
 	"github.com/aws/aws-sdk-go-v2/service/s3/types"
+	"github.com/hashicorp/go-multierror"
 	"github.com/stretchr/testify/assert"
 
 	"github.com/exoscale/cli/pkg/storage/sos"
@@ -103,6 +104,7 @@ func TestDeleteObjects(t *testing.T) {
 	commonPrefix := "myobjects/"
 	objectKeys := []string{commonPrefix + "object1", commonPrefix + "object2", commonPrefix + "object3"}
 
+	// Happy path
 	nCalls := 0
 	expectedDeleteInput := &s3.DeleteObjectsInput{
 		Bucket: &bucket,
@@ -149,6 +151,7 @@ func TestDeleteObjects(t *testing.T) {
 		assert.Equal(t, objectKeys[i], *key.Key)
 	}
 
+	// General error
 	client = sos.Client{
 		S3Client: &MockS3API{
 			mockDeleteObjects: func(ctx context.Context, params *s3.DeleteObjectsInput, optFns ...func(*s3.Options)) (*s3.DeleteObjectsOutput, error) {
@@ -169,6 +172,50 @@ func TestDeleteObjects(t *testing.T) {
 
 	_, err = client.DeleteObjects(context.Background(), bucket, commonPrefix, false)
 	assert.Error(t, err)
+
+	// Individual error in batch delete
+	client = sos.Client{
+		S3Client: &MockS3API{
+			mockDeleteObjects: func(ctx context.Context, params *s3.DeleteObjectsInput, optFns ...func(*s3.Options)) (*s3.DeleteObjectsOutput, error) {
+				nCalls++
+				assert.Equal(t, expectedDeleteInput, params)
+				return &s3.DeleteObjectsOutput{
+					Deleted: []types.DeletedObject{
+						{Key: aws.String(objectKeys[0])},
+						{Key: aws.String(objectKeys[2])},
+					},
+					Errors: []types.Error{
+						{
+							Code:      aws.String("AccessDenied"),
+							Key:       aws.String("1"),
+							Message:   aws.String("Access Denied"),
+							VersionId: aws.String("1"),
+						},
+					},
+				}, nil
+			},
+			mockListObjectsV2: func(ctx context.Context, params *s3.ListObjectsV2Input, optFns ...func(*s3.Options)) (*s3.ListObjectsV2Output, error) {
+				return &s3.ListObjectsV2Output{
+					IsTruncated: false,
+					Contents: []types.Object{
+						{Key: aws.String(objectKeys[0])},
+						{Key: aws.String(objectKeys[1])},
+						{Key: aws.String(objectKeys[2])},
+					},
+				}, nil
+			},
+		},
+	}
+	deleted, err = client.DeleteObjects(context.Background(), bucket, commonPrefix, false)
+
+	assert.Equal(t, 2, len(deleted))
+	assert.Error(t, err)
+	if merr, ok := err.(*multierror.Error); ok {
+		assert.Equal(t, 1, len(merr.Errors))
+		assert.Equal(t, "Access Denied", merr.Errors[0].Error())
+	} else {
+		assert.NoError(t, err)
+	}
 }
 
 type MockUploader struct {