Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-5.26] backport API changes copy and list components from upstream. #2071

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
29d86b6
list: ListUpdate add Platform field
flouthoc Jul 6, 2023
85a92ce
blob: TryReusingBlobWithOptions consider requiredCompression if set
flouthoc Jul 2, 2023
76f7a38
helpers_test,cleanup: correct argument order
flouthoc Jul 13, 2023
45c50cf
Fix TestOCI1IndexChooseInstanceByCompression on non-amd64
mtrmac Jul 15, 2023
668ff9b
listupdate,oci: instance show read-only annotations and CompressionAl…
flouthoc Jul 14, 2023
27e00a8
Remove a comment
mtrmac Jul 18, 2023
3ec1d30
Add *copy.Options to copier
mtrmac Jul 18, 2023
906e173
Use copier.options in setupSigners
mtrmac Jul 18, 2023
71fabfb
Use copier.options in sourceSignatures
mtrmac Jul 18, 2023
426ed94
Make compareImageDestinationManifestEqual a method on imageCopier
mtrmac Jul 18, 2023
d720db5
Use copier.options in copySingleImage
mtrmac Jul 18, 2023
1e0dafd
Use copier.options in copyMultipleImages
mtrmac Jul 18, 2023
34bb3d7
Consistently use c.options in copy.Image after it is set
mtrmac Jul 18, 2023
1f912a3
Eliminate copier.progress and progressInterval
mtrmac Jul 19, 2023
b82a12f
Eliminate copier.ociDecryptConfig
mtrmac Jul 19, 2023
9f5ec36
Eliminate copier.ociEncryptConfig
mtrmac Jul 19, 2023
5d09be4
Eliminate copier.downloadForeignLayers
mtrmac Jul 19, 2023
9bdb796
Eliminate imageCopier.ociEncryptLayers
mtrmac Jul 19, 2023
2f9d323
Introduce copier.unparsedToplevel
mtrmac Jul 19, 2023
5c88283
Use copier.unparsedToplevel in copySingleImage
mtrmac Jul 19, 2023
41b8377
Use c.unparsedToplevel in copyMultipleImages
mtrmac Jul 19, 2023
3abf692
Eliminate the unparsedToplevel variable in copy.Image
mtrmac Jul 19, 2023
96446da
Add policyContext to copier
mtrmac Jul 19, 2023
e3482e5
Use copier.policyContext in copySingleImage
mtrmac Jul 19, 2023
76b14a9
Remove the policyContext parameter from copyMultipleImages
mtrmac Jul 19, 2023
1154668
Return a new struct copySingleImageResult from copySingleImage
mtrmac Jul 19, 2023
171a085
Have compareImageDestinationManifestEqual return a *copySingleImageRe…
mtrmac Jul 19, 2023
6d08bb4
Track the manifest upload data in a wipResult variable
mtrmac Jul 19, 2023
c459834
oci_index: init Annotations if nil and required
flouthoc Jul 18, 2023
924b778
copy/single: set requiredCompression if compressionFormat is changed
flouthoc Jul 18, 2023
c37c390
copy/single: accept requireCompressionFormatMatch for imagecopier
flouthoc Jul 20, 2023
61ac58f
copy/multiple: instanceCopyCopy honor UpdateCompressionAlgorithms
flouthoc Jul 18, 2023
d86b8db
internal/set: verify if no duplicates in set
flouthoc Jul 20, 2023
63deaa7
copy/single: wrap arguments in copySingleImageOptions
flouthoc Jul 21, 2023
6272732
copy/single: add custom compressionFormat, compressionLevel fields
flouthoc Jul 21, 2023
cb28b83
copy/single: honor c.options.DestCtx for regular copy
flouthoc Jul 21, 2023
09ac80a
copy: add EnsureCompressionVariantExist for instanceCopyClone
flouthoc Jul 21, 2023
4536074
internal/set: implement AddSlice for easier syntax
flouthoc Jul 21, 2023
ed1cb97
copy/multiple: implement instanceCopyClone
flouthoc Jul 21, 2023
e2551af
copy/multiple: instanceCopyClone set custom compression
flouthoc Jul 22, 2023
d392ed7
copy/multiple: error on EnsureCompressionVariant and CopySpecificImages
flouthoc Jul 22, 2023
0aaab65
copy/multiple: report more meaningful copy msg
flouthoc Jul 22, 2023
3b5d09e
copy/multiple_test: retrofit tests for instanceCopyCopy
flouthoc Jul 22, 2023
3214de2
copy/multiple_test: implement test for instanceCopyClone with helpers
flouthoc Jul 22, 2023
e4749bd
copy/multiple_test: multiple copy requests of same compression
flouthoc Jul 25, 2023
4c97ed8
copy/multiple_test: clone must happen once for duplicate entries
flouthoc Jul 25, 2023
ca3c953
copy/copy: fail copySingleImage cases on EnsureCompressionVariantsExist
flouthoc Jul 25, 2023
d932df1
copy/multiple: priority of instanceCopyCopy must be higher
flouthoc Jul 27, 2023
c7f3872
.cirrus: lock skopeo to stable version and correct dest branch
flouthoc Aug 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
#### Global variables used for all tasks
####
# Name of the ultimate destination branch for this CI run
DEST_BRANCH: "main"
DEST_BRANCH: "release-5.26"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great catch!

