Handle OCI-Chunk-Max-Length header field in blob pushes #4144
Conversation
|
Thank you, @gabivlj. We may need to put this PR on hold as the upstream issue is still being discussed. Once your proposed solution is merged, we can start the review. Also, could you please add UT to verify the functionality of the header? |
gabivlj
force-pushed
the
dry-run-test
branch
2 times, most recently
from
66f6603
to
e06cdf7
Compare
|
Thanks for the suggestion @wy65701436! Added more tests now. |
|
Please sign your commit @gabivlj |
|
@milosgajdos Oh I thought I signed it. I added the footer Isn't that enough? (EDIT: I guess I should've matched the entire name i have in user.name) |
This header comes from the proposal of adding a way for registries to limit chunk sizes from the client. The implementation is simple enough; we're just adding a io.LimitReader and don't let it read more than the bytes that were specified by the registry. Signed-off-by: Gabi Villalonga Simon <gvillalongasimon@cloudflare.com>
|
Thanks, @gabivlj, as @wy65701436 mentioned, we'll hold back on this until the OCI issue is resolved but I like where this is going. |
|
This would actually be useful for us - we want to put Cloudflare's CDN cache in front of our registry but this is a limitation we face today with large layer sizes. It looks like the discussion from OCI has stalled, is there a world it can move forward with implementation anyway since a new extension header should be very backwards compatible? Maybe we can just rename the header, X-Experimental-Max-Length or some other pattern like this? |
I know quite a few companies front their registries with Cloudflare CDN. I am not sure how not having this option is preventing you from doing that. Though I think you'd need a storage driver middleware that works with CF 🤔 |
|
Yep! There is probably registries backed by Cloudflare's CDN; however if you try to push for example to a Cloudflare proxy with a layer that is bigger than 1GB, you will encounter failures. I can repro by using a free cloudflared tunnel for example: We can also repro by putting a Worker in front of another registry. If you try to upload through the worker it will also fail. See more about the limits that can be encountered on the CDN: And worker limits: I think it would be nice for the registry to optionally define a chunk upload boundary like we're doing here to circumvent possible proxy upload size limitations. |
|
Hi @milosgajdos, is there any way to circle back on this? Is there any interest on adding this? Is there any reviewer that we should call-in to have more feedback? |
There is interest from my side 💯 but I still think we should power through to get this into |
This header comes from the proposal of adding a way for registries to limit chunk sizes from the client.
The implementation is simple enough; we're just adding a io.LimitReader and don't let it read more than the bytes that were specified by the registry.
Implements this:
opencontainers/distribution-spec#485
What do folks here think about this?
This has been tested manually on the moby repository and works with a registry that implements this header.