From 089e209aad9f76e1da1aaa9bbdf0673bcec5725a Mon Sep 17 00:00:00 2001
From: Clayton Coleman <ccoleman@redhat.com>
Date: Wed, 15 Nov 2017 21:02:34 -0500
Subject: [PATCH] Guarantee that status errors have a kind set

Some errors are invoked outside of negotiation. These errors should
still have a kind and apiVersion (which is only set by structured
encoders, not always availabe). Ensure that all errors by default get a
status kind and version set.

Kubernetes-commit: a1e44fc69bf0faeb47e6d2ebfc2709bbc3f17221
---
 pkg/endpoints/apiserver_test.go                | 18 ++++++++++++++++++
 .../handlers/responsewriters/errors_test.go    | 10 +++++-----
 .../handlers/responsewriters/status.go         |  6 ++++++
 .../handlers/responsewriters/status_test.go    |  2 ++
 4 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/pkg/endpoints/apiserver_test.go b/pkg/endpoints/apiserver_test.go
index 4b8ee2988..11024bf5f 100644
--- a/pkg/endpoints/apiserver_test.go
+++ b/pkg/endpoints/apiserver_test.go
@@ -1922,6 +1922,15 @@ func TestGetTable(t *testing.T) {
 			if resp.StatusCode != test.statusCode {
 				t.Errorf("%d: unexpected response: %#v", i, resp)
 			}
+			obj, _, err := extractBodyObject(resp, unstructured.UnstructuredJSONScheme)
+			if err != nil {
+				t.Errorf("%d: unexpected body read error: %v", err)
+				continue
+			}
+			gvk := schema.GroupVersionKind{Version: "v1", Kind: "Status"}
+			if obj.GetObjectKind().GroupVersionKind() != gvk {
+				t.Errorf("%d: unexpected error body: %#v", obj)
+			}
 			continue
 		}
 		if resp.StatusCode != http.StatusOK {
@@ -2038,6 +2047,15 @@ func TestGetPartialObjectMetadata(t *testing.T) {
 			if resp.StatusCode != test.statusCode {
 				t.Errorf("%d: unexpected response: %#v", i, resp)
 			}
+			obj, _, err := extractBodyObject(resp, unstructured.UnstructuredJSONScheme)
+			if err != nil {
+				t.Errorf("%d: unexpected body read error: %v", err)
+				continue
+			}
+			gvk := schema.GroupVersionKind{Version: "v1", Kind: "Status"}
+			if obj.GetObjectKind().GroupVersionKind() != gvk {
+				t.Errorf("%d: unexpected error body: %#v", obj)
+			}
 			continue
 		}
 		if resp.StatusCode != http.StatusOK {
diff --git a/pkg/endpoints/handlers/responsewriters/errors_test.go b/pkg/endpoints/handlers/responsewriters/errors_test.go
index 3496f6c8e..0dcf7adf9 100644
--- a/pkg/endpoints/handlers/responsewriters/errors_test.go
+++ b/pkg/endpoints/handlers/responsewriters/errors_test.go
@@ -68,15 +68,15 @@ func TestForbidden(t *testing.T) {
 		reason      string
 		contentType string
 	}{
-		{`{"metadata":{},"status":"Failure","message":"forbidden: User \"NAME\" cannot GET path \"/whatever\"","reason":"Forbidden","details":{},"code":403}
+		{`{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"forbidden: User \"NAME\" cannot GET path \"/whatever\"","reason":"Forbidden","details":{},"code":403}
 `, authorizer.AttributesRecord{User: u, Verb: "GET", Path: "/whatever"}, "", "application/json"},
-		{`{"metadata":{},"status":"Failure","message":"forbidden: User \"NAME\" cannot GET path \"/\u0026lt;script\u0026gt;\"","reason":"Forbidden","details":{},"code":403}
+		{`{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"forbidden: User \"NAME\" cannot GET path \"/\u0026lt;script\u0026gt;\"","reason":"Forbidden","details":{},"code":403}
 `, authorizer.AttributesRecord{User: u, Verb: "GET", Path: "/<script>"}, "", "application/json"},
-		{`{"metadata":{},"status":"Failure","message":"pod is forbidden: User \"NAME\" cannot GET pod at the cluster scope","reason":"Forbidden","details":{"kind":"pod"},"code":403}
+		{`{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"pod is forbidden: User \"NAME\" cannot GET pod at the cluster scope","reason":"Forbidden","details":{"kind":"pod"},"code":403}
 `, authorizer.AttributesRecord{User: u, Verb: "GET", Resource: "pod", ResourceRequest: true}, "", "application/json"},
-		{`{"metadata":{},"status":"Failure","message":"pod \"mypod\" is forbidden: User \"NAME\" cannot GET pod at the cluster scope","reason":"Forbidden","details":{"name":"mypod","kind":"pod"},"code":403}
+		{`{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"pod \"mypod\" is forbidden: User \"NAME\" cannot GET pod at the cluster scope","reason":"Forbidden","details":{"name":"mypod","kind":"pod"},"code":403}
 `, authorizer.AttributesRecord{User: u, Verb: "GET", Resource: "pod", ResourceRequest: true, Name: "mypod"}, "", "application/json"},
-		{`{"metadata":{},"status":"Failure","message":"pod.v2 is forbidden: User \"NAME\" cannot GET pod.v2/quota in the namespace \"test\"","reason":"Forbidden","details":{"group":"v2","kind":"pod"},"code":403}
+		{`{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"pod.v2 is forbidden: User \"NAME\" cannot GET pod.v2/quota in the namespace \"test\"","reason":"Forbidden","details":{"group":"v2","kind":"pod"},"code":403}
 `, authorizer.AttributesRecord{User: u, Verb: "GET", Namespace: "test", APIGroup: "v2", Resource: "pod", Subresource: "quota", ResourceRequest: true}, "", "application/json"},
 	}
 	for _, test := range cases {
diff --git a/pkg/endpoints/handlers/responsewriters/status.go b/pkg/endpoints/handlers/responsewriters/status.go
index 2f67cb6f9..e37a8ea47 100755
--- a/pkg/endpoints/handlers/responsewriters/status.go
+++ b/pkg/endpoints/handlers/responsewriters/status.go
@@ -46,6 +46,8 @@ func ErrorToAPIStatus(err error) *metav1.Status {
 				status.Code = http.StatusInternalServerError
 			}
 		}
+		status.Kind = "Status"
+		status.APIVersion = "v1"
 		//TODO: check for invalid responses
 		return &status
 	default:
@@ -61,6 +63,10 @@ func ErrorToAPIStatus(err error) *metav1.Status {
 		// cases.
 		runtime.HandleError(fmt.Errorf("apiserver received an error that is not an metav1.Status: %v", err))
 		return &metav1.Status{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Status",
+				APIVersion: "v1",
+			},
 			Status:  metav1.StatusFailure,
 			Code:    int32(status),
 			Reason:  metav1.StatusReasonUnknown,
diff --git a/pkg/endpoints/handlers/responsewriters/status_test.go b/pkg/endpoints/handlers/responsewriters/status_test.go
index 60168e24d..b4a8bf42c 100644
--- a/pkg/endpoints/handlers/responsewriters/status_test.go
+++ b/pkg/endpoints/handlers/responsewriters/status_test.go
@@ -65,6 +65,8 @@ func TestAPIStatus(t *testing.T) {
 	}
 	for k, v := range cases {
 		actual := ErrorToAPIStatus(k)
+		v.APIVersion = "v1"
+		v.Kind = "Status"
 		if !reflect.DeepEqual(actual, &v) {
 			t.Errorf("%s: Expected %#v, Got %#v", k, v, actual)
 		}