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.

Sign up for GitHub

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

Jump to bottom

fix skopeo copy can't decrypt to docker-daemon image #1604

Closed
wants to merge 1 commit into from
Closed

fix skopeo copy can't decrypt to docker-daemon image #1604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 15 additions & 7 deletions internal/image/docker_schema1.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,23 @@ func (m *manifestSchema1) UpdatedImageNeedsLayerDiffIDs(options types.ManifestUp

// UpdatedImage returns a types.Image modified according to options.
// This does not change the state of the original Image object.
func (m *manifestSchema1) UpdatedImage(ctx context.Context, options types.ManifestUpdateOptions) (types.Image, error) {
func (m *manifestSchema1) UpdatedImage(ctx context.Context, options types.ManifestUpdateOptions) (image types.Image, retErr error) {
copy := manifestSchema1{m: manifest.Schema1Clone(m.m)}

manifestTmp := m
defer func() {
if retErr != nil {
m = manifestTmp
}
}()

// No conversion required, update manifest
if options.LayerInfos != nil {
if err := copy.m.UpdateLayerInfos(options.LayerInfos); err != nil {
return nil, err
}
}

// We have 2 MIME types for schema 1, which are basically equivalent (even the un-"Signed" MIME type will be rejected if there isn’t a signature; so,
// handle conversions between them by doing nothing.
if options.ManifestMIMEType != manifest.DockerV2Schema1MediaType && options.ManifestMIMEType != manifest.DockerV2Schema1SignedMediaType {
Expand All @@ -124,12 +138,6 @@ func (m *manifestSchema1) UpdatedImage(ctx context.Context, options types.Manife
}
}

// No conversion required, update manifest
if options.LayerInfos != nil {
if err := copy.m.UpdateLayerInfos(options.LayerInfos); err != nil {
return nil, err
}
}
if options.EmbeddedDockerReference != nil {
copy.m.Name = reference.Path(options.EmbeddedDockerReference)
if tagged, isTagged := options.EmbeddedDockerReference.(reference.NamedTagged); isTagged {
Expand Down
22 changes: 15 additions & 7 deletions internal/image/docker_schema2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,13 +161,27 @@ func (m *manifestSchema2) UpdatedImageNeedsLayerDiffIDs(options types.ManifestUp
// The returned error will be a manifest.ManifestLayerCompressionIncompatibilityError
// if the CompressionOperation and CompressionAlgorithm specified in one or more
// options.LayerInfos items is anything other than gzip.
func (m *manifestSchema2) UpdatedImage(ctx context.Context, options types.ManifestUpdateOptions) (types.Image, error) {
func (m *manifestSchema2) UpdatedImage(ctx context.Context, options types.ManifestUpdateOptions) (image types.Image, retErr error) {
copy := manifestSchema2{ // NOTE: This is not a deep copy, it still shares slices etc.
src: m.src,
configBlob: m.configBlob,
m: manifest.Schema2Clone(m.m),
}

manifestTmp := m
defer func() {
if retErr != nil {
m = manifestTmp
}
}()

// No conversion required, update manifest
if options.LayerInfos != nil {
if err := copy.m.UpdateLayerInfos(options.LayerInfos); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

As mentioned previously, this would break one-step ”convert to OCI and encrypt”.

We need to split that, and do the decryption MIME type changes before format conversions, and encryption MIME type changes after format conversions.

return nil, err
}
}

converted, err := convertManifestIfRequiredWithUpdate(ctx, options, map[string]manifestConvertFn{
manifest.DockerV2Schema1MediaType: copy.convertToManifestSchema1,
manifest.DockerV2Schema1SignedMediaType: copy.convertToManifestSchema1,
Expand All @@ -181,12 +195,6 @@ func (m *manifestSchema2) UpdatedImage(ctx context.Context, options types.Manife
return converted, nil
}

// No conversion required, update manifest
if options.LayerInfos != nil {
if err := copy.m.UpdateLayerInfos(options.LayerInfos); err != nil {
return nil, err
}
}
// Ignore options.EmbeddedDockerReference: it may be set when converting from schema1 to schema2, but we really don't care.

return memoryImageFromManifest(&copy), nil
Expand Down
22 changes: 14 additions & 8 deletions internal/image/oci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,26 @@ func (m *manifestOCI1) UpdatedImageNeedsLayerDiffIDs(options types.ManifestUpdat
// if the combination of CompressionOperation and CompressionAlgorithm specified
// in one or more options.LayerInfos items indicates that a layer is compressed using
// an algorithm that is not allowed in OCI.
func (m *manifestOCI1) UpdatedImage(ctx context.Context, options types.ManifestUpdateOptions) (types.Image, error) {
func (m *manifestOCI1) UpdatedImage(ctx context.Context, options types.ManifestUpdateOptions) (image types.Image, retErr error) {
copy := manifestOCI1{ // NOTE: This is not a deep copy, it still shares slices etc.
src: m.src,
configBlob: m.configBlob,
m: manifest.OCI1Clone(m.m),
}

manifestTmp := m
defer func() {
if retErr != nil {
m = manifestTmp
}
}()
Comment on lines +153 to +158
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don’t understand at all what this is intended to do. m is local to this function, and just a pointer. Assigning to it, AFAICS, does nothing visible to the caller.


// No conversion required, update manifest
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment is not correct an this place.

if options.LayerInfos != nil {
if err := copy.m.UpdateLayerInfos(options.LayerInfos); err != nil {
return nil, err
}
}
converted, err := convertManifestIfRequiredWithUpdate(ctx, options, map[string]manifestConvertFn{
manifest.DockerV2Schema2MediaType: copy.convertToManifestSchema2Generic,
manifest.DockerV2Schema1MediaType: copy.convertToManifestSchema1,
Expand All @@ -162,13 +175,6 @@ func (m *manifestOCI1) UpdatedImage(ctx context.Context, options types.ManifestU
if converted != nil {
return converted, nil
}

// No conversion required, update manifest
if options.LayerInfos != nil {
if err := copy.m.UpdateLayerInfos(options.LayerInfos); err != nil {
return nil, err
}
}
// Ignore options.EmbeddedDockerReference: it may be set when converting from schema1, but we really don't care.

return memoryImageFromManifest(&copy), nil
Expand Down