Move parsing layer digests from copy.go to types.Image #103
Conversation
|
|
||
| dest, err := parseImageDestination(context, context.Args()[1]) | ||
| if err != nil { | ||
| logrus.Fatalf("Error initializing %s: %s", context.Args()[1], err.Error()) | ||
| } | ||
| signBy := context.String("sign-by") | ||
|
|
||
| manifest, _, err := src.GetManifest([]string{utils.DockerV2Schema1MIMEType}) | ||
| manifest, err := src.Manifest() |
There was a problem hiding this comment.
if we use image here, how do we deal with mime type selection? eg. I need a v2s2 for OCI generally (can hack up something from v2s1 but it's ugly)
There was a problem hiding this comment.
We don’t, here.
AFAICS there aren’t any cases when the images has both v2s1 and v2s2 and we would want to use the v2s1 one; the “right” one (and also the one that matches a digest reference) is v2s2. Is that incorrect?
For signature verification to make sense, we really need a types.Image to be a single image; not a set of several possibly completely different images, with different methods and different parameters to these methods choosing any of them.
So I expect that types.Image would, for example:
- Prefer OCI to v2s2, and v2s2 to v2s1
- Given a single
ImageSourcewith the same contents, always choose the same version.
If we really needed to refer to the v2s1 manifest of a combined v2s1+v2s2 ImageSource, that would be a different types.Image object. (image.FromSourceWithSpecifiedMIMEType?) and very rarely used.
There was a problem hiding this comment.
alright got it. But Image(Source|Destination) need to handle different manifests because a registry does not assure you'll get what you asked but only what's supported, is that correct?
There was a problem hiding this comment.
That’s not essential in my thinking (we could instead first detect what the registry contains, and then create an appropriate ImageSource), but it is the easiest way to do it at the moment, yes.
I do wonder about v2s2→OCI conversions and the like; perhaps that would find it useful to have an ImageDestination with a known intended MIME type? At the moment the types are mutually exclusive, so “ociImageDestination supports OCI and none of v2s1 v2s2“ is enough, which is a property of the type, not of the instance.
mtrmac
force-pushed
the
image-layer-digests
branch
from
0c6f0db
to
112f5c7
Compare
|
can you rebase this? |
Per the discussion in containers#73 , types.Image.Manifest should not need to expose MIME types: ImageSource.GetManifest allows supplying MIME types; the intent is for clients who want to parse the manifests to use an ImageSource. Clients who want to use skopeo’s parsing should use types.Image, and then they don’t need to care about MIME types. In fact, types.Image needs to decide among the various manifest alternatives which one to parse (and which one to match against the provided or signed manifest digest). So, Image.Manifest will not be all that useful for parsing the contents, it is basically useful only for verifying against a digest.
The .Layers() method name is too nice to contain this layering violation; make it more explicit in the naming.
To do so, have (skopeo copy) work with a types.Image, and replace uses of types.ImageSource with types.Image where possible to allow the caching in types.Image to work. This is a slight behavior change: - The manifest is now processed through fixManifestLayers - Duplicate layers (created e.g. when a non-filesystem-altering command is used in a Dockerfile) are only copied once.
... using the new uniqueLayerDigests().
mtrmac
force-pushed
the
image-layer-digests
branch
from
112f5c7
to
5b1ca76
Compare
|
Rebased. |
See individual commits for detailed comments.