Skip to content

Commit

Permalink
remove unnecessary calls to storage driver (#2432)
Browse files Browse the repository at this point in the history 
fix(storage): remove unnecessary calls to storage driver

Signed-off-by: Petu Eusebiu <[email protected]>
  • Loading branch information
@eusebiu-constantin-petu-dbk
eusebiu-constantin-petu-dbk authored Jun 4, 2024
1 parent 1594852 commit a4b6892
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 102 deletions.
1 change: 1 addition & 0 deletions .github/workflows/ecosystem-tools.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
54 changes: 15 additions & 39 deletions pkg/storage/imagestore/imagestore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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 err != nil {
return false, err
if path.Base(file) == ispec.ImageIndexFile {
found[ispec.ImageIndexFile] = true
}

filename, err := filepath.Rel(dir, file)
if err != nil {
return false, err
if strings.HasSuffix(file, ispec.ImageLayoutFile) {
found[ispec.ImageLayoutFile] = true
}

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 {
if !v && k != storageConstants.BlobUploadDir {
for _, v := range found {
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
}

Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
9 changes: 4 additions & 5 deletions pkg/storage/local/local_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1562,18 +1562,17 @@ func TestNegativeCases(t *testing.T) {
panic(err)
}
isValid, err = imgStore.ValidateRepo("invalid-test")
So(err, ShouldNotBeNil)
So(isValid, ShouldEqual, false)
So(err, ShouldBeNil)
So(isValid, ShouldEqual, true)

err = os.WriteFile(path.Join(dir, "invalid-test", ispec.ImageLayoutFile), []byte("{}"), 0o755) //nolint: gosec
if err != nil {
panic(err)
}

isValid, err = imgStore.ValidateRepo("invalid-test")
So(err, ShouldNotBeNil)
So(err, ShouldEqual, zerr.ErrRepoBadVersion)
So(isValid, ShouldEqual, false)
So(err, ShouldBeNil)
So(isValid, ShouldEqual, true)

files, err := os.ReadDir(path.Join(dir, "test"))
if err != nil {
Expand Down
58 changes: 0 additions & 58 deletions pkg/storage/s3/s3_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit a4b6892

Please sign in to comment.