Skip to content

Commit

Permalink
Ignore self reference object on empty prefix
Browse files Browse the repository at this point in the history
When a given prefix is empty and we attempt to list its content AWS
returns that the prefix contains one object with key defined as the
prefix with an extra "/" at the end.

e.g.

If we call ListObjects() passing to it an existing but empty prefix,
say "my/empty/prefix", AWS will return that "my/empty/prefix/" is an
object inside "my/empty/prefix" (ListObjectsOutput.Contents).

This extra "/" causes the upload purging process to panic. On normal
circunstances we never find empty prefixes on S3 but users may touch
it.

Signed-off-by: Ricardo Maraschini <rmarasch@redhat.com>
  • Loading branch information
ricardomaraschini authored and antoineco committed Oct 11, 2023
1 parent d607c6c commit 5a96fbb
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 0 deletions.
5 changes: 5 additions & 0 deletions registry/storage/driver/s3-aws/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,11 @@ func (d *driver) doWalk(parentCtx context.Context, objectCount *int64, path, pre
}

for _, file := range objects.Contents {
// empty prefixes are listed as objects inside its own prefix.
// https://docs.aws.amazon.com/AmazonS3/latest/user-guide/using-folders.html
if strings.HasSuffix(*file.Key, "/") {
continue
}
walkInfos = append(walkInfos, walkInfoContainer{
FileInfoFields: storagedriver.FileInfoFields{
IsDir: false,
Expand Down
34 changes: 34 additions & 0 deletions registry/storage/driver/s3-aws/s3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/rand"
"io/ioutil"
"os"
"reflect"
"strconv"
"testing"

Expand Down Expand Up @@ -164,6 +165,39 @@ func TestEmptyRootList(t *testing.T) {
}
}

// TestWalkEmptySubDirectory assures we list an empty sub directory only once when walking
// through its parent directory.
func TestWalkEmptySubDirectory(t *testing.T) {
if skipS3() != "" {
t.Skip(skipS3())
}

drv, err := s3DriverConstructor("", s3.StorageClassStandard)
if err != nil {
t.Fatalf("unexpected error creating rooted driver: %v", err)
}

// create an empty sub directory.
s3driver := drv.StorageDriver.(*driver)
if _, err := s3driver.S3.PutObject(&s3.PutObjectInput{
Bucket: aws.String(os.Getenv("S3_BUCKET")),
Key: aws.String("/testdir/emptydir/"),
}); err != nil {
t.Fatalf("error creating empty directory: %s", err)
}

bucketFiles := []string{}
s3driver.Walk(context.Background(), "/testdir", func(fileInfo storagedriver.FileInfo) error {
bucketFiles = append(bucketFiles, fileInfo.Path())
return nil
})

expected := []string{"/testdir/emptydir"}
if !reflect.DeepEqual(bucketFiles, expected) {
t.Errorf("expecting files %+v, found %+v instead", expected, bucketFiles)
}
}

func TestStorageClass(t *testing.T) {
if skipS3() != "" {
t.Skip(skipS3())
Expand Down

0 comments on commit 5a96fbb

Please sign in to comment.