# CI container image tag (c/skopeo branch name)
SKOPEO_CI_TAG: "main"
SKOPEO_CI_TAG: "v1.13.0"
mtrmac marked this conversation as resolved.
Show resolved Hide resolved
# Use GO module mirror (reason unknown, travis did it this way)
GOPROXY: https://proxy.golang.org
# Overrides default location (/tmp/cirrus) for repo clone
Expand Down Expand Up @@ -78,7 +78,7 @@
script: make cross


test_task:

Check warning on line 81 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L81

task "Test" depends on task "validate", but their only_if conditions are different

Check warning on line 81 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L81

task "Test w/ opengpg" depends on task "validate", but their only_if conditions are different

Check warning on line 81 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L81

task "Test" depends on task "validate", but their only_if conditions are different

Check warning on line 81 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L81

task "Test w/ opengpg" depends on task "validate", but their only_if conditions are different
alias: test
depends_on:
- validate
Expand All @@ -99,7 +99,7 @@
##### repository's `.cirrus.yml`. Changes made here should be fully merged
##### prior to being manually duplicated and maintained in containers/skopeo.
#####
test_skopeo_task:

Check warning on line 102 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L102

task "Skopeo Test" depends on task "validate", but their only_if conditions are different

Check warning on line 102 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L102

task "Skopeo Test w/ opengpg" depends on task "validate", but their only_if conditions are different

Check warning on line 102 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L102

task "Skopeo Test" depends on task "validate", but their only_if conditions are different

Check warning on line 102 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L102

task "Skopeo Test w/ opengpg" depends on task "validate", but their only_if conditions are different
alias: test_skopeo
only_if: *not_docs
depends_on:
Expand Down Expand Up @@ -161,7 +161,7 @@
# Status aggregator for all tests. This task simply ensures a defined
# set of tasks all passed, and allows confirming that based on the status
# of this task.
success_task:

Check warning on line 164 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L164

task "Total Success" depends on task "validate", but their only_if conditions are different

Check warning on line 164 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L164

task "Total Success" depends on task "cross", but their only_if conditions are different

Check warning on line 164 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L164

task "Total Success" depends on task "Test", but their only_if conditions are different

Check warning on line 164 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L164

task "Total Success" depends on task "Test w/ opengpg", but their only_if conditions are different

Check warning on line 164 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L164

task "Total Success" depends on task "Skopeo Test", but their only_if conditions are different

Check warning on line 164 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L164

task "Total Success" depends on task "Skopeo Test w/ opengpg", but their only_if conditions are different

Check warning on line 164 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L164

task "Total Success" depends on task "validate", but their only_if conditions are different

Check warning on line 164 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L164

task "Total Success" depends on task "cross", but their only_if conditions are different

Check warning on line 164 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L164

task "Total Success" depends on task "Test", but their only_if conditions are different

Check warning on line 164 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L164

task "Total Success" depends on task "Test w/ opengpg", but their only_if conditions are different

Check warning on line 164 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L164

task "Total Success" depends on task "Skopeo Test", but their only_if conditions are different

Check warning on line 164 in .cirrus.yml

View check run for this annotation

Cirrus CI / Build Parsing Results

.cirrus.yml#L164

task "Total Success" depends on task "Skopeo Test w/ opengpg", but their only_if conditions are different
name: "Total Success"
alias: success
# N/B: ALL tasks must be listed here, minus their '_task' suffix.
Expand Down
8 changes: 4 additions & 4 deletions copy/blob.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,12 @@ func (ic *imageCopier) copyBlobFromStream(ctx context.Context, srcReader io.Read
return types.BlobInfo{}, err
}

