-
Notifications
You must be signed in to change notification settings - Fork 205
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
Allow for automatic content discovery for cross-mounting blobs #275
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -357,7 +357,7 @@ Here, `<blob-location>` is a pullable blob URL. | |
|
||
##### Mounting a blob from another repository | ||
|
||
If a necessary blob exists already in another repository, it can be mounted into a different repository via a `POST` | ||
If a necessary blob exists already in another repository within the same registry, it can be mounted into a different repository via a `POST` | ||
request in the following format: | ||
|
||
`/v2/<name>/blobs/uploads/?mount=<digest>&from=<other_name>` <sup>[end-11](#endpoints)</sup>. | ||
|
@@ -376,6 +376,8 @@ The Location header will contain the registry URL to access the accepted layer f | |
header returns the canonical digest of the uploaded blob which MAY differ from the provided digest. Most clients MAY | ||
ignore the value but if it is used, the client SHOULD verify the value against the uploaded blob data. | ||
|
||
The registry MAY treat the `from` parameter as optional, and it MAY cross-mount the blob if it can be found. | ||
|
||
Alternatively, if a registry does not support cross-repository mounting or is unable to mount the requested blob, | ||
it SHOULD return a `202`. This indicates that the upload session has begun and that the client MAY proceed with the upload. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to specify the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that's unrelated to this change, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a bit of the "else" path we hit, when the blob isn't found. But yes, to your point, that path isn't being changed with this PR, so we could clarify that in a separate PR. |
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we change
if it can be found
toif it can be found and can be accessed
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to me, that ACL implication is included in
can be found
, because ideally if it can not be accessed then it should be found. Or is there another use case you're thinking about?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although it is outside of the specification, I would suggest that we indicate that it is not a good idea to implement this feature on any registry with cross-repo ACLs due to the possibility of information disclosure. I do not think
found
implies any notion of "access" in the sense of "Security"So, I would rather say something like:
"If the registry does not implement a security model which allows for attenuation of access to reading blobs, it MAY treat the
from
parameter as optional, and it MAY cross-mount the blob if it can be found."There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed on adding more to wording here. While we don't define an ACL model, it is a potential pitfall with a very small change that is worth calling out. The updated language is reasonable, but I think it can be simplified some to just mentioned the read access. Like @jonjohnsonjr brought it, it could be that the registry only searches in a list of a blob that are known public and therefore able to be read by anyone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could leverage this for MCR/ACR as well and for avoiding pushing these public well known layers multiple times.