containerd / containerd

An open and reliable container runtime
https://containerd.io
Apache License 2.0
17.32k stars 3.43k forks source link

Pulling images uses Content-Type from the wrong response #7390

Open lgalfaso opened 2 years ago

lgalfaso commented 2 years ago

Description

When pulling a manifest, there are two request/response. The first one is a HEAD in order to know whether the manifest exists, the second is a GET to retrieve it. The spec mandates that the GET operation returns the Content-Type of the manifest, but there is no such requirement from the HEAD request.

https://github.com/distribution/distribution/blob/5cb406d511b7b9163bff9b6439072e4892e5ae3b/docs/spec/api.md#existing-manifests

Currently if the response from the HEAD request does not contain a Content-Type, then the pull fails.

Steps to reproduce the issue

1. 2. 3.

Describe the results you received and expected

The value of MediaType should be calculated from the response from the GET request, and not from the HEAD request.

What version of containerd are you using?

1.6.6

Any other relevant information

Using Minikube v1.26.1

Click to check the information ``` $ runc --version runc version 1.1.2 commit: a916309fff0f838eb94e928713dbc3c0d0ac7aa4 spec: 1.0.2-dev go: go1.18.3 libseccomp: 2.4.4 ``` ``` $ crictl info { "status": { "conditions": [ { "type": "RuntimeReady", "status": true, "reason": "", "message": "" }, { "type": "NetworkReady", "status": true, "reason": "", "message": "" } ] }, "cniconfig": { "PluginDirs": [ "/opt/cni/bin" ], "PluginConfDir": "/etc/cni/net.d", "PluginMaxConfNum": 1, "Prefix": "eth", "Networks": [ { "Config": { "Name": "cni-loopback", "CNIVersion": "0.3.1", "Plugins": [ { "Network": { "type": "loopback", "ipam": {}, "dns": {} }, "Source": "{\"type\":\"loopback\"}" } ], "Source": "{\n\"cniVersion\": \"0.3.1\",\n\"name\": \"cni-loopback\",\n\"plugins\": [{\n \"type\": \"loopback\"\n}]\n}" }, "IFName": "lo" }, { "Config": { "Name": "bridge", "CNIVersion": "0.3.1", "Plugins": [ { "Network": { "type": "bridge", "ipam": { "type": "host-local" }, "dns": {} }, "Source": "{\"addIf\":\"true\",\"bridge\":\"bridge\",\"forceAddress\":false,\"hairpinMode\":true,\"ipMasq\":true,\"ipam\":{\"subnet\":\"10.244.0.0/16\",\"type\":\"host-local\"},\"isDefaultGateway\":true,\"type\":\"bridge\"}" }, { "Network": { "type": "portmap", "capabilities": { "portMappings": true }, "ipam": {}, "dns": {} }, "Source": "{\"capabilities\":{\"portMappings\":true},\"type\":\"portmap\"}" } ], "Source": "\n{\n \"cniVersion\": \"0.3.1\",\n \"name\": \"bridge\",\n \"plugins\": [\n {\n \"type\": \"bridge\",\n \"bridge\": \"bridge\",\n \"addIf\": \"true\",\n \"isDefaultGateway\": true,\n \"forceAddress\": false,\n \"ipMasq\": true,\n \"hairpinMode\": true,\n \"ipam\": {\n \"type\": \"host-local\",\n \"subnet\": \"10.244.0.0/16\"\n }\n },\n {\n \"type\": \"portmap\",\n \"capabilities\": {\n \"portMappings\": true\n }\n }\n ]\n}\n" }, "IFName": "eth0" } ] }, "config": { "containerd": { "snapshotter": "overlayfs", "defaultRuntimeName": "runc", "defaultRuntime": { "runtimeType": "", "runtimePath": "", "runtimeEngine": "", "PodAnnotations": null, "ContainerAnnotations": null, "runtimeRoot": "", "options": null, "privileged_without_host_devices": false, "baseRuntimeSpec": "", "cniConfDir": "", "cniMaxConfNum": 0 }, "untrustedWorkloadRuntime": { "runtimeType": "", "runtimePath": "", "runtimeEngine": "", "PodAnnotations": null, "ContainerAnnotations": null, "runtimeRoot": "", "options": null, "privileged_without_host_devices": false, "baseRuntimeSpec": "", "cniConfDir": "", "cniMaxConfNum": 0 }, "runtimes": { "runc": { "runtimeType": "io.containerd.runc.v2", "runtimePath": "", "runtimeEngine": "", "PodAnnotations": null, "ContainerAnnotations": null, "runtimeRoot": "", "options": { "BinaryName": "", "CriuImagePath": "", "CriuPath": "", "CriuWorkPath": "", "IoGid": 0, "IoUid": 0, "NoNewKeyring": false, "NoPivotRoot": false, "Root": "", "ShimCgroup": "", "SystemdCgroup": false }, "privileged_without_host_devices": false, "baseRuntimeSpec": "", "cniConfDir": "", "cniMaxConfNum": 0 } }, "noPivot": false, "disableSnapshotAnnotations": true, "discardUnpackedLayers": false, "ignoreRdtNotEnabledErrors": false }, "cni": { "binDir": "/opt/cni/bin", "confDir": "/etc/cni/net.d", "maxConfNum": 1, "confTemplate": "", "ipPref": "" }, "registry": { "configPath": "", "mirrors": { "lmirelmann.local:8080": { "endpoint": [ "http://lmirelmann.local:8080" ] } }, "configs": null, "auths": null, "headers": null }, "imageDecryption": { "keyModel": "node" }, "disableTCPService": true, "streamServerAddress": "127.0.0.1", "streamServerPort": "0", "streamIdleTimeout": "4h0m0s", "enableSelinux": false, "selinuxCategoryRange": 1024, "sandboxImage": "k8s.gcr.io/pause:3.6", "statsCollectPeriod": 10, "systemdCgroup": false, "enableTLSStreaming": false, "x509KeyPairStreaming": { "tlsCertFile": "", "tlsKeyFile": "" }, "maxContainerLogSize": 16384, "disableCgroup": false, "disableApparmor": false, "restrictOOMScoreAdj": false, "maxConcurrentDownloads": 3, "disableProcMount": false, "unsetSeccompProfile": "", "tolerateMissingHugetlbController": true, "disableHugetlbController": true, "device_ownership_from_security_context": false, "ignoreImageDefinedVolumes": false, "netnsMountsUnderStateDir": false, "enableUnprivilegedPorts": false, "enableUnprivilegedICMP": false, "containerdRootDir": "/mnt/vda1/var/lib/containerd", "containerdEndpoint": "/run/containerd/containerd.sock", "rootDir": "/mnt/vda1/var/lib/containerd/io.containerd.grpc.v1.cri", "stateDir": "/run/containerd/io.containerd.grpc.v1.cri" }, "golang": "go1.18.3", "lastCNILoadStatus": "OK", "lastCNILoadStatus.default": "OK" } ``` ``` $ uname -a Linux minikube 5.10.57 #1 SMP Sat Jul 16 03:51:15 UTC 2022 x86_64 GNU/Linux ``` ### Show configuration if it is related to CRI plugin. ``` version = 2 root = "/var/lib/containerd" state = "/run/containerd" oom_score = 0 imports = ["/etc/containerd/containerd.conf.d/02-containerd.conf"] [grpc] address = "/run/containerd/containerd.sock" uid = 0 gid = 0 max_recv_message_size = 16777216 max_send_message_size = 16777216 [debug] address = "" uid = 0 gid = 0 level = "" [metrics] address = "" grpc_histogram = false [cgroup] path = "" [plugins] [plugins."io.containerd.monitor.v1.cgroups"] no_prometheus = false [plugins."io.containerd.grpc.v1.cri"] stream_server_address = "" stream_server_port = "10010" enable_selinux = false sandbox_image = "k8s.gcr.io/pause:3.7" stats_collect_period = 10 enable_tls_streaming = false max_container_log_line_size = 16384 restrict_oom_score_adj = false [plugins."io.containerd.grpc.v1.cri".containerd] discard_unpacked_layers = true snapshotter = "overlayfs" [plugins."io.containerd.grpc.v1.cri".containerd.default_runtime] runtime_type = "io.containerd.runc.v2" [plugins."io.containerd.grpc.v1.cri".containerd.untrusted_workload_runtime] runtime_type = "" runtime_engine = "" runtime_root = "" [plugins."io.containerd.grpc.v1.cri".containerd.runtimes] [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc] runtime_type = "io.containerd.runc.v2" [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options] SystemdCgroup = false [plugins."io.containerd.grpc.v1.cri".cni] bin_dir = "/opt/cni/bin" conf_dir = "/etc/cni/net.d" conf_template = "" [plugins."io.containerd.grpc.v1.cri".registry] [plugins."io.containerd.grpc.v1.cri".registry.mirrors] [plugins."io.containerd.grpc.v1.cri".registry.mirrors."docker.io"] endpoint = ["https://registry-1.docker.io"] [plugins."io.containerd.service.v1.diff-service"] default = ["walking"] [plugins."io.containerd.gc.v1.scheduler"] pause_threshold = 0.02 deletion_threshold = 0 mutation_threshold = 100 schedule_delay = "0s" startup_delay = "100ms" ```
thaJeztah commented 2 years ago

