From c62dae06c9fcead09f1f9fa26c016ca832a6a331 Mon Sep 17 00:00:00 2001
From: Petu Eusebiu <peusebiu@cisco.com>
Date: Fri, 18 Mar 2022 11:24:24 +0200
Subject: [PATCH] s3: fix initRepo not creating index.json in some edge cases

Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>
---
 pkg/storage/s3/s3_test.go | 80 +++++++++++++++++++++++++++++++++++++++
 pkg/storage/s3/storage.go |  4 --
 2 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/pkg/storage/s3/s3_test.go b/pkg/storage/s3/s3_test.go
index 1131e9d2..507cbccf 100644
--- a/pkg/storage/s3/s3_test.go
+++ b/pkg/storage/s3/s3_test.go
@@ -255,6 +255,86 @@ func (s *StorageDriverMock) Walk(ctx context.Context, path string, f driver.Walk
 	return nil
 }
 
+func TestStorageDriverStatFunction(t *testing.T) {
+	skipIt(t)
+
+	uuid, err := guuid.NewV4()
+	if err != nil {
+		panic(err)
+	}
+
+	testDir := path.Join("/oci-repo-test", uuid.String())
+
+	storeDriver, imgStore, _ := createObjectsStore(testDir)
+	defer cleanupStorage(storeDriver, testDir)
+
+	/* There is an issue with storageDriver.Stat() that returns a storageDriver.FileInfo()
+	which falsely reports isDir() as true for paths under certain circumstances
+	for example:
+	1) create a file, eg: repo/testImageA/file
+	2) run storageDriver.Stat() on a partial path, eg: storageDriver.Stat("repo/testImage") - without 'A' char
+	3) the returned storageDriver.FileInfo will report that isDir() is true.
+	*/
+	Convey("Validate storageDriver.Stat() and isDir() functions with zot storage API", t, func(c C) {
+		repo1 := "repo/testImageA"
+		repo2 := "repo/testImage"
+
+		So(imgStore, ShouldNotBeNil)
+
+		err = imgStore.InitRepo(repo1)
+		So(err, ShouldBeNil)
+
+		isValid, err := imgStore.ValidateRepo(repo1)
+		So(err, ShouldBeNil)
+		So(isValid, ShouldBeTrue)
+
+		err = imgStore.InitRepo(repo2)
+		So(err, ShouldBeNil)
+
+		isValid, err = imgStore.ValidateRepo(repo2)
+		So(err, ShouldBeNil)
+		So(isValid, ShouldBeTrue)
+	})
+
+	Convey("Validate storageDriver.Stat() and isDir() functions with storageDriver API", t, func(c C) {
+		testFile := "/ab/cd/file"
+
+		shouldBeDirectoryPath1 := "/ab/cd"
+		shouldBeDirectoryPath2 := "/ab"
+
+		shouldNotBeDirectoryPath1 := "/ab/c"
+		shouldNotBeDirectoryPath2 := "/a"
+
+		err := storeDriver.PutContent(context.Background(), testFile, []byte("file contents"))
+		So(err, ShouldBeNil)
+
+		fileInfo, err := storeDriver.Stat(context.Background(), testFile)
+		So(err, ShouldBeNil)
+
+		So(fileInfo.IsDir(), ShouldBeFalse)
+
+		fileInfo, err = storeDriver.Stat(context.Background(), shouldBeDirectoryPath1)
+		So(err, ShouldBeNil)
+		So(fileInfo.IsDir(), ShouldBeTrue)
+
+		fileInfo, err = storeDriver.Stat(context.Background(), shouldBeDirectoryPath2)
+		So(err, ShouldBeNil)
+		So(fileInfo.IsDir(), ShouldBeTrue)
+
+		fileInfo, err = storeDriver.Stat(context.Background(), shouldNotBeDirectoryPath1)
+		// err should actually be storageDriver.PathNotFoundError but it's nil
+		So(err, ShouldBeNil)
+		// should be false instead
+		So(fileInfo.IsDir(), ShouldBeTrue)
+
+		fileInfo, err = storeDriver.Stat(context.Background(), shouldNotBeDirectoryPath2)
+		// err should actually be storageDriver.PathNotFoundError but it's nils
+		So(err, ShouldBeNil)
+		// should be false instead
+		So(fileInfo.IsDir(), ShouldBeTrue)
+	})
+}
+
 func TestNegativeCasesObjectsStorage(t *testing.T) {
 	skipIt(t)
 
diff --git a/pkg/storage/s3/storage.go b/pkg/storage/s3/storage.go
index c333a861..a0dc0203 100644
--- a/pkg/storage/s3/storage.go
+++ b/pkg/storage/s3/storage.go
@@ -116,10 +116,6 @@ func (is *ObjectStorage) Unlock(lockStart *time.Time) {
 func (is *ObjectStorage) initRepo(name string) error {
 	repoDir := path.Join(is.rootDir, name)
 
-	if fi, err := is.store.Stat(context.Background(), repoDir); err == nil && fi.IsDir() {
-		return nil
-	}
-
 	// "oci-layout" file - create if it doesn't exist
 	ilPath := path.Join(repoDir, ispec.ImageLayoutFile)
 	if _, err := is.store.Stat(context.Background(), ilPath); err != nil {