Update spec language to support pushing multiple tags at once#600
Update spec language to support pushing multiple tags at once#600jcarter3 wants to merge 3 commits intoopencontainers:mainfrom
Conversation
083f71d to
f271626
Compare
spec.md
Outdated
| When pushing a manifest by digest, the registry MAY support the pushing of tags specified by addition of `tag` query | ||
| parameters. If a registry supports this, it MUST: | ||
|
|
||
| 1. Not limit the number of tags that can be pushed at once. |
There was a problem hiding this comment.
My personal preference is to allow a registry to respond with an error if a limit is exceeded and have the spec identify a minimum that registries are expected to support, similar to how large manifests are handled.
There was a problem hiding this comment.
The issue I see here is that if a client passes 100, but the registry only supports 25, then the client has no way of knowing if the other 75 failed for error, or if there was a limit. We could set this as something reasonably high enough (100?) so that it would rarely happen in practice. Or also add an OCI-Tag-Limit response header to indicate the max so that the client could intelligently batch. But the thinking here is that this can make both the registry and the client much more efficient with validations and network calls, so as long as it's a valid URL length, it should work.
There was a problem hiding this comment.
so as long as it's a valid URL length
What's an invalid URL length?
There was a problem hiding this comment.
This seems a bit tricky to nail down as it's all over the place. Browsers seem to support a lot, like in the 65,000-100,000 character range. (>500 tags at max tag length). Some security recommendations say to limit it to 1k-2k on webservers to prevent some kinds of attacks, which is more like a max of 10 tags at a time at max tag length.
Maybe it should be updated to say it MUST support a minimum of 10, SHOULD not set an upper limit.
There was a problem hiding this comment.
Yeah, I'm not finding a limit in the http RFCs. In Go, the limit defaults to 1MB for net/http Server.MaxHeaderBytes which includes all headers and the request, but that is configurable. I'd say registries SHOULD support at least 10 tags per request. If a registry rejects the request for exceeding a tag limit, it MUST return a 414 Request-URI Too Long, and clients MAY retry with fewer tags.
If they hit the Go limit, that throws a 431 Request Header Fields Too Large, which we should document as another possible error.
I hesitate to respond with an OCI-Tag-Limit header because the 414/431 could be from different causes, including a proxy or the internal Go http server, and it may be from other headers, the overall URI length, or a fixed limit on the number of tags enforced by the registry implementation.
In practice, I think most users would rarely exceed 5 tags per manifest (3 tags for semver + a fixed/latest tag or two), so I don't think a failure or split+retry is going to impact folks doing normal things.
There was a problem hiding this comment.
I updated it to support 10 (still under a 1kb max URI), and defined additional error codes.
Signed-off-by: Jeff <jscarter3@gmail.com> Update spec language to support issue opencontainers#591, pushing multiple tags at once. Also can be used when pushing tags for a non-canonical algorithm (sha-512, blake3) Signed-off-by: Jeff Carter <jeff.carter@docker.com>
333daf2 to
fde1798
Compare
| The server would respond with the following header: | ||
| ``` | ||
| OCI-Tag: 1.2.3, 1.2, 1, latest |
There was a problem hiding this comment.
Should we do something with this example to make the RFC 2616 semantics more obvious at a glance? So that we don't get implementations that choke if every tag is a completely separate header, for example?
Implementation of opencontainers/distribution-spec#600 Signed-off-by: Brandon Mitchell <git@bmitch.net>
Implementation of opencontainers/distribution-spec#600. Signed-off-by: Brandon Mitchell <git@bmitch.net>
sudo-bmitch
left a comment
There was a problem hiding this comment.
I noticed there's a newer version of the RFC. So this updates that pointer and makes some, hopefully minor, tweaks.
| 1. MUST support pushing at least 10 tags at once | ||
| 1. MAY return a `414 Request-URI Too Long` status if more than 10 tags are pushed. |
There was a problem hiding this comment.
MUST feels a bit strong to me. My preference is for a SHOULD.
| 1. MUST support pushing at least 10 tags at once | |
| 1. MAY return a `414 Request-URI Too Long` status if more than 10 tags are pushed. | |
| 1. SHOULD support pushing at least 10 tags per request. | |
| 1. MAY return a `414 Request-URI Too Long` status if too many tags are included in the request. |
|
|
||
| 1. MUST support pushing at least 10 tags at once | ||
| 1. MAY return a `414 Request-URI Too Long` status if more than 10 tags are pushed. | ||
| 1. For each tag that was successfully pushed, include an `OCI-Tag` response header in accordance with [RFC 2616 (section 4.2)](https://datatracker.ietf.org/doc/html/rfc2616#section-4.2) semantics |
There was a problem hiding this comment.
Slight reword and using rfc-editor.org for consistency with other parts of the spec, and pointing to the most recent version of the RFC:
| 1. For each tag that was successfully pushed, include an `OCI-Tag` response header in accordance with [RFC 2616 (section 4.2)](https://datatracker.ietf.org/doc/html/rfc2616#section-4.2) semantics | |
| 1. MUST include an `OCI-Tag` response header, in accordance with [RFC 9110 (section 5)](https://www.rfc-editor.org/rfc/rfc9110#name-fields) semantics, for each accepted tag. |
|
|
||
| The server would respond with the following header: | ||
| ``` | ||
| OCI-Tag: 1.2.3, 1.2, 1, latest |
There was a problem hiding this comment.
| OCI-Tag: 1.2.3, 1.2, 1, latest | |
| OCI-Tag: 1.2.3, 1.2, 1 | |
| OCI-Tag: latest |
| PUT /v2/<name>/manifests/<digest>?tag=1.2.3&tag=1.2&tag=1&tag=latest | ||
| ``` | ||
|
|
||
| The server would respond with the following header: |
There was a problem hiding this comment.
| The server would respond with the following header: | |
| The server could respond with the following headers: |
See opencontainers/distribution-spec#600 Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
See opencontainers/distribution-spec#600 Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
See opencontainers/distribution-spec#600 Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
See opencontainers/distribution-spec#600 Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
|
I've added a test for this in the conformance redesign (disabled by default, but enabled with the |
See opencontainers/distribution-spec#600 Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
See opencontainers/distribution-spec#600 Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
Also can be used when pushing tags for a non-canonical algorithm (sha-512, blake3)
Implements #591