// === Report progress using the ic.c.progress channel, if required.
if ic.c.progress != nil && ic.c.progressInterval > 0 {
// === Report progress using the ic.c.options.Progress channel, if required.
if ic.c.options.Progress != nil && ic.c.options.ProgressInterval > 0 {
progressReader := newProgressReader(
stream.reader,
ic.c.progress,
ic.c.progressInterval,
ic.c.options.Progress,
ic.c.options.ProgressInterval,
srcInfo,
)
defer progressReader.reportDone()
Expand Down
107 changes: 63 additions & 44 deletions copy/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/containers/image/v5/internal/private"
"github.com/containers/image/v5/manifest"
"github.com/containers/image/v5/pkg/blobinfocache"
compression "github.com/containers/image/v5/pkg/compression/types"
"github.com/containers/image/v5/signature"
"github.com/containers/image/v5/signature/signer"
"github.com/containers/image/v5/transports"
Expand Down Expand Up @@ -126,36 +127,46 @@ type Options struct {
// Download layer contents with "nondistributable" media types ("foreign" layers) and translate the layer media type
// to not indicate "nondistributable".
DownloadForeignLayers bool

// Contains slice of OptionCompressionVariant, where copy will ensure that for each platform
// in the manifest list, a variant with the requested compression will exist.
// Invalid when copying a non-multi-architecture image. That will probably
// change in the future.
EnsureCompressionVariantsExist []OptionCompressionVariant
}

// OptionCompressionVariant allows to supply information about
// selected compression algorithm and compression level by the
// end-user. Refer to EnsureCompressionVariantsExist to know
// more about its usage.
type OptionCompressionVariant struct {
Algorithm compression.Algorithm
Level *int // Only used when we are creating a new image instance using the specified algorithm, not when the image already contains such an instance
}

// copier allows us to keep track of diffID values for blobs, and other
// data shared across one or more images in a possible manifest list.
// The owner must call close() when done.
type copier struct {
dest private.ImageDestination
rawSource private.ImageSource
reportWriter io.Writer
progressOutput io.Writer
progressInterval time.Duration
progress chan types.ProgressProperties
policyContext *signature.PolicyContext
dest private.ImageDestination
rawSource private.ImageSource
options *Options // never nil

reportWriter io.Writer
progressOutput io.Writer

unparsedToplevel *image.UnparsedImage // for rawSource
blobInfoCache internalblobinfocache.BlobInfoCache2
ociDecryptConfig *encconfig.DecryptConfig
ociEncryptConfig *encconfig.EncryptConfig
concurrentBlobCopiesSemaphore *semaphore.Weighted // Limits the amount of concurrently copied blobs
downloadForeignLayers bool
signers []*signer.Signer // Signers to use to create new signatures for the image
signersToClose []*signer.Signer // Signers that should be closed when this copier is destroyed.
signers []*signer.Signer // Signers to use to create new signatures for the image
signersToClose []*signer.Signer // Signers that should be closed when this copier is destroyed.
}

// Image copies image from srcRef to destRef, using policyContext to validate
// source image admissibility. It returns the manifest which was written to
// the new copy of the image.
func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef, srcRef types.ImageReference, options *Options) (copiedManifest []byte, retErr error) {
// NOTE this function uses an output parameter for the error return value.
// Setting this and returning is the ideal way to return an error.
//
// the defers in this routine will wrap the error return with its own errors
// which can be valuable context in the middle of a multi-streamed copy.
if options == nil {
options = &Options{}
}
Expand Down Expand Up @@ -209,96 +220,104 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef,
}

c := &copier{
dest: dest,
rawSource: rawSource,
reportWriter: reportWriter,
progressOutput: progressOutput,
progressInterval: options.ProgressInterval,
progress: options.Progress,
policyContext: policyContext,
dest: dest,
rawSource: rawSource,
options: options,

reportWriter: reportWriter,
progressOutput: progressOutput,

unparsedToplevel: image.UnparsedInstance(rawSource, nil),
// FIXME? The cache is used for sources and destinations equally, but we only have a SourceCtx and DestinationCtx.
// For now, use DestinationCtx (because blob reuse changes the behavior of the destination side more); eventually
// we might want to add a separate CommonCtx — or would that be too confusing?
blobInfoCache: internalblobinfocache.FromBlobInfoCache(blobinfocache.DefaultCache(options.DestinationCtx)),
ociDecryptConfig: options.OciDecryptConfig,
ociEncryptConfig: options.OciEncryptConfig,
downloadForeignLayers: options.DownloadForeignLayers,
blobInfoCache: internalblobinfocache.FromBlobInfoCache(blobinfocache.DefaultCache(options.DestinationCtx)),
}
defer c.close()

// Set the concurrentBlobCopiesSemaphore if we can copy layers in parallel.
if dest.HasThreadSafePutBlob() && rawSource.HasThreadSafeGetBlob() {
c.concurrentBlobCopiesSemaphore = options.ConcurrentBlobCopiesSemaphore
c.concurrentBlobCopiesSemaphore = c.options.ConcurrentBlobCopiesSemaphore
if c.concurrentBlobCopiesSemaphore == nil {
max := options.MaxParallelDownloads
max := c.options.MaxParallelDownloads
if max == 0 {
max = maxParallelDownloads
}
c.concurrentBlobCopiesSemaphore = semaphore.NewWeighted(int64(max))
}
} else {
c.concurrentBlobCopiesSemaphore = semaphore.NewWeighted(int64(1))
if options.ConcurrentBlobCopiesSemaphore != nil {
if err := options.ConcurrentBlobCopiesSemaphore.Acquire(ctx, 1); err != nil {
if c.options.ConcurrentBlobCopiesSemaphore != nil {
if err := c.options.ConcurrentBlobCopiesSemaphore.Acquire(ctx, 1); err != nil {
return nil, fmt.Errorf("acquiring semaphore for concurrent blob copies: %w", err)
}
defer options.ConcurrentBlobCopiesSemaphore.Release(1)
defer c.options.ConcurrentBlobCopiesSemaphore.Release(1)
}
}

if err := c.setupSigners(options); err != nil {
if err := c.setupSigners(); err != nil {
return nil, err
}

unparsedToplevel := image.UnparsedInstance(rawSource, nil)
multiImage, err := isMultiImage(ctx, unparsedToplevel)
multiImage, err := isMultiImage(ctx, c.unparsedToplevel)
if err != nil {
return nil, fmt.Errorf("determining manifest MIME type for %s: %w", transports.ImageName(srcRef), err)
}

if !multiImage {
if len(options.EnsureCompressionVariantsExist) > 0 {
return nil, fmt.Errorf("EnsureCompressionVariantsExist is not implemented when not creating a multi-architecture image")
}
// The simple case: just copy a single image.
if copiedManifest, _, _, err = c.copySingleImage(ctx, policyContext, options, unparsedToplevel, unparsedToplevel, nil); err != nil {
single, err := c.copySingleImage(ctx, c.unparsedToplevel, nil, copySingleImageOptions{requireCompressionFormatMatch: false})
if err != nil {
return nil, err
}
} else if options.ImageListSelection == CopySystemImage {
copiedManifest = single.manifest
} else if c.options.ImageListSelection == CopySystemImage {
if len(options.EnsureCompressionVariantsExist) > 0 {
return nil, fmt.Errorf("EnsureCompressionVariantsExist is not implemented when not creating a multi-architecture image")
}
// This is a manifest list, and we weren't asked to copy multiple images. Choose a single image that
// matches the current system to copy, and copy it.
mfest, manifestType, err := unparsedToplevel.Manifest(ctx)
mfest, manifestType, err := c.unparsedToplevel.Manifest(ctx)
if err != nil {
return nil, fmt.Errorf("reading manifest for %s: %w", transports.ImageName(srcRef), err)
}
manifestList, err := internalManifest.ListFromBlob(mfest, manifestType)
if err != nil {
return nil, fmt.Errorf("parsing primary manifest as list for %s: %w", transports.ImageName(srcRef), err)
}
instanceDigest, err := manifestList.ChooseInstanceByCompression(options.SourceCtx, options.PreferGzipInstances) // try to pick one that matches options.SourceCtx
instanceDigest, err := manifestList.ChooseInstanceByCompression(c.options.SourceCtx, c.options.PreferGzipInstances) // try to pick one that matches c.options.SourceCtx
if err != nil {
return nil, fmt.Errorf("choosing an image from manifest list %s: %w", transports.ImageName(srcRef), err)
}
logrus.Debugf("Source is a manifest list; copying (only) instance %s for current system", instanceDigest)
unparsedInstance := image.UnparsedInstance(rawSource, &instanceDigest)

if copiedManifest, _, _, err = c.copySingleImage(ctx, policyContext, options, unparsedToplevel, unparsedInstance, nil); err != nil {
single, err := c.copySingleImage(ctx, unparsedInstance, nil, copySingleImageOptions{requireCompressionFormatMatch: false})
if err != nil {
return nil, fmt.Errorf("copying system image from manifest list: %w", err)
}
} else { /* options.ImageListSelection == CopyAllImages or options.ImageListSelection == CopySpecificImages, */
copiedManifest = single.manifest
} else { /* c.options.ImageListSelection == CopyAllImages or c.options.ImageListSelection == CopySpecificImages, */
// If we were asked to copy multiple images and can't, that's an error.
if !supportsMultipleImages(c.dest) {
return nil, fmt.Errorf("copying multiple images: destination transport %q does not support copying multiple images as a group", destRef.Transport().Name())
}
// Copy some or all of the images.
switch options.ImageListSelection {
switch c.options.ImageListSelection {
case CopyAllImages:
logrus.Debugf("Source is a manifest list; copying all instances")
case CopySpecificImages:
logrus.Debugf("Source is a manifest list; copying some instances")
}
if copiedManifest, err = c.copyMultipleImages(ctx, policyContext, options, unparsedToplevel); err != nil {
if copiedManifest, err = c.copyMultipleImages(ctx); err != nil {
return nil, err
}
}

if err := c.dest.Commit(ctx, unparsedToplevel); err != nil {
if err := c.dest.Commit(ctx, c.unparsedToplevel); err != nil {
return nil, fmt.Errorf("committing the finished image: %w", err)
}

Expand Down
8 changes: 4 additions & 4 deletions copy/encryption.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type bpDecryptionStepData struct {
// srcInfo is only used for error messages.
// Returns data for other steps; the caller should eventually use updateCryptoOperation.
func (ic *imageCopier) blobPipelineDecryptionStep(stream *sourceStream, srcInfo types.BlobInfo) (*bpDecryptionStepData, error) {
if !isOciEncrypted(stream.info.MediaType) || ic.c.ociDecryptConfig == nil {
if !isOciEncrypted(stream.info.MediaType) || ic.c.options.OciDecryptConfig == nil {
return &bpDecryptionStepData{
decrypting: false,
}, nil
Expand All @@ -47,7 +47,7 @@ func (ic *imageCopier) blobPipelineDecryptionStep(stream *sourceStream, srcInfo
desc := imgspecv1.Descriptor{
Annotations: stream.info.Annotations,
}
reader, decryptedDigest, err := ocicrypt.DecryptLayer(ic.c.ociDecryptConfig, stream.reader, desc, false)
reader, decryptedDigest, err := ocicrypt.DecryptLayer(ic.c.options.OciDecryptConfig, stream.reader, desc, false)
if err != nil {
return nil, fmt.Errorf("decrypting layer %s: %w", srcInfo.Digest, err)
}
Expand Down Expand Up @@ -81,7 +81,7 @@ type bpEncryptionStepData struct {
// Returns data for other steps; the caller should eventually call updateCryptoOperationAndAnnotations.
func (ic *imageCopier) blobPipelineEncryptionStep(stream *sourceStream, toEncrypt bool, srcInfo types.BlobInfo,
decryptionStep *bpDecryptionStepData) (*bpEncryptionStepData, error) {
if !toEncrypt || isOciEncrypted(srcInfo.MediaType) || ic.c.ociEncryptConfig == nil {
if !toEncrypt || isOciEncrypted(srcInfo.MediaType) || ic.c.options.OciEncryptConfig == nil {
return &bpEncryptionStepData{
encrypting: false,
}, nil
Expand All @@ -101,7 +101,7 @@ func (ic *imageCopier) blobPipelineEncryptionStep(stream *sourceStream, toEncryp
Size: srcInfo.Size,
Annotations: annotations,
}
reader, finalizer, err := ocicrypt.EncryptLayer(ic.c.ociEncryptConfig, stream.reader, desc)
reader, finalizer, err := ocicrypt.EncryptLayer(ic.c.options.OciEncryptConfig, stream.reader, desc)
if err != nil {
return nil, fmt.Errorf("encrypting blob %s: %w", srcInfo.Digest, err)
}
Expand Down
Loading