-
Notifications
You must be signed in to change notification settings - Fork 213
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
Document manifest size limit recommendation #293
Conversation
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.
Thanks for driving this! Few small suggestions, and the DCO is going to need a new push.
@@ -402,6 +402,10 @@ The `<location>` is a pullable manifest URL. | |||
|
|||
An attempt to pull a nonexistent repository MUST return response code `404 Not Found` | |||
|
|||
A registry SHOULD enforce some limit on the maximum manifest size that it can accept. |
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.
While this is recommended for public registries, I can see a case for private registries not wanting to implement this.
A registry SHOULD enforce some limit on the maximum manifest size that it can accept. | |
A registry MAY enforce limits on the maximum manifest size that it can accept. |
@@ -402,6 +402,10 @@ The `<location>` is a pullable manifest URL. | |||
|
|||
An attempt to pull a nonexistent repository MUST return response code `404 Not Found` | |||
|
|||
A registry SHOULD enforce some limit on the maximum manifest size that it can accept. | |||
A registry that enforces this limit SHOULD respond to a request to push a manifest over this limit with a response code `413 Payload Too Large`. | |||
Client and registry implementations SHOULD expect to be able to support manifest pushes of at least 4 megabytes. |
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.
I'd suggest splitting this into two lines, one for clients, and one for registries:
Client and registry implementations SHOULD expect to be able to support manifest pushes of at least 4 megabytes. | |
A registry that implementations this SHOULD support manifest pushes of at least 4 megabytes. | |
A client SHOULD avoiding pushing a manifest larger than 4 megabytes and SHOULD support pulling a manifest of at least 4 megabytes. |
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.
Maybe something like:
Registries SHOULD NOT impose a limit less than 4 megabytes on the size of a manifest.
Clients SHOULD support handling manifest sizes of at least 4 megabytes.
Clients concerned with portability SHOULD NOT generate manifests larger than 4 megabytes.
FWIW, if quay has a 1MB limit, I would lean toward 1MB as the recommendation.
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.
The wording from @jonjohnsonjr is more clear.
We'll probably also want to reach out to see what all the major registries have for existing limits to avoid any potential issues with the 4MB number. |
Yeah, I've attempted to start a survey of existing registries here: #260 (comment)
Maybe the recommendation should be 1MB? 😄 |
The more I think about it.. the more I think we should just recommend that they SHOULD have a limit, refer to a table stored somewhere on here showing survey results for existing limits for registries and certain clients. I think the error returned should be a MUST.. |
I don't think it makes sense to suggest registries impose an artificial limit. This is kind of philosophical, but I'd lean towards Postel's Law:
It is expected that registries will have some practical limit in terms of what it can accept. They should have predictable behavior for that case by returning a 413, but I don't see the point in having a registry reject something that it's otherwise able to handle just fine. If they want to impose some artificial limitation to give themselves flexibility in the future, that's fine, but I don't think we should be recommending that. On the other hand, clients are actually in control of the artifacts they're producing, so they have the flexibility to fix any problems caused by the size of the manifest they produce. If I know that my runtime can support arbitrarily large manifests, then there's not any benefit from a registry imposing an arbitrary limit. For example, imagine this situation:
We can successfully push a 2MB manifest, but it will fail when the client tries to pull it. There might be some benefit to shifting this failure "left" by imposing a 1MB limit in the registry, but what happens if eventually the client gets upgraded to remove that limitation?
Well... now we're still stuck at 1MB, even though we would prefer to be pushing a 2MB manifest :/ |
nod.. Cheers, Mike |
@mikebrow are you suggesting we don't provide a specific size? From the server, I understand this, it gives flexibility, and we have a clear API communication to the client when the registry specific limit is exceeded. But from a client, it means that I don't know if my manifest can be pushed to any OCI compliant registry. An image building tool will not know which registry the image may be retagged and pushed to in the future. I'll end up pushing the problem off to the user, which means some users may push images designed for their specific registry. That could create a registry lock-in where those images can't be easily copied to another registry. |
I don't think we should be mandating physical resource size requirements/limits on the client or the server. Reason being someone might want the limit to be 1GB another might want it to be 1KB still another 1MB and all three may have a perfectly valid reason. I think we should provide a location for a table of known current limits (client/server) and let the market solutions decide. Registries with larger maximums holding a lock on the accounts exploiting said maximum.. I'd suggest clients consider using the table as a benchmark to warn users when their manifest are so large as to have a limited number of registries where they can currently be stored. We could provide just such a suggestion for clients maybe even an api. Clients could also provide a default limit with an override option or what not and certainly registries may have various limits based on account type, storage service, etc. Cheers, Mike |
Signed-off-by: Jason Hall <jasonhall@redhat.com>
Picking this back up, I think a reasonable first cut of this change would be:
I think we could improve on this over time, but this seems relatively uncontroversial subset of the change that we can iterate on. wdyt? |
@imjasonh sounds good. Something that would likely be nice is if there was an HTTP response from the registry where this limit could be hinted in a header. That could be an eventual SHOULD language. But is not diversion from this PR for now. |
Agreed. I can imagine two reasonable ways to handle this:
Three are more interesting use cases for the second option, but for the issue at hand, we could do something like: {
"errors": [
{
"code": "MANIFEST_TOO_LARGE",
"message": "boot too big",
"detail": {
"limit": 1048576
}
}
]
} There's room for this in the spec:
As an aside: I noticed that specs-go has distribution-spec/specs-go/v1/error.go Line 39 in 13fa739
Was this change deliberate? |
We shouldn't try to mandate a minimum manifest size that all registries should accept. Especially not if picking X because that's the lowest we know (e.g., 1MB because that's that maximum that Quay accepts...). There are many registry implementations out there, each with its own technical limitations (either by design or not), and these are likely to change over time. The approach that @imjasonh proposes in #293 (comment) sounds like the best to me. I also agree that it should be clear to clients whenever they hit such limits, and it should be possible to determine what that limit is programmatically. I would be in favor of a custom error code like @jonjohnsonjr suggested in #293 (comment). |
Also note that "maximum" may be dependent on the resources made available to the registry. For example, docker registry with a 1GB disk will have a different maximum than the same docker registry with a 1TB disk. Unless this scenario out of scope of this issue. |
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.
LGTM
This provides solid guidance and doesn't impose any new requirements, other than explicitly requiring a status code.
I suppose we add this to 1.1 then? |
Document manifest size limit recommendation Signed-off-by: Jason Hall <jasonhall@redhat.com>
Just to make sure this is written down somewhere (because "megabytes" is a legally disputed unit that's either base 2 or base 10 depending on whether you're in/bound by the hard drive industry), I'm going to assume from @jonjohnsonjr's example in #293 (comment) that this was intended to be base 2 (and thus 4194304 bytes, not 4000000). 👍 |
Fixes #260
cc @sudo-bmitch @jonjohnsonjr