Fix sync extension logging (#2537)

* fix: nil pointer dereference on localimagestore fixes #2527 Signed-off-by: Anders Bennedsgaard <[email protected]> * fix: no logging from sync extension imagestore Signed-off-by: Anders Bennedsgaard <[email protected]> * feat: create local imagestore not found error Signed-off-by: Anders Bennedsgaard <[email protected]> * fix: add test Signed-off-by: Anders Bennedsgaard <[email protected]> --------- Signed-off-by: Anders Bennedsgaard <[email protected]>
project-zot · Jul 15, 2024 · 8262c46 · 8262c46
1 parent e5eacaa
commit 8262c46
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 9 deletions.
diff --git a/errors/errors.go b/errors/errors.go
@@ -87,6 +87,7 @@ var (
 	ErrDuplicateConfigName            = errors.New("cli config name already added")
 	ErrInvalidRoute                   = errors.New("invalid route prefix")
 	ErrImgStoreNotFound               = errors.New("image store not found corresponding to given route")
+	ErrLocalImgStoreNotFound          = errors.New("local image store not found corresponding to given route")
 	ErrEmptyValue                     = errors.New("empty cache value")
 	ErrEmptyRepoList                  = errors.New("no repository found")
 	ErrCVESearchDisabled              = errors.New("cve search is disabled")

diff --git a/pkg/extensions/sync/destination.go b/pkg/extensions/sync/destination.go
@@ -92,7 +92,7 @@ func (registry *DestinationRegistry) GetImageReference(repo, reference string) (
 func (registry *DestinationRegistry) CommitImage(imageReference types.ImageReference, repo, reference string) error {
 	imageStore := registry.storeController.GetImageStore(repo)
 
-	tempImageStore := getImageStoreFromImageReference(imageReference, repo, reference)
+	tempImageStore := getImageStoreFromImageReference(imageReference, repo, reference, registry.log)
 
 	defer os.RemoveAll(tempImageStore.RootDir())
 
@@ -282,11 +282,11 @@ func (registry *DestinationRegistry) copyBlob(repo string, blobDigest digest.Dig
 }
 
 // use only with local imageReferences.
-func getImageStoreFromImageReference(imageReference types.ImageReference, repo, reference string,
+func getImageStoreFromImageReference(imageReference types.ImageReference, repo, reference string, log log.Logger,
 ) storageTypes.ImageStore {
 	tmpRootDir := getTempRootDirFromImageReference(imageReference, repo, reference)
 
-	return getImageStore(tmpRootDir)
+	return getImageStore(tmpRootDir, log)
 }
 
 func getTempRootDirFromImageReference(imageReference types.ImageReference, repo, reference string) string {
@@ -301,8 +301,8 @@ func getTempRootDirFromImageReference(imageReference types.ImageReference, repo,
 	return tmpRootDir
 }
 
-func getImageStore(rootDir string) storageTypes.ImageStore {
-	metrics := monitoring.NewMetricsServer(false, log.Logger{})
+func getImageStore(rootDir string, log log.Logger) storageTypes.ImageStore {
+	metrics := monitoring.NewMetricsServer(false, log)
 
-	return local.NewImageStore(rootDir, false, false, log.Logger{}, metrics, nil, nil)
+	return local.NewImageStore(rootDir, false, false, log, metrics, nil, nil)
 }
diff --git a/pkg/extensions/sync/oci_layout.go b/pkg/extensions/sync/oci_layout.go
@@ -12,6 +12,7 @@ import (
 	"github.com/containers/image/v5/types"
 	"github.com/gofrs/uuid"
 
+	zerr "zotregistry.dev/zot/errors"
 	"zotregistry.dev/zot/pkg/extensions/sync/constants"
 	"zotregistry.dev/zot/pkg/storage"
 	storageConstants "zotregistry.dev/zot/pkg/storage/constants"
@@ -40,6 +41,9 @@ func (oci OciLayoutStorageImpl) GetContext() *types.SystemContext {
 
 func (oci OciLayoutStorageImpl) GetImageReference(repo string, reference string) (types.ImageReference, error) {
 	localImageStore := oci.storeController.GetImageStore(repo)
+	if localImageStore == nil {
+		return nil, zerr.ErrLocalImgStoreNotFound
+	}
 	tempSyncPath := path.Join(localImageStore.RootDir(), repo, constants.SyncBlobUploadDir)
 
 	// create session folder

diff --git a/pkg/extensions/sync/service.go b/pkg/extensions/sync/service.go
@@ -83,7 +83,7 @@ func New(
 		service.destination = NewDestinationRegistry(
 			storeController,
 			storage.StoreController{
-				DefaultStore: getImageStore(tmpDir),
+				DefaultStore: getImageStore(tmpDir, log),
 			},
 			metadb,
 			log,

diff --git a/pkg/extensions/sync/sync_internal_test.go b/pkg/extensions/sync/sync_internal_test.go
@@ -83,6 +83,14 @@ func TestInjectSyncUtils(t *testing.T) {
 	})
 }
 
+func TestNilDefaultStore(t *testing.T) {
+	Convey("Nil default store", t, func() {
+		ols := NewOciLayoutStorage(storage.StoreController{})
+		_, err := ols.GetImageReference(testImage, testImageTag)
+		So(err, ShouldEqual, zerr.ErrLocalImgStoreNotFound)
+	})
+}
+
 func TestSyncInternal(t *testing.T) {
 	Convey("Verify parseRepositoryReference func", t, func() {
 		repositoryReference := fmt.Sprintf("%s/%s", host, testImage)
@@ -214,7 +222,7 @@ func TestDestinationRegistry(t *testing.T) {
 		So(err, ShouldBeNil)
 		So(imageReference, ShouldNotBeNil)
 
-		imgStore := getImageStoreFromImageReference(imageReference, repoName, "1.0")
+		imgStore := getImageStoreFromImageReference(imageReference, repoName, "1.0", log)
 
 		// create a blob/layer
 		upload, err := imgStore.NewBlobUpload(repoName)
@@ -393,7 +401,7 @@ func TestDestinationRegistry(t *testing.T) {
 			So(err, ShouldBeNil)
 			So(imageReference, ShouldNotBeNil)
 
-			imgStore := getImageStoreFromImageReference(imageReference, repoName, "2.0")
+			imgStore := getImageStoreFromImageReference(imageReference, repoName, "2.0", log)
 
 			// upload image