Add cross-repository mounting section to spec/test#204
Add cross-repository mounting section to spec/test#204jdolitsky merged 3 commits intoopencontainers:masterfrom
Conversation
b910554 to
ff20463
Compare
jdolitsky
left a comment
There was a problem hiding this comment.
LGTM. Tested against Docker distribution
| In this case, `<name>` is the namespace to which the blob will be mounted. `<digest>` is the digest of the blob to mount, | ||
| and `<other_namespace>` is the namespace from which the blob should be mounted. | ||
|
|
||
| The response to a successful mount MUST be `201 Created`, and MUST contain the following header: |
There was a problem hiding this comment.
I think it should be worth mentioning that this is normally done in place of the normal call to .../uploads/ to start a blob upload. So if a registry does not support cross repository mount, it will return a normal 202 and the client will continue the upload (not reissue the request without the mount param).
Did we have 201 listed as valid return code for /uploads without mount for a reason? Or was that just listed because of this. We should be explicit about the difference between 201 and 202 here.
There was a problem hiding this comment.
@dmcgowan based on yesterday's call, we have decided to require cross-repository mount, making the distinction between 201 and 202 unnecessary here -- it will always be 201 unless the source blob is not found, in which case it will be 404.
I have pushed a new commit removing 405 as an acceptable status code, thus requiring cross-repository implementation.
Allowing both 201 and 202 on /uploads was a compromise solution based on what existing registries were already doing -- see #171 and @jdolitsky 's comment on #68
There was a problem hiding this comment.
I've written several clients that rely on the fact that 201 means the mount succeeded and 202 means continue with the upload -- ideally registry behavior would be consistent here. I don't think requiring cross-repo mount invalidates the requirement of this distinction.
There was a problem hiding this comment.
unless the source blob is not found, in which case it will be
404
This would also slow down uploads because you'd require an additional roundtrip to start the upload process if the mount fails.
There was a problem hiding this comment.
@jonjohnsonjr This is my proposed solution:
In the first test, allow either a 201 or 202. If 201, a fetch for the blob will be tested to ensure its existence. If 202, we will verify that a valid <session-id> is returned.
If I'm getting it right, this should satisfy the requirements -- please let me know if there's anything further I should know about before making changes.
cdfc3ec to
810b94f
Compare
|
Tested against ACR. LGTM. |
|
@jonjohnsonjr @dmcgowan I just pushed the requested changes. Please review, and feel free to approve if all looks good. Thanks! |
* New test in 02_push_test.go * Section added to spec.md detailing cross-repository mounting * Section added to conformance/README.md on new configuration options for cross-repository mounting Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
efd5ae5 to
5adbeea
Compare
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
5adbeea to
97af77e
Compare
for cross-repository mounting