From 1e65162d44f83d74e9802caf34848abed3eb403c Mon Sep 17 00:00:00 2001
From: "zuoxiu.jm" <291271447@qq.com>
Date: Wed, 28 Nov 2018 16:11:11 +0800
Subject: [PATCH] properly transform decoder error into status error

---
 .../apiserver/pkg/endpoints/handlers/BUILD    |  2 +
 .../apiserver/pkg/endpoints/handlers/rest.go  |  4 +-
 .../pkg/endpoints/handlers/rest_test.go       | 50 +++++++++++++++++++
 3 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD
index 20733ff384..85695ff863 100644
--- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD
+++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD
@@ -21,6 +21,7 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/apis/testapigroup/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library",
@@ -33,6 +34,7 @@ go_test(
         "//staging/src/k8s.io/apiserver/pkg/apis/example/v1:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library",
+        "//staging/src/k8s.io/client-go/kubernetes/scheme:go_default_library",
         "//vendor/github.com/evanphx/json-patch:go_default_library",
         "//vendor/github.com/google/gofuzz:go_default_library",
         "//vendor/k8s.io/utils/trace:go_default_library",
diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go
index 5b76cfa455..6adbf84955 100644
--- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go
+++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go
@@ -229,11 +229,11 @@ func finishRequest(timeout time.Duration, fn resultFunc) (result runtime.Object,
 	}
 }
 
-// transformDecodeError adds additional information when a decode fails.
+// transformDecodeError adds additional information into a bad-request api error when a decode fails.
 func transformDecodeError(typer runtime.ObjectTyper, baseErr error, into runtime.Object, gvk *schema.GroupVersionKind, body []byte) error {
 	objGVKs, _, err := typer.ObjectKinds(into)
 	if err != nil {
-		return err
+		return errors.NewBadRequest(err.Error())
 	}
 	objGVK := objGVKs[0]
 	if gvk != nil && len(gvk.Kind) > 0 {
diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go
index 1003de9d6f..2c9dab5b94 100644
--- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go
+++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go
@@ -31,6 +31,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	testapigroupv1 "k8s.io/apimachinery/pkg/apis/testapigroup/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
@@ -43,6 +44,7 @@ import (
 	examplev1 "k8s.io/apiserver/pkg/apis/example/v1"
 	"k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/rest"
+	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	utiltrace "k8s.io/utils/trace"
 )
 
@@ -950,3 +952,51 @@ func (f mutateObjectUpdateFunc) Handles(operation admission.Operation) bool {
 func (f mutateObjectUpdateFunc) Admit(a admission.Attributes, o admission.ObjectInterfaces) (err error) {
 	return f(a.GetObject(), a.GetOldObject())
 }
+
+func TestTransformDecodeErrorEnsuresBadRequestError(t *testing.T) {
+	testCases := []struct {
+		name             string
+		typer            runtime.ObjectTyper
+		decodedGVK       *schema.GroupVersionKind
+		decodeIntoObject runtime.Object
+		baseErr          error
+		expectedErr      error
+	}{
+		{
+			name:  "decoding normal objects fails and returns a bad-request error",
+			typer: clientgoscheme.Scheme,
+			decodedGVK: &schema.GroupVersionKind{
+				Group:   testapigroupv1.GroupName,
+				Version: "v1",
+				Kind:    "Carp",
+			},
+			decodeIntoObject: &testapigroupv1.Carp{}, // which client-go's scheme doesn't recognize
+			baseErr:          fmt.Errorf("plain error"),
+		},
+		{
+			name:             "decoding objects with unknown GVK fails and returns a bad-request error",
+			typer:            alwaysErrorTyper{},
+			decodedGVK:       nil,
+			decodeIntoObject: &testapigroupv1.Carp{}, // which client-go's scheme doesn't recognize
+			baseErr:          nil,
+		},
+	}
+	for _, testCase := range testCases {
+		err := transformDecodeError(testCase.typer, testCase.baseErr, testCase.decodeIntoObject, testCase.decodedGVK, []byte(``))
+		if apiStatus, ok := err.(apierrors.APIStatus); !ok || apiStatus.Status().Code != http.StatusBadRequest {
+			t.Errorf("expected bad request error but got: %v", err)
+		}
+	}
+}
+
+var _ runtime.ObjectTyper = alwaysErrorTyper{}
+
+type alwaysErrorTyper struct{}
+
+func (alwaysErrorTyper) ObjectKinds(runtime.Object) ([]schema.GroupVersionKind, bool, error) {
+	return nil, false, fmt.Errorf("always error")
+}
+
+func (alwaysErrorTyper) Recognizes(gvk schema.GroupVersionKind) bool {
+	return false
+}