fix(containerd): Use img platform in exporter instead of strict host platform

[AliDatadog]

Description

This PR replaces the platform comparer from DefaultStrict to the platform currently available on the host.

Motivation

Trivy fails to export arm images running on arm64 hosts even though these images are used by running containers.

This issue can be reproduced by ctr which uses a similar underlying export function.

# image:tag is an `arm` image
ctr -n k8s.io i export test.tar image:tag --platform linux/arm64 # DefaultStrict() will only match linux/arm64
ctr: no manifest found for platform: not found
ctr -n k8s.io i export test.tar image:tag --platform linux/arm
# works

The only way to retrieve the correct platform is to deserialize the image config or the spec.

Additional notes

Scan will most likely fail if a host pulls a compatible version of a multi-arch image if this version is not preferred.
For example, if a amd64 host pulls the i386 version of alpine which support both amd64 and i386.

Checklist

• I've read the guidelines for contributing to this repository.
• I've followed the conventions in the PR title.
• I've added tests that prove my fix is effective or that my feature works.
• I've updated the documentation with the relevant information (if needed).
• I've added usage information (if the PR introduces new options)
• I've included a "before" and "after" example to the description (if the PR is a user interface change).

[DmitriyLewen]

Hello @AliDatadog
Thanks for your work!

I left a comment.

[DmitriyLewen]

Thanks for work!
LGTM.

@knqyf263 Can you take a look and merge this PR if that's ok for you?

[AliDatadog]

@knqyf263 any update ?

[knqyf263]

If I correctly get your point, it still doesn't work in my environment.

$ go env GOOS GOARCH
linux
amd64

$ ctr images pull docker.io/library/alpine:3.17 --platform linux/arm
$ ctr i export test.tar docker.io/library/alpine:3.17
ctr: content digest sha256:81a4480db344455c6e0dbbca3188962273b5eb5c22efa3fef05824de6048bfa8: not found
$ ctr i export test.tar docker.io/library/alpine:3.17 --platform linux/arm

$ ctr images ls | grep 3.17
ERRO[0000] failed calculating size for image docker.io/library/alpine:3.17  error="content digest sha256:81a4480db344455c6e0dbbca3188962273b5eb5c22efa3fef05824de6048bfa8: not found"
docker.io/library/alpine:3.17                     application/vnd.docker.distribution.manifest.list.v2+json sha256:e95676db9e4a4f16f6cc01a8915368f82b018cc07aba951c1bd1db586c081388 2.1 KiB  linux/386,linux/amd64,linux/arm/v6,linux/arm/v7,linux/arm64/v8,linux/ppc64le,linux/s390x                -

$ trivy image image --platform linux/arm --image-src containerd docker.io/library/alpine:3.17
...
2023-07-06T12:27:38.484Z        FATAL   image scan error: scan error: unable to initialize a scanner: unable to initialize a docker scanner: 1 error occurred:
        * inspect error: content digest sha256:81a4480db344455c6e0dbbca3188962273b5eb5c22efa3fef05824de6048bfa8: not found

Looks like we also need to pass the platform to the client.

trivy/pkg/fanal/image/daemon/containerd.go

Lines 120 to 123 in cb02f4f

	client, err := containerd.New(addr)
	if err != nil {
	return nil, cleanup, xerrors.Errorf("failed to initialize a containerd client: %w", err)
	}

Otherwise, img.Config will fail because the platform won't match.

trivy/pkg/fanal/image/daemon/containerd.go

Line 212 in cb02f4f

configDesc, err := img.Config(ctx) // aware of img.platform

The image platform inherits from the client's platform.
https://github.com/containerd/containerd/blob/d5ec7286aed5b09fa4e28ae3232e250d4db73767/image.go#L123

Please let me know if I'm missing something.

Also, I'm confused about the desired behavior. As shown above, ctr even doesn't work with the wrong platform. I think Trivy should work in the same way as ctr. So, do we need this logic?

trivy/pkg/fanal/image/daemon/containerd.go

Lines 58 to 71 in cb02f4f

	manifest, err := images.Manifest(ctx, img.ContentStore(), img.Target(), img.Platform())
	if err != nil {
	return nil, xerrors.Errorf("error getting image manifest: %w", err)
	}
	ps, err := images.Platforms(ctx, img.ContentStore(), manifest.Config)
	if err != nil {
	return nil, xerrors.Errorf("error getting image platforms: %w", err)
	}
	if len(ps) == 0 {
	return nil, xerrors.Errorf("no platform for image %s", img.Name())
	}
	return platforms.OnlyStrict(ps[0]), nil

