remove unnecessary calls to storage driver (#2432)

fix(storage): remove unnecessary calls to storage driver

Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>

.github/workflows/ecosystem-tools.yaml

@@ -49,6 +49,7 @@ jobs:
           # install dex
           git clone https://github.com/dexidp/dex.git
           cd dex/
+          git checkout v2.39.1
           make bin/dex
           ./bin/dex serve $GITHUB_WORKSPACE/test/dex/config-dev.yaml &
           cd $GITHUB_WORKSPACE

pkg/storage/imagestore/imagestore.go

@@ -140,7 +140,7 @@ func (is *ImageStore) initRepo(name string) error {
 	}
 
 	// create "blobs" subdir
-	err := is.storeDriver.EnsureDir(path.Join(repoDir, "blobs"))
+	err := is.storeDriver.EnsureDir(path.Join(repoDir, ispec.ImageBlobsDir))
 	if err != nil {
 		is.log.Error().Err(err).Str("repository", name).Str("dir", repoDir).Msg("failed to create blobs subdir")
 
@@ -174,7 +174,7 @@ func (is *ImageStore) initRepo(name string) error {
 	}
 
 	// "index.json" file - create if it doesn't exist
-	indexPath := path.Join(repoDir, "index.json")
+	indexPath := path.Join(repoDir, ispec.ImageIndexFile)
 	if _, err := is.storeDriver.Stat(indexPath); err != nil {
 		index := ispec.Index{}
 		index.SchemaVersion = 2
@@ -217,9 +217,6 @@ func (is *ImageStore) ValidateRepo(name string) (bool, error) {
 	// and an additional/optional BlobUploadDir in each image store
 	// for s3 we can not create empty dirs, so we check only against index.json and oci-layout
 	dir := path.Join(is.rootDir, name)
-	if fi, err := is.storeDriver.Stat(dir); err != nil || !fi.IsDir() {
-		return false, zerr.ErrRepoNotFound
-	}
 
 	files, err := is.storeDriver.List(dir)
 	if err != nil {
@@ -235,54 +232,32 @@ func (is *ImageStore) ValidateRepo(name string) (bool, error) {
 
 	found := map[string]bool{
 		ispec.ImageLayoutFile: false,
-		"index.json":          false,
+		ispec.ImageIndexFile:  false,
 	}
 
 	for _, file := range files {
-		fileInfo, err := is.storeDriver.Stat(file)
+		if path.Base(file) == ispec.ImageIndexFile {
-		if err != nil {
+			found[ispec.ImageIndexFile] = true
-			return false, err
 		}
 
-		filename, err := filepath.Rel(dir, file)
+		if strings.HasSuffix(file, ispec.ImageLayoutFile) {
-		if err != nil {
+			found[ispec.ImageLayoutFile] = true
-			return false, err
 		}
-
-		if filename == "blobs" && !fileInfo.IsDir() {
-			return false, nil
-		}
-
-		found[filename] = true
 	}
 
 	// check blobs dir exists only for filesystem, in s3 we can't have empty dirs
 	if is.storeDriver.Name() == storageConstants.LocalStorageDriverName {
-		if !is.storeDriver.DirExists(path.Join(dir, "blobs")) {
+		if !is.storeDriver.DirExists(path.Join(dir, ispec.ImageBlobsDir)) {
 			return false, nil
 		}
 	}
 
-	for k, v := range found {
+	for _, v := range found {
-		if !v && k != storageConstants.BlobUploadDir {
+		if !v {
 			return false, nil
 		}
 	}
 
-	buf, err := is.storeDriver.ReadFile(path.Join(dir, ispec.ImageLayoutFile))
-	if err != nil {
-		return false, err
-	}
-
-	var il ispec.ImageLayout
-	if err := json.Unmarshal(buf, &il); err != nil {
-		return false, err
-	}
-
-	if il.Version != ispec.ImageLayoutVersion {
-		return false, zerr.ErrRepoBadVersion
-	}
-
 	return true, nil
 }
 
@@ -304,6 +279,7 @@ func (is *ImageStore) GetRepositories() ([]string, error) {
 
 		// skip .sync and .uploads dirs no need to try to validate them
 		if strings.HasSuffix(fileInfo.Path(), syncConstants.SyncBlobUploadDir) ||
+			strings.HasSuffix(fileInfo.Path(), ispec.ImageBlobsDir) ||
 			strings.HasSuffix(fileInfo.Path(), storageConstants.BlobUploadDir) {
 			return driver.ErrSkipDir
 		}
@@ -669,7 +645,7 @@ func (is *ImageStore) deleteImageManifest(repo, reference string, detectCollisio
 
 	// now update "index.json"
 	dir := path.Join(is.rootDir, repo)
-	file := path.Join(dir, "index.json")
+	file := path.Join(dir, ispec.ImageIndexFile)
 
 	buf, err := json.Marshal(index)
 	if err != nil {
@@ -1453,7 +1429,7 @@ func (is *ImageStore) GetReferrers(repo string, gdigest godigest.Digest, artifac
 func (is *ImageStore) GetIndexContent(repo string) ([]byte, error) {
 	dir := path.Join(is.rootDir, repo)
 
-	buf, err := is.storeDriver.ReadFile(path.Join(dir, "index.json"))
+	buf, err := is.storeDriver.ReadFile(path.Join(dir, ispec.ImageIndexFile))
 	if err != nil {
 		if errors.Is(err, driver.PathNotFoundError{}) {
 			is.log.Error().Err(err).Str("dir", dir).Msg("failed to read index.json")
@@ -1470,7 +1446,7 @@ func (is *ImageStore) GetIndexContent(repo string) ([]byte, error) {
 }
 
 func (is *ImageStore) StatIndex(repo string) (bool, int64, time.Time, error) {
-	repoIndexPath := path.Join(is.rootDir, repo, "index.json")
+	repoIndexPath := path.Join(is.rootDir, repo, ispec.ImageIndexFile)
 
 	fileInfo, err := is.storeDriver.Stat(repoIndexPath)
 	if err != nil {
@@ -1491,7 +1467,7 @@ func (is *ImageStore) StatIndex(repo string) (bool, int64, time.Time, error) {
 func (is *ImageStore) PutIndexContent(repo string, index ispec.Index) error {
 	dir := path.Join(is.rootDir, repo)
 
-	indexPath := path.Join(dir, "index.json")
+	indexPath := path.Join(dir, ispec.ImageIndexFile)
 
 	buf, err := json.Marshal(index)
 	if err != nil {

pkg/storage/local/local_test.go

@@ -1562,8 +1562,8 @@ func TestNegativeCases(t *testing.T) {
 			panic(err)
 		}
 		isValid, err = imgStore.ValidateRepo("invalid-test")
-		So(err, ShouldNotBeNil)
+		So(err, ShouldBeNil)
-		So(isValid, ShouldEqual, false)
+		So(isValid, ShouldEqual, true)
 
 		err = os.WriteFile(path.Join(dir, "invalid-test", ispec.ImageLayoutFile), []byte("{}"), 0o755) //nolint: gosec
 		if err != nil {
@@ -1571,9 +1571,8 @@ func TestNegativeCases(t *testing.T) {
 		}
 
 		isValid, err = imgStore.ValidateRepo("invalid-test")
-		So(err, ShouldNotBeNil)
+		So(err, ShouldBeNil)
-		So(err, ShouldEqual, zerr.ErrRepoBadVersion)
+		So(isValid, ShouldEqual, true)
-		So(isValid, ShouldEqual, false)
 
 		files, err := os.ReadDir(path.Join(dir, "test"))
 		if err != nil {

pkg/storage/s3/s3_test.go

@@ -862,64 +862,6 @@ func TestNegativeCasesObjectsStorage(t *testing.T) {
 
 			_, err := imgStore.ValidateRepo(testImage)
 			So(err, ShouldNotBeNil)
-
-			imgStore = createMockStorage(testDir, tdir, false, &StorageDriverMock{
-				ListFn: func(ctx context.Context, path string) ([]string, error) {
-					return []string{testImage, testImage}, nil
-				},
-				StatFn: func(ctx context.Context, path string) (driver.FileInfo, error) {
-					return nil, errS3
-				},
-			})
-
-			_, err = imgStore.ValidateRepo(testImage)
-			So(err, ShouldNotBeNil)
-		})
-
-		Convey("Test ValidateRepo2", func(c C) {
-			imgStore = createMockStorage(testDir, tdir, false, &StorageDriverMock{
-				ListFn: func(ctx context.Context, path string) ([]string, error) {
-					return []string{"test/test/oci-layout", "test/test/index.json"}, nil
-				},
-				StatFn: func(ctx context.Context, path string) (driver.FileInfo, error) {
-					return &FileInfoMock{}, nil
-				},
-			})
-			_, err := imgStore.ValidateRepo(testImage)
-			So(err, ShouldNotBeNil)
-		})
-
-		Convey("Test ValidateRepo3", func(c C) {
-			imgStore = createMockStorage(testDir, tdir, false, &StorageDriverMock{
-				ListFn: func(ctx context.Context, path string) ([]string, error) {
-					return []string{"test/test/oci-layout", "test/test/index.json"}, nil
-				},
-				StatFn: func(ctx context.Context, path string) (driver.FileInfo, error) {
-					return &FileInfoMock{}, nil
-				},
-				GetContentFn: func(ctx context.Context, path string) ([]byte, error) {
-					return []byte{}, errS3
-				},
-			})
-			_, err := imgStore.ValidateRepo(testImage)
-			So(err, ShouldNotBeNil)
-		})
-
-		Convey("Test ValidateRepo4", func(c C) {
-			ociLayout := []byte(`{"imageLayoutVersion": "9.9.9"}`)
-			imgStore = createMockStorage(testDir, tdir, false, &StorageDriverMock{
-				ListFn: func(ctx context.Context, path string) ([]string, error) {
-					return []string{"test/test/oci-layout", "test/test/index.json"}, nil
-				},
-				StatFn: func(ctx context.Context, path string) (driver.FileInfo, error) {
-					return &FileInfoMock{}, nil
-				},
-				GetContentFn: func(ctx context.Context, path string) ([]byte, error) {
-					return ociLayout, nil
-				},
-			})
-			_, err := imgStore.ValidateRepo(testImage)
-			So(err, ShouldNotBeNil)
 		})
 
 		Convey("Test GetRepositories", func(c C) {