-
Notifications
You must be signed in to change notification settings - Fork 395
Fix for image spec #223
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
Fix for image spec #223
Changes from all commits
7f03b77
6bb5100
4f6f739
6a51d25
3f53928
707ca5d
18949fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,17 @@ func newImageSource(ctx *types.SystemContext, ref dockerReference, requestedMani | |
| if requestedManifestMIMETypes == nil { | ||
| requestedManifestMIMETypes = manifest.DefaultRequestedManifestMIMETypes | ||
| } | ||
| supportedMIMEs := supportedManifestMIMETypesMap() | ||
| acceptableRequestedMIMEs := false | ||
| for _, mtrequested := range requestedManifestMIMETypes { | ||
| if supportedMIMEs[mtrequested] { | ||
| acceptableRequestedMIMEs = true | ||
| break | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would ignore the input value if any value is unrecognized; I think it should ignore input only if all values are unrecognized (so that a caller can say “i prefer v2s1, then OCI, then v2s2 as a last resort”.) |
||
| } | ||
| } | ||
| if !acceptableRequestedMIMEs { | ||
| requestedManifestMIMETypes = manifest.DefaultRequestedManifestMIMETypes | ||
| } | ||
| return &dockerImageSource{ | ||
| ref: ref, | ||
| requestedManifestMIMETypes: requestedManifestMIMETypes, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ import ( | |
| "github.com/containers/image/manifest" | ||
| "github.com/containers/image/types" | ||
| "github.com/opencontainers/go-digest" | ||
| imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" | ||
| "github.com/pkg/errors" | ||
| ) | ||
|
|
||
|
|
@@ -166,13 +167,40 @@ func (m *manifestSchema2) UpdatedImage(options types.ManifestUpdateOptions) (typ | |
| case "": // No conversion, OK | ||
| case manifest.DockerV2Schema1SignedMediaType, manifest.DockerV2Schema1MediaType: | ||
| return copy.convertToManifestSchema1(options.InformationOnly.Destination) | ||
| case imgspecv1.MediaTypeImageManifest: | ||
| return copy.convertToManifestOCI1() | ||
| default: | ||
| return nil, errors.Errorf("Conversion of image manifest from %s to %s is not implemented", manifest.DockerV2Schema2MediaType, options.ManifestMIMEType) | ||
| } | ||
|
|
||
| return memoryImageFromManifest(©), nil | ||
| } | ||
|
|
||
| func (m *manifestSchema2) convertToManifestOCI1() (types.Image, error) { | ||
| // Create a copy of the descriptor. | ||
| config := m.ConfigDescriptor | ||
|
|
||
| // The only difference between OCI and DockerSchema2 is the mediatypes. The | ||
| // media type of the manifest is handled by manifestSchema2FromComponents. | ||
| config.MediaType = imgspecv1.MediaTypeImageConfig | ||
|
|
||
| layers := make([]descriptor, len(m.LayersDescriptors)) | ||
| for idx := range layers { | ||
| layers[idx] = m.LayersDescriptors[idx] | ||
| if m.LayersDescriptors[idx].MediaType == manifest.DockerV2Schema2ForeignLayerMediaType { | ||
| layers[idx].MediaType = imgspecv1.MediaTypeImageLayerNonDistributable | ||
| } else { | ||
| layers[idx].MediaType = imgspecv1.MediaTypeImageLayerGzip | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This works for now (i.e. at it was perfectly valid a week ago), longer-term we should actually know whether the input is gzipped. Filed #229. |
||
| } | ||
| } | ||
|
|
||
| // Rather than copying the ConfigBlob now, we just pass m.src to the | ||
| // translated manifest, since the only difference is the mediatype of | ||
| // descriptors there is no change to any blob stored in m.src. | ||
| m1 := manifestOCI1FromComponents(config, m.src, nil, layers) | ||
| return memoryImageFromManifest(m1), nil | ||
| } | ||
|
|
||
| // Based on docker/distribution/manifest/schema1/config_builder.go | ||
| func (m *manifestSchema2) convertToManifestSchema1(dest types.ImageDestination) (types.Image, error) { | ||
| configBytes, err := m.ConfigBlob() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| { | ||
| "schemaVersion": 2, | ||
| "config": { | ||
| "mediaType": "application/vnd.oci.image.config.v1+json", | ||
| "size": 5940, | ||
| "digest": "sha256:9ca4bda0a6b3727a6ffcc43e981cad0f24e2ec79d338f6ba325b4dfd0756fb8f" | ||
| }, | ||
| "layers": [{ | ||
| "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", | ||
| "size": 51354364, | ||
| "digest": "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb" | ||
| }, { | ||
| "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", | ||
| "size": 150, | ||
| "digest": "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c" | ||
| }, { | ||
| "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", | ||
| "size": 11739507, | ||
| "digest": "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9" | ||
| }, { | ||
| "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", | ||
| "size": 8841833, | ||
| "digest": "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909" | ||
| }, { | ||
| "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", | ||
| "size": 291, | ||
| "digest": "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa" | ||
| }] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,7 +37,6 @@ func (d *ociImageDestination) Close() { | |
| func (d *ociImageDestination) SupportedManifestMIMETypes() []string { | ||
| return []string{ | ||
| imgspecv1.MediaTypeImageManifest, | ||
| manifest.DockerV2Schema2MediaType, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -134,60 +133,16 @@ func (d *ociImageDestination) ReapplyBlob(info types.BlobInfo) (types.BlobInfo, | |
| return info, nil | ||
| } | ||
|
|
||
| func createManifest(m []byte) ([]byte, string, error) { | ||
| om := imgspecv1.Manifest{} | ||
| mt := manifest.GuessMIMEType(m) | ||
| switch mt { | ||
| case manifest.DockerV2Schema1MediaType, manifest.DockerV2Schema1SignedMediaType: | ||
| // There a simple reason about not yet implementing this. | ||
| // OCI image-spec assure about backward compatibility with docker v2s2 but not v2s1 | ||
| // generating a v2s2 is a migration docker does when upgrading to 1.10.3 | ||
| // and I don't think we should bother about this now (I don't want to have migration code here in skopeo) | ||
| return nil, "", errors.New("can't create an OCI manifest from Docker V2 schema 1 manifest") | ||
| case manifest.DockerV2Schema2MediaType: | ||
| if err := json.Unmarshal(m, &om); err != nil { | ||
| return nil, "", err | ||
| } | ||
| om.MediaType = imgspecv1.MediaTypeImageManifest | ||
| for i, l := range om.Layers { | ||
| if l.MediaType == manifest.DockerV2Schema2ForeignLayerMediaType { | ||
| om.Layers[i].MediaType = imgspecv1.MediaTypeImageLayerNonDistributable | ||
| } else { | ||
| om.Layers[i].MediaType = imgspecv1.MediaTypeImageLayer | ||
| } | ||
| } | ||
| om.Config.MediaType = imgspecv1.MediaTypeImageConfig | ||
| b, err := json.Marshal(om) | ||
| if err != nil { | ||
| return nil, "", err | ||
| } | ||
| return b, om.MediaType, nil | ||
| case manifest.DockerV2ListMediaType: | ||
| return nil, "", errors.New("can't create an OCI manifest from Docker V2 schema 2 manifest list") | ||
| case imgspecv1.MediaTypeImageManifestList: | ||
| return nil, "", errors.New("can't create an OCI manifest from OCI manifest list") | ||
| case imgspecv1.MediaTypeImageManifest: | ||
| return m, mt, nil | ||
| } | ||
| return nil, "", errors.Errorf("unrecognized manifest media type %q", mt) | ||
| } | ||
|
|
||
| func (d *ociImageDestination) PutManifest(m []byte) error { | ||
| // TODO(mitr, runcom): this breaks signatures entirely since at this point we're creating a new manifest | ||
| // and signatures don't apply anymore. Will fix. | ||
| ociMan, mt, err := createManifest(m) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| digest, err := manifest.Digest(ociMan) | ||
| digest, err := manifest.Digest(m) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| desc := imgspecv1.Descriptor{} | ||
| desc.Digest = digest.String() | ||
| desc.Digest = digest | ||
| // TODO(runcom): beaware and add support for OCI manifest list | ||
| desc.MediaType = mt | ||
| desc.Size = int64(len(ociMan)) | ||
| desc.MediaType = imgspecv1.MediaTypeImageManifest | ||
| desc.Size = int64(len(m)) | ||
| data, err := json.Marshal(desc) | ||
| if err != nil { | ||
| return err | ||
|
|
@@ -197,7 +152,10 @@ func (d *ociImageDestination) PutManifest(m []byte) error { | |
| if err != nil { | ||
| return err | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I missed it somehow
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
| if err := ioutil.WriteFile(blobPath, ociMan, 0644); err != nil { | ||
| if err := ensureParentDirectoryExists(blobPath); err != nil { | ||
| return err | ||
| } | ||
| if err := ioutil.WriteFile(blobPath, m, 0644); err != nil { | ||
| return err | ||
| } | ||
| // TODO(runcom): ugly here? | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a
map[string]struct{}is more idiomatic, but I really don’t care enough.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arg, that's how it was, then changed my mind, I'll put it back with
struct{}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on a second though, I like comparing with just
!- we change this any time though...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, whatever works.