Perhaps the spec should call this out explicitly, but the HTTP spec expects a HEAD to return the same headers as GET, with the exception of some headers that may require calculation (eg content length)

https://www.rfc-editor.org/rfc/rfc9110#name-head

Have you run into issues with a registy that doesn't return the same headers for HEAD as for GET?

lgalfaso commented 2 years ago

One more data point, both the HEAD and GET requests are made, at a later validation step, there is the check for Content-Type in the HEAD response.

fuweid commented 1 year ago

@lgalfaso do you have any update? Currently, we doesn't hit the error. Please let us know if you run into issues with specific registry.

lgalfaso commented 1 year ago

@fuweid this report does not come from an error any existing registy, but from an error seen when updating manifest to a new media type, in which the manifest itself does not define a mediaType.

fuweid commented 1 year ago

but from an error seen when updating manifest to a new media type, in which the manifest itself does not define a mediaType.

Would you share how to do that? push manifest without mediatype? I don't think it is valid.

lgalfaso commented 1 year ago

The spec states the following

Clients SHOULD set the Content-Type header to the type of the manifest being pushed. All manifests SHOULD include a mediaType field declaring the type of the manifest being pushed. If a manifest includes a mediaType field, clients MUST set the Content-Type header to the value specified by the mediaType field.

Ref: https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pushing-manifests

This means that mediaType is not mandatory, just that if this were present, then it must match the Content-Type.

fuweid commented 1 year ago

Sorry for the late reply. If you push a manifest without media-type, how can we handle it in the follow-up? And if there is a manifest without media-type, what kind of media-type will the GET response return? empty? or application/octet-stream.

lgalfaso commented 1 year ago

The spec states

In a successful response, the Content-Type header will indicate the type of the returned manifest.

https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pull

The use of the word "will" is a little ambiguous, but my interpretation is that this should be understood as "must".

fuweid commented 1 year ago

Sorry for the late reply.

For the issue you mentioned,

but from an error seen when updating manifest to a new media type, in which the manifest itself does not define a mediaType.

It works as expected. For the image pulling, we need to know what type the manifest is. If it is empty or unsupported, containerd will return error.

The use of the word "will" is a little ambiguous, but my interpretation is that this should be understood as "must".

The distribution-spec also applies to other artifacts, not just container image. Again, we need the media type of manifest for pulling target layer blobs. If it is empty, pulling action will fail.

For the HEAD request, currently we don't break any functionality. And unfortunately I didn't find the history about this. Maybe like what https://github.com/containerd/containerd/issues/7390#issuecomment-1242986635 said, it is just convention. Maybe we should file issue in distribution-spec repo to update the spec?

cc @thaJeztah @AkihiroSuda @dmcgowan

shk3 commented 3 weeks ago

We actually hit this with JFrog Artifactory.

We are referencing the image using its digest, and when containerd resolves the image by digest, it makes a HEAD call first to the manifest endpoint with failover to the blob endpoint. https://github.com/containerd/containerd/blob/3df2cc1a6ba18e1809786c6c9376f106a62bf91a/core/remotes/docker/resolver.go#L253-L265

The image registry has an inconsistent behavior for calls to the blobs endpoint for the manifest object. For HEAD calls, it returns 200 with content type application/octet-stream, while for GET calls, it returns 404... Nevertheless, this seems to be an undefined behavior in the OCI spec when someone calls the blobs endpoint for a manifest object.

Due to some random network congestion situation, when containerd tries to resolve an image, the first call to manifest object could actually fail, and containerd seems to then failover to blobs endpoint and caches the incorrect application/octet-stream as media type for the following image unpack attempts on the k8s node.

Besides the inconsistent behavior of the image registry between GET calls and HEAD calls on the blobs endpoint for manifest object digests, any reason why containerd has this failover path to call the blobs endpoint for a manifest when it's referenced by a digest?