From 8eab5d1bd6c69e895d762c30a0d34031ccc872ee Mon Sep 17 00:00:00 2001 From: Milos Gajdos Date: Sat, 9 Apr 2022 12:16:46 +0100 Subject: [PATCH] Update s3 ListObjects to V2 API Signed-off-by: Milos Gajdos --- registry/storage/driver/s3-aws/s3.go | 22 +++--- registry/storage/driver/s3-aws/s3_test.go | 83 +++++++++++++++++++++++ 2 files changed, 94 insertions(+), 11 deletions(-) diff --git a/registry/storage/driver/s3-aws/s3.go b/registry/storage/driver/s3-aws/s3.go index ba76f4248..dc4a416e4 100644 --- a/registry/storage/driver/s3-aws/s3.go +++ b/registry/storage/driver/s3-aws/s3.go @@ -684,7 +684,7 @@ func (d *driver) Writer(ctx context.Context, path string, appendParam bool) (sto // Stat retrieves the FileInfo for the given path, including the current size // in bytes and the creation time. func (d *driver) Stat(ctx context.Context, path string) (storagedriver.FileInfo, error) { - resp, err := d.S3.ListObjects(&s3.ListObjectsInput{ + resp, err := d.S3.ListObjectsV2(&s3.ListObjectsV2Input{ Bucket: aws.String(d.Bucket), Prefix: aws.String(d.s3Path(path)), MaxKeys: aws.Int64(1), @@ -729,7 +729,7 @@ func (d *driver) List(ctx context.Context, opath string) ([]string, error) { prefix = "/" } - resp, err := d.S3.ListObjects(&s3.ListObjectsInput{ + resp, err := d.S3.ListObjectsV2(&s3.ListObjectsV2Input{ Bucket: aws.String(d.Bucket), Prefix: aws.String(d.s3Path(path)), Delimiter: aws.String("/"), @@ -753,12 +753,12 @@ func (d *driver) List(ctx context.Context, opath string) ([]string, error) { } if *resp.IsTruncated { - resp, err = d.S3.ListObjects(&s3.ListObjectsInput{ - Bucket: aws.String(d.Bucket), - Prefix: aws.String(d.s3Path(path)), - Delimiter: aws.String("/"), - MaxKeys: aws.Int64(listMax), - Marker: resp.NextMarker, + resp, err = d.S3.ListObjectsV2(&s3.ListObjectsV2Input{ + Bucket: aws.String(d.Bucket), + Prefix: aws.String(d.s3Path(path)), + Delimiter: aws.String("/"), + MaxKeys: aws.Int64(listMax), + ContinuationToken: resp.NextContinuationToken, }) if err != nil { return nil, err @@ -907,14 +907,14 @@ func (d *driver) Delete(ctx context.Context, path string) error { // list objects under the given path as a subpath (suffix with slash "/") s3Path := d.s3Path(path) + "/" - listObjectsInput := &s3.ListObjectsInput{ + listObjectsInput := &s3.ListObjectsV2Input{ Bucket: aws.String(d.Bucket), Prefix: aws.String(s3Path), } ListLoop: for { // list all the objects - resp, err := d.S3.ListObjects(listObjectsInput) + resp, err := d.S3.ListObjectsV2(listObjectsInput) // resp.Contents can only be empty on the first call // if there were no more results to return after the first call, resp.IsTruncated would have been false @@ -930,7 +930,7 @@ ListLoop: } // resp.Contents must have at least one element or we would have returned not found - listObjectsInput.Marker = resp.Contents[len(resp.Contents)-1].Key + listObjectsInput.StartAfter = resp.Contents[len(resp.Contents)-1].Key // from the s3 api docs, IsTruncated "specifies whether (true) or not (false) all of the results were returned" // if everything has been returned, break diff --git a/registry/storage/driver/s3-aws/s3_test.go b/registry/storage/driver/s3-aws/s3_test.go index 41a5fc7da..06a2db582 100644 --- a/registry/storage/driver/s3-aws/s3_test.go +++ b/registry/storage/driver/s3-aws/s3_test.go @@ -7,7 +7,9 @@ import ( "io/ioutil" "math/rand" "os" + "path" "reflect" + "sort" "strconv" "strings" "testing" @@ -770,6 +772,87 @@ func TestMoveWithMultipartCopy(t *testing.T) { } } +func TestListObjectsV2(t *testing.T) { + rootDir, err := ioutil.TempDir("", "driver-") + if err != nil { + t.Fatalf("unexpected error creating temporary directory: %v", err) + } + defer os.Remove(rootDir) + + d, err := s3DriverConstructor(rootDir, s3.StorageClassStandard) + if err != nil { + t.Fatalf("unexpected error creating driver: %v", err) + } + + ctx := context.Background() + n := 6 + prefix := "/test-list-objects-v2" + var filePaths []string + for i := 0; i < n; i++ { + filePaths = append(filePaths, fmt.Sprintf("%s/%d", prefix, i)) + } + for _, path := range filePaths { + if err := d.PutContent(ctx, path, []byte(path)); err != nil { + t.Fatalf("unexpected error putting content: %v", err) + } + } + + info, err := d.Stat(ctx, filePaths[0]) + if err != nil { + t.Fatalf("unexpected error stating: %v", err) + } + + if info.IsDir() || info.Size() != int64(len(filePaths[0])) || info.Path() != filePaths[0] { + t.Fatal("unexcepted state info") + } + + subDirPath := prefix + "/sub/0" + if err := d.PutContent(ctx, subDirPath, []byte(subDirPath)); err != nil { + t.Fatalf("unexpected error putting content: %v", err) + } + + subPaths := append(filePaths, path.Dir(subDirPath)) + + result, err := d.List(ctx, prefix) + if err != nil { + t.Fatalf("unexpected error listing: %v", err) + } + + sort.Strings(subPaths) + sort.Strings(result) + if !reflect.DeepEqual(subPaths, result) { + t.Fatalf("unexpected list result") + } + + var walkPathes []string + if err := d.Walk(ctx, prefix, func(fileInfo storagedriver.FileInfo) error { + walkPathes = append(walkPathes, fileInfo.Path()) + if fileInfo.Path() == path.Dir(subDirPath) { + if !fileInfo.IsDir() { + t.Fatalf("unexpected walking file info") + } + } else { + if fileInfo.IsDir() || fileInfo.Size() != int64(len(fileInfo.Path())) { + t.Fatalf("unexpected walking file info") + } + } + return nil + }); err != nil { + t.Fatalf("unexpected error walking: %v", err) + } + + subPaths = append(subPaths, subDirPath) + sort.Strings(walkPathes) + sort.Strings(subPaths) + if !reflect.DeepEqual(subPaths, walkPathes) { + t.Fatalf("unexpected walking pathes") + } + + if err := d.Delete(ctx, prefix); err != nil { + t.Fatalf("unexpected error deleting: %v", err) + } +} + func compareWalked(t *testing.T, expected, walked []string) { if len(walked) != len(expected) { t.Fatalf("Mismatch number of fileInfo walked %d expected %d; walked %s; expected %s;", len(walked), len(expected), walked, expected)