Support pulling/pushing OCI manifests from a docker registry#338
Support pulling/pushing OCI manifests from a docker registry#338mtrmac merged 2 commits intocontainers:masterfrom
Conversation
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
Thanks!
ACK, just two trivial things.
Could you also prepare the skopeo update, please, per https://github.com/projectatomic/skopeo/blob/master/README.md#dependencies-management ?
(Right now you are probably seeing unrelated failures due to containers/skopeo#399 / containers/skopeo#399 (comment) )
copy/copy.go
Outdated
| // We compute preferredManifestMIMEType only to show it in error messages. | ||
| // Without having to add this context in an error message, we would be happy enough to know only that no conversion is needed. | ||
| destSupportedManifestMIMETypes := dest.SupportedManifestMIMETypes() | ||
| preferredManifestMIMEType, otherManifestMIMETypeCandidates, err := determineManifestConversion(&manifestUpdates, src, destSupportedManifestMIMETypes, canModifyManifest) |
There was a problem hiding this comment.
Nit: inlining the call directly and eliminating the variable would be shorter.
| if requestedManifestMIMETypes == nil { | ||
| requestedManifestMIMETypes = manifest.DefaultRequestedManifestMIMETypes | ||
| } | ||
| supportedMIMEs := supportedManifestMIMETypesMap() |
There was a problem hiding this comment.
This removes the only user of supportedManifestMIMETypesMap; can you remove it, please, and perhaps inline the global manifestMIMETypes variable into dockerImageDestination.SupportedManifestMIMETypes?
owtaylor
force-pushed
the
docker-oci-manifests
branch
from
297df8b to
86813cd
Compare
|
The latest set of pushes makes both code improvements you suggested, and the skopeo update is in containers/skopeo#403. |
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
👍
Please drop the backup files though.
oci/layout/oci_transport.go.rej
Outdated
| @@ -0,0 +1,15 @@ | |||
| diff a/oci/layout/oci_transport.go b/oci/layout/oci_transport.go (rejected hunks) | |||
| @@ -0,0 +1,366 @@ | |||
| package signature | |||
owtaylor
force-pushed
the
docker-oci-manifests
branch
from
86813cd to
52ececd
Compare
|
Oy, sorry about the stray files! Should not have deviated from my normal git habits. Fixed now. |
( |
owtaylor
force-pushed
the
docker-oci-manifests
branch
from
52ececd to
4f9e306
Compare
|
Added the sign-offs |
|
👍 pending tests |
What tests are you waiting for? containers/skopeo#403 seems to pass (can repush with a sign-off now, or just do that when rebasing when this lands), tests here aren't going to pass. |
You’re right, some failures are expected; just to make sure there are no unexpected failures. E.g. right now, please run gofmt -s -w docker/docker_image_src.go openshift/openshift.go |
owtaylor
force-pushed
the
docker-oci-manifests
branch
from
4f9e306 to
af4a19f
Compare
Good point. Repushed with the go fmt changes. |
|
One more test failure, due to #314 landing since the original PR was prepared. |
…meter The requestedManifestMIMETypes parameter was added because a destination might not support all manifest MIME types that the the source supports, but the original use case now passes all manifest types and lets containers/image convert internally. In generally, internal conversion may be more comprehensive, is more predictable, and avoids bypassing internal checks. Fixes: containers#331 Signed-off-by: Owen W. Taylor <otaylor@fishsoup.net>
Since OCI support is in progress for the docker registry, if we have an OCI manifest, we should try uploading it literally to the registry before falling back to converting it to a v2 manifest. Signed-off-by: Owen W. Taylor <otaylor@fishsoup.net>
owtaylor
force-pushed
the
docker-oci-manifests
branch
from
af4a19f to
ae52c73
Compare
|
This and containers/skopeo#403 are now rebased on top of the latest code and (I think) passing all the tests they are supposed to pass. |
|
Thanks! Please re-vendor containers/skopeo#403 back to the |
2 participants
This pull request has a tiny one-line patch to add imgspecv1.MediaTypeImageManifest to the list of MIME types that a docker registry might support (as always, the exact list depends on the version of the registry we are talking to), and a bigger, though still trivial patch that prevents some breakage from the change - as discussed in #331.
(With the obvious compilation fix to skopeo, the skopeo tests all pass with these changes.)
The breakage that is avoided is that for copying from docker to oci, we don't want to pass only the imgspecv1.MediaTypeImageManifest in the Accept header - a) the particular docker registry might not in fact support imgspecv1.MediaTypeImageManifest b) and if it did, the particular manifest being requested might not be a imgspecv1.MediaTypeImageManifest, instead we want to pass in our normal list, get the manifest in it's original form and convert if necessary.