-
Notifications
You must be signed in to change notification settings - Fork 206
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
Tag list API should include pagination #461
Comments
As you've alluded to, this is one of the things that was part of the spec, but the language was (accidentally?) removed (as noted in #443 🙈) -- looking at the history and adding back some appropriate pagination references would probably be appreciated (although to be clear, I am not a maintainer of the distribution spec, only image/runtime 😇). |
nod was removed and partially moved to detail.md and missed in review |
The text we missed appears to be:
Unfortunately, with the 1.0 release, we have the following text:
I don't think there's an easy way to combine the "MUST include If we are going to break clients, I'd prefer to do that in a way that explicitly fails, with an error code response and a 400 status code, to a request that exceeds a limit on the server. Similar to the maximum manifest size, we could document a limit that should be supported by registries, and that clients should not exceed, for interoperability. Registries could elect to support more than that, particularly for legacy clients that do not specify a count on a large repo. And if a link header is seen, clients can either use that or use the legacy behavior of creating the next request using the Does this seem like a feasible path forward to others, given the text of the 1.0 spec? |
in the spec, the next sentence reads
it looks like at some point there was a We use the |
It may be included since there's nothing saying it may not be included, but there's no requirement for clients to use it. |
Also hitting various issues with this. You basically can't write a compliant implementation of "listing tags" that works in practice: every registry implements pagination differently now. Can this issue please be fixed? Related: #116. |
Agreed. I'd propose that |
Uh, that would be really bad: clients want to maximize page size, servers want to minimize. Is a client going to bisect the server for the largest supported I think you're saying this because you want to rely on "less than n results" signals "end of data", which is necessary cause old clients do not hint (with The much more obvious solution is to let the server return at most I would recommend against For clients it's also much easier to parse a bit of JSON. RFC-conforming header value parsers are rare, and made extra difficult since servers may not always exactly conform to it. |
Sounds reasonable. The main reason why I am proposing not to have the client be able to dictate |
That would only work for new clients going forward but break existing clients that are following older versions of the spec in a non-explicit way (giving a false indication that all tags have been pulled). One of the challenges being addressed is there are a lot of existing clients, including written with a simple curl command, that are not aware they are receiving a partial list when they request all tags, and those that implement pagination that depend of the registry returning less than a full list to indicate the last page of results. #470 opts for an explicit failure when clients exceed the limit so the need to update an implementation to work with the registry is clear. And it provides a header on the failure to allow clients to gracefully downgrade to the maximum allowable number of tags. If registries support returning more tags than requested, I think using the link header would be an acceptable way for registries to return that in the later responses (first response would be 1-100 tags, and the link header would point to 101-1100). Support for that link header is also optional, and I'd expect a spec conforming client that sees an invalid link header to ignore it and default to the pagination args in the URL instead. |
I would not consider it very breaking.
You could even see it as a security bug embedded in the current spec (no pagination may result in denial of service), so fixing that problem is non-breaking. It makes absolutely no sense to let clients guess how many tags they can fetch from an arbitrary registry. |
#470 removes all the guessing, clients are told how many tags a registry should support, and the registry tells the client how many it does support if the client requests too many. |
The current [https://github.com/opencontainers/distribution-spec/blob/main/spec.md#listing-tags](tag list API spec) does not have a limit on the response from the registry side. This exposes the registry to a potential DDoS when a client requests tags for a repo that has a lot of tags.
Right now all major registries have pagination and wont return the full tag list. From the discussion on the OCI call, it seems like there was language around pagination which was accidentally deleted. Can we please add the pagination back into the tag list spec
The text was updated successfully, but these errors were encountered: