Skip to content

Commit

Permalink
Fix bulk delete operation in S3 driver.
Browse files Browse the repository at this point in the history
The original implementation of bulk delete was omitting the error check
returned from the bulk operation thus potentially not cathing errors
that may have occurred when deleting individual S3 keys.

This commit introduces storagedriver.Errors type that allows to return
multierrors as a single error from any storage driver implementation.

Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
  • Loading branch information
milosgajdos committed Aug 15, 2022
1 parent ac6c07b commit e3d3e15
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
15 changes: 14 additions & 1 deletion registry/storage/driver/s3-aws/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"bytes"
"context"
"crypto/tls"
"errors"
"fmt"
"io"
"io/ioutil"
Expand Down Expand Up @@ -937,7 +938,7 @@ func (d *driver) Delete(ctx context.Context, path string) error {
// by default the response returns up to 1,000 key names. The response _might_ contain fewer keys but it will never contain more.
// 10000 keys is coincidentally (?) also the max number of keys that can be deleted in a single Delete operation, so we'll just smack
// Delete here straight away and reset the object slice when successful.
_, err = d.S3.DeleteObjects(&s3.DeleteObjectsInput{
resp, err := d.S3.DeleteObjects(&s3.DeleteObjectsInput{
Bucket: aws.String(d.Bucket),
Delete: &s3.Delete{
Objects: s3Objects,
Expand All @@ -948,6 +949,18 @@ func (d *driver) Delete(ctx context.Context, path string) error {
return err
}

if len(resp.Errors) > 0 {
// NOTE: AWS SDK s3.Error does not implement error interface which
// is pretty intensely sad, so we have to do away with this for now.
errs := make([]error, 0, len(resp.Errors))
for _, err := range resp.Errors {
errs = append(errs, errors.New(err.String()))
}
return storagedriver.Errors{
DriverName: driverName,
Errs: errs,
}
}
}
// NOTE: we don't want to reallocate
// the slice so we simply "reset" it
Expand Down
24 changes: 24 additions & 0 deletions registry/storage/driver/storagedriver.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,27 @@ type Error struct {
func (err Error) Error() string {
return fmt.Sprintf("%s: %s", err.DriverName, err.Enclosed)
}

// Errors provides the envelope for multiple errors
// for use within the storagedriver implementations.
type Errors struct {
DriverName string
Errs []error
}

var _ error = Errors{}

func (e Errors) Error() string {
switch len(e.Errs) {
case 0:
return "<nil>"
case 1:
return e.Errs[0].Error()
default:
msg := "errors:\n"
for _, err := range e.Errs {
msg += err.Error() + "\n"
}
return msg
}
}

0 comments on commit e3d3e15

Please sign in to comment.