Scan will most likely fail if a host pulls a compatible version of a multi-arch image if this version is not preferred.

As you said, it could bring complex behavior.

I want to keep it simple.

• By default (no --platform): try to get the image to match the host platform
  • e.g. Run trivy image alpine:3.17 on the linux/amd64 host
    • alpine:3.17 with linux/amd64 exists -> success
    • alpine:3.17 with linux/amd64 doesn't exist -> fail
• With --platform: try to get the image with the specified platform
  • e.g. Run trivy image alpine:3.17 --platform linux/arm on the linux/amd64 host
    • alpine:3.17 with linux/arm exists -> success
    • alpine:3.17 with linux/arm doesn't exist -> fail

In other words, the default value of --platform is $GOOS/$GOARCH. What do you think?

[AliDatadog]

I think my description was a little misleading. alpine:3.17 is a multi-arch image with a platform that strictly matches your host's architecture and other compatible platforms. I think it's normal to keep the current behavior here (try to strictly match the host arch).

For a mono-arch image or a multi-arch image that does not have a version strictly matching the host's platform (for example mono-arch linux/arm while the host is linux/arm64), I think we should fall back on the latest compatible platform. What do you think about this ?

Otherwise, img.Config will fail because the platform won't match.

Also, nice catch! I will push a commit to include the image platform to the client (or the image actually, I think there is a function for that).

[knqyf263]

I think we should fall back on the latest compatible platform.

How does ctr work? I'd follow their behavior.

[AliDatadog]

I think we should fall back on the latest compatible platform.

How does ctr work? I'd follow their behavior.

ctr used to work in these cases.

root@ali-cluster-worker:/# uname -a
Linux ali-cluster-worker 5.15.49-linuxkit #1 SMP PREEMPT Tue Sep 13 07:51:32 UTC 2022 aarch64 GNU/Linux

root@ali-cluster-worker:/# ctr version
Client:
  Version:  v1.7.1
  Revision: 1677a17964311325ed1c31e2c0a3589ce6d5c30d
  Go version: go1.20.4

Server:
  Version:  v1.7.1
  Revision: 1677a17964311325ed1c31e2c0a3589ce6d5c30d
  UUID: 0fa84410-c3b1-4123-87bb-5142c1cfa405

root@ali-cluster-worker:/# ctr -n k8s.io i pull docker.io/arm32v7/ubuntu
ctr: failed to resolve reference "docker.io/arm32v7/ubuntu": object required

root@ali-cluster-worker:/# ctr -n k8s.io i pull docker.io/arm32v7/ubuntu:latest
docker.io/arm32v7/ubuntu:latest:                                                  resolved       |++++++++++++++++++++++++++++++++++++++|
manifest-sha256:f01654f07625f4a1d40eecfc528693bd734f6c52d9b982da9a963abe3d9de6c8: done           |++++++++++++++++++++++++++++++++++++++|
layer-sha256:579b5cf0e0f32d6ae38b8b84633c3075f3c12fbbadd8cef3ad028003eb456473:    done           |++++++++++++++++++++++++++++++++++++++|
config-sha256:c2055d043d36664323c87d819decb339cfbe483a4ea379e6ba7a19b72dbad842:   done           |++++++++++++++++++++++++++++++++++++++|
elapsed: 4.0 s                                                                    total:  24.0 M (6.0 MiB/s)
unpacking linux/arm64/v8 sha256:f01654f07625f4a1d40eecfc528693bd734f6c52d9b982da9a963abe3d9de6c8...
done: 398.569333ms

root@ali-cluster-worker:/# ctr -n k8s.io i export test.tar docker.io/arm32v7/ubuntu:latest

I think that since 1.16.x:

• ctr pull falls back to the best matching platform
• ctr run falls back to the best matching platform
• ctr export does not fall back to the best matching platform. See ctr images export doesn't work containerd/containerd#5895

While I think it's great to keep things simple, it could be insecure for users which have both arm32 and arm64 images on an arm64 host. Scans are still possible by simply specifying --platform linux/arm32 on these exceptions, but this approach assumes familiarity with container image workings. For automated tools, it also requires some logic modification (inspect the image, get the right platform, call with the right flag).

[knqyf263]

but this approach assumes familiarity with container image workings

I think we can assume people using containerd are familiar with multi-arch images. We don't have to do something different from ctr.
I'd suggest putting this discussion aside for now and focusing on the fix of the --platform support on containerd to move this PR forward. Then, we can come back to the discussion.

[knqyf263]

LGTM🚀