Skip to content

c8d/load: Don't unpack pseudo images #45644

Merged
neersighted merged 5 commits intomoby:masterfrom
vvoland:c8d-load-unpack-attestation
Jun 2, 2023
Merged

c8d/load: Don't unpack pseudo images #45644
neersighted merged 5 commits intomoby:masterfrom
vvoland:c8d-load-unpack-attestation

Conversation

@vvoland
Copy link
Copy Markdown
Contributor

@vvoland vvoland commented May 29, 2023

- What I did

  • Introduced a better way to iterate over platform-specific manifests from an image.
  • Skip pseudo-images (this includes attestations) when unpacking images in docker load.

- How I did it
Introduced a function which walks over a containerd image and calls the provided handler on a containerd.Image wrapper, that acts against the platform-specific manifest, instead of the parent manifest list.

This is a more convenient way to interact with a multi platform image by containerd.Image interface when acting on a specific platform-manifest.

See individual commits for details.

- How to verify it

$ echo 'FROM alpine' | docker buildx build - --platform linux/amd64,linux/arm64 -t alp -o type=oci,dest=alp.tar
...
$ docker load -i alp.tar
Loaded image: alp

- Description for the changelog
containerd integration: Fix docker load failing when loading an image with attestations

- A picture of a cute animal (not mandatory but encouraged)

@vvoland vvoland added status/2-code-review impact/changelog area/images Image Distribution kind/bugfix PR's that fix bugs containerd-integration Issues and PRs related to containerd integration labels May 29, 2023
@vvoland vvoland added this to the 25.0.0 milestone May 29, 2023
@vvoland vvoland force-pushed the c8d-load-unpack-attestation branch from 9e3d5ea to 89e7770 Compare May 29, 2023 13:25
@vvoland vvoland force-pushed the c8d-load-unpack-attestation branch from 89e7770 to cd2dd3a Compare May 31, 2023 08:24
vvoland added 4 commits May 31, 2023 10:47
The default implementation of the containerd.Image interface provided by
the containerd operates on the parent index/manifest list of the image
and the platform matcher.

This isn't convenient when a specific manifest is already known and it's
redundant to search the whole index for a manifest that matches the
given platform matcher. It can also result in a different manifest
picked up than expected when multiple manifests with the same platform
are present.

This introduces a walkImageManifests which walks the provided image and
calls a handler with a ImageManifest, which is a simple wrapper that
implements containerd.Image interfaces and performs all containerd.Image
operations against a platform specific manifest instead of the root
manifest list/index.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Don't unpack image manifests which are not a real images that can't be
unpacked.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland force-pushed the c8d-load-unpack-attestation branch from cd2dd3a to 4d3238d Compare May 31, 2023 08:47
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Copy link
Copy Markdown
Member

@neersighted neersighted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This form LGTM; I think this is fine with a squash 👍

@thaJeztah
Copy link
Copy Markdown
Member

I think this is fine with a squash

I'm good with keeping them separate; it's slightly easier to follow the changes

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/images Image Distribution containerd-integration Issues and PRs related to containerd integration impact/changelog kind/bugfix PR's that fix bugs process/cherry-picked status/2-code-review

Projects

Development

Successfully merging this pull request may close these issues.

'docker image load' with containerd storage throws 'failed to get stream processor for application/vnd.in-toto+json' error

3 participants