From 8262c46ad727ee61a182932ef8618ac2dc5d0b3e Mon Sep 17 00:00:00 2001 From: Anders Bennedsgaard Date: Mon, 15 Jul 2024 19:30:43 +0200 Subject: [PATCH] Fix sync extension logging (#2537) * fix: nil pointer dereference on localimagestore fixes https://github.com/project-zot/zot/issues/2527 Signed-off-by: Anders Bennedsgaard * fix: no logging from sync extension imagestore Signed-off-by: Anders Bennedsgaard * feat: create local imagestore not found error Signed-off-by: Anders Bennedsgaard * fix: add test Signed-off-by: Anders Bennedsgaard --------- Signed-off-by: Anders Bennedsgaard --- errors/errors.go | 1 + pkg/extensions/sync/destination.go | 12 ++++++------ pkg/extensions/sync/oci_layout.go | 4 ++++ pkg/extensions/sync/service.go | 2 +- pkg/extensions/sync/sync_internal_test.go | 12 ++++++++++-- 5 files changed, 22 insertions(+), 9 deletions(-) diff --git a/errors/errors.go b/errors/errors.go index e5e63ea1..84bf769c 100644 --- 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 index 3eaee8ab..400a73a6 100644 --- 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 index 1798b11c..c44a955c 100644 --- 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 index 1e3f1b87..75bf56f7 100644 --- 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 index bb5368dc..ed79e59e 100644 --- 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