Skip to content

refactor(storage): refactoring storage#1459

Merged
rchincha merged 1 commit into
project-zot:mainfrom
laurentiuNiculae:refactor-storage
May 26, 2023
Merged

refactor(storage): refactoring storage#1459
rchincha merged 1 commit into
project-zot:mainfrom
laurentiuNiculae:refactor-storage

Conversation

@laurentiuNiculae

Copy link
Copy Markdown
Contributor

What type of PR is this?

Which issue does this PR fix:
#Closes

What does this PR do / Why do we need it:

If an issue # is not available please add repro steps and logs showing the issue:

Testing done on this change:

Automation added to e2e:

Will this break upgrades or downgrades?

Does this PR introduce any user-facing change?:


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

var err error
if _, err = os.ReadDir(localRepoPath); os.IsNotExist(err) {
if err = os.MkdirAll(localRepoPath, local.DefaultDirPerms); err != nil {
if err = os.MkdirAll(localRepoPath, storageConstants.DefaultDirPerms); err != nil {

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2). This path depends on a [user-provided value](3). This path depends on a [user-provided value](4).

cachedRepoPath := path.Join(localCachePath, repo)
if err = os.MkdirAll(cachedRepoPath, local.DefaultDirPerms); err != nil {
if err = os.MkdirAll(cachedRepoPath, storageConstants.DefaultDirPerms); err != nil {

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2). This path depends on a [user-provided value](3). This path depends on a [user-provided value](4).
) types.ImageStore {
if _, err := os.Stat(rootDir); os.IsNotExist(err) {
if err := os.MkdirAll(rootDir, DefaultDirPerms); err != nil {
if err := os.MkdirAll(rootDir, storageConstants.DefaultDirPerms); err != nil {

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2). This path depends on a [user-provided value](3). This path depends on a [user-provided value](4).
blobUploadPath := is.BlobUploadPath(repo, uid)

file, err := os.OpenFile(blobUploadPath, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, DefaultFilePerms)
file, err := os.OpenFile(blobUploadPath, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, storageConstants.DefaultFilePerms)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1).
}

file, err := os.OpenFile(blobUploadPath, os.O_WRONLY|os.O_CREATE, DefaultFilePerms)
file, err := os.OpenFile(blobUploadPath, os.O_WRONLY|os.O_CREATE, storageConstants.DefaultFilePerms)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1).
}

file, err := os.OpenFile(blobUploadPath, os.O_WRONLY|os.O_CREATE, DefaultFilePerms)
file, err := os.OpenFile(blobUploadPath, os.O_WRONLY|os.O_CREATE, storageConstants.DefaultFilePerms)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).
func (is *ImageStoreLocal) writeFile(filename string, data []byte) error {
if !is.commit {
return os.WriteFile(filename, data, DefaultFilePerms)
return os.WriteFile(filename, data, storageConstants.DefaultFilePerms)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2). This path depends on a [user-provided value](3). This path depends on a [user-provided value](4). This path depends on a [user-provided value](5). This path depends on a [user-provided value](6). This path depends on a [user-provided value](7). This path depends on a [user-provided value](8). This path depends on a [user-provided value](9). This path depends on a [user-provided value](10).
}

fhandle, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, DefaultFilePerms)
fhandle, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, storageConstants.DefaultFilePerms)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2). This path depends on a [user-provided value](3). This path depends on a [user-provided value](4). This path depends on a [user-provided value](5). This path depends on a [user-provided value](6). This path depends on a [user-provided value](7). This path depends on a [user-provided value](8). This path depends on a [user-provided value](9). This path depends on a [user-provided value](10).

func ensureDir(dir string, log zerolog.Logger) error {
if err := os.MkdirAll(dir, DefaultDirPerms); err != nil {
if err := os.MkdirAll(dir, storageConstants.DefaultDirPerms); err != nil {

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2). This path depends on a [user-provided value](3). This path depends on a [user-provided value](4). This path depends on a [user-provided value](5). This path depends on a [user-provided value](6). This path depends on a [user-provided value](7). This path depends on a [user-provided value](8). This path depends on a [user-provided value](9). This path depends on a [user-provided value](10).
@laurentiuNiculae
laurentiuNiculae force-pushed the refactor-storage branch 3 times, most recently from c8cc3f8 to b9ec429 Compare May 18, 2023 14:06
@codecov

codecov Bot commented May 18, 2023

Copy link
Copy Markdown

Codecov Report

Merging #1459 (edcfa2a) into main (9acd19f) will increase coverage by 0.01%.
The diff coverage is 91.90%.

@@            Coverage Diff             @@
##             main    #1459      +/-   ##
==========================================
+ Coverage   90.97%   90.98%   +0.01%     
==========================================
  Files         103      104       +1     
  Lines       22635    22639       +4     
==========================================
+ Hits        20592    20599       +7     
+ Misses       1522     1520       -2     
+ Partials      521      520       -1     
Impacted Files Coverage Δ
pkg/extensions/extension_scrub.go 100.00% <ø> (ø)
pkg/extensions/lint/lint-disabled.go 100.00% <ø> (ø)
pkg/extensions/lint/lint.go 87.50% <ø> (ø)
pkg/extensions/sync/signatures.go 86.86% <ø> (ø)
pkg/meta/repodb/storage_parsing.go 100.00% <ø> (ø)
pkg/test/inject/dev.go 92.30% <ø> (ø)
pkg/test/inject/prod.go 71.42% <ø> (ø)
pkg/test/common.go 84.69% <50.00%> (ø)
pkg/storage/storage.go 89.93% <89.93%> (ø)
pkg/storage/local/local.go 81.96% <97.95%> (-0.02%) ⬇️
... and 13 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@laurentiuNiculae
laurentiuNiculae force-pushed the refactor-storage branch 4 times, most recently from 442b5e5 to 1d25fb1 Compare May 19, 2023 10:41
@laurentiuNiculae
laurentiuNiculae marked this pull request as ready for review May 19, 2023 11:41
Comment thread pkg/storage/common/common.go Outdated
Comment thread pkg/api/routes_test.go Outdated
Comment thread pkg/storage/cache.go Outdated
Comment thread pkg/storage/cache.go Outdated
Comment thread pkg/storage/cache.go Outdated
Comment thread pkg/storage/local/local.go Outdated
Comment thread pkg/storage/local/local.go
Comment thread pkg/storage/storage.go Outdated
Comment thread pkg/storage/storage.go Outdated
Comment thread pkg/storage/storage_controller.go Outdated
@laurentiuNiculae
laurentiuNiculae force-pushed the refactor-storage branch 5 times, most recently from 22c0705 to 99006bb Compare May 23, 2023 16:04
Comment thread pkg/storage/storage.go
@andaaron

Copy link
Copy Markdown
Contributor

Besides my 2 remaining comments, the code looks good.

andaaron
andaaron previously approved these changes May 25, 2023

@andaaron andaaron left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -51,8 +51,9 @@ import (
"zotregistry.io/zot/pkg/log"
"zotregistry.io/zot/pkg/meta/repodb/repodbfactory"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we still use "factory"? why?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactoring pkg/meta is out of scope of this PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Laurentiu has a draft at: #1476

Signed-off-by: Laurentiu Niculae <niculae.laurentiu1@gmail.com>

@rchincha rchincha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@rchincha
rchincha merged commit a3f355c into project-zot:main May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants