Skip to content
This repository has been archived by the owner on Oct 13, 2023. It is now read-only.

[19.03 backport] Fix error handling of incorrect --platform values #365

Conversation

thaJeztah
Copy link
Member

backport of moby#39527

Relates to docker/docker-py#2382 (comment)

errdefs: convert containerd errors to the correct status code

In situations where the containerd error is consumed directly
and not received over gRPC, errors were not translated.

Add regression tests for invalid platform status codes

Before we handled containerd errors, using an invalid platform produced a 500 status:

curl -v \
-X POST \
--unix-socket /var/run/docker.sock \
"http://localhost:2375/v1.40/images/create?fromImage=hello-world&platform=foobar&tag=latest" \
-H "Content-Type: application/json"
* Connected to localhost (docker.sock) port 80 (#0)
> POST /v1.40/images/create?fromImage=hello-world&platform=foobar&tag=latest HTTP/1.1
> Host: localhost:2375
> User-Agent: curl/7.54.0
> Accept: */*
> Content-Type: application/json
>
< HTTP/1.1 500 Internal Server Error
< Api-Version: 1.40
< Content-Length: 85
< Content-Type: application/json
< Date: Mon, 15 Jul 2019 15:25:44 GMT
< Docker-Experimental: true
< Ostype: linux
< Server: Docker/19.03.0-rc2 (linux)
<
{"message":"\"foobar\": unknown operating system or architecture: invalid argument"}

That problem is now fixed, and the API correctly returns a 4xx status:

curl -v \
-X POST \
--unix-socket /var/run/docker.sock \
"http://localhost:2375/v1.40/images/create?fromImage=hello-world&platform=foobar&tag=latest" \
-H "Content-Type: application/json"
* Connected to localhost (/var/run/docker.sock) port 80 (#0)
> POST /v1.40/images/create?fromImage=hello-world&platform=foobar&tag=latest HTTP/1.1
> Host: localhost:2375
> User-Agent: curl/7.52.1
> Accept: */*
> Content-Type: application/json
>
< HTTP/1.1 400 Bad Request
< Api-Version: 1.41
< Content-Type: application/json
< Docker-Experimental: true
< Ostype: linux
< Server: Docker/dev (linux)
< Date: Mon, 15 Jul 2019 15:13:42 GMT
< Content-Length: 85
<
{"message":"\"foobar\": unknown operating system or architecture: invalid argument"}
* Curl_http_done: called premature == 0

This patch adds tests to validate the behaviour

- How to verify it

make \
  DOCKER_GRAPHDRIVER=vfs \
  TESTDIRS='github.com/docker/docker/integration/image' \
  TESTFLAGS='-test.run ^TestImagePullPlatformInvalid$' \
  binary test-integration


make \
  DOCKER_GRAPHDRIVER=vfs \
  TESTDIRS='github.com/docker/docker/integration/build' \
  TESTFLAGS='-test.run ^TestBuildPlatformInvalid$' \
  binary test-integration

- Description for the changelog

* Fix `POST /images/create` returning a 500 status code when providing an incorrect platform option
* Fix `POST /build` returning a 500 status code when providing an incorrect platform option

In situations where the containerd error is consumed directly
and not received over gRPC, errors were not translated.

This patch converts containerd errors to the correct HTTP
status code.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 4a51621)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Before we handled containerd errors, using an invalid platform produced a 500 status:

```bash
curl -v \
  -X POST \
  --unix-socket /var/run/docker.sock \
  "http://localhost:2375/v1.40/images/create?fromImage=hello-world&platform=foobar&tag=latest" \
  -H "Content-Type: application/json"
```

```
* Connected to localhost (docker.sock) port 80 (#0)
> POST /v1.40/images/create?fromImage=hello-world&platform=foobar&tag=latest HTTP/1.1
> Host: localhost:2375
> User-Agent: curl/7.54.0
> Accept: */*
> Content-Type: application/json
>
< HTTP/1.1 500 Internal Server Error
< Api-Version: 1.40
< Content-Length: 85
< Content-Type: application/json
< Date: Mon, 15 Jul 2019 15:25:44 GMT
< Docker-Experimental: true
< Ostype: linux
< Server: Docker/19.03.0-rc2 (linux)
<
{"message":"\"foobar\": unknown operating system or architecture: invalid argument"}
```

That problem is now fixed, and the API correctly returns a 4xx status:

```bash
curl -v \
  -X POST \
  --unix-socket /var/run/docker.sock \
  "http://localhost:2375/v1.40/images/create?fromImage=hello-world&platform=foobar&tag=latest" \
  -H "Content-Type: application/json"
```

```
* Connected to localhost (/var/run/docker.sock) port 80 (#0)
> POST /v1.40/images/create?fromImage=hello-world&platform=foobar&tag=latest HTTP/1.1
> Host: localhost:2375
> User-Agent: curl/7.52.1
> Accept: */*
> Content-Type: application/json
>
< HTTP/1.1 400 Bad Request
< Api-Version: 1.41
< Content-Type: application/json
< Docker-Experimental: true
< Ostype: linux
< Server: Docker/dev (linux)
< Date: Mon, 15 Jul 2019 15:13:42 GMT
< Content-Length: 85
<
{"message":"\"foobar\": unknown operating system or architecture: invalid argument"}
* Curl_http_done: called premature == 0
```

This patch adds tests to validate the behaviour

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 9d1b4f5)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah added this to the 19.03.3 milestone Sep 16, 2019
Copy link

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

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

LGTM

@andrewhsu andrewhsu merged commit 80727e5 into docker-archive:19.03 Sep 19, 2019
@thaJeztah thaJeztah deleted the 19.03_backport_pull_platform_regression branch September 20, 2019 09:20
docker-jenkins pushed a commit that referenced this pull request May 19, 2021
Do not "Bind" docker "To" containerd (carry #365)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants