Image's digest from HEAD request

[crazy-max]

Context

Docker Hub is instituting pull rate-limits for anonymous and free users since Nov 2, 2020.

You are receiving this email because of a policy change to Docker products and services you use. On Monday, November 2, 2020 at 9am Pacific Standard Time, Docker will begin enforcing rate limits on container pulls for Anonymous and Free users. Anonymous (unauthenticated) users will be limited to 100 container image pulls every six hours, and Free (authenticated) users will be limited to 200 container image pulls every six hours, when enforcement is fully implemented. Docker Pro and Team subscribers can pull container images from Docker Hub without restriction, as long as the quantities are not excessive or abusive.

With this limit in place, every time a NewImageSource is created, it will make a GET request on the manifest endpoint which counts as a pull and will return this body response if limit is reached:

{"message":"toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit"}

Solution

First I have removed the ensureManifestIsLoaded in NewImage/NewImageSource func to prevent the initial (not required?) manifest GET request.

I have also added a new interface to be able to retrieve the manifest digest through a HEAD request which does not count as a pull on Docker Hub. GetDigest is exposed to be able to use the digest from the HEAD request on Diun and compared with a manifest stored in my bbolt database:

Tue, 10 Nov 2020 17:11:52 CET DBG [containers/image] Loading registries configuration "/etc/containers/registries.conf"
Tue, 10 Nov 2020 17:11:52 CET DBG [containers/image] Trying to access "docker.io/crazymax/diun:latest"
Tue, 10 Nov 2020 17:11:52 CET DBG [containers/image] Returning credentials from DockerAuthConfig
Tue, 10 Nov 2020 17:11:52 CET DBG [containers/image] Using registries.d directory /etc/containers/registries.d for sigstore configuration
Tue, 10 Nov 2020 17:11:52 CET DBG [containers/image]  No signature storage configuration found for docker.io/crazymax/diun:latest, using built-in default file://C:%5CUsers%5Ccrazy%5C.local%5Cshare%5Ccontainers%5Csigstore
Tue, 10 Nov 2020 17:11:52 CET DBG [containers/image] Looking for TLS certificates and private keys in \etc\docker\certs.d\docker.io
Tue, 10 Nov 2020 17:11:52 CET DBG [containers/image] GET https://registry-1.docker.io/v2/
Tue, 10 Nov 2020 17:11:53 CET DBG [containers/image] Ping https://registry-1.docker.io/v2/ status 401
Tue, 10 Nov 2020 17:11:53 CET DBG [containers/image] GET https://auth.docker.io/token?account=pharmagest&scope=repository%3Acrazymax%2Fdiun%3Apull&service=registry.docker.io
Tue, 10 Nov 2020 17:11:53 CET DBG [containers/image] HEAD https://registry-1.docker.io/v2/crazymax/diun/manifests/latest
Tue, 10 Nov 2020 17:11:54 CET DBG [containers/image] Content-Type from digest HEAD is "application/vnd.docker.distribution.manifest.list.v2+json"

Further enhancement

Maybe this mechanic could be used in your internal cache to avoid unnecessary call to the manifest GET endpoint?

Let me know if I've missed something and thanks again for this great lib!

[rhatdan]

@mtrmac PTAL

[rhatdan]

Does this article help?

https://developers.redhat.com/blog/2019/08/14/best-practices-for-running-buildah-in-a-container/

Podman inside of a container is a lot more complex. Remember if you are doing this with Docker, you need to disable the seccomp filters, or use /usr/share/containers/seccomp.json

Using podman for running the locked down Buildah container is the preferred mechanism.

[mtrmac]

Thanks for the PR.

Removing ensureManifestIsLoaded from newImageSourceAttempt would completely break the mirroring implementation.

Does this actually end up reducing the number of requests? Basically every operation that can be done while reading c/image (copy an image, “inspect” it, …) will end up reading the manifest as one of the first things, and ensureManifestIsLoaded as well as image.Sourced EDIT cache the read data, so AFAICT this typically only moves the manifest read, not eliminates one.

Unless I guess you have an external caller in mind for the new GetDigest call? What does it do with the value?

[crazy-max]

@mtrmac

Removing ensureManifestIsLoaded from newImageSourceAttempt would completely break the mirroring implementation. Does this actually end up reducing the number of requests?

Argh that's what I thought but yeah it helps removing the first manifest GET request when a new image interface is setted up. Maybe ensureManifestIsLoaded could be called in a further func to preserve mirroring impl?

Basically every operation that can be done while reading c/image (copy an image, “inspect” it, …) will end up reading the manifest as one of the first things, and ensureManifestIsLoaded as well as image.Sourced, so AFAICT this typically only moves the manifest read, not eliminates one.

Yeah that's my point I don't want the GET manifest endpoint to be called when I instantiate a new image.

Unless I guess you have an external caller in mind for the new GetDigest call? What does it do with the value?

I compare a manifest digest located in my own bolt db to the GetDigest HEAD req to know if I need or not to GET a fresh manifest. This helps to reduce unnecessary calls to the GET manifest endpoint.

I understand that my implementation might be out of scope for this library so don't hesitate to tell me if this is the case.

[mtrmac]

I compare a manifest digest located in my own bolt db to the GetDigest HEAD req to know if I need or not to GET a fresh manifest. This helps to reduce unnecessary calls to the GET manifest endpoint.

I understand that my implementation might be outside the scope of this library so don't hesitate to tell me if this is the case.

That’s not something the library currently can do, OTOH there’s a clear benefit to sharing the auth/token/… infrastructure with c/image/docker. I’m interested in enabling this, although not at any cost.

Adding methods to an interface like ImageSource breaks API (external implementations of that interface); it’s still possible to build another way (although we don’t have a settled convention), with more code.

Primarily it’s not obvious to me that this generalizes — basically this is an optimization for a single server on the internet providing two operations that are ~exactly as expensive but one is by policy rate-limited, whereas most other servers and most other ImageTransport implementations really don’t care either way and could just as well provide the GetManifest data.

---

At this point it seems more natural to me to say “this is a very Docker-specific”, so I’d prefer to provide this as a Docker-specific API integrating with the Docker auth/token infrastructure but not with ImageTransport (like GetRepositoryTags and SearchRegistry): c/image/docker.GetDigest(context.Context, *types.SystemContext, types.ImageReference) or so. If we find out later that this does generalize, we can always add it to ImageSource (modulo the API concerns) later.

OTOH that would require setting up a separate dockerClient = getting a new bearer token. Is that rate-limited as well? That would make this approach unviable. The FAQ seems to suggest it’s not rate-limited, at least.

@vrothberg RFC

[crazy-max]

@mtrmac

At this point it seems more natural to me to say “this is a very Docker-specific”, so I’d prefer to provide this as a Docker-specific API integrating with the Docker auth/token infrastructure but not with ImageTransport (like GetRepositoryTags and SearchRegistry): c/image/docker.GetDigest(context.Context, *types.SystemContext, types.ImageReference) or so. If we find out later that this does generalize, we can always add it to ImageSource (modulo the API concerns) later.

You're absolutely right, it's a Docker-specific case. I've made changes to reflect that without breaking the API. Let me know if it looks good to you.

[mtrmac]

ACK overall.

[crazy-max]

@mtrmac Done, thanks for your review.

[mtrmac]

Thanks! LGTM.

@vrothberg PTAL, I’d like a second pair of eyes on the API.

[vrothberg]

LGTM, nice work @crazy-max and @mtrmac