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

v1.Manifest is no longer compatible with docker

Closed
vishvananda opened this issue Feb 9, 2017 · 2 comments
Closed

v1.Manifest is no longer compatible with docker #562

vishvananda opened this issue Feb 9, 2017 · 2 comments

Comments

@vishvananda
Copy link
Contributor

vishvananda commented Feb 9, 2017

I think this was intentional, but I wanted to verify since this changed between rc3 and rc4.

Now that v1.Manifest no longer has a self describing MediaType associated with it, it is incompatible with the docker layout for manifest. This means converting back and forth is a bit trickier. Given an oci manifest origManifest, converting it to docker format (so you can upload it into a v2 registry, for example), used to look like:

manifest := origManifest
manifest.mediaType = "application/vnd.docker.distribution.manifest.v2+json"
manifest.Config.MediaType = "application/vnd.docker.container.image.v1+json"
for _, l := range manifest.Layers {
    layer.MediaType =  "application/vnd.docker.image.rootfs.diff.tar.gzip"
}

Now it is necessary to wrap the manifest in a new version that has the removed field. For example:

type maybeDockerManifest struct {
    v1.Manifest
    MediaType string `json:"mediaType,omitempty"`
}
manifest := maybeDockerManifest{Manifest: origManifest}
manifest.mediaType = "application/vnd.docker.distribution.manifest.v2+json"
manifest.Config.MediaType = "application/vnd.docker.container.image.v1+json"
for _, l := range manifest.Layers {
    layer.MediaType =  "application/vnd.docker.image.rootfs.diff.tar.gzip"
}

This isn't hugely painful, but I wanted to make sure the ramifications of changing that field and breaking compatiblity are understood.

@vbatts
Copy link
Member

vbatts commented Feb 9, 2017

@stevvooe what's your position on the mediaType field not being present in this files (being marked as reserved)

@stevvooe
Copy link
Contributor

stevvooe commented Feb 9, 2017

This removal is intentional. The field has only served to cause massive confusion, of which this issue is an example. The format was never meant to be self-describing nor is it so support autodetect. Doing so is both insecure and broken. To understand this, compare the usage of the mediatype field you are pointing out with the descriptor usage around it. It is not supposed to be there.

The main goal we want with compatibility is to remove the need to rewrite layers, which is preserved. The rest of the metadata, manifests and config will undergo a slight change, but that should cause any issues and should largely be transparent.

@stevvooe stevvooe closed this as completed Feb 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants