diff --git a/pkg/storage/common/common_test.go b/pkg/storage/common/common_test.go index 768b756ad..e8ef700b6 100644 --- a/pkg/storage/common/common_test.go +++ b/pkg/storage/common/common_test.go @@ -25,6 +25,8 @@ import ( "zotregistry.dev/zot/pkg/test/mocks" ) +var ErrTestError = errors.New("TestError") + func TestValidateManifest(t *testing.T) { Convey("Make manifest", t, func(c C) { dir := t.TempDir() @@ -510,3 +512,32 @@ func TestIsSignature(t *testing.T) { So(isSingature, ShouldBeFalse) }) } + +func TestDedupeGeneratorErrors(t *testing.T) { + log := log.Logger{Logger: zerolog.New(os.Stdout)} + + // Ideally this would be covered by the end-to-end test, + // but the coverage for the error is unpredictable, prone to race conditions + Convey("GetNextDigestWithBlobPaths errors", t, func(c C) { + imgStore := &mocks.MockedImageStore{ + GetRepositoriesFn: func() ([]string, error) { + return []string{"repo1", "repo2"}, nil + }, + GetNextDigestWithBlobPathsFn: func(repos []string, lastDigests []godigest.Digest) ( + godigest.Digest, []string, error, + ) { + return "sha256:123", []string{}, ErrTestError + }, + } + + generator := &common.DedupeTaskGenerator{ + ImgStore: imgStore, + Dedupe: true, + Log: log, + } + + task, err := generator.Next() + So(err, ShouldNotBeNil) + So(task, ShouldBeNil) + }) +} diff --git a/pkg/storage/local/driver.go b/pkg/storage/local/driver.go index 8d81779df..528152edb 100644 --- a/pkg/storage/local/driver.go +++ b/pkg/storage/local/driver.go @@ -5,11 +5,9 @@ import ( "bytes" "errors" "io" - "io/fs" "os" "path" "sort" - "syscall" "time" "unicode/utf8" @@ -45,13 +43,10 @@ func (driver *Driver) DirExists(path string) bool { fileInfo, err := os.Stat(path) if err != nil { - if e, ok := err.(*fs.PathError); ok && errors.Is(e.Err, syscall.ENAMETOOLONG) || //nolint: errorlint - errors.Is(e.Err, syscall.EINVAL) { - return false - } - } - - if err != nil && os.IsNotExist(err) { + // if os.Stat returns any error, fileInfo will be nil + // we can't check if the path is a directory using fileInfo if we received an error + // let's assume the directory doesn't exist in all error cases + // see possible errors http://man.he.net/man2/newfstatat return false } diff --git a/pkg/storage/local/driver_test.go b/pkg/storage/local/driver_test.go new file mode 100644 index 000000000..4e0e5acab --- /dev/null +++ b/pkg/storage/local/driver_test.go @@ -0,0 +1,115 @@ +package local_test + +import ( + "os" + "path" + "strings" + "testing" + + storagedriver "github.com/docker/distribution/registry/storage/driver" + . "github.com/smartystreets/goconvey/convey" + + storageConstants "zotregistry.dev/zot/pkg/storage/constants" + "zotregistry.dev/zot/pkg/storage/local" +) + +func TestStorageDriver(t *testing.T) { + driver := local.New(true) + + Convey("Test DirExists", t, func() { + rootDir := t.TempDir() + + // Folder exists + result := driver.DirExists(rootDir) + So(result, ShouldBeTrue) + + // Folder name triggering ENAMETOOLONG + result = driver.DirExists(path.Join(rootDir, strings.Repeat("1234567890", 1000))) + So(result, ShouldBeFalse) + + // Folder which does not exist + result = driver.DirExists(path.Join(rootDir, "someName")) + So(result, ShouldBeFalse) + + // Path is actually a file + fileName := "testFile" + _, err := os.Create(path.Join(rootDir, fileName)) + So(err, ShouldBeNil) + result = driver.DirExists(path.Join(rootDir, fileName)) + So(result, ShouldBeFalse) + + // Folder name triggering ENOTDIR a one of the parents is not a folder + result = driver.DirExists(path.Join(rootDir, fileName, "someName")) + So(result, ShouldBeFalse) + + // New folder created by driver + repoName := "testRepo" + err = driver.EnsureDir(path.Join(rootDir, repoName)) + So(err, ShouldBeNil) + + result = driver.DirExists(path.Join(rootDir, repoName)) + So(result, ShouldBeTrue) + + // Folder without permissions + err = os.Chmod(path.Join(rootDir, repoName), 0o000) + So(err, ShouldBeNil) + defer os.Chmod(path.Join(rootDir, repoName), storageConstants.DefaultDirPerms) //nolint:errcheck + + result = driver.DirExists(path.Join(rootDir, repoName)) + So(result, ShouldBeTrue) + }) + + Convey("Test Walk", t, func() { + Convey("Test all folders are walked and files are identified correctly", func() { + rootDir := t.TempDir() + err := driver.EnsureDir(path.Join(rootDir, "d1", "d11")) + So(err, ShouldBeNil) + err = driver.EnsureDir(path.Join(rootDir, "d1", "d12")) + So(err, ShouldBeNil) + err = driver.EnsureDir(path.Join(rootDir, "d2")) + So(err, ShouldBeNil) + _, err = os.Create(path.Join(rootDir, "d1", "d11", "f111")) + So(err, ShouldBeNil) + _, err = os.Create(path.Join(rootDir, "d2", "f21")) + So(err, ShouldBeNil) + + fileList := []string{} + folderList := []string{} + + err = driver.Walk(rootDir, func(fileInfo storagedriver.FileInfo) error { + if fileInfo.IsDir() { + folderList = append(folderList, fileInfo.Path()) + } else { + fileList = append(fileList, fileInfo.Path()) + } + + return nil + }) + So(err, ShouldBeNil) + + So(len(fileList), ShouldEqual, 2) + So(fileList, ShouldContain, path.Join(rootDir, "d1", "d11", "f111")) + So(fileList, ShouldContain, path.Join(rootDir, "d2", "f21")) + So(len(folderList), ShouldEqual, 4) + So(folderList, ShouldContain, path.Join(rootDir, "d1")) + So(folderList, ShouldContain, path.Join(rootDir, "d1", "d11")) + So(folderList, ShouldContain, path.Join(rootDir, "d1", "d12")) + So(folderList, ShouldContain, path.Join(rootDir, "d2")) + }) + + Convey("Test deleting folders while walking raises doesn't raise errors", func() { + rootDir := t.TempDir() + err := driver.EnsureDir(path.Join(rootDir, "d1")) + So(err, ShouldBeNil) + err = driver.EnsureDir(path.Join(rootDir, "d2")) + So(err, ShouldBeNil) + + // List/Sort d1 and d2, delete d2 while d1 is walked + // While d2 is walked the PathNotFoundError should be ignored + err = driver.Walk(rootDir, func(fileInfo storagedriver.FileInfo) error { + return driver.Delete(path.Join(rootDir, "d2")) + }) + So(err, ShouldBeNil) + }) + }) +} diff --git a/pkg/storage/s3/s3_test.go b/pkg/storage/s3/s3_test.go index 73a948e17..c2fff6060 100644 --- a/pkg/storage/s3/s3_test.go +++ b/pkg/storage/s3/s3_test.go @@ -2261,9 +2261,29 @@ func TestRebuildDedupeIndex(t *testing.T) { }) } -func TestRebuildDedupeMockStoreDriver(t *testing.T) { - tskip.SkipS3(t) +func TestNextRepositoryMockStoreDriver(t *testing.T) { + testDir := t.TempDir() + tdir := t.TempDir() + + // some s3 implementations (eg, digitalocean spaces) will return pathnotfounderror for walk but not list + // This code cannot be reliably covered by end to end tests + Convey("Trigger PathNotFound error when Walk() is called in GetNextRepository()", t, func() { + imgStore := createMockStorage(testDir, tdir, false, &StorageDriverMock{ + ListFn: func(ctx context.Context, path string) ([]string, error) { + return []string{}, nil + }, + WalkFn: func(ctx context.Context, path string, walkFn driver.WalkFn) error { + return driver.PathNotFoundError{} + }, + }) + nextRepository, err := imgStore.GetNextRepository("testRepo") + So(err, ShouldBeNil) + So(nextRepository, ShouldEqual, "") + }) +} + +func TestRebuildDedupeMockStoreDriver(t *testing.T) { uuid, err := guuid.NewV4() if err != nil { panic(err)