Merge pull request moby#39527 from thaJeztah/pull_platform_regression

Fix error handling of incorrect --platform values

Author: thaJeztah

errdefs/http_helpers.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net/http"
 
+	containerderrors "github.com/containerd/containerd/errdefs"
 	"github.com/docker/distribution/registry/api/errcode"
 	"github.com/sirupsen/logrus"
 	"google.golang.org/grpc/codes"
@@ -47,6 +48,10 @@ func GetHTTPErrorStatusCode(err error) int {
 		if statusCode != http.StatusInternalServerError {
 			return statusCode
 		}
+		statusCode = statusCodeFromContainerdError(err)
+		if statusCode != http.StatusInternalServerError {
+			return statusCode
+		}
 		statusCode = statusCodeFromDistributionError(err)
 		if statusCode != http.StatusInternalServerError {
 			return statusCode
@@ -163,3 +168,24 @@ func statusCodeFromDistributionError(err error) int {
 	}
 	return http.StatusInternalServerError
 }
+
+// statusCodeFromContainerdError returns status code for containerd errors when
+// consumed directly (not through gRPC)
+func statusCodeFromContainerdError(err error) int {
+	switch {
+	case containerderrors.IsInvalidArgument(err):
+		return http.StatusBadRequest
+	case containerderrors.IsNotFound(err):
+		return http.StatusNotFound
+	case containerderrors.IsAlreadyExists(err):
+		return http.StatusConflict
+	case containerderrors.IsFailedPrecondition(err):
+		return http.StatusPreconditionFailed
+	case containerderrors.IsUnavailable(err):
+		return http.StatusServiceUnavailable
+	case containerderrors.IsNotImplemented(err):
+		return http.StatusNotImplemented
+	default:
+		return http.StatusInternalServerError
+	}
+}

integration/build/build_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/api/types/versions"
+	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/internal/test/fakecontext"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"gotest.tools/assert"
@@ -562,6 +563,35 @@ func TestBuildPreserveOwnership(t *testing.T) {
 	}
 }
 
+func TestBuildPlatformInvalid(t *testing.T) {
+	skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.40"), "experimental in older versions")
+
+	ctx := context.Background()
+	defer setupTest(t)()
+
+	dockerfile := `FROM busybox
+`
+
+	buf := bytes.NewBuffer(nil)
+	w := tar.NewWriter(buf)
+	writeTarRecord(t, w, "Dockerfile", dockerfile)
+	err := w.Close()
+	assert.NilError(t, err)
+
+	apiclient := testEnv.APIClient()
+	_, err = apiclient.ImageBuild(ctx,
+		buf,
+		types.ImageBuildOptions{
+			Remove:      true,
+			ForceRemove: true,
+			Platform:    "foobar",
+		})
+
+	assert.Assert(t, err != nil)
+	assert.ErrorContains(t, err, "unknown operating system or architecture")
+	assert.Assert(t, errdefs.IsInvalidParameter(err))
+}
+
 func writeTarRecord(t *testing.T, w *tar.Writer, fn, contents string) {
 	err := w.WriteHeader(&tar.Header{
 		Name:     fn,

integration/image/pull_test.go

@@ -0,0 +1,24 @@
+package image
+
+import (
+	"context"
+	"testing"
+
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/versions"
+	"github.com/docker/docker/errdefs"
+	"gotest.tools/assert"
+	"gotest.tools/skip"
+)
+
+func TestImagePullPlatformInvalid(t *testing.T) {
+	skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.40"), "experimental in older versions")
+	defer setupTest(t)()
+	client := testEnv.APIClient()
+	ctx := context.Background()
+
+	_, err := client.ImagePull(ctx, "docker.io/library/hello-world:latest", types.ImagePullOptions{Platform: "foobar"})
+	assert.Assert(t, err != nil)
+	assert.ErrorContains(t, err, "unknown operating system or architecture")
+	assert.Assert(t, errdefs.IsInvalidParameter(err))
+}