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

Update name/tag embedded into schema1 manifests #88

Merged
merged 2 commits into from
May 10, 2017

Conversation

mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Sep 14, 2016

FIXME: Actually handle the manifest-embedded tags in schema 1. Without that, this is a fairly broken implementation (though might be better than nothing I suppose).

EDIT When pushing a schema1 image, update the name/tag embedded in the manifest. Recent registry versions ignore the fields, but older ones require them to match the registry/tag used to upload the image.

@mtrmac mtrmac force-pushed the docker-push-to-tag branch 4 times, most recently from 3d64071 to 7e35eb2 Compare September 19, 2016 20:55
@mtrmac mtrmac changed the title WIP: Support pushing to docker: tag Support pushing to docker: tag Sep 19, 2016
@mtrmac
Copy link
Collaborator Author

mtrmac commented Sep 19, 2016

OK, this seems to work. @runcom PTAL.

For the most part, we just need to stop ignoring the tag in dockerImageDestination; originally we did that because IIRC OpenShift really didn’t like pushing schema1 when the embedded metadata didn’t match. Right now, in my testing, none of the destinations actually care (but OpenShift 1.1 definitely did).

Anyway, updating the manifest as well now that it is easy.

I still need to finish tests in skopeo, so this may turn out to be broken, but manual testing suggests this is fine.

@mtrmac
Copy link
Collaborator Author

mtrmac commented Sep 19, 2016

Apparently this will break various tests, fairly significantly. For the most part, it’s the tests that are broken, because they are using schema1 all over the place and assume that manifests won’t change, that broken signatures will be preserved, etc., but that still might be reason enough to defer this a bit. We strictly need working v2 support for some of that, and that seems not to be available.

@mtrmac mtrmac changed the title Support pushing to docker: tag Do not merge WIP: Support pushing to docker: tag Sep 19, 2016
@mtrmac
Copy link
Collaborator Author

mtrmac commented Sep 20, 2016

Eventually, this fixes #51,

@mtrmac mtrmac force-pushed the docker-push-to-tag branch 4 times, most recently from 34030f0 to 6a8c5c7 Compare April 4, 2017 17:31
@mtrmac mtrmac force-pushed the docker-push-to-tag branch 2 times, most recently from 42b8df2 to 01c6ac1 Compare April 12, 2017 15:07
@mtrmac mtrmac force-pushed the docker-push-to-tag branch 3 times, most recently from 5afcc2b to 7136b19 Compare April 27, 2017 16:49
@mtrmac mtrmac force-pushed the docker-push-to-tag branch 3 times, most recently from 032e8f7 to ce40ecc Compare May 5, 2017 22:01
@mtrmac mtrmac changed the title Do not merge WIP: Support pushing to docker: tag Update name/tag embedded into schema1 manifests (blocked on #266) May 6, 2017
@mtrmac
Copy link
Collaborator Author

mtrmac commented May 6, 2017

Fixed this to only use the registry-relative path instead of the full hostname/ns/repo name. The changes as such are quite stand-alone, but converting tests is much easier when coupled with schema1/schema2 autodetection and schema2 support for atomic:. Hence this depends on, and includes, all of #266. I will rebase as necessary.

@aweiteka @baude FYI. This should allow pushing to old schema1-only registries (including old OpenShift), possible user-visible impact is that copying a schema1 image to a different repository path now changes the manifest digest.

@mtrmac
Copy link
Collaborator Author

mtrmac commented May 6, 2017

The CI tests in this repo fail with current skopeo, see containers/skopeo#337 for the skopeo counterpart which fixes, and extends, them.

@runcom
Copy link
Member

runcom commented May 6, 2017

LGTM (reviewed last 2 commits here)

Approved with PullApprove

@mtrmac mtrmac force-pushed the docker-push-to-tag branch 3 times, most recently from 6d5ff14 to 8282ebf Compare May 9, 2017 13:47
@mtrmac mtrmac changed the title Update name/tag embedded into schema1 manifests (blocked on #266) Update name/tag embedded into schema1 manifests May 9, 2017
This allows us to detect whether we need to edit the reference before
trying to (i.e. before uploading any of the layers).  The method
interface may be a bit surprising, it is designed to hide the
complexity and schema1 specifics in manifestSchema1; see the comment
in that implementation for more details.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Add EmbbeddedDockerReference to types.ManifestUpdateOptions, and use it
in copy.Image if necessary.  Pushing signed schema1 images to a different
location is impossible (skopeo users can use --remove-signatures), but
signing an unsigned schema1 image while pushing it does work.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac mtrmac merged commit 911e781 into containers:master May 10, 2017
@mtrmac mtrmac deleted the docker-push-to-tag branch May 10, 2017 10:12
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

Successfully merging this pull request may close these issues.

None yet

2 participants