Cli: manifest subcommand creates invalid manifest list

Created on 20 Jun 2018 · 7Comments · Source: docker/cli

Currently creating manifest lists from the experimental manifest subcommand uses an incorrect for the manifest, creating invalid manifests. These manifests fail to be pullable with containerd since containerd validates the size. This has lead to broken images being pushed to registries.

See conversation from https://github.com/containerd/containerd/issues/2401

aredistribution kinbug kinexperimental

Source

dmcgowan

All 7 comments

From looking at the code the issue seems to be that the original manifest is not stored in the original form and instead it is reserialized inside a new json object ImageManifest. Even though it uses Payload() function and variables suggests the size is taken over raw data it has actually been remarshalled in the disk already and the original form may be lost.

tonistiigi on 20 Jun 2018

👍1

ping @clnperez @estesp PTAL

thaJeztah on 20 Jun 2018

Odd. There was an issue (that I can't find now) about this. And I went to a lot of trouble to get it back to looking exactly the way it looked originally. The issue had to do with the manifest changing (just the tabs in it), and so the hash was different. If anyone has that in their history or inbox please link.

@dmcgowan what version of the cli do you have?

clnperez on 20 Jun 2018

Ah, hidden in collapsed history. Starting here: hinshun commented on Nov 2, 2017

So it should be fixed in the original, but is not, apparently. I'll take a look.

clnperez on 20 Jun 2018

Downstream impact includes a broken CoreDNS for Kubernetes release at https://github.com/kubernetes/kubernetes/issues/65253

which is marked as priority/critical-urgent in that repo.

vielmetti on 22 Jun 2018

Answering https://github.com/containerd/containerd/issues/2401#issuecomment-399574048

@tonistiigi so you're right if you restrict yourself exclusively to code comments of the exported functions/fields. But then, only 3 lines below, this can be confusing:
https://github.com/docker/distribution/blob/749f6afb4572201e3c37325d0ffedb6f32be8950/manifest/schema2/manifest.go#L92-L93

Anyway, the current manifest code seems to need the unmarshalled schema2.DeserializedManifest, in order to perform the blob mount requests to the registry. So carrying an immutable blob of the struct + the struct would be redundant with what's already present indocker/distribution.