diff --git a/pkg/cli/server/extensions_test.go b/pkg/cli/server/extensions_test.go index eab785ff9d..5cc6cfa0a3 100644 --- a/pkg/cli/server/extensions_test.go +++ b/pkg/cli/server/extensions_test.go @@ -1637,3 +1637,134 @@ func TestOverlappingSyncRetentionConfig(t *testing.T) { So(string(data), ShouldContainSubstring, "overlapping sync content\":{\"Prefix\":\"prod/*") }) } + +func TestSyncWithRemoteStorageConfig(t *testing.T) { + oldArgs := os.Args + + defer func() { os.Args = oldArgs }() + + Convey("Test verify sync with remote storage works if sync.tmpdir is provided", t, func(c C) { + tmpfile, err := os.CreateTemp("", "zot-test*.json") + So(err, ShouldBeNil) + defer os.Remove(tmpfile.Name()) // clean up + + content := `{ + "distSpecVersion": "1.1.0-dev", + "storage": { + "rootDirectory": "%s", + "dedupe": false, + "remoteCache": false, + "storageDriver": { + "name": "s3", + "rootdirectory": "/zot", + "region": "us-east-2", + "regionendpoint": "localhost:4566", + "bucket": "zot-storage", + "secure": false, + "skipverify": false + } + }, + "http": { + "address": "0.0.0.0", + "port": "%s" + }, + "log": { + "level": "debug", + "output": "%s" + }, + "extensions": { + "sync": { + "tmpDir": "/tmp/sync", + "registries": [ + { + "urls": [ + "http://localhost:9000" + ], + "onDemand": true, + "tlsVerify": false, + "content": [ + { + "prefix": "**" + } + ] + } + ] + } + } + }` + + logPath, err := runCLIWithConfig(t.TempDir(), content) + So(err, ShouldBeNil) + + data, err := os.ReadFile(logPath) + So(err, ShouldBeNil) + defer os.Remove(logPath) // clean up + So(string(data), ShouldNotContainSubstring, + "using both sync and remote storage features needs config.Extensions.Sync.TmpDir to be specified") + }) + + Convey("Test verify sync with remote storage panics if sync.tmpdir is not provided", t, func(c C) { + port := GetFreePort() + logFile, err := os.CreateTemp("", "zot-log*.txt") + So(err, ShouldBeNil) + defer os.Remove(logFile.Name()) // clean up + + tmpfile, err := os.CreateTemp("", "zot-test*.json") + So(err, ShouldBeNil) + defer os.Remove(tmpfile.Name()) // clean up + content := fmt.Sprintf(`{ + "distSpecVersion": "1.1.0-dev", + "storage": { + "rootDirectory": "%s", + "dedupe": false, + "remoteCache": false, + "storageDriver": { + "name": "s3", + "rootdirectory": "/zot", + "region": "us-east-2", + "regionendpoint": "localhost:4566", + "bucket": "zot-storage", + "secure": false, + "skipverify": false + } + }, + "http": { + "address": "0.0.0.0", + "port": "%s" + }, + "log": { + "level": "debug", + "output": "%s" + }, + "extensions": { + "sync": { + "registries": [ + { + "urls": [ + "http://localhost:9000" + ], + "onDemand": true, + "tlsVerify": false, + "content": [ + { + "prefix": "**" + } + ] + } + ] + } + } + }`, t.TempDir(), port, logFile.Name()) + + err = os.WriteFile(tmpfile.Name(), []byte(content), 0o0600) + So(err, ShouldBeNil) + + So(func() { _ = cli.NewServerRootCmd().Execute() }, ShouldPanic) + + data, err := os.ReadFile(logFile.Name()) + So(err, ShouldBeNil) + defer os.Remove(logFile.Name()) // clean up + So(string(data), ShouldContainSubstring, + "using both sync and remote storage features needs config.Extensions.Sync.TmpDir to be specified") + }) +} diff --git a/pkg/cli/server/root.go b/pkg/cli/server/root.go index 1f5b3682a7..c129d5ddc0 100644 --- a/pkg/cli/server/root.go +++ b/pkg/cli/server/root.go @@ -381,6 +381,13 @@ func validateConfiguration(config *config.Config, log zlog.Logger) error { return zerr.ErrBadConfig } + // enforce tmpDir in case sync + s3 + if config.Extensions != nil && config.Extensions.Sync != nil && config.Extensions.Sync.TmpDir == "" { + log.Error().Err(zerr.ErrBadConfig). + Msg("using both sync and remote storage features needs config.Extensions.Sync.TmpDir to be specified") + + return zerr.ErrBadConfig + } } // enforce s3 driver on subpaths in case of using storage driver @@ -396,6 +403,14 @@ func validateConfiguration(config *config.Config, log zlog.Logger) error { return zerr.ErrBadConfig } + + // enforce tmpDir in case sync + s3 + if config.Extensions != nil && config.Extensions.Sync != nil && config.Extensions.Sync.TmpDir == "" { + log.Error().Err(zerr.ErrBadConfig). + Msg("using both sync and remote storage features needs config.Extensions.Sync.TmpDir to be specified") + + return zerr.ErrBadConfig + } } } } diff --git a/pkg/extensions/extension_sync.go b/pkg/extensions/extension_sync.go index a8f9c16f9f..f72028fa40 100644 --- a/pkg/extensions/extension_sync.go +++ b/pkg/extensions/extension_sync.go @@ -6,7 +6,6 @@ package extensions import ( "net" "net/url" - "os" "strings" zerr "zotregistry.io/zot/errors" @@ -47,13 +46,9 @@ func EnableSyncExtension(config *config.Config, metaDB mTypes.MetaDB, } tmpDir := config.Extensions.Sync.TmpDir - if tmpDir == "" { - // use an os tmpdir as tmpdir if not set - tmpDir = os.TempDir() - } + credsPath := config.Extensions.Sync.CredentialsFile - service, err := sync.New(registryConfig, config.Extensions.Sync.CredentialsFile, tmpDir, - storeController, metaDB, log) + service, err := sync.New(registryConfig, credsPath, tmpDir, storeController, metaDB, log) if err != nil { return nil, err } diff --git a/pkg/extensions/sync/destination.go b/pkg/extensions/sync/destination.go index 36483d4cf1..5150bc7576 100644 --- a/pkg/extensions/sync/destination.go +++ b/pkg/extensions/sync/destination.go @@ -37,22 +37,18 @@ type DestinationRegistry struct { } func NewDestinationRegistry( - storeController storage.StoreController, - tmpStorage OciLayoutStorage, + storeController storage.StoreController, // local store controller + tempStoreController storage.StoreController, // temp store controller metaDB mTypes.MetaDB, log log.Logger, ) Destination { - if tmpStorage == nil { - // to allow passing nil we can do this, noting that it will only work for a local StoreController - tmpStorage = NewOciLayoutStorage(storeController) - } return &DestinationRegistry{ storeController: storeController, + tempStorage: NewOciLayoutStorage(tempStoreController), metaDB: metaDB, // first we sync from remote (using containers/image copy from docker:// to oci:) to a temp imageStore // then we copy the image from tempStorage to zot's storage using ImageStore APIs - tempStorage: tmpStorage, - log: log, + log: log, } } @@ -288,9 +284,11 @@ func getImageStoreFromImageReference(imageReference types.ImageReference, repo, tempRootDir = strings.ReplaceAll(imageReference.StringWithinTransport(), fmt.Sprintf("%s:", repo), "") } - metrics := monitoring.NewMetricsServer(false, log.Logger{}) + return getImageStore(tempRootDir) +} - tempImageStore := local.NewImageStore(tempRootDir, false, false, log.Logger{}, metrics, nil, nil) +func getImageStore(rootDir string) storageTypes.ImageStore { + metrics := monitoring.NewMetricsServer(false, log.Logger{}) - return tempImageStore + return local.NewImageStore(rootDir, false, false, log.Logger{}, metrics, nil, nil) } diff --git a/pkg/extensions/sync/service.go b/pkg/extensions/sync/service.go index 594044df8e..02d6cc9d5f 100644 --- a/pkg/extensions/sync/service.go +++ b/pkg/extensions/sync/service.go @@ -20,7 +20,6 @@ import ( "zotregistry.io/zot/pkg/log" mTypes "zotregistry.io/zot/pkg/meta/types" "zotregistry.io/zot/pkg/storage" - "zotregistry.io/zot/pkg/storage/local" ) type BaseService struct { @@ -67,13 +66,20 @@ func New( service.contentManager = NewContentManager(opts.Content, log) - tmpImageStore := local.NewImageStore(tmpDir, - false, false, log, nil, nil, nil, - ) - - tmpStorage := NewOciLayoutStorage(storage.StoreController{DefaultStore: tmpImageStore}) - - service.destination = NewDestinationRegistry(storeController, tmpStorage, metadb, log) + if len(tmpDir) == 0 { + // first it will sync in tmpDir then it will move everything into local ImageStore + service.destination = NewDestinationRegistry(storeController, storeController, metadb, log) + } else { + // first it will sync under /rootDir/reponame/.sync/ then it will move everything into local ImageStore + service.destination = NewDestinationRegistry( + storeController, + storage.StoreController{ + DefaultStore: getImageStore(tmpDir), + }, + metadb, + log, + ) + } retryOptions := &retry.RetryOptions{} @@ -140,7 +146,7 @@ func (service *BaseService) SetNextAvailableClient() error { if err != nil { service.log.Error().Err(err).Str("url", url).Msg("sync: failed to initialize http client") - continue + return err } if !service.client.Ping() { @@ -355,7 +361,8 @@ func (service *BaseService) SyncRepo(ctx context.Context, repo string) error { return nil } -func (service *BaseService) syncTag(ctx context.Context, destinationRepo, remoteRepo, tag string) (digest.Digest, error) { +func (service *BaseService) syncTag(ctx context.Context, destinationRepo, remoteRepo, tag string, +) (digest.Digest, error) { copyOptions := getCopyOptions(service.remote.GetContext(), service.destination.GetContext()) policyContext, err := getPolicyContext(service.log) diff --git a/pkg/extensions/sync/sync_internal_test.go b/pkg/extensions/sync/sync_internal_test.go index 538318c0cd..50c572ca14 100644 --- a/pkg/extensions/sync/sync_internal_test.go +++ b/pkg/extensions/sync/sync_internal_test.go @@ -185,7 +185,8 @@ func TestDestinationRegistry(t *testing.T) { syncImgStore := local.NewImageStore(dir, true, true, log, metrics, nil, cacheDriver) repoName := "repo" - registry := NewDestinationRegistry(storage.StoreController{DefaultStore: syncImgStore}, nil, nil, log) + storeController := storage.StoreController{DefaultStore: syncImgStore} + registry := NewDestinationRegistry(storeController, storeController, nil, log) imageReference, err := registry.GetImageReference(repoName, "1.0") So(err, ShouldBeNil) So(imageReference, ShouldNotBeNil) @@ -302,7 +303,8 @@ func TestDestinationRegistry(t *testing.T) { syncImgStore := local.NewImageStore(dir, true, true, log, metrics, linter, cacheDriver) repoName := "repo" - registry := NewDestinationRegistry(storage.StoreController{DefaultStore: syncImgStore}, nil, nil, log) + storeController := storage.StoreController{DefaultStore: syncImgStore} + registry := NewDestinationRegistry(storeController, storeController, nil, log) err = registry.CommitImage(imageReference, repoName, "1.0") So(err, ShouldBeNil) @@ -336,7 +338,8 @@ func TestDestinationRegistry(t *testing.T) { }) Convey("trigger metaDB error on index manifest in CommitImage()", func() { - registry := NewDestinationRegistry(storage.StoreController{DefaultStore: syncImgStore}, nil, mocks.MetaDBMock{ + storeController := storage.StoreController{DefaultStore: syncImgStore} + registry := NewDestinationRegistry(storeController, storeController, mocks.MetaDBMock{ SetRepoReferenceFn: func(ctx context.Context, repo string, reference string, imageMeta mTypes.ImageMeta) error { if reference == "1.0" { return zerr.ErrRepoMetaNotFound @@ -351,7 +354,8 @@ func TestDestinationRegistry(t *testing.T) { }) Convey("trigger metaDB error on image manifest in CommitImage()", func() { - registry := NewDestinationRegistry(storage.StoreController{DefaultStore: syncImgStore}, nil, mocks.MetaDBMock{ + storeController := storage.StoreController{DefaultStore: syncImgStore} + registry := NewDestinationRegistry(storeController, storeController, mocks.MetaDBMock{ SetRepoReferenceFn: func(ctx context.Context, repo, reference string, imageMeta mTypes.ImageMeta) error { return zerr.ErrRepoMetaNotFound }, diff --git a/test/blackbox/ci.sh b/test/blackbox/ci.sh index 0859b69c88..63bc7d0367 100755 --- a/test/blackbox/ci.sh +++ b/test/blackbox/ci.sh @@ -9,7 +9,7 @@ PATH=$PATH:${SCRIPTPATH}/../../hack/tools/bin tests=("pushpull" "pushpull_authn" "delete_images" "referrers" "metadata" "anonymous_policy" "annotations" "detect_manifest_collision" "cve" "sync" "sync_docker" "sync_replica_cluster" - "scrub" "garbage_collect" "metrics" "metrics_minimal") + "sync_remote" "scrub" "garbage_collect" "metrics" "metrics_minimal") for test in ${tests[*]}; do ${BATS} ${BATS_FLAGS} ${SCRIPTPATH}/${test}.bats > ${test}.log & pids+=($!) diff --git a/test/blackbox/sync.bats b/test/blackbox/sync.bats index 36b14722e3..9542e61526 100644 --- a/test/blackbox/sync.bats +++ b/test/blackbox/sync.bats @@ -58,21 +58,33 @@ function setup_file() { { "distSpecVersion": "1.1.0-dev", "storage": { - "rootDirectory": "${zot_sync_per_root_dir}" + "rootDirectory": "${zot_sync_per_root_dir}", + "dedupe": false, + "remoteCache": false, + "storageDriver": { + "name": "s3", + "rootdirectory": "/zot", + "region": "us-east-2", + "regionendpoint": "localhost:4566", + "bucket": "zot-storage", + "secure": false, + "skipverify": false + } }, "http": { "address": "0.0.0.0", - "port": "${zot_port1}" + "port": "8081" }, "log": { "level": "debug" }, "extensions": { "sync": { + "tmpDir": "${zot_sync_per_root_dir}", "registries": [ { "urls": [ - "http://localhost:${zot_port3}" + "http://localhost:9000" ], "onDemand": false, "tlsVerify": false, @@ -94,20 +106,32 @@ EOF "distSpecVersion": "1.1.0-dev", "storage": { "rootDirectory": "${zot_sync_ondemand_root_dir}" + "dedupe": false, + "remoteCache": false, + "storageDriver": { + "name": "s3", + "rootdirectory": "/zot", + "region": "us-east-2", + "regionendpoint": "localhost:4566", + "bucket": "zot-storage", + "secure": false, + "skipverify": false + }, }, "http": { "address": "0.0.0.0", - "port": "${zot_port2}" + "port": "8082" }, "log": { "level": "debug" }, "extensions": { "sync": { + "tmpDir": "${zot_sync_per_root_dir}", "registries": [ { "urls": [ - "http://localhost:${zot_port3}" + "http://localhost:9000" ], "onDemand": true, "tlsVerify": false, diff --git a/test/blackbox/sync_remote.bats b/test/blackbox/sync_remote.bats index 6a1ee20261..b794db0f96 100644 --- a/test/blackbox/sync_remote.bats +++ b/test/blackbox/sync_remote.bats @@ -1,3 +1,564 @@ +# Note: Intended to be run as "make run-blackbox-tests" or "make run-blackbox-ci" +# Makefile target installs & checks all necessary tooling +# Extra tools that are not covered in Makefile target needs to be added in verify_prerequisites() + +load helpers_zot +load helpers_wait + + +function verify_prerequisites() { + if [ ! $(command -v curl) ]; then + echo "you need to install curl as a prerequisite to running the tests" >&3 + return 1 + fi + + if [ ! $(command -v jq) ]; then + echo "you need to install jq as a prerequisite to running the tests" >&3 + return 1 + fi + + return 0 +} + +function setup_file() { + export COSIGN_PASSWORD="" + export COSIGN_OCI_EXPERIMENTAL=1 + export COSIGN_EXPERIMENTAL=1 + + # Verify prerequisites are available + if ! $(verify_prerequisites); then + exit 1 + fi + + # Download test data to folder common for the entire suite, not just this file + skopeo --insecure-policy copy --format=oci docker://ghcr.io/project-zot/golang:1.20 oci:${TEST_DATA_DIR}/golang:1.20 + # Setup zot server + local zot_sync_per_root_dir=${BATS_FILE_TMPDIR}/zot-per + local zot_sync_ondemand_root_dir=${BATS_FILE_TMPDIR}/zot-ondemand + + local zot_sync_per_config_file=${BATS_FILE_TMPDIR}/zot_sync_per_config.json + local zot_sync_ondemand_config_file=${BATS_FILE_TMPDIR}/zot_sync_ondemand_config.json + + local zot_minimal_root_dir=${BATS_FILE_TMPDIR}/zot-minimal + local zot_minimal_config_file=${BATS_FILE_TMPDIR}/zot_minimal_config.json + + local oci_data_dir=${BATS_FILE_TMPDIR}/oci + mkdir -p ${zot_sync_per_root_dir} + mkdir -p ${zot_sync_ondemand_root_dir} + mkdir -p ${zot_minimal_root_dir} + mkdir -p ${oci_data_dir} + zot_port1=$(get_free_port) + echo ${zot_port1} > ${BATS_FILE_TMPDIR}/zot.port1 + zot_port2=$(get_free_port) + echo ${zot_port2} > ${BATS_FILE_TMPDIR}/zot.port2 + zot_port3=$(get_free_port) + echo ${zot_port3} > ${BATS_FILE_TMPDIR}/zot.port3 + + cat >${zot_sync_per_config_file} <${zot_sync_ondemand_config_file} <${zot_minimal_config_file} <${trust_policy_file} < config.json + echo "hello world" > artifact.txt + run oras push --plain-http 127.0.0.1:${zot_port3}/hello-artifact:v2 \ + --config config.json:application/vnd.acme.rocket.config.v1+json artifact.txt:text/plain -d -v + [ "$status" -eq 0 ] + rm -f artifact.txt + rm -f config.json +} + +@test "sync oras artifact periodically" { + zot_port1=`cat ${BATS_FILE_TMPDIR}/zot.port1` + # wait for oras artifact to be copied + run sleep 15s + run oras pull --plain-http 127.0.0.1:${zot_port1}/hello-artifact:v2 -d -v + [ "$status" -eq 0 ] + grep -q "hello world" artifact.txt + rm -f artifact.txt +} + +@test "sync oras artifact on demand" { + zot_port2=`cat ${BATS_FILE_TMPDIR}/zot.port2` + run oras pull --plain-http 127.0.0.1:${zot_port2}/hello-artifact:v2 -d -v + [ "$status" -eq 0 ] + grep -q "hello world" artifact.txt + rm -f artifact.txt +} + +# sync helm chart +@test "push helm chart" { + zot_port3=`cat ${BATS_FILE_TMPDIR}/zot.port3` + run helm package ${BATS_FILE_TMPDIR}/helm-charts/charts/zot -d ${BATS_FILE_TMPDIR} + [ "$status" -eq 0 ] + local chart_version=$(awk '/version/{printf $2}' ${BATS_FILE_TMPDIR}/helm-charts/charts/zot/Chart.yaml) + run helm push ${BATS_FILE_TMPDIR}/zot-${chart_version}.tgz oci://localhost:${zot_port3}/zot-chart + [ "$status" -eq 0 ] +} + +@test "sync helm chart periodically" { + zot_port1=`cat ${BATS_FILE_TMPDIR}/zot.port1` + # wait for helm chart to be copied + run sleep 15s + + local chart_version=$(awk '/version/{printf $2}' ${BATS_FILE_TMPDIR}/helm-charts/charts/zot/Chart.yaml) + run helm pull oci://localhost:${zot_port1}/zot-chart/zot --version ${chart_version} -d ${BATS_FILE_TMPDIR} + [ "$status" -eq 0 ] +} + +@test "sync helm chart on demand" { + zot_port2=`cat ${BATS_FILE_TMPDIR}/zot.port2` + local chart_version=$(awk '/version/{printf $2}' ${BATS_FILE_TMPDIR}/helm-charts/charts/zot/Chart.yaml) + run helm pull oci://localhost:${zot_port2}/zot-chart/zot --version ${chart_version} -d ${BATS_FILE_TMPDIR} + [ "$status" -eq 0 ] +} + +# sync OCI artifacts +@test "push OCI artifact (oci image mediatype) with regclient" { + zot_port1=`cat ${BATS_FILE_TMPDIR}/zot.port1` + zot_port2=`cat ${BATS_FILE_TMPDIR}/zot.port2` + zot_port3=`cat ${BATS_FILE_TMPDIR}/zot.port3` + run regctl registry set localhost:${zot_port3} --tls disabled + run regctl registry set localhost:${zot_port1} --tls disabled + run regctl registry set localhost:${zot_port2} --tls disabled + + run regctl artifact put localhost:${zot_port3}/artifact:demo <