Enhanced resolve image config #3007
Conversation
| dt, err := content.ReadBlob(ctx, cache, config) | ||
| if err != nil { | ||
| return "", nil, err | ||
| // NOTE: most of this logic is butchered from containerd's images.Manifest, |
There was a problem hiding this comment.
The large diff is mostly a result of this change ^
Importing this logic from images.Manifest is really not great, however, the API it provides is not usable to provide the raw []byte content of the manifest. I can't find an alternative, but maybe someone more familiar with the containerd API might know?
Signed-off-by: Justin Chadwell <me@jedevc.com>
Along with the response, as well as returning the digest of the root image object and the image configuration, we now additionally return the contents of the manifest and the index (if available). Signed-off-by: Justin Chadwell <me@jedevc.com>
jedevc
force-pushed
the
enhanced-resolve-image-config
branch
from
aed30b5
to
a76a8f1
Compare
| @@ -90,6 +90,8 @@ message ResolveImageConfigRequest { | |||
| message ResolveImageConfigResponse { | |||
| string Digest = 1 [(gogoproto.customtype) = "github.com/opencontainers/go-digest.Digest", (gogoproto.nullable) = false]; | |||
| bytes Config = 2; | |||
| bytes Manifest = 3; | |||
There was a problem hiding this comment.
Having these 2 levels of indirection in here doesn't really cover all the cases. There could be nested indexes, and in attestations PR you are adding descriptors for the attestation manifests etc that can't be accessed(and therefore copied by imagetools) with this method.
I wonder if instead we should just always return the root manifest(the one that digest is pointing to). Client can parse it and make more specific digest results next to resolve the internal objects. This would make Config value optional so maybe there should be a flag in request side that controls if empty config should be an error or not.
|
This has been made more flexible in #4563 and can be attempted again. I think my previous comment #3007 (review) still holds. Closing for now. |
This implements the first option from #2944 🎉
We extend
ResolveImageConfigResponseto include aManifestand anIndex, while leaving the previous fields unchanged.With this PR, we should be able to move the image resolution logic out of buildx imagetools, and into buildkit, which will allow respecting registry settings.