[RFC] Add GetManifestDigest that uses HTTP HEAD to check if manifest is cached about image HOT 9 CLOSED

not sure, that header is there for security and content verification - not to hack on it for caching, shouldn't you take care of the whole index/cache system instead of having the library doing this? (I'm not sure how you would do this, just thinking out loud)
@mtrmac wdyt

from image.

not sure, that header is there for security and content verification - not to hack on it for caching

docker/distribution seems to use in in their tag service [1] to get the descriptor.

shouldn't you take care of the whole index/cache system instead of having the library doing this?

That's an interesting question. In GrootFS we consume containers/image for the Docker registry interaction and implement the cache (storing the blobs in the filesystem) ourselves. We would be very happy to PR some of the caching behaviour (and thus share the maintenance cost) if you think it's worthy. But that would be more of a long term thing.

[1] https://github.com/docker/distribution/blob/master/registry/client/repository.go#L294

from image.

that header is there for security and content verification - not to hack on it for caching

It is really for caching and discovering random corruption during transfers; nothing prevents the registry from lying in this header, or from returning the expected header value but a non-matching manifest.

I am more concerned about https://github.com/docker/docker/blob/master/distribution/pull_v2.go#L341 :

// NOTE: not using TagService.Get, since it uses HEAD requests
// against the manifests endpoint, which are not supported by
// all registry versions.

though I suppose treating this as an optimization which might fail would work.

More importantly, have you actually measured, or do you have anything to suggest, that using HEAD to look up in the cache would be an improvement? My first-order intuition is that a roundtrip is a roundtrip is a roundtrip, at least with the manifest sizes between a few hundred and a few thousand bytes, so I would expect HEAD manifest to take about as much time as GET manifest; and HEAD+GET is definitely worse than just GET.

from image.

// NOTE: not using TagService.Get, since it uses HEAD requests
// against the manifests endpoint, which are not supported by
// all registry versions.

Interesting.

More importantly, have you actually measured, or do you have anything to suggest, that using HEAD to look up in the cache would be an improvement? My first-order intuition is that a roundtrip is a roundtrip is a roundtrip, at least with the manifest sizes between a few hundred and a few thousand bytes, so I would expect HEAD manifest to take about as much time as GET manifest; and HEAD+GET is definitely worse than just GET.

Correct, we do not expect a huge improvement there. We will probably try and investigate how much this would give us before making a PR...

from image.

It is really for caching and discovering random corruption during transfers; nothing prevents the registry from lying in this header, or from returning the expected header value but a non-matching manifest.

but it isn't documented as such https://github.com/docker/distribution/blob/master/docs/spec/api.md#digest-header - I'm not sure either though

from image.

It is really for caching and discovering random corruption during transfers; nothing prevents the registry from lying in this header, or from returning the expected header value but a non-matching manifest.

but it isn't documented as such https://github.com/docker/distribution/blob/master/docs/spec/api.md#digest-header - I'm not sure either though

That says

IMPORTANT: If a digest is used to fetch content, the client should use the same digest used to fetch the content to verify it. The header Docker-Content-Digest should not be trusted over the "local" digest.

from image.

Correct, we do not expect a huge improvement there. We will probably try and investigate how much this would give us before making a PR...

(Philosophically, I am not opposed to exposing this server-side operation if you need it; just because I don’t see the value for our uses doesn’t mean it is useless or shouldn’t be merged. Of course, if it is not something other users of containers/image are likely to use, adding tests for that functionality would help minimize the risk of unintentionally breaking the code.)

from image.

@glestaris @mtrmac @vrothberg What is the state of this issue. It is two years old, can we close it?

from image.

After (almost) three years, we can close it.

from image.