diff --git a/registry/storage/blobstore.go b/registry/storage/blobstore.go index 1008aad8c06..ee7a03f0c48 100644 --- a/registry/storage/blobstore.go +++ b/registry/storage/blobstore.go @@ -93,12 +93,7 @@ func (bs *blobStore) Enumerate(ctx context.Context, ingester func(dgst digest.Di return err } - return bs.driver.Walk(ctx, specPath, func(fileInfo driver.FileInfo) error { - // skip directories - if fileInfo.IsDir() { - return nil - } - + return bs.driver.WalkFiles(ctx, specPath, func(fileInfo driver.FileInfo) error { currentPath := fileInfo.Path() // we only want to parse paths that end with /data _, fileName := path.Split(currentPath) diff --git a/registry/storage/driver/azure/azure.go b/registry/storage/driver/azure/azure.go index 58c6293b234..c36fa7dfb16 100644 --- a/registry/storage/driver/azure/azure.go +++ b/registry/storage/driver/azure/azure.go @@ -360,11 +360,17 @@ func (d *driver) URLFor(ctx context.Context, path string, options map[string]int } // Walk traverses a filesystem defined within driver, starting -// from the given path, calling f on each file +// from the given path, calling f on each file and directory func (d *driver) Walk(ctx context.Context, path string, f storagedriver.WalkFn) error { return storagedriver.WalkFallback(ctx, d, path, f) } +// WalkFiles traverses a filesystem defined within driver, starting +// from the given path, calling f on each file +func (d *driver) WalkFiles(ctx context.Context, path string, f storagedriver.WalkFn) error { + return storagedriver.WalkFilesFallback(ctx, d, path, f) +} + // directDescendants will find direct descendants (blobs or virtual containers) // of from list of blob paths and will return their full paths. Elements in blobs // list must be prefixed with a "/" and diff --git a/registry/storage/driver/base/base.go b/registry/storage/driver/base/base.go index 3864eacf70c..e2f65f270e7 100644 --- a/registry/storage/driver/base/base.go +++ b/registry/storage/driver/base/base.go @@ -241,3 +241,15 @@ func (base *Base) Walk(ctx context.Context, path string, f storagedriver.WalkFn) return base.setDriverName(base.StorageDriver.Walk(ctx, path, f)) } + +// WalkFiles wraps WalkFiles of underlying storage driver. +func (base *Base) WalkFiles(ctx context.Context, path string, f storagedriver.WalkFn) error { + ctx, done := dcontext.WithTrace(ctx) + defer done("%s.WalkFiles(%q)", base.Name(), path) + + if !storagedriver.PathRegexp.MatchString(path) && path != "/" { + return storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} + } + + return base.setDriverName(base.StorageDriver.WalkFiles(ctx, path, f)) +} diff --git a/registry/storage/driver/filesystem/driver.go b/registry/storage/driver/filesystem/driver.go index 8fc9d1ca00d..b4613f455b8 100644 --- a/registry/storage/driver/filesystem/driver.go +++ b/registry/storage/driver/filesystem/driver.go @@ -290,11 +290,17 @@ func (d *driver) URLFor(ctx context.Context, path string, options map[string]int } // Walk traverses a filesystem defined within driver, starting -// from the given path, calling f on each file +// from the given path, calling f on each file and directory func (d *driver) Walk(ctx context.Context, path string, f storagedriver.WalkFn) error { return storagedriver.WalkFallback(ctx, d, path, f) } +// WalkFiles traverses a filesystem defined within driver, starting +// from the given path, calling f on each file +func (d *driver) WalkFiles(ctx context.Context, path string, f storagedriver.WalkFn) error { + return storagedriver.WalkFilesFallback(ctx, d, path, f) +} + // fullPath returns the absolute path of a key within the Driver's storage. func (d *driver) fullPath(subPath string) string { return path.Join(d.rootDirectory, subPath) diff --git a/registry/storage/driver/inmemory/driver.go b/registry/storage/driver/inmemory/driver.go index 4b1f5f48dcc..0832bb23857 100644 --- a/registry/storage/driver/inmemory/driver.go +++ b/registry/storage/driver/inmemory/driver.go @@ -245,11 +245,17 @@ func (d *driver) URLFor(ctx context.Context, path string, options map[string]int } // Walk traverses a filesystem defined within driver, starting -// from the given path, calling f on each file +// from the given path, calling f on each file and directory func (d *driver) Walk(ctx context.Context, path string, f storagedriver.WalkFn) error { return storagedriver.WalkFallback(ctx, d, path, f) } +// WalkFiles traverses a filesystem defined within driver, starting +// from the given path, calling f on each file +func (d *driver) WalkFiles(ctx context.Context, path string, f storagedriver.WalkFn) error { + return storagedriver.WalkFilesFallback(ctx, d, path, f) +} + type writer struct { d *driver f *file diff --git a/registry/storage/driver/s3-aws/s3.go b/registry/storage/driver/s3-aws/s3.go index 7ee0f79e24d..2c0b9d08dd1 100644 --- a/registry/storage/driver/s3-aws/s3.go +++ b/registry/storage/driver/s3-aws/s3.go @@ -1057,7 +1057,7 @@ func (d *driver) URLFor(ctx context.Context, path string, options map[string]int } // Walk traverses a filesystem defined within driver, starting -// from the given path, calling f on each file +// from the given path, calling f on each file and directory func (d *driver) Walk(ctx context.Context, from string, f storagedriver.WalkFn) error { path := from if !strings.HasSuffix(path, "/") { @@ -1070,7 +1070,33 @@ func (d *driver) Walk(ctx context.Context, from string, f storagedriver.WalkFn) } var objectCount int64 - if err := d.doWalk(ctx, &objectCount, d.s3Path(path), prefix, f); err != nil { + if err := d.doWalk(ctx, &objectCount, d.s3Path(path), prefix, true, f); err != nil { + return err + } + + // S3 doesn't have the concept of empty directories, so it'll return path not found if there are no objects + if objectCount == 0 { + return storagedriver.PathNotFoundError{Path: from} + } + + return nil +} + +// WalkFiles traverses a filesystem defined within driver, starting +// from the given path, calling f on each file +func (d *driver) WalkFiles(ctx context.Context, from string, f storagedriver.WalkFn) error { + path := from + if !strings.HasSuffix(path, "/") { + path = path + "/" + } + + prefix := "" + if d.s3Path("") == "" { + prefix = "/" + } + + var objectCount int64 + if err := d.doWalk(ctx, &objectCount, d.s3Path(path), prefix, false, f); err != nil { return err } @@ -1110,13 +1136,17 @@ func (wi walkInfoContainer) IsDir() bool { return wi.FileInfoFields.IsDir } -func (d *driver) doWalk(parentCtx context.Context, objectCount *int64, path, prefix string, f storagedriver.WalkFn) error { +func (d *driver) doWalk(parentCtx context.Context, objectCount *int64, path, prefix string, walkDirectories bool, f storagedriver.WalkFn) error { var retError error + delimiter := "" + if walkDirectories { + delimiter = "/" + } listObjectsInput := &s3.ListObjectsV2Input{ Bucket: aws.String(d.Bucket), Prefix: aws.String(path), - Delimiter: aws.String("/"), + Delimiter: aws.String(delimiter), MaxKeys: aws.Int64(listMax), } @@ -1180,8 +1210,8 @@ func (d *driver) doWalk(parentCtx context.Context, objectCount *int64, path, pre return false } - if walkInfo.IsDir() { - if err := d.doWalk(ctx, objectCount, *walkInfo.prefix, prefix, f); err != nil { + if walkInfo.IsDir() && walkDirectories { + if err := d.doWalk(ctx, objectCount, *walkInfo.prefix, prefix, walkDirectories, f); err != nil { retError = err return false } diff --git a/registry/storage/driver/storagedriver.go b/registry/storage/driver/storagedriver.go index 340d1080434..025b6c67d92 100644 --- a/registry/storage/driver/storagedriver.go +++ b/registry/storage/driver/storagedriver.go @@ -90,6 +90,11 @@ type StorageDriver interface { // to a directory, the directory will not be entered and Walk // will continue the traversal. If fileInfo refers to a normal file, processing stops Walk(ctx context.Context, path string, f WalkFn) error + + // WalkFiles traverses a filesystem defined within driver, starting + // from the given path, calling f on each file but does not call f with directories. + // If an error is returned from the WalkFn, processing stops + WalkFiles(ctx context.Context, path string, f WalkFn) error } // FileWriter provides an abstraction for an opened writable file-like object in diff --git a/registry/storage/driver/swift/swift.go b/registry/storage/driver/swift/swift.go index b64e3b66cd3..a4bea57ea04 100644 --- a/registry/storage/driver/swift/swift.go +++ b/registry/storage/driver/swift/swift.go @@ -658,11 +658,17 @@ func (d *driver) URLFor(ctx context.Context, path string, options map[string]int } // Walk traverses a filesystem defined within driver, starting -// from the given path, calling f on each file +// from the given path, calling f on each file and directory func (d *driver) Walk(ctx context.Context, path string, f storagedriver.WalkFn) error { return storagedriver.WalkFallback(ctx, d, path, f) } +// WalkFiles traverses a filesystem defined within driver, starting +// from the given path, calling f on each file +func (d *driver) WalkFiles(ctx context.Context, path string, f storagedriver.WalkFn) error { + return storagedriver.WalkFilesFallback(ctx, d, path, f) +} + func (d *driver) swiftPath(path string) string { return strings.TrimLeft(strings.TrimRight(d.Prefix+"/files"+path, "/"), "/") } diff --git a/registry/storage/driver/walk.go b/registry/storage/driver/walk.go index 8ded5b8be72..23fc8800cc5 100644 --- a/registry/storage/driver/walk.go +++ b/registry/storage/driver/walk.go @@ -22,6 +22,20 @@ type WalkFn func(fileInfo FileInfo) error // to a directory, the directory will not be entered and Walk // will continue the traversal. If fileInfo refers to a normal file, processing stops func WalkFallback(ctx context.Context, driver StorageDriver, from string, f WalkFn) error { + return doWalk(ctx, driver, from, true, f) +} + +// WalkFilesFallback traverses a filesystem defined within driver, starting +// from the given path, calling f on each file. It uses the List method and Stat to drive itself. +// If an error is returned from WalkFn, processing stops +func WalkFilesFallback(ctx context.Context, driver StorageDriver, from string, f WalkFn) error { + return doWalk(ctx, driver, from, false, f) +} + +// WalkFilesFallback traverses a filesystem defined within driver, starting +// from the given path, calling f on each file. It uses the List method and Stat to drive itself. +// If an error is returned from WalkFn, processing stops +func doWalk(ctx context.Context, driver StorageDriver, from string, walkDir bool, f WalkFn) error { children, err := driver.List(ctx, from) if err != nil { return err @@ -43,12 +57,15 @@ func WalkFallback(ctx context.Context, driver StorageDriver, from string, f Walk return err } } - err = f(fileInfo) + err = nil + if !fileInfo.IsDir() || walkDir { + err = f(fileInfo) + } if err == nil && fileInfo.IsDir() { - if err := WalkFallback(ctx, driver, child, f); err != nil { + if err := doWalk(ctx, driver, child, walkDir, f); err != nil { return err } - } else if err == ErrSkipDir { + } else if err == ErrSkipDir && walkDir { // Stop iteration if it's a file, otherwise noop if it's a directory if !fileInfo.IsDir() { return nil diff --git a/registry/storage/driver/walk_test.go b/registry/storage/driver/walk_test.go index d6e9beb64bd..36d9c0f489f 100644 --- a/registry/storage/driver/walk_test.go +++ b/registry/storage/driver/walk_test.go @@ -2,7 +2,9 @@ package driver import ( "context" + "errors" "fmt" + "strings" "testing" ) @@ -26,6 +28,31 @@ func (cfs *changingFileSystem) Stat(ctx context.Context, path string) (FileInfo, } return nil, PathNotFoundError{} } + +type fileSystem struct { + StorageDriver + fileset map[string][]string +} + +func (cfs *fileSystem) List(_ context.Context, path string) ([]string, error) { + return cfs.fileset[path], nil +} + +func (cfs *fileSystem) Stat(_ context.Context, path string) (FileInfo, error) { + _, isDir := cfs.fileset[path] + return &FileInfoInternal{ + FileInfoFields: FileInfoFields{ + Path: path, + IsDir: isDir, + Size: int64(len(path)), + }, + }, nil +} +func (cfs *fileSystem) isDir(path string) bool { + _, isDir := cfs.fileset[path] + return isDir +} + func TestWalkFileRemoved(t *testing.T) { d := &changingFileSystem{ fileset: []string{"zoidberg", "bender"}, @@ -45,3 +72,217 @@ func TestWalkFileRemoved(t *testing.T) { t.Fatalf(err.Error()) } } + +func TestWalkFilesFileRemoved(t *testing.T) { + d := &changingFileSystem{ + fileset: []string{"zoidberg", "bender"}, + keptFiles: map[string]bool{ + "zoidberg": true, + }, + } + infos := []FileInfo{} + err := WalkFilesFallback(context.Background(), d, "", func(fileInfo FileInfo) error { + infos = append(infos, fileInfo) + return nil + }) + if len(infos) != 1 || infos[0].Path() != "zoidberg" { + t.Errorf(fmt.Sprintf("unexpected path set during walk: %s", infos)) + } + if err != nil { + t.Fatalf(err.Error()) + } +} + +func TestWalkFallback(t *testing.T) { + d := &fileSystem{ + fileset: map[string][]string{ + "/": {"/file1", "/folder1", "/folder2"}, + "/folder1": {"/folder1/file1"}, + "/folder2": {"/folder2/file1"}, + }, + } + expected := []string{ + "/file1", + "/folder1", + "/folder1/file1", + "/folder2", + "/folder2/file1", + } + + var walked []string + err := WalkFallback(context.Background(), d, "/", func(fileInfo FileInfo) error { + if fileInfo.IsDir() != d.isDir(fileInfo.Path()) { + t.Fatalf("fileInfo isDir not matching file system: expected %t actual %t", d.isDir(fileInfo.Path()), fileInfo.IsDir()) + } + walked = append(walked, fileInfo.Path()) + return nil + }) + if err != nil { + t.Fatalf(err.Error()) + } + compareWalked(t, expected, walked) +} + +// Walk is expected to skip directory on ErrSkipDir +func TestWalkFallbackSkipDirOnDir(t *testing.T) { + d := &fileSystem{ + fileset: map[string][]string{ + "/": {"/file1", "/folder1", "/folder2"}, + "/folder1": {"/folder1/file1"}, + "/folder2": {"/folder2/file1"}, + }, + } + skipDir := "/folder1" + expected := []string{ + "/file1", + "/folder1", // return ErrSkipDir, skip anything under /folder1 + // skip /folder1/file1 + "/folder2", + "/folder2/file1", + } + + var walked []string + err := WalkFallback(context.Background(), d, "/", func(fileInfo FileInfo) error { + walked = append(walked, fileInfo.Path()) + if fileInfo.Path() == skipDir { + return ErrSkipDir + } + if strings.Contains(fileInfo.Path(), skipDir) { + t.Fatalf("skipped dir %s and should not walk %s", skipDir, fileInfo.Path()) + } + return nil + }) + if err != nil { + t.Fatalf("expected Walk to not error %v", err) + } + compareWalked(t, expected, walked) +} + +func TestWalkFallbackSkipDirOnFile(t *testing.T) { + d := &fileSystem{ + fileset: map[string][]string{ + "/": {"/file1", "/file2", "/file3"}, + }, + } + skipFile := "/file2" + expected := []string{ + "/file1", + "/file2", // return ErrSkipDir, stop early + } + + var walked []string + err := WalkFallback(context.Background(), d, "/", func(fileInfo FileInfo) error { + walked = append(walked, fileInfo.Path()) + if fileInfo.Path() == skipFile { + return ErrSkipDir + } + return nil + }) + if err != nil { + t.Fatalf("expected Walk to not error %v", err) + } + compareWalked(t, expected, walked) +} + +// Walk is expected to skip directory on ErrSkipDir +func TestWalkFallbackErr(t *testing.T) { + d := &fileSystem{ + fileset: map[string][]string{ + "/": {"/file1", "/file2", "/file3"}, + }, + } + errFile := "/file2" + expected := []string{ + "/file1", + "/file2", // return ErrSkipDir, stop early + } + expectedErr := errors.New("foo") + + var walked []string + err := WalkFallback(context.Background(), d, "/", func(fileInfo FileInfo) error { + walked = append(walked, fileInfo.Path()) + if fileInfo.Path() == errFile { + return expectedErr + } + return nil + }) + if err != expectedErr { + t.Fatalf("unexpected err %v", err) + } + compareWalked(t, expected, walked) +} + +// WalkFiles is expected to only walk files, not directories +func TestWalkFilesFallback(t *testing.T) { + d := &fileSystem{ + fileset: map[string][]string{ + "/": {"/folder1", "/file1", "/folder2"}, + "/folder1": {"/folder1/folder1"}, + "/folder2": {"/folder2/file1", "/folder2/file2"}, + "/folder1/folder1": {"/folder1/folder1/file1", "/folder1/folder1/file2"}, + }, + } + expected := []string{ + "/file1", + "/folder1/folder1/file1", + "/folder1/folder1/file2", + "/folder2/file1", + "/folder2/file2", + } + + var walked []string + err := WalkFilesFallback(context.Background(), d, "/", func(fileInfo FileInfo) error { + if fileInfo.IsDir() { + t.Fatalf("can't walk over dir %s", fileInfo.Path()) + } + if fileInfo.IsDir() != d.isDir(fileInfo.Path()) { + t.Fatalf("fileInfo isDir not matching file system: expected %t actual %t", d.isDir(fileInfo.Path()), fileInfo.IsDir()) + } + walked = append(walked, fileInfo.Path()) + return nil + }) + if err != nil { + t.Fatalf(err.Error()) + } + compareWalked(t, expected, walked) +} + +// WalkFiles is expected to stop when any error is given +func TestWalkFilesFallbackErr(t *testing.T) { + d := &fileSystem{ + fileset: map[string][]string{ + "/": {"/file1", "/folder1", "/folder2"}, + "/folder1": {"/folder1/file1"}, + "/folder2": {"/folder2/file1"}, + }, + } + skipFile := "/folder1/file1" + expected := []string{ + "/file1", "/folder1/file1", + } + + var walked []string + err := WalkFilesFallback(context.Background(), d, "/", func(fileInfo FileInfo) error { + fmt.Println("Walk ", fileInfo.Path()) + walked = append(walked, fileInfo.Path()) + if fileInfo.Path() == skipFile { + return ErrSkipDir + } + return nil + }) + if err == nil { + t.Fatalf("expected Walk to ErrSkipDir %v", err) + } + compareWalked(t, expected, walked) +} + +func compareWalked(t *testing.T, expected, walked []string) { + if len(walked) != len(expected) { + t.Fatalf("Mismatch number of fileInfo walked %d expected %d", len(walked), len(expected)) + } + for i := range walked { + if walked[i] != expected[i] { + t.Fatalf("expected walked to come in order expected: walked %s", walked) + } + } +} diff --git a/registry/storage/linkedblobstore.go b/registry/storage/linkedblobstore.go index 3e3fffe1903..55992d00de0 100644 --- a/registry/storage/linkedblobstore.go +++ b/registry/storage/linkedblobstore.go @@ -247,11 +247,7 @@ func (lbs *linkedBlobStore) Enumerate(ctx context.Context, ingestor func(digest. if err != nil { return err } - return lbs.driver.Walk(ctx, rootPath, func(fileInfo driver.FileInfo) error { - // exit early if directory... - if fileInfo.IsDir() { - return nil - } + return lbs.driver.WalkFiles(ctx, rootPath, func(fileInfo driver.FileInfo) error { filePath := fileInfo.Path() // check if it's a link