From e74cd8f5a425c214faa1a5f3e7d8ef86705dd299 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Wed, 30 Apr 2025 14:33:21 +0100 Subject: [PATCH 1/8] feat(sync): Add ability to sync specific architectures --- examples/config-sync-architectures.json | 100 ++++++ pkg/extensions/config/sync/config.go | 3 +- pkg/extensions/sync/destination.go | 78 ++++- pkg/extensions/sync/service.go | 99 +++++- test/blackbox/sync_architectures.bats | 409 ++++++++++++++++++++++++ 5 files changed, 680 insertions(+), 9 deletions(-) create mode 100644 examples/config-sync-architectures.json create mode 100644 test/blackbox/sync_architectures.bats diff --git a/examples/config-sync-architectures.json b/examples/config-sync-architectures.json new file mode 100644 index 000000000..48880d5ec --- /dev/null +++ b/examples/config-sync-architectures.json @@ -0,0 +1,100 @@ +{ + "distSpecVersion": "1.1.1", + "storage": { + "rootDirectory": "/tmp/zot" + }, + "http": { + "address": "127.0.0.1", + "port": "8080" + }, + "log": { + "level": "debug" + }, + "extensions": { + "sync": { + "enable": true, + "credentialsFile": "./examples/sync-auth-filepath.json", + "registries": [ + { + "urls": [ + "https://registry1:5000" + ], + "onDemand": false, + "pollInterval": "6h", + "tlsVerify": true, + "certDir": "/home/user/certs", + "maxRetries": 3, + "retryDelay": "5m", + "onlySigned": true, + "architectures": ["amd64", "arm64"], + "content": [ + { + "prefix": "/repo1/repo", + "tags": { + "regex": "4.*", + "semver": true + } + }, + { + "prefix": "/repo2/repo", + "destination": "/repo", + "stripPrefix": true + }, + { + "prefix": "/repo3/**" + }, + { + "prefix": "/repo4/**", + "tags": { + "excludeRegex": ".*-(amd64|arm64)$" + } + } + ] + }, + { + "urls": [ + "https://registry2:5000", + "https://registry3:5000" + ], + "pollInterval": "12h", + "tlsVerify": false, + "onDemand": false, + "architectures": ["amd64"], + "content": [ + { + "prefix": "**", + "tags": { + "semver": true + } + } + ] + }, + { + "urls": [ + "https://index.docker.io" + ], + "onDemand": true, + "tlsVerify": true, + "maxRetries": 5, + "retryDelay": "30s", + "architectures": ["amd64", "arm64", "arm"] + }, + { + "urls": [ + "https://demo.goharbor.io" + ], + "pollInterval": "12h", + "content": [ + { + "prefix": "zot/**" + } + ], + "onDemand": true, + "tlsVerify": true, + "maxRetries": 5, + "retryDelay": "1m" + } + ] + } + } +} \ No newline at end of file diff --git a/pkg/extensions/config/sync/config.go b/pkg/extensions/config/sync/config.go index 775092877..eb8dd98d5 100644 --- a/pkg/extensions/config/sync/config.go +++ b/pkg/extensions/config/sync/config.go @@ -33,7 +33,8 @@ type RegistryConfig struct { RetryDelay *time.Duration OnlySigned *bool CredentialHelper string - PreserveDigest bool // sync without converting + PreserveDigest bool // sync without converting + Architectures []string // filter architectures during sync } type Content struct { diff --git a/pkg/extensions/sync/destination.go b/pkg/extensions/sync/destination.go index 160e47a66..5c6cdc1dd 100644 --- a/pkg/extensions/sync/destination.go +++ b/pkg/extensions/sync/destination.go @@ -19,6 +19,7 @@ import ( zerr "zotregistry.dev/zot/errors" "zotregistry.dev/zot/pkg/common" + syncconf "zotregistry.dev/zot/pkg/extensions/config/sync" "zotregistry.dev/zot/pkg/extensions/monitoring" "zotregistry.dev/zot/pkg/log" "zotregistry.dev/zot/pkg/meta" @@ -34,6 +35,7 @@ type DestinationRegistry struct { tempStorage OciLayoutStorage metaDB mTypes.MetaDB log log.Logger + config *syncconf.RegistryConfig // Config used for filtering architectures } func NewDestinationRegistry( @@ -41,14 +43,21 @@ func NewDestinationRegistry( tempStoreController storage.StoreController, // temp store controller metaDB mTypes.MetaDB, log log.Logger, + config ...*syncconf.RegistryConfig, // optional config for filtering ) Destination { + var cfg *syncconf.RegistryConfig + if len(config) > 0 { + cfg = config[0] + } + 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 - log: log, + log: log, + config: cfg, } } @@ -227,7 +236,54 @@ func (registry *DestinationRegistry) copyManifest(repo string, desc ispec.Descri return err } - for _, manifest := range indexManifest.Manifests { + // Filter manifests based on architectures if configured + var filteredManifests []ispec.Descriptor + if registry.config != nil && len(registry.config.Architectures) > 0 { + registry.log.Info(). + Strs("architectures", registry.config.Architectures). + Str("repository", repo). + Str("reference", reference). + Msg("filtering manifest list by architectures") + + for _, manifest := range indexManifest.Manifests { + if manifest.Platform != nil && manifest.Platform.Architecture != "" { + // Check if this architecture should be included + shouldInclude := false + for _, configArch := range registry.config.Architectures { + if manifest.Platform.Architecture == configArch { + shouldInclude = true + break + } + } + + if shouldInclude { + filteredManifests = append(filteredManifests, manifest) + } else { + registry.log.Info(). + Str("repository", repo). + Str("architecture", manifest.Platform.Architecture). + Msg("skipping architecture during sync") + } + } else { + // No platform info, include the manifest + filteredManifests = append(filteredManifests, manifest) + } + } + + // If we have no filtered manifests but had original ones, warn + if len(filteredManifests) == 0 && len(indexManifest.Manifests) > 0 { + registry.log.Warn(). + Str("repository", repo). + Str("reference", reference). + Msg("no architecture matched the configured filters, manifest list might be empty") + } + } else { + // No filtering, use all manifests + filteredManifests = indexManifest.Manifests + } + + // Process the filtered manifests + for _, manifest := range filteredManifests { reference := GetDescriptorReference(manifest) manifestBuf, err := tempImageStore.GetBlobContent(repo, manifest.Digest) @@ -254,6 +310,24 @@ func (registry *DestinationRegistry) copyManifest(repo string, desc ispec.Descri } } + // If we've filtered the manifest list, we need to update it + if registry.config != nil && len(registry.config.Architectures) > 0 && + len(filteredManifests) != len(indexManifest.Manifests) && len(filteredManifests) > 0 { + // Create a new index with the filtered manifests + indexManifest.Manifests = filteredManifests + + // Update the manifest content with the filtered list + updatedContent, err := json.Marshal(indexManifest) + if err != nil { + registry.log.Error().Str("errorType", common.TypeOf(err)). + Err(err).Str("repository", repo). + Msg("failed to marshal updated index manifest") + return err + } + + manifestContent = updatedContent + } + _, _, err := imageStore.PutImageManifest(repo, reference, desc.MediaType, manifestContent) if err != nil { registry.log.Error().Str("errorType", common.TypeOf(err)).Str("repo", repo).Str("reference", reference). diff --git a/pkg/extensions/sync/service.go b/pkg/extensions/sync/service.go index 83ac73339..6750b3176 100644 --- a/pkg/extensions/sync/service.go +++ b/pkg/extensions/sync/service.go @@ -18,6 +18,7 @@ import ( "github.com/regclient/regclient/config" "github.com/regclient/regclient/mod" "github.com/regclient/regclient/scheme/reg" + "github.com/regclient/regclient/types/manifest" "github.com/regclient/regclient/types/ref" zerr "zotregistry.dev/zot/errors" @@ -115,7 +116,7 @@ func New( 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) + service.destination = NewDestinationRegistry(storeController, storeController, metadb, log, &service.config) } else { // first it will sync under /rootDir/reponame/.sync/ then it will move everything into local ImageStore service.destination = NewDestinationRegistry( @@ -125,6 +126,7 @@ func New( }, metadb, log, + &service.config, ) } @@ -447,6 +449,24 @@ func (service *BaseService) SyncRepo(ctx context.Context, repo string) error { return nil } +// shouldIncludeArchitecture determines if an architecture should be included in the sync +// based on the configured architectures (if any) +func (service *BaseService) shouldIncludeArchitecture(arch string) bool { + // If no architectures are configured, include all architectures + if len(service.config.Architectures) == 0 { + return true + } + + // Check if the architecture is in the configured list + for _, configArch := range service.config.Architectures { + if configArch == arch { + return true + } + } + + return false +} + func (service *BaseService) syncRef(ctx context.Context, localRepo string, remoteImageRef, localImageRef ref.Ref, remoteDigest godigest.Digest, recursive bool, ) error { @@ -467,6 +487,18 @@ func (service *BaseService) syncRef(ctx context.Context, localRepo string, remot copyOpts = append(copyOpts, regclient.ImageWithReferrers()) } + // Architecture filtering - use ImageWithChildren to handle manifest lists + // ImageWithChildren ensures we get the complete manifest list, then we can filter architectures + copyOpts = append(copyOpts, regclient.ImageWithChildren()) + + // Add platform filter option if architectures are configured + if len(service.config.Architectures) > 0 { + service.log.Info(). + Strs("architectures", service.config.Architectures). + Str("image", remoteImageRef.CommonName()). + Msg("filtering architectures during sync") + } + // check if image is already synced skipImage, err = service.destination.CanSkipImage(localRepo, reference, remoteDigest) if err != nil { @@ -479,11 +511,66 @@ func (service *BaseService) syncRef(ctx context.Context, localRepo string, remot service.log.Info().Str("remote image", remoteImageRef.CommonName()). Str("local image", fmt.Sprintf("%s:%s", localRepo, remoteImageRef.Tag)).Msg("syncing image") - err = service.rc.ImageCopy(ctx, remoteImageRef, localImageRef, copyOpts...) - if err != nil { - service.log.Error().Err(err).Str("errortype", common.TypeOf(err)). - Str("remote image", remoteImageRef.CommonName()). - Str("local image", fmt.Sprintf("%s:%s", localRepo, remoteImageRef.Tag)).Msg("failed to sync image") + // When architectures are specified, we need to filter the manifest list + if len(service.config.Architectures) > 0 { + // Get the manifest to check if it's a manifest list + man, err := service.rc.ManifestGet(ctx, remoteImageRef) + if err != nil { + service.log.Error().Err(err).Str("errortype", common.TypeOf(err)). + Str("remote image", remoteImageRef.CommonName()). + Msg("failed to get manifest for architecture filtering") + return err + } + + // If it's a manifest list (multi-arch image), we need to filter architectures + _, isIndexer := man.(manifest.Indexer) + if isIndexer { + service.log.Info(). + Str("remote image", remoteImageRef.CommonName()). + Msg("filtering architectures for multi-arch image") + + // Use ImageCopy with the architecture filtering options + err = service.rc.ImageCopy(ctx, remoteImageRef, localImageRef, copyOpts...) + if err != nil { + service.log.Error().Err(err).Str("errortype", common.TypeOf(err)). + Str("remote image", remoteImageRef.CommonName()). + Str("local image", fmt.Sprintf("%s:%s", localRepo, remoteImageRef.Tag)). + Msg("failed to sync image") + } + + // The architecture filtering will be applied during processing before the commit stage + // The actual filtering happens in the destination.CommitAll method + } else { + // It's a single-arch image, just verify if we should include this architecture + if man.GetDescriptor().Platform != nil { + arch := man.GetDescriptor().Platform.Architecture + if !service.shouldIncludeArchitecture(arch) { + service.log.Info(). + Str("remote image", remoteImageRef.CommonName()). + Str("architecture", arch). + Msg("skipping image with excluded architecture") + return nil + } + } + + // Single architecture image that should be included + err = service.rc.ImageCopy(ctx, remoteImageRef, localImageRef, copyOpts...) + if err != nil { + service.log.Error().Err(err).Str("errortype", common.TypeOf(err)). + Str("remote image", remoteImageRef.CommonName()). + Str("local image", fmt.Sprintf("%s:%s", localRepo, remoteImageRef.Tag)). + Msg("failed to sync image") + } + } + } else { + // No architecture filtering, standard behavior + err = service.rc.ImageCopy(ctx, remoteImageRef, localImageRef, copyOpts...) + if err != nil { + service.log.Error().Err(err).Str("errortype", common.TypeOf(err)). + Str("remote image", remoteImageRef.CommonName()). + Str("local image", fmt.Sprintf("%s:%s", localRepo, remoteImageRef.Tag)). + Msg("failed to sync image") + } } return err diff --git a/test/blackbox/sync_architectures.bats b/test/blackbox/sync_architectures.bats new file mode 100644 index 000000000..63b715ad9 --- /dev/null +++ b/test/blackbox/sync_architectures.bats @@ -0,0 +1,409 @@ +# Test suite for architecture filtering during sync operations +# Verifies the behavior of the Architectures field in RegistryConfig + +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 + + if [ ! $(command -v skopeo) ]; then + echo "you need to install skopeo as a prerequisite to running the tests" >&3 + return 1 + fi + + return 0 +} + +function setup_file() { + # Verify prerequisites are available + if ! $(verify_prerequisites); then + exit 1 + fi + + # Create directories for the different zot instances + local zot_source_dir=${BATS_FILE_TMPDIR}/zot-source + local zot_arch_filter_dir=${BATS_FILE_TMPDIR}/zot-arch-filter + local zot_no_filter_dir=${BATS_FILE_TMPDIR}/zot-no-filter + + local zot_source_config=${BATS_FILE_TMPDIR}/zot_source_config.json + local zot_arch_filter_config=${BATS_FILE_TMPDIR}/zot_arch_filter_config.json + local zot_no_filter_config=${BATS_FILE_TMPDIR}/zot_no_filter_config.json + + mkdir -p ${zot_source_dir} + mkdir -p ${zot_arch_filter_dir} + mkdir -p ${zot_no_filter_dir} + + # Get free ports for our zot instances + zot_source_port=$(get_free_port) + echo ${zot_source_port} > ${BATS_FILE_TMPDIR}/zot.source.port + + zot_arch_filter_port=$(get_free_port) + echo ${zot_arch_filter_port} > ${BATS_FILE_TMPDIR}/zot.arch_filter.port + + zot_no_filter_port=$(get_free_port) + echo ${zot_no_filter_port} > ${BATS_FILE_TMPDIR}/zot.no_filter.port + + # Create config for source registry (basic config without sync) + cat >${zot_source_config} <${zot_arch_filter_config} <${zot_no_filter_config} < ${BATS_FILE_TMPDIR}/zot.multi_arch_filter.port + + # Create config for multi-architecture filtered registry (with amd64 and arm64) + cat >${zot_multi_arch_filter_config} < ${BATS_FILE_TMPDIR}/zot.ondemand_filter.port + + # Create config for on-demand architecture filtered registry (with amd64 only) + cat >${zot_ondemand_filter_config} < Date: Wed, 30 Apr 2025 22:05:25 +0100 Subject: [PATCH 2/8] feat(sync): Add support for platform/architecture --- examples/config-sync-architectures.json | 16 +- examples/config-sync-platforms.json | 109 ++++++++++ pkg/extensions/config/sync/config.go | 3 +- pkg/extensions/sync/destination.go | 110 ++++++++-- pkg/extensions/sync/service.go | 106 ++++++++-- test/blackbox/sync_architectures.bats | 254 +++++++++++++++++++++++- 6 files changed, 552 insertions(+), 46 deletions(-) create mode 100644 examples/config-sync-platforms.json diff --git a/examples/config-sync-architectures.json b/examples/config-sync-architectures.json index 48880d5ec..6f1bd1ead 100644 --- a/examples/config-sync-architectures.json +++ b/examples/config-sync-architectures.json @@ -27,6 +27,7 @@ "retryDelay": "5m", "onlySigned": true, "architectures": ["amd64", "arm64"], + "platforms": ["linux/amd64", "linux/arm64"], "content": [ { "prefix": "/repo1/repo", @@ -60,6 +61,7 @@ "tlsVerify": false, "onDemand": false, "architectures": ["amd64"], + "platforms": ["linux/amd64", "windows/amd64"], "content": [ { "prefix": "**", @@ -77,7 +79,8 @@ "tlsVerify": true, "maxRetries": 5, "retryDelay": "30s", - "architectures": ["amd64", "arm64", "arm"] + "architectures": ["amd64", "arm64", "arm"], + "platforms": ["linux/amd64", "linux/arm64", "linux/arm"] }, { "urls": [ @@ -92,7 +95,16 @@ "onDemand": true, "tlsVerify": true, "maxRetries": 5, - "retryDelay": "1m" + "retryDelay": "1m", + "platforms": ["darwin/amd64", "linux/amd64", "linux/arm64"] + }, + { + "urls": [ + "https://registry5:5000" + ], + "onDemand": false, + "tlsVerify": true, + "platforms": ["linux/amd64", "amd64", "arm64"] } ] } diff --git a/examples/config-sync-platforms.json b/examples/config-sync-platforms.json new file mode 100644 index 000000000..9980cd07a --- /dev/null +++ b/examples/config-sync-platforms.json @@ -0,0 +1,109 @@ +{ + "distSpecVersion": "1.1.1", + "storage": { + "rootDirectory": "/tmp/zot" + }, + "http": { + "address": "127.0.0.1", + "port": "8080" + }, + "log": { + "level": "debug" + }, + "extensions": { + "sync": { + "enable": true, + "credentialsFile": "./examples/sync-auth-filepath.json", + "registries": [ + { + "urls": [ + "https://registry1:5000" + ], + "onDemand": false, + "pollInterval": "6h", + "tlsVerify": true, + "certDir": "/home/user/certs", + "maxRetries": 3, + "retryDelay": "5m", + "onlySigned": true, + "platforms": ["linux/amd64", "linux/arm64", "windows/amd64"], + "content": [ + { + "prefix": "/repo1/repo", + "tags": { + "regex": "4.*", + "semver": true + } + }, + { + "prefix": "/repo2/repo", + "destination": "/repo", + "stripPrefix": true + }, + { + "prefix": "/repo3/**" + }, + { + "prefix": "/repo4/**", + "tags": { + "excludeRegex": ".*-(amd64|arm64)$" + } + } + ] + }, + { + "urls": [ + "https://registry2:5000", + "https://registry3:5000" + ], + "pollInterval": "12h", + "tlsVerify": false, + "onDemand": false, + "platforms": ["linux/amd64"], + "content": [ + { + "prefix": "**", + "tags": { + "semver": true + } + } + ] + }, + { + "urls": [ + "https://index.docker.io" + ], + "onDemand": true, + "tlsVerify": true, + "maxRetries": 5, + "retryDelay": "30s", + "architectures": ["amd64", "arm64", "arm"] + }, + { + "urls": [ + "https://demo.goharbor.io" + ], + "pollInterval": "12h", + "content": [ + { + "prefix": "zot/**" + } + ], + "onDemand": true, + "tlsVerify": true, + "maxRetries": 5, + "retryDelay": "1m", + "platforms": ["darwin/amd64", "linux/amd64", "linux/arm64"] + }, + { + "urls": [ + "https://registry4:5000" + ], + "onDemand": false, + "tlsVerify": true, + "platforms": ["linux/amd64", "amd64", "arm64"] + } + ] + } + } +} \ No newline at end of file diff --git a/pkg/extensions/config/sync/config.go b/pkg/extensions/config/sync/config.go index eb8dd98d5..cc38df0bf 100644 --- a/pkg/extensions/config/sync/config.go +++ b/pkg/extensions/config/sync/config.go @@ -34,7 +34,8 @@ type RegistryConfig struct { OnlySigned *bool CredentialHelper string PreserveDigest bool // sync without converting - Architectures []string // filter architectures during sync + Architectures []string `mapstructure:",omitempty"` // filter architectures during sync (DEPRECATED: use Platforms instead) + Platforms []string `mapstructure:",omitempty"` // filter platforms during sync (supports both "arch" and "os/arch" formats) } type Content struct { diff --git a/pkg/extensions/sync/destination.go b/pkg/extensions/sync/destination.go index 5c6cdc1dd..e49764e0c 100644 --- a/pkg/extensions/sync/destination.go +++ b/pkg/extensions/sync/destination.go @@ -30,6 +30,58 @@ import ( storageTypes "zotregistry.dev/zot/pkg/storage/types" ) +// Platform represents an OS/architecture combination +type Platform struct { + OS string + Architecture string +} + +// ParsePlatform parses a platform string into a Platform struct +// The string can be in the format "os/arch" or just "arch" +func ParsePlatform(platform string) Platform { + parts := strings.Split(platform, "/") + if len(parts) == 2 { + return Platform{ + OS: parts[0], + Architecture: parts[1], + } + } + // If only one part, assume it's the architecture + return Platform{ + OS: "", + Architecture: platform, + } +} + +// MatchesPlatform checks if the given platform matches any of the platform specifications +// Platform specs can be in format "os/arch" or just "arch" +func MatchesPlatform(platform *ispec.Platform, platformSpecs []string) bool { + if platform == nil || len(platformSpecs) == 0 { + return true + } + + for _, spec := range platformSpecs { + specPlatform := ParsePlatform(spec) + + // Check if architecture matches + if specPlatform.Architecture != "" && + specPlatform.Architecture != platform.Architecture { + continue + } + + // Check if OS matches (if specified) + if specPlatform.OS != "" && + specPlatform.OS != platform.OS { + continue + } + + // If we got here, it's a match + return true + } + + return false +} + type DestinationRegistry struct { storeController storage.StoreController tempStorage OciLayoutStorage @@ -236,33 +288,46 @@ func (registry *DestinationRegistry) copyManifest(repo string, desc ispec.Descri return err } - // Filter manifests based on architectures if configured + // Filter manifests based on platforms/architectures if configured var filteredManifests []ispec.Descriptor - if registry.config != nil && len(registry.config.Architectures) > 0 { - registry.log.Info(). - Strs("architectures", registry.config.Architectures). - Str("repository", repo). - Str("reference", reference). - Msg("filtering manifest list by architectures") - for _, manifest := range indexManifest.Manifests { - if manifest.Platform != nil && manifest.Platform.Architecture != "" { - // Check if this architecture should be included - shouldInclude := false - for _, configArch := range registry.config.Architectures { - if manifest.Platform.Architecture == configArch { - shouldInclude = true - break - } - } + // Determine which platform specifications to use + var platformSpecs []string + if registry.config != nil { + if len(registry.config.Platforms) > 0 { + platformSpecs = registry.config.Platforms + registry.log.Info(). + Strs("platforms", registry.config.Platforms). + Str("repository", repo). + Str("reference", reference). + Msg("filtering manifest list by platforms") + } else if len(registry.config.Architectures) > 0 { + platformSpecs = registry.config.Architectures + registry.log.Info(). + Strs("architectures", registry.config.Architectures). + Str("repository", repo). + Str("reference", reference). + Msg("filtering manifest list by architectures (deprecated)") + } + } - if shouldInclude { + // Apply filtering if we have platform specifications + if len(platformSpecs) > 0 { + for _, manifest := range indexManifest.Manifests { + if manifest.Platform != nil { + // Check if this platform should be included + if MatchesPlatform(manifest.Platform, platformSpecs) { filteredManifests = append(filteredManifests, manifest) } else { + platformDesc := manifest.Platform.Architecture + if manifest.Platform.OS != "" { + platformDesc = manifest.Platform.OS + "/" + manifest.Platform.Architecture + } + registry.log.Info(). Str("repository", repo). - Str("architecture", manifest.Platform.Architecture). - Msg("skipping architecture during sync") + Str("platform", platformDesc). + Msg("skipping platform during sync") } } else { // No platform info, include the manifest @@ -275,7 +340,7 @@ func (registry *DestinationRegistry) copyManifest(repo string, desc ispec.Descri registry.log.Warn(). Str("repository", repo). Str("reference", reference). - Msg("no architecture matched the configured filters, manifest list might be empty") + Msg("no platform matched the configured filters, manifest list might be empty") } } else { // No filtering, use all manifests @@ -311,7 +376,8 @@ func (registry *DestinationRegistry) copyManifest(repo string, desc ispec.Descri } // If we've filtered the manifest list, we need to update it - if registry.config != nil && len(registry.config.Architectures) > 0 && + if registry.config != nil && + (len(registry.config.Architectures) > 0 || len(registry.config.Platforms) > 0) && len(filteredManifests) != len(indexManifest.Manifests) && len(filteredManifests) > 0 { // Create a new index with the filtered manifests indexManifest.Manifests = filteredManifests diff --git a/pkg/extensions/sync/service.go b/pkg/extensions/sync/service.go index 6750b3176..5a3a2c548 100644 --- a/pkg/extensions/sync/service.go +++ b/pkg/extensions/sync/service.go @@ -14,6 +14,7 @@ import ( "time" godigest "github.com/opencontainers/go-digest" + ispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/regclient/regclient" "github.com/regclient/regclient/config" "github.com/regclient/regclient/mod" @@ -449,22 +450,83 @@ func (service *BaseService) SyncRepo(ctx context.Context, repo string) error { return nil } +// shouldIncludePlatform determines if a platform should be included in the sync +// based on the configured platforms or architectures +func (service *BaseService) shouldIncludePlatform(platform *ispec.Platform) bool { + // Get platform specifications from the configuration + var platformSpecs []string + + // Check if we have platform specifications + if len(service.config.Platforms) > 0 { + platformSpecs = service.config.Platforms + } else if len(service.config.Architectures) > 0 { + // Fall back to architectures for backward compatibility + platformSpecs = service.config.Architectures + } else { + // If no platforms or architectures are configured, include all + return true + } + + // If platform is nil, include it + if platform == nil || len(platformSpecs) == 0 { + return true + } + + for _, spec := range platformSpecs { + parts := strings.Split(spec, "/") + if len(parts) == 2 { + // OS/Arch format + specOS := parts[0] + specArch := parts[1] + + // Check if OS matches (if specified) + if specOS != "" && specOS != platform.OS { + continue + } + + // Check if architecture matches + if specArch != "" && specArch != platform.Architecture { + continue + } + } else if len(parts) == 1 { + // Arch only format + if parts[0] != platform.Architecture { + continue + } + } + + // If we got here, it's a match + return true + } + + return false +} + // shouldIncludeArchitecture determines if an architecture should be included in the sync // based on the configured architectures (if any) +// DEPRECATED: Use shouldIncludePlatform instead func (service *BaseService) shouldIncludeArchitecture(arch string) bool { // If no architectures are configured, include all architectures - if len(service.config.Architectures) == 0 { - return true + if len(service.config.Architectures) > 0 && len(service.config.Platforms) == 0 { + // Check if the architecture is in the configured list + for _, configArch := range service.config.Architectures { + if configArch == arch { + return true + } + } + return false } - // Check if the architecture is in the configured list - for _, configArch := range service.config.Architectures { - if configArch == arch { - return true + // When using the Platforms field, we need a different check + if len(service.config.Platforms) > 0 { + platform := &ispec.Platform{ + Architecture: arch, } + return service.shouldIncludePlatform(platform) } - return false + // No filters configured, include all + return true } func (service *BaseService) syncRef(ctx context.Context, localRepo string, remoteImageRef, localImageRef ref.Ref, @@ -487,16 +549,21 @@ func (service *BaseService) syncRef(ctx context.Context, localRepo string, remot copyOpts = append(copyOpts, regclient.ImageWithReferrers()) } - // Architecture filtering - use ImageWithChildren to handle manifest lists - // ImageWithChildren ensures we get the complete manifest list, then we can filter architectures + // Platform/Architecture filtering - use ImageWithChildren to handle manifest lists + // ImageWithChildren ensures we get the complete manifest list, then we can filter platforms copyOpts = append(copyOpts, regclient.ImageWithChildren()) - // Add platform filter option if architectures are configured - if len(service.config.Architectures) > 0 { + // Log platform filtering information + if len(service.config.Platforms) > 0 { + service.log.Info(). + Strs("platforms", service.config.Platforms). + Str("image", remoteImageRef.CommonName()). + Msg("filtering platforms during sync") + } else if len(service.config.Architectures) > 0 { service.log.Info(). Strs("architectures", service.config.Architectures). Str("image", remoteImageRef.CommonName()). - Msg("filtering architectures during sync") + Msg("filtering architectures during sync (deprecated)") } // check if image is already synced @@ -541,14 +608,19 @@ func (service *BaseService) syncRef(ctx context.Context, localRepo string, remot // The architecture filtering will be applied during processing before the commit stage // The actual filtering happens in the destination.CommitAll method } else { - // It's a single-arch image, just verify if we should include this architecture + // It's a single-arch image, verify if we should include this platform if man.GetDescriptor().Platform != nil { - arch := man.GetDescriptor().Platform.Architecture - if !service.shouldIncludeArchitecture(arch) { + platform := man.GetDescriptor().Platform + if !service.shouldIncludePlatform(platform) { + platformDesc := platform.Architecture + if platform.OS != "" { + platformDesc = platform.OS + "/" + platform.Architecture + } + service.log.Info(). Str("remote image", remoteImageRef.CommonName()). - Str("architecture", arch). - Msg("skipping image with excluded architecture") + Str("platform", platformDesc). + Msg("skipping image with excluded platform") return nil } } diff --git a/test/blackbox/sync_architectures.bats b/test/blackbox/sync_architectures.bats index 63b715ad9..dcd31edda 100644 --- a/test/blackbox/sync_architectures.bats +++ b/test/blackbox/sync_architectures.bats @@ -1,6 +1,6 @@ -# Test suite for architecture filtering during sync operations -# Verifies the behavior of the Architectures field in RegistryConfig - +# Test suite for platform filtering during sync operations +# Verifies the behavior of the Architectures and Platforms fields in RegistryConfig +# Platforms supports both architecture-only ("amd64") and OS/architecture ("linux/amd64") formats load helpers_zot load helpers_wait @@ -33,14 +33,17 @@ function setup_file() { local zot_source_dir=${BATS_FILE_TMPDIR}/zot-source local zot_arch_filter_dir=${BATS_FILE_TMPDIR}/zot-arch-filter local zot_no_filter_dir=${BATS_FILE_TMPDIR}/zot-no-filter + local zot_platform_filter_dir=${BATS_FILE_TMPDIR}/zot-platform-filter local zot_source_config=${BATS_FILE_TMPDIR}/zot_source_config.json local zot_arch_filter_config=${BATS_FILE_TMPDIR}/zot_arch_filter_config.json local zot_no_filter_config=${BATS_FILE_TMPDIR}/zot_no_filter_config.json + local zot_platform_filter_config=${BATS_FILE_TMPDIR}/zot_platform_filter_config.json mkdir -p ${zot_source_dir} mkdir -p ${zot_arch_filter_dir} mkdir -p ${zot_no_filter_dir} + mkdir -p ${zot_platform_filter_dir} # Get free ports for our zot instances zot_source_port=$(get_free_port) @@ -52,6 +55,9 @@ function setup_file() { zot_no_filter_port=$(get_free_port) echo ${zot_no_filter_port} > ${BATS_FILE_TMPDIR}/zot.no_filter.port + zot_platform_filter_port=$(get_free_port) + echo ${zot_platform_filter_port} > ${BATS_FILE_TMPDIR}/zot.platform_filter.port + # Create config for source registry (basic config without sync) cat >${zot_source_config} <${zot_platform_filter_config} < ${BATS_FILE_TMPDIR}/zot.mixed_filter.port + + # Create config for mixed filtered registry + cat >${zot_mixed_filter_config} < ${BATS_FILE_TMPDIR}/zot.os_filter.port + + # Create config for OS filtered registry + cat >${zot_os_filter_config} < Date: Wed, 30 Apr 2025 22:32:54 +0100 Subject: [PATCH 3/8] fix(build): Fix build issues --- pkg/extensions/sync/service.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/pkg/extensions/sync/service.go b/pkg/extensions/sync/service.go index 5a3a2c548..bc700f74c 100644 --- a/pkg/extensions/sync/service.go +++ b/pkg/extensions/sync/service.go @@ -549,9 +549,8 @@ func (service *BaseService) syncRef(ctx context.Context, localRepo string, remot copyOpts = append(copyOpts, regclient.ImageWithReferrers()) } - // Platform/Architecture filtering - use ImageWithChildren to handle manifest lists - // ImageWithChildren ensures we get the complete manifest list, then we can filter platforms - copyOpts = append(copyOpts, regclient.ImageWithChildren()) + // In regclient v0.8.3, we don't need a special option for manifest lists + // The platform filtering is already handled in our custom code below // Log platform filtering information if len(service.config.Platforms) > 0 { @@ -610,7 +609,14 @@ func (service *BaseService) syncRef(ctx context.Context, localRepo string, remot } else { // It's a single-arch image, verify if we should include this platform if man.GetDescriptor().Platform != nil { - platform := man.GetDescriptor().Platform + // Convert platform to ispec.Platform for compatibility with regclient v0.8.3 + platform := &ispec.Platform{ + Architecture: man.GetDescriptor().Platform.Architecture, + OS: man.GetDescriptor().Platform.OS, + OSVersion: man.GetDescriptor().Platform.OSVersion, + OSFeatures: man.GetDescriptor().Platform.OSFeatures, + Variant: man.GetDescriptor().Platform.Variant, + } if !service.shouldIncludePlatform(platform) { platformDesc := platform.Architecture if platform.OS != "" { From 90ffcb0823924be9027b708adc2b9be9cdd05cbb Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Thu, 1 May 2025 00:40:28 +0100 Subject: [PATCH 4/8] refactor(platform): drop architecture reference in favor of platform detection --- pkg/extensions/config/sync/config.go | 1 - pkg/extensions/sync/destination.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/extensions/config/sync/config.go b/pkg/extensions/config/sync/config.go index cc38df0bf..6096fe553 100644 --- a/pkg/extensions/config/sync/config.go +++ b/pkg/extensions/config/sync/config.go @@ -34,7 +34,6 @@ type RegistryConfig struct { OnlySigned *bool CredentialHelper string PreserveDigest bool // sync without converting - Architectures []string `mapstructure:",omitempty"` // filter architectures during sync (DEPRECATED: use Platforms instead) Platforms []string `mapstructure:",omitempty"` // filter platforms during sync (supports both "arch" and "os/arch" formats) } diff --git a/pkg/extensions/sync/destination.go b/pkg/extensions/sync/destination.go index e49764e0c..4a50963a5 100644 --- a/pkg/extensions/sync/destination.go +++ b/pkg/extensions/sync/destination.go @@ -46,7 +46,7 @@ func ParsePlatform(platform string) Platform { Architecture: parts[1], } } - // If only one part, assume it's the architecture + // For any other case, assume only architecture is specified return Platform{ OS: "", Architecture: platform, From 0e1e93f51fcc23e5a042250f9af0d5d023570bc7 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Thu, 1 May 2025 01:11:43 +0100 Subject: [PATCH 5/8] chore: remove deprecated architectures and transition to platforms --- examples/config-sync-architectures.json | 9 +- examples/config-sync-platforms.json | 109 ---- pkg/extensions/sync/destination.go | 9 +- pkg/extensions/sync/service.go | 43 +- test/blackbox/sync_platforms.bats | 655 ++++++++++++++++++++++++ 5 files changed, 669 insertions(+), 156 deletions(-) delete mode 100644 examples/config-sync-platforms.json create mode 100644 test/blackbox/sync_platforms.bats diff --git a/examples/config-sync-architectures.json b/examples/config-sync-architectures.json index 6f1bd1ead..5d2906b5a 100644 --- a/examples/config-sync-architectures.json +++ b/examples/config-sync-architectures.json @@ -26,8 +26,7 @@ "maxRetries": 3, "retryDelay": "5m", "onlySigned": true, - "architectures": ["amd64", "arm64"], - "platforms": ["linux/amd64", "linux/arm64"], + "platforms": ["amd64", "arm64", "linux/amd64", "linux/arm64"], "content": [ { "prefix": "/repo1/repo", @@ -60,8 +59,7 @@ "pollInterval": "12h", "tlsVerify": false, "onDemand": false, - "architectures": ["amd64"], - "platforms": ["linux/amd64", "windows/amd64"], + "platforms": ["amd64", "linux/amd64", "windows/amd64"], "content": [ { "prefix": "**", @@ -79,8 +77,7 @@ "tlsVerify": true, "maxRetries": 5, "retryDelay": "30s", - "architectures": ["amd64", "arm64", "arm"], - "platforms": ["linux/amd64", "linux/arm64", "linux/arm"] + "platforms": ["amd64", "arm64", "arm", "linux/amd64", "linux/arm64", "linux/arm"] }, { "urls": [ diff --git a/examples/config-sync-platforms.json b/examples/config-sync-platforms.json deleted file mode 100644 index 9980cd07a..000000000 --- a/examples/config-sync-platforms.json +++ /dev/null @@ -1,109 +0,0 @@ -{ - "distSpecVersion": "1.1.1", - "storage": { - "rootDirectory": "/tmp/zot" - }, - "http": { - "address": "127.0.0.1", - "port": "8080" - }, - "log": { - "level": "debug" - }, - "extensions": { - "sync": { - "enable": true, - "credentialsFile": "./examples/sync-auth-filepath.json", - "registries": [ - { - "urls": [ - "https://registry1:5000" - ], - "onDemand": false, - "pollInterval": "6h", - "tlsVerify": true, - "certDir": "/home/user/certs", - "maxRetries": 3, - "retryDelay": "5m", - "onlySigned": true, - "platforms": ["linux/amd64", "linux/arm64", "windows/amd64"], - "content": [ - { - "prefix": "/repo1/repo", - "tags": { - "regex": "4.*", - "semver": true - } - }, - { - "prefix": "/repo2/repo", - "destination": "/repo", - "stripPrefix": true - }, - { - "prefix": "/repo3/**" - }, - { - "prefix": "/repo4/**", - "tags": { - "excludeRegex": ".*-(amd64|arm64)$" - } - } - ] - }, - { - "urls": [ - "https://registry2:5000", - "https://registry3:5000" - ], - "pollInterval": "12h", - "tlsVerify": false, - "onDemand": false, - "platforms": ["linux/amd64"], - "content": [ - { - "prefix": "**", - "tags": { - "semver": true - } - } - ] - }, - { - "urls": [ - "https://index.docker.io" - ], - "onDemand": true, - "tlsVerify": true, - "maxRetries": 5, - "retryDelay": "30s", - "architectures": ["amd64", "arm64", "arm"] - }, - { - "urls": [ - "https://demo.goharbor.io" - ], - "pollInterval": "12h", - "content": [ - { - "prefix": "zot/**" - } - ], - "onDemand": true, - "tlsVerify": true, - "maxRetries": 5, - "retryDelay": "1m", - "platforms": ["darwin/amd64", "linux/amd64", "linux/arm64"] - }, - { - "urls": [ - "https://registry4:5000" - ], - "onDemand": false, - "tlsVerify": true, - "platforms": ["linux/amd64", "amd64", "arm64"] - } - ] - } - } -} \ No newline at end of file diff --git a/pkg/extensions/sync/destination.go b/pkg/extensions/sync/destination.go index 4a50963a5..c5f351676 100644 --- a/pkg/extensions/sync/destination.go +++ b/pkg/extensions/sync/destination.go @@ -301,13 +301,6 @@ func (registry *DestinationRegistry) copyManifest(repo string, desc ispec.Descri Str("repository", repo). Str("reference", reference). Msg("filtering manifest list by platforms") - } else if len(registry.config.Architectures) > 0 { - platformSpecs = registry.config.Architectures - registry.log.Info(). - Strs("architectures", registry.config.Architectures). - Str("repository", repo). - Str("reference", reference). - Msg("filtering manifest list by architectures (deprecated)") } } @@ -377,7 +370,7 @@ func (registry *DestinationRegistry) copyManifest(repo string, desc ispec.Descri // If we've filtered the manifest list, we need to update it if registry.config != nil && - (len(registry.config.Architectures) > 0 || len(registry.config.Platforms) > 0) && + len(registry.config.Platforms) > 0 && len(filteredManifests) != len(indexManifest.Manifests) && len(filteredManifests) > 0 { // Create a new index with the filtered manifests indexManifest.Manifests = filteredManifests diff --git a/pkg/extensions/sync/service.go b/pkg/extensions/sync/service.go index bc700f74c..2c7cdede0 100644 --- a/pkg/extensions/sync/service.go +++ b/pkg/extensions/sync/service.go @@ -451,7 +451,7 @@ func (service *BaseService) SyncRepo(ctx context.Context, repo string) error { } // shouldIncludePlatform determines if a platform should be included in the sync -// based on the configured platforms or architectures +// based on the configured platforms func (service *BaseService) shouldIncludePlatform(platform *ispec.Platform) bool { // Get platform specifications from the configuration var platformSpecs []string @@ -459,11 +459,8 @@ func (service *BaseService) shouldIncludePlatform(platform *ispec.Platform) bool // Check if we have platform specifications if len(service.config.Platforms) > 0 { platformSpecs = service.config.Platforms - } else if len(service.config.Architectures) > 0 { - // Fall back to architectures for backward compatibility - platformSpecs = service.config.Architectures } else { - // If no platforms or architectures are configured, include all + // If no platforms are configured, include all return true } @@ -503,30 +500,15 @@ func (service *BaseService) shouldIncludePlatform(platform *ispec.Platform) bool } // shouldIncludeArchitecture determines if an architecture should be included in the sync -// based on the configured architectures (if any) // DEPRECATED: Use shouldIncludePlatform instead func (service *BaseService) shouldIncludeArchitecture(arch string) bool { - // If no architectures are configured, include all architectures - if len(service.config.Architectures) > 0 && len(service.config.Platforms) == 0 { - // Check if the architecture is in the configured list - for _, configArch := range service.config.Architectures { - if configArch == arch { - return true - } - } - return false - } - - // When using the Platforms field, we need a different check - if len(service.config.Platforms) > 0 { - platform := &ispec.Platform{ - Architecture: arch, - } - return service.shouldIncludePlatform(platform) + // Create a platform with just the architecture field + platform := &ispec.Platform{ + Architecture: arch, } - // No filters configured, include all - return true + // Use the platform-based filtering method + return service.shouldIncludePlatform(platform) } func (service *BaseService) syncRef(ctx context.Context, localRepo string, remoteImageRef, localImageRef ref.Ref, @@ -558,11 +540,6 @@ func (service *BaseService) syncRef(ctx context.Context, localRepo string, remot Strs("platforms", service.config.Platforms). Str("image", remoteImageRef.CommonName()). Msg("filtering platforms during sync") - } else if len(service.config.Architectures) > 0 { - service.log.Info(). - Strs("architectures", service.config.Architectures). - Str("image", remoteImageRef.CommonName()). - Msg("filtering architectures during sync (deprecated)") } // check if image is already synced @@ -577,8 +554,8 @@ func (service *BaseService) syncRef(ctx context.Context, localRepo string, remot service.log.Info().Str("remote image", remoteImageRef.CommonName()). Str("local image", fmt.Sprintf("%s:%s", localRepo, remoteImageRef.Tag)).Msg("syncing image") - // When architectures are specified, we need to filter the manifest list - if len(service.config.Architectures) > 0 { + // When platforms are specified, we need to filter the manifest list + if len(service.config.Platforms) > 0 { // Get the manifest to check if it's a manifest list man, err := service.rc.ManifestGet(ctx, remoteImageRef) if err != nil { @@ -593,7 +570,7 @@ func (service *BaseService) syncRef(ctx context.Context, localRepo string, remot if isIndexer { service.log.Info(). Str("remote image", remoteImageRef.CommonName()). - Msg("filtering architectures for multi-arch image") + Msg("filtering platforms for multi-arch image") // Use ImageCopy with the architecture filtering options err = service.rc.ImageCopy(ctx, remoteImageRef, localImageRef, copyOpts...) diff --git a/test/blackbox/sync_platforms.bats b/test/blackbox/sync_platforms.bats new file mode 100644 index 000000000..122bc10c0 --- /dev/null +++ b/test/blackbox/sync_platforms.bats @@ -0,0 +1,655 @@ +# Test suite for platform filtering during sync operations +# Verifies the behavior of the Platforms field in RegistryConfig +# Platforms supports both architecture-only ("amd64") and OS/architecture ("linux/amd64") formats +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 + + if [ ! $(command -v skopeo) ]; then + echo "you need to install skopeo as a prerequisite to running the tests" >&3 + return 1 + fi + + return 0 +} + +function setup_file() { + # Verify prerequisites are available + if ! $(verify_prerequisites); then + exit 1 + fi + + # Create directories for the different zot instances + local zot_source_dir=${BATS_FILE_TMPDIR}/zot-source + local zot_arch_filter_dir=${BATS_FILE_TMPDIR}/zot-arch-filter + local zot_no_filter_dir=${BATS_FILE_TMPDIR}/zot-no-filter + local zot_platform_filter_dir=${BATS_FILE_TMPDIR}/zot-platform-filter + + local zot_source_config=${BATS_FILE_TMPDIR}/zot_source_config.json + local zot_arch_filter_config=${BATS_FILE_TMPDIR}/zot_arch_filter_config.json + local zot_no_filter_config=${BATS_FILE_TMPDIR}/zot_no_filter_config.json + local zot_platform_filter_config=${BATS_FILE_TMPDIR}/zot_platform_filter_config.json + + mkdir -p ${zot_source_dir} + mkdir -p ${zot_arch_filter_dir} + mkdir -p ${zot_no_filter_dir} + mkdir -p ${zot_platform_filter_dir} + + # Get free ports for our zot instances + zot_source_port=$(get_free_port) + echo ${zot_source_port} > ${BATS_FILE_TMPDIR}/zot.source.port + + zot_arch_filter_port=$(get_free_port) + echo ${zot_arch_filter_port} > ${BATS_FILE_TMPDIR}/zot.arch_filter.port + + zot_no_filter_port=$(get_free_port) + echo ${zot_no_filter_port} > ${BATS_FILE_TMPDIR}/zot.no_filter.port + + zot_platform_filter_port=$(get_free_port) + echo ${zot_platform_filter_port} > ${BATS_FILE_TMPDIR}/zot.platform_filter.port + + # Create config for source registry (basic config without sync) + cat >${zot_source_config} <${zot_arch_filter_config} <${zot_no_filter_config} <${zot_platform_filter_config} < ${BATS_FILE_TMPDIR}/zot.multi_arch_filter.port + + # Create config for multi-platform filtered registry (with amd64 and arm64) + cat >${zot_multi_arch_filter_config} < ${BATS_FILE_TMPDIR}/zot.ondemand_filter.port + + # Create config for on-demand platform filtered registry (with amd64 only) + cat >${zot_ondemand_filter_config} < ${BATS_FILE_TMPDIR}/zot.mixed_filter.port + + # Create config for mixed filtered registry + cat >${zot_mixed_filter_config} < ${BATS_FILE_TMPDIR}/zot.os_filter.port + + # Create config for OS filtered registry + cat >${zot_os_filter_config} < Date: Thu, 1 May 2025 09:19:05 +0100 Subject: [PATCH 6/8] chore: Clean up the bat tests --- ...ctures.json => config-sync-platforms.json} | 0 test/blackbox/sync_architectures.bats | 348 ++++++++++++++++-- 2 files changed, 318 insertions(+), 30 deletions(-) rename examples/{config-sync-architectures.json => config-sync-platforms.json} (100%) diff --git a/examples/config-sync-architectures.json b/examples/config-sync-platforms.json similarity index 100% rename from examples/config-sync-architectures.json rename to examples/config-sync-platforms.json diff --git a/test/blackbox/sync_architectures.bats b/test/blackbox/sync_architectures.bats index dcd31edda..8f10df250 100644 --- a/test/blackbox/sync_architectures.bats +++ b/test/blackbox/sync_architectures.bats @@ -1,6 +1,17 @@ -# Test suite for platform filtering during sync operations -# Verifies the behavior of the Architectures and Platforms fields in RegistryConfig -# Platforms supports both architecture-only ("amd64") and OS/architecture ("linux/amd64") formats +# Test suite for architecture and platform filtering during sync operations +# +# This file tests two related filtering parameters in RegistryConfig: +# +# 1. "architectures": Filters by architecture only (e.g., "amd64", "arm64") +# Example: "architectures": ["amd64"] +# +# 2. "platforms": Supports both architecture-only and OS/architecture formats +# Example: "platforms": ["amd64"] (architecture-only format) +# Example: "platforms": ["linux/amd64"] (OS/architecture format) +# +# Both parameters allow selective syncing of multi-architecture images based on +# the architecture and/or OS platform, reducing storage and bandwidth requirements. + load helpers_zot load helpers_wait @@ -112,7 +123,7 @@ EOF } EOF - # Create config for non-filtered registry (no architecture filter) + # Create config for non-filtered registry (no architecture/platform filter) cat >${zot_no_filter_config} < ${BATS_FILE_TMPDIR}/zot.multi_platform_filter.port + + # Create config for multi-platform filtered registry (with amd64 and arm64) + cat >${zot_multi_platform_filter_config} < ${BATS_FILE_TMPDIR}/zot.ondemand_filter.port + zot_ondemand_arch_filter_port=$(get_free_port) + echo ${zot_ondemand_arch_filter_port} > ${BATS_FILE_TMPDIR}/zot.ondemand_arch_filter.port # Create config for on-demand architecture filtered registry (with amd64 only) - cat >${zot_ondemand_filter_config} <${zot_ondemand_arch_filter_config} < ${BATS_FILE_TMPDIR}/zot.ondemand_platform_filter.port + + # Create config for on-demand platform filtered registry (with amd64 only) + cat >${zot_ondemand_platform_filter_config} < ${BATS_FILE_TMPDIR}/zot.os_filter.port - # Create config for OS filtered registry + # Create config for OS filtered registry - only including Linux platforms cat >${zot_os_filter_config} < ${BATS_FILE_TMPDIR}/zot.both_params.port + + # Create config with both architectures and platforms parameters + # When both are specified, they should be combined with logical OR + cat >${zot_both_params_config} < Date: Thu, 1 May 2025 13:16:58 +0100 Subject: [PATCH 7/8] feat(sync): Add support for platform/architecture/variant --- examples/README.md | 24 +++++ examples/config-sync-platforms.json | 8 +- pkg/extensions/sync/destination.go | 27 +++++- test/blackbox/sync_platforms.bats | 131 ++++++++++++++++++++++++++++ 4 files changed, 182 insertions(+), 8 deletions(-) diff --git a/examples/README.md b/examples/README.md index 026eac550..c52976830 100644 --- a/examples/README.md +++ b/examples/README.md @@ -1017,6 +1017,30 @@ Configure each registry sync: ``` Prefixes can be strings that exactly match repositories or they can be [glob](https://en.wikipedia.org/wiki/Glob_(programming)) patterns. +### Platform Filtering in Sync + +You can selectively sync multi-architecture images by specifying which platforms to include: + +``` +"platforms": ["amd64", "arm64", "linux/amd64", "linux/arm64", "linux/arm/v7"] +``` + +The platforms field accepts three formats: + +1. Architecture-only format: `"amd64"`, `"arm64"`, `"arm"`, etc. +2. OS/Architecture format: `"linux/amd64"`, `"windows/amd64"`, etc. +3. OS/Architecture/Variant format: `"linux/arm/v7"`, `"linux/arm/v8"`, etc. + +This is particularly useful for ARM architectures where variants like "v6", "v7", and "v8" are common. + +Example configuration with variant filtering: + +```json +"platforms": ["linux/amd64", "linux/arm64", "linux/arm/v7"] +``` + +This would sync only manifests for Linux AMD64, Linux ARM64, and Linux ARM v7 architectures, saving bandwidth and storage by excluding other architectures and variants. + ### Sync's certDir option sync uses the same logic for reading cert directory as docker: https://docs.docker.com/engine/security/certificates/#understand-the-configuration diff --git a/examples/config-sync-platforms.json b/examples/config-sync-platforms.json index 5d2906b5a..dddc51da3 100644 --- a/examples/config-sync-platforms.json +++ b/examples/config-sync-platforms.json @@ -26,7 +26,7 @@ "maxRetries": 3, "retryDelay": "5m", "onlySigned": true, - "platforms": ["amd64", "arm64", "linux/amd64", "linux/arm64"], + "platforms": ["amd64", "arm64", "linux/amd64", "linux/arm64", "linux/arm/v7"], "content": [ { "prefix": "/repo1/repo", @@ -77,7 +77,7 @@ "tlsVerify": true, "maxRetries": 5, "retryDelay": "30s", - "platforms": ["amd64", "arm64", "arm", "linux/amd64", "linux/arm64", "linux/arm"] + "platforms": ["amd64", "arm64", "arm", "linux/amd64", "linux/arm64", "linux/arm/v7", "linux/arm/v8"] }, { "urls": [ @@ -93,7 +93,7 @@ "tlsVerify": true, "maxRetries": 5, "retryDelay": "1m", - "platforms": ["darwin/amd64", "linux/amd64", "linux/arm64"] + "platforms": ["darwin/amd64", "linux/amd64", "linux/arm64", "linux/arm/v7"] }, { "urls": [ @@ -101,7 +101,7 @@ ], "onDemand": false, "tlsVerify": true, - "platforms": ["linux/amd64", "amd64", "arm64"] + "platforms": ["linux/amd64", "amd64", "arm64", "linux/arm/v6", "linux/arm/v7", "linux/arm/v8"] } ] } diff --git a/pkg/extensions/sync/destination.go b/pkg/extensions/sync/destination.go index c5f351676..0d1d0c19b 100644 --- a/pkg/extensions/sync/destination.go +++ b/pkg/extensions/sync/destination.go @@ -30,17 +30,27 @@ import ( storageTypes "zotregistry.dev/zot/pkg/storage/types" ) -// Platform represents an OS/architecture combination +// Platform represents an OS/architecture/variant combination type Platform struct { OS string Architecture string + Variant string } // ParsePlatform parses a platform string into a Platform struct -// The string can be in the format "os/arch" or just "arch" +// The string can be in the following formats: +// - "arch" (e.g., "amd64") +// - "os/arch" (e.g., "linux/amd64") +// - "os/arch/variant" (e.g., "linux/arm/v7") func ParsePlatform(platform string) Platform { parts := strings.Split(platform, "/") - if len(parts) == 2 { + if len(parts) == 3 { + return Platform{ + OS: parts[0], + Architecture: parts[1], + Variant: parts[2], + } + } else if len(parts) == 2 { return Platform{ OS: parts[0], Architecture: parts[1], @@ -54,7 +64,7 @@ func ParsePlatform(platform string) Platform { } // MatchesPlatform checks if the given platform matches any of the platform specifications -// Platform specs can be in format "os/arch" or just "arch" +// Platform specs can be in format "os/arch/variant", "os/arch", or just "arch" func MatchesPlatform(platform *ispec.Platform, platformSpecs []string) bool { if platform == nil || len(platformSpecs) == 0 { return true @@ -75,6 +85,12 @@ func MatchesPlatform(platform *ispec.Platform, platformSpecs []string) bool { continue } + // Check if variant matches (if specified) + if specPlatform.Variant != "" && platform.Variant != "" && + specPlatform.Variant != platform.Variant { + continue + } + // If we got here, it's a match return true } @@ -315,6 +331,9 @@ func (registry *DestinationRegistry) copyManifest(repo string, desc ispec.Descri platformDesc := manifest.Platform.Architecture if manifest.Platform.OS != "" { platformDesc = manifest.Platform.OS + "/" + manifest.Platform.Architecture + if manifest.Platform.Variant != "" { + platformDesc += "/" + manifest.Platform.Variant + } } registry.log.Info(). diff --git a/test/blackbox/sync_platforms.bats b/test/blackbox/sync_platforms.bats index 122bc10c0..47934369c 100644 --- a/test/blackbox/sync_platforms.bats +++ b/test/blackbox/sync_platforms.bats @@ -652,4 +652,135 @@ EOF fi fi fi +} + +# Test syncing with ARM architecture variant filtering +@test "sync with ARM architecture variant filtering" { + # Get ports for our registries + zot_source_port=$(cat ${BATS_FILE_TMPDIR}/zot.source.port) + + # Create a new registry that filters by ARM variant + local zot_variant_filter_dir=${BATS_FILE_TMPDIR}/zot-variant-filter + local zot_variant_filter_config=${BATS_FILE_TMPDIR}/zot_variant_filter_config.json + mkdir -p ${zot_variant_filter_dir} + + zot_variant_filter_port=$(get_free_port) + echo ${zot_variant_filter_port} > ${BATS_FILE_TMPDIR}/zot.variant_filter.port + + # Create config for variant filtered registry (linux/arm/v7 only) + cat >${zot_variant_filter_config} <${arm_index_file} < Date: Thu, 1 May 2025 14:02:16 +0100 Subject: [PATCH 8/8] chore: Additional tests --- pkg/extensions/sync/destination_test.go | 196 ++++++ pkg/extensions/sync/service_platform_test.go | 276 +++++++++ pkg/extensions/sync/sync_ref_test.go | 604 +++++++++++++++++++ test/blackbox/sync_platforms.bats | 90 +++ 4 files changed, 1166 insertions(+) create mode 100644 pkg/extensions/sync/destination_test.go create mode 100644 pkg/extensions/sync/service_platform_test.go create mode 100644 pkg/extensions/sync/sync_ref_test.go diff --git a/pkg/extensions/sync/destination_test.go b/pkg/extensions/sync/destination_test.go new file mode 100644 index 000000000..96722748c --- /dev/null +++ b/pkg/extensions/sync/destination_test.go @@ -0,0 +1,196 @@ +//go:build sync + +package sync + +import ( + "testing" + + ispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/stretchr/testify/assert" +) + +func TestParsePlatform(t *testing.T) { + tests := []struct { + name string + platformString string + expected Platform + }{ + { + name: "OS/arch format", + platformString: "linux/amd64", + expected: Platform{ + OS: "linux", + Architecture: "amd64", + }, + }, + { + name: "arch-only format", + platformString: "arm64", + expected: Platform{ + OS: "", + Architecture: "arm64", + }, + }, + { + name: "empty string", + platformString: "", + expected: Platform{ + OS: "", + Architecture: "", + }, + }, + { + name: "OS with slash but no arch", + platformString: "linux/", + expected: Platform{ + OS: "linux", + Architecture: "", + }, + }, + { + name: "slash but no OS", + platformString: "/amd64", + expected: Platform{ + OS: "", + Architecture: "amd64", + }, + }, + { + name: "multiple slashes", + platformString: "linux/amd64/v8", + expected: Platform{ + OS: "", + Architecture: "linux/amd64/v8", + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := ParsePlatform(test.platformString) + assert.Equal(t, test.expected, result) + }) + } +} + +func TestMatchesPlatform(t *testing.T) { + tests := []struct { + name string + platform *ispec.Platform + platformSpecs []string + expected bool + }{ + { + name: "nil platform", + platform: nil, + platformSpecs: []string{"linux/amd64", "linux/arm64"}, + expected: true, + }, + { + name: "empty platform specs", + platform: &ispec.Platform{ + OS: "linux", + Architecture: "amd64", + }, + platformSpecs: []string{}, + expected: true, + }, + { + name: "exact OS/arch match", + platform: &ispec.Platform{ + OS: "linux", + Architecture: "amd64", + }, + platformSpecs: []string{"linux/amd64"}, + expected: true, + }, + { + name: "exact OS/arch non-match", + platform: &ispec.Platform{ + OS: "linux", + Architecture: "amd64", + }, + platformSpecs: []string{"linux/arm64"}, + expected: false, + }, + { + name: "arch-only match", + platform: &ispec.Platform{ + OS: "linux", + Architecture: "amd64", + }, + platformSpecs: []string{"amd64"}, + expected: true, + }, + { + name: "arch-only non-match", + platform: &ispec.Platform{ + OS: "linux", + Architecture: "amd64", + }, + platformSpecs: []string{"arm64"}, + expected: false, + }, + { + name: "OS/arch and non-matching OS", + platform: &ispec.Platform{ + OS: "windows", + Architecture: "amd64", + }, + platformSpecs: []string{"linux/amd64"}, + expected: false, + }, + { + name: "multiple specs with match", + platform: &ispec.Platform{ + OS: "linux", + Architecture: "amd64", + }, + platformSpecs: []string{"windows/amd64", "linux/arm64", "linux/amd64"}, + expected: true, + }, + { + name: "multiple specs with no match", + platform: &ispec.Platform{ + OS: "darwin", + Architecture: "arm64", + }, + platformSpecs: []string{"windows/amd64", "linux/arm64", "linux/amd64"}, + expected: false, + }, + { + name: "match with empty OS in spec", + platform: &ispec.Platform{ + OS: "linux", + Architecture: "arm64", + }, + platformSpecs: []string{"/arm64"}, + expected: true, + }, + { + name: "match with empty architecture in spec", + platform: &ispec.Platform{ + OS: "linux", + Architecture: "", + }, + platformSpecs: []string{"linux/"}, + expected: true, + }, + { + name: "match with both arch-only and OS/arch formats", + platform: &ispec.Platform{ + OS: "linux", + Architecture: "amd64", + }, + platformSpecs: []string{"arm64", "linux/amd64"}, + expected: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := MatchesPlatform(test.platform, test.platformSpecs) + assert.Equal(t, test.expected, result) + }) + } +} diff --git a/pkg/extensions/sync/service_platform_test.go b/pkg/extensions/sync/service_platform_test.go new file mode 100644 index 000000000..02b8506b4 --- /dev/null +++ b/pkg/extensions/sync/service_platform_test.go @@ -0,0 +1,276 @@ +//go:build sync +// +build sync + +package sync + +import ( + "testing" + + ispec "github.com/opencontainers/image-spec/specs-go/v1" + syncconf "zotregistry.dev/zot/pkg/extensions/config/sync" + "zotregistry.dev/zot/pkg/log" +) + +func TestShouldIncludePlatform(t *testing.T) { + logger := log.NewLogger("debug", "") + + // Test case 1: When no platforms are configured (should return true) + t.Run("NoPlatformsConfigured", func(t *testing.T) { + service := &BaseService{ + config: syncconf.RegistryConfig{ + Platforms: []string{}, + }, + log: logger, + } + + platform := &ispec.Platform{ + Architecture: "amd64", + OS: "linux", + } + + if !service.shouldIncludePlatform(platform) { + t.Errorf("Expected shouldIncludePlatform to return true when no platforms configured, got false") + } + }) + + // Test case 2: When the platform is nil (should return true) + t.Run("NilPlatform", func(t *testing.T) { + service := &BaseService{ + config: syncconf.RegistryConfig{ + Platforms: []string{"linux/amd64", "arm64"}, + }, + log: logger, + } + + if !service.shouldIncludePlatform(nil) { + t.Errorf("Expected shouldIncludePlatform to return true for nil platform, got false") + } + }) + + // Test case 3: When the platform matches an OS/arch combination in the config + t.Run("MatchesOSAndArch", func(t *testing.T) { + service := &BaseService{ + config: syncconf.RegistryConfig{ + Platforms: []string{"linux/amd64", "linux/arm64"}, + }, + log: logger, + } + + platform := &ispec.Platform{ + Architecture: "amd64", + OS: "linux", + } + + if !service.shouldIncludePlatform(platform) { + t.Errorf("Expected shouldIncludePlatform to return true for matching OS/arch, got false") + } + + // Should not match when OS is different + platform.OS = "windows" + if service.shouldIncludePlatform(platform) { + t.Errorf("Expected shouldIncludePlatform to return false for non-matching OS, got true") + } + }) + + // Test case 4: When the platform matches just an architecture in the config + t.Run("MatchesArchOnly", func(t *testing.T) { + service := &BaseService{ + config: syncconf.RegistryConfig{ + Platforms: []string{"amd64", "linux/arm64"}, + }, + log: logger, + } + + // Should match any OS with amd64 architecture + platform := &ispec.Platform{ + Architecture: "amd64", + OS: "linux", + } + if !service.shouldIncludePlatform(platform) { + t.Errorf("Expected shouldIncludePlatform to return true for matching arch-only, got false") + } + + platform.OS = "windows" + if !service.shouldIncludePlatform(platform) { + t.Errorf("Expected shouldIncludePlatform to return true for matching arch-only with different OS, got false") + } + + // Should match specific OS/arch combo + platform = &ispec.Platform{ + Architecture: "arm64", + OS: "linux", + } + if !service.shouldIncludePlatform(platform) { + t.Errorf("Expected shouldIncludePlatform to return true for matching OS/arch, got false") + } + + // Should not match when OS is different from specified OS/arch + platform.OS = "windows" + if service.shouldIncludePlatform(platform) { + t.Errorf("Expected shouldIncludePlatform to return false for non-matching OS, got true") + } + }) + + // Test case 5: When the platform doesn't match any of the configured platforms + t.Run("NoMatches", func(t *testing.T) { + service := &BaseService{ + config: syncconf.RegistryConfig{ + Platforms: []string{"linux/amd64", "linux/arm64"}, + }, + log: logger, + } + + platform := &ispec.Platform{ + Architecture: "ppc64le", + OS: "linux", + } + + if service.shouldIncludePlatform(platform) { + t.Errorf("Expected shouldIncludePlatform to return false for non-matching platform, got true") + } + }) + + // Test case 6: Empty OS in platform specification + t.Run("EmptyOSInConfig", func(t *testing.T) { + service := &BaseService{ + config: syncconf.RegistryConfig{ + Platforms: []string{"/amd64"}, // This is an edge case with empty OS + }, + log: logger, + } + + platform := &ispec.Platform{ + Architecture: "amd64", + OS: "linux", + } + + if !service.shouldIncludePlatform(platform) { + t.Errorf("Expected shouldIncludePlatform to return true for matching arch with empty OS, got false") + } + }) + + // Test case 7: Mixed format platforms (OS/arch and arch-only) + t.Run("MixedFormatPlatforms", func(t *testing.T) { + service := &BaseService{ + config: syncconf.RegistryConfig{ + Platforms: []string{"amd64", "linux/arm64"}, + }, + log: logger, + } + + // Should match amd64 arch on any OS + platform := &ispec.Platform{ + Architecture: "amd64", + OS: "windows", + } + if !service.shouldIncludePlatform(platform) { + t.Errorf("Expected shouldIncludePlatform to return true for matching arch-only, got false") + } + + // Should match linux/arm64 combo + platform = &ispec.Platform{ + Architecture: "arm64", + OS: "linux", + } + if !service.shouldIncludePlatform(platform) { + t.Errorf("Expected shouldIncludePlatform to return true for matching OS/arch, got false") + } + + // Should not match non-linux arm64 + platform = &ispec.Platform{ + Architecture: "arm64", + OS: "windows", + } + if service.shouldIncludePlatform(platform) { + t.Errorf("Expected shouldIncludePlatform to return false for non-matching OS with arm64, got true") + } + }) +} + +func TestShouldIncludeArchitecture(t *testing.T) { + logger := log.NewLogger("debug", "") + + // Test case 1: When no platforms are configured (should return true) + t.Run("NoPlatformsConfigured", func(t *testing.T) { + service := &BaseService{ + config: syncconf.RegistryConfig{ + Platforms: []string{}, + }, + log: logger, + } + + if !service.shouldIncludeArchitecture("amd64") { + t.Errorf("Expected shouldIncludeArchitecture to return true when no platforms configured, got false") + } + }) + + // Test case 2: When the architecture matches one in the config (platform format) + t.Run("MatchingArchInPlatform", func(t *testing.T) { + service := &BaseService{ + config: syncconf.RegistryConfig{ + Platforms: []string{"linux/amd64", "arm64"}, + }, + log: logger, + } + + if !service.shouldIncludeArchitecture("amd64") { + t.Errorf("Expected shouldIncludeArchitecture to return true for matching arch, got false") + } + }) + + // Test case 3: When the architecture matches one in the config (arch-only format) + t.Run("MatchingArchOnly", func(t *testing.T) { + service := &BaseService{ + config: syncconf.RegistryConfig{ + Platforms: []string{"amd64", "linux/arm64"}, + }, + log: logger, + } + + if !service.shouldIncludeArchitecture("amd64") { + t.Errorf("Expected shouldIncludeArchitecture to return true for matching arch-only, got false") + } + + if !service.shouldIncludeArchitecture("arm64") { + t.Errorf("Expected shouldIncludeArchitecture to return true for matching arch in OS/arch, got false") + } + }) + + // Test case 4: When the architecture doesn't match any in the config + t.Run("NonMatchingArch", func(t *testing.T) { + service := &BaseService{ + config: syncconf.RegistryConfig{ + Platforms: []string{"linux/amd64", "arm64"}, + }, + log: logger, + } + + if service.shouldIncludeArchitecture("ppc64le") { + t.Errorf("Expected shouldIncludeArchitecture to return false for non-matching arch, got true") + } + }) + + // Test case 5: Verify that shouldIncludeArchitecture delegates to shouldIncludePlatform + t.Run("DelegatesToShouldIncludePlatform", func(t *testing.T) { + service := &BaseService{ + config: syncconf.RegistryConfig{ + Platforms: []string{"linux/amd64", "arm64"}, + }, + log: logger, + } + + // Create platform equivalent of architecture + arch := "amd64" + platform := &ispec.Platform{ + Architecture: arch, + } + + // Results from both functions should match + includeArch := service.shouldIncludeArchitecture(arch) + includePlatform := service.shouldIncludePlatform(platform) + + if includeArch != includePlatform { + t.Errorf("Expected shouldIncludeArchitecture to delegate to shouldIncludePlatform, got different results") + } + }) +} diff --git a/pkg/extensions/sync/sync_ref_test.go b/pkg/extensions/sync/sync_ref_test.go new file mode 100644 index 000000000..c49ddfe45 --- /dev/null +++ b/pkg/extensions/sync/sync_ref_test.go @@ -0,0 +1,604 @@ +//go:build sync +// +build sync + +package sync + +import ( + "context" + "fmt" + "testing" + + godigest "github.com/opencontainers/go-digest" + ispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/regclient/regclient" + "github.com/regclient/regclient/types/manifest" + "github.com/regclient/regclient/types/platform" + "github.com/regclient/regclient/types/ref" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + syncconf "zotregistry.dev/zot/pkg/extensions/config/sync" + "zotregistry.dev/zot/pkg/log" +) + +// Mock implementations for testing +type mockRemote struct { + mock.Mock +} + +func (m *mockRemote) GetImageReference(repo string, tag string) (ref.Ref, error) { + args := m.Called(repo, tag) + return args.Get(0).(ref.Ref), args.Error(1) +} + +func (m *mockRemote) GetHostName() string { + args := m.Called() + return args.String(0) +} + +func (m *mockRemote) GetRepositories(ctx context.Context) ([]string, error) { + args := m.Called(ctx) + return args.Get(0).([]string), args.Error(1) +} + +func (m *mockRemote) GetTags(ctx context.Context, repo string) ([]string, error) { + args := m.Called(ctx, repo) + return args.Get(0).([]string), args.Error(1) +} + +func (m *mockRemote) GetOCIDigest(ctx context.Context, repo, tag string) (godigest.Digest, godigest.Digest, bool, error) { + args := m.Called(ctx, repo, tag) + return args.Get(0).(godigest.Digest), args.Get(1).(godigest.Digest), args.Bool(2), args.Error(3) +} + +func (m *mockRemote) GetDigest(ctx context.Context, repo, tag string) (godigest.Digest, error) { + args := m.Called(ctx, repo, tag) + return args.Get(0).(godigest.Digest), args.Error(1) +} + +type mockDestination struct { + mock.Mock +} + +func (m *mockDestination) GetImageReference(repo string, tag string) (ref.Ref, error) { + args := m.Called(repo, tag) + return args.Get(0).(ref.Ref), args.Error(1) +} + +func (m *mockDestination) CanSkipImage(repo string, tag string, digest godigest.Digest) (bool, error) { + args := m.Called(repo, tag, digest) + return args.Bool(0), args.Error(1) +} + +func (m *mockDestination) CommitAll(repo string, imageReference ref.Ref) error { + args := m.Called(repo, imageReference) + return args.Error(0) +} + +func (m *mockDestination) CleanupImage(imageReference ref.Ref, repo string) error { + args := m.Called(imageReference, repo) + return args.Error(0) +} + +// Mock RegClient for testing +type mockRegClient struct { + mock.Mock +} + +func (m *mockRegClient) ManifestGet(ctx context.Context, r ref.Ref) (manifest.Manifest, error) { + args := m.Called(ctx, r) + return args.Get(0).(manifest.Manifest), args.Error(1) +} + +func (m *mockRegClient) ImageCopy(ctx context.Context, src, dst ref.Ref, opts ...regclient.ImageOpts) error { + // Convert from variadic to an array that can be captured by the mock + args := m.Called(ctx, src, dst, opts) + return args.Error(0) +} + +func (m *mockRegClient) Close(ctx context.Context, r ref.Ref) error { + args := m.Called(ctx, r) + return args.Error(0) +} + +// Mock manifest types for testing +type mockManifest struct { + mock.Mock + descriptor ispec.Descriptor +} + +func (m *mockManifest) GetDescriptor() ispec.Descriptor { + args := m.Called() + if len(args) > 0 { + return args.Get(0).(ispec.Descriptor) + } + return m.descriptor +} + +type mockIndexManifest struct { + mockManifest + manifests []ispec.Descriptor +} + +func (m *mockIndexManifest) GetManifestList() ([]ispec.Descriptor, error) { + args := m.Called() + if len(args) > 0 { + return args.Get(0).([]ispec.Descriptor), args.Error(1) + } + return m.manifests, nil +} + +// Setup function to create a service with the desired configuration +func setupServiceWithConfig(platforms []string) (*BaseService, *mockRemote, *mockDestination, *mockRegClient) { + logger := log.NewLogger("debug", "") + + // Create configuration + config := syncconf.RegistryConfig{ + Platforms: platforms, + } + + // Create mocks + mockRemote := new(mockRemote) + mockDestination := new(mockDestination) + mockRegClient := new(mockRegClient) + + // Create service + service := &BaseService{ + config: config, + remote: mockRemote, + destination: mockDestination, + rc: mockRegClient, + log: logger, + } + + return service, mockRemote, mockDestination, mockRegClient +} + +// Helper function to create a mock manifest with a specific platform +func createSingleArchManifest(os, arch string) manifest.Manifest { + descriptor := ispec.Descriptor{ + Platform: &ispec.Platform{ + OS: os, + Architecture: arch, + }, + } + + mockMan := &mockManifest{ + descriptor: descriptor, + } + + mockMan.On("GetDescriptor").Return(descriptor) + + return mockMan +} + +// Helper function to create a mock multi-arch manifest with specified platforms +func createMultiArchManifest(platforms []platform.Platform) manifest.Manifest { + descriptors := make([]ispec.Descriptor, len(platforms)) + + for i, plat := range platforms { + descriptors[i] = ispec.Descriptor{ + Platform: &ispec.Platform{ + OS: plat.OS, + Architecture: plat.Architecture, + Variant: plat.Variant, + OSVersion: plat.OSVersion, + OSFeatures: plat.OSFeatures, + }, + } + } + + indexMan := &mockIndexManifest{ + manifests: descriptors, + } + + indexMan.On("GetManifestList").Return(descriptors, nil) + + // Make sure it satisfies the Indexer interface + manifest.Indexer(indexMan) + + return indexMan +} + +func TestSyncRefWithNoConfiguredPlatforms(t *testing.T) { + // Create service with no platforms configured + service, mockRemote, mockDestination, mockRegClient := setupServiceWithConfig([]string{}) + + // Test context + ctx := context.Background() + localRepo := "test-repo" + remoteRef := ref.Ref{ + Scheme: "docker", + Registry: "remote-registry.com", + Repository: "repo", + Tag: "latest", + } + localRef := ref.Ref{ + Scheme: "docker", + Registry: "localhost", + Repository: "test-repo", + Tag: "latest", + } + digest := godigest.FromString("test") + + // Setup expectation for CanSkipImage + mockDestination.On("CanSkipImage", localRepo, "latest", digest).Return(false, nil) + + // Setup single-arch manifest + manifest := createSingleArchManifest("linux", "amd64") + mockRegClient.On("ManifestGet", ctx, remoteRef).Return(manifest, nil) + + // Expect ImageCopy to be called since no platforms are configured (should copy everything) + mockRegClient.On("ImageCopy", ctx, remoteRef, localRef, mock.Anything).Return(nil) + + // Run the test + err := service.syncRef(ctx, localRepo, remoteRef, localRef, digest, false) + + // Verify expectations + assert.NoError(t, err) + mockRegClient.AssertNumberOfCalls(t, "ImageCopy", 1) + mockDestination.AssertCalled(t, "CanSkipImage", localRepo, "latest", digest) +} + +func TestSyncRefWithArchitectureOnly(t *testing.T) { + // Create service with only amd64 architecture configured + service, _, mockDestination, mockRegClient := setupServiceWithConfig([]string{"amd64"}) + + // Test context + ctx := context.Background() + localRepo := "test-repo" + remoteRef := ref.Ref{ + Scheme: "docker", + Registry: "remote-registry.com", + Repository: "repo", + Tag: "latest", + } + localRef := ref.Ref{ + Scheme: "docker", + Registry: "localhost", + Repository: "test-repo", + Tag: "latest", + } + digest := godigest.FromString("test") + + // Setup expectation for CanSkipImage + mockDestination.On("CanSkipImage", localRepo, "latest", digest).Return(false, nil) + + t.Run("MatchingArchitecture", func(t *testing.T) { + // Create a single-arch manifest with matching architecture + manifest := createSingleArchManifest("linux", "amd64") + mockRegClient.On("ManifestGet", ctx, remoteRef).Return(manifest, nil).Once() + + // Expect ImageCopy to be called since the architecture matches + mockRegClient.On("ImageCopy", ctx, remoteRef, localRef, mock.Anything).Return(nil).Once() + + // Run the test + err := service.syncRef(ctx, localRepo, remoteRef, localRef, digest, false) + + // Verify expectations + assert.NoError(t, err) + mockRegClient.AssertCalled(t, "ImageCopy", ctx, remoteRef, localRef, mock.Anything) + }) + + t.Run("NonMatchingArchitecture", func(t *testing.T) { + // Create a single-arch manifest with non-matching architecture + manifest := createSingleArchManifest("linux", "arm64") + mockRegClient.On("ManifestGet", ctx, remoteRef).Return(manifest, nil).Once() + + // ImageCopy should not be called since the architecture doesn't match + + // Run the test + err := service.syncRef(ctx, localRepo, remoteRef, localRef, digest, false) + + // Verify expectations + assert.NoError(t, err) + // Check that ImageCopy was NOT called again + mockRegClient.AssertNumberOfCalls(t, "ImageCopy", 1) // Still 1 from the previous subtest + }) +} + +func TestSyncRefWithFullPlatform(t *testing.T) { + // Create service with full platform specification + service, _, mockDestination, mockRegClient := setupServiceWithConfig([]string{"linux/amd64"}) + + // Test context + ctx := context.Background() + localRepo := "test-repo" + remoteRef := ref.Ref{ + Scheme: "docker", + Registry: "remote-registry.com", + Repository: "repo", + Tag: "latest", + } + localRef := ref.Ref{ + Scheme: "docker", + Registry: "localhost", + Repository: "test-repo", + Tag: "latest", + } + digest := godigest.FromString("test") + + // Setup expectation for CanSkipImage + mockDestination.On("CanSkipImage", localRepo, "latest", digest).Return(false, nil) + + t.Run("MatchingPlatform", func(t *testing.T) { + // Create a single-arch manifest with matching platform + manifest := createSingleArchManifest("linux", "amd64") + mockRegClient.On("ManifestGet", ctx, remoteRef).Return(manifest, nil).Once() + + // Expect ImageCopy to be called since the platform matches + mockRegClient.On("ImageCopy", ctx, remoteRef, localRef, mock.Anything).Return(nil).Once() + + // Run the test + err := service.syncRef(ctx, localRepo, remoteRef, localRef, digest, false) + + // Verify expectations + assert.NoError(t, err) + mockRegClient.AssertCalled(t, "ImageCopy", ctx, remoteRef, localRef, mock.Anything) + }) + + t.Run("MatchingArchDifferentOS", func(t *testing.T) { + // Create a single-arch manifest with matching arch but different OS + manifest := createSingleArchManifest("windows", "amd64") + mockRegClient.On("ManifestGet", ctx, remoteRef).Return(manifest, nil).Once() + + // ImageCopy should not be called since the OS doesn't match + + // Run the test + err := service.syncRef(ctx, localRepo, remoteRef, localRef, digest, false) + + // Verify expectations + assert.NoError(t, err) + // Check that ImageCopy was NOT called again + mockRegClient.AssertNumberOfCalls(t, "ImageCopy", 1) // Still 1 from the previous subtest + }) + + t.Run("NonMatchingPlatform", func(t *testing.T) { + // Create a single-arch manifest with non-matching platform + manifest := createSingleArchManifest("linux", "arm64") + mockRegClient.On("ManifestGet", ctx, remoteRef).Return(manifest, nil).Once() + + // ImageCopy should not be called since the platform doesn't match + + // Run the test + err := service.syncRef(ctx, localRepo, remoteRef, localRef, digest, false) + + // Verify expectations + assert.NoError(t, err) + // Check that ImageCopy was NOT called again + mockRegClient.AssertNumberOfCalls(t, "ImageCopy", 1) // Still 1 from the previous subtest + }) +} + +func TestSyncRefWithMultiplePlatforms(t *testing.T) { + // Create service with multiple platforms + service, _, mockDestination, mockRegClient := setupServiceWithConfig([]string{"amd64", "linux/arm64"}) + + // Test context + ctx := context.Background() + localRepo := "test-repo" + remoteRef := ref.Ref{ + Scheme: "docker", + Registry: "remote-registry.com", + Repository: "repo", + Tag: "latest", + } + localRef := ref.Ref{ + Scheme: "docker", + Registry: "localhost", + Repository: "test-repo", + Tag: "latest", + } + digest := godigest.FromString("test") + + // Setup expectation for CanSkipImage + mockDestination.On("CanSkipImage", localRepo, "latest", digest).Return(false, nil) + + t.Run("AnyOSWithAMD64", func(t *testing.T) { + // Create a single-arch manifest with amd64 on Windows (should be included) + manifest := createSingleArchManifest("windows", "amd64") + mockRegClient.On("ManifestGet", ctx, remoteRef).Return(manifest, nil).Once() + + // Expect ImageCopy to be called since amd64 matches regardless of OS + mockRegClient.On("ImageCopy", ctx, remoteRef, localRef, mock.Anything).Return(nil).Once() + + // Run the test + err := service.syncRef(ctx, localRepo, remoteRef, localRef, digest, false) + + // Verify expectations + assert.NoError(t, err) + mockRegClient.AssertCalled(t, "ImageCopy", ctx, remoteRef, localRef, mock.Anything) + }) + + t.Run("LinuxARM64", func(t *testing.T) { + // Create a single-arch manifest with Linux/arm64 (should be included) + manifest := createSingleArchManifest("linux", "arm64") + mockRegClient.On("ManifestGet", ctx, remoteRef).Return(manifest, nil).Once() + + // Expect ImageCopy to be called since linux/arm64 matches + mockRegClient.On("ImageCopy", ctx, remoteRef, localRef, mock.Anything).Return(nil).Once() + + // Run the test + err := service.syncRef(ctx, localRepo, remoteRef, localRef, digest, false) + + // Verify expectations + assert.NoError(t, err) + mockRegClient.AssertCalled(t, "ImageCopy", ctx, remoteRef, localRef, mock.Anything) + mockRegClient.AssertNumberOfCalls(t, "ImageCopy", 2) // Now 2 calls total + }) + + t.Run("WindowsARM64", func(t *testing.T) { + // Create a single-arch manifest with Windows/arm64 (should be excluded) + manifest := createSingleArchManifest("windows", "arm64") + mockRegClient.On("ManifestGet", ctx, remoteRef).Return(manifest, nil).Once() + + // ImageCopy should not be called since only linux/arm64 matches, not windows/arm64 + + // Run the test + err := service.syncRef(ctx, localRepo, remoteRef, localRef, digest, false) + + // Verify expectations + assert.NoError(t, err) + // Check that ImageCopy was NOT called again + mockRegClient.AssertNumberOfCalls(t, "ImageCopy", 2) // Still 2 from previous subtests + }) +} + +func TestSyncRefWithMultiArchImage(t *testing.T) { + // Various platform configurations to test + testCases := []struct { + name string + configPlatforms []string + platforms []platform.Platform + expectedCalls int + }{ + { + name: "NoFilteringConfigured", + configPlatforms: []string{}, + platforms: []platform.Platform{ + {OS: "linux", Architecture: "amd64"}, + {OS: "linux", Architecture: "arm64"}, + {OS: "windows", Architecture: "amd64"}, + }, + expectedCalls: 1, // Should copy the whole manifest list + }, + { + name: "SingleArchFilter", + configPlatforms: []string{"amd64"}, + platforms: []platform.Platform{ + {OS: "linux", Architecture: "amd64"}, + {OS: "linux", Architecture: "arm64"}, + {OS: "windows", Architecture: "amd64"}, + }, + expectedCalls: 1, // Should still copy once and filtering happens in destination + }, + { + name: "FullPlatformFilter", + configPlatforms: []string{"linux/amd64"}, + platforms: []platform.Platform{ + {OS: "linux", Architecture: "amd64"}, + {OS: "linux", Architecture: "arm64"}, + {OS: "windows", Architecture: "amd64"}, + }, + expectedCalls: 1, // Should still copy once and filtering happens in destination + }, + { + name: "MultiplePlatformFilter", + configPlatforms: []string{"linux/amd64", "linux/arm64"}, + platforms: []platform.Platform{ + {OS: "linux", Architecture: "amd64"}, + {OS: "linux", Architecture: "arm64"}, + {OS: "windows", Architecture: "amd64"}, + }, + expectedCalls: 1, // Should still copy once and filtering happens in destination + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Create service with the configured platforms + service, _, mockDestination, mockRegClient := setupServiceWithConfig(tc.configPlatforms) + + // Test context + ctx := context.Background() + localRepo := "test-repo" + remoteRef := ref.Ref{ + Scheme: "docker", + Registry: "remote-registry.com", + Repository: "repo", + Tag: "latest", + } + localRef := ref.Ref{ + Scheme: "docker", + Registry: "localhost", + Repository: "test-repo", + Tag: "latest", + } + digest := godigest.FromString("test") + + // Setup expectation for CanSkipImage + mockDestination.On("CanSkipImage", localRepo, "latest", digest).Return(false, nil).Once() + + // Create a multi-arch manifest + manifest := createMultiArchManifest(tc.platforms) + mockRegClient.On("ManifestGet", ctx, remoteRef).Return(manifest, nil).Once() + + // Expect ImageCopy to be called + mockRegClient.On("ImageCopy", ctx, remoteRef, localRef, mock.Anything).Return(nil).Once() + + // Run the test + err := service.syncRef(ctx, localRepo, remoteRef, localRef, digest, false) + + // Verify expectations + assert.NoError(t, err) + mockRegClient.AssertNumberOfCalls(t, "ImageCopy", tc.expectedCalls) + }) + } +} + +func TestSyncRefWithImageAlreadySynced(t *testing.T) { + // Create service with no platforms configured + service, _, mockDestination, mockRegClient := setupServiceWithConfig([]string{}) + + // Test context + ctx := context.Background() + localRepo := "test-repo" + remoteRef := ref.Ref{ + Scheme: "docker", + Registry: "remote-registry.com", + Repository: "repo", + Tag: "latest", + } + localRef := ref.Ref{ + Scheme: "docker", + Registry: "localhost", + Repository: "test-repo", + Tag: "latest", + } + digest := godigest.FromString("test") + + // Setup expectation for CanSkipImage - return true to indicate image already synced + mockDestination.On("CanSkipImage", localRepo, "latest", digest).Return(true, nil) + + // ManifestGet and ImageCopy should not be called + + // Run the test + err := service.syncRef(ctx, localRepo, remoteRef, localRef, digest, false) + + // Verify expectations + assert.NoError(t, err) + mockRegClient.AssertNotCalled(t, "ManifestGet") + mockRegClient.AssertNotCalled(t, "ImageCopy") +} + +func TestSyncRefCanSkipImageError(t *testing.T) { + // Create service with no platforms configured + service, _, mockDestination, _ := setupServiceWithConfig([]string{}) + + // Test context + ctx := context.Background() + localRepo := "test-repo" + remoteRef := ref.Ref{ + Scheme: "docker", + Registry: "remote-registry.com", + Repository: "repo", + Tag: "latest", + } + localRef := ref.Ref{ + Scheme: "docker", + Registry: "localhost", + Repository: "test-repo", + Tag: "latest", + } + digest := godigest.FromString("test") + + // Setup expectation for CanSkipImage - return error + testError := fmt.Errorf("test error") + mockDestination.On("CanSkipImage", localRepo, "latest", digest).Return(false, testError) + + // Run the test + err := service.syncRef(ctx, localRepo, remoteRef, localRef, digest, false) + + // Verify expectations + assert.Equal(t, testError, err) +} diff --git a/test/blackbox/sync_platforms.bats b/test/blackbox/sync_platforms.bats index 47934369c..ea9f8d039 100644 --- a/test/blackbox/sync_platforms.bats +++ b/test/blackbox/sync_platforms.bats @@ -783,4 +783,94 @@ EOF [ "$arm_v7_count" -gt 0 ] [ "$arm_v8_count" -eq 0 ] fi +} + +# Test for comprehensive platform filtering with all three formats combined +@test "sync with comprehensive platform format combinations" { + # Get ports for our registries + zot_source_port=$(cat ${BATS_FILE_TMPDIR}/zot.source.port) + + # Create a registry with a comprehensive mix of all platform filter types + local zot_comprehensive_dir=${BATS_FILE_TMPDIR}/zot-comprehensive + local zot_comprehensive_config=${BATS_FILE_TMPDIR}/zot_comprehensive_config.json + mkdir -p ${zot_comprehensive_dir} + + zot_comprehensive_port=$(get_free_port) + echo ${zot_comprehensive_port} > ${BATS_FILE_TMPDIR}/zot.comprehensive.port + + # Create config with a comprehensive mix of platform specifications: + # - Architecture-only format: "amd64" + # - OS/Architecture format: "linux/arm64" + # - OS/Architecture/Variant format: "linux/arm/v7" + cat >${zot_comprehensive